[RISCV] Support global address as inline asm memory operand of `m`

[wangpc]

Summary

In D146245, we have supported lowering inline asm m with offset
to register+imm, but we didn't handle the case that the offset
is the low part of global address.

This patch will emit %lo(g) when g is a global address.

Fixes #64656

[wangpc]

llvm/lib/Target/RISCV/RISCVAsmPrinter.cpp

246

Is this the right way to print this operand?

[wangpc]

Use getName().

[asb]

LGTM, though I'd appreciate an extra pair of eyes that there aren't other lurking problems that have been missed.

Could you please expand the test cases a bit? It seems worth testing e.g.

@ga1 = dso_local global [4000 x i32] zeroinitializer, align 4
...
  call void asm "sw zero, $0", "=*m"(ptr nonnull elementtype(i32) getelementptr inbounds ([400000 x i32], ptr @ga1, i32 0, i32 2000))

and likely a variant with a smaller offset too. Both seem to generate reasonable output to my eye.

[kito-cheng]

Confirmed this match GCC behavior, LGTM too,

[wangpc]

Add more tests.

[jrtc27]

medany tests for %pcrel_lo12 being handled correctly?

[wangpc]

In D157839#4587325, @jrtc27 wrote:

medany tests for %pcrel_lo12 being handled correctly?

For medany code model, TargetGlobalAddress will be lowered to LLA or LGA pseudos, and SelectAddrRegImm in SelectInlineAsmMemoryOperand will return the LLA or LGA and offset 0. The offset will always be 0, so I think it is handled correctly. And I just confirmed that GCC has the same behavior in medany code model.

[jrtc27]

In D157839#4587408, @wangpc wrote:

In D157839#4587325, @jrtc27 wrote:

medany tests for %pcrel_lo12 being handled correctly?

For medany code model, TargetGlobalAddress will be lowered to LLA or LGA pseudos, and SelectAddrRegImm in SelectInlineAsmMemoryOperand will return the LLA or LGA and offset 0. The offset will always be 0, so I think it is handled correctly. And I just confirmed that GCC has the same behavior in medany code model.

That's not what I asked. I asked for tests, so please add them. What you say may be true today, but the point of tests isn't just to demonstrate that the code today does what you think it does, but also to ensure that the code tomorrow continues to do so. Otherwise someone could come along and alter how medany TGAs get lowered and end up exposing them to PrintAsmMemoryOperand without realising due to having no test coverage of it.

I'll also note that we should be able to fold the %pcrel_lo12 into the inline assembly, even if we don't do so today.

llvm/lib/Target/RISCV/RISCVAsmPrinter.cpp

246

Should be not lower it to an MCOperand with lowerOperand (or lowerSymbolOperand if you want to keep the imm case separate) and let that do the printing rather than hand-rolling a subset of it? At least assert on the MachineOperand's target flags so we don't emit %lo for something that's not a lo.

[wangpc]

• Use lowerOperand()
• Test PIC code.

[wangpc]

Rename DispImm to Offset.

[wangpc]

Update tests.

[wangpc]

Rebase.

[asb]

I think all outstanding questions / requests were resolved, so marking as LGTM again.

llvm/lib/Target/RISCV/RISCVAsmPrinter.cpp

bool RISCVAsmPrinter::PrintAsmMemoryOperand(const MachineInstr *MI,
 
   const MachineOperand &AddrReg = MI->getOperand(OpNo);
   assert(MI->getNumOperands() > OpNo + 1 && "Expected additional operand");
   const MachineOperand &DispImm = MI->getOperand(OpNo + 1);
   // All memory operands should have a register and an immediate operand (see
   // RISCVDAGToDAGISel::SelectInlineAsmMemoryOperand).
   if (!AddrReg.isReg())
     return true;
-  if (!DispImm.isImm())
+  if (!DispImm.isImm() && !DispImm.isGlobal())
     return true;
 
-  OS << DispImm.getImm() << "("
-     << RISCVInstPrinter::getRegisterName(AddrReg.getReg()) << ")";
+  if (DispImm.isImm())
+    OS << DispImm.getImm();
+  else if (DispImm.isGlobal())
+    OS << "%lo(" << DispImm.getGlobal()->getGlobalIdentifier() << ")";
+  OS << "(" << RISCVInstPrinter::getRegisterName(AddrReg.getReg()) << ")";
   return false;
 }
 
 void RISCVAsmPrinter::emitDirectiveOptionArch() {
   RISCVTargetStreamer &RTS =
       static_cast<RISCVTargetStreamer &>(*OutStreamer->getTargetStreamer());
   SmallVector<RISCVOptionArchArg> NeedEmitStdOptionArgs;
   const MCSubtargetInfo &MCSTI = *TM.getMCSubtargetInfo();

llvm/test/CodeGen/RISCV/inline-asm.ll

 ; RV64I-NEXT:    lw a0, 4(a0)
 ; RV64I-NEXT:    #NO_APP
 ; RV64I-NEXT:    ret
   %1 = getelementptr i32, ptr %a, i32 1
   %2 = tail call i32 asm "lw $0, $1", "=r,*m"(ptr elementtype(i32) %1)
   ret i32 %2
 }
 
+define void @constraint_m_with_global() nounwind {
+; RV32I-LABEL: constraint_m_with_global:
+; RV32I:       # %bb.0:
+; RV32I-NEXT:    lui a0, %hi(gi)
+; RV32I-NEXT:    #APP
+; RV32I-NEXT:    sw zero, %lo(gi)(a0)
+; RV32I-NEXT:    #NO_APP
+; RV32I-NEXT:    ret
+;
+; RV64I-LABEL: constraint_m_with_global:
+; RV64I:       # %bb.0:
+; RV64I-NEXT:    lui a0, %hi(gi)
+; RV64I-NEXT:    #APP
+; RV64I-NEXT:    sw zero, %lo(gi)(a0)
+; RV64I-NEXT:    #NO_APP
+; RV64I-NEXT:    ret
+  call void asm "sw zero, $0", "=*m"(ptr elementtype(i32) @gi)
+  ret void
+}
+
 define void @constraint_o(ptr %a) nounwind {
 ; RV32I-LABEL: constraint_o:
 ; RV32I:       # %bb.0:
 ; RV32I-NEXT:    #APP
 ; RV32I-NEXT:    #NO_APP
 ; RV32I-NEXT:    ret
 ;
 ; RV64I-LABEL: constraint_o: