[llvm-branch-commits] [llvm] 2aeaaf8 - [GlobalISel] Add missing operand update when copy is required
Mikael Holmen via llvm-branch-commits
llvm-branch-commits at lists.llvm.org
Wed Jan 20 01:39:05 PST 2021
Author: Gabriel Hjort Åkerlund
Date: 2021-01-20T10:32:52+01:00
New Revision: 2aeaaf841b58b2a6721f9271ae897e392fd0b357
URL: https://github.com/llvm/llvm-project/commit/2aeaaf841b58b2a6721f9271ae897e392fd0b357
DIFF: https://github.com/llvm/llvm-project/commit/2aeaaf841b58b2a6721f9271ae897e392fd0b357.diff
LOG: [GlobalISel] Add missing operand update when copy is required
When constraining an operand register using constrainOperandRegClass(),
the function may emit a COPY in case the provided register class does
not match the current operand register class. However, the operand
itself is not updated to make use of the COPY, thereby resulting in
incorrect code. This patch fixes that bug by updating the machine
operand accordingly.
Reviewed By: dsanders
Differential Revision: https://reviews.llvm.org/D91244
Added:
Modified:
llvm/include/llvm/CodeGen/GlobalISel/InstructionSelector.h
llvm/include/llvm/CodeGen/GlobalISel/InstructionSelectorImpl.h
llvm/include/llvm/CodeGen/GlobalISel/Utils.h
llvm/lib/CodeGen/GlobalISel/InstructionSelector.cpp
llvm/lib/CodeGen/GlobalISel/Utils.cpp
llvm/lib/Target/AArch64/GISel/AArch64CallLowering.cpp
Removed:
################################################################################
diff --git a/llvm/include/llvm/CodeGen/GlobalISel/InstructionSelector.h b/llvm/include/llvm/CodeGen/GlobalISel/InstructionSelector.h
index bf9991eb08de..5b8243a93e7f 100644
--- a/llvm/include/llvm/CodeGen/GlobalISel/InstructionSelector.h
+++ b/llvm/include/llvm/CodeGen/GlobalISel/InstructionSelector.h
@@ -527,16 +527,6 @@ class InstructionSelector {
"Subclasses must override this with a tablegen-erated function");
}
- /// Constrain a register operand of an instruction \p I to a specified
- /// register class. This could involve inserting COPYs before (for uses) or
- /// after (for defs) and may replace the operand of \p I.
- /// \returns whether operand regclass constraining succeeded.
- bool constrainOperandRegToRegClass(MachineInstr &I, unsigned OpIdx,
- const TargetRegisterClass &RC,
- const TargetInstrInfo &TII,
- const TargetRegisterInfo &TRI,
- const RegisterBankInfo &RBI) const;
-
bool isOperandImmEqual(const MachineOperand &MO, int64_t Value,
const MachineRegisterInfo &MRI) const;
diff --git a/llvm/include/llvm/CodeGen/GlobalISel/InstructionSelectorImpl.h b/llvm/include/llvm/CodeGen/GlobalISel/InstructionSelectorImpl.h
index bcb84c337f5e..82e26b0bc355 100644
--- a/llvm/include/llvm/CodeGen/GlobalISel/InstructionSelectorImpl.h
+++ b/llvm/include/llvm/CodeGen/GlobalISel/InstructionSelectorImpl.h
@@ -1058,8 +1058,12 @@ bool InstructionSelector::executeMatchTable(
int64_t OpIdx = MatchTable[CurrentIdx++];
int64_t RCEnum = MatchTable[CurrentIdx++];
assert(OutMIs[InsnID] && "Attempted to add to undefined instruction");
- constrainOperandRegToRegClass(*OutMIs[InsnID].getInstr(), OpIdx,
- *TRI.getRegClass(RCEnum), TII, TRI, RBI);
+ MachineInstr &I = *OutMIs[InsnID].getInstr();
+ MachineFunction &MF = *I.getParent()->getParent();
+ MachineRegisterInfo &MRI = MF.getRegInfo();
+ const TargetRegisterClass &RC = *TRI.getRegClass(RCEnum);
+ MachineOperand &MO = I.getOperand(OpIdx);
+ constrainOperandRegClass(MF, TRI, MRI, TII, RBI, I, RC, MO);
DEBUG_WITH_TYPE(TgtInstructionSelector::getName(),
dbgs() << CurrentIdx << ": GIR_ConstrainOperandRC(OutMIs["
<< InsnID << "], " << OpIdx << ", " << RCEnum
diff --git a/llvm/include/llvm/CodeGen/GlobalISel/Utils.h b/llvm/include/llvm/CodeGen/GlobalISel/Utils.h
index 9bd5180f7222..ed75cde6f316 100644
--- a/llvm/include/llvm/CodeGen/GlobalISel/Utils.h
+++ b/llvm/include/llvm/CodeGen/GlobalISel/Utils.h
@@ -52,9 +52,10 @@ Register constrainRegToClass(MachineRegisterInfo &MRI,
/// Constrain the Register operand OpIdx, so that it is now constrained to the
/// TargetRegisterClass passed as an argument (RegClass).
-/// If this fails, create a new virtual register in the correct class and
-/// insert a COPY before \p InsertPt if it is a use or after if it is a
-/// definition. The debug location of \p InsertPt is used for the new copy.
+/// If this fails, create a new virtual register in the correct class and insert
+/// a COPY before \p InsertPt if it is a use or after if it is a definition.
+/// In both cases, the function also updates the register of RegMo. The debug
+/// location of \p InsertPt is used for the new copy.
///
/// \return The virtual register constrained to the right register class.
Register constrainOperandRegClass(const MachineFunction &MF,
@@ -64,12 +65,13 @@ Register constrainOperandRegClass(const MachineFunction &MF,
const RegisterBankInfo &RBI,
MachineInstr &InsertPt,
const TargetRegisterClass &RegClass,
- const MachineOperand &RegMO);
+ MachineOperand &RegMO);
-/// Try to constrain Reg so that it is usable by argument OpIdx of the
-/// provided MCInstrDesc \p II. If this fails, create a new virtual
-/// register in the correct class and insert a COPY before \p InsertPt
-/// if it is a use or after if it is a definition.
+/// Try to constrain Reg so that it is usable by argument OpIdx of the provided
+/// MCInstrDesc \p II. If this fails, create a new virtual register in the
+/// correct class and insert a COPY before \p InsertPt if it is a use or after
+/// if it is a definition. In both cases, the function also updates the register
+/// of RegMo.
/// This is equivalent to constrainOperandRegClass(..., RegClass, ...)
/// with RegClass obtained from the MCInstrDesc. The debug location of \p
/// InsertPt is used for the new copy.
@@ -81,7 +83,7 @@ Register constrainOperandRegClass(const MachineFunction &MF,
const TargetInstrInfo &TII,
const RegisterBankInfo &RBI,
MachineInstr &InsertPt, const MCInstrDesc &II,
- const MachineOperand &RegMO, unsigned OpIdx);
+ MachineOperand &RegMO, unsigned OpIdx);
/// Mutate the newly-selected instruction \p I to constrain its (possibly
/// generic) virtual register operands to the instruction's register class.
diff --git a/llvm/lib/CodeGen/GlobalISel/InstructionSelector.cpp b/llvm/lib/CodeGen/GlobalISel/InstructionSelector.cpp
index 3eca16808ea6..4fec9e628ddb 100644
--- a/llvm/lib/CodeGen/GlobalISel/InstructionSelector.cpp
+++ b/llvm/lib/CodeGen/GlobalISel/InstructionSelector.cpp
@@ -33,18 +33,6 @@ InstructionSelector::MatcherState::MatcherState(unsigned MaxRenderers)
InstructionSelector::InstructionSelector() = default;
-bool InstructionSelector::constrainOperandRegToRegClass(
- MachineInstr &I, unsigned OpIdx, const TargetRegisterClass &RC,
- const TargetInstrInfo &TII, const TargetRegisterInfo &TRI,
- const RegisterBankInfo &RBI) const {
- MachineBasicBlock &MBB = *I.getParent();
- MachineFunction &MF = *MBB.getParent();
- MachineRegisterInfo &MRI = MF.getRegInfo();
-
- return constrainOperandRegClass(MF, TRI, MRI, TII, RBI, I, RC,
- I.getOperand(OpIdx));
-}
-
bool InstructionSelector::isOperandImmEqual(
const MachineOperand &MO, int64_t Value,
const MachineRegisterInfo &MRI) const {
diff --git a/llvm/lib/CodeGen/GlobalISel/Utils.cpp b/llvm/lib/CodeGen/GlobalISel/Utils.cpp
index aeacd4fb797d..868385c2deff 100644
--- a/llvm/lib/CodeGen/GlobalISel/Utils.cpp
+++ b/llvm/lib/CodeGen/GlobalISel/Utils.cpp
@@ -48,7 +48,7 @@ Register llvm::constrainOperandRegClass(
const MachineFunction &MF, const TargetRegisterInfo &TRI,
MachineRegisterInfo &MRI, const TargetInstrInfo &TII,
const RegisterBankInfo &RBI, MachineInstr &InsertPt,
- const TargetRegisterClass &RegClass, const MachineOperand &RegMO) {
+ const TargetRegisterClass &RegClass, MachineOperand &RegMO) {
Register Reg = RegMO.getReg();
// Assume physical registers are properly constrained.
assert(Register::isVirtualRegister(Reg) && "PhysReg not implemented");
@@ -69,6 +69,13 @@ Register llvm::constrainOperandRegClass(
TII.get(TargetOpcode::COPY), Reg)
.addReg(ConstrainedReg);
}
+ if (GISelChangeObserver *Observer = MF.getObserver()) {
+ Observer->changingInstr(*RegMO.getParent());
+ }
+ RegMO.setReg(ConstrainedReg);
+ if (GISelChangeObserver *Observer = MF.getObserver()) {
+ Observer->changedInstr(*RegMO.getParent());
+ }
} else {
if (GISelChangeObserver *Observer = MF.getObserver()) {
if (!RegMO.isDef()) {
@@ -86,7 +93,7 @@ Register llvm::constrainOperandRegClass(
const MachineFunction &MF, const TargetRegisterInfo &TRI,
MachineRegisterInfo &MRI, const TargetInstrInfo &TII,
const RegisterBankInfo &RBI, MachineInstr &InsertPt, const MCInstrDesc &II,
- const MachineOperand &RegMO, unsigned OpIdx) {
+ MachineOperand &RegMO, unsigned OpIdx) {
Register Reg = RegMO.getReg();
// Assume physical registers are properly constrained.
assert(Register::isVirtualRegister(Reg) && "PhysReg not implemented");
@@ -156,8 +163,7 @@ bool llvm::constrainSelectedInstRegOperands(MachineInstr &I,
// If the operand is a vreg, we should constrain its regclass, and only
// insert COPYs if that's impossible.
// constrainOperandRegClass does that for us.
- MO.setReg(constrainOperandRegClass(MF, TRI, MRI, TII, RBI, I, I.getDesc(),
- MO, OpI));
+ constrainOperandRegClass(MF, TRI, MRI, TII, RBI, I, I.getDesc(), MO, OpI);
// Tie uses to defs as indicated in MCInstrDesc if this hasn't already been
// done.
diff --git a/llvm/lib/Target/AArch64/GISel/AArch64CallLowering.cpp b/llvm/lib/Target/AArch64/GISel/AArch64CallLowering.cpp
index c6be2007d61b..cb81377b7675 100644
--- a/llvm/lib/Target/AArch64/GISel/AArch64CallLowering.cpp
+++ b/llvm/lib/Target/AArch64/GISel/AArch64CallLowering.cpp
@@ -933,10 +933,9 @@ bool AArch64CallLowering::lowerTailCall(
// If Callee is a reg, since it is used by a target specific instruction,
// it must have a register class matching the constraint of that instruction.
if (Info.Callee.isReg())
- MIB->getOperand(0).setReg(constrainOperandRegClass(
- MF, *TRI, MRI, *MF.getSubtarget().getInstrInfo(),
- *MF.getSubtarget().getRegBankInfo(), *MIB, MIB->getDesc(), Info.Callee,
- 0));
+ constrainOperandRegClass(MF, *TRI, MRI, *MF.getSubtarget().getInstrInfo(),
+ *MF.getSubtarget().getRegBankInfo(), *MIB,
+ MIB->getDesc(), Info.Callee, 0);
MF.getFrameInfo().setHasTailCall();
Info.LoweredTailCall = true;
@@ -1018,10 +1017,9 @@ bool AArch64CallLowering::lowerCall(MachineIRBuilder &MIRBuilder,
// instruction, it must have a register class matching the
// constraint of that instruction.
if (Info.Callee.isReg())
- MIB->getOperand(0).setReg(constrainOperandRegClass(
- MF, *TRI, MRI, *MF.getSubtarget().getInstrInfo(),
- *MF.getSubtarget().getRegBankInfo(), *MIB, MIB->getDesc(), Info.Callee,
- 0));
+ constrainOperandRegClass(MF, *TRI, MRI, *MF.getSubtarget().getInstrInfo(),
+ *MF.getSubtarget().getRegBankInfo(), *MIB,
+ MIB->getDesc(), Info.Callee, 0);
// Finally we can copy the returned value back into its virtual-register. In
// symmetry with the arguments, the physical register must be an
More information about the llvm-branch-commits
mailing list