[RISCV] Add option to disable ASM compress. #131670
Conversation
|
@llvm/pr-subscribers-mc @llvm/pr-subscribers-backend-risc-v Author: Francesco Petrogalli (fpetrogalli) ChangesFull diff: https://github.com/llvm/llvm-project/pull/131670.diff 2 Files Affected:
diff --git a/llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp b/llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp
index e68674e830436..05378692cc9b4 100644
--- a/llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp
+++ b/llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp
@@ -3273,9 +3273,17 @@ bool RISCVAsmParser::parseDirectiveVariantCC() {
return false;
}
+static cl::opt<bool> RVDisableInlineAsmCompress(
+ "riscv-disable-inline-asm-compress",
+ cl::desc("disable compressing inline-asm instructions to their compress "
+ "counterpart."),
+ cl::init(false), cl::Hidden);
+
void RISCVAsmParser::emitToStreamer(MCStreamer &S, const MCInst &Inst) {
MCInst CInst;
- bool Res = RISCVRVC::compress(CInst, Inst, getSTI());
+ bool Res = false;
+ if (!RVDisableInlineAsmCompress)
+ Res = RISCVRVC::compress(CInst, Inst, getSTI());
if (Res)
++RISCVNumInstrsCompressed;
S.emitInstruction((Res ? CInst : Inst), getSTI());
diff --git a/llvm/test/MC/RISCV/disable-asm-compress.s b/llvm/test/MC/RISCV/disable-asm-compress.s
new file mode 100644
index 0000000000000..41e0ffcf9b588
--- /dev/null
+++ b/llvm/test/MC/RISCV/disable-asm-compress.s
@@ -0,0 +1,38 @@
+; NOTE: Assertions have been autogenerated by utils/update_mc_test_checks.py UTC_ARGS: --llvm-mc-binary bin/llvm-mc --version 5
+# RUN: llvm-mc -triple riscv32 -mattr=+c -assemble -riscv-no-aliases -show-encoding < %s | FileCheck %s --check-prefix=ENABLED
+# RUN: llvm-mc -triple riscv32 -mattr=+c -assemble -riscv-no-aliases -show-encoding -riscv-disable-inline-asm-compress < %s | FileCheck %s --check-prefix=DISABLED
+
+ addi sp, sp, -16
+// ENABLED: c.addi sp, -16 # encoding: [0x41,0x11]
+// DISABLED: addi sp, sp, -16 # encoding: [0x13,0x01,0x01,0xff]
+ sw ra, 12(sp)
+// ENABLED: c.swsp ra, 12(sp) # encoding: [0x06,0xc6]
+// DISABLED: sw ra, 12(sp) # encoding: [0x23,0x26,0x11,0x00]
+ sw s0, 8(sp)
+// ENABLED: c.swsp s0, 8(sp) # encoding: [0x22,0xc4]
+// DISABLED: sw s0, 8(sp) # encoding: [0x23,0x24,0x81,0x00]
+ addi s0, sp, 16
+// ENABLED: c.addi4spn s0, sp, 16 # encoding: [0x00,0x08]
+// DISABLED: addi s0, sp, 16 # encoding: [0x13,0x04,0x01,0x01]
+ li a0, 0
+// ENABLED: c.li a0, 0 # encoding: [0x01,0x45]
+// DISABLED: addi a0, zero, 0 # encoding: [0x13,0x05,0x00,0x00]
+ sw a0, -12(s0)
+// ENABLED: sw a0, -12(s0) # encoding: [0x23,0x2a,0xa4,0xfe]
+// DISABLED: sw a0, -12(s0) # encoding: [0x23,0x2a,0xa4,0xfe]
+ lw ra, 12(sp)
+// ENABLED: c.lwsp ra, 12(sp) # encoding: [0xb2,0x40]
+// DISABLED: lw ra, 12(sp) # encoding: [0x83,0x20,0xc1,0x00]
+ lw s0, 8(sp)
+// ENABLED: c.lwsp s0, 8(sp) # encoding: [0x22,0x44]
+// DISABLED: lw s0, 8(sp) # encoding: [0x03,0x24,0x81,0x00]
+ addi sp, sp, 16
+// ENABLED: c.addi sp, 16 # encoding: [0x41,0x01]
+// DISABLED: addi sp, sp, 16 # encoding: [0x13,0x01,0x01,0x01]
+ ret
+// ENABLED: c.jr ra # encoding: [0x82,0x80]
+// DISABLED: jalr zero, 0(ra) # encoding: [0x67,0x80,0x00,0x00]
+
+
+
+
|
|
Why do you need this as an option? There is no explanation in the PR. You can currently use |
Sure - it is needed for testing.
The only limitation with this option that we can not put compressed instruction between the: Could live with that by wrapping all non compressed instructions, but that would increase the verbosity of the assembly source. What would the option for "exact" mode do? Does it guarantee that something written as a non-compressed instruction in assembly, stays as a non-compressed instruction all the way down to the binary, event when targeting the C extension? |
In LLVM, or in e.g. external projects? If only the former, we can land this, under the understanding we can remove it when exact mode lands, which I'm aiming to be before 21.0.
I wrote a proposal here, which hasn't been accepted yet but seems to have broad consensus for the main parts (disabling compression and both branch and linker relaxation) riscv-non-isa/riscv-asm-manual#122 And I have an in-progress PR here which I need to get back to to sort out disabling linker relaxations, hopefully this week: #122483 I believe it covers your use-cases. The reason it also disables linker relaxation is to provide a better end-to-end guarantee which toolchain users (e.g. DV engineers trying to test a chip or simulator) can take advantage of. |
External projects, no need to temporarily land the option.
Your proposal is useful. I stated my support in there.
Indeed it does. How long will it take for the proposal to get ratified? I guess that requires more time than doing the actual implementation, right? |
I think it will be ratified after we have an LLVM implementation that works, rather than needing ratification to implement this in LLVM. The snag for ratification is we do need to ensure binutils can implement the proposal too, but I think someone is looking at that. |
|
@fpetrogalli |
|
Closed, it has been replaced by #122483 |
3 participants
No description provided.