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
Paths
| Differential D157839
[RISCV] Support global address as inline asm memory operand of `m` ClosedPublic Authored by wangpc on Aug 14 2023, 1:06 AM.
Details Summary In D146245, we have supported lowering inline asm m with offset This patch will emit %lo(g) when g is a global address. Fixes #64656
Diff Detail
Unit TestsFailed
Event Timeline
Comment Actions 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. This revision is now accepted and ready to land.Aug 14 2023, 6:59 AM Comment Actions
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. Comment Actions
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.
wangpc added a parent revision: D157965: [RISCV][NFC] Move tests of inline asm memory constraints to separate file.Aug 15 2023, 4:49 AM wangpc added a child revision: D158062: [RISCV] Teach RISCVMergeBaseOffset to handle inline asm.Aug 16 2023, 2:53 AM Comment Actions I think all outstanding questions / requests were resolved, so marking as LGTM again. This revision was landed with ongoing or failed builds.Aug 21 2023, 4:00 AM Closed by commit rGdc60003ec8b2: [RISCV] Support global address as inline asm memory operand of `m` (authored by wangpc). · Explain Why This revision was automatically updated to reflect the committed changes.
Revision Contents
Diff 550308 llvm/lib/Target/RISCV/RISCVAsmPrinter.cpp
llvm/test/CodeGen/RISCV/inline-asm-mem-constraint.ll
|
Is this the right way to print this operand?