Skip to content
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

[CodeGen] Check entire block if no threshold was given for neighbors #83526

Closed
wants to merge 1 commit into from

Conversation

AtariDreams
Copy link
Contributor

No way to do this in C++ directly, so let's reserve 0 as a special value.

Copy link

github-actions bot commented Mar 1, 2024

⚠️ We detected that you are using a GitHub private e-mail address to contribute to the repo.
Please turn off Keep my email addresses private setting in your account.
See LLVM Discourse for more information.

@llvmbot
Copy link
Collaborator

llvmbot commented Mar 1, 2024

@llvm/pr-subscribers-backend-x86

@llvm/pr-subscribers-backend-arm

Author: AtariDreams (AtariDreams)

Changes

No way to do this in C++ directly, so let's reserve 0 as a special value.


Full diff: https://github.com/llvm/llvm-project/pull/83526.diff

7 Files Affected:

  • (modified) llvm/include/llvm/CodeGen/MachineBasicBlock.h (+1-2)
  • (modified) llvm/lib/CodeGen/MachineBasicBlock.cpp (+4-3)
  • (modified) llvm/lib/Target/AMDGPU/SIFoldOperands.cpp (+1-1)
  • (modified) llvm/lib/Target/AMDGPU/SIInstrInfo.cpp (+1-1)
  • (modified) llvm/lib/Target/AMDGPU/SIPeepholeSDWA.cpp (+1-1)
  • (modified) llvm/lib/Target/ARM/ARMLoadStoreOptimizer.cpp (+3-3)
  • (modified) llvm/lib/Target/X86/X86FixupLEAs.cpp (+2-2)
diff --git a/llvm/include/llvm/CodeGen/MachineBasicBlock.h b/llvm/include/llvm/CodeGen/MachineBasicBlock.h
index dc2035fa598c46..e437c3190a7a96 100644
--- a/llvm/include/llvm/CodeGen/MachineBasicBlock.h
+++ b/llvm/include/llvm/CodeGen/MachineBasicBlock.h
@@ -1149,8 +1149,7 @@ class MachineBasicBlock
   /// \p Reg must be a physical register.
   LivenessQueryResult computeRegisterLiveness(const TargetRegisterInfo *TRI,
                                               MCRegister Reg,
-                                              const_iterator Before,
-                                              unsigned Neighborhood = 10) const;
+                                              const_iterator Before) const;
 
   // Debugging methods.
   void dump() const;
