[PATCH] D10659: Split BaseIndexOffset and MemOpLink into separate header

hfinkel at anl.gov hfinkel at anl.gov
Sun Jul 26 05:22:32 PDT 2015

hfinkel added a subscriber: hfinkel.

Comment at: include/llvm/CodeGen/BaseIndexOffset.h:12
@@ +11,3 @@
+namespace llvm {
Please include llvm/CodeGen/SelectionDAGNodes.h here.

Comment at: include/llvm/CodeGen/BaseIndexOffset.h:68
@@ +67,3 @@
+    //                   (i64 %element_size)))
+    if (Ptr->getOperand(1)->getOpcode() == ISD::MUL)
+      return BaseIndexOffset(Ptr, SDValue(), 0, IsIndexSignExt);
I don't understand this code. Why do we give up if the operand of the add is a multiply? The fact that this is special seems to encode some assumption about the order of visitation, and we may need to be careful about generalizing here.

Comment at: include/llvm/CodeGen/BaseIndexOffset.h:73
@@ +72,3 @@
+    SDValue Base = Ptr->getOperand(0);
+    SDValue IndexOffset = Ptr->getOperand(1);
Given that we have Base, Index and Offset, naming the Index here 'IndexOffset' is a bit confusing. Either just call it Index (and redefine it later if you can split off an offset), or call it IndexAndOffset.

Comment at: include/llvm/CodeGen/BaseIndexOffset.h:106
@@ +105,3 @@
+/// is located in a sequence of memory operations connected by a chain.
+struct MemOpLink {
+  MemOpLink (LSBaseSDNode *N, int64_t Offset, unsigned Seq):
Why is this here too?


More information about the llvm-commits mailing list