Skip to content
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.

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

[RISCV] Add register bank and instruction selection support for FP G_SELECT. #72726

Merged
merged 6 commits into from
Nov 27, 2023

Conversation

topperc
Copy link
Collaborator

@topperc topperc commented Nov 18, 2023

Try to pick the FP register bank based on surrounding use/defs. Code is basically copied from AArch64.

Need legalizer changes to make this more useful. Right now we're stuck with only being able to FP select types the same size as XLen.

@llvmbot
Copy link
Collaborator

llvmbot commented Nov 18, 2023

@llvm/pr-subscribers-backend-risc-v

@llvm/pr-subscribers-llvm-globalisel

Author: Craig Topper (topperc)

Changes

Try to pick the FP register bank based on surrounding use/defs. Code is basically copied from AArch64.

Need legalizer changes to make this more useful. Right now we're stuck with only being able to FP select types the same size as XLen.


Full diff: https://github.com/llvm/llvm-project/pull/72726.diff

6 Files Affected:

  • (modified) llvm/lib/Target/RISCV/GISel/RISCVInstructionSelector.cpp (+10-2)
  • (modified) llvm/lib/Target/RISCV/GISel/RISCVRegisterBankInfo.cpp (+47-4)
  • (added) llvm/test/CodeGen/RISCV/GlobalISel/instruction-select/fp-select-rv32.mir (+33)
  • (added) llvm/test/CodeGen/RISCV/GlobalISel/instruction-select/fp-select-rv64.mir (+34)
  • (added) llvm/test/CodeGen/RISCV/GlobalISel/regbankselect/fp-select-rv32.mir (+34)
  • (added) llvm/test/CodeGen/RISCV/GlobalISel/regbankselect/fp-select-rv64.mir (+34)
