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] Use lookup tables to find CVTFOpc #88742

Closed
wants to merge 4 commits into from

Conversation

michaelmaitland
Copy link
Contributor

@michaelmaitland michaelmaitland commented Apr 15, 2024

This patch contains 3 commits which will be committed individually:

  • [RISCV][NFC] Move RISCVMaskedPseudoTable to RISCVInstrInfo
  • [RISCV][NFC] Include RISCVVInversePseudosTable in RISCVInstrInfo
  • [RISCV] Use lookup tables to find CVTFOpc

This addresses #88568 (comment).

@llvmbot
Copy link
Collaborator

llvmbot commented Apr 15, 2024

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

Author: Michael Maitland (michaelmaitland)

Changes

This patch contains 3 commits which will be committed individually:

  • [RISCV][NFC] Move RISCVMaskedPseudoTable to RISCVInstrInfo
  • [RISCV][NFC] Include RISCVVInversePseudosTable in RISCVInstrInfo
  • [RISCV] Use lookup tables to find CVTFOpc

This addresses #88568 (comment).


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

6 Files Affected:

  • (modified) llvm/lib/Target/RISCV/RISCVISelDAGToDAG.cpp (-1)
  • (modified) llvm/lib/Target/RISCV/RISCVISelDAGToDAG.h (-9)
  • (modified) llvm/lib/Target/RISCV/RISCVISelLowering.cpp (+7-74)
  • (modified) llvm/lib/Target/RISCV/RISCVInstrInfo.cpp (+16)
  • (modified) llvm/lib/Target/RISCV/RISCVInstrInfo.h (+26)
  • (modified) llvm/lib/Target/RISCV/RISCVInstrInfoVPseudos.td (+3-3)
diff --git a/llvm/lib/Target/RISCV/RISCVISelDAGToDAG.cpp b/llvm/lib/Target/RISCV/RISCVISelDAGToDAG.cpp
index f99dc0b8576368..b0568297a470a7 100644
--- a/llvm/lib/Target/RISCV/RISCVISelDAGToDAG.cpp
+++ b/llvm/lib/Target/RISCV/RISCVISelDAGToDAG.cpp
@@ -43,7 +43,6 @@ namespace llvm::RISCV {
 #define GET_RISCVVSETable_IMPL
 #define GET_RISCVVLXTable_IMPL
 #define GET_RISCVVSXTable_IMPL
-#define GET_RISCVMaskedPseudosTable_IMPL
 #include "RISCVGenSearchableTables.inc"
 } // namespace llvm::RISCV
 
diff --git a/llvm/lib/Target/RISCV/RISCVISelDAGToDAG.h b/llvm/lib/Target/RISCV/RISCVISelDAGToDAG.h
index 92f818b0dc4891..7d4aec2dfdc984 100644
--- a/llvm/lib/Target/RISCV/RISCVISelDAGToDAG.h
+++ b/llvm/lib/Target/RISCV/RISCVISelDAGToDAG.h
@@ -261,13 +261,6 @@ struct VLX_VSXPseudo {
   uint16_t Pseudo;
 };
 
-struct RISCVMaskedPseudoInfo {
-  uint16_t MaskedPseudo;
-  uint16_t UnmaskedPseudo;
-  uint8_t MaskOpIdx;
-  uint8_t MaskAffectsResult : 1;
-};
-
 #define GET_RISCVVSSEGTable_DECL
 #define GET_RISCVVLSEGTable_DECL
 #define GET_RISCVVLXSEGTable_DECL
@@ -276,8 +269,6 @@ struct RISCVMaskedPseudoInfo {
 #define GET_RISCVVSETable_DECL
 #define GET_RISCVVLXTable_DECL
 #define GET_RISCVVSXTable_DECL
-#define GET_RISCVMaskedPseudosTable_DECL
-#include "RISCVGenSearchableTables.inc"
 } // namespace RISCV
 
 } // namespace llvm
