[llvm] [RISCV][GISel] Add a special case to selectCopy for FPR32<->GPR on RV64. (PR #70526)
Craig Topper via llvm-commits
llvm-commits at lists.llvm.org
Fri Oct 27 17:15:21 PDT 2023
https://github.com/topperc created https://github.com/llvm/llvm-project/pull/70526
copyPhysReg can't copy a FPR32 to a 64-bit GPR since the sizes don't
match. Replace with FMV_W_X/FMV_X_W during GISel while we still have the
size information.
This is stacked on #70525
>From 79a6d18386666d4e2e91e9d2d9bb905d318d4dda Mon Sep 17 00:00:00 2001
From: Craig Topper <craig.topper at sifive.com>
Date: Fri, 27 Oct 2023 15:13:11 -0700
Subject: [PATCH 1/4] [RISCV] Teach copyPhysReg to allow copies between
GPR<->FPR32/FPR64 when the GPR is the same size.
This is needed because GISel emits copies instead of bitcasts like SelectionDAG.
I've restricted to the case where size matches in this patch. GISel
can also emit copies from FPR32->GPR on RV64, but maybe that should be handled
in GISel itself.
---
llvm/lib/Target/RISCV/RISCVInstrInfo.cpp | 28 +++++++++++++++++++
.../RISCV/GlobalISel/fpr-gpr-copy-rv32.ll | 19 +++++++++++++
.../RISCV/GlobalISel/fpr-gpr-copy-rv64.ll | 19 +++++++++++++
3 files changed, 66 insertions(+)
create mode 100644 llvm/test/CodeGen/RISCV/GlobalISel/fpr-gpr-copy-rv32.ll
create mode 100644 llvm/test/CodeGen/RISCV/GlobalISel/fpr-gpr-copy-rv64.ll
diff --git a/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp b/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp
index bfe43bae7cb12a5..7b76504b7e8fc2e 100644
--- a/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp
+++ b/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp
@@ -471,6 +471,34 @@ void RISCVInstrInfo::copyPhysReg(MachineBasicBlock &MBB,
return;
}
+ if (STI.getXLen() == 32 && RISCV::FPR32RegClass.contains(DstReg) &&
+ RISCV::GPRRegClass.contains(SrcReg)) {
+ BuildMI(MBB, MBBI, DL, get(RISCV::FMV_W_X), DstReg)
+ .addReg(SrcReg, getKillRegState(KillSrc));
+ return;
+ }
+
+ if (STI.getXLen() == 32 && RISCV::GPRRegClass.contains(DstReg) &&
+ RISCV::FPR32RegClass.contains(SrcReg)) {
+ BuildMI(MBB, MBBI, DL, get(RISCV::FMV_X_W), DstReg)
+ .addReg(SrcReg, getKillRegState(KillSrc));
+ return;
+ }
+
+ if (STI.getXLen() == 64 && RISCV::FPR64RegClass.contains(DstReg) &&
+ RISCV::GPRRegClass.contains(SrcReg)) {
+ BuildMI(MBB, MBBI, DL, get(RISCV::FMV_D_X), DstReg)
+ .addReg(SrcReg, getKillRegState(KillSrc));
+ return;
+ }
+
+ if (STI.getXLen() == 64 && RISCV::GPRRegClass.contains(DstReg) &&
+ RISCV::FPR64RegClass.contains(SrcReg)) {
+ BuildMI(MBB, MBBI, DL, get(RISCV::FMV_X_D), DstReg)
+ .addReg(SrcReg, getKillRegState(KillSrc));
+ return;
+ }
+
// VR->VR copies.
if (RISCV::VRRegClass.contains(DstReg, SrcReg)) {
copyPhysRegVector(MBB, MBBI, DL, DstReg, SrcReg, KillSrc, RISCV::VMV1R_V);
diff --git a/llvm/test/CodeGen/RISCV/GlobalISel/fpr-gpr-copy-rv32.ll b/llvm/test/CodeGen/RISCV/GlobalISel/fpr-gpr-copy-rv32.ll
new file mode 100644
index 000000000000000..1757e5550f81aeb
--- /dev/null
+++ b/llvm/test/CodeGen/RISCV/GlobalISel/fpr-gpr-copy-rv32.ll
@@ -0,0 +1,19 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py UTC_ARGS: --version 3
+; RUN: llc -mtriple=riscv32 -global-isel -mattr=+f -target-abi=ilp32 \
+; RUN: -verify-machineinstrs < %s | FileCheck -check-prefix=RV32I %s
+
+; Test copying between FPR32 and GPR on RV32.
+; FIXME: This test should be replaced with a more general calling convention
+; test once we have more FP implemented.
+
+define float @fadd(float %x, float %y) {
+; RV32I-LABEL: fadd:
+; RV32I: # %bb.0:
+; RV32I-NEXT: fmv.w.x fa5, a0
+; RV32I-NEXT: fmv.w.x fa4, a1
+; RV32I-NEXT: fadd.s fa5, fa5, fa4
+; RV32I-NEXT: fmv.x.w a0, fa5
+; RV32I-NEXT: ret
+ %a = fadd float %x, %y
+ ret float %a
+}
diff --git a/llvm/test/CodeGen/RISCV/GlobalISel/fpr-gpr-copy-rv64.ll b/llvm/test/CodeGen/RISCV/GlobalISel/fpr-gpr-copy-rv64.ll
new file mode 100644
index 000000000000000..6155e73a855a479
--- /dev/null
+++ b/llvm/test/CodeGen/RISCV/GlobalISel/fpr-gpr-copy-rv64.ll
@@ -0,0 +1,19 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py UTC_ARGS: --version 3
+; RUN: llc -mtriple=riscv64 -global-isel -mattr=+d -target-abi=lp64 \
+; RUN: -verify-machineinstrs < %s | FileCheck -check-prefix=RV32I %s
+
+; Test copying between FPR32 and GPR on RV32.
+; FIXME: This test should be replaced with a more general calling convention
+; test once we have more FP implemented.
+
+define double @fadd(double %x, double %y) {
+; RV32I-LABEL: fadd:
+; RV32I: # %bb.0:
+; RV32I-NEXT: fmv.d.x fa5, a0
+; RV32I-NEXT: fmv.d.x fa4, a1
+; RV32I-NEXT: fadd.d fa5, fa5, fa4
+; RV32I-NEXT: fmv.x.d a0, fa5
+; RV32I-NEXT: ret
+ %a = fadd double %x, %y
+ ret double %a
+}
>From 4b52934b4497c5f4ae6342c050416255991d8f7c Mon Sep 17 00:00:00 2001
From: Craig Topper <craig.topper at sifive.com>
Date: Fri, 27 Oct 2023 15:15:49 -0700
Subject: [PATCH 2/4] !fixup clang-format
---
llvm/lib/Target/RISCV/RISCVInstrInfo.cpp | 8 ++++----
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp b/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp
index 7b76504b7e8fc2e..91575bd0dd174b0 100644
--- a/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp
+++ b/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp
@@ -474,28 +474,28 @@ void RISCVInstrInfo::copyPhysReg(MachineBasicBlock &MBB,
if (STI.getXLen() == 32 && RISCV::FPR32RegClass.contains(DstReg) &&
RISCV::GPRRegClass.contains(SrcReg)) {
BuildMI(MBB, MBBI, DL, get(RISCV::FMV_W_X), DstReg)
- .addReg(SrcReg, getKillRegState(KillSrc));
+ .addReg(SrcReg, getKillRegState(KillSrc));
return;
}
if (STI.getXLen() == 32 && RISCV::GPRRegClass.contains(DstReg) &&
RISCV::FPR32RegClass.contains(SrcReg)) {
BuildMI(MBB, MBBI, DL, get(RISCV::FMV_X_W), DstReg)
- .addReg(SrcReg, getKillRegState(KillSrc));
+ .addReg(SrcReg, getKillRegState(KillSrc));
return;
}
if (STI.getXLen() == 64 && RISCV::FPR64RegClass.contains(DstReg) &&
RISCV::GPRRegClass.contains(SrcReg)) {
BuildMI(MBB, MBBI, DL, get(RISCV::FMV_D_X), DstReg)
- .addReg(SrcReg, getKillRegState(KillSrc));
+ .addReg(SrcReg, getKillRegState(KillSrc));
return;
}
if (STI.getXLen() == 64 && RISCV::GPRRegClass.contains(DstReg) &&
RISCV::FPR64RegClass.contains(SrcReg)) {
BuildMI(MBB, MBBI, DL, get(RISCV::FMV_X_D), DstReg)
- .addReg(SrcReg, getKillRegState(KillSrc));
+ .addReg(SrcReg, getKillRegState(KillSrc));
return;
}
>From 2057f1281d6377bbc42b105b967cf8ebaa406d89 Mon Sep 17 00:00:00 2001
From: Craig Topper <craig.topper at sifive.com>
Date: Fri, 27 Oct 2023 15:17:30 -0700
Subject: [PATCH 3/4] !fixup correct comment in test
---
llvm/test/CodeGen/RISCV/GlobalISel/fpr-gpr-copy-rv64.ll | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/llvm/test/CodeGen/RISCV/GlobalISel/fpr-gpr-copy-rv64.ll b/llvm/test/CodeGen/RISCV/GlobalISel/fpr-gpr-copy-rv64.ll
index 6155e73a855a479..88dc15087206519 100644
--- a/llvm/test/CodeGen/RISCV/GlobalISel/fpr-gpr-copy-rv64.ll
+++ b/llvm/test/CodeGen/RISCV/GlobalISel/fpr-gpr-copy-rv64.ll
@@ -2,7 +2,7 @@
; RUN: llc -mtriple=riscv64 -global-isel -mattr=+d -target-abi=lp64 \
; RUN: -verify-machineinstrs < %s | FileCheck -check-prefix=RV32I %s
-; Test copying between FPR32 and GPR on RV32.
+; Test copying between FPR64 and GPR on RV64.
; FIXME: This test should be replaced with a more general calling convention
; test once we have more FP implemented.
>From dc5ef676c8a533127b610a88ae1db7cb9c112773 Mon Sep 17 00:00:00 2001
From: Craig Topper <craig.topper at sifive.com>
Date: Fri, 27 Oct 2023 17:07:02 -0700
Subject: [PATCH 4/4] [RISCV][GISel] Add a special case to selectCopy for
FPR32<->GPR on RV64.
copyPhysReg can't copy a FPR32 to a 64-bit GPR since the sizes don't
match. Replace with FMV_W_X/FMV_X_W during GISel while we still have the
size information.
---
.../RISCV/GISel/RISCVInstructionSelector.cpp | 31 +++++++++++++++++--
.../RISCV/GlobalISel/fpr-gpr-copy-rv64.ll | 16 ++++++++++
2 files changed, 45 insertions(+), 2 deletions(-)
diff --git a/llvm/lib/Target/RISCV/GISel/RISCVInstructionSelector.cpp b/llvm/lib/Target/RISCV/GISel/RISCVInstructionSelector.cpp
index a8ce01f703f33fa..a21936ced31e4ec 100644
--- a/llvm/lib/Target/RISCV/GISel/RISCVInstructionSelector.cpp
+++ b/llvm/lib/Target/RISCV/GISel/RISCVInstructionSelector.cpp
@@ -468,8 +468,10 @@ bool RISCVInstructionSelector::selectCopy(MachineInstr &MI,
if (DstReg.isPhysical())
return true;
- const TargetRegisterClass *DstRC = getRegClassForTypeOnBank(
- MRI.getType(DstReg), *RBI.getRegBank(DstReg, MRI, TRI));
+ const RegisterBank &DstRegBank = *RBI.getRegBank(DstReg, MRI, TRI);
+
+ const TargetRegisterClass *DstRC =
+ getRegClassForTypeOnBank(MRI.getType(DstReg), DstRegBank);
assert(DstRC &&
"Register class not available for LLT, register bank combination");
@@ -482,6 +484,31 @@ bool RISCVInstructionSelector::selectCopy(MachineInstr &MI,
return false;
}
+ // RV64 requires special handling for copies between GPR and FPR32.
+ // copyPhysReg considers GPR to be 64 bits.
+ // FIXME: Should we remove the XLen check from copyPhysReg?
+ if (MI.isCopy() && Subtarget->is64Bit()) {
+ Register SrcReg = MI.getOperand(1).getReg();
+ const RegisterBank &SrcRegBank = *RBI.getRegBank(SrcReg, MRI, TRI);
+
+ unsigned DstSize = RBI.getSizeInBits(DstReg, MRI, TRI);
+ unsigned SrcSize = RBI.getSizeInBits(SrcReg, MRI, TRI);
+
+ if (DstSize == 32 && SrcSize == 32 &&
+ SrcRegBank.getID() == RISCV::GPRRegBankID &&
+ DstRegBank.getID() == RISCV::FPRRegBankID) {
+ MI.setDesc(TII.get(RISCV::FMV_W_X));
+ return true;
+ }
+
+ if (DstSize == 32 && SrcSize == 32 &&
+ SrcRegBank.getID() == RISCV::FPRRegBankID &&
+ DstRegBank.getID() == RISCV::GPRRegBankID) {
+ MI.setDesc(TII.get(RISCV::FMV_X_W));
+ return true;
+ }
+ }
+
MI.setDesc(TII.get(RISCV::COPY));
return true;
}
diff --git a/llvm/test/CodeGen/RISCV/GlobalISel/fpr-gpr-copy-rv64.ll b/llvm/test/CodeGen/RISCV/GlobalISel/fpr-gpr-copy-rv64.ll
index 88dc15087206519..c0b674fcbd3d247 100644
--- a/llvm/test/CodeGen/RISCV/GlobalISel/fpr-gpr-copy-rv64.ll
+++ b/llvm/test/CodeGen/RISCV/GlobalISel/fpr-gpr-copy-rv64.ll
@@ -17,3 +17,19 @@ define double @fadd(double %x, double %y) {
%a = fadd double %x, %y
ret double %a
}
+
+; Test copying between FPR32 and GPR on RV64.
+; FIXME: This test should be replaced with a more general calling convention
+; test once we have more FP implemented.
+
+define float @fadd_f32(float %x, float %y) {
+; RV32I-LABEL: fadd:
+; RV32I: # %bb.0:
+; RV32I-NEXT: fmv.d.x fa5, a0
+; RV32I-NEXT: fmv.d.x fa4, a1
+; RV32I-NEXT: fadd.d fa5, fa5, fa4
+; RV32I-NEXT: fmv.x.d a0, fa5
+; RV32I-NEXT: ret
+ %a = fadd float %x, %y
+ ret float %a
+}
More information about the llvm-commits
mailing list