[WebAssembly] Don't fold frame offset for global addresses

[aheejin]

Summary

When machine instructions are in the form of

%0 = CONST_I32 @str
%1 = ADD_I32 %stack.0, %0
%2 = LOAD 0, 0, %1

In the ADD_I32 instruction, it is possible to fold it if %0 is a
CONST_I32 from an immediate number. But in this case it is a global
address, so we shouldn't do that. But we haven't checked if the operand
of ADD is an immediate so far. This fixes the problem. (The case
applies the same for ADD_I64 and CONST_I64 instructions.)

Fixes https://bugs.llvm.org/show_bug.cgi?id=47944.

Patch by Julien Jorge ([e-mail])

[aheejin]

• Fix comment wrapping

[dschuff]

I think in theory, we could actually keep this optimization if we had a (ULEB?) relocation type with an addend (I think R_WASM_MEMORY_ADDR_LEB would work, actually).
Then we'd emit a load with a reloc against its constant offset field, where the addend would be the frame offset and the symbol would be the operand (similarly to what LadPatImmOff does for ISel)?
Of course if nobody discovered this bug before now it might not be that worthwhile...

[aheejin]

I guess that optimization cannot be done here and should be done later? And there's also an existing TODO to generalize this optimization. Maybe we can do it as a part of the 'generalization' later? But I also doubt how worthwhile it will be, given that this bug was found first time 5 years after this part of the code was first written...

[dschuff]

oh, I guess you're referring to the TODO on line 97? Yeah I guess including the case where the operand is a GA instead of an immediate counts as "more cases"...

llvm/lib/Target/WebAssembly/WebAssemblyRegisterInfo.cpp

if (OtherMO.isReg()) {
         MachineInstr *Def = MF.getRegInfo().getUniqueVRegDef(OtherMOReg);
         // TODO: For now we just opportunistically do this in the case where
         // the CONST_I32/64 happens to have exactly one def and one use. We
         // should generalize this to optimize in more cases.
         if (Def && Def->getOpcode() ==
               WebAssemblyFrameLowering::getOpcConst(MF) &&
             MRI.hasOneNonDBGUse(Def->getOperand(0).getReg())) {
           MachineOperand &ImmMO = Def->getOperand(1);
+          if (ImmMO.isImm()) {
             ImmMO.setImm(ImmMO.getImm() + uint32_t(FrameOffset));
             MI.getOperand(FIOperandNum)
                 .ChangeToRegister(FrameRegister, /*isDef=*/false);
             return;
           }
         }
       }
     }
+  }
 
   // Otherwise create an i32/64.add SP, offset and make it the operand.
   const auto *TII = MF.getSubtarget<WebAssemblySubtarget>().getInstrInfo();
 
   unsigned FIRegOperand = FrameRegister;
   if (FrameOffset) {
     // Create i32/64.add SP, offset and make it the operand.
     const TargetRegisterClass *PtrRC =

llvm/test/CodeGen/WebAssembly/userstack.ll

 ; CHECK-NEXT:  # %{{[0-9]+}}{{$}}
 ; CHECK-NEXT:  #NO_APP
 define void @inline_asm() {
   %tmp = alloca i8
   call void asm sideeffect "# %0", "r"(i8* %tmp)
   ret void
 }
 
+; We optimize the format of "frame offset + operand" by folding it, but this is
+; only possible when that operand is an immediate. In this example it is a
+; global address, so we should not fold it.
+; CHECK-LABEL: frame_offset_with_global_address
+; CHECK: i[[PTR]].const ${{.*}}=, str
+@str = local_unnamed_addr global [3 x i8] c"abc", align 16
+define i8 @frame_offset_with_global_address() {
+  %1 = alloca i8, align 4
+  %2 = ptrtoint i8* %1 to i32
+  ;; Here @str is a global address and not an immediate, so cannot be folded
+  %3 = getelementptr [3 x i8], [3 x i8]* @str, i32 0, i32 %2
+  %4 = load i8, i8* %3, align 8
+  %5 = and i8 %4, 67
+  ret i8 %5
+}
+
 ; CHECK: .globaltype  __stack_pointer, i[[PTR]]{{$}}
 
 ; TODO: test over-aligned alloca