[X86] X86LowerTileCopy: Find dead register to use to prevent save-reload of tile register #83628

AZero13 · 2024-03-01T22:47:49Z

No description provided.

llvmbot · 2024-03-01T22:48:30Z

@llvm/pr-subscribers-backend-x86

Author: AtariDreams (AtariDreams)

Changes

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

1 Files Affected:

(modified) llvm/lib/Target/X86/X86LowerTileCopy.cpp (+51-15)

diff --git a/llvm/lib/Target/X86/X86LowerTileCopy.cpp b/llvm/lib/Target/X86/X86LowerTileCopy.cpp
index e7afc49240e547..3d82360942ddbf 100644
--- a/llvm/lib/Target/X86/X86LowerTileCopy.cpp
+++ b/llvm/lib/Target/X86/X86LowerTileCopy.cpp
@@ -20,6 +20,7 @@
 #include "X86InstrBuilder.h"
 #include "X86InstrInfo.h"
 #include "X86Subtarget.h"
+#include "llvm/CodeGen/LivePhysRegs.h"
 #include "llvm/CodeGen/MachineBasicBlock.h"
 #include "llvm/CodeGen/MachineFrameInfo.h"
 #include "llvm/CodeGen/MachineFunction.h"
@@ -73,6 +74,7 @@ bool X86LowerTileCopy::runOnMachineFunction(MachineFunction &MF) {
   const X86Subtarget &ST = MF.getSubtarget<X86Subtarget>();
   const X86InstrInfo *TII = ST.getInstrInfo();
   bool Changed = false;
+  MachineRegisterInfo &MRI = MF.getRegInfo();
 
   for (MachineBasicBlock &MBB : MF) {
     for (MachineInstr &MI : llvm::make_early_inc_range(MBB)) {
@@ -90,22 +92,54 @@ bool X86LowerTileCopy::runOnMachineFunction(MachineFunction &MF) {
       unsigned Size = TRI->getSpillSize(X86::TILERegClass);
       Align Alignment = TRI->getSpillAlign(X86::TILERegClass);
       int TileSS = MF.getFrameInfo().CreateSpillStackObject(Size, Alignment);
-      // Allocate stack slot for stride register
-      Size = TRI->getSpillSize(X86::GR64RegClass);
-      Alignment = TRI->getSpillAlign(X86::GR64RegClass);
-      int StrideSS = MF.getFrameInfo().CreateSpillStackObject(Size, Alignment);
+      int StrideSS = 0;
 
-      // TODO: Pick a killed regiter to avoid save/reload. There is problem
+      LivePhysRegs UsedRegs(*TRI);
+      UsedRegs.addLiveOuts(MBB);
+
+      const MCPhysReg *CSRegs = TRI->getCalleeSavedRegs(&MF);
+      for (unsigned i = 0; CSRegs[i]; ++i)
+        UsedRegs.addReg(CSRegs[i]);
+
+      auto InstUpToMI = MBB.end();
+      while (InstUpToMI != MI)
+        // The pre-decrement is on purpose here.
+        // We want to have the liveness right before I.
+        UsedRegs.stepBackward(*--InstUpToMI);
+
+      // Look for a temporary register to use.
+      BitVector GR64Regs =
+          TRI->getAllocatableSet(MF, TRI->getRegClass(X86::GR64RegClassID));
+      // Pick a killed regiter to avoid save/reload. There is problem
       // to get live interval in this stage.
       Register GR64Cand = X86::RAX;
 
+      // Find the first available-register that is available
+      bool found = false;
+      for (auto RegT : GR64Regs.set_bits()) {
+        if (UsedRegs.available(MRI, RegT)) {
+          GR64Cand = RegT;
+          break;
+        }
+      }
+
       const DebugLoc &DL = MI.getDebugLoc();
-      // mov %rax (%sp)
-      BuildMI(MBB, MI, DL, TII->get(X86::IMPLICIT_DEF), GR64Cand);
-      addFrameReference(BuildMI(MBB, MI, DL, TII->get(X86::MOV64mr)), StrideSS)
-          .addReg(GR64Cand);
-      // mov 64 %rax
-      BuildMI(MBB, MI, DL, TII->get(X86::MOV64ri), GR64Cand).addImm(64);
+      if (found) {
+        // mov 64 %reg
+        BuildMI(MBB, MI, DL, TII->get(X86::MOV64ri), GR64Cand).addImm(64);
+      } else {
+        // Allocate stack slot for stride register
+        Size = TRI->getSpillSize(X86::GR64RegClass);
+        Alignment = TRI->getSpillAlign(X86::GR64RegClass);
+        StrideSS = MF.getFrameInfo().CreateSpillStackObject(Size, Alignment);
+        // mov %rax (%sp)
+        BuildMI(MBB, MI, DL, TII->get(X86::IMPLICIT_DEF), GR64Cand);
+        addFrameReference(BuildMI(MBB, MI, DL, TII->get(X86::MOV64mr)),
+                          StrideSS)
+            .addReg(GR64Cand);
+        // mov 64 %rax
+        BuildMI(MBB, MI, DL, TII->get(X86::MOV64ri), GR64Cand).addImm(64);
+      }
       // tilestored %tmm, (%sp, %idx)
 #define GET_EGPR_IF_ENABLED(OPC) (ST.hasEGPR() ? OPC##_EVEX : OPC)
       unsigned Opc = GET_EGPR_IF_ENABLED(X86::TILESTORED);
@@ -120,10 +154,12 @@ bool X86LowerTileCopy::runOnMachineFunction(MachineFunction &MF) {
 #undef GET_EGPR_IF_ENABLED
       NewMI = addFrameReference(BuildMI(MBB, MI, DL, TII->get(Opc), DstReg),
                                 TileSS);
-      // restore %rax
-      // mov (%sp) %rax
-      addFrameReference(BuildMI(MBB, MI, DL, TII->get(X86::MOV64rm), GR64Cand),
-                        StrideSS);
+      if (!found) {
+        // restore %rax
+        // mov (%sp) %rax
+        addFrameReference(
+            BuildMI(MBB, MI, DL, TII->get(X86::MOV64rm), GR64Cand), StrideSS);
+      }
       MI.eraseFromParent();
       Changed = true;
     }

github-actions · 2024-03-01T22:48:56Z

⚠️ 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.

RKSimon

test coverage?

phoebewang · 2024-04-20T09:41:31Z

@AtariDreams Reverse ping~

I met a problem cased by #86973 which was confused by IMPLICIT_DEF in this pass, and checked this patch can fix it. I put a commit to add the missing found = true; and optimize a bit. Please take a look, thanks!

llvm/lib/Target/X86/X86LowerTileCopy.cpp

…oad of tile register

AZero13 · 2024-04-20T20:17:18Z

@AtariDreams Reverse ping~

I met a problem cased by #86973 which was confused by IMPLICIT_DEF in this pass, and checked this patch can fix it. I put a commit to add the missing found = true; and optimize a bit. Please take a look, thanks!

Thank you! I just made a few changes, and I think this is ready now!

phoebewang

LGTM.

AZero13 · 2024-04-21T01:23:03Z

LGTM.

I don't have commit perms, just an FYI.

…save-reload of tile register (#83628)" This reverts commit 34acbb3. This change causes major compile-time regressions.

nikic · 2024-04-21T07:04:04Z

Reverted due to large compile-time regressions, especially for O0 builds, see http://llvm-compile-time-tracker.com/compare.php?from=b6824c9d459da059e247a60c1ebd1aeb580dacc2&to=34acbb3801515f9f41cc2d790d26072eb004ac46&stat=instructions:u.

phoebewang

Thanks @nikic!

I didn't think UsedRegs.stepBackward(MI); is such expensive. I think we can early out with below code. How can we check it before commit?

phoebewang · 2024-04-21T07:34:24Z

llvm/lib/Target/X86/X86LowerTileCopy.cpp

  bool Changed = false;

  for (MachineBasicBlock &MBB : MF) {
-    for (MachineInstr &MI : llvm::make_early_inc_range(MBB)) {
+    LiveRegUnits UsedRegs(*TRI);
+    UsedRegs.addLiveOuts(MBB);


I think we can early out the loop by checking tile registers, e.g.,

BitVector TILERegs = TRI->getAllocatableSet(MF, TRI->getRegClass(X86::TILERegClassID)); bool Changed = false; for (MachineBasicBlock &MBB : MF) { LiveRegUnits UsedRegs(*TRI); UsedRegs.addLiveOuts(MBB); // There won't be a tile copy if no tile register live out. if (!TILERegs.anyCommon(UsedRegs.getBitVector()) continue;

Should we add if (!ST.hasAMXTILE()) return false at line 75?

Yes, that's a good idea.

…oad of tile register (llvm#83628)

…save-reload of tile register (llvm#83628)" This reverts commit 34acbb3. This change causes major compile-time regressions.

…save-reload of tile register (#83628)" Fixes compile time regression in previous commit.

We need to check if `GR64Cand` a valid register before using it. Test is not needed since it's covered in llvm-test-suite. Fixes llvm#90954

We need to check if `GR64Cand` a valid register before using it. Test is not needed since it's covered in llvm-test-suite. Fixes #90954

Another bug fix for llvm#83628.

Another bug fix for #83628.

llvmbot added the backend:X86 label Mar 1, 2024

phoebewang requested a review from yubingex007-a11y March 2, 2024 12:36

RKSimon changed the title ~~Find dead register to use to prevent save-reload~~ [X86] X86LowerTileCopy - Find dead register to use to prevent save-reload Mar 2, 2024

RKSimon reviewed Mar 2, 2024

View reviewed changes

AZero13 force-pushed the Target branch from 8cfae93 to 8dae1d7 Compare March 3, 2024 18:26

AZero13 force-pushed the Target branch 3 times, most recently from 10f95c2 to edef42d Compare March 14, 2024 14:59

phoebewang reviewed Apr 20, 2024

View reviewed changes

llvm/lib/Target/X86/X86LowerTileCopy.cpp Show resolved Hide resolved

AZero13 changed the title ~~[X86] X86LowerTileCopy - Find dead register to use to prevent save-reload~~ [X86] X86LowerTileCopy: Find dead register to use to prevent save-reload of tile register Apr 20, 2024

[X86] X86LowerTileCopy: Find dead register to use to prevent save-rel…

878a0ff

…oad of tile register

AZero13 force-pushed the Target branch from d1367d6 to 878a0ff Compare April 20, 2024 20:11

AZero13 requested review from phoebewang and RKSimon April 20, 2024 20:11

phoebewang approved these changes Apr 21, 2024

View reviewed changes

phoebewang merged commit 34acbb3 into llvm:main Apr 21, 2024
3 of 4 checks passed

AZero13 deleted the Target branch April 21, 2024 03:22

nikic added a commit that referenced this pull request Apr 21, 2024

Revert "[X86] X86LowerTileCopy: Find dead register to use to prevent …

e32c4df

…save-reload of tile register (#83628)" This reverts commit 34acbb3. This change causes major compile-time regressions.

phoebewang reviewed Apr 21, 2024

View reviewed changes

KanRobert self-requested a review April 21, 2024 07:41

aniplcc pushed a commit to aniplcc/llvm-project that referenced this pull request Apr 21, 2024

[X86] X86LowerTileCopy: Find dead register to use to prevent save-rel…

a0d44cc

…oad of tile register (llvm#83628)

This was referenced Apr 22, 2024

[libc] Make fenv and math tests preserve fenv_t state #89658

Merged

[libc] Clean up alternate test framework support #89659

Merged

phoebewang added a commit that referenced this pull request Apr 29, 2024

Reland "[X86] X86LowerTileCopy: Find dead register to use to prevent …

42bc4f6

…save-reload of tile register (#83628)" Fixes compile time regression in previous commit.

phoebewang added a commit to phoebewang/llvm-project that referenced this pull request May 6, 2024

[X86][AMX] Fix a bug after llvm#83628

609666e

We need to check if `GR64Cand` a valid register before using it. Test is not needed since it's covered in llvm-test-suite. Fixes llvm#90954

phoebewang added a commit that referenced this pull request May 15, 2024

[X86][AMX] Fix a bug after #83628 (#91207)

b576a6b

We need to check if `GR64Cand` a valid register before using it. Test is not needed since it's covered in llvm-test-suite. Fixes #90954

phoebewang added a commit to phoebewang/llvm-project that referenced this pull request May 29, 2024

[X86][AMX] Check also AMX register live out for copy lowering

c64a5d9

Another bug fix for llvm#83628.

phoebewang mentioned this pull request May 29, 2024

[X86][AMX] Check also AMX register live out for copy lowering #93692

Merged

phoebewang added a commit that referenced this pull request Jun 3, 2024

[X86][AMX] Check also AMX register live out for copy lowering (#93692)

8aa33f1

Another bug fix for #83628.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[X86] X86LowerTileCopy: Find dead register to use to prevent save-reload of tile register #83628

[X86] X86LowerTileCopy: Find dead register to use to prevent save-reload of tile register #83628

Uh oh!

AZero13 commented Mar 1, 2024

Uh oh!

llvmbot commented Mar 1, 2024

Uh oh!

github-actions bot commented Mar 1, 2024

Uh oh!

RKSimon left a comment

Uh oh!

phoebewang commented Apr 20, 2024

Uh oh!

Uh oh!

AZero13 commented Apr 20, 2024

Uh oh!

phoebewang left a comment

Uh oh!

AZero13 commented Apr 21, 2024

Uh oh!

Uh oh!

nikic commented Apr 21, 2024

Uh oh!

phoebewang left a comment

Uh oh!

phoebewang Apr 21, 2024 •

edited

Loading

Uh oh!

KanRobert Apr 21, 2024

Uh oh!

phoebewang Apr 21, 2024

Uh oh!

Uh oh!

[X86] X86LowerTileCopy: Find dead register to use to prevent save-reload of tile register #83628

[X86] X86LowerTileCopy: Find dead register to use to prevent save-reload of tile register #83628

Uh oh!

Conversation

AZero13 commented Mar 1, 2024

Uh oh!

llvmbot commented Mar 1, 2024

Uh oh!

github-actions bot commented Mar 1, 2024

Uh oh!

RKSimon left a comment

Choose a reason for hiding this comment

Uh oh!

phoebewang commented Apr 20, 2024

Uh oh!

Uh oh!

AZero13 commented Apr 20, 2024

Uh oh!

phoebewang left a comment

Choose a reason for hiding this comment

Uh oh!

AZero13 commented Apr 21, 2024

Uh oh!

Uh oh!

nikic commented Apr 21, 2024

Uh oh!

phoebewang left a comment

Choose a reason for hiding this comment

Uh oh!

phoebewang Apr 21, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

KanRobert Apr 21, 2024

Choose a reason for hiding this comment

Uh oh!

phoebewang Apr 21, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

phoebewang Apr 21, 2024 •

edited

Loading