Skip to content

[X86][APX] Try to replace non-NF with NF instructions when optimizeCompareInstr #130488

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.

Sign up for GitHub

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

Merged
merged 2 commits into from
Mar 10, 2025
Merged

[X86][APX] Try to replace non-NF with NF instructions when optimizeCompareInstr #130488

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3f0314d
[X86][APX] Try to replace NDD with NF instructions when optimizeCompa…
phoebewang Mar 9, 2025
1c4bc9b
Address review comments
phoebewang Mar 10, 2025
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
15 changes: 15 additions & 0 deletions llvm/lib/Target/X86/X86InstrInfo.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5352,10 +5352,12 @@ bool X86InstrInfo::optimizeCompareInstr(MachineInstr &CmpInstr, Register SrcReg,
MachineInstr *MI = nullptr;
MachineInstr *Sub = nullptr;
MachineInstr *Movr0Inst = nullptr;
SmallVector<MachineInstr *, 4> NDDInsts;
bool NoSignFlag = false;
bool ClearsOverflowFlag = false;
bool ShouldUpdateCC = false;
bool IsSwapped = false;
bool HasCF = Subtarget.hasNF();
Copy link
Contributor

@KanRobert KanRobert Mar 10, 2025

Choose a reason for hiding this comment

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

HasCF->HasNF

Copy link
Contributor Author

@phoebewang phoebewang Mar 10, 2025

Choose a reason for hiding this comment

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

Good catch! done.

unsigned OpNo = 0;
X86::CondCode NewCC = X86::COND_INVALID;
int64_t ImmDelta = 0;
Expand Down Expand Up @@ -5441,6 +5443,13 @@ bool X86InstrInfo::optimizeCompareInstr(MachineInstr &CmpInstr, Register SrcReg,
continue;
}

// Try to replace NDD with NF instructions.
if (HasCF && X86II::hasNewDataDest(Inst.getDesc().TSFlags) &&
Inst.registerDefIsDead(X86::EFLAGS, TRI)) {
NDDInsts.push_back(&Inst);
continue;
}

// Cannot do anything for any other EFLAG changes.
return false;
}
Expand Down Expand Up @@ -5637,6 +5646,12 @@ bool X86InstrInfo::optimizeCompareInstr(MachineInstr &CmpInstr, Register SrcReg,
return false;
}

// Replace NDD with NF instructions.
for (MachineInstr *NDD : NDDInsts) {
NDD->setDesc(get(X86::getNFVariant(NDD->getOpcode())));
NDD->removeOperand(NDD->getNumOperands() - 1);
}

// Make sure Sub instruction defines EFLAGS and mark the def live.
MachineOperand *FlagDef =
Sub->findRegisterDefOperand(X86::EFLAGS, /*TRI=*/nullptr);
Expand Down
28 changes: 23 additions & 5 deletions llvm/test/CodeGen/X86/apx/cf.ll
View file
Open in desktop
Copy link
Contributor

@KanRobert KanRobert Mar 10, 2025

Choose a reason for hiding this comment

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

BTW, this test should be added in nf.ll instead of cf.ll

Copy link
Contributor Author

@phoebewang phoebewang Mar 10, 2025

Choose a reason for hiding this comment

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

There's no nf.ll. The nf tests are sacttered in add/or/sub/...ll, so put it in cf.ll is ok since we happen to have nf condition lowering here :)

Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
; RUN: llc < %s -mtriple=x86_64 -mattr=+cf,+avx512f -verify-machineinstrs | FileCheck %s
; RUN: llc < %s -mtriple=x86_64 -mattr=+cf,+nf,+ndd,+avx512f -verify-machineinstrs | FileCheck %s

define void @basic(i32 %a, ptr %b, ptr %p, ptr %q) {
; CHECK-LABEL: basic:
Expand Down Expand Up @@ -57,9 +57,8 @@ entry:
define i64 @reduced_data_dependency(i64 %a, i64 %b, ptr %c) {
; CHECK-LABEL: reduced_data_dependency:
; CHECK: # %bb.0: # %entry
; CHECK-NEXT: movq %rdi, %rcx
; CHECK-NEXT: subq %rsi, %rcx
; CHECK-NEXT: cfcmovnsq (%rdx), %rdi, %rax
; CHECK-NEXT: subq %rsi, %rdi, %rax
; CHECK-NEXT: cfcmovnsq (%rdx), %rdi, %rcx
; CHECK-NEXT: addq %rcx, %rax
; CHECK-NEXT: retq
entry:
Expand Down Expand Up @@ -125,7 +124,7 @@ entry:
ret void
}

define void @single_cmp(i32 %a, i32 %b, ptr %c, ptr %d) #2 {
define void @single_cmp(i32 %a, i32 %b, ptr %c, ptr %d) {
; CHECK-LABEL: single_cmp:
; CHECK: # %bb.0: # %entry
; CHECK-NEXT: cmpl %esi, %edi
Expand All @@ -139,3 +138,22 @@ entry:
tail call void @llvm.masked.store.v1i16.p0(<1 x i16> %2, ptr %d, i32 2, <1 x i1> %1)
ret void
}

define void @load_add_store(i32 %a, i32 %b, ptr %p) {
; CHECK-LABEL: load_add_store:
; CHECK: # %bb.0: # %entry
; CHECK-NEXT: cmpl %esi, %edi
; CHECK-NEXT: cfcmovnew (%rdx), %ax
; CHECK-NEXT: {nf} incw %ax
; CHECK-NEXT: cfcmovnew %ax, (%rdx)
; CHECK-NEXT: retq
entry:
%0 = icmp ne i32 %a, %b
%1 = insertelement <1 x i1> poison, i1 %0, i64 0
%2 = tail call <1 x i16> @llvm.masked.load.v1i16.p0(ptr %p, i32 2, <1 x i1> %1, <1 x i16> poison)
%3 = extractelement <1 x i16> %2, i64 0
%4 = add i16 %3, 1
%5 = insertelement <1 x i16> poison, i16 %4, i64 0
tail call void @llvm.masked.store.v1i16.p0(<1 x i16> %5, ptr %p, i32 2, <1 x i1> %1)
ret void
}