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

[TwoAddressInstruction] Handle physical registers with LiveIntervals #66784

Merged
merged 1 commit into from
Oct 19, 2023
Merged

[TwoAddressInstruction] Handle physical registers with LiveIntervals #66784

merged 1 commit into from
Oct 19, 2023

Conversation

jayfoad
Copy link
Contributor

@jayfoad jayfoad commented Sep 19, 2023

Teach the LiveIntervals path in isPlainlyKilled to handle physical
registers, to get equivalent functionality with the LiveVariables path.

Test this by adding -early-live-intervals RUN lines to a handful of
tests that would fail without this.

@llvmbot
Copy link
Collaborator

llvmbot commented Sep 19, 2023

@llvm/pr-subscribers-backend-x86

Changes

Teach the LiveIntervals path in isPlainlyKilled to handle physical
registers, to get equivalent functionality with the LiveVariables path.

Test this by adding -early-live-intervals RUN lines to a handful of
tests that would fail without this.


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

7 Files Affected:

  • (modified) llvm/lib/CodeGen/TwoAddressInstructionPass.cpp (+27-16)
  • (modified) llvm/test/CodeGen/RISCV/rvv/fmaximum-sdnode.ll (+4)
  • (modified) llvm/test/CodeGen/RISCV/rvv/fminimum-sdnode.ll (+4)
  • (modified) llvm/test/CodeGen/RISCV/rvv/vfmsub-sdnode.ll (+4)
  • (modified) llvm/test/CodeGen/X86/combine-or.ll (+2-3)
  • (modified) llvm/test/CodeGen/X86/machine-cse.ll (+1)
  • (modified) llvm/test/CodeGen/X86/ternlog.ll (+1)
