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

[hfinkel at anl.gov]

hfinkel added a subscriber: hfinkel.

================
Comment at: include/llvm/CodeGen/BaseIndexOffset.h:12
@@ +11,3 @@
+#define _LLVM_CODEGEN_BASEINDEXOFFSET_H_
+
+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?



http://reviews.llvm.org/D10659