[RISCV][GISEL] regbankselect and instruction-select for G_IMPLICIT_DEF #73060

michaelmaitland · 2023-11-22T01:34:36Z

This is similar to the selection of G_IMPLICIT_DEF in AArch64. Regbankselect may need to be improved in a future patch.

llvmbot · 2023-11-22T01:34:55Z

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

@llvm/pr-subscribers-llvm-globalisel

Author: Michael Maitland (michaelmaitland)

Changes

This is similar to the selection of G_IMPLICIT_DEF in AArch64. Register bank
selection needs to be implemented in a future patch. It is not so
straight forward since the register bank information is ambiguous on its
own and depends on its uses.

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

3 Files Affected:

(modified) llvm/lib/Target/RISCV/GISel/RISCVInstructionSelector.cpp (+23)
(added) llvm/test/CodeGen/RISCV/GlobalISel/instruction-select/implicit-def32.mir (+75)
(added) llvm/test/CodeGen/RISCV/GlobalISel/instruction-select/implicit-def64.mir (+75)

diff --git a/llvm/lib/Target/RISCV/GISel/RISCVInstructionSelector.cpp b/llvm/lib/Target/RISCV/GISel/RISCVInstructionSelector.cpp
index 3c72269d1e00c2f..5999fb4840b9ea0 100644
--- a/llvm/lib/Target/RISCV/GISel/RISCVInstructionSelector.cpp
+++ b/llvm/lib/Target/RISCV/GISel/RISCVInstructionSelector.cpp
@@ -62,6 +62,8 @@ class RISCVInstructionSelector : public InstructionSelector {
 
   // Custom selection methods
   bool selectCopy(MachineInstr &MI, MachineRegisterInfo &MRI) const;
+  bool selectImplicitDef(MachineInstr &MI, MachineIRBuilder &MIB,
+                         MachineRegisterInfo &MRI) const;
   bool materializeImm(Register Reg, int64_t Imm, MachineIRBuilder &MIB) const;
   bool selectGlobalValue(MachineInstr &MI, MachineIRBuilder &MIB,
                          MachineRegisterInfo &MRI) const;
@@ -564,6 +566,8 @@ bool RISCVInstructionSelector::select(MachineInstr &MI) {
     return selectSelect(MI, MIB, MRI);
   case TargetOpcode::G_FCMP:
     return selectFPCompare(MI, MIB, MRI);
+  case TargetOpcode::G_IMPLICIT_DEF:
+    return selectImplicitDef(MI, MIB, MRI);
   default:
     return false;
   }
@@ -677,6 +681,25 @@ bool RISCVInstructionSelector::selectCopy(MachineInstr &MI,
   return true;
 }
 
+bool RISCVInstructionSelector::selectImplicitDef(
+    MachineInstr &MI, MachineIRBuilder &MIB, MachineRegisterInfo &MRI) const {
+  assert(MI.getOpcode() == TargetOpcode::G_IMPLICIT_DEF);
+
+  const Register DstReg = MI.getOperand(0).getReg();
+  const TargetRegisterClass *DstRC = getRegClassForTypeOnBank(
+      MRI.getType(DstReg), *RBI.getRegBank(DstReg, MRI, TRI));
+
+  assert(DstRC &&
+         "Register class not available for LLT, register bank combination");
+
+  if (!RBI.constrainGenericRegister(DstReg, *DstRC, MRI)) {
+    LLVM_DEBUG(dbgs() << "Failed to constrain " << TII.getName(MI.getOpcode())
+                      << " operand\n");
+  }
+  MI.setDesc(TII.get(TargetOpcode::IMPLICIT_DEF));
+  return true;
+}
+
 bool RISCVInstructionSelector::materializeImm(Register DstReg, int64_t Imm,
                                               MachineIRBuilder &MIB) const {
   MachineRegisterInfo &MRI = *MIB.getMRI();
diff --git a/llvm/test/CodeGen/RISCV/GlobalISel/instruction-select/implicit-def32.mir b/llvm/test/CodeGen/RISCV/GlobalISel/instruction-select/implicit-def32.mir
new file mode 100644
index 000000000000000..10a5246a99d2b4d
--- /dev/null
+++ b/llvm/test/CodeGen/RISCV/GlobalISel/instruction-select/implicit-def32.mir
@@ -0,0 +1,75 @@
+# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py
+# RUN: llc -mtriple=riscv32 -mattr=+f -run-pass=instruction-select -simplify-mir -verify-machineinstrs %s -o - \
+# RUN: | FileCheck -check-prefix=RV32F %s
+
+---
+name:            implicit_def_gpr
+legalized:       true
+regBankSelected: true
+registers:
+  - { id: 0, class: gpr }
+  - { id: 1, class: gpr }
+
+body:             |
+  bb.0:
+    ; RV32F-LABEL: name: implicit_def_gpr
+    ; RV32F: [[DEF:%[0-9]+]]:gpr = IMPLICIT_DEF
+    ; RV32F-NEXT: [[ADD:%[0-9]+]]:gpr = ADD [[DEF]], [[DEF]]
+    ; RV32F-NEXT: $x10 = COPY [[ADD]]
+    %0(s32) = G_IMPLICIT_DEF
+    %1(s32) = G_ADD %0, %0
+    $x10 = COPY %1(s32)
+...
+---
+name:            implicit_def_copy_gpr
+legalized:       true
+regBankSelected: true
+registers:
+  - { id: 0, class: gpr }
+  - { id: 1, class: gpr }
+
+body:             |
+  bb.0:
+    ; RV32F-LABEL: name: implicit_def_copy_gpr
+    ; RV32F: [[DEF:%[0-9]+]]:gpr = IMPLICIT_DEF
+    ; RV32F-NEXT: $x10 = COPY [[DEF]]
+    %0(s32) = G_IMPLICIT_DEF
+    %1(s32) = COPY %0(s32)
+    $x10 = COPY %1(s32)
+...
+
+---
+name:            implicit_def_fpr
+legalized:       true
+regBankSelected: true
+registers:
+  - { id: 0, class: fprb }
+  - { id: 1, class: fprb }
+
+body:             |
+  bb.0:
+    ; RV32F-LABEL: name: implicit_def_fpr
+    ; RV32F: [[DEF:%[0-9]+]]:fpr32 = IMPLICIT_DEF
+    ; RV32F-NEXT: [[FADD_S:%[0-9]+]]:fpr32 = nofpexcept FADD_S [[DEF]], [[DEF]], 7
+    ; RV32F-NEXT: $f10_f = COPY [[FADD_S]]
+    %0(s32) = G_IMPLICIT_DEF
+    %1(s32) = G_FADD %0, %0
+    $f10_f = COPY %1(s32)
+...
+---
+name:            implicit_def_copy_fpr
+legalized:       true
+regBankSelected: true
+registers:
+  - { id: 0, class: fprb }
+  - { id: 1, class: fprb }
+
+body:             |
+  bb.0:
+    ; RV32F-LABEL: name: implicit_def_copy_fpr
+    ; RV32F: [[DEF:%[0-9]+]]:fpr32 = IMPLICIT_DEF
+    ; RV32F-NEXT: $f10_f = COPY [[DEF]]
+    %0(s32) = G_IMPLICIT_DEF
+    %1(s32) = COPY %0(s32)
+    $f10_f = COPY %1(s32)
+...
diff --git a/llvm/test/CodeGen/RISCV/GlobalISel/instruction-select/implicit-def64.mir b/llvm/test/CodeGen/RISCV/GlobalISel/instruction-select/implicit-def64.mir
new file mode 100644
index 000000000000000..1aafca8374ff908
--- /dev/null
+++ b/llvm/test/CodeGen/RISCV/GlobalISel/instruction-select/implicit-def64.mir
@@ -0,0 +1,75 @@
+# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py
+# RUN: llc -mtriple=riscv64 -mattr=+d -run-pass=instruction-select -simplify-mir -verify-machineinstrs %s -o - \
+# RUN: | FileCheck -check-prefix=RV64D %s
+
+---
+name:            implicit_def_gpr
+legalized:       true
+regBankSelected: true
+registers:
+  - { id: 0, class: gpr }
+  - { id: 1, class: gpr }
+
+body:             |
+  bb.0:
+    ; RV64D-LABEL: name: implicit_def_gpr
+    ; RV64D: [[DEF:%[0-9]+]]:gpr = IMPLICIT_DEF
+    ; RV64D-NEXT: [[ADD:%[0-9]+]]:gpr = ADD [[DEF]], [[DEF]]
+    ; RV64D-NEXT: $x10 = COPY [[ADD]]
+    %0(s64) = G_IMPLICIT_DEF
+    %1(s64) = G_ADD %0, %0
+    $x10 = COPY %1(s64)
+...
+---
+name:            implicit_def_copy_gpr
+legalized:       true
+regBankSelected: true
+registers:
+  - { id: 0, class: gpr }
+  - { id: 1, class: gpr }
+
+body:             |
+  bb.0:
+    ; RV64D-LABEL: name: implicit_def_copy_gpr
+    ; RV64D: [[DEF:%[0-9]+]]:gpr = IMPLICIT_DEF
+    ; RV64D-NEXT: $x10 = COPY [[DEF]]
+    %0(s64) = G_IMPLICIT_DEF
+    %1(s64) = COPY %0(s64)
+    $x10 = COPY %1(s64)
+...
+
+---
+name:            implicit_def_fpr
+legalized:       true
+regBankSelected: true
+registers:
+  - { id: 0, class: fprb }
+  - { id: 1, class: fprb }
+
+body:             |
+  bb.0:
+    ; RV64D-LABEL: name: implicit_def_fpr
+    ; RV64D: [[DEF:%[0-9]+]]:fpr64 = IMPLICIT_DEF
+    ; RV64D-NEXT: [[FADD_D:%[0-9]+]]:fpr64 = nofpexcept FADD_D [[DEF]], [[DEF]], 7
+    ; RV64D-NEXT: $f10_d = COPY [[FADD_D]]
+    %0(s64) = G_IMPLICIT_DEF
+    %1(s64) = G_FADD %0, %0
+    $f10_d = COPY %1(s64)
+...
+---
+name:            implicit_def_copy_fpr
+legalized:       true
+regBankSelected: true
+registers:
+  - { id: 0, class: fprb }
+  - { id: 1, class: fprb }
+
+body:             |
+  bb.0:
+    ; RV64D-LABEL: name: implicit_def_copy_fpr
+    ; RV64D: [[DEF:%[0-9]+]]:fpr64 = IMPLICIT_DEF
+    ; RV64D-NEXT: $f10_d = COPY [[DEF]]
+    %0(s64) = G_IMPLICIT_DEF
+    %1(s64) = COPY %0(s64)
+    $f10_d = COPY %1(s64)
+...

llvm/test/CodeGen/RISCV/GlobalISel/instruction-select/implicit-def64.mir

topperc · 2023-11-28T01:06:01Z

llvm/test/CodeGen/RISCV/GlobalISel/instruction-select/implicit-def-rv32.mir

+legalized:       true
+regBankSelected: true
+registers:
+  - { id: 0, class: fprb }


I'm confused. fprb is a register bank, but the gpr tests don't use gprb, they use gpr

Good catch. My mistake. Updated.

I'd also suggest dropping the registers section and just using the inline specifiers. We should really trim the registers section down to just register hints (and even then we should probably introduce an inline syntax for them)

@arsenm I dropped the register section in favor of the inline specifiers in the last fixup.

arsenm · 2023-11-28T01:47:45Z

llvm/test/CodeGen/RISCV/GlobalISel/instruction-select/implicit-def-rv32.mir

+    %0:fprb(s32) = G_IMPLICIT_DEF
+    %1:fprb(s32) = COPY %0(s32)
+    $f10_f = COPY %1(s32)
+...


Can you add a test with a user with a user that has constraints, such that the selection of the user will have already pre-constrained the register?

Is that different than implicit_def_fpr? I assumed that G_FADD would constrain all operands to FPR. I don't know where to read the definition of selectImpl which handles G_FADD, but I would guess that it constrains reg operands.

Are you asking to test something else?

No, that's what I meant. I was looking at the copy chains below

topperc · 2023-11-28T18:22:47Z

llvm/test/CodeGen/RISCV/GlobalISel/instruction-select/implicit-def-rv32.mir

+    ; RV32F: [[DEF:%[0-9]+]]:fpr32 = IMPLICIT_DEF
+    ; RV32F-NEXT: [[FADD_S:%[0-9]+]]:fpr32 = nofpexcept FADD_S [[DEF]], [[DEF]], 7
+    ; RV32F-NEXT: $f10_f = COPY [[FADD_S]]
+    %0:fprb(s32) = G_IMPLICIT_DEF


Can register bank selection produce this? There's no code for G_IMPLICIT_DEF in RISCVRegistrBankInfo so it would always have gprb?

That is a good point. I should add support and test regbankselection for this case.

topperc

LGTM

This is similiar to the selection of G_IMPLICIT_DEF in AArch64. Regbankselect may need to be improved in a future patch.

michaelmaitland added backend:RISC-V llvm:globalisel labels Nov 22, 2023

michaelmaitland requested review from arsenm, tschuett and topperc November 22, 2023 01:34

michaelmaitland mentioned this pull request Nov 22, 2023

[GISEL][RISCV] Legalize llvm.vacopy intrinsic #73066

Merged

topperc reviewed Nov 25, 2023

View reviewed changes

llvm/test/CodeGen/RISCV/GlobalISel/instruction-select/implicit-def64.mir Outdated Show resolved Hide resolved

topperc reviewed Nov 28, 2023

View reviewed changes

michaelmaitland force-pushed the select-implicit-def branch from 4405d49 to 4b56f19 Compare November 28, 2023 01:23

arsenm reviewed Nov 28, 2023

View reviewed changes

topperc reviewed Nov 28, 2023

View reviewed changes

topperc approved these changes Nov 28, 2023

View reviewed changes

michaelmaitland changed the title ~~[RISCV][GISEL] Select G_IMPLICIT_DEF~~ [RISCV][GISEL] regbankselect and instruction-select for G_IMPLICIT_DEF Nov 30, 2023

michaelmaitland force-pushed the select-implicit-def branch from c69bf65 to 9fbbeb2 Compare November 30, 2023 16:30

michaelmaitland added 4 commits November 30, 2023 08:32

[RISCV][GISEL] Select G_IMPLICIT_DEF

0f62c2a

This is similiar to the selection of G_IMPLICIT_DEF in AArch64. Regbankselect may need to be improved in a future patch.

!fixup fix test naming conventions

09f7b19

!fixup use gprb instead of gpr

79eb8b6

!fixup regbankselect G_IMPLICIT_DEF

c2cafff

michaelmaitland force-pushed the select-implicit-def branch from 9fbbeb2 to c2cafff Compare November 30, 2023 16:32

michaelmaitland merged commit 6976dac into llvm:main Nov 30, 2023
2 of 3 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[RISCV][GISEL] regbankselect and instruction-select for G_IMPLICIT_DEF #73060

[RISCV][GISEL] regbankselect and instruction-select for G_IMPLICIT_DEF #73060

michaelmaitland commented Nov 22, 2023 •

edited

Loading

llvmbot commented Nov 22, 2023 •

edited

Loading

topperc Nov 28, 2023

michaelmaitland Nov 28, 2023

arsenm Nov 28, 2023

michaelmaitland Nov 28, 2023

arsenm Nov 28, 2023

michaelmaitland Nov 28, 2023

arsenm Nov 29, 2023

topperc Nov 28, 2023

michaelmaitland Nov 28, 2023

topperc left a comment

[RISCV][GISEL] regbankselect and instruction-select for G_IMPLICIT_DEF #73060

[RISCV][GISEL] regbankselect and instruction-select for G_IMPLICIT_DEF #73060

Conversation

michaelmaitland commented Nov 22, 2023 • edited Loading

llvmbot commented Nov 22, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

topperc left a comment

Choose a reason for hiding this comment

michaelmaitland commented Nov 22, 2023 •

edited

Loading

llvmbot commented Nov 22, 2023 •

edited

Loading