Skip to content
Merged
Show file tree
Hide file tree
Changes from 9 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
34 changes: 29 additions & 5 deletions llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3173,6 +3173,26 @@ bool RISCVAsmParser::parseDirectiveOption() {
return false;
}

if (Option == "exact") {
if (Parser.parseEOL())
return true;

getTargetStreamer().emitDirectiveOptionExact();
setFeatureBits(RISCV::FeatureExactAssembly, "exact-asm");
clearFeatureBits(RISCV::FeatureRelax, "relax");
return false;
}

if (Option == "noexact") {
if (Parser.parseEOL())
return true;

getTargetStreamer().emitDirectiveOptionNoExact();
clearFeatureBits(RISCV::FeatureExactAssembly, "exact-asm");
setFeatureBits(RISCV::FeatureRelax, "relax");
return false;
}

if (Option == "rvc") {
if (Parser.parseEOL())
return true;
Expand Down Expand Up @@ -3229,9 +3249,10 @@ bool RISCVAsmParser::parseDirectiveOption() {
}

// Unknown option.
Warning(Parser.getTok().getLoc(), "unknown option, expected 'push', 'pop', "
"'rvc', 'norvc', 'arch', 'relax' or "
"'norelax'");
Warning(Parser.getTok().getLoc(),
"unknown option, expected 'push', 'pop', "
"'rvc', 'norvc', 'arch', 'relax', 'norelax', "
"'exact' or 'noexact'");
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

while you're here. oxford comma

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Learnt, the community is a good place to learn new English phrases. :-)