diff --git a/llvm/lib/Target/RISCV/RISCVISelLowering.cpp b/llvm/lib/Target/RISCV/RISCVISelLowering.cpp
index 27387595164a46..3c5b93e532f96f 100644
--- a/llvm/lib/Target/RISCV/RISCVISelLowering.cpp
+++ b/llvm/lib/Target/RISCV/RISCVISelLowering.cpp
@@ -17677,80 +17677,13 @@ static MachineBasicBlock *emitVFROUND_NOEXCEPT_MASK(MachineInstr &MI,
   unsigned Log2SEW = MI.getOperand(RISCVII::getSEWOpNum(MI.getDesc())).getImm();
   // There is no E8 variant for VFCVT_F_X.
   assert(Log2SEW >= 4);
-  // Since MI (VFROUND) isn't SEW specific, we cannot use a macro to make
-  // handling of different (LMUL, SEW) pairs easier because we need to pull the
-  // SEW immediate from MI, and that information is not avaliable during macro
-  // expansion.
-  unsigned CVTFOpc;
-  if (Log2SEW == 4) {
-    switch (LMul) {
-    case RISCVII::LMUL_1:
-      CVTFOpc = RISCV::PseudoVFCVT_F_X_V_M1_E16_MASK;
-      break;
-    case RISCVII::LMUL_2:
-      CVTFOpc = RISCV::PseudoVFCVT_F_X_V_M2_E16_MASK;
-      break;
-    case RISCVII::LMUL_4:
-      CVTFOpc = RISCV::PseudoVFCVT_F_X_V_M4_E16_MASK;
-      break;
-    case RISCVII::LMUL_8:
-      CVTFOpc = RISCV::PseudoVFCVT_F_X_V_M8_E16_MASK;
-      break;
-    case RISCVII::LMUL_F2:
-      CVTFOpc = RISCV::PseudoVFCVT_F_X_V_MF2_E16_MASK;
-      break;
-    case RISCVII::LMUL_F4:
-      CVTFOpc = RISCV::PseudoVFCVT_F_X_V_MF4_E16_MASK;
-      break;
-    case RISCVII::LMUL_F8:
-    case RISCVII::LMUL_RESERVED:
-      llvm_unreachable("Unexpected LMUL and SEW combination value for MI.");
-    }
-  } else if (Log2SEW == 5) {
-    switch (LMul) {
-    case RISCVII::LMUL_1:
-      CVTFOpc = RISCV::PseudoVFCVT_F_X_V_M1_E32_MASK;
-      break;
-    case RISCVII::LMUL_2:
-      CVTFOpc = RISCV::PseudoVFCVT_F_X_V_M2_E32_MASK;
-      break;
-    case RISCVII::LMUL_4:
-      CVTFOpc = RISCV::PseudoVFCVT_F_X_V_M4_E32_MASK;
-      break;
-    case RISCVII::LMUL_8:
-      CVTFOpc = RISCV::PseudoVFCVT_F_X_V_M8_E32_MASK;
-      break;
-    case RISCVII::LMUL_F2:
-      CVTFOpc = RISCV::PseudoVFCVT_F_X_V_MF2_E32_MASK;
-      break;
-    case RISCVII::LMUL_F4:
-    case RISCVII::LMUL_F8:
-    case RISCVII::LMUL_RESERVED:
-      llvm_unreachable("Unexpected LMUL and SEW combination value for MI.");
-    }
-  } else if (Log2SEW == 6) {
-    switch (LMul) {
-    case RISCVII::LMUL_1:
-      CVTFOpc = RISCV::PseudoVFCVT_F_X_V_M1_E64_MASK;
-      break;
-    case RISCVII::LMUL_2:
-      CVTFOpc = RISCV::PseudoVFCVT_F_X_V_M2_E64_MASK;
-      break;
-    case RISCVII::LMUL_4:
-      CVTFOpc = RISCV::PseudoVFCVT_F_X_V_M4_E64_MASK;
-      break;
-    case RISCVII::LMUL_8:
-      CVTFOpc = RISCV::PseudoVFCVT_F_X_V_M8_E64_MASK;
-      break;
-    case RISCVII::LMUL_F2:
-    case RISCVII::LMUL_F4:
-    case RISCVII::LMUL_F8:
-    case RISCVII::LMUL_RESERVED:
-      llvm_unreachable("Unexpected LMUL and SEW combination value for MI.");
-    }
-  } else {
-    llvm_unreachable("Unexpected LMUL and SEW combination value for MI.");
-  }
+  const RISCVVInversePseudosTable::PseudoInfo *Inverse =
+      RISCVVInversePseudosTable::getBaseInfo(RISCV::VFCVT_F_X_V, LMul,
+                                             1 << Log2SEW);
+  assert(Inverse && "Unexpected LMUL and SEW pair for VFCVT_F_X_V instruction");
+  auto *Masked = RISCV::lookupMaskedIntrinsicByUnmasked(Inverse->Pseudo);
+  assert(Masked && "Unexpected LMUL and SEW pair for VFCVT_F_X_V instruction");
+  unsigned CVTFOpc = Masked->MaskedPseudo;
 
   BuildMI(*BB, MI, DL, TII.get(CVTFOpc))
       .add(MI.getOperand(0))
diff --git a/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp b/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp
index 508f607fab20fd..383ca35890aaa4 100644
--- a/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp
+++ b/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp
@@ -66,6 +66,22 @@ using namespace RISCV;
 
 } // namespace llvm::RISCVVPseudosTable
 
