Skip to content

[RISCV] Implement foward inserting save/restore FRM instructions. #77744

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

Merged
merged 7 commits into from
Jan 25, 2024
Merged
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
90 changes: 88 additions & 2 deletions llvm/lib/Target/RISCV/RISCVInsertReadWriteCSR.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,11 @@ using namespace llvm;
#define DEBUG_TYPE "riscv-insert-read-write-csr"
#define RISCV_INSERT_READ_WRITE_CSR_NAME "RISC-V Insert Read/Write CSR Pass"

static cl::opt<bool>
DisableFRMInsertOpt("riscv-disable-frm-insert-opt", cl::init(false),
cl::Hidden,
cl::desc("Disable optimized frm insertion."));

namespace {

class RISCVInsertReadWriteCSR : public MachineFunctionPass {
Expand All @@ -46,6 +51,7 @@ class RISCVInsertReadWriteCSR : public MachineFunctionPass {

private:
bool emitWriteRoundingMode(MachineBasicBlock &MBB);
bool emitWriteRoundingModeOpt(MachineBasicBlock &MBB);
};

} // end anonymous namespace
Expand All @@ -55,6 +61,82 @@ char RISCVInsertReadWriteCSR::ID = 0;
INITIALIZE_PASS(RISCVInsertReadWriteCSR, DEBUG_TYPE,
RISCV_INSERT_READ_WRITE_CSR_NAME, false, false)

// TODO: Use more accurate rounding mode at the start of MBB.
bool RISCVInsertReadWriteCSR::emitWriteRoundingModeOpt(MachineBasicBlock &MBB) {
bool Changed = false;
MachineInstr *LastFRMChanger = nullptr;
unsigned CurrentRM = RISCVFPRndMode::DYN;
Register SavedFRM;

for (MachineInstr &MI : MBB) {
if (MI.getOpcode() == RISCV::SwapFRMImm ||
MI.getOpcode() == RISCV::WriteFRMImm) {
CurrentRM = MI.getOperand(0).getImm();
SavedFRM = Register();
continue;
}

if (MI.getOpcode() == RISCV::WriteFRM) {
CurrentRM = RISCVFPRndMode::DYN;
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not sure here, but I think you may be confusing CurrentRM being DYN and CurrentRM being unknown. I don't know if it matters, but I think if I'm reading this right that DYN is supposed to be a "don't care" value, not an "unknown" value.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My idea is using DYN as the right rounding mode when executing MI. And I found that I don't need std::optional for CurrentRM, since we don't have variable frm instruction.

SavedFRM = Register();
continue;
}

if (MI.isCall() || MI.isInlineAsm() || MI.readsRegister(RISCV::FRM)) {
// Restore FRM before unknown operations.
if (SavedFRM.isValid())
BuildMI(MBB, MI, MI.getDebugLoc(), TII->get(RISCV::WriteFRM))
.addReg(SavedFRM);
CurrentRM = RISCVFPRndMode::DYN;
SavedFRM = Register();
continue;
}

assert(!MI.modifiesRegister(RISCV::FRM) &&
"Expected that MI could not modify FRM.");

int FRMIdx = RISCVII::getFRMOpNum(MI.getDesc());
if (FRMIdx < 0)
continue;
unsigned InstrRM = MI.getOperand(FRMIdx).getImm();

LastFRMChanger = &MI;

// Make MI implicit use FRM.
MI.addOperand(MachineOperand::CreateReg(RISCV::FRM, /*IsDef*/ false,
/*IsImp*/ true));
Changed = true;

// Skip if MI uses same rounding mode as FRM.
if (InstrRM == CurrentRM)
continue;

if (!SavedFRM.isValid()) {
// Save current FRM value to SavedFRM.
MachineRegisterInfo *MRI = &MBB.getParent()->getRegInfo();
SavedFRM = MRI->createVirtualRegister(&RISCV::GPRRegClass);
BuildMI(MBB, MI, MI.getDebugLoc(), TII->get(RISCV::SwapFRMImm), SavedFRM)
.addImm(InstrRM);
} else {
// Don't need to save current FRM when SavedFRM having value.
BuildMI(MBB, MI, MI.getDebugLoc(), TII->get(RISCV::WriteFRMImm))
.addImm(InstrRM);
}
CurrentRM = InstrRM;
}

// Restore FRM if needed.
if (SavedFRM.isValid()) {
assert(LastFRMChanger && "Expected valid pointer.");
MachineInstrBuilder MIB =
BuildMI(*MBB.getParent(), {}, TII->get(RISCV::WriteFRM))
.addReg(SavedFRM);
MBB.insertAfter(LastFRMChanger, MIB);
}

return Changed;
}

// This function also swaps frm and restores it when encountering an RVV
// floating point instruction with a static rounding mode.
bool RISCVInsertReadWriteCSR::emitWriteRoundingMode(MachineBasicBlock &MBB) {
Expand Down Expand Up @@ -99,8 +181,12 @@ bool RISCVInsertReadWriteCSR::runOnMachineFunction(MachineFunction &MF) {

bool Changed = false;

for (MachineBasicBlock &MBB : MF)
Changed |= emitWriteRoundingMode(MBB);
for (MachineBasicBlock &MBB : MF) {
if (DisableFRMInsertOpt)
Changed |= emitWriteRoundingMode(MBB);
Copy link
Collaborator

Choose a reason for hiding this comment

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

No test coverage for the unoptimized version.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you think it is a good idea to remove the unoptimized part?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

BTW, I had added unoptimized test cases in llvm/test/CodeGen/RISCV/rvv/frm-insert.ll

else
Changed |= emitWriteRoundingModeOpt(MBB);
}

return Changed;
}
Expand Down
Loading