[PATCH] D38278: [CGP] Make optimizeMemoryInst capable of handling multiple AddrModes

[John Brawn via Phabricator via llvm-commits]

john.brawn added inline comments.


================
Comment at: lib/CodeGen/CodeGenPrepare.cpp:2696
 
-  bool operator==(const ExtAddrMode& O) const {
-    return (BaseReg == O.BaseReg) && (ScaledReg == O.ScaledReg) &&
-           (BaseGV == O.BaseGV) && (BaseOffs == O.BaseOffs) &&
-           (HasBaseReg == O.HasBaseReg) && (Scale == O.Scale);
+  FieldName compare(const ExtAddrMode &other) {
+    unsigned Result = NoField;
----------------
skatkov wrote:
> Don't you want to check type compatibility also here?
> Say if type of base regs are different we can return Incompatible types here.
> 
> The similar for others...
> 
> Or we can do it as a separate patch...
Will do.


================
Comment at: lib/CodeGen/CodeGenPrepare.cpp:2720
+  bool isTrivial() {
+    return (BaseGV && BaseGV == OriginalValue) ||
+      (BaseReg && BaseReg == OriginalValue);
----------------
skatkov wrote:
> reames wrote:
> > Don't you also need to check that the offset and scale are zero?  If not, then I'm still confused by what you mean by "trivial".
> OriginalValue = gv + base + scale * Index + offset.
> 
> If OriginalValue == base => scale == 0, offset == 0, gv == 0
> 
> the same if OriginalValue == gv
Yes, scale and offset are expected to be zero in a trivial addrmode. I'll add a couple of asserts to make sure that's the case.


Repository:
  rL LLVM

https://reviews.llvm.org/D38278