[PATCH] D139415: [RFC][WebAssembly] Optimize GEPs

[Sam Parker via Phabricator via llvm-commits]

samparker added inline comments.


================
Comment at: llvm/lib/Target/WebAssembly/WebAssemblyOptimizeGEPs.cpp:138
+
+  // Addresses are currently hardcoded to use i32.
+  if (ST.hasAddr64()) {
----------------
tlively wrote:
> Would this be hard to change? Wasm64 is generally supported elsewhere in LLVM these days.
Okay, will do.


================
Comment at: llvm/lib/Target/WebAssembly/WebAssemblyOptimizeGEPs.cpp:166-167
+  Value *BasePtr = GEP->getPointerOperand();
+  unsigned Key = getBaseOffsetHash(BasePtr, RegOffset);
+  Candidates[Key].emplace_back(GEP, ImmOffset);
+  if (!BaseOffsetPairs.count(Key)) {
----------------
tlively wrote:
> Why use a hash value as the key? Wouldn't it be better to use the pair itself as the key?
Good point.


================
Comment at: llvm/test/CodeGen/WebAssembly/optimize-geps.ll:6
+
+define hidden void @one_dim(ptr nocapture noundef readonly %arg, ptr nocapture noundef readonly %arg1, ptr nocapture noundef writeonly %arg2) {
+; CHECK-LABEL: @one_dim(
----------------
tlively wrote:
> Would it be possible to have a smaller test case? It's hard to tell what to look at here.
Yeah, if I remove one array and move to i32 this should hopefully half the size.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139415