Skip to content

[X86][GlobalIsel] fix regbank issue fabs #145674

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 4 commits into from
Jun 30, 2025
Merged

[X86][GlobalIsel] fix regbank issue fabs #145674

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
1bf7cec
[X86][GlobalIsel] fix regbank issue fabs
Jun 25, 2025
74c2324
fix 87 only legalize
Jun 27, 2025
6f2a059
Merge branch 'main' into fix_regbank_fabs
mahesh-attarde Jun 30, 2025
b86dab6
Merge branch 'main' into fix_regbank_fabs
mahesh-attarde Jun 30, 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
33 changes: 0 additions & 33 deletions llvm/lib/Target/X86/GISel/X86InstructionSelector.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,8 +95,6 @@ class X86InstructionSelector : public InstructionSelector {
MachineFunction &MF) const;
bool selectFCmp(MachineInstr &I, MachineRegisterInfo &MRI,
MachineFunction &MF) const;
bool selectFAbs(MachineInstr &I, MachineRegisterInfo &MRI,
MachineFunction &MF) const;
bool selectUAddSub(MachineInstr &I, MachineRegisterInfo &MRI,
MachineFunction &MF) const;
bool selectDebugInstr(MachineInstr &I, MachineRegisterInfo &MRI) const;
Expand Down Expand Up @@ -393,8 +391,6 @@ bool X86InstructionSelector::select(MachineInstr &I) {
switch (I.getOpcode()) {
default:
return false;
case TargetOpcode::G_FABS:
return selectFAbs(I, MRI, MF);
case TargetOpcode::G_STORE:
case TargetOpcode::G_LOAD:
return selectLoadStoreOp(I, MRI, MF);
Expand Down Expand Up @@ -1054,35 +1050,6 @@ bool X86InstructionSelector::selectCmp(MachineInstr &I,
I.eraseFromParent();
return true;
}
bool X86InstructionSelector::selectFAbs(MachineInstr &I,
MachineRegisterInfo &MRI,
MachineFunction &MF) const {
assert((I.getOpcode() == TargetOpcode::G_FABS) && "unexpected instruction");
Register SrcReg = I.getOperand(1).getReg();
Register DstReg = I.getOperand(0).getReg();
LLT Ty = MRI.getType(SrcReg);
unsigned OpAbs;
const TargetRegisterClass *DstRC;
switch (Ty.getSizeInBits()) {
default:
return false;
case 32:
OpAbs = X86::ABS_Fp32;
DstRC = &X86::FR32RegClass;
break;
case 64:
OpAbs = X86::ABS_Fp64;
DstRC = &X86::FR64RegClass;
break;
}
MRI.setRegClass(DstReg, DstRC);
MachineInstr &FAbsInst =
*BuildMI(*I.getParent(), I, I.getDebugLoc(), TII.get(OpAbs), DstReg)
.addReg(SrcReg);
constrainSelectedInstRegOperands(FAbsInst, TII, TRI, RBI);
I.eraseFromParent();
return true;
}

bool X86InstructionSelector::selectFCmp(MachineInstr &I,
MachineRegisterInfo &MRI,
Expand Down
3 changes: 2 additions & 1 deletion llvm/lib/Target/X86/GISel/X86LegalizerInfo.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -419,7 +419,8 @@ X86LegalizerInfo::X86LegalizerInfo(const X86Subtarget &STI,
.legalFor(UseX87, {s80});

getActionDefinitionsBuilder(G_FABS)
.legalFor(UseX87 && !HasSSE2 && !HasSSE1, {s64, s80})
.legalFor(UseX87 && !HasSSE2 && !HasSSE1, {s80})
Copy link
Contributor

@e-kud e-kud Jun 26, 2025

Choose a reason for hiding this comment

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

I think there is no need for SSE checks.

Copy link
Collaborator

@RKSimon RKSimon Jun 26, 2025

Choose a reason for hiding this comment

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

Agreed - x87_f80 is only available with UseX87 - SSE availability is irrelevant

Copy link
Contributor Author

@mahesh-attarde mahesh-attarde Jun 27, 2025

Choose a reason for hiding this comment

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

done.

.legalFor(UseX87 && !HasSSE2 && !HasSSE1 && !Is64Bit, {s64})
Copy link
Contributor

@e-kud e-kud Jun 26, 2025

Choose a reason for hiding this comment

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

Do we really need !HasSSE1 here? Does lower work with SSE1?

.lower();

// fp comparison
Expand Down
7 changes: 4 additions & 3 deletions llvm/lib/Target/X86/GISel/X86RegisterBankInfo.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -341,6 +341,7 @@ X86RegisterBankInfo::getInstrMapping(const MachineInstr &MI) const {
/* Predicate */ PMI_None, FpRegBank, FpRegBank};
break;
}
case TargetOpcode::G_FABS:
case TargetOpcode::G_TRUNC:
case TargetOpcode::G_ANYEXT: {
auto &Op0 = MI.getOperand(0);
Expand All @@ -354,9 +355,9 @@ X86RegisterBankInfo::getInstrMapping(const MachineInstr &MI) const {
Ty0.getSizeInBits() == 128 &&
(Ty1.getSizeInBits() == 32 || Ty1.getSizeInBits() == 64) &&
Opc == TargetOpcode::G_ANYEXT;

getInstrPartialMappingIdxs(MI, MRI, /* isFP= */ isFPTrunc || isFPAnyExt,
OpRegBankIdx);
bool isFAbs = (Opc == TargetOpcode::G_FABS);
getInstrPartialMappingIdxs(
MI, MRI, /* isFP= */ isFPTrunc || isFPAnyExt || isFAbs, OpRegBankIdx);
break;
}
case TargetOpcode::G_LOAD: {
Expand Down
77 changes: 54 additions & 23 deletions llvm/test/CodeGen/X86/isel-fabs-x87.ll
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,36 +1,67 @@
; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py UTC_ARGS: --version 5
; RUN: llc < %s -mtriple=x86_64-- -mattr=+x87,-sse2,-sse | FileCheck %s --check-prefixes=X64
; RUN: llc < %s -mtriple=x86_64-- -mattr=+x87,-sse2,-sse -fast-isel | FileCheck %s --check-prefixes=X64
; RUN: llc < %s -mtriple=x86_64-- -mattr=+x87,-sse2,-sse -global-isel -global-isel-abort=1 | FileCheck %s --check-prefixes=X64
; RUN: llc < %s -mtriple=i686-- -mattr=+x87,-sse2,-sse | FileCheck %s --check-prefixes=X86,SDAG-ISEL
; RUN: llc < %s -mtriple=i686-- -mattr=+x87,-sse2,-sse -fast-isel | FileCheck %s --check-prefixes=X86,Fast-ISEL
; RUN: llc < %s -mtriple=i686-- -mattr=+x87,-sse2,-sse -global-isel -global-isel-abort=0 | FileCheck %s --check-prefixes=X86,GISEL-ISEL
; RUN: llc < %s -mtriple=x86_64-- -mattr=+x87,-sse2,-sse | FileCheck %s --check-prefixes=X64,SDAG-X64-ISEL
; RUN: llc < %s -mtriple=x86_64-- -mattr=+x87,-sse2,-sse -fast-isel | FileCheck %s --check-prefixes=X64,FAST-X64-ISEL
; RUN: llc < %s -mtriple=x86_64-- -mattr=+x87,-sse2,-sse -global-isel -global-isel-abort=1 | FileCheck %s --check-prefixes=X64,GISEL-X64-ISEL
; RUN: llc < %s -mtriple=i686-- -mattr=+x87,-sse2,-sse | FileCheck %s --check-prefixes=X86,SDAG-X86-ISEL
; RUN: llc < %s -mtriple=i686-- -mattr=+x87,-sse2,-sse -fast-isel | FileCheck %s --check-prefixes=X86,FAST-X86-ISEL
; RUN: llc < %s -mtriple=i686-- -mattr=+x87,-sse2,-sse -global-isel -global-isel-abort=0 | FileCheck %s --check-prefixes=X86,GISEL-X86-ISEL

define void @test_float_abs(ptr %argptr) {
; SDAG-ISEL-LABEL: test_float_abs:
; SDAG-ISEL: # %bb.0:
; SDAG-ISEL-NEXT: movl {{[0-9]+}}(%esp), %eax
; SDAG-ISEL-NEXT: andb $127, 3(%eax)
; SDAG-ISEL-NEXT: retl
;
; Fast-ISEL-LABEL: test_float_abs:
; Fast-ISEL: # %bb.0:
; Fast-ISEL-NEXT: movl {{[0-9]+}}(%esp), %eax
; Fast-ISEL-NEXT: andb $127, 3(%eax)
; Fast-ISEL-NEXT: retl
;
; GISEL-ISEL-LABEL: test_float_abs:
; GISEL-ISEL: # %bb.0:
; GISEL-ISEL-NEXT: movl {{[0-9]+}}(%esp), %eax
; GISEL-ISEL-NEXT: andl $2147483647, (%eax) # imm = 0x7FFFFFFF
; GISEL-ISEL-NEXT: retl
; SDAG-X64-ISEL-LABEL: test_float_abs:
; SDAG-X64-ISEL: # %bb.0:
; SDAG-X64-ISEL-NEXT: andb $127, 3(%rdi)
; SDAG-X64-ISEL-NEXT: retq
;
; FAST-X64-ISEL-LABEL: test_float_abs:
; FAST-X64-ISEL: # %bb.0:
; FAST-X64-ISEL-NEXT: andb $127, 3(%rdi)
; FAST-X64-ISEL-NEXT: retq
;
; GISEL-X64-ISEL-LABEL: test_float_abs:
; GISEL-X64-ISEL: # %bb.0:
; GISEL-X64-ISEL-NEXT: andl $2147483647, (%rdi) # imm = 0x7FFFFFFF
; GISEL-X64-ISEL-NEXT: retq
;
; SDAG-X86-ISEL-LABEL: test_float_abs:
; SDAG-X86-ISEL: # %bb.0:
; SDAG-X86-ISEL-NEXT: movl {{[0-9]+}}(%esp), %eax
; SDAG-X86-ISEL-NEXT: andb $127, 3(%eax)
; SDAG-X86-ISEL-NEXT: retl
;
; FAST-X86-ISEL-LABEL: test_float_abs:
; FAST-X86-ISEL: # %bb.0:
; FAST-X86-ISEL-NEXT: movl {{[0-9]+}}(%esp), %eax
; FAST-X86-ISEL-NEXT: andb $127, 3(%eax)
; FAST-X86-ISEL-NEXT: retl
;
; GISEL-X86-ISEL-LABEL: test_float_abs:
; GISEL-X86-ISEL: # %bb.0:
; GISEL-X86-ISEL-NEXT: movl {{[0-9]+}}(%esp), %eax
; GISEL-X86-ISEL-NEXT: andl $2147483647, (%eax) # imm = 0x7FFFFFFF
; GISEL-X86-ISEL-NEXT: retl
%arg = load float, float* %argptr
%abs = tail call float @llvm.fabs.f32(float %arg)
store float %abs, ptr %argptr
ret void
}

define void @test_double_abs(ptr %argptr) {
; SDAG-X64-ISEL-LABEL: test_double_abs:
; SDAG-X64-ISEL: # %bb.0:
; SDAG-X64-ISEL-NEXT: andb $127, 7(%rdi)
; SDAG-X64-ISEL-NEXT: retq
;
; FAST-X64-ISEL-LABEL: test_double_abs:
; FAST-X64-ISEL: # %bb.0:
; FAST-X64-ISEL-NEXT: andb $127, 7(%rdi)
; FAST-X64-ISEL-NEXT: retq
;
; GISEL-X64-ISEL-LABEL: test_double_abs:
; GISEL-X64-ISEL: # %bb.0:
; GISEL-X64-ISEL-NEXT: movabsq $9223372036854775807, %rax # imm = 0x7FFFFFFFFFFFFFFF
; GISEL-X64-ISEL-NEXT: andq %rax, (%rdi)
; GISEL-X64-ISEL-NEXT: retq
;
; X86-LABEL: test_double_abs:
; X86: # %bb.0:
; X86-NEXT: movl {{[0-9]+}}(%esp), %eax
Expand Down
Loading