Skip to content

[MacroFusion][RISCV] Allocate same register for second instruction of fusible pair #77461

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

[MacroFusion][RISCV] Allocate same register for second instruction of fusible pair #77461

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
4 changes: 4 additions & 0 deletions llvm/include/llvm/CodeGen/MachineInstr.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -115,6 +115,7 @@ class MachineInstr
// this instruction.
Unpredictable = 1 << 16, // Instruction with unpredictable condition.
NoConvergent = 1 << 17, // Call does not require convergence guarantees.
Fusible = 1 << 18, // Instruction is the second of a fusible pair.
Copy link
Contributor

Choose a reason for hiding this comment

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

Is the point to add a flag to simplify the code which detects fusible patterns? I realise that some processors may have a lot of fusions, but is adding a new flag really the right solution?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Is the point to add a flag to simplify the code which detects fusible patterns?

Yes!

I realise that some processors may have a lot of fusions, but is adding a new flag really the right solution?

I think this can be feasible, there are already some values in MIFlag that are not so common, like NoConvergent (SIMT only I think), Unpredictable (X86 only), etc.

Copy link
Collaborator

Choose a reason for hiding this comment

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

What if the second fusion instruction is an instruction with 2 register sources instead of 1 register and an immediate. We'll need to know which operand to constrain.

Copy link
Contributor Author

@wangpc-pp wangpc-pp Jan 11, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

What if the second fusion instruction is an instruction with 2 register sources

Currently, I think this is a uncommon case and I don't know if there are such fusions. Do you have any example? Maybe integer multiply-add fusion?
Even if so, I think we can know the constrainted operands via its opcode and constrain them in the getRegAllocationHints implementation. We don't think we need to record this info in target-independent part.

Copy link
Collaborator

Choose a reason for hiding this comment

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

From the RISC-V section of this page https://en.wikichip.org/wiki/macro-operation_fusion which are also listed here https://xiangshan-doc.readthedocs.io/zh-cn/latest/frontend/decode/

slli rd, rs1, {1,2,3}
add rd, rd, rs2

There are a few others on https://xiangshan-doc.readthedocs.io/zh-cn/latest/frontend/decode/

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, I didn't take a deep look into XiangShan's fusions (facepalming).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What if the second fusion instruction is an instruction with 2 register sources instead of 1 register and an immediate. We'll need to know which operand to constrain.

Then we need to handle them case by case via opcode when adding hints.

};

private:
Expand Down Expand Up @@ -1031,6 +1032,9 @@ class MachineInstr
return hasProperty(MCID::Convergent, Type);
}

/// Return true if this instruction is fusible.
bool isFusible() const { return getFlag(Fusible); }

/// Returns true if the specified instruction has a delay slot
/// which must be filled by the code generator.
bool hasDelaySlot(QueryType Type = AnyInBundle) const {
Expand Down
6 changes: 6 additions & 0 deletions llvm/lib/CodeGen/MacroFusion.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -128,6 +128,12 @@ bool llvm::fuseInstructionPair(ScheduleDAGInstrs &DAG, SUnit &FirstSU,
}
}

// Mark the second instruction of fusible pair as MachineInstr::Fusible if
// this mutation is running in pre-ra scheduler.
if (!DAG.MF.getProperties().hasProperty(
MachineFunctionProperties::Property::NoVRegs))
SecondSU.getInstr()->setFlag(MachineInstr::Fusible);

++NumFused;
return true;
}
Expand Down
3 changes: 3 additions & 0 deletions llvm/lib/Target/RISCV/RISCVRegisterInfo.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -838,6 +838,9 @@ bool RISCVRegisterInfo::getRegAllocationHints(
tryAddHint(MO, MI.getOperand(0), NeedGPRC);
}
}
if (MI.isFusible())
if (OpIdx == 1 || (OpIdx == 2 && MI.isCommutable()))
tryAddHint(MO, MI.getOperand(0), false);
}

for (MCPhysReg OrderReg : Order)
Expand Down
37 changes: 37 additions & 0 deletions llvm/test/CodeGen/RISCV/pr76779.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
;RUN: llc < %s -mtriple=riscv64 -mattr=+f -target-abi=lp64f \
;RUN: | FileCheck %s --check-prefix=NOFUSION
;RUN: llc < %s -mtriple=riscv64 -mattr=+f,+lui-addi-fusion \
;RUN: -target-abi=lp64f | FileCheck %s --check-prefix=FUSION
;RUN: llc < %s -mtriple=riscv64 -mattr=+f,+lui-addi-fusion,+use-postra-scheduler \
;RUN: -target-abi=lp64f | FileCheck %s --check-prefixes=FUSION-POSTRA

define void @foo(i32 noundef signext %0, i32 noundef signext %1) {
; NOFUSION-LABEL: foo:
; NOFUSION: # %bb.0:
; NOFUSION-NEXT: lui a0, 3014
; NOFUSION-NEXT: addiw a2, a0, 334
; NOFUSION-NEXT: mv a0, a1
; NOFUSION-NEXT: mv a1, a2
; NOFUSION-NEXT: tail bar
;
; FUSION-LABEL: foo:
; FUSION: # %bb.0:
; FUSION-NEXT: lui a2, 3014
; FUSION-NEXT: addiw a2, a2, 334
; FUSION-NEXT: mv a0, a1
; FUSION-NEXT: mv a1, a2
; FUSION-NEXT: tail bar
;
; FUSION-POSTRA-LABEL: foo:
; FUSION-POSTRA: # %bb.0:
; FUSION-POSTRA-NEXT: lui a2, 3014
; FUSION-POSTRA-NEXT: addiw a2, a2, 334
; FUSION-POSTRA-NEXT: mv a0, a1
; FUSION-POSTRA-NEXT: mv a1, a2
; FUSION-POSTRA-NEXT: tail bar
tail call void @bar(i32 noundef signext %1, i32 noundef signext 12345678)
ret void
}

declare void @bar(i32 noundef signext, i32 noundef signext)