Parser.eatToEndOfStatement();
return false;
}
Expand Down Expand Up @@ -3441,10 +3462,13 @@ bool RISCVAsmParser::parseDirectiveVariantCC() {

void RISCVAsmParser::emitToStreamer(MCStreamer &S, const MCInst &Inst) {
MCInst CInst;
bool Res = RISCVRVC::compress(CInst, Inst, getSTI());
bool Res = false;
const MCSubtargetInfo &STI = getSTI();
if (!STI.hasFeature(RISCV::FeatureExactAssembly))
Res = RISCVRVC::compress(CInst, Inst, STI);
if (Res)
++RISCVNumInstrsCompressed;
S.emitInstruction((Res ? CInst : Inst), getSTI());
S.emitInstruction((Res ? CInst : Inst), STI);
}

void RISCVAsmParser::emitLoadImm(MCRegister DestReg, int64_t Value,
Expand Down
10 changes: 10 additions & 0 deletions llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -344,6 +344,10 @@ std::pair<bool, bool> RISCVAsmBackend::relaxLEB128(const MCAssembler &Asm,
// Given a compressed control flow instruction this function returns
// the expanded instruction.
unsigned RISCVAsmBackend::getRelaxedOpcode(unsigned Op) const {
// Disable relaxation if FeatureExactAssembly
if (STI.hasFeature(RISCV::FeatureExactAssembly))
Copy link
Collaborator

@topperc topperc Mar 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a different STI than what mayNeedRelaxation is using right?

Should we check in relaxInstruction instead?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, this is indeed a different STI to mayNeedRelaxation.

I wasn't sure about changing relaxInstruction, because of the llvm_unreachable which suggested to me it would never be called if mayNeedRelaxation returned false, and I also wasn't sure what would happen if we never updated Inst. I can certainly try that approach, I agree the current check is not actually very good.

Copy link
Collaborator

@topperc topperc Mar 19, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure getRelaxedOpcode should be a member. It only has 2 callers, it's not virtual, and it doesn't use any members variables. It could be a file local static function.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I will do that refactoring.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done

return Op;

switch (Op) {
default:
return Op;
Expand Down Expand Up @@ -371,6 +375,12 @@ unsigned RISCVAsmBackend::getRelaxedOpcode(unsigned Op) const {

bool RISCVAsmBackend::mayNeedRelaxation(const MCInst &Inst,
const MCSubtargetInfo &STI) const {
// This function has access to two STIs, the member of the AsmBackend, and the
// one passed as an argument. The latter is more specific, so we query it for
// specific features.
if (STI.hasFeature(RISCV::FeatureExactAssembly))
return false;

return getRelaxedOpcode(Inst.getOpcode()) != Inst.getOpcode();
}

Expand Down
2 changes: 2 additions & 0 deletions llvm/lib/Target/RISCV/MCTargetDesc/RISCVELFStreamer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,8 @@ void RISCVTargetELFStreamer::emitDirectiveOptionPIC() {}
void RISCVTargetELFStreamer::emitDirectiveOptionNoPIC() {}
void RISCVTargetELFStreamer::emitDirectiveOptionRVC() {}
void RISCVTargetELFStreamer::emitDirectiveOptionNoRVC() {}
void RISCVTargetELFStreamer::emitDirectiveOptionExact() {}
void RISCVTargetELFStreamer::emitDirectiveOptionNoExact() {}
void RISCVTargetELFStreamer::emitDirectiveOptionRelax() {}
void RISCVTargetELFStreamer::emitDirectiveOptionNoRelax() {}

Expand Down
2 changes: 2 additions & 0 deletions llvm/lib/Target/RISCV/MCTargetDesc/RISCVELFStreamer.h
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,8 @@ class RISCVTargetELFStreamer : public RISCVTargetStreamer {
void emitDirectiveOptionNoPIC() override;
void emitDirectiveOptionRVC() override;
void emitDirectiveOptionNoRVC() override;
void emitDirectiveOptionExact() override;
void emitDirectiveOptionNoExact() override;
void emitDirectiveOptionRelax() override;
void emitDirectiveOptionNoRelax() override;
void emitDirectiveVariantCC(MCSymbol &Symbol) override;
Expand Down
10 changes: 10 additions & 0 deletions llvm/lib/Target/RISCV/MCTargetDesc/RISCVTargetStreamer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,8 @@ void RISCVTargetStreamer::emitDirectiveOptionPIC() {}
void RISCVTargetStreamer::emitDirectiveOptionNoPIC() {}
void RISCVTargetStreamer::emitDirectiveOptionRVC() {}
void RISCVTargetStreamer::emitDirectiveOptionNoRVC() {}
void RISCVTargetStreamer::emitDirectiveOptionExact() {}
void RISCVTargetStreamer::emitDirectiveOptionNoExact() {}
void RISCVTargetStreamer::emitDirectiveOptionRelax() {}
void RISCVTargetStreamer::emitDirectiveOptionNoRelax() {}
void RISCVTargetStreamer::emitDirectiveOptionArch(
Expand Down Expand Up @@ -125,6 +127,14 @@ void RISCVTargetAsmStreamer::emitDirectiveOptionNoRVC() {
OS << "\t.option\tnorvc\n";
}

void RISCVTargetAsmStreamer::emitDirectiveOptionExact() {
OS << "\t.option\texact\n";
}

void RISCVTargetAsmStreamer::emitDirectiveOptionNoExact() {
OS << "\t.option\tnoexact\n";
}

void RISCVTargetAsmStreamer::emitDirectiveOptionRelax() {
OS << "\t.option\trelax\n";
}
Expand Down
4 changes: 4 additions & 0 deletions llvm/lib/Target/RISCV/MCTargetDesc/RISCVTargetStreamer.h
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,8 @@ class RISCVTargetStreamer : public MCTargetStreamer {
virtual void emitDirectiveOptionNoPIC();
virtual void emitDirectiveOptionRVC();
virtual void emitDirectiveOptionNoRVC();
virtual void emitDirectiveOptionExact();
virtual void emitDirectiveOptionNoExact();
virtual void emitDirectiveOptionRelax();
virtual void emitDirectiveOptionNoRelax();
virtual void emitDirectiveOptionArch(ArrayRef<RISCVOptionArchArg> Args);
Expand Down Expand Up @@ -84,6 +86,8 @@ class RISCVTargetAsmStreamer : public RISCVTargetStreamer {
void emitDirectiveOptionNoPIC() override;
void emitDirectiveOptionRVC() override;
void emitDirectiveOptionNoRVC() override;
void emitDirectiveOptionExact() override;
void emitDirectiveOptionNoExact() override;
void emitDirectiveOptionRelax() override;
void emitDirectiveOptionNoRelax() override;
void emitDirectiveOptionArch(ArrayRef<RISCVOptionArchArg> Args) override;
Expand Down
4 changes: 3 additions & 1 deletion llvm/lib/Target/RISCV/RISCVAsmPrinter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -254,7 +254,9 @@ void RISCVAsmPrinter::LowerSTATEPOINT(MCStreamer &OutStreamer, StackMaps &SM,
bool RISCVAsmPrinter::EmitToStreamer(MCStreamer &S, const MCInst &Inst,
const MCSubtargetInfo &SubtargetInfo) {
MCInst CInst;
bool Res = RISCVRVC::compress(CInst, Inst, SubtargetInfo);
bool Res = false;
if (!SubtargetInfo.hasFeature(RISCV::FeatureExactAssembly))
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a test for this?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No. I think this would need a LLC test - let me work out what to do here - I'm not sure which EmitToStreamer is used for inline assembly.

I think I only want to keep this if inline assembly hits it, otherwise I don't want to present this as something for compilation - exact mode is trying to just be for assembly.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Inline assembly does not use this emit function, so I have removed this change.

Res = RISCVRVC::compress(CInst, Inst, SubtargetInfo);
if (Res)
++RISCVNumInstrsCompressed;
S.emitInstruction(Res ? CInst : Inst, SubtargetInfo);
Expand Down
4 changes: 4 additions & 0 deletions llvm/lib/Target/RISCV/RISCVFeatures.td
Original file line number Diff line number Diff line change
Expand Up @@ -1470,6 +1470,10 @@ def FeatureRelax
: SubtargetFeature<"relax", "EnableLinkerRelax", "true",
"Enable Linker relaxation.">;

def FeatureExactAssembly
: SubtargetFeature<"exact-asm", "EnableExactAssembly", "true",
"Enable Exact Assembly (Disables Compression and Relaxation)">;

foreach i = {1-31} in
def FeatureReserveX#i :
SubtargetFeature<"reserve-x"#i, "UserReservedRegister[RISCV::X"#i#"]",
Expand Down
72 changes: 72 additions & 0 deletions llvm/test/MC/RISCV/option-exact-compression.s
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
# RUN: llvm-mc -triple riscv32 -show-encoding -mattr=+c %s \
# RUN: | FileCheck -check-prefixes=CHECK,CHECK-ALIAS %s
# RUN: llvm-mc -triple riscv32 -show-encoding -mattr=+c \
# RUN: -M no-aliases %s | FileCheck -check-prefixes=CHECK,CHECK-INST %s
# RUN: llvm-mc -triple riscv32 -filetype=obj -mattr=+c %s \
# RUN: | llvm-objdump --triple=riscv32 --mattr=+c --no-print-imm-hex -d - \
# RUN: | FileCheck -check-prefixes=CHECK-BYTES,CHECK-ALIAS %s
# RUN: llvm-mc -triple riscv32 -filetype=obj -mattr=+c %s \
# RUN: | llvm-objdump --triple=riscv32 --mattr=+c --no-print-imm-hex -d -M no-aliases - \
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we need so many RUN lines? I think with no-aliases, we do not tests without -M no-aliases (the instruction aliases are the primary concerns of other tests, not this test)

Could this and optin-exact-relaxation.s be combined?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. Combined.

# RUN: | FileCheck -check-prefixes=CHECK-BYTES,CHECK-INST %s

# RUN: llvm-mc -triple riscv64 -show-encoding -mattr=+c %s \
# RUN: | FileCheck -check-prefixes=CHECK-ALIAS %s
# RUN: llvm-mc -triple riscv64 -show-encoding -mattr=+c \
# RUN: -M no-aliases %s | FileCheck -check-prefixes=CHECK-INST %s
# RUN: llvm-mc -triple riscv64 -filetype=obj -mattr=+c %s \
# RUN: | llvm-objdump --triple=riscv64 --mattr=+c --no-print-imm-hex -d - \
# RUN: | FileCheck -check-prefixes=CHECK-BYTES,CHECK-ALIAS %s
# RUN: llvm-mc -triple riscv64 -filetype=obj -mattr=+c %s \
# RUN: | llvm-objdump --triple=riscv64 --mattr=+c --no-print-imm-hex -d -M no-aliases - \
# RUN: | FileCheck -check-prefixes=CHECK-BYTES,CHECK-INST %s


## `.option exact` disables a variety of assembler behaviour:
## - automatic compression
## - branch relaxation (of short branches to longer equivalent sequences)
## - linker relaxation (emitting R_RISCV_RELAX)
## `.option noexact` enables these behaviours again. It is also the default.

## This test only checks the automatic compression part of this behaviour.

# CHECK-BYTES: 4108
# CHECK-INST: c.lw a0, 0(a0)
# CHECK-ALIAS: lw a0, 0(a0)
# CHECK: # encoding: [0x08,0x41]
lw a0, 0(a0)

# CHECK-BYTES: 4108
# CHECK-INST: c.lw a0, 0(a0)
# CHECK-ALIAS: lw a0, 0(a0)
# CHECK: # encoding: [0x08,0x41]
c.lw a0, 0(a0)

# CHECK: .option exact
.option exact

# CHECK-BYTES: 00052503
# CHECK-INST: lw a0, 0(a0)
# CHECK-ALIAS: lw a0, 0(a0)
# CHECK: # encoding: [0x03,0x25,0x05,0x00]
lw a0, 0(a0)

# CHECK-BYTES: 4108
# CHECK-INST: c.lw a0, 0(a0)
# CHECK-ALIAS: lw a0, 0(a0)
# CHECK: # encoding: [0x08,0x41]
c.lw a0, 0(a0)

# CHECK: .option noexact
.option noexact

# CHECK-BYTES: 4108
# CHECK-INST: c.lw a0, 0(a0)
# CHECK-ALIAS: lw a0, 0(a0)
# CHECK: # encoding: [0x08,0x41]
lw a0, 0(a0)

# CHECK-BYTES: 4108
# CHECK-INST: c.lw a0, 0(a0)
# CHECK-ALIAS: lw a0, 0(a0)
# CHECK: # encoding: [0x08,0x41]
c.lw a0, 0(a0)
74 changes: 74 additions & 0 deletions llvm/test/MC/RISCV/option-exact-relaxation.s
Original file line number Diff line number Diff line change
@@ -0,0 +1,74 @@
# RUN: llvm-mc -triple riscv32 -show-encoding -mattr=+relax %s \
# RUN: | FileCheck -check-prefixes=CHECK-ASM %s
# RUN: llvm-mc -triple riscv32 -filetype=obj -mattr=+relax %s \
# RUN: | llvm-objdump --triple=riscv32 --no-show-raw-insn -dr - \
# RUN: | FileCheck -check-prefixes=CHECK-OBJDUMP %s

# RUN: llvm-mc -triple riscv64 -show-encoding -mattr=+relax %s \
# RUN: | FileCheck -check-prefixes=CHECK-ASM %s
# RUN: llvm-mc -triple riscv64 -filetype=obj -mattr=+relax %s \
# RUN: | llvm-objdump --triple=riscv64 --no-show-raw-insn -dr - \
# RUN: | FileCheck -check-prefixes=CHECK-OBJDUMP %s

## `.option exact` disables a variety of assembler behaviour:
## - automatic compression
## - branch relaxation (of short branches to longer equivalent sequences)
## - linker relaxation (emitting R_RISCV_RELAX)
## `.option noexact` enables these behaviours again. It is also the default.

## This test only checks the branch and linker relaxation part of this behaviour.



# CHECK-ASM: call undefined
# CHECK-ASM-NEXT: fixup A - offset: 0, value: undefined, kind: fixup_riscv_call_plt
# CHECK-ASM-NEXT: fixup B - offset: 0, value: 0, kind: fixup_riscv_relax
# CHECK-OBJDUMP: auipc ra, 0x0
# CHECK-OBJDUMP-NEXT: R_RISCV_CALL_PLT undefined
# CHECK-OBJDUMP-NEXT: R_RISCV_RELAX *ABS*
# CHECK-OBJDUMP-NEXT: jalr ra
call undefined@plt

# CHECK-ASM: beq a0, a1, undefined
# CHECK-ASM-NEXT: fixup A - offset: 0, value: undefined, kind: fixup_riscv_branch
# CHECK-OBJDUMP: bne a0, a1, 0x10
# CHECK-OBJDUMP-NEXT: j 0xc
# CHECK-OBJDUMP-NEXT: R_RISCV_JAL undefined
beq a0, a1, undefined

# CHECK-ASM: .option exact
.option exact

# CHECK-ASM: call undefined
# CHECK-ASM-NEXT: fixup A - offset: 0, value: undefined, kind: fixup_riscv_call_plt
# CHECK-ASM-NOT: fixup_riscv_relax
# CHECK-OBJDUMP: auipc ra, 0x0
# CHECK-OBJDUMP-NEXT: R_RISCV_CALL_PLT undefined
# CHECK-OBJDUMP-NOT: R_RISCV_RELAX
# CHECK-OBJDUMP-NEXT: jalr ra
call undefined@plt

# CHECK-ASM: beq a0, a1, undefined
# CHECK-ASM-NEXT: fixup A - offset: 0, value: undefined, kind: fixup_riscv_branch
# CHECK-OBJDUMP: beq a0, a1, 0x18
# CHECK-OBJDUMP-NEXT: R_RISCV_BRANCH undefined
beq a0, a1, undefined

# CHECK-ASM: .option noexact
.option noexact

# CHECK-ASM: call undefined
# CHECK-ASM-NEXT: fixup A - offset: 0, value: undefined, kind: fixup_riscv_call_plt
# CHECK-ASM-NEXT: fixup B - offset: 0, value: 0, kind: fixup_riscv_relax
# CHECK-OBJDUMP: auipc ra, 0x0
# CHECK-OBJDUMP-NEXT: R_RISCV_CALL_PLT undefined
# CHECK-OBJDUMP-NEXT: R_RISCV_RELAX *ABS*
# CHECK-OBJDUMP-NEXT: jalr ra
call undefined@plt

# CHECK-ASM: beq a0, a1, undefined
# CHECK-ASM-NEXT: fixup A - offset: 0, value: undefined, kind: fixup_riscv_branch
# CHECK-OBJDUMP: bne a0, a1, 0x2c
# CHECK-OBJDUMP-NEXT: j 0x28
# CHECK-OBJDUMP-NEXT: R_RISCV_JAL undefined
beq a0, a1, undefined
2 changes: 1 addition & 1 deletion llvm/test/MC/RISCV/option-invalid.s
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@
# CHECK: :[[#@LINE+1]]:13: error: expected newline
.option rvc foo

# CHECK: :[[#@LINE+1]]:12: warning: unknown option, expected 'push', 'pop', 'rvc', 'norvc', 'arch', 'relax' or 'norelax'
# CHECK: :[[#@LINE+1]]:12: warning: unknown option, expected 'push', 'pop', 'rvc', 'norvc', 'arch', 'relax', 'norelax', 'exact' or 'noexact'
.option bar

# CHECK: :[[#@LINE+1]]:12: error: .option pop with no .option push
Expand Down
Loading