diff --git a/llvm/lib/CodeGen/MachineBasicBlock.cpp b/llvm/lib/CodeGen/MachineBasicBlock.cpp
index 4410fb7ecd23b6..8086111f7b09b9 100644
--- a/llvm/lib/CodeGen/MachineBasicBlock.cpp
+++ b/llvm/lib/CodeGen/MachineBasicBlock.cpp
@@ -1604,11 +1604,12 @@ MachineBasicBlock::getProbabilityIterator(MachineBasicBlock::succ_iterator I) {
 /// Search is localised to a neighborhood of
 /// Neighborhood instructions before (searching for defs or kills) and N
 /// instructions after (searching just for defs) MI.
+/// Special value of 0: Check size of the whole block
 MachineBasicBlock::LivenessQueryResult
 MachineBasicBlock::computeRegisterLiveness(const TargetRegisterInfo *TRI,
-                                           MCRegister Reg, const_iterator Before,
-                                           unsigned Neighborhood) const {
-  unsigned N = Neighborhood;
+                                           MCRegister Reg,
+                                           const_iterator Before) const {
+  unsigned N = this->size();
 
   // Try searching forwards from Before, looking for reads or defs.
   const_iterator I(Before);
diff --git a/llvm/lib/Target/AMDGPU/SIFoldOperands.cpp b/llvm/lib/Target/AMDGPU/SIFoldOperands.cpp
index 634b4aeb30a730..99a1622401dbc7 100644
--- a/llvm/lib/Target/AMDGPU/SIFoldOperands.cpp
+++ b/llvm/lib/Target/AMDGPU/SIFoldOperands.cpp
@@ -375,7 +375,7 @@ bool SIFoldOperands::updateOperand(FoldCandidate &Fold) const {
 
   if ((Fold.isImm() || Fold.isFI() || Fold.isGlobal()) && Fold.needsShrink()) {
     MachineBasicBlock *MBB = MI->getParent();
-    auto Liveness = MBB->computeRegisterLiveness(TRI, AMDGPU::VCC, MI, 16);
+    auto Liveness = MBB->computeRegisterLiveness(TRI, AMDGPU::VCC, MI);
     if (Liveness != MachineBasicBlock::LQR_Dead) {
       LLVM_DEBUG(dbgs() << "Not shrinking " << MI << " due to vcc liveness\n");
       return false;
diff --git a/llvm/lib/Target/AMDGPU/SIInstrInfo.cpp b/llvm/lib/Target/AMDGPU/SIInstrInfo.cpp
index 31ced9d41e15e2..88d7d24bda0f44 100644
--- a/llvm/lib/Target/AMDGPU/SIInstrInfo.cpp
+++ b/llvm/lib/Target/AMDGPU/SIInstrInfo.cpp
@@ -6324,7 +6324,7 @@ loadMBUFScalarOperandsFromVGPR(const SIInstrInfo &TII, MachineInstr &MI,
 
   // Save SCC. Waterfall Loop may overwrite SCC.
   Register SaveSCCReg;
-  bool SCCNotDead = (MBB.computeRegisterLiveness(TRI, AMDGPU::SCC, MI, 30) !=
+  bool SCCNotDead = (MBB.computeRegisterLiveness(TRI, AMDGPU::SCC, MI) !=
                      MachineBasicBlock::LQR_Dead);
   if (SCCNotDead) {
     SaveSCCReg = MRI.createVirtualRegister(&AMDGPU::SReg_32RegClass);
diff --git a/llvm/lib/Target/AMDGPU/SIPeepholeSDWA.cpp b/llvm/lib/Target/AMDGPU/SIPeepholeSDWA.cpp
index afc380b4203457..49087bf720097e 100644
--- a/llvm/lib/Target/AMDGPU/SIPeepholeSDWA.cpp
+++ b/llvm/lib/Target/AMDGPU/SIPeepholeSDWA.cpp
@@ -886,7 +886,7 @@ void SIPeepholeSDWA::pseudoOpConvertToVOP2(MachineInstr &MI,
     return;
   // Make sure VCC or its subregs are dead before MI.
   MachineBasicBlock &MBB = *MI.getParent();
-  auto Liveness = MBB.computeRegisterLiveness(TRI, AMDGPU::VCC, MI, 25);
+  auto Liveness = MBB.computeRegisterLiveness(TRI, AMDGPU::VCC, MI);
   if (Liveness != MachineBasicBlock::LQR_Dead)
     return;
   // Check if VCC is referenced in range of (MI,MISucc].
diff --git a/llvm/lib/Target/ARM/ARMLoadStoreOptimizer.cpp b/llvm/lib/Target/ARM/ARMLoadStoreOptimizer.cpp
index 6121055eb02176..1f5fbf178e51c0 100644
--- a/llvm/lib/Target/ARM/ARMLoadStoreOptimizer.cpp
+++ b/llvm/lib/Target/ARM/ARMLoadStoreOptimizer.cpp
@@ -635,9 +635,9 @@ MachineInstr *ARMLoadStoreOpt::CreateLoadStoreMulti(
 
   // For Thumb1 targets, it might be necessary to clobber the CPSR to merge.
   // Compute liveness information for that register to make the decision.
-  bool SafeToClobberCPSR = !isThumb1 ||
-    (MBB.computeRegisterLiveness(TRI, ARM::CPSR, InsertBefore, 20) ==
-     MachineBasicBlock::LQR_Dead);
+  bool SafeToClobberCPSR =
+      !isThumb1 || (MBB.computeRegisterLiveness(TRI, ARM::CPSR, InsertBefore) ==
+                    MachineBasicBlock::LQR_Dead);
 
   bool Writeback = isThumb1; // Thumb1 LDM/STM have base reg writeback.
 
diff --git a/llvm/lib/Target/X86/X86FixupLEAs.cpp b/llvm/lib/Target/X86/X86FixupLEAs.cpp
index beeebf42dfe81a..446754a6815f8d 100644
--- a/llvm/lib/Target/X86/X86FixupLEAs.cpp
+++ b/llvm/lib/Target/X86/X86FixupLEAs.cpp
@@ -698,7 +698,7 @@ void FixupLEAPass::processInstructionForSlowLEA(MachineBasicBlock::iterator &I,
   const MachineOperand &Segment = MI.getOperand(1 + X86::AddrSegmentReg);
 
   if (Segment.getReg() != 0 || !Offset.isImm() ||
-      MBB.computeRegisterLiveness(TRI, X86::EFLAGS, I, 4) !=
+      MBB.computeRegisterLiveness(TRI, X86::EFLAGS, I) !=
           MachineBasicBlock::LQR_Dead)
     return;
   const Register DstR = Dst.getReg();
@@ -750,7 +750,7 @@ void FixupLEAPass::processInstrForSlow3OpLEA(MachineBasicBlock::iterator &I,
   const MachineOperand &Segment = MI.getOperand(1 + X86::AddrSegmentReg);
 
   if (!(TII->isThreeOperandsLEA(MI) || hasInefficientLEABaseReg(Base, Index)) ||
-      MBB.computeRegisterLiveness(TRI, X86::EFLAGS, I, 4) !=
+      MBB.computeRegisterLiveness(TRI, X86::EFLAGS, I) !=
           MachineBasicBlock::LQR_Dead ||
       Segment.getReg() != X86::NoRegister)
     return;

@AtariDreams AtariDreams changed the title Make default size MBB.size() [CodeGen] Check entire block if no threshold was given for liveliness Mar 1, 2024
@AtariDreams AtariDreams force-pushed the folding branch 3 times, most recently from 5001c1f to 0b28645 Compare March 3, 2024 18:31
@AtariDreams AtariDreams changed the title [CodeGen] Check entire block if no threshold was given for liveliness [CodeGen] Check entire block if no threshold was given for neighbors Mar 3, 2024
@AtariDreams AtariDreams requested a review from arsenm March 3, 2024 19:21
Copy link
Contributor

@arsenm arsenm left a comment

Choose a reason for hiding this comment

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

I don't think this is worth it. You're duplicating an API, to have an unbounded vs. bounded version. I'm also skeptical it's worth exposing a way to make this unbounded

@@ -1598,15 +1598,42 @@ MachineBasicBlock::getProbabilityIterator(MachineBasicBlock::succ_iterator I) {
return Probs.begin() + index;
}

MachineBasicBlock::LivenessQueryResult
MachineBasicBlock::computeRegisterLiveness(const TargetRegisterInfo *TRI,
Copy link
Contributor

Choose a reason for hiding this comment

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

This is introducing a second copy, which I doubt is worth it

llvm/lib/CodeGen/MachineBasicBlock.cpp Outdated Show resolved Hide resolved
We could pass MBB.size(), but we should compute liveness the correct way if we have the entire block.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants