[RISCV] Add missing hasPostISelHook = 1 to vector pseudos that might read FRM.

[topperc]

We need an implicit FRM read operand anytime the rounding mode is dynamic. The post isel hook is responsible for this when isel creates an instruction with dynamic rounding mode.

Add a MachineVerifier check to verify the operand is present.

[wangpc-pp]

I don't know if I understand correctly, we don't add frm operand for some cases before?

[topperc]

I don't know if I understand correctly, we don't add frm operand for some cases before?

That's correct. I think InsertReadWriteCSR would add it in the tests I looked at, but that pass really isn't supposed to anything for DYN instructions.

[preames]

LGTM

[preames]

This is checking the pseudo form, is there any way that we can add an additional assert which checks the pseudo against the underlying instruction? The underlying instruction needs to have the use of FRM too doesn't it?

(Possible follow up)

[topperc]

The underlying instruction should have it but I don't think anything breaks if it doesn't.

[preames]

Wouldn't a disassembly use case be wrong if we didn't have the implicit use?

[topperc]

It doesn't affect the printing of the instruction so the main use of disassembly wouldn't notice.

[preames]

It doesn't look like we have support for this today, but maybe we should add a target hook for this instead? Having the post ISEL hook driven by tablgen (as opposed to a query on the underlying instruction), seems error prone. Another idea might be a target specific consistency check before emission so at least we know at build time that pseudos and instructions are in sync?

[topperc]

I was planning to move the hasPostISelHook=1 into the base class where has HasRoundMode is set. The flag should be set anytime HasRoundMode==1 && UsesVXRM=0.

[llvmbot]

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

Author: Craig Topper (topperc)

Changes

We need an implicit FRM read operand anytime the rounding mode is dynamic. The post isel hook is responsible for this when isel creates an instruction with dynamic rounding mode.

Add a MachineVerifier check to verify the operand is present.

---

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

2 Files Affected:

• (modified) llvm/lib/Target/RISCV/RISCVInstrInfo.cpp (+7)
• (modified) llvm/lib/Target/RISCV/RISCVInstrInfoVPseudos.td (+8-9)

diff --git a/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp b/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp
index d5b086861d71e6..3d515e57982e23 100644
--- a/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp
+++ b/llvm/lib/Target/RISCV/RISCVInstrInfo.cpp
@@ -2623,6 +2623,13 @@ bool RISCVInstrInfo::verifyInstruction(const MachineInstr &MI,
     }
   }
 
+  if (int Idx = RISCVII::getFRMOpNum(Desc);
+      Idx >= 0 && MI.getOperand(Idx).getImm() == RISCVFPRndMode::DYN &&
+      !MI.readsRegister(RISCV::FRM, /*TRI=*/nullptr)) {
+    ErrInfo = "dynamic rounding mode should read FRM";
+    return false;
+  }
+
   return true;
 }
 
diff --git a/llvm/lib/Target/RISCV/RISCVInstrInfoVPseudos.td b/llvm/lib/Target/RISCV/RISCVInstrInfoVPseudos.td
index 6ffdae1d7df2ae..f84aaa4c82d23a 100644
--- a/llvm/lib/Target/RISCV/RISCVInstrInfoVPseudos.td
+++ b/llvm/lib/Target/RISCV/RISCVInstrInfoVPseudos.td
@@ -6479,7 +6479,7 @@ defm PseudoVFRDIV : VPseudoVFRDIV_VF_RM;
 //===----------------------------------------------------------------------===//
 // 13.5. Vector Widening Floating-Point Multiply
 //===----------------------------------------------------------------------===//
-let mayRaiseFPException = true, hasSideEffects = 0 in {
+let mayRaiseFPException = true, hasSideEffects = 0, hasPostISelHook = 1 in {
 defm PseudoVFWMUL : VPseudoVWMUL_VV_VF_RM;
 }
 
@@ -6512,7 +6512,7 @@ defm PseudoVFWMACCBF16  : VPseudoVWMAC_VV_VF_BF_RM;
 //===----------------------------------------------------------------------===//
 // 13.8. Vector Floating-Point Square-Root Instruction
 //===----------------------------------------------------------------------===//
-let mayRaiseFPException = true, hasSideEffects = 0 in
+let mayRaiseFPException = true, hasSideEffects = 0, hasPostISelHook = 1 in
 defm PseudoVFSQRT : VPseudoVSQR_V_RM;
 
 //===----------------------------------------------------------------------===//
@@ -6524,7 +6524,7 @@ defm PseudoVFRSQRT7 : VPseudoVRCP_V;
 //===----------------------------------------------------------------------===//
 // 13.10. Vector Floating-Point Reciprocal Estimate Instruction
 //===----------------------------------------------------------------------===//
-let mayRaiseFPException = true, hasSideEffects = 0 in
+let mayRaiseFPException = true, hasSideEffects = 0, hasPostISelHook = 1 in
 defm PseudoVFREC7 : VPseudoVRCP_V_RM;
 
 //===----------------------------------------------------------------------===//
@@ -6636,9 +6636,10 @@ defm PseudoVFNCVT_F_X      : VPseudoVNCVTF_W_RM;
 defm PseudoVFNCVT_RM_F_XU  : VPseudoVNCVTF_RM_W;
 defm PseudoVFNCVT_RM_F_X   : VPseudoVNCVTF_RM_W;
 
-let hasSideEffects = 0, hasPostISelHook = 1 in
+let hasSideEffects = 0, hasPostISelHook = 1 in {
 defm PseudoVFNCVT_F_F      : VPseudoVNCVTD_W_RM;
 defm PseudoVFNCVTBF16_F_F :  VPseudoVNCVTD_W_RM;
+}
 
 defm PseudoVFNCVT_ROD_F_F  : VPseudoVNCVTD_W;
 } // mayRaiseFPException = true
@@ -6674,8 +6675,7 @@ let Predicates = [HasVInstructionsAnyF] in {
 //===----------------------------------------------------------------------===//
 // 14.3. Vector Single-Width Floating-Point Reduction Instructions
 //===----------------------------------------------------------------------===//
-let mayRaiseFPException = true,
-    hasSideEffects = 0 in {
+let mayRaiseFPException = true, hasSideEffects = 0, hasPostISelHook = 1 in {
 defm PseudoVFREDOSUM : VPseudoVFREDO_VS_RM;
 defm PseudoVFREDUSUM : VPseudoVFRED_VS_RM;
 }
@@ -6687,9 +6687,8 @@ defm PseudoVFREDMAX  : VPseudoVFREDMINMAX_VS;
 //===----------------------------------------------------------------------===//
 // 14.4. Vector Widening Floating-Point Reduction Instructions
 //===----------------------------------------------------------------------===//
-let IsRVVWideningReduction = 1,
-    hasSideEffects = 0,
-    mayRaiseFPException = true in {
+let IsRVVWideningReduction = 1, hasSideEffects = 0, mayRaiseFPException = true,
+    hasPostISelHook = 1 in {
 defm PseudoVFWREDUSUM  : VPseudoVFWRED_VS_RM;
 defm PseudoVFWREDOSUM  : VPseudoVFWREDO_VS_RM;
 }