[llvm] [RISCV][GISel] Add really basic support for FP regbank selection for G_LOAD/G_STORE. (PR #70896)
Craig Topper via llvm-commits
llvm-commits at lists.llvm.org
Fri Nov 10 15:21:29 PST 2023
https://github.com/topperc updated https://github.com/llvm/llvm-project/pull/70896
>From 2a75650b57cf9d36ebebd2a067086e4fd5b2bbf2 Mon Sep 17 00:00:00 2001
From: Craig Topper <craig.topper at sifive.com>
Date: Tue, 31 Oct 2023 22:20:43 -0700
Subject: [PATCH 1/3] [RISCV][GISel] Add really basic support for FP regbank
selection for G_LOAD/G_STORE.
Coerce the register bank based on the users of the G_LOAD or the
defining instruction for the G_STORE.
No tests for RV32 because we need to make s64 load/store legal for
rv32 with D extension first.
---
.../AArch64/GISel/AArch64RegisterBankInfo.cpp | 1 +
.../RISCV/GISel/RISCVRegisterBankInfo.cpp | 89 ++++++++-
.../regbankselect/fp-load-store.mir | 170 ++++++++++++++++++
3 files changed, 258 insertions(+), 2 deletions(-)
create mode 100644 llvm/test/CodeGen/RISCV/GlobalISel/regbankselect/fp-load-store.mir
diff --git a/llvm/lib/Target/AArch64/GISel/AArch64RegisterBankInfo.cpp b/llvm/lib/Target/AArch64/GISel/AArch64RegisterBankInfo.cpp
index 21f9f6437e4fe91..e1ea4e4920f9e24 100644
--- a/llvm/lib/Target/AArch64/GISel/AArch64RegisterBankInfo.cpp
+++ b/llvm/lib/Target/AArch64/GISel/AArch64RegisterBankInfo.cpp
@@ -869,6 +869,7 @@ AArch64RegisterBankInfo::getInstrMapping(const MachineInstr &MI) const {
// Check if that store is fed by fp instructions.
if (OpRegBankIdx[0] == PMI_FirstGPR) {
Register VReg = MI.getOperand(0).getReg();
+ assert(VReg);
if (!VReg)
break;
MachineInstr *DefMI = MRI.getVRegDef(VReg);
diff --git a/llvm/lib/Target/RISCV/GISel/RISCVRegisterBankInfo.cpp b/llvm/lib/Target/RISCV/GISel/RISCVRegisterBankInfo.cpp
index 3cdee17be8d1d88..b6287fe8f497b2f 100644
--- a/llvm/lib/Target/RISCV/GISel/RISCVRegisterBankInfo.cpp
+++ b/llvm/lib/Target/RISCV/GISel/RISCVRegisterBankInfo.cpp
@@ -109,6 +109,51 @@ static const RegisterBankInfo::ValueMapping *getFPValueMapping(unsigned Size) {
return &RISCV::ValueMappings[Idx];
}
+// TODO: Make this more like AArch64?
+static bool onlyUsesFP(const MachineInstr &MI) {
+ switch (MI.getOpcode()) {
+ case TargetOpcode::G_FADD:
+ case TargetOpcode::G_FSUB:
+ case TargetOpcode::G_FMUL:
+ case TargetOpcode::G_FDIV:
+ case TargetOpcode::G_FNEG:
+ case TargetOpcode::G_FABS:
+ case TargetOpcode::G_FSQRT:
+ case TargetOpcode::G_FMAXNUM:
+ case TargetOpcode::G_FMINNUM:
+ case TargetOpcode::G_FPEXT:
+ case TargetOpcode::G_FPTRUNC:
+ case TargetOpcode::G_FCMP:
+ return true;
+ default:
+ break;
+ }
+
+ return false;
+}
+
+// TODO: Make this more like AArch64?
+static bool onlyDefinesFP(const MachineInstr &MI) {
+ switch (MI.getOpcode()) {
+ case TargetOpcode::G_FADD:
+ case TargetOpcode::G_FSUB:
+ case TargetOpcode::G_FMUL:
+ case TargetOpcode::G_FDIV:
+ case TargetOpcode::G_FNEG:
+ case TargetOpcode::G_FABS:
+ case TargetOpcode::G_FSQRT:
+ case TargetOpcode::G_FMAXNUM:
+ case TargetOpcode::G_FMINNUM:
+ case TargetOpcode::G_FPEXT:
+ case TargetOpcode::G_FPTRUNC:
+ return true;
+ default:
+ break;
+ }
+
+ return false;
+}
+
const RegisterBankInfo::InstructionMapping &
RISCVRegisterBankInfo::getInstrMapping(const MachineInstr &MI) const {
const unsigned Opc = MI.getOpcode();
@@ -158,11 +203,51 @@ RISCVRegisterBankInfo::getInstrMapping(const MachineInstr &MI) const {
case TargetOpcode::G_ANYEXT:
case TargetOpcode::G_SEXT:
case TargetOpcode::G_ZEXT:
- case TargetOpcode::G_LOAD:
case TargetOpcode::G_SEXTLOAD:
case TargetOpcode::G_ZEXTLOAD:
- case TargetOpcode::G_STORE:
break;
+ case TargetOpcode::G_LOAD: {
+ LLT Ty = MRI.getType(MI.getOperand(0).getReg());
+ // Use FPR64 for s64 loads on rv32.
+ if (GPRSize == 32 && Ty.getSizeInBits() == 64) {
+ OperandsMapping =
+ getOperandsMapping({getFPValueMapping(64), GPRValueMapping});
+ break;
+ }
+
+ // Check if that load feeds fp instructions.
+ // In that case, we want the default mapping to be on FPR
+ // instead of blind map every scalar to GPR.
+ if (any_of(MRI.use_nodbg_instructions(MI.getOperand(0).getReg()),
+ [&](const MachineInstr &UseMI) {
+ // If we have at least one direct use in a FP instruction,
+ // assume this was a floating point load in the IR. If it was
+ // not, we would have had a bitcast before reaching that
+ // instruction.
+ return onlyUsesFP(UseMI);
+ })) {
+ OperandsMapping = getOperandsMapping(
+ {getFPValueMapping(Ty.getSizeInBits()), GPRValueMapping});
+ }
+
+ break;
+ }
+ case TargetOpcode::G_STORE: {
+ LLT Ty = MRI.getType(MI.getOperand(0).getReg());
+ // Use FPR64 for s64 stores on rv32.
+ if (GPRSize == 32 && Ty.getSizeInBits() == 64) {
+ OperandsMapping =
+ getOperandsMapping({getFPValueMapping(64), GPRValueMapping});
+ break;
+ }
+
+ MachineInstr *DefMI = MRI.getVRegDef(MI.getOperand(0).getReg());
+ if (onlyDefinesFP(*DefMI)) {
+ OperandsMapping = getOperandsMapping(
+ {getFPValueMapping(Ty.getSizeInBits()), GPRValueMapping});
+ }
+ break;
+ }
case TargetOpcode::G_CONSTANT:
case TargetOpcode::G_FRAME_INDEX:
case TargetOpcode::G_GLOBAL_VALUE:
diff --git a/llvm/test/CodeGen/RISCV/GlobalISel/regbankselect/fp-load-store.mir b/llvm/test/CodeGen/RISCV/GlobalISel/regbankselect/fp-load-store.mir
new file mode 100644
index 000000000000000..f5b233ad5fd23fb
--- /dev/null
+++ b/llvm/test/CodeGen/RISCV/GlobalISel/regbankselect/fp-load-store.mir
@@ -0,0 +1,170 @@
+# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py
+# RUN: llc -mtriple=riscv32 -mattr=+d -run-pass=regbankselect \
+# RUN: -simplify-mir -verify-machineinstrs %s \
+# RUN: -o - | FileCheck %s --check-prefixes=CHECK,RV32
+# RUN: llc -mtriple=riscv64 -mattr=+d -run-pass=regbankselect \
+# RUN: -simplify-mir -verify-machineinstrs %s \
+# RUN: -o - | FileCheck %s --check-prefixes=CHECK,RV64
+
+---
+name: fp_store_fp_def_f32
+legalized: true
+tracksRegLiveness: true
+body: |
+ bb.1:
+ liveins: $x10, $f10_f, $f11_f
+
+ ; CHECK-LABEL: name: fp_store_fp_def_f32
+ ; CHECK: liveins: $x10, $f10_f, $f11_f
+ ; CHECK-NEXT: {{ $}}
+ ; CHECK-NEXT: [[COPY:%[0-9]+]]:gprb(p0) = COPY $x10
+ ; CHECK-NEXT: [[COPY1:%[0-9]+]]:fprb(s32) = COPY $f10_f
+ ; CHECK-NEXT: [[COPY2:%[0-9]+]]:fprb(s32) = COPY $f11_f
+ ; CHECK-NEXT: [[FADD:%[0-9]+]]:fprb(s32) = G_FADD [[COPY1]], [[COPY2]]
+ ; CHECK-NEXT: G_STORE [[FADD]](s32), [[COPY]](p0) :: (store (s32))
+ ; CHECK-NEXT: PseudoRET
+ %0:_(p0) = COPY $x10
+ %1:_(s32) = COPY $f10_f
+ %2:_(s32) = COPY $f11_f
+ %3:_(s32) = G_FADD %1, %2
+ G_STORE %3(s32), %0(p0) :: (store (s32))
+ PseudoRET
+
+...
+---
+name: fp_store_fp_def_f64
+legalized: true
+tracksRegLiveness: true
+body: |
+ bb.1:
+ liveins: $x10, $f10_d, $f11_d
+
+ ; CHECK-LABEL: name: fp_store_fp_def_f64
+ ; CHECK: liveins: $x10, $f10_d, $f11_d
+ ; CHECK-NEXT: {{ $}}
+ ; CHECK-NEXT: [[COPY:%[0-9]+]]:gprb(p0) = COPY $x10
+ ; CHECK-NEXT: [[COPY1:%[0-9]+]]:fprb(s64) = COPY $f10_d
+ ; CHECK-NEXT: [[COPY2:%[0-9]+]]:fprb(s64) = COPY $f11_d
+ ; CHECK-NEXT: [[FADD:%[0-9]+]]:fprb(s64) = G_FADD [[COPY1]], [[COPY2]]
+ ; CHECK-NEXT: G_STORE [[FADD]](s64), [[COPY]](p0) :: (store (s64))
+ ; CHECK-NEXT: PseudoRET
+ %0:_(p0) = COPY $x10
+ %1:_(s64) = COPY $f10_d
+ %2:_(s64) = COPY $f11_d
+ %3:_(s64) = G_FADD %1, %2
+ G_STORE %3(s64), %0(p0) :: (store (s64))
+ PseudoRET
+
+...
+---
+name: fp_store_no_def_f64
+legalized: true
+tracksRegLiveness: true
+body: |
+ bb.1:
+ liveins: $x10, $f10_d, $f11_d
+
+ ; RV32-LABEL: name: fp_store_no_def_f64
+ ; RV32: liveins: $x10, $f10_d, $f11_d
+ ; RV32-NEXT: {{ $}}
+ ; RV32-NEXT: [[COPY:%[0-9]+]]:gprb(p0) = COPY $x10
+ ; RV32-NEXT: [[COPY1:%[0-9]+]]:fprb(s64) = COPY $f10_d
+ ; RV32-NEXT: G_STORE [[COPY1]](s64), [[COPY]](p0) :: (store (s64))
+ ; RV32-NEXT: PseudoRET
+ ;
+ ; RV64-LABEL: name: fp_store_no_def_f64
+ ; RV64: liveins: $x10, $f10_d, $f11_d
+ ; RV64-NEXT: {{ $}}
+ ; RV64-NEXT: [[COPY:%[0-9]+]]:gprb(p0) = COPY $x10
+ ; RV64-NEXT: [[COPY1:%[0-9]+]]:fprb(s64) = COPY $f10_d
+ ; RV64-NEXT: [[COPY2:%[0-9]+]]:gprb(s64) = COPY [[COPY1]](s64)
+ ; RV64-NEXT: G_STORE [[COPY2]](s64), [[COPY]](p0) :: (store (s64))
+ ; RV64-NEXT: PseudoRET
+ %0:_(p0) = COPY $x10
+ %1:_(s64) = COPY $f10_d
+ G_STORE %1(s64), %0(p0) :: (store (s64))
+ PseudoRET
+
+...
+---
+name: fp_load_fp_use_f32
+legalized: true
+tracksRegLiveness: true
+body: |
+ bb.1:
+ liveins: $x10, $f10_f
+
+ ; CHECK-LABEL: name: fp_load_fp_use_f32
+ ; CHECK: liveins: $x10, $f10_f
+ ; CHECK-NEXT: {{ $}}
+ ; CHECK-NEXT: [[COPY:%[0-9]+]]:gprb(p0) = COPY $x10
+ ; CHECK-NEXT: [[COPY1:%[0-9]+]]:fprb(s32) = COPY $f10_f
+ ; CHECK-NEXT: [[LOAD:%[0-9]+]]:fprb(s32) = G_LOAD [[COPY]](p0) :: (load (s32))
+ ; CHECK-NEXT: [[FADD:%[0-9]+]]:fprb(s32) = G_FADD [[LOAD]], [[COPY1]]
+ ; CHECK-NEXT: $f10_f = COPY [[FADD]](s32)
+ ; CHECK-NEXT: PseudoRET implicit $f10_f
+ %0:_(p0) = COPY $x10
+ %1:_(s32) = COPY $f10_f
+ %2:_(s32) = G_LOAD %0(p0) :: (load (s32))
+ %3:_(s32) = G_FADD %2, %1
+ $f10_f = COPY %3(s32)
+ PseudoRET implicit $f10_f
+
+...
+---
+name: fp_load_fp_use_f64
+legalized: true
+tracksRegLiveness: true
+body: |
+ bb.1:
+ liveins: $x10, $f10_d
+
+ ; CHECK-LABEL: name: fp_load_fp_use_f64
+ ; CHECK: liveins: $x10, $f10_d
+ ; CHECK-NEXT: {{ $}}
+ ; CHECK-NEXT: [[COPY:%[0-9]+]]:gprb(p0) = COPY $x10
+ ; CHECK-NEXT: [[COPY1:%[0-9]+]]:fprb(s64) = COPY $f10_d
+ ; CHECK-NEXT: [[LOAD:%[0-9]+]]:fprb(s64) = G_LOAD [[COPY]](p0) :: (load (s64))
+ ; CHECK-NEXT: [[FADD:%[0-9]+]]:fprb(s64) = G_FADD [[LOAD]], [[COPY1]]
+ ; CHECK-NEXT: $f10_d = COPY [[FADD]](s64)
+ ; CHECK-NEXT: PseudoRET implicit $f10_d
+ %0:_(p0) = COPY $x10
+ %1:_(s64) = COPY $f10_d
+ %2:_(s64) = G_LOAD %0(p0) :: (load (s64))
+ %3:_(s64) = G_FADD %2, %1
+ $f10_d = COPY %3(s64)
+ PseudoRET implicit $f10_d
+
+...
+---
+name: fp_load_no_use_f64
+legalized: true
+tracksRegLiveness: true
+body: |
+ bb.1:
+ liveins: $x10, $f10_d
+
+ ; RV32-LABEL: name: fp_load_no_use_f64
+ ; RV32: liveins: $x10, $f10_d
+ ; RV32-NEXT: {{ $}}
+ ; RV32-NEXT: [[COPY:%[0-9]+]]:gprb(p0) = COPY $x10
+ ; RV32-NEXT: [[COPY1:%[0-9]+]]:fprb(s64) = COPY $f10_d
+ ; RV32-NEXT: [[LOAD:%[0-9]+]]:fprb(s64) = G_LOAD [[COPY]](p0) :: (load (s64))
+ ; RV32-NEXT: $f10_d = COPY [[LOAD]](s64)
+ ; RV32-NEXT: PseudoRET implicit $f10_d
+ ;
+ ; RV64-LABEL: name: fp_load_no_use_f64
+ ; RV64: liveins: $x10, $f10_d
+ ; RV64-NEXT: {{ $}}
+ ; RV64-NEXT: [[COPY:%[0-9]+]]:gprb(p0) = COPY $x10
+ ; RV64-NEXT: [[COPY1:%[0-9]+]]:fprb(s64) = COPY $f10_d
+ ; RV64-NEXT: [[LOAD:%[0-9]+]]:gprb(s64) = G_LOAD [[COPY]](p0) :: (load (s64))
+ ; RV64-NEXT: $f10_d = COPY [[LOAD]](s64)
+ ; RV64-NEXT: PseudoRET implicit $f10_d
+ %0:_(p0) = COPY $x10
+ %1:_(s64) = COPY $f10_d
+ %2:_(s64) = G_LOAD %0(p0) :: (load (s64))
+ $f10_d = COPY %2(s64)
+ PseudoRET implicit $f10_d
+
+...
>From 798cae729ed549f493494d07fddb3a9951c2b14f Mon Sep 17 00:00:00 2001
From: Craig Topper <craig.topper at sifive.com>
Date: Mon, 6 Nov 2023 23:36:44 -0800
Subject: [PATCH 2/3] remove accidental change.
---
llvm/lib/Target/AArch64/GISel/AArch64RegisterBankInfo.cpp | 1 -
1 file changed, 1 deletion(-)
diff --git a/llvm/lib/Target/AArch64/GISel/AArch64RegisterBankInfo.cpp b/llvm/lib/Target/AArch64/GISel/AArch64RegisterBankInfo.cpp
index e1ea4e4920f9e24..21f9f6437e4fe91 100644
--- a/llvm/lib/Target/AArch64/GISel/AArch64RegisterBankInfo.cpp
+++ b/llvm/lib/Target/AArch64/GISel/AArch64RegisterBankInfo.cpp
@@ -869,7 +869,6 @@ AArch64RegisterBankInfo::getInstrMapping(const MachineInstr &MI) const {
// Check if that store is fed by fp instructions.
if (OpRegBankIdx[0] == PMI_FirstGPR) {
Register VReg = MI.getOperand(0).getReg();
- assert(VReg);
if (!VReg)
break;
MachineInstr *DefMI = MRI.getVRegDef(VReg);
>From bfe2ec6209360eccfc45909a916cfa0710b1c562 Mon Sep 17 00:00:00 2001
From: Craig Topper <craig.topper at sifive.com>
Date: Fri, 10 Nov 2023 15:14:01 -0800
Subject: [PATCH 3/3] Add more operations to onlyUsesFP and onlyDefinesFP.
---
llvm/lib/Target/RISCV/GISel/RISCVRegisterBankInfo.cpp | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/llvm/lib/Target/RISCV/GISel/RISCVRegisterBankInfo.cpp b/llvm/lib/Target/RISCV/GISel/RISCVRegisterBankInfo.cpp
index b6287fe8f497b2f..e2c7fa0c981d1cb 100644
--- a/llvm/lib/Target/RISCV/GISel/RISCVRegisterBankInfo.cpp
+++ b/llvm/lib/Target/RISCV/GISel/RISCVRegisterBankInfo.cpp
@@ -124,6 +124,8 @@ static bool onlyUsesFP(const MachineInstr &MI) {
case TargetOpcode::G_FPEXT:
case TargetOpcode::G_FPTRUNC:
case TargetOpcode::G_FCMP:
+ case TargetOpcode::G_FPTOSI:
+ case TargetOpcode::G_FPTOUI:
return true;
default:
break;
@@ -146,6 +148,9 @@ static bool onlyDefinesFP(const MachineInstr &MI) {
case TargetOpcode::G_FMINNUM:
case TargetOpcode::G_FPEXT:
case TargetOpcode::G_FPTRUNC:
+ case TargetOpcode::G_SITOFP:
+ case TargetOpcode::G_UITOFP:
+ case TargetOpcode::G_FCONSTANT:
return true;
default:
break;
More information about the llvm-commits
mailing list