+namespace llvm::RISCVVInversePseudosTable {
+
+using namespace RISCV;
+
+#define GET_RISCVVInversePseudosTable_IMPL
+#include "RISCVGenSearchableTables.inc"
+
+} // end namespace RISCVVInversePseudosTable
+
+namespace llvm::RISCV {
+
+#define GET_RISCVMaskedPseudosTable_IMPL
+#include "RISCVGenSearchableTables.inc"
+
+} // end namespace RISCVMaskedPseudosTable
+
 RISCVInstrInfo::RISCVInstrInfo(RISCVSubtarget &STI)
     : RISCVGenInstrInfo(RISCV::ADJCALLSTACKDOWN, RISCV::ADJCALLSTACKUP),
       STI(STI) {}
diff --git a/llvm/lib/Target/RISCV/RISCVInstrInfo.h b/llvm/lib/Target/RISCV/RISCVInstrInfo.h
index 70fe7da85be0e7..788f2f568765f0 100644
--- a/llvm/lib/Target/RISCV/RISCVInstrInfo.h
+++ b/llvm/lib/Target/RISCV/RISCVInstrInfo.h
@@ -359,5 +359,31 @@ struct PseudoInfo {
 
 } // end namespace RISCVVPseudosTable
 
+namespace RISCVVInversePseudosTable {
+
+struct PseudoInfo {
+  uint16_t Pseudo;
+  uint16_t BaseInstr;
+  uint8_t VLMul;
+  uint8_t SEW;
+};
+
+#define GET_RISCVVInversePseudosTable_DECL
+#include "RISCVGenSearchableTables.inc"
+
+} // end namespace RISCVVInversePseudosTable
+
+namespace RISCV {
+
+struct RISCVMaskedPseudoInfo {
+  uint16_t MaskedPseudo;
+  uint16_t UnmaskedPseudo;
+  uint8_t MaskOpIdx;
+  uint8_t MaskAffectsResult : 1;
+};
+#define GET_RISCVMaskedPseudosTable_DECL
+#include "RISCVGenSearchableTables.inc"
+} // end namespace RISCV
+
 } // end namespace llvm
 #endif
