[PATCH] D109419: [GlobalISel] Implement merging of stores of truncates.

[Jessica Paquette via Phabricator via llvm-commits]

paquette added inline comments.


================
Comment at: llvm/include/llvm/CodeGen/GlobalISel/CombinerHelper.h:83
+
+using BuildFnTy = std::function<void(MachineIRBuilder &)>;
+
----------------
Looks like this part isn't used in the change?


================
Comment at: llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp:3869
+  }
+  if (StoreMI.isAtomic() || StoreMI.isVolatile())
+    return false;
----------------
`isSimple`?


================
Comment at: llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp:3879
+  // 2) of an LSHR/ASHR of a single wide value.
+  //   a) by the appropriate shift to get the partial value stored.
+  // 3) where the offsets form either a little or big-endian sequence.
----------------
weird that there is no b) case


================
Comment at: llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp:3886
+  Register BaseReg;
+  // The store
+  int64_t LastOffset;
----------------
comment seems incorrect?


================
Comment at: llvm/lib/CodeGen/GlobalISel/CombinerHelper.cpp:3937
+    }
+    if (BaseReg != NewBaseReg)
+      break;
----------------
maybe not for this patch, but maybe we should have a specific register matcher?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D109419/new/

https://reviews.llvm.org/D109419