[PowerPC] $carry should be added to successors liveins if still alive after expanding ISEL #127376

bzEq · 2025-02-16T05:46:40Z

Fix bug reported in #116984.
The fix is similar to 647e861.

Fix bug reported in llvm#116984. The fix is similar to 647e861.

llvmbot · 2025-02-16T05:47:14Z

@llvm/pr-subscribers-backend-x86

@llvm/pr-subscribers-backend-powerpc

Author: Kai Luo (bzEq)

Changes

Fix bug reported in #116984.
The fix is similar to 647e861.

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

5 Files Affected:

(modified) llvm/include/llvm/CodeGen/LivePhysRegs.h (+3)
(modified) llvm/lib/CodeGen/LivePhysRegs.cpp (+22)
(modified) llvm/lib/Target/PowerPC/PPCISelLowering.cpp (+5)
(modified) llvm/lib/Target/X86/X86ISelLowering.cpp (+2-16)
(added) llvm/test/CodeGen/PowerPC/carry-liveness-after-expand-isel.ll (+83)

diff --git a/llvm/include/llvm/CodeGen/LivePhysRegs.h b/llvm/include/llvm/CodeGen/LivePhysRegs.h
index 037905119eb22..596e2b76aabb6 100644
--- a/llvm/include/llvm/CodeGen/LivePhysRegs.h
+++ b/llvm/include/llvm/CodeGen/LivePhysRegs.h
@@ -195,6 +195,9 @@ void addLiveIns(MachineBasicBlock &MBB, const LivePhysRegs &LiveRegs);
 void computeAndAddLiveIns(LivePhysRegs &LiveRegs,
                           MachineBasicBlock &MBB);
 
+/// Check if physical register \p Reg is alive after \p MBI.
+bool isPhysRegLiveAfter(Register Reg, MachineBasicBlock::iterator MBI);
+
 /// Convenience function for recomputing live-in's for a MBB. Returns true if
 /// any changes were made.
 static inline bool recomputeLiveIns(MachineBasicBlock &MBB) {
diff --git a/llvm/lib/CodeGen/LivePhysRegs.cpp b/llvm/lib/CodeGen/LivePhysRegs.cpp
index 2ba17e46be5a6..e50789d63d5d9 100644
--- a/llvm/lib/CodeGen/LivePhysRegs.cpp
+++ b/llvm/lib/CodeGen/LivePhysRegs.cpp
@@ -338,3 +338,25 @@ void llvm::computeAndAddLiveIns(LivePhysRegs &LiveRegs,
   computeLiveIns(LiveRegs, MBB);
   addLiveIns(MBB, LiveRegs);
 }
+
+bool llvm::isPhysRegLiveAfter(Register Reg, MachineBasicBlock::iterator MBI) {
+  assert(Reg.isPhysical() && "Apply to physical register only");
+
+  MachineBasicBlock *MBB = MBI->getParent();
+  // Scan forward through BB for a use/def of Reg .
+  for (const MachineInstr &MI : llvm::make_range(std::next(MBI), MBB->end())) {
+    if (MI.readsRegister(Reg, /*TRI=*/nullptr))
+      return true;
+    // If we found a def, we can stop searching.
+    if (MI.definesRegister(Reg, /*TRI=*/nullptr))
+      return false;
+  }
+
+  // If we hit the end of the block, check whether Reg is live into a
+  // successor.
+  for (MachineBasicBlock *Succ : MBB->successors())
+    if (Succ->isLiveIn(Reg))
+      return true;
+
+  return false;
+}
diff --git a/llvm/lib/Target/PowerPC/PPCISelLowering.cpp b/llvm/lib/Target/PowerPC/PPCISelLowering.cpp
index f1195feea80e8..c3baac2aa30be 100644
--- a/llvm/lib/Target/PowerPC/PPCISelLowering.cpp
+++ b/llvm/lib/Target/PowerPC/PPCISelLowering.cpp
@@ -36,6 +36,7 @@
 #include "llvm/ADT/StringRef.h"
 #include "llvm/CodeGen/CallingConvLower.h"
 #include "llvm/CodeGen/ISDOpcodes.h"
+#include "llvm/CodeGen/LivePhysRegs.h"
 #include "llvm/CodeGen/MachineBasicBlock.h"
 #include "llvm/CodeGen/MachineFrameInfo.h"
 #include "llvm/CodeGen/MachineFunction.h"
@@ -13296,6 +13297,10 @@ PPCTargetLowering::EmitInstrWithCustomInserter(MachineInstr &MI,
     DebugLoc dl = MI.getDebugLoc();
     F->insert(It, copy0MBB);
     F->insert(It, sinkMBB);
+    if (isPhysRegLiveAfter(PPC::CARRY , MI.getIterator())) {
+      copy0MBB->addLiveIn(PPC::CARRY);
+      sinkMBB->addLiveIn(PPC::CARRY);
+    }
 
     // Set the call frame size on entry to the new basic blocks.
     // See https://reviews.llvm.org/D156113.
diff --git a/llvm/lib/Target/X86/X86ISelLowering.cpp b/llvm/lib/Target/X86/X86ISelLowering.cpp
index 9592137b34842..86b7a333ccc12 100644
--- a/llvm/lib/Target/X86/X86ISelLowering.cpp
+++ b/llvm/lib/Target/X86/X86ISelLowering.cpp
@@ -28,6 +28,7 @@
 #include "llvm/Analysis/ProfileSummaryInfo.h"
 #include "llvm/Analysis/VectorUtils.h"
 #include "llvm/CodeGen/IntrinsicLowering.h"
+#include "llvm/CodeGen/LivePhysRegs.h"
 #include "llvm/CodeGen/MachineFrameInfo.h"
 #include "llvm/CodeGen/MachineFunction.h"
 #include "llvm/CodeGen/MachineInstrBuilder.h"
@@ -35349,22 +35350,7 @@ MVT X86TargetLowering::getPreferredSwitchConditionType(LLVMContext &Context,
 // basic block or any successors of the basic block.
 static bool isEFLAGSLiveAfter(MachineBasicBlock::iterator Itr,
                               MachineBasicBlock *BB) {
-  // Scan forward through BB for a use/def of EFLAGS.
-  for (const MachineInstr &mi : llvm::make_range(std::next(Itr), BB->end())) {
-    if (mi.readsRegister(X86::EFLAGS, /*TRI=*/nullptr))
-      return true;
-    // If we found a def, we can stop searching.
-    if (mi.definesRegister(X86::EFLAGS, /*TRI=*/nullptr))
-      return false;
-  }
-
-  // If we hit the end of the block, check whether EFLAGS is live into a
-  // successor.
-  for (MachineBasicBlock *Succ : BB->successors())
-    if (Succ->isLiveIn(X86::EFLAGS))
-      return true;
-
-  return false;
+  return isPhysRegLiveAfter(X86::EFLAGS, Itr);
 }
 
 /// Utility function to emit xbegin specifying the start of an RTM region.
diff --git a/llvm/test/CodeGen/PowerPC/carry-liveness-after-expand-isel.ll b/llvm/test/CodeGen/PowerPC/carry-liveness-after-expand-isel.ll
new file mode 100644
index 0000000000000..7cf326041c5ec
--- /dev/null
+++ b/llvm/test/CodeGen/PowerPC/carry-liveness-after-expand-isel.ll
@@ -0,0 +1,83 @@
+; NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py UTC_ARGS: --version 5
+; RUN: llc -stop-after=finalize-isel -verify-machineinstrs < %s | FileCheck %s
+
+target datalayout = "E-m:e-p:32:32-Fn32-i64:64-n32"
+target triple = "powerpc-unknown-linux-gnu"
+
+@md_seq_show___trans_tmp_57 = external global i8
+
+define i32 @md_seq_show(i64 %0, i32 %1) #0 {
+  ; CHECK-LABEL: name: md_seq_show
+  ; CHECK: bb.0.entry:
+  ; CHECK-NEXT:   successors: %bb.3(0x40000000), %bb.4(0x40000000)
+  ; CHECK-NEXT:   liveins: $r3, $r4, $r5
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT:   [[COPY:%[0-9]+]]:gprc = COPY $r5
+  ; CHECK-NEXT:   [[COPY1:%[0-9]+]]:gprc = COPY $r4
+  ; CHECK-NEXT:   [[COPY2:%[0-9]+]]:gprc = COPY $r3
+  ; CHECK-NEXT:   [[COPY3:%[0-9]+]]:gprc = COPY [[COPY1]]
+  ; CHECK-NEXT:   [[COPY4:%[0-9]+]]:gprc = COPY [[COPY2]]
+  ; CHECK-NEXT:   [[ADDIC:%[0-9]+]]:gprc = ADDIC [[COPY1]], 1, implicit-def $carry
+  ; CHECK-NEXT:   [[CMPLWI:%[0-9]+]]:crrc = CMPLWI killed [[ADDIC]], 1
+  ; CHECK-NEXT:   [[LI:%[0-9]+]]:gprc_and_gprc_nor0 = LI 0
+  ; CHECK-NEXT:   [[LI1:%[0-9]+]]:gprc_and_gprc_nor0 = LI 1
+  ; CHECK-NEXT:   BCC 44, [[CMPLWI]], %bb.4
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT: bb.3.entry:
+  ; CHECK-NEXT:   successors: %bb.4(0x80000000)
+  ; CHECK-NEXT:   liveins: $carry
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT: bb.4.entry:
+  ; CHECK-NEXT:   successors: %bb.5(0x40000000), %bb.6(0x40000000)
+  ; CHECK-NEXT:   liveins: $carry
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT:   [[PHI:%[0-9]+]]:gprc_and_gprc_nor0 = PHI [[LI]], %bb.3, [[LI1]], %bb.0
+  ; CHECK-NEXT:   [[ADDZE:%[0-9]+]]:gprc = ADDZE [[COPY2]], implicit-def dead $carry, implicit $carry
+  ; CHECK-NEXT:   [[ADDIC1:%[0-9]+]]:gprc = ADDIC [[ADDZE]], -1, implicit-def $carry
+  ; CHECK-NEXT:   [[SUBFE:%[0-9]+]]:gprc_and_gprc_nor0 = SUBFE killed [[ADDIC1]], [[ADDZE]], implicit-def dead $carry, implicit $carry
+  ; CHECK-NEXT:   [[CMPLWI1:%[0-9]+]]:crrc = CMPLWI [[ADDZE]], 0
+  ; CHECK-NEXT:   BCC 76, [[CMPLWI1]], %bb.6
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT: bb.5.entry:
+  ; CHECK-NEXT:   successors: %bb.6(0x80000000)
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT: bb.6.entry:
+  ; CHECK-NEXT:   successors: %bb.1(0x55555556), %bb.2(0x2aaaaaaa)
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT:   [[PHI1:%[0-9]+]]:gprc = PHI [[SUBFE]], %bb.5, [[PHI]], %bb.4
+  ; CHECK-NEXT:   [[CMPLWI2:%[0-9]+]]:crrc = CMPLWI killed [[PHI1]], 0
+  ; CHECK-NEXT:   BCC 68, killed [[CMPLWI2]], %bb.2
+  ; CHECK-NEXT:   B %bb.1
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT: bb.1.for.cond.i.preheader:
+  ; CHECK-NEXT:   [[LI2:%[0-9]+]]:gprc = LI 0
+  ; CHECK-NEXT:   $r3 = COPY [[LI2]]
+  ; CHECK-NEXT:   BLR implicit $lr, implicit $rm, implicit $r3
+  ; CHECK-NEXT: {{  $}}
+  ; CHECK-NEXT: bb.2.status_resync.exit:
+  ; CHECK-NEXT:   [[ADDIC2:%[0-9]+]]:gprc = ADDIC [[COPY]], -1, implicit-def $carry
+  ; CHECK-NEXT:   [[SUBFE1:%[0-9]+]]:gprc = SUBFE killed [[ADDIC2]], [[COPY]], implicit-def dead $carry, implicit $carry
+  ; CHECK-NEXT:   [[LIS:%[0-9]+]]:gprc_and_gprc_nor0 = LIS target-flags(ppc-ha) @md_seq_show___trans_tmp_57
+  ; CHECK-NEXT:   STB killed [[SUBFE1]], target-flags(ppc-lo) @md_seq_show___trans_tmp_57, killed [[LIS]] :: (store (s8) into @md_seq_show___trans_tmp_57)
+  ; CHECK-NEXT:   [[LI3:%[0-9]+]]:gprc = LI 0
+  ; CHECK-NEXT:   $r3 = COPY [[LI3]]
+  ; CHECK-NEXT:   BLR implicit $lr, implicit $rm, implicit $r3
+entry:
+  switch i64 %0, label %status_resync.exit [
+    i64 -1, label %for.cond.i.preheader
+    i64 0, label %for.cond.i.preheader
+  ]
+
+for.cond.i.preheader:                             ; preds = %entry, %entry
+  ret i32 0
+
+status_resync.exit:                               ; preds = %entry
+  %tobool = icmp ne i32 %1, 0
+  %storedv = zext i1 %tobool to i8
+  store i8 %storedv, ptr @md_seq_show___trans_tmp_57, align 1
+  ret i32 0
+}
+
+attributes #0 = { "target-features"="-aix-shared-lib-tls-model-opt,-aix-small-local-dynamic-tls,-aix-small-local-exec-tls,-altivec,-bpermd,-crbits,-crypto,-direct-move,-extdiv,-htm,-isa-v206-instructions,-isa-v207-instructions,-isa-v30-instructions,-power8-vector,-power9-vector,-privileged,-quadword-atomics,-rop-protect,-spe,-vsx" }
+;; NOTE: These prefixes are unused and the list is autogenerated. Do not add tests below this line:
+; CHECK: {{.*}}

github-actions · 2025-02-16T05:51:40Z

✅ With the latest revision this PR passed the C/C++ code formatter.

lei137 · 2025-02-18T21:00:11Z

llvm/lib/Target/X86/X86ISelLowering.cpp

-      return true;
-
-  return false;
+  return isPhysRegLiveAfter(X86::EFLAGS, Itr);


Can we just replace all references to isEFLAGSLiveAfter() with isPhysRegLiveAfter(X86::EFLAGS, Itr); and remove this static function?

bzEq · 2025-02-19T15:37:08Z

Hi @lei137, I'm closing it since #116984 is reverted. I think the fix in this PR might be included in the PR when relanding #116984.

[PowerPC] $carry should be added to successors liveins if still alive

abd4882

Fix bug reported in llvm#116984. The fix is similar to 647e861.

bzEq requested review from nathanchance, topperc and diggerlin February 16, 2025 05:46

llvmbot added backend:PowerPC backend:X86 labels Feb 16, 2025

Remove redundant lines

d556765

bzEq changed the title ~~[PowerPC] $carry should be added to successors liveins if still alive~~ [PowerPC] $carry should be added to successors liveins if still alive after expand ISEL Feb 16, 2025

bzEq changed the title ~~[PowerPC] $carry should be added to successors liveins if still alive after expand ISEL~~ [PowerPC] $carry should be added to successors liveins if still alive after expanding ISEL Feb 16, 2025

clang-format-region

ff081db

nathanchance mentioned this pull request Feb 18, 2025

Crash after LLVM commit 7763119c6eb0976e4836f81c9876c49a36d46d73 with 32-bit powerpc ClangBuiltLinux/linux#2070

Closed

lei137 reviewed Feb 18, 2025

View reviewed changes

nathanchance mentioned this pull request Feb 19, 2025

Boot failure with ppc64_guest_defconfig after LLVM commit 7763119c6eb0976e4836f81c9876c49a36d46d73 ClangBuiltLinux/linux#2072

Closed

bzEq mentioned this pull request Feb 19, 2025

[PowerPC] Deprecate uses of ISD::ADDC/ISD::ADDE/ISD::SUBC/ISD::SUBE #116984

Merged

bzEq closed this Feb 19, 2025

diggerlin mentioned this pull request Mar 26, 2025

[PowerPC] Deprecate uses of ISD::ADDC/ISD::ADDE/ISD::SUBC/ISD::SUBE #133155

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[PowerPC] $carry should be added to successors liveins if still alive after expanding ISEL #127376

[PowerPC] $carry should be added to successors liveins if still alive after expanding ISEL #127376

Uh oh!

bzEq commented Feb 16, 2025

Uh oh!

llvmbot commented Feb 16, 2025 •

edited

Loading

Uh oh!

github-actions bot commented Feb 16, 2025 •

edited

Loading

Uh oh!

lei137 Feb 18, 2025 •

edited

Loading

Uh oh!

bzEq commented Feb 19, 2025

Uh oh!

Uh oh!

[PowerPC] $carry should be added to successors liveins if still alive after expanding ISEL #127376

[PowerPC] $carry should be added to successors liveins if still alive after expanding ISEL #127376

Uh oh!

Conversation

bzEq commented Feb 16, 2025

Uh oh!

llvmbot commented Feb 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Feb 16, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lei137 Feb 18, 2025 • 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!

bzEq commented Feb 19, 2025

Uh oh!

Uh oh!

llvmbot commented Feb 16, 2025 •

edited

Loading

github-actions bot commented Feb 16, 2025 •

edited

Loading

lei137 Feb 18, 2025 •

edited

Loading