Subzero: Produce actually correct code in --asm-verbose mode.

The "pnacl-sz --asm-verbose=1" mode annotates the asm output with physical register liveness information, including which registers are live at the beginning and end of each basic block, and which registers' live ranges end at each instruction.  Computing this information requires a final liveness analysis pass.  One of the side effects of liveness analysis is to remove dead instructions, which happens when the instruction's dest variable is not live and the instruction lacks important side effects.

In some cases, direct manipulation of physical registers was missing extra fakedef/fakeuse/etc., and as as result these instructions could be eliminated, leading to incorrect code.  Without --asm-verbose, these instructions were being created after the last run of liveness analysis, so they had no chance of being eliminated and everything was fine.  But with --asm-verbose, some instructions would be eliminated.

This CL fixes the omissions so that the resulting code is runnable.

An alternative would be to add a flag to liveness analysis directing it not to dead-code eliminate any more instructions.  However, it's better to get the liveness right in case future late-stage optimizations rely on it.

BUG= https://code.google.com/p/nativeclient/issues/detail?id=4135
TEST= pydir/szbuild_spec2k.py --filetype=asm -v --sz=--asm-verbose=1 --force
R=jvoung@chromium.org, kschimpf@google.com

Review URL: https://codereview.chromium.org/1113683002

diff --git a/src/IceInstX8632.cpp b/src/IceInstX8632.cpp
index 15e0249..2fbb370 100644 (file)
--- a/src/IceInstX8632.cpp
+++ b/src/IceInstX8632.cpp
@@ -318,7 +318,15 @@ InstX8632Fstp::InstX8632Fstp(Cfg *Func, Variable *Dest)
     : InstX8632(Func, InstX8632::Fstp, 0, Dest) {}
 
 InstX8632Pop::InstX8632Pop(Cfg *Func, Variable *Dest)
-    : InstX8632(Func, InstX8632::Pop, 0, Dest) {}
+    : InstX8632(Func, InstX8632::Pop, 0, Dest) {
+  // A pop instruction affects the stack pointer and so it should not
+  // be allowed to be automatically dead-code eliminated.  (The
+  // corresponding push instruction doesn't need this treatment
+  // because it has no dest variable and therefore won't be dead-code
+  // eliminated.)  This is needed for late-stage liveness analysis
+  // (e.g. asm-verbose mode).
+  HasSideEffects = true;
+}
 
 InstX8632Push::InstX8632Push(Cfg *Func, Variable *Source)
     : InstX8632(Func, InstX8632::Push, 1, nullptr) {

diff --git a/src/IceTargetLoweringX8632.cpp b/src/IceTargetLoweringX8632.cpp
index d4c4c54..82b0f7c 100644 (file)
--- a/src/IceTargetLoweringX8632.cpp
+++ b/src/IceTargetLoweringX8632.cpp
@@ -798,6 +798,9 @@ void TargetX8632::addProlog(CfgNode *Node) {
     Variable *esp = getPhysicalRegister(RegX8632::Reg_esp);
     _push(ebp);
     _mov(ebp, esp);
+    // Keep ebp live for late-stage liveness analysis
+    // (e.g. asm-verbose mode).
+    Context.insert(InstFakeUse::create(Func, ebp));
   }
 
   // Align the variables area. SpillAreaPaddingBytes is the size of
@@ -940,6 +943,10 @@ void TargetX8632::addEpilog(CfgNode *Node) {
   Variable *esp = getPhysicalRegister(RegX8632::Reg_esp);
   if (IsEbpBasedFrame) {
     Variable *ebp = getPhysicalRegister(RegX8632::Reg_ebp);
+    // For late-stage liveness analysis (e.g. asm-verbose mode),
+    // adding a fake use of esp before the assignment of esp=ebp keeps
+    // previous esp adjustments from being dead-code eliminated.
+    Context.insert(InstFakeUse::create(Func, esp));
     _mov(esp, ebp);
     _pop(ebp);
   } else {
@@ -4309,6 +4316,10 @@ void TargetX8632::lowerPhiAssignments(CfgNode *Node,
         RegNum = RegsForType.find_first();
         Preg = getPhysicalRegister(RegNum, Dest->getType());
         SpillLoc = Func->makeVariable(Dest->getType());
+        // Create a fake def of the physical register to avoid
+        // liveness inconsistency problems during late-stage liveness
+        // analysis (e.g. asm-verbose mode).
+        Context.insert(InstFakeDef::create(Func, Preg));
         if (IsVector)
           _movp(SpillLoc, Preg);
         else
@@ -4335,6 +4346,9 @@ void TargetX8632::lowerPhiAssignments(CfgNode *Node,
           _movp(Preg, SpillLoc);
         else
           _mov(Preg, SpillLoc);
+        // Create a fake use of the physical register to keep it live
+        // for late-stage liveness analysis (e.g. asm-verbose mode).
+        Context.insert(InstFakeUse::create(Func, Preg));
       }
     }
     // Update register availability before moving to the previous