Skip to content

[RISCV] Pattern-match frameindex #120917

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.

Sign up for GitHub

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 2 commits into from
Dec 23, 2024
Merged

[RISCV] Pattern-match frameindex #120917

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
a54b879
[RISCV] Pattern-match frameindex (NFCI)
s-barannikov Dec 22, 2024
9fb21e7
Merge branch 'main' into riscv/frame-index
s-barannikov Dec 23, 2024
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
18 changes: 10 additions & 8 deletions llvm/lib/Target/RISCV/GISel/RISCVInstructionSelector.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,8 @@ class RISCVInstructionSelector : public InstructionSelector {
int OpIdx) const;
void renderImm(MachineInstrBuilder &MIB, const MachineInstr &MI,
int OpIdx) const;
void renderFrameIndex(MachineInstrBuilder &MIB, const MachineInstr &MI,
int OpIdx) const;

void renderTrailingZeros(MachineInstrBuilder &MIB, const MachineInstr &MI,
int OpIdx) const;
Expand Down Expand Up @@ -715,14 +717,6 @@ bool RISCVInstructionSelector::select(MachineInstr &MI) {
MI.setDesc(TII.get(RISCV::PseudoBRIND));
MI.addOperand(MachineOperand::CreateImm(0));
return constrainSelectedInstRegOperands(MI, TII, TRI, RBI);
case TargetOpcode::G_FRAME_INDEX: {
// TODO: We may want to replace this code with the SelectionDAG patterns,
// which fail to get imported because it uses FrameAddrRegImm, which is a
// ComplexPattern
MI.setDesc(TII.get(RISCV::ADDI));
MI.addOperand(MachineOperand::CreateImm(0));
return constrainSelectedInstRegOperands(MI, TII, TRI, RBI);
}
case TargetOpcode::G_SELECT:
return selectSelect(MI, MIB);
case TargetOpcode::G_FCMP:
Expand Down Expand Up @@ -859,6 +853,14 @@ void RISCVInstructionSelector::renderImm(MachineInstrBuilder &MIB,
MIB.addImm(CstVal);
}

void RISCVInstructionSelector::renderFrameIndex(MachineInstrBuilder &MIB,
const MachineInstr &MI,
int OpIdx) const {
assert(MI.getOpcode() == TargetOpcode::G_FRAME_INDEX && OpIdx == -1 &&
"Expected G_FRAME_INDEX");
MIB.add(MI.getOperand(1));
}

void RISCVInstructionSelector::renderTrailingZeros(MachineInstrBuilder &MIB,
const MachineInstr &MI,
int OpIdx) const {
Expand Down
23 changes: 0 additions & 23 deletions llvm/lib/Target/RISCV/RISCVISelDAGToDAG.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2531,29 +2531,6 @@ bool RISCVDAGToDAGISel::SelectAddrFrameIndex(SDValue Addr, SDValue &Base,
return false;
}

// Select a frame index and an optional immediate offset from an ADD or OR.
bool RISCVDAGToDAGISel::SelectFrameAddrRegImm(SDValue Addr, SDValue &Base,
SDValue &Offset) {
if (SelectAddrFrameIndex(Addr, Base, Offset))
return true;

if (!CurDAG->isBaseWithConstantOffset(Addr))
return false;

if (auto *FIN = dyn_cast<FrameIndexSDNode>(Addr.getOperand(0))) {
int64_t CVal = cast<ConstantSDNode>(Addr.getOperand(1))->getSExtValue();
if (isInt<12>(CVal)) {
Base = CurDAG->getTargetFrameIndex(FIN->getIndex(),
Subtarget->getXLenVT());
Offset = CurDAG->getSignedTargetConstant(CVal, SDLoc(Addr),
Subtarget->getXLenVT());
return true;
}
}

return false;
}

// Fold constant addresses.
static bool selectConstantAddr(SelectionDAG *CurDAG, const SDLoc &DL,
const MVT VT, const RISCVSubtarget *Subtarget,
Expand Down
1 change: 0 additions & 1 deletion llvm/lib/Target/RISCV/RISCVISelDAGToDAG.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@ class RISCVDAGToDAGISel : public SelectionDAGISel {
std::vector<SDValue> &OutOps) override;

bool SelectAddrFrameIndex(SDValue Addr, SDValue &Base, SDValue &Offset);
bool SelectFrameAddrRegImm(SDValue Addr, SDValue &Base, SDValue &Offset);
bool SelectAddrRegImm(SDValue Addr, SDValue &Base, SDValue &Offset,
bool IsRV32Zdinx = false);
bool SelectAddrRegImmRV32Zdinx(SDValue Addr, SDValue &Base, SDValue &Offset) {
Expand Down
18 changes: 13 additions & 5 deletions llvm/lib/Target/RISCV/RISCVInstrInfo.td
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -400,9 +400,6 @@ def uimm6gt32 : ImmLeaf<XLenVT, [{
}]>;

// Addressing modes.
// Necessary because a frameindex can't be matched directly in a pattern.
def FrameAddrRegImm : ComplexPattern<iPTR, 2, "SelectFrameAddrRegImm",
[frameindex, or, add]>;
def AddrRegImm : ComplexPattern<iPTR, 2, "SelectAddrRegImm">;

// Return the negation of an immediate value.
Expand Down Expand Up @@ -1401,8 +1398,19 @@ def PseudoAddTPRel : Pseudo<(outs GPR:$rd),

/// FrameIndex calculations

def : Pat<(FrameAddrRegImm (iPTR GPR:$rs1), simm12:$imm12),
(ADDI GPR:$rs1, simm12:$imm12)>;
// Transforms frameindex -> tframeindex.
def to_tframeindex : SDNodeXForm<frameindex, [{
return CurDAG->getTargetFrameIndex(N->getIndex(), N->getValueType(0));
}]>;

def : GICustomOperandRenderer<"renderFrameIndex">,
GISDNodeXFormEquiv<to_tframeindex>;

def : Pat<(frameindex:$fi), (ADDI (iPTR (to_tframeindex $fi)), 0)>;

def : Pat<(add_like frameindex:$fi, simm12:$offset),
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think add_like might be less capable than SelectionDAG::isBaseWithConstantOffset, but if it shows up its probably fixable.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

isBaseWithConstantOffset additionally handles (xor x, INT_MIN), that's all the difference I noticed.

Copy link
Member

Choose a reason for hiding this comment

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

Given you think there is a difference, please don't mark this as "NFCI" in the message.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I removed NFCI now

(ADDI (iPTR (to_tframeindex $fi)), simm12:$offset)>;

def GIAddrRegImm :
GIComplexOperandMatcher<s32, "selectAddrRegImm">,
GIComplexPatternEquiv<AddrRegImm>;
Expand Down
Loading