diff --git a/llvm/lib/Target/RISCV/GISel/RISCVInstructionSelector.cpp b/llvm/lib/Target/RISCV/GISel/RISCVInstructionSelector.cpp
index 25f4a217c070349..2586390e8562d1c 100644
--- a/llvm/lib/Target/RISCV/GISel/RISCVInstructionSelector.cpp
+++ b/llvm/lib/Target/RISCV/GISel/RISCVInstructionSelector.cpp
@@ -816,8 +816,16 @@ bool RISCVInstructionSelector::selectSelect(MachineInstr &MI,
   RISCVCC::CondCode CC;
   getOperandsForBranch(SelectMI.getCondReg(), MRI, CC, LHS, RHS);
 
-  MachineInstr *Result = MIB.buildInstr(RISCV::Select_GPR_Using_CC_GPR)
-                             .addDef(SelectMI.getReg(0))
+  Register DstReg = SelectMI.getReg(0);
+
+  unsigned Opc = RISCV::Select_GPR_Using_CC_GPR;
+  if (RBI.getRegBank(DstReg, MRI, TRI)->getID() == RISCV::FPRBRegBankID) {
+    unsigned Size = MRI.getType(DstReg).getSizeInBits();
+    Opc = Size == 32 ? RISCV::Select_FPR32_Using_CC_GPR : RISCV::Select_FPR64_Using_CC_GPR;
+  }
+
+  MachineInstr *Result = MIB.buildInstr(Opc)
+                             .addDef(DstReg)
                              .addReg(LHS)
                              .addReg(RHS)
                              .addImm(CC)
diff --git a/llvm/lib/Target/RISCV/GISel/RISCVRegisterBankInfo.cpp b/llvm/lib/Target/RISCV/GISel/RISCVRegisterBankInfo.cpp
index 6d77e2b7edd9010..5721bd9609040e6 100644
--- a/llvm/lib/Target/RISCV/GISel/RISCVRegisterBankInfo.cpp
+++ b/llvm/lib/Target/RISCV/GISel/RISCVRegisterBankInfo.cpp
@@ -323,12 +323,55 @@ RISCVRegisterBankInfo::getInstrMapping(const MachineInstr &MI) const {
       OpdsMapping[0] = getFPValueMapping(Ty.getSizeInBits());
     break;
   }
-  case TargetOpcode::G_SELECT:
-    OpdsMapping[0] = GPRValueMapping;
+  case TargetOpcode::G_SELECT: {
+    LLT Ty = MRI.getType(MI.getOperand(0).getReg());
+
+    // Try to minimize the number of copies. If we have more floating point
+    // constrained values than not, then we'll put everything on FPR. Otherwise,
+    // everything has to be on GPR.
+    unsigned NumFP = 0;
+
+    // Check if the uses of the result always produce floating point values.
+    //
+    // For example:
+    //
+    // %z = G_SELECT %cond %x %y
+    // fpr = G_FOO %z ...
+    if (any_of(MRI.use_nodbg_instructions(MI.getOperand(0).getReg()),
+               [&](const MachineInstr &UseMI) { return onlyUsesFP(UseMI, MRI, TRI); }))
+      ++NumFP;
+
+    // Check if the defs of the source values always produce floating point
+    // values.
+    //
+    // For example:
+    //
+    // %x = G_SOMETHING_ALWAYS_FLOAT %a ...
+    // %z = G_SELECT %cond %x %y
+    //
+    // Also check whether or not the sources have already been decided to be
+    // FPR. Keep track of this.
+    //
+    // This doesn't check the condition, since it's just whatever is in NZCV.
+    // This isn't passed explicitly in a register to fcsel/csel.
+    for (unsigned Idx = 2; Idx < 4; ++Idx) {
+      Register VReg = MI.getOperand(Idx).getReg();
+      MachineInstr *DefMI = MRI.getVRegDef(VReg);
+      if (getRegBank(VReg, MRI, TRI) == &RISCV::FPRBRegBank ||
+          onlyDefinesFP(*DefMI, MRI, TRI))
+        ++NumFP;
+    }
+
+    // Condition operand is always GPR.
     OpdsMapping[1] = GPRValueMapping;
-    OpdsMapping[2] = GPRValueMapping;
-    OpdsMapping[3] = GPRValueMapping;
+
+    const ValueMapping *Mapping = GPRValueMapping;
+    if (NumFP >= 2)
+      Mapping = getFPValueMapping(Ty.getSizeInBits());
+
+    OpdsMapping[0] = OpdsMapping[2] = OpdsMapping[3] = Mapping;
     break;
+  }
   case TargetOpcode::G_FPTOSI:
   case TargetOpcode::G_FPTOUI: {
     LLT Ty = MRI.getType(MI.getOperand(1).getReg());
diff --git a/llvm/test/CodeGen/RISCV/GlobalISel/instruction-select/fp-select-rv32.mir b/llvm/test/CodeGen/RISCV/GlobalISel/instruction-select/fp-select-rv32.mir
new file mode 100644
index 000000000000000..4fffbc42b2cff34
--- /dev/null
+++ b/llvm/test/CodeGen/RISCV/GlobalISel/instruction-select/fp-select-rv32.mir
@@ -0,0 +1,33 @@
+# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py
+# RUN: llc -mtriple=riscv32 -mattr=+d -run-pass=instruction-select \
+# RUN:   -simplify-mir -verify-machineinstrs %s -o - | FileCheck %s
+
+---
+name:            fp_select_s32
+legalized:       true
+regBankSelected: true
+tracksRegLiveness: true
+body:             |
+  bb.0.entry:
+    liveins: $x10, $f10_f, $f11_f
+
+    ; CHECK-LABEL: name: fp_select_s32
+    ; CHECK: liveins: $x10, $f10_f, $f11_f
+    ; CHECK-NEXT: {{  $}}
+    ; CHECK-NEXT: [[COPY:%[0-9]+]]:gpr = COPY $x10
+    ; CHECK-NEXT: [[COPY1:%[0-9]+]]:fpr32 = COPY $f10_f
+    ; CHECK-NEXT: [[COPY2:%[0-9]+]]:fpr32 = COPY $f11_f
+    ; CHECK-NEXT: [[ANDI:%[0-9]+]]:gpr = ANDI [[COPY]], 1
+    ; CHECK-NEXT: [[Select_FPR32_Using_CC_GPR:%[0-9]+]]:fpr32 = Select_FPR32_Using_CC_GPR [[ANDI]], $x0, 1, [[COPY1]], [[COPY2]]
+    ; CHECK-NEXT: $f10_f = COPY [[Select_FPR32_Using_CC_GPR]]
+    ; CHECK-NEXT: PseudoRET implicit $f10_f
+    %0:gprb(s32) = COPY $x10
+    %1:fprb(s32) = COPY $f10_f
+    %2:fprb(s32) = COPY $f11_f
+    %3:gprb(s32) = G_CONSTANT i32 1
+    %4:gprb(s32) = G_AND %0, %3
+    %5:fprb(s32) = G_SELECT %4(s32), %1, %2
+    $f10_f = COPY %5(s32)
+    PseudoRET implicit $f10_f
+
+...
diff --git a/llvm/test/CodeGen/RISCV/GlobalISel/instruction-select/fp-select-rv64.mir b/llvm/test/CodeGen/RISCV/GlobalISel/instruction-select/fp-select-rv64.mir
new file mode 100644
index 000000000000000..8efb8b10d8a931d
--- /dev/null
+++ b/llvm/test/CodeGen/RISCV/GlobalISel/instruction-select/fp-select-rv64.mir
@@ -0,0 +1,34 @@
+# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py
+# RUN: llc -mtriple=riscv64 -mattr=+d -run-pass=instruction-select \
+# RUN:   -simplify-mir -verify-machineinstrs %s -o - | FileCheck %s
+
+---
+name:            fp_select_s64
+alignment:       1
+legalized:       true
+regBankSelected: true
+tracksRegLiveness: true
+body:             |
+  bb.0.entry:
+    liveins: $x10, $f10_d, $f11_d
+
+    ; CHECK-LABEL: name: fp_select_s64
+    ; CHECK: liveins: $x10, $f10_d, $f11_d
+    ; CHECK-NEXT: {{  $}}
+    ; CHECK-NEXT: [[COPY:%[0-9]+]]:gpr = COPY $x10
+    ; CHECK-NEXT: [[COPY1:%[0-9]+]]:fpr64 = COPY $f10_d
+    ; CHECK-NEXT: [[COPY2:%[0-9]+]]:fpr64 = COPY $f11_d
+    ; CHECK-NEXT: [[ANDI:%[0-9]+]]:gpr = ANDI [[COPY]], 1
+    ; CHECK-NEXT: [[Select_FPR64_Using_CC_GPR:%[0-9]+]]:fpr64 = Select_FPR64_Using_CC_GPR [[ANDI]], $x0, 1, [[COPY1]], [[COPY2]]
+    ; CHECK-NEXT: $f10_d = COPY [[Select_FPR64_Using_CC_GPR]]
+    ; CHECK-NEXT: PseudoRET implicit $f10_d
+    %0:gprb(s64) = COPY $x10
+    %1:fprb(s64) = COPY $f10_d
+    %2:fprb(s64) = COPY $f11_d
+    %3:gprb(s64) = G_CONSTANT i64 1
+    %4:gprb(s64) = G_AND %0, %3
+    %5:fprb(s64) = G_SELECT %4(s64), %1, %2
+    $f10_d = COPY %5(s64)
+    PseudoRET implicit $f10_d
+
+...
diff --git a/llvm/test/CodeGen/RISCV/GlobalISel/regbankselect/fp-select-rv32.mir b/llvm/test/CodeGen/RISCV/GlobalISel/regbankselect/fp-select-rv32.mir
new file mode 100644
index 000000000000000..312fadca866206c
--- /dev/null
+++ b/llvm/test/CodeGen/RISCV/GlobalISel/regbankselect/fp-select-rv32.mir
@@ -0,0 +1,34 @@
+# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py
+# RUN: llc -mtriple=riscv32 -mattr=+d -run-pass=regbankselect \
+# RUN:   -disable-gisel-legality-check -simplify-mir -verify-machineinstrs %s \
+# RUN:   -o - | FileCheck -check-prefix=RV32I %s
+
+---
+name:            fp_select_s32
+legalized:       true
+tracksRegLiveness: true
+body:             |
+  bb.0.entry:
+    liveins: $x10, $f10_f, $f11_f
+
+    ; RV32I-LABEL: name: fp_select_s32
+    ; RV32I: liveins: $x10, $f10_f, $f11_f
+    ; RV32I-NEXT: {{  $}}
+    ; RV32I-NEXT: [[COPY:%[0-9]+]]:gprb(s32) = COPY $x10
+    ; RV32I-NEXT: [[COPY1:%[0-9]+]]:fprb(s32) = COPY $f10_f
+    ; RV32I-NEXT: [[COPY2:%[0-9]+]]:fprb(s32) = COPY $f11_f
+    ; RV32I-NEXT: [[C:%[0-9]+]]:gprb(s32) = G_CONSTANT i32 1
+    ; RV32I-NEXT: [[AND:%[0-9]+]]:gprb(s32) = G_AND [[COPY]], [[C]]
+    ; RV32I-NEXT: [[SELECT:%[0-9]+]]:fprb(s32) = G_SELECT [[AND]](s32), [[COPY1]], [[COPY2]]
+    ; RV32I-NEXT: $f10_f = COPY [[SELECT]](s32)
+    ; RV32I-NEXT: PseudoRET implicit $f10_f
+    %3:_(s32) = COPY $x10
+    %4:_(s32) = COPY $f10_f
+    %5:_(s32) = COPY $f11_f
+    %12:_(s32) = G_CONSTANT i32 1
+    %11:_(s32) = G_AND %3, %12
+    %10:_(s32) = G_SELECT %11(s32), %4, %5
+    $f10_f = COPY %10(s32)
+    PseudoRET implicit $f10_f
+
+...
diff --git a/llvm/test/CodeGen/RISCV/GlobalISel/regbankselect/fp-select-rv64.mir b/llvm/test/CodeGen/RISCV/GlobalISel/regbankselect/fp-select-rv64.mir
new file mode 100644
index 000000000000000..22ebd5e93b01407
--- /dev/null
+++ b/llvm/test/CodeGen/RISCV/GlobalISel/regbankselect/fp-select-rv64.mir
@@ -0,0 +1,34 @@
+# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py
+# RUN: llc -mtriple=riscv64 -mattr=+d -run-pass=regbankselect \
+# RUN:   -disable-gisel-legality-check -simplify-mir -verify-machineinstrs %s \
+# RUN:   -o - | FileCheck -check-prefix=RV32I %s
+
+---
+name:            fp_select_s64
+legalized:       true
+tracksRegLiveness: true
+body:             |
+  bb.0.entry:
+    liveins: $x10, $f10_d, $f11_d
+
+    ; RV32I-LABEL: name: fp_select_s64
+    ; RV32I: liveins: $x10, $f10_d, $f11_d
+    ; RV32I-NEXT: {{  $}}
+    ; RV32I-NEXT: [[COPY:%[0-9]+]]:gprb(s64) = COPY $x10
+    ; RV32I-NEXT: [[COPY1:%[0-9]+]]:fprb(s64) = COPY $f10_d
+    ; RV32I-NEXT: [[COPY2:%[0-9]+]]:fprb(s64) = COPY $f11_d
+    ; RV32I-NEXT: [[C:%[0-9]+]]:gprb(s64) = G_CONSTANT i64 1
+    ; RV32I-NEXT: [[AND:%[0-9]+]]:gprb(s64) = G_AND [[COPY]], [[C]]
+    ; RV32I-NEXT: [[SELECT:%[0-9]+]]:fprb(s64) = G_SELECT [[AND]](s64), [[COPY1]], [[COPY2]]
+    ; RV32I-NEXT: $f10_d = COPY [[SELECT]](s64)
+    ; RV32I-NEXT: PseudoRET implicit $f10_d
+    %3:_(s64) = COPY $x10
+    %4:_(s64) = COPY $f10_d
+    %5:_(s64) = COPY $f11_d
+    %12:_(s64) = G_CONSTANT i64 1
+    %11:_(s64) = G_AND %3, %12
+    %10:_(s64) = G_SELECT %11(s64), %4, %5
+    $f10_d = COPY %10(s64)
+    PseudoRET implicit $f10_d
+
+...

Copy link

github-actions bot commented Nov 18, 2023

✅ With the latest revision this PR passed the C/C++ code formatter.

llvm/lib/Target/RISCV/GISel/RISCVRegisterBankInfo.cpp Outdated Show resolved Hide resolved
%10:_(s32) = G_SELECT %11(s32), %4, %5
$f10_f = COPY %10(s32)
PseudoRET implicit $f10_f

Copy link
Contributor

Choose a reason for hiding this comment

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

Should we test some cases where one of the operands are from onlyDefinesFP but the other operand can be either?

Should we test some cases where the def is used by instructions that onlyDefineFP?

…ECT.

Try to pick the FP register bank based on surrounding use/defs. Code
is basically copied from AArch64.

Need legalizer changes to make this more useful. Right now we're stuck
with only being able to FP typs the same size as XLen.
Copy link
Contributor

@michaelmaitland michaelmaitland left a comment

Choose a reason for hiding this comment

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

LGTM

@topperc topperc merged commit 5f31dbd into llvm:main Nov 27, 2023
2 of 3 checks passed
@topperc topperc deleted the pr/gisel-select branch November 27, 2023 18:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants