Skip to content

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

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

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions llvm/include/llvm/CodeGen/LivePhysRegs.h
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
22 changes: 22 additions & 0 deletions llvm/lib/CodeGen/LivePhysRegs.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
6 changes: 6 additions & 0 deletions llvm/lib/Target/PowerPC/PPCISelLowering.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -13297,6 +13298,11 @@ PPCTargetLowering::EmitInstrWithCustomInserter(MachineInstr &MI,
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.
unsigned CallFrameSize = TII->getCallFrameSizeAt(MI);
Expand Down
18 changes: 2 additions & 16 deletions llvm/lib/Target/X86/X86ISelLowering.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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);
Copy link
Contributor

@lei137 lei137 Feb 18, 2025

Choose a reason for hiding this comment

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

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

}

/// Utility function to emit xbegin specifying the start of an RTM region.
Expand Down
81 changes: 81 additions & 0 deletions llvm/test/CodeGen/PowerPC/carry-liveness-after-expand-isel.ll
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
; 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" }
Loading