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

[Derek Schuff via Phabricator via llvm-commits]

dschuff added inline comments.


================
Comment at: llvm/test/CodeGen/WebAssembly/userstack.ll:336
+; global address, so we should fold the global address into the offset, but not
+; the frame offset.
 ; CHECK-LABEL: frame_offset_with_global_address
----------------
luke wrote:
> dschuff wrote:
> > so the result of this is that the frame offset will end up in a const or add expression (consumed by the i32.load8_u), and the global address will be folded?
> > Is this better than what we did before (i.e. the address in a const and the frame offset folded)?
> In this case the frame offset wasn't being folded, although I'm not sure if that is intentional or not. This is the codegen before the patch:
> 
> ```
> frame_offset_with_global_address:
> 	.functype	frame_offset_with_global_address () -> (i32)
> 	i32.const	$push0=, str
> 	global.get	$push5=, __stack_pointer
> 	i32.const	$push6=, 16
> 	i32.sub 	$push9=, $pop5, $pop6
> 	i32.const	$push7=, 12
> 	i32.add 	$push8=, $pop9, $pop7
> 	i32.add 	$push1=, $pop0, $pop8
> 	i32.load8_u	$push2=, 0($pop1)
> 	i32.const	$push3=, 67
> 	i32.and 	$push4=, $pop2, $pop3
> 	return	$pop4
> ```
> 
> With the patch the global address gets folded in which saves a `const` and an `add` instruction:
> 
> ```
> frame_offset_with_global_address:
> 	.functype	frame_offset_with_global_address () -> (i32)
> 	global.get	$push3=, __stack_pointer
> 	i32.const	$push4=, 16
> 	i32.sub 	$push7=, $pop3, $pop4
> 	i32.const	$push5=, 12
> 	i32.add 	$push6=, $pop7, $pop5
> 	i32.load8_u	$push1=, str($pop6)
> 	i32.const	$push0=, 67
> 	i32.and 	$push2=, $pop1, $pop0
> 	return	$pop2
> ```
> 
> 
Ah ok yeah this test was added in https://reviews.llvm.org/D90577 which fixed a bug in the case below the one you are modifying in this CL. So I guess what's happening now is that this IR is being ISel'ed differently and is ending up in the first case in eliminateFrameIndex instead of the second.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139645