[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
Mon Nov 13 10:54:54 PST 2023
https://github.com/topperc updated https://github.com/llvm/llvm-project/pull/70896
>From 306a5d6b4a6b82fe6e14a32e7373759f21735ec2 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/4] [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 3cd6c9accd88d50..0bb8148cc04dd22 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();
@@ -159,11 +204,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 95500337c3965401570909ae206cbac85ef53ce3 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/4] 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 0f3ab112b05265e2d13f1f4d99d44079cdc533c5 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/4] 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 0bb8148cc04dd22..39dc6b71efb974f 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;
>From 741b43c9fe65024b887af46f0564fb74fb1e662b Mon Sep 17 00:00:00 2001
From: Craig Topper <craig.topper at sifive.com>
Date: Mon, 13 Nov 2023 10:43:02 -0800
Subject: [PATCH 4/4] Address review comments.
---
llvm/lib/Target/RISCV/GISel/RISCVRegisterBankInfo.cpp | 2 ++
.../RISCV/GlobalISel/regbankselect/fp-load-store.mir | 6 +++---
2 files changed, 5 insertions(+), 3 deletions(-)
diff --git a/llvm/lib/Target/RISCV/GISel/RISCVRegisterBankInfo.cpp b/llvm/lib/Target/RISCV/GISel/RISCVRegisterBankInfo.cpp
index 39dc6b71efb974f..f81ff3e27131bf5 100644
--- a/llvm/lib/Target/RISCV/GISel/RISCVRegisterBankInfo.cpp
+++ b/llvm/lib/Target/RISCV/GISel/RISCVRegisterBankInfo.cpp
@@ -216,6 +216,7 @@ RISCVRegisterBankInfo::getInstrMapping(const MachineInstr &MI) const {
LLT Ty = MRI.getType(MI.getOperand(0).getReg());
// Use FPR64 for s64 loads on rv32.
if (GPRSize == 32 && Ty.getSizeInBits() == 64) {
+ assert(MF.getSubtarget<RISCVSubtarget>().hasStdExtD());
OperandsMapping =
getOperandsMapping({getFPValueMapping(64), GPRValueMapping});
break;
@@ -242,6 +243,7 @@ RISCVRegisterBankInfo::getInstrMapping(const MachineInstr &MI) const {
LLT Ty = MRI.getType(MI.getOperand(0).getReg());
// Use FPR64 for s64 stores on rv32.
if (GPRSize == 32 && Ty.getSizeInBits() == 64) {
+ assert(MF.getSubtarget<RISCVSubtarget>().hasStdExtD());
OperandsMapping =
getOperandsMapping({getFPValueMapping(64), GPRValueMapping});
break;
diff --git a/llvm/test/CodeGen/RISCV/GlobalISel/regbankselect/fp-load-store.mir b/llvm/test/CodeGen/RISCV/GlobalISel/regbankselect/fp-load-store.mir
index f5b233ad5fd23fb..db9ee833b3b99c0 100644
--- a/llvm/test/CodeGen/RISCV/GlobalISel/regbankselect/fp-load-store.mir
+++ b/llvm/test/CodeGen/RISCV/GlobalISel/regbankselect/fp-load-store.mir
@@ -62,10 +62,10 @@ legalized: true
tracksRegLiveness: true
body: |
bb.1:
- liveins: $x10, $f10_d, $f11_d
+ liveins: $x10, $f10_d
; RV32-LABEL: name: fp_store_no_def_f64
- ; RV32: liveins: $x10, $f10_d, $f11_d
+ ; 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
@@ -73,7 +73,7 @@ body: |
; RV32-NEXT: PseudoRET
;
; RV64-LABEL: name: fp_store_no_def_f64
- ; RV64: liveins: $x10, $f10_d, $f11_d
+ ; 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
More information about the llvm-commits
mailing list