[PATCH] D139645: [WebAssembly] Fold adds with global addresses into load offset

Thu Dec 22 11:08:22 PST 2022

dschuff requested changes to this revision.
dschuff added a comment.
This revision now requires changes to proceed.

Sorry, I missed your comment  2 days ago.  See my comment below.

================
Comment at: llvm/test/CodeGen/WebAssembly/negative-base-reg.ll:20-22
+; CHECK:      local.get $push[[L1:[0-9]+]]=, 0
+; CHECK-NEXT: i32.const $push[[V:[0-9]+]]=, 1
+; CHECK-NEXT: i32.store args+128($pop[[L1]]), $pop[[V]]
----------------
luke wrote:
> @dschuff This got shuffled about, and now the global address gets folded into the store. But is the offset here still unfolded? Or by "offset" here is it also referring to the global address too
OK, so looking back at https://github.com/llvm/llvm-project/issues/29497 and D24053 this actually does look like the problem that we fixed back then.
IIRC the problem is that sometimes the address operand of the store (i.e. local 0, or L1 here) can have a negative value (here it's -128), but the store's effective address calculation (i.e. the operand plus the constant offset, L1 + args + 128 with this CL applied) is unsigned, so it will overflow. 
So we have to ensure that the calculation that recombines the native local value with the compile-time constant (here 128) happens with an add instruction rather than getting folded. Does that make sense?

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139645