This is an archive of the discontinued LLVM Phabricator instance.

Differential D155502

[RISCV] Do not use FPR registers for fastcc if zfh/f/d is not specified in the architecture
ClosedPublic

Authored by eopXD on Jul 17 2023, 11:29 AM.

Details

Reviewers
craig.topper
kito-cheng
asb
Commits
rGeb89bf8d0d10: [RISCV] Do not use FPR registers for fastcc if zfh/f/d is not specified in the…
Summary

Resolves #63917.

Also lets the compiler check for available GPR before hitting the stack.

Diff Detail

Unit TestsFailed

TimeTest
270 msx64 debian > LLVM.CodeGen/RISCV::fastcc-float.ll

Event Timeline

eopXD created this revision.Jul 17 2023, 11:29 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 17 2023, 11:29 AM
Herald added subscribers: jobnoorman, luke, VincentWu and 27 others. · View Herald Transcript
eopXD requested review of this revision.Jul 17 2023, 11:29 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 17 2023, 11:29 AM
Herald added subscribers: llvm-commits, wangpc, MaskRay. · View Herald Transcript
craig.topper added inline comments.Jul 17 2023, 11:32 AM
llvm/lib/Target/RISCV/RISCVISelLowering.cpp
15154

I think you meant Zfh? But even then you probably mean Zfh || Zfhmin?

craig.topper added a comment.Jul 17 2023, 11:32 AM

Test?

eopXD updated this revision to Diff 541159.Jul 17 2023, 11:38 AM
eopXD marked an inline comment as done.

Add test case and fix if-condition for f16.

llvm/lib/Target/RISCV/RISCVISelLowering.cpp
15154

Yes indeed, I keep mistaking them. Thank you for correcting me.

craig.topper added inline comments.Jul 17 2023, 11:42 AM
llvm/test/CodeGen/RISCV/fastcc-without-f-reg.ll
8

Drop dso_local and local_unnamed_addr. but add nowind

eopXD updated this revision to Diff 541164.Jul 17 2023, 11:45 AM

Improve test case

  • Drop dso_local and local_unnamed_addr. but add nowind
  • Add test case for zdinx
eopXD marked an inline comment as done.Jul 17 2023, 11:45 AM
craig.topper added inline comments.Jul 17 2023, 12:05 PM
llvm/lib/Target/RISCV/RISCVISelLowering.cpp
15200

If we skip the FPRs above, should we be going through GPR before we hit this stack code?

eopXD updated this revision to Diff 541183.Jul 17 2023, 12:23 PM

Let the compiler check for available GPR-s before hitting the spec.

eopXD edited the summary of this revision. (Show Details)Jul 17 2023, 12:24 PM
eopXD marked an inline comment as done.
eopXD mentioned this in D155507: [RISCV] Use the stack for MVT::f16 for fastcc when there are no other registers available .Jul 17 2023, 12:30 PM
craig.topper retitled this revision from [RISCV] Do not use F registers if zvfh/f/d is not specified in the architecture to [RISCV] Do not use FPR registers for fastcc if zfh/f/d is not specified in the architecture.Jul 17 2023, 12:37 PM
craig.topper added a reviewer: asb.
nigelp-xmos added a subscriber: nigelp-xmos.Jul 17 2023, 1:12 PM
asb accepted this revision.Jul 17 2023, 1:15 PM

LGTM.

llvm/lib/Target/RISCV/RISCVISelLowering.cpp
15191

nit: end with full stop and "an available".

This revision is now accepted and ready to land.Jul 17 2023, 1:15 PM
Harbormaster completed remote builds in B245970: Diff 541183.Jul 17 2023, 5:49 PM
eopXD updated this revision to Diff 541379.EditedJul 18 2023, 1:07 AM
eopXD marked an inline comment as done.

Fix if-condition, use GPR register when zhinx/zfinx/zdinx is available.

Harbormaster completed remote builds in B246116: Diff 541379.Jul 18 2023, 5:22 AM
Closed by commit rGeb89bf8d0d10: [RISCV] Do not use FPR registers for fastcc if zfh/f/d is not specified in the… (authored by eopXD). · Explain WhyJul 18 2023, 10:03 AM
This revision was automatically updated to reflect the committed changes.
eopXD added a commit: rGeb89bf8d0d10: [RISCV] Do not use FPR registers for fastcc if zfh/f/d is not specified in the….
eopXD mentioned this in rG32c257d384f3: [RISCV] Use the stack for MVT::f16 for fastcc when there are no other registers….Jul 18 2023, 7:49 PM

