[PATCH] Scalar/PHI code genration

Johannes Doerfert doerfert at cs.uni-saarland.de
Tue Feb 10 16:37:45 PST 2015


Initial comments.


================
Comment at: lib/CodeGen/BlockGenerators.cpp:321
@@ +320,3 @@
+      InstMapped = InstCopy;
+    }
+  }
----------------
grosser wrote:
> I do not understand why this condition (and the forwarding of the instructions in copyInstruction) is necessary. If I remove the full if (InstCopy) block, all test cases still pass for me. Is the BBMap not already updated when the statements are copied?
> 
> For example:
> 
> ```
>  // Compute NewStore before its insertion in BBMap to make the insertion
>  // deterministic.
> ​ BBMap[Store] = NewStore;
> ​ return NewStore;
> ```
During the development this looked different and when I changed it I assumed that if there is a inst copy there is a mapping, however for synthezisable instructions there is none (currently), the same holds for phi nodes (but then there is no inst copy either).

================
Comment at: lib/CodeGen/BlockGenerators.cpp:461
@@ +460,3 @@
+    Instruction *ScalarInst = MA->getAccessInstruction();
+    PHINode *ScalarBasePHI = dyn_cast<PHINode>(ScalarBase);
+    int ScalarBasePHIIdx =
----------------
grosser wrote:
> The non-PHI case is pretty straightforward, but it becomes unclear due to the mix of PHI and non-PHI code. My comments below try to point out how those code paths could be made more separate.
I will seperate them as far as possible.

http://reviews.llvm.org/D7513

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/






More information about the llvm-commits mailing list