[llvm] [SelectionDAG] Prevent converting a virtual register to an MCRegister. (PR #122857)
Craig Topper via llvm-commits
llvm-commits at lists.llvm.org
Mon Jan 13 23:00:29 PST 2025
https://github.com/topperc updated https://github.com/llvm/llvm-project/pull/122857
>From 569820c28eb3e3cbb397aeae28d18e9db21ea26b Mon Sep 17 00:00:00 2001
From: Craig Topper <craig.topper at sifive.com>
Date: Mon, 13 Jan 2025 20:40:56 -0800
Subject: [PATCH 1/2] [SelectionDAG] Prevent converting a virtual register to
an MCRegister.
I believe the goal is that MCRegister is only for physical registers.
---
.../CodeGen/SelectionDAG/SelectionDAGISel.cpp | 96 ++++++++++---------
1 file changed, 50 insertions(+), 46 deletions(-)
diff --git a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
index d64a90bcaae7da..c918ca3df7cd21 100644
--- a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
@@ -705,53 +705,57 @@ bool SelectionDAGISel::runOnMachineFunction(MachineFunction &mf) {
if (InstrRef)
continue;
- // If Reg is live-in then update debug info to track its copy in a vreg.
- DenseMap<MCRegister, Register>::iterator LDI = LiveInMap.find(Reg);
- if (LDI != LiveInMap.end()) {
- assert(!hasFI && "There's no handling of frame pointer updating here yet "
- "- add if needed");
- MachineInstr *Def = RegInfo->getVRegDef(LDI->second);
- MachineBasicBlock::iterator InsertPos = Def;
- const MDNode *Variable = MI->getDebugVariable();
- const MDNode *Expr = MI->getDebugExpression();
- DebugLoc DL = MI->getDebugLoc();
- bool IsIndirect = MI->isIndirectDebugValue();
- if (IsIndirect)
- assert(MI->getDebugOffset().getImm() == 0 &&
- "DBG_VALUE with nonzero offset");
- assert(cast<DILocalVariable>(Variable)->isValidLocationForIntrinsic(DL) &&
- "Expected inlined-at fields to agree");
- assert(MI->getOpcode() != TargetOpcode::DBG_VALUE_LIST &&
- "Didn't expect to see a DBG_VALUE_LIST here");
- // Def is never a terminator here, so it is ok to increment InsertPos.
- BuildMI(*EntryMBB, ++InsertPos, DL, TII->get(TargetOpcode::DBG_VALUE),
- IsIndirect, LDI->second, Variable, Expr);
-
- // If this vreg is directly copied into an exported register then
- // that COPY instructions also need DBG_VALUE, if it is the only
- // user of LDI->second.
- MachineInstr *CopyUseMI = nullptr;
- for (MachineInstr &UseMI : RegInfo->use_instructions(LDI->second)) {
- if (UseMI.isDebugValue())
- continue;
- if (UseMI.isCopy() && !CopyUseMI && UseMI.getParent() == EntryMBB) {
- CopyUseMI = &UseMI;
- continue;
+ if (Reg.isPhysical()) {
+ // If Reg is live-in then update debug info to track its copy in a vreg.
+ DenseMap<MCRegister, Register>::iterator LDI = LiveInMap.find(Reg);
+ if (LDI != LiveInMap.end()) {
+ assert(!hasFI &&
+ "There's no handling of frame pointer updating here yet "
+ "- add if needed");
+ MachineInstr *Def = RegInfo->getVRegDef(LDI->second);
+ MachineBasicBlock::iterator InsertPos = Def;
+ const MDNode *Variable = MI->getDebugVariable();
+ const MDNode *Expr = MI->getDebugExpression();
+ DebugLoc DL = MI->getDebugLoc();
+ bool IsIndirect = MI->isIndirectDebugValue();
+ if (IsIndirect)
+ assert(MI->getDebugOffset().getImm() == 0 &&
+ "DBG_VALUE with nonzero offset");
+ assert(
+ cast<DILocalVariable>(Variable)->isValidLocationForIntrinsic(DL) &&
+ "Expected inlined-at fields to agree");
+ assert(MI->getOpcode() != TargetOpcode::DBG_VALUE_LIST &&
+ "Didn't expect to see a DBG_VALUE_LIST here");
+ // Def is never a terminator here, so it is ok to increment InsertPos.
+ BuildMI(*EntryMBB, ++InsertPos, DL, TII->get(TargetOpcode::DBG_VALUE),
+ IsIndirect, LDI->second, Variable, Expr);
+
+ // If this vreg is directly copied into an exported register then
+ // that COPY instructions also need DBG_VALUE, if it is the only
+ // user of LDI->second.
+ MachineInstr *CopyUseMI = nullptr;
+ for (MachineInstr &UseMI : RegInfo->use_instructions(LDI->second)) {
+ if (UseMI.isDebugValue())
+ continue;
+ if (UseMI.isCopy() && !CopyUseMI && UseMI.getParent() == EntryMBB) {
+ CopyUseMI = &UseMI;
+ continue;
+ }
+ // Otherwise this is another use or second copy use.
+ CopyUseMI = nullptr;
+ break;
+ }
+ if (CopyUseMI &&
+ TRI.getRegSizeInBits(LDI->second, MRI) ==
+ TRI.getRegSizeInBits(CopyUseMI->getOperand(0).getReg(), MRI)) {
+ // Use MI's debug location, which describes where Variable was
+ // declared, rather than whatever is attached to CopyUseMI.
+ MachineInstr *NewMI =
+ BuildMI(*MF, DL, TII->get(TargetOpcode::DBG_VALUE), IsIndirect,
+ CopyUseMI->getOperand(0).getReg(), Variable, Expr);
+ MachineBasicBlock::iterator Pos = CopyUseMI;
+ EntryMBB->insertAfter(Pos, NewMI);
}
- // Otherwise this is another use or second copy use.
- CopyUseMI = nullptr;
- break;
- }
- if (CopyUseMI &&
- TRI.getRegSizeInBits(LDI->second, MRI) ==
- TRI.getRegSizeInBits(CopyUseMI->getOperand(0).getReg(), MRI)) {
- // Use MI's debug location, which describes where Variable was
- // declared, rather than whatever is attached to CopyUseMI.
- MachineInstr *NewMI =
- BuildMI(*MF, DL, TII->get(TargetOpcode::DBG_VALUE), IsIndirect,
- CopyUseMI->getOperand(0).getReg(), Variable, Expr);
- MachineBasicBlock::iterator Pos = CopyUseMI;
- EntryMBB->insertAfter(Pos, NewMI);
}
}
}
>From 9ac8a175cb5270a8f73c8f7fa62c1d1dae187d5f Mon Sep 17 00:00:00 2001
From: Craig Topper <craig.topper at sifive.com>
Date: Mon, 13 Jan 2025 22:41:16 -0800
Subject: [PATCH 2/2] fixup! use early continue.
---
.../CodeGen/SelectionDAG/SelectionDAGISel.cpp | 98 +++++++++----------
1 file changed, 48 insertions(+), 50 deletions(-)
diff --git a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
index c918ca3df7cd21..b416e98fe61a8b 100644
--- a/llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
+++ b/llvm/lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp
@@ -705,57 +705,55 @@ bool SelectionDAGISel::runOnMachineFunction(MachineFunction &mf) {
if (InstrRef)
continue;
- if (Reg.isPhysical()) {
- // If Reg is live-in then update debug info to track its copy in a vreg.
- DenseMap<MCRegister, Register>::iterator LDI = LiveInMap.find(Reg);
- if (LDI != LiveInMap.end()) {
- assert(!hasFI &&
- "There's no handling of frame pointer updating here yet "
- "- add if needed");
- MachineInstr *Def = RegInfo->getVRegDef(LDI->second);
- MachineBasicBlock::iterator InsertPos = Def;
- const MDNode *Variable = MI->getDebugVariable();
- const MDNode *Expr = MI->getDebugExpression();
- DebugLoc DL = MI->getDebugLoc();
- bool IsIndirect = MI->isIndirectDebugValue();
- if (IsIndirect)
- assert(MI->getDebugOffset().getImm() == 0 &&
- "DBG_VALUE with nonzero offset");
- assert(
- cast<DILocalVariable>(Variable)->isValidLocationForIntrinsic(DL) &&
- "Expected inlined-at fields to agree");
- assert(MI->getOpcode() != TargetOpcode::DBG_VALUE_LIST &&
- "Didn't expect to see a DBG_VALUE_LIST here");
- // Def is never a terminator here, so it is ok to increment InsertPos.
- BuildMI(*EntryMBB, ++InsertPos, DL, TII->get(TargetOpcode::DBG_VALUE),
- IsIndirect, LDI->second, Variable, Expr);
-
- // If this vreg is directly copied into an exported register then
- // that COPY instructions also need DBG_VALUE, if it is the only
- // user of LDI->second.
- MachineInstr *CopyUseMI = nullptr;
- for (MachineInstr &UseMI : RegInfo->use_instructions(LDI->second)) {
- if (UseMI.isDebugValue())
- continue;
- if (UseMI.isCopy() && !CopyUseMI && UseMI.getParent() == EntryMBB) {
- CopyUseMI = &UseMI;
- continue;
- }
- // Otherwise this is another use or second copy use.
- CopyUseMI = nullptr;
- break;
- }
- if (CopyUseMI &&
- TRI.getRegSizeInBits(LDI->second, MRI) ==
- TRI.getRegSizeInBits(CopyUseMI->getOperand(0).getReg(), MRI)) {
- // Use MI's debug location, which describes where Variable was
- // declared, rather than whatever is attached to CopyUseMI.
- MachineInstr *NewMI =
- BuildMI(*MF, DL, TII->get(TargetOpcode::DBG_VALUE), IsIndirect,
- CopyUseMI->getOperand(0).getReg(), Variable, Expr);
- MachineBasicBlock::iterator Pos = CopyUseMI;
- EntryMBB->insertAfter(Pos, NewMI);
+ // If Reg is live-in then update debug info to track its copy in a vreg.
+ if (!Reg.isPhysical())
+ continue;
+ DenseMap<MCRegister, Register>::iterator LDI = LiveInMap.find(Reg);
+ if (LDI != LiveInMap.end()) {
+ assert(!hasFI && "There's no handling of frame pointer updating here yet "
+ "- add if needed");
+ MachineInstr *Def = RegInfo->getVRegDef(LDI->second);
+ MachineBasicBlock::iterator InsertPos = Def;
+ const MDNode *Variable = MI->getDebugVariable();
+ const MDNode *Expr = MI->getDebugExpression();
+ DebugLoc DL = MI->getDebugLoc();
+ bool IsIndirect = MI->isIndirectDebugValue();
+ if (IsIndirect)
+ assert(MI->getDebugOffset().getImm() == 0 &&
+ "DBG_VALUE with nonzero offset");
+ assert(cast<DILocalVariable>(Variable)->isValidLocationForIntrinsic(DL) &&
+ "Expected inlined-at fields to agree");
+ assert(MI->getOpcode() != TargetOpcode::DBG_VALUE_LIST &&
+ "Didn't expect to see a DBG_VALUE_LIST here");
+ // Def is never a terminator here, so it is ok to increment InsertPos.
+ BuildMI(*EntryMBB, ++InsertPos, DL, TII->get(TargetOpcode::DBG_VALUE),
+ IsIndirect, LDI->second, Variable, Expr);
+
+ // If this vreg is directly copied into an exported register then
+ // that COPY instructions also need DBG_VALUE, if it is the only
+ // user of LDI->second.
+ MachineInstr *CopyUseMI = nullptr;
+ for (MachineInstr &UseMI : RegInfo->use_instructions(LDI->second)) {
+ if (UseMI.isDebugValue())
+ continue;
+ if (UseMI.isCopy() && !CopyUseMI && UseMI.getParent() == EntryMBB) {
+ CopyUseMI = &UseMI;
+ continue;
}
+ // Otherwise this is another use or second copy use.
+ CopyUseMI = nullptr;
+ break;
+ }
+ if (CopyUseMI &&
+ TRI.getRegSizeInBits(LDI->second, MRI) ==
+ TRI.getRegSizeInBits(CopyUseMI->getOperand(0).getReg(), MRI)) {
+ // Use MI's debug location, which describes where Variable was
+ // declared, rather than whatever is attached to CopyUseMI.
+ MachineInstr *NewMI =
+ BuildMI(*MF, DL, TII->get(TargetOpcode::DBG_VALUE), IsIndirect,
+ CopyUseMI->getOperand(0).getReg(), Variable, Expr);
+ MachineBasicBlock::iterator Pos = CopyUseMI;
+ EntryMBB->insertAfter(Pos, NewMI);
}
}
}
More information about the llvm-commits
mailing list