Skip to content

[RISCV] Fix alignment when mixing rvc/norvc relax/norelax code #150159

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all 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
88 changes: 0 additions & 88 deletions lld/test/ELF/riscv-relax-align-rvc.s

This file was deleted.

22 changes: 22 additions & 0 deletions llvm/lib/Target/RISCV/AsmParser/RISCVAsmParser.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3177,6 +3177,17 @@ bool RISCVAsmParser::parseDirectiveOption() {
if (Parser.parseEOL())
return true;

auto &Streamer = getTargetStreamer().getStreamer();
auto *Assembler = Streamer.getAssemblerPtr();
auto *Section = Streamer.getCurrentSectionOnly();
// Update RVCEver status only if any instruction emitted.
if (Section->hasInstructions() && Assembler != nullptr &&
getSTI().hasFeature(RISCV::FeatureStdExtZca)) {
RISCVAsmBackend &MAB =
static_cast<RISCVAsmBackend &>(Assembler->getBackend());
MAB.setRVCEver(Section);
}

getTargetStreamer().emitDirectiveOptionNoRVC();
clearFeatureBits(RISCV::FeatureStdExtC, "c");
clearFeatureBits(RISCV::FeatureStdExtZca, "zca");
Expand Down Expand Up @@ -3214,6 +3225,17 @@ bool RISCVAsmParser::parseDirectiveOption() {
if (Parser.parseEOL())
return true;

auto &Streamer = getTargetStreamer().getStreamer();
auto *Assembler = Streamer.getAssemblerPtr();
auto *Section = Streamer.getCurrentSectionOnly();
// Update RelaxEver status only if any instruction emitted.
if (Assembler != nullptr && Section->hasInstructions() &&
getSTI().hasFeature(RISCV::FeatureRelax)) {
RISCVAsmBackend &MAB =
static_cast<RISCVAsmBackend &>(Assembler->getBackend());
MAB.setRelaxEver(Section);
}

getTargetStreamer().emitDirectiveOptionNoRelax();
clearFeatureBits(RISCV::FeatureRelax, "relax");
return false;
Expand Down
7 changes: 5 additions & 2 deletions llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -309,9 +309,12 @@ bool RISCVAsmBackend::relaxAlign(MCFragment &F, unsigned &Size) {
// Use default handling unless linker relaxation is enabled and the alignment
// is larger than the nop size.
const MCSubtargetInfo *STI = F.getSubtargetInfo();
if (!STI->hasFeature(RISCV::FeatureRelax))
if (!STI->hasFeature(RISCV::FeatureRelax) && !hasRelaxEver(F.getParent()))
return false;
unsigned MinNopLen = STI->hasFeature(RISCV::FeatureStdExtZca) ? 2 : 4;
unsigned MinNopLen =
hasRVCEver(F.getParent()) || STI->hasFeature(RISCV::FeatureStdExtZca)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Check STI->hasFeature(RISCV::FeatureStdExtZca) first?

? 2
: 4;
if (F.getAlignment() <= MinNopLen)
return false;

Expand Down
14 changes: 14 additions & 0 deletions llvm/lib/Target/RISCV/MCTargetDesc/RISCVAsmBackend.h
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,10 @@
#include "MCTargetDesc/RISCVBaseInfo.h"
#include "MCTargetDesc/RISCVFixupKinds.h"
#include "MCTargetDesc/RISCVMCTargetDesc.h"
#include "llvm/ADT/SmallPtrSet.h"
#include "llvm/ADT/StringMap.h"
#include "llvm/MC/MCAsmBackend.h"
#include "llvm/MC/MCSection.h"
#include "llvm/MC/MCSubtargetInfo.h"

namespace llvm {
Expand All @@ -33,11 +35,23 @@ class RISCVAsmBackend : public MCAsmBackend {

StringMap<MCSymbol *> VendorSymbols;

SmallPtrSet<const MCSection *, 8> RelaxEverSections;
SmallPtrSet<const MCSection *, 8> RVCEverSections;

public:
RISCVAsmBackend(const MCSubtargetInfo &STI, uint8_t OSABI, bool Is64Bit,
const MCTargetOptions &Options);
~RISCVAsmBackend() override = default;

void setRVCEver(const MCSection *Sec) { RVCEverSections.insert(Sec); }
bool hasRVCEver(const MCSection *Sec) const {
return RVCEverSections.contains(Sec);
}
void setRelaxEver(const MCSection *Sec) { RelaxEverSections.insert(Sec); }
bool hasRelaxEver(const MCSection *Sec) const {
return RelaxEverSections.contains(Sec);
}

std::optional<bool> evaluateFixup(const MCFragment &, MCFixup &, MCValue &,
uint64_t &) override;
bool addReloc(const MCFragment &, const MCFixup &, const MCValue &,
Expand Down
34 changes: 34 additions & 0 deletions llvm/test/MC/RISCV/align-option-norelax-norvc.s
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
# RUN: llvm-mc -filetype=obj -triple=riscv64 -mattr=+relax,+c,+m %s -o - | \
# RUN: llvm-objdump -dr --mattr=+c,+m - | FileCheck %s

.text
.option relax
.balign 4
.global _start
.type _start, @function
# This R_RISCV_ALIGN for .balign 4
# CHECK: R_RISCV_ALIGN *ABS*+0x2
_start:
lui a0, %hi(foo)
addi a0, a0, %lo(foo)
mul a0, a1, a4
.option push

.option norelax
.option norvc
# This R_RISCV_ALIGN for .balign 8, we should emit that even
# norelax is set, because the code before this point might relax,
# and size may changed, so that we need to align this again at linker
# time.
# Also padding pad should be +6 rather than +4 here, because we have enabled
# RVC before, and linker may relax instructions to RVC instructions,
# That will cause 4 byte padding might not be enough to fix the alignment.
# CHECK: R_RISCV_ALIGN *ABS*+0x6
.balign 8
SHOULD_ALIGN_8_HERE:
.word 0x12345678

.option pop

foo:
ret
30 changes: 30 additions & 0 deletions llvm/test/MC/RISCV/align-option-norelax.s
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
# RUN: llvm-mc -filetype=obj -triple=riscv64 -mattr=+relax,+c,+m %s -o - | \
# RUN: llvm-objdump -dr --mattr=+c,+m - | FileCheck %s

.text
.option relax
.balign 4
.global _start
.type _start, @function
# This R_RISCV_ALIGN for .balign 4
# CHECK: R_RISCV_ALIGN *ABS*+0x2
_start:
lui a0, %hi(foo)
addi a0, a0, %lo(foo)
mul a0, a1, a4
.option push

.option norelax
# This R_RISCV_ALIGN for .balign 8, we should emit that even
# norelax is set, because the code before this point might relax,
# and size may changed, so that we need to align this again at linker
# time.
# CHECK: R_RISCV_ALIGN *ABS*+0x6
.balign 8
SHOULD_ALIGN_8_HERE:
.word 0x12345678

.option pop

foo:
ret
Loading