diff --git a/llvm/lib/CodeGen/TwoAddressInstructionPass.cpp b/llvm/lib/CodeGen/TwoAddressInstructionPass.cpp
index 560a0a4fbac66b3..a1f9cdac8612479 100644
--- a/llvm/lib/CodeGen/TwoAddressInstructionPass.cpp
+++ b/llvm/lib/CodeGen/TwoAddressInstructionPass.cpp
@@ -125,6 +125,7 @@ class TwoAddressInstructionPass : public MachineFunctionPass {
   bool isCopyToReg(MachineInstr &MI, Register &SrcReg, Register &DstReg,
                    bool &IsSrcPhys, bool &IsDstPhys) const;
 
+  bool isPlainlyKilled(const MachineInstr *MI, LiveRange &LR) const;
   bool isPlainlyKilled(const MachineInstr *MI, Register Reg) const;
   bool isPlainlyKilled(const MachineOperand &MO) const;
 
@@ -305,27 +306,37 @@ bool TwoAddressInstructionPass::isCopyToReg(MachineInstr &MI, Register &SrcReg,
   return true;
 }
 
+bool TwoAddressInstructionPass::isPlainlyKilled(const MachineInstr *MI,
+                                                LiveRange &LR) const {
+  // This is to match the kill flag version where undefs don't have kill flags.
+  if (!LR.hasAtLeastOneValue())
+    return false;
+
+  SlotIndex useIdx = LIS->getInstructionIndex(*MI);
+  LiveInterval::const_iterator I = LR.find(useIdx);
+  assert(I != LR.end() && "Reg must be live-in to use.");
+  return !I->end.isBlock() && SlotIndex::isSameInstr(I->end, useIdx);
+}
+
 /// Test if the given register value, which is used by the
 /// given instruction, is killed by the given instruction.
 bool TwoAddressInstructionPass::isPlainlyKilled(const MachineInstr *MI,
                                                 Register Reg) const {
-  if (LIS && Reg.isVirtual() && !LIS->isNotInMIMap(*MI)) {
-    // FIXME: Sometimes tryInstructionTransform() will add instructions and
-    // test whether they can be folded before keeping them. In this case it
-    // sets a kill before recursively calling tryInstructionTransform() again.
-    // If there is no interval available, we assume that this instruction is
-    // one of those. A kill flag is manually inserted on the operand so the
-    // check below will handle it.
-    LiveInterval &LI = LIS->getInterval(Reg);
-    // This is to match the kill flag version where undefs don't have kill
-    // flags.
-    if (!LI.hasAtLeastOneValue())
+  // FIXME: Sometimes tryInstructionTransform() will add instructions and
+  // test whether they can be folded before keeping them. In this case it
+  // sets a kill before recursively calling tryInstructionTransform() again.
+  // If there is no interval available, we assume that this instruction is
+  // one of those. A kill flag is manually inserted on the operand so the
+  // check below will handle it.
+  if (LIS && !LIS->isNotInMIMap(*MI)) {
+    if (Reg.isVirtual())
+      return isPlainlyKilled(MI, LIS->getInterval(Reg));
+    // Reserved registers are considered always live.
+    if (MRI->isReserved(Reg))
       return false;
-
-    SlotIndex useIdx = LIS->getInstructionIndex(*MI);
-    LiveInterval::const_iterator I = LI.find(useIdx);
-    assert(I != LI.end() && "Reg must be live-in to use.");
-    return !I->end.isBlock() && SlotIndex::isSameInstr(I->end, useIdx);
+    return all_of(TRI->regunits(Reg), [&](MCRegUnit U) {
+      return isPlainlyKilled(MI, LIS->getRegUnit(U));
+    });
   }
 
   return MI->killsRegister(Reg);
diff --git a/llvm/test/CodeGen/RISCV/rvv/fmaximum-sdnode.ll b/llvm/test/CodeGen/RISCV/rvv/fmaximum-sdnode.ll
index c523344298522e6..6d9fc954ec51ce1 100644
--- a/llvm/test/CodeGen/RISCV/rvv/fmaximum-sdnode.ll
+++ b/llvm/test/CodeGen/RISCV/rvv/fmaximum-sdnode.ll
@@ -1,8 +1,12 @@
 ; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
 ; RUN: llc -mtriple=riscv32 -mattr=+d,+zfh,+zvfh,+v -target-abi=ilp32d \
 ; RUN:     -verify-machineinstrs < %s | FileCheck %s
+; RUN: llc -mtriple=riscv32 -mattr=+d,+zfh,+zvfh,+v -target-abi=ilp32d \
+; RUN:     -verify-machineinstrs -early-live-intervals < %s | FileCheck %s
 ; RUN: llc -mtriple=riscv64 -mattr=+d,+zfh,+zvfh,+v -target-abi=lp64d \
 ; RUN:     -verify-machineinstrs < %s | FileCheck %s
+; RUN: llc -mtriple=riscv64 -mattr=+d,+zfh,+zvfh,+v -target-abi=lp64d \
+; RUN:     -verify-machineinstrs -early-live-intervals < %s | FileCheck %s
 
 declare <vscale x 1 x half> @llvm.maximum.nxv1f16(<vscale x 1 x half>, <vscale x 1 x half>)
 
diff --git a/llvm/test/CodeGen/RISCV/rvv/fminimum-sdnode.ll b/llvm/test/CodeGen/RISCV/rvv/fminimum-sdnode.ll
index 7c201d3bc692203..805637c10bb4044 100644
--- a/llvm/test/CodeGen/RISCV/rvv/fminimum-sdnode.ll
+++ b/llvm/test/CodeGen/RISCV/rvv/fminimum-sdnode.ll
@@ -1,8 +1,12 @@
 ; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
 ; RUN: llc -mtriple=riscv32 -mattr=+d,+zfh,+zvfh,+v -target-abi=ilp32d \
 ; RUN:     -verify-machineinstrs < %s | FileCheck %s
+; RUN: llc -mtriple=riscv32 -mattr=+d,+zfh,+zvfh,+v -target-abi=ilp32d \
+; RUN:     -verify-machineinstrs -early-live-intervals < %s | FileCheck %s
 ; RUN: llc -mtriple=riscv64 -mattr=+d,+zfh,+zvfh,+v -target-abi=lp64d \
 ; RUN:     -verify-machineinstrs < %s | FileCheck %s
+; RUN: llc -mtriple=riscv64 -mattr=+d,+zfh,+zvfh,+v -target-abi=lp64d \
+; RUN:     -verify-machineinstrs -early-live-intervals < %s | FileCheck %s
 
 declare <vscale x 1 x half> @llvm.minimum.nxv1f16(<vscale x 1 x half>, <vscale x 1 x half>)
 
diff --git a/llvm/test/CodeGen/RISCV/rvv/vfmsub-sdnode.ll b/llvm/test/CodeGen/RISCV/rvv/vfmsub-sdnode.ll
index 5792171269057bb..433b0d1cbdd8531 100644
--- a/llvm/test/CodeGen/RISCV/rvv/vfmsub-sdnode.ll
+++ b/llvm/test/CodeGen/RISCV/rvv/vfmsub-sdnode.ll
@@ -1,8 +1,12 @@
 ; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
 ; RUN: llc -mtriple=riscv32 -mattr=+d,+zfh,+zvfh,+v -target-abi=ilp32d \
 ; RUN:     -verify-machineinstrs < %s | FileCheck %s
+; RUN: llc -mtriple=riscv32 -mattr=+d,+zfh,+zvfh,+v -target-abi=ilp32d \
+; RUN:     -verify-machineinstrs -early-live-intervals < %s | FileCheck %s
 ; RUN: llc -mtriple=riscv64 -mattr=+d,+zfh,+zvfh,+v -target-abi=lp64d \
 ; RUN:     -verify-machineinstrs < %s | FileCheck %s
+; RUN: llc -mtriple=riscv64 -mattr=+d,+zfh,+zvfh,+v -target-abi=lp64d \
+; RUN:     -verify-machineinstrs -early-live-intervals < %s | FileCheck %s
 
 ; This tests a mix of vfmsac and vfmsub by using different operand orders to
 ; trigger commuting in TwoAddressInstructionPass.
diff --git a/llvm/test/CodeGen/X86/combine-or.ll b/llvm/test/CodeGen/X86/combine-or.ll
index bfb9885c10c4e5d..b6b6994834dc1f0 100644
--- a/llvm/test/CodeGen/X86/combine-or.ll
+++ b/llvm/test/CodeGen/X86/combine-or.ll
@@ -249,9 +249,8 @@ define <4 x i32> @test18(<4 x i32> %a, <4 x i32> %b) {
 ; CHECK-LIS-LABEL: test18:
 ; CHECK-LIS:       # %bb.0:
 ; CHECK-LIS-NEXT:    pxor %xmm2, %xmm2
-; CHECK-LIS-NEXT:    pxor %xmm3, %xmm3
-; CHECK-LIS-NEXT:    pblendw {{.*#+}} xmm3 = xmm0[0,1],xmm3[2,3,4,5,6,7]
-; CHECK-LIS-NEXT:    pshufd {{.*#+}} xmm0 = xmm3[1,0,1,1]
+; CHECK-LIS-NEXT:    pblendw {{.*#+}} xmm0 = xmm0[0,1],xmm2[2,3,4,5,6,7]
+; CHECK-LIS-NEXT:    pshufd {{.*#+}} xmm0 = xmm0[1,0,1,1]
 ; CHECK-LIS-NEXT:    pblendw {{.*#+}} xmm2 = xmm1[0,1],xmm2[2,3,4,5,6,7]
 ; CHECK-LIS-NEXT:    por %xmm2, %xmm0
 ; CHECK-LIS-NEXT:    retq
diff --git a/llvm/test/CodeGen/X86/machine-cse.ll b/llvm/test/CodeGen/X86/machine-cse.ll
index d436f10fdc1f41f..431031136e4a429 100644
--- a/llvm/test/CodeGen/X86/machine-cse.ll
+++ b/llvm/test/CodeGen/X86/machine-cse.ll
@@ -1,5 +1,6 @@
 ; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
 ; RUN: llc -mtriple=x86_64-unknown-unknown < %s | FileCheck %s
+; RUN: llc -mtriple=x86_64-unknown-unknown -early-live-intervals < %s | FileCheck %s
 ; rdar://7610418
 
 %ptr = type { ptr }
diff --git a/llvm/test/CodeGen/X86/ternlog.ll b/llvm/test/CodeGen/X86/ternlog.ll
index da3b7d5baffd557..cef044acbc5a941 100644
--- a/llvm/test/CodeGen/X86/ternlog.ll
+++ b/llvm/test/CodeGen/X86/ternlog.ll
@@ -1,5 +1,6 @@
 ; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
 ; RUN: opt < %s -passes=instcombine -mtriple=x86_64-unknown-unknown -S | llc -mtriple=x86_64-unknown-unknown -mattr=+avx512f,+avx512vl | FileCheck %s
+; RUN: opt < %s -passes=instcombine -mtriple=x86_64-unknown-unknown -S | llc -mtriple=x86_64-unknown-unknown -mattr=+avx512f,+avx512vl -early-live-intervals | FileCheck %s
 
 ;; This is just a simple test to make sure there are no regressions
 ;; cause by splitting/recombining ternlog intrinsics.

@jayfoad jayfoad requested review from nikic, RKSimon and topperc and removed request for a team October 19, 2023 09:03
@jayfoad
Copy link
Contributor Author

jayfoad commented Oct 19, 2023

Ping!

Copy link
Contributor

@perlfu perlfu left a comment

Choose a reason for hiding this comment

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

LGTM - but should probably have a second opinion

@RKSimon
Copy link
Collaborator

RKSimon commented Oct 19, 2023

@jayfoad Please can you rebase this?

Teach the LiveIntervals path in isPlainlyKilled to handle physical
registers, to get equivalent functionality with the LiveVariables path.

Test this by adding -early-live-intervals RUN lines to a handful of
tests that would fail without this.
Copy link
Collaborator

@RKSimon RKSimon left a comment

Choose a reason for hiding this comment

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

LGTM - cheers

@jayfoad jayfoad merged commit 21e1b13 into llvm:main Oct 19, 2023
2 of 3 checks passed
@jayfoad jayfoad deleted the twoaddress-liveintervals-physregs branch October 19, 2023 15:26
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.

4 participants