diff --git a/llvm/lib/Target/RISCV/RISCVInstrInfoVPseudos.td b/llvm/lib/Target/RISCV/RISCVInstrInfoVPseudos.td
index ad1821d57256bc..8b0352c76598af 100644
--- a/llvm/lib/Target/RISCV/RISCVInstrInfoVPseudos.td
+++ b/llvm/lib/Target/RISCV/RISCVInstrInfoVPseudos.td
@@ -3585,7 +3585,7 @@ multiclass VPseudoConversion<VReg RetClass,
                              int sew = 0,
                              int TargetConstraintType = 1> {
   defvar suffix = !if(sew, "_" # MInfo.MX # "_E" # sew, "_" # MInfo.MX);
-  let VLMul = MInfo.value in {
+  let VLMul = MInfo.value, SEW=sew in {
     def suffix : VPseudoUnaryNoMask<RetClass, Op1Class, Constraint, TargetConstraintType>;
     def suffix # "_MASK" : VPseudoUnaryMask<RetClass, Op1Class,
                                             Constraint, TargetConstraintType>,
@@ -3599,7 +3599,7 @@ multiclass VPseudoConversionRoundingMode<VReg RetClass,
                              string Constraint = "",
                              int sew = 0,
                              int TargetConstraintType = 1> {
-  let VLMul = MInfo.value in {
+  let VLMul = MInfo.value, SEW=sew in {
     defvar suffix = !if(sew, "_" # MInfo.MX # "_E" # sew, "_" # MInfo.MX);
     def suffix : VPseudoUnaryNoMaskRoundingMode<RetClass, Op1Class, Constraint, TargetConstraintType>;
     def suffix # "_MASK" : VPseudoUnaryMaskRoundingMode<RetClass, Op1Class,
@@ -3616,7 +3616,7 @@ multiclass VPseudoConversionRM<VReg RetClass,
                                string Constraint = "",
                                int sew = 0,
                                int TargetConstraintType = 1> {
-  let VLMul = MInfo.value in {
+  let VLMul = MInfo.value, SEW=sew in {
     defvar suffix = !if(sew, "_" # MInfo.MX # "_E" # sew, "_" # MInfo.MX);
     def suffix : VPseudoUnaryNoMask_FRM<RetClass, Op1Class,
                                         Constraint, TargetConstraintType>;

Copy link

github-actions bot commented Apr 15, 2024

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

@michaelmaitland michaelmaitland force-pushed the cvtf-lookup branch 2 times, most recently from e95d5ae to c6352bc Compare April 15, 2024 14:53
@michaelmaitland michaelmaitland force-pushed the cvtf-lookup branch 2 times, most recently from fecda61 to 37e883c Compare April 15, 2024 14:56
llvm/lib/Target/RISCV/RISCVInstrInfo.h Outdated Show resolved Hide resolved
@@ -3585,7 +3585,7 @@ multiclass VPseudoConversion<VReg RetClass,
int sew = 0,
int TargetConstraintType = 1> {
defvar suffix = !if(sew, "_" # MInfo.MX # "_E" # sew, "_" # MInfo.MX);
let VLMul = MInfo.value in {
let VLMul = MInfo.value, SEW=sew in {
Copy link
Contributor

Choose a reason for hiding this comment

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

Not related to this patch, but can you check if there are more misses of setting SEW?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes I will check and can add a new PR.

llvm/lib/Target/RISCV/RISCVISelLowering.cpp Outdated Show resolved Hide resolved
Copy link
Contributor

@wangpc-pp wangpc-pp left a comment

Choose a reason for hiding this comment

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

This patch contains 5 commits

I think 3 commits will be enough:

pick [RISCV][NFC] Move RISCVMaskedPseudoTable to RISCVInstrInfo
pick [RISCV][NFC] Include RISCVVInversePseudosTable in RISCVInstrInfo
squash [RISCV] Move RISCVInversePseudosTable to RISCVMCTargetDesc.h
pick [RISCV] Use lookup tables to find CVTFOpc
squash [RISCV] Add and use helper lookupMaskedIntrinsic

#define GET_RISCVVInversePseudosTable_IMPL
#define GET_RISCVVInversePseudosTable_DECL
#include "RISCVGenSearchableTables.inc"
Copy link
Contributor

Choose a reason for hiding this comment

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

We don't need to include this any more, we can use the one in RISCVMCTargetDesc.h.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Are you sure? It looks like we need to have the include after the _IMPL define. The include in RISCVMCTargetDesc.h occurs before this and GET_RISCVVInversePseudosTable_IMPL is not set yet. RISCVInstrInfo.cpp does it like we have here for RISCVVPseudosTable_IMPL for example.

Copy link
Contributor

Choose a reason for hiding this comment

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

I meant we can put GET_RISCVVInversePseudosTable_DECL in RISCVMCTargetDesc.h and put GET_RISCVVInversePseudosTable_IMPL in RISCVMCTargetDesc.cpp.

@@ -66,6 +66,22 @@ using namespace RISCV;

} // namespace llvm::RISCVVPseudosTable

namespace llvm::RISCVVInversePseudosTable {
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be put in RISCVMCTargetDesc.cpp

#define GET_RISCVVInversePseudosTable_IMPL
#define GET_RISCVVInversePseudosTable_DECL
#include "RISCVGenSearchableTables.inc"
Copy link
Contributor

Choose a reason for hiding this comment

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

I meant we can put GET_RISCVVInversePseudosTable_DECL in RISCVMCTargetDesc.h and put GET_RISCVVInversePseudosTable_IMPL in RISCVMCTargetDesc.cpp.

Copy link
Contributor

@wangpc-pp wangpc-pp left a comment

Choose a reason for hiding this comment

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

LGTM.

Copy link
Collaborator

@topperc topperc left a comment

Choose a reason for hiding this comment

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

LGTM

@michaelmaitland
Copy link
Contributor Author

Closed by 12d4724, cdc3931, 80f510b

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

4 participants