Revision Contents

PathSize
llvm/
lib/
Target/
RISCV/
18 lines
test/
CodeGen/
RISCV/
218 lines

Diff 541183

llvm/lib/Target/RISCV/RISCVISelLowering.cpp

  • This file is larger than 256 KB, so syntax highlighting is disabled by default.
Show First 20 Lines • Show All 15,143 Lines • ▼ Show 20 Linesbool RISCV::CC_RISCV_FastCC(const DataLayout &DL, RISCVABI::ABI ABI,
if (LocVT == MVT::i32 || LocVT == MVT::i64) { if (LocVT == MVT::i32 || LocVT == MVT::i64) {
if (unsigned Reg = State.AllocateReg(GPRList)) { if (unsigned Reg = State.AllocateReg(GPRList)) {
State.addLoc(CCValAssign::getReg(ValNo, ValVT, Reg, LocVT, LocInfo)); State.addLoc(CCValAssign::getReg(ValNo, ValVT, Reg, LocVT, LocInfo));
return false; return false;
} }
} }
if (LocVT == MVT::f16) { const RISCVSubtarget &Subtarget = TLI.getSubtarget();
if (LocVT == MVT::f16 &&
craig.topperUnsubmitted
Done
ReplyInline Actions

I think you meant Zfh? But even then you probably mean Zfh || Zfhmin?

craig.topper: I think you meant Zfh? But even then you probably mean Zfh || Zfhmin?
eopXDAuthorUnsubmitted
Done
ReplyInline Actions

Yes indeed, I keep mistaking them. Thank you for correcting me.

eopXD: Yes indeed, I keep mistaking them. Thank you for correcting me.
(Subtarget.hasStdExtZfh() || Subtarget.hasStdExtZfhmin())) {
static const MCPhysReg FPR16List[] = { static const MCPhysReg FPR16List[] = {
RISCV::F10_H, RISCV::F11_H, RISCV::F12_H, RISCV::F13_H, RISCV::F14_H, RISCV::F10_H, RISCV::F11_H, RISCV::F12_H, RISCV::F13_H, RISCV::F14_H,
RISCV::F15_H, RISCV::F16_H, RISCV::F17_H, RISCV::F0_H, RISCV::F1_H, RISCV::F15_H, RISCV::F16_H, RISCV::F17_H, RISCV::F0_H, RISCV::F1_H,
RISCV::F2_H, RISCV::F3_H, RISCV::F4_H, RISCV::F5_H, RISCV::F6_H, RISCV::F2_H, RISCV::F3_H, RISCV::F4_H, RISCV::F5_H, RISCV::F6_H,
RISCV::F7_H, RISCV::F28_H, RISCV::F29_H, RISCV::F30_H, RISCV::F31_H}; RISCV::F7_H, RISCV::F28_H, RISCV::F29_H, RISCV::F30_H, RISCV::F31_H};
if (unsigned Reg = State.AllocateReg(FPR16List)) { if (unsigned Reg = State.AllocateReg(FPR16List)) {
State.addLoc(CCValAssign::getReg(ValNo, ValVT, Reg, LocVT, LocInfo)); State.addLoc(CCValAssign::getReg(ValNo, ValVT, Reg, LocVT, LocInfo));
return false; return false;
} }
} }
if (LocVT == MVT::f32) { if (LocVT == MVT::f32 && Subtarget.hasStdExtF()) {
static const MCPhysReg FPR32List[] = { static const MCPhysReg FPR32List[] = {
RISCV::F10_F, RISCV::F11_F, RISCV::F12_F, RISCV::F13_F, RISCV::F14_F, RISCV::F10_F, RISCV::F11_F, RISCV::F12_F, RISCV::F13_F, RISCV::F14_F,
RISCV::F15_F, RISCV::F16_F, RISCV::F17_F, RISCV::F0_F, RISCV::F1_F, RISCV::F15_F, RISCV::F16_F, RISCV::F17_F, RISCV::F0_F, RISCV::F1_F,
RISCV::F2_F, RISCV::F3_F, RISCV::F4_F, RISCV::F5_F, RISCV::F6_F, RISCV::F2_F, RISCV::F3_F, RISCV::F4_F, RISCV::F5_F, RISCV::F6_F,
RISCV::F7_F, RISCV::F28_F, RISCV::F29_F, RISCV::F30_F, RISCV::F31_F}; RISCV::F7_F, RISCV::F28_F, RISCV::F29_F, RISCV::F30_F, RISCV::F31_F};
if (unsigned Reg = State.AllocateReg(FPR32List)) { if (unsigned Reg = State.AllocateReg(FPR32List)) {
State.addLoc(CCValAssign::getReg(ValNo, ValVT, Reg, LocVT, LocInfo)); State.addLoc(CCValAssign::getReg(ValNo, ValVT, Reg, LocVT, LocInfo));
return false; return false;
} }
} }
if (LocVT == MVT::f64) { if (LocVT == MVT::f64 && Subtarget.hasStdExtD()) {
static const MCPhysReg FPR64List[] = { static const MCPhysReg FPR64List[] = {
RISCV::F10_D, RISCV::F11_D, RISCV::F12_D, RISCV::F13_D, RISCV::F14_D, RISCV::F10_D, RISCV::F11_D, RISCV::F12_D, RISCV::F13_D, RISCV::F14_D,
RISCV::F15_D, RISCV::F16_D, RISCV::F17_D, RISCV::F0_D, RISCV::F1_D, RISCV::F15_D, RISCV::F16_D, RISCV::F17_D, RISCV::F0_D, RISCV::F1_D,
RISCV::F2_D, RISCV::F3_D, RISCV::F4_D, RISCV::F5_D, RISCV::F6_D, RISCV::F2_D, RISCV::F3_D, RISCV::F4_D, RISCV::F5_D, RISCV::F6_D,
RISCV::F7_D, RISCV::F28_D, RISCV::F29_D, RISCV::F30_D, RISCV::F31_D}; RISCV::F7_D, RISCV::F28_D, RISCV::F29_D, RISCV::F30_D, RISCV::F31_D};
if (unsigned Reg = State.AllocateReg(FPR64List)) { if (unsigned Reg = State.AllocateReg(FPR64List)) {
State.addLoc(CCValAssign::getReg(ValNo, ValVT, Reg, LocVT, LocInfo)); State.addLoc(CCValAssign::getReg(ValNo, ValVT, Reg, LocVT, LocInfo));
return false; return false;
} }
} }
// Check if there is available GPR before hitting the stack
asbUnsubmitted
Done
ReplyInline Actions

nit: end with full stop and "an available".

asb: nit: end with full stop and "an available".
if (LocVT == MVT::f16 || LocVT == MVT::f32 ||
(LocVT == MVT::f64 && Subtarget.is64Bit())) {
if (unsigned Reg = State.AllocateReg(GPRList)) {
State.addLoc(CCValAssign::getReg(ValNo, ValVT, Reg, LocVT, LocInfo));
return false;
}
}
if (LocVT == MVT::i32 || LocVT == MVT::f32) { if (LocVT == MVT::i32 || LocVT == MVT::f32) {
craig.topperUnsubmitted
Done
ReplyInline Actions

If we skip the FPRs above, should we be going through GPR before we hit this stack code?

craig.topper: If we skip the FPRs above, should we be going through GPR before we hit this stack code?
unsigned Offset4 = State.AllocateStack(4, Align(4)); unsigned Offset4 = State.AllocateStack(4, Align(4));
State.addLoc(CCValAssign::getMem(ValNo, ValVT, Offset4, LocVT, LocInfo)); State.addLoc(CCValAssign::getMem(ValNo, ValVT, Offset4, LocVT, LocInfo));
return false; return false;
} }
if (LocVT == MVT::i64 || LocVT == MVT::f64) { if (LocVT == MVT::i64 || LocVT == MVT::f64) {
unsigned Offset5 = State.AllocateStack(8, Align(8)); unsigned Offset5 = State.AllocateStack(8, Align(8));
State.addLoc(CCValAssign::getMem(ValNo, ValVT, Offset5, LocVT, LocInfo)); State.addLoc(CCValAssign::getMem(ValNo, ValVT, Offset5, LocVT, LocInfo));
▲ Show 20 Lines • Show All 2,257 Lines • Show Last 20 Lines

llvm/test/CodeGen/RISCV/fastcc-without-f-reg.ll

  • This file was added.
; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py UTC_ARGS: --version 2
; RUN: llc -mtriple=riscv32 -mattr=+zhinx -verify-machineinstrs < %s \
; RUN: | FileCheck -check-prefix=ZHINX32 %s
; RUN: llc -mtriple=riscv64 -mattr=+zhinx -verify-machineinstrs < %s \
; RUN: | FileCheck -check-prefix=ZHINX64 %s
; RUN: llc -mtriple=riscv32 -mattr=+zfinx -verify-machineinstrs < %s \
; RUN: | FileCheck -check-prefix=ZFINX32 %s
; RUN: llc -mtriple=riscv64 -mattr=+zfinx -verify-machineinstrs < %s \
craig.topperUnsubmitted
Done
ReplyInline Actions

Drop dso_local and local_unnamed_addr. but add nowind

craig.topper: Drop dso_local and local_unnamed_addr. but add `nowind`
; RUN: | FileCheck -check-prefix=ZFINX64 %s
; RUN: llc -mtriple=riscv32 -mattr=+zdinx -verify-machineinstrs < %s \
; RUN: | FileCheck -check-prefix=ZDINX32 %s
; RUN: llc -mtriple=riscv64 -mattr=+zdinx -verify-machineinstrs < %s \
; RUN: | FileCheck -check-prefix=ZDINX64 %s
define half @caller_half(half %x) nounwind {
; ZHINX32-LABEL: caller_half:
; ZHINX32: # %bb.0: # %entry
; ZHINX32-NEXT: tail h
;
; ZHINX64-LABEL: caller_half:
; ZHINX64: # %bb.0: # %entry
; ZHINX64-NEXT: tail h
;
; ZFINX32-LABEL: caller_half:
; ZFINX32: # %bb.0: # %entry
; ZFINX32-NEXT: lui a1, 1048560
; ZFINX32-NEXT: or a0, a0, a1
; ZFINX32-NEXT: tail h
;
; ZFINX64-LABEL: caller_half:
; ZFINX64: # %bb.0: # %entry
; ZFINX64-NEXT: lui a1, 1048560
; ZFINX64-NEXT: or a0, a0, a1
; ZFINX64-NEXT: tail h
;
; ZDINX32-LABEL: caller_half:
; ZDINX32: # %bb.0: # %entry
; ZDINX32-NEXT: lui a1, 1048560
; ZDINX32-NEXT: or a0, a0, a1
; ZDINX32-NEXT: tail h
;
; ZDINX64-LABEL: caller_half:
; ZDINX64: # %bb.0: # %entry
; ZDINX64-NEXT: lui a1, 1048560
; ZDINX64-NEXT: or a0, a0, a1
; ZDINX64-NEXT: tail h
entry:
%0 = tail call fastcc half @h(half %x)
ret half %0
}
define internal fastcc half @h(half %x) nounwind {
; ZHINX32-LABEL: h:
; ZHINX32: # %bb.0:
; ZHINX32-NEXT: ret
;
; ZHINX64-LABEL: h:
; ZHINX64: # %bb.0:
; ZHINX64-NEXT: ret
;
; ZFINX32-LABEL: h:
; ZFINX32: # %bb.0:
; ZFINX32-NEXT: lui a1, 1048560
; ZFINX32-NEXT: or a0, a0, a1
; ZFINX32-NEXT: ret
;
; ZFINX64-LABEL: h:
; ZFINX64: # %bb.0:
; ZFINX64-NEXT: lui a1, 1048560
; ZFINX64-NEXT: or a0, a0, a1
; ZFINX64-NEXT: ret
;
; ZDINX32-LABEL: h:
; ZDINX32: # %bb.0:
; ZDINX32-NEXT: lui a1, 1048560
; ZDINX32-NEXT: or a0, a0, a1
; ZDINX32-NEXT: ret
;
; ZDINX64-LABEL: h:
; ZDINX64: # %bb.0:
; ZDINX64-NEXT: lui a1, 1048560
; ZDINX64-NEXT: or a0, a0, a1
; ZDINX64-NEXT: ret
ret half %x
}
define float @caller_float(float %x) nounwind {
; ZHINX32-LABEL: caller_float:
; ZHINX32: # %bb.0: # %entry
; ZHINX32-NEXT: tail f
;
; ZHINX64-LABEL: caller_float:
; ZHINX64: # %bb.0: # %entry
; ZHINX64-NEXT: tail f
;
; ZFINX32-LABEL: caller_float:
; ZFINX32: # %bb.0: # %entry
; ZFINX32-NEXT: tail f
;
; ZFINX64-LABEL: caller_float:
; ZFINX64: # %bb.0: # %entry
; ZFINX64-NEXT: tail f
;
; ZDINX32-LABEL: caller_float:
; ZDINX32: # %bb.0: # %entry
; ZDINX32-NEXT: tail f
;
; ZDINX64-LABEL: caller_float:
; ZDINX64: # %bb.0: # %entry
; ZDINX64-NEXT: tail f
entry:
%0 = tail call fastcc float @f(float %x)
ret float %0
}
define internal fastcc float @f(float %x) nounwind {
; ZHINX32-LABEL: f:
; ZHINX32: # %bb.0: # %entry
; ZHINX32-NEXT: ret
;
; ZHINX64-LABEL: f:
; ZHINX64: # %bb.0: # %entry
; ZHINX64-NEXT: ret
;
; ZFINX32-LABEL: f:
; ZFINX32: # %bb.0: # %entry
; ZFINX32-NEXT: ret
;
; ZFINX64-LABEL: f:
; ZFINX64: # %bb.0: # %entry
; ZFINX64-NEXT: ret
;
; ZDINX32-LABEL: f:
; ZDINX32: # %bb.0: # %entry
; ZDINX32-NEXT: ret
;
; ZDINX64-LABEL: f:
; ZDINX64: # %bb.0: # %entry
; ZDINX64-NEXT: ret
entry:
ret float %x
}
define double @caller_double(double %x) nounwind {
; ZHINX32-LABEL: caller_double:
; ZHINX32: # %bb.0: # %entry
; ZHINX32-NEXT: tail d
;
; ZHINX64-LABEL: caller_double:
; ZHINX64: # %bb.0: # %entry
; ZHINX64-NEXT: tail d
;
; ZFINX32-LABEL: caller_double:
; ZFINX32: # %bb.0: # %entry
; ZFINX32-NEXT: tail d
;
; ZFINX64-LABEL: caller_double:
; ZFINX64: # %bb.0: # %entry
; ZFINX64-NEXT: tail d
;
; ZDINX32-LABEL: caller_double:
; ZDINX32: # %bb.0: # %entry
; ZDINX32-NEXT: addi sp, sp, -32
; ZDINX32-NEXT: sw ra, 28(sp) # 4-byte Folded Spill
; ZDINX32-NEXT: sw a0, 16(sp)
; ZDINX32-NEXT: sw a1, 20(sp)
; ZDINX32-NEXT: lw a0, 16(sp)
; ZDINX32-NEXT: lw a1, 20(sp)
; ZDINX32-NEXT: sw a0, 0(sp)
; ZDINX32-NEXT: sw a1, 4(sp)
; ZDINX32-NEXT: call d
; ZDINX32-NEXT: lw ra, 28(sp) # 4-byte Folded Reload
; ZDINX32-NEXT: addi sp, sp, 32
; ZDINX32-NEXT: ret
;
; ZDINX64-LABEL: caller_double:
; ZDINX64: # %bb.0: # %entry
; ZDINX64-NEXT: tail d
entry:
%0 = tail call fastcc double @d(double %x)
ret double %0
}
define internal fastcc double @d(double %x) nounwind {
; ZHINX32-LABEL: d:
; ZHINX32: # %bb.0: # %entry
; ZHINX32-NEXT: ret
;
; ZHINX64-LABEL: d:
; ZHINX64: # %bb.0: # %entry
; ZHINX64-NEXT: ret
;
; ZFINX32-LABEL: d:
; ZFINX32: # %bb.0: # %entry
; ZFINX32-NEXT: ret
;
; ZFINX64-LABEL: d:
; ZFINX64: # %bb.0: # %entry
; ZFINX64-NEXT: ret
;
; ZDINX32-LABEL: d:
; ZDINX32: # %bb.0: # %entry
; ZDINX32-NEXT: addi sp, sp, -16
; ZDINX32-NEXT: lw a0, 16(sp)
; ZDINX32-NEXT: lw a1, 20(sp)
; ZDINX32-NEXT: sw a0, 8(sp)
; ZDINX32-NEXT: sw a1, 12(sp)
; ZDINX32-NEXT: lw a0, 8(sp)
; ZDINX32-NEXT: lw a1, 12(sp)
; ZDINX32-NEXT: addi sp, sp, 16
; ZDINX32-NEXT: ret
;
; ZDINX64-LABEL: d:
; ZDINX64: # %bb.0: # %entry
; ZDINX64-NEXT: ret
entry:
ret double %x
}