[RISCV] Use AdjustInstrPostInstrSelection to insert a FRM dependency for scalar FP instructions with dynamic rounding mode.

[craig.topper]

Summary

In order to support constrained FP intrinsics we need to model FRM
dependency. Whether or not a instruction uses FRM is based on a 3
bit field in the instruction. Because of this we can't add
'Uses = [FRM]' to the tablegen descriptions.

This patch examines the immediate after isel and adds an implicit
use of FRM. This idea came from Roger Ferrer Ibanez.

Other ideas:
We could be overly conservative and just pretend all instructions with
frm field read the FRM register. Or we could have pseudoinstructions
for CodeGen with rounding mode.

[jrtc27]

As it stands the implementation is a bit assuming, relying on it only ever being called for floating point instructions (or those that don't have a funct3), but there are other uses, so should also be checking getOperandType is OpTypes::frmarg? Could even ditch the name lookup entirely and just iterate over the use operands, don't know what's better.

llvm/lib/Target/RISCV/RISCVInstrInfo.h

24

Combine?

[jrtc27]

Regarding something like using pseudos instead, you could, but that's a lot more effort, so if this works it's probably better. If it doesn't work then it's easy enough to switch to something more involved, this is pretty non-invasive.

[craig.topper]

Rename funct3 in the underlying tablegen classes to frm so that we can call
it frm in the lookup. This is what I wanted to do all along, but failed to
realize that FP was using its own *Frm format class.

[arcbbb]

LGTM

[asb]

Using AdjustInstrPostInstrSelection is a neat way to handle this I think. LGTM.

[frasercrmck]

LGTM too. The only thing I'd be concerned about is something in LLVM dropping those implicit uses but we can probably fix those bugs if and when.

llvm/lib/Target/RISCV/RISCVISelLowering.h

public:
   void LowerAsmOperandForConstraint(SDValue Op, std::string &Constraint,
                                     std::vector<SDValue> &Ops,
                                     SelectionDAG &DAG) const override;
 
   MachineBasicBlock *
   EmitInstrWithCustomInserter(MachineInstr &MI,
                               MachineBasicBlock *BB) const override;
 
+  void AdjustInstrPostInstrSelection(MachineInstr &MI,
+                                     SDNode *Node) const override;
+
   EVT getSetCCResultType(const DataLayout &DL, LLVMContext &Context,
                          EVT VT) const override;
 
   bool convertSetCCLogicToBitwiseLogic(EVT VT) const override {
     return VT.isScalarInteger();
   }
   bool convertSelectOfConstantsToMath(EVT VT) const override { return true; }
 

llvm/lib/Target/RISCV/RISCVISelLowering.cpp

case RISCV::Select_FPR64_Using_CC_GPR:
     return emitSelectPseudo(MI, BB, Subtarget);
   case RISCV::BuildPairF64Pseudo:
     return emitBuildPairF64Pseudo(MI, BB);
   case RISCV::SplitF64Pseudo:
     return emitSplitF64Pseudo(MI, BB);
   }
 }
 
+void RISCVTargetLowering::AdjustInstrPostInstrSelection(MachineInstr &MI,
+                                                        SDNode *Node) const {
+  // Add FRM dependency to any instructions with dynamic rounding mode.
+  unsigned Opc = MI.getOpcode();
+  auto Idx = RISCV::getNamedOperandIdx(Opc, RISCV::OpName::frm);
+  if (Idx < 0)
+    return;
+  if (MI.getOperand(Idx).getImm() != RISCVFPRndMode::DYN)
+    return;
+  // If the instruction already reads FRM, don't add another read.
+  if (MI.readsRegister(RISCV::FRM))
+    return;
+  MI.addOperand(
+      MachineOperand::CreateReg(RISCV::FRM, /*isDef*/ false, /*isImp*/ true));
+}
+
 // Calling Convention Implementation.
 // The expectations for frontend ABI lowering vary from target to target.
 // Ideally, an LLVM frontend would be able to avoid worrying about many ABI
 // details, but this is a longer term goal. For now, we simply try to keep the
 // role of the frontend as simple and well-defined as possible. The rules can
 // be summarised as:
 // * Never split up large scalar arguments. We handle them here.
 // * If a hardfloat calling convention is being used, and the struct may be

llvm/lib/Target/RISCV/RISCVInstrFormats.td

 }
 
 class RVInstR4Frm<bits<2> funct2, RISCVOpcode opcode, dag outs, dag ins,
                   string opcodestr, string argstr>
     : RVInst<outs, ins, opcodestr, argstr, [], InstFormatR4> {
   bits<5> rs3;
   bits<5> rs2;
   bits<5> rs1;
-  bits<3> funct3;
+  bits<3> frm;
   bits<5> rd;
 
   let Inst{31-27} = rs3;
   let Inst{26-25} = funct2;
   let Inst{24-20} = rs2;
   let Inst{19-15} = rs1;
-  let Inst{14-12} = funct3;
+  let Inst{14-12} = frm;
   let Inst{11-7} = rd;
   let Opcode = opcode.Value;
 }
 
 class RVInstRAtomic<bits<5> funct5, bit aq, bit rl, bits<3> funct3,
                     RISCVOpcode opcode, dag outs, dag ins, string opcodestr,
                     string argstr>
     : RVInst<outs, ins, opcodestr, argstr, [], InstFormatR> {
class RVInstRAtomic<bits<5> funct5, bit aq, bit rl, bits<3> funct3,
   let Opcode = opcode.Value;
 }
 
 class RVInstRFrm<bits<7> funct7, RISCVOpcode opcode, dag outs, dag ins,
                  string opcodestr, string argstr>
     : RVInst<outs, ins, opcodestr, argstr, [], InstFormatR> {
   bits<5> rs2;
   bits<5> rs1;
-  bits<3> funct3;
+  bits<3> frm;
   bits<5> rd;
 
   let Inst{31-25} = funct7;
   let Inst{24-20} = rs2;
   let Inst{19-15} = rs1;
-  let Inst{14-12} = funct3;
+  let Inst{14-12} = frm;
   let Inst{11-7} = rd;
   let Opcode = opcode.Value;
 }
 
 class RVInstI<bits<3> funct3, RISCVOpcode opcode, dag outs, dag ins,
               string opcodestr, string argstr>
     : RVInst<outs, ins, opcodestr, argstr, [], InstFormatI> {
   bits<12> imm12;

llvm/lib/Target/RISCV/RISCVInstrInfo.h

 #ifndef LLVM_LIB_TARGET_RISCV_RISCVINSTRINFO_H
 #define LLVM_LIB_TARGET_RISCV_RISCVINSTRINFO_H
 
 #include "RISCVRegisterInfo.h"
 #include "llvm/CodeGen/TargetInstrInfo.h"
 #include "llvm/IR/DiagnosticInfo.h"
 
 #define GET_INSTRINFO_HEADER
+#define GET_INSTRINFO_OPERAND_ENUM
 #include "RISCVGenInstrInfo.inc"
 
 namespace llvm {
 
 class RISCVSubtarget;
 
 namespace RISCVCC {
 
 enum CondCode {
   COND_EQ,
   COND_NE,
public:
   Optional<std::pair<unsigned, unsigned>>
   isRVVSpillForZvlsseg(unsigned Opcode) const;
 
 protected:
   const RISCVSubtarget &STI;
 };
 
 namespace RISCV {
+
+// Implemented in RISCVGenInstrInfo.inc
+int16_t getNamedOperandIdx(uint16_t Opcode, uint16_t NamedIndex);
+
 // Special immediate for AVL operand of V pseudo instructions to indicate VLMax.
 static constexpr int64_t VLMaxSentinel = -1LL;
 } // namespace RISCV
 
 namespace RISCVVPseudosTable {
 
 struct PseudoInfo {
   uint16_t Pseudo;

llvm/lib/Target/RISCV/RISCVInstrInfo.cpp

 #include "llvm/Support/ErrorHandling.h"
 
 using namespace llvm;
 
 #define GEN_CHECK_COMPRESS_INSTR
 #include "RISCVGenCompressInstEmitter.inc"
 
 #define GET_INSTRINFO_CTOR_DTOR
+#define GET_INSTRINFO_NAMED_OPS
 #include "RISCVGenInstrInfo.inc"
 
 static cl::opt<bool> PreferWholeRegisterMove(
     "riscv-prefer-whole-register-move", cl::init(false), cl::Hidden,
     cl::desc("Prefer whole register move for vector registers."));
 
 namespace llvm {
 namespace RISCVVPseudosTable {

llvm/lib/Target/RISCV/RISCVInstrInfoF.td

def frmarg : Operand<XLenVT> {
   let PrintMethod = "printFRMArg";
   let DecoderMethod = "decodeFRMArg";
 }
 
 //===----------------------------------------------------------------------===//
 // Instruction class templates
 //===----------------------------------------------------------------------===//
 
-let hasSideEffects = 0, mayLoad = 0, mayStore = 0, mayRaiseFPException = 1 in
+let hasSideEffects = 0, mayLoad = 0, mayStore = 0, mayRaiseFPException = 1,
+    UseNamedOperandTable = 1, hasPostISelHook = 1 in
 class FPFMA_rrr_frm<RISCVOpcode opcode, bits<2> funct2, string opcodestr,
                     RegisterClass rty>
     : RVInstR4Frm<funct2, opcode, (outs rty:$rd),
-                  (ins rty:$rs1, rty:$rs2, rty:$rs3, frmarg:$funct3),
-                  opcodestr, "$rd, $rs1, $rs2, $rs3, $funct3">;
+                  (ins rty:$rs1, rty:$rs2, rty:$rs3, frmarg:$frm),
+                  opcodestr, "$rd, $rs1, $rs2, $rs3, $frm">;
 
 class FPFMADynFrmAlias<FPFMA_rrr_frm Inst, string OpcodeStr,
                        RegisterClass rty>
     : InstAlias<OpcodeStr#" $rd, $rs1, $rs2, $rs3",
                 (Inst rty:$rd, rty:$rs1, rty:$rs2, rty:$rs3, 0b111)>;
 
 let hasSideEffects = 0, mayLoad = 0, mayStore = 0, mayRaiseFPException = 1 in
 class FPALU_rr<bits<7> funct7, bits<3> funct3, string opcodestr,
                RegisterClass rty>
     : RVInstR<funct7, funct3, OPC_OP_FP, (outs rty:$rd),
               (ins rty:$rs1, rty:$rs2), opcodestr, "$rd, $rs1, $rs2">;
 
-let hasSideEffects = 0, mayLoad = 0, mayStore = 0, mayRaiseFPException = 1 in
+let hasSideEffects = 0, mayLoad = 0, mayStore = 0, mayRaiseFPException = 1,
+    UseNamedOperandTable = 1, hasPostISelHook = 1 in
 class FPALU_rr_frm<bits<7> funct7, string opcodestr, RegisterClass rty>
     : RVInstRFrm<funct7, OPC_OP_FP, (outs rty:$rd),
-                 (ins rty:$rs1, rty:$rs2, frmarg:$funct3), opcodestr,
-                  "$rd, $rs1, $rs2, $funct3">;
+                 (ins rty:$rs1, rty:$rs2, frmarg:$frm), opcodestr,
+                  "$rd, $rs1, $rs2, $frm">;
 
 class FPALUDynFrmAlias<FPALU_rr_frm Inst, string OpcodeStr,
                        RegisterClass rty>
     : InstAlias<OpcodeStr#" $rd, $rs1, $rs2",
                 (Inst rty:$rd, rty:$rs1, rty:$rs2, 0b111)>;
 
 let hasSideEffects = 0, mayLoad = 0, mayStore = 0, mayRaiseFPException = 1 in
 class FPUnaryOp_r<bits<7> funct7, bits<5> rs2val, bits<3> funct3,
                   RegisterClass rdty, RegisterClass rs1ty, string opcodestr>
     : RVInstR<funct7, funct3, OPC_OP_FP, (outs rdty:$rd), (ins rs1ty:$rs1),
               opcodestr, "$rd, $rs1"> {
   let rs2 = rs2val;
 }
 
-let hasSideEffects = 0, mayLoad = 0, mayStore = 0, mayRaiseFPException = 1 in
+let hasSideEffects = 0, mayLoad = 0, mayStore = 0, mayRaiseFPException = 1,
+    UseNamedOperandTable = 1, hasPostISelHook = 1 in
 class FPUnaryOp_r_frm<bits<7> funct7, bits<5> rs2val, RegisterClass rdty,
                       RegisterClass rs1ty, string opcodestr>
     : RVInstRFrm<funct7, OPC_OP_FP, (outs rdty:$rd),
-                 (ins rs1ty:$rs1, frmarg:$funct3), opcodestr,
-                  "$rd, $rs1, $funct3"> {
+                 (ins rs1ty:$rs1, frmarg:$frm), opcodestr,
+                  "$rd, $rs1, $frm"> {
   let rs2 = rs2val;
 }
 
 class FPUnaryOpDynFrmAlias<FPUnaryOp_r_frm Inst, string OpcodeStr,
                            RegisterClass rdty, RegisterClass rs1ty>
     : InstAlias<OpcodeStr#" $rd, $rs1",
                 (Inst rdty:$rd, rs1ty:$rs1, 0b111)>;
 

llvm/test/CodeGen/RISCV/frm-dependency.ll

+; NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py
+; RUN: llc -mtriple=riscv32 -mattr=+f,+d -stop-after=finalize-isel < %s \
+; RUN:   | FileCheck -check-prefixes=RV32IF %s
+; RUN: llc -mtriple=riscv64 -mattr=+f,+d -stop-after=finalize-isel < %s \
+; RUN:   | FileCheck -check-prefixes=RV64IF %s
+
+; Make sure an implicit FRM dependency is added to instructions with dynamic
+; rounding.
+
+define float @fadd_s(float %a, float %b) nounwind {
+  ; RV32IF-LABEL: name: fadd_s
+  ; RV32IF: bb.0 (%ir-block.0):
+  ; RV32IF-NEXT:   liveins: $x10, $x11
+  ; RV32IF-NEXT: {{  $}}
+  ; RV32IF-NEXT:   [[COPY:%[0-9]+]]:gpr = COPY $x11
+  ; RV32IF-NEXT:   [[COPY1:%[0-9]+]]:gpr = COPY $x10
+  ; RV32IF-NEXT:   [[FMV_W_X:%[0-9]+]]:fpr32 = FMV_W_X [[COPY]]
+  ; RV32IF-NEXT:   [[FMV_W_X1:%[0-9]+]]:fpr32 = FMV_W_X [[COPY1]]
+  ; RV32IF-NEXT:   %4:fpr32 = nofpexcept FADD_S killed [[FMV_W_X1]], killed [[FMV_W_X]], 7, implicit $frm
+  ; RV32IF-NEXT:   [[FMV_X_W:%[0-9]+]]:gpr = FMV_X_W killed %4
+  ; RV32IF-NEXT:   $x10 = COPY [[FMV_X_W]]
+  ; RV32IF-NEXT:   PseudoRET implicit $x10
+  ; RV64IF-LABEL: name: fadd_s
+  ; RV64IF: bb.0 (%ir-block.0):
+  ; RV64IF-NEXT:   liveins: $x10, $x11
+  ; RV64IF-NEXT: {{  $}}
+  ; RV64IF-NEXT:   [[COPY:%[0-9]+]]:gpr = COPY $x11
+  ; RV64IF-NEXT:   [[COPY1:%[0-9]+]]:gpr = COPY $x10
+  ; RV64IF-NEXT:   [[FMV_W_X:%[0-9]+]]:fpr32 = FMV_W_X [[COPY]]
+  ; RV64IF-NEXT:   [[FMV_W_X1:%[0-9]+]]:fpr32 = FMV_W_X [[COPY1]]
+  ; RV64IF-NEXT:   %4:fpr32 = nofpexcept FADD_S killed [[FMV_W_X1]], killed [[FMV_W_X]], 7, implicit $frm
+  ; RV64IF-NEXT:   [[FMV_X_W:%[0-9]+]]:gpr = FMV_X_W killed %4
+  ; RV64IF-NEXT:   $x10 = COPY [[FMV_X_W]]
+  ; RV64IF-NEXT:   PseudoRET implicit $x10
+  %1 = fadd float %a, %b
+  ret float %1
+}
+
+declare float @llvm.fma.f32(float, float, float)
+
+define float @fmadd_s(float %a, float %b, float %c) nounwind {
+  ; RV32IF-LABEL: name: fmadd_s
+  ; RV32IF: bb.0 (%ir-block.0):
+  ; RV32IF-NEXT:   liveins: $x10, $x11, $x12
+  ; RV32IF-NEXT: {{  $}}
+  ; RV32IF-NEXT:   [[COPY:%[0-9]+]]:gpr = COPY $x12
+  ; RV32IF-NEXT:   [[COPY1:%[0-9]+]]:gpr = COPY $x11
+  ; RV32IF-NEXT:   [[COPY2:%[0-9]+]]:gpr = COPY $x10
+  ; RV32IF-NEXT:   [[FMV_W_X:%[0-9]+]]:fpr32 = FMV_W_X [[COPY]]
+  ; RV32IF-NEXT:   [[FMV_W_X1:%[0-9]+]]:fpr32 = FMV_W_X [[COPY1]]
+  ; RV32IF-NEXT:   [[FMV_W_X2:%[0-9]+]]:fpr32 = FMV_W_X [[COPY2]]
+  ; RV32IF-NEXT:   %6:fpr32 = nofpexcept FMADD_S killed [[FMV_W_X2]], killed [[FMV_W_X1]], killed [[FMV_W_X]], 7, implicit $frm
+  ; RV32IF-NEXT:   [[FMV_X_W:%[0-9]+]]:gpr = FMV_X_W killed %6
+  ; RV32IF-NEXT:   $x10 = COPY [[FMV_X_W]]
+  ; RV32IF-NEXT:   PseudoRET implicit $x10
+  ; RV64IF-LABEL: name: fmadd_s
+  ; RV64IF: bb.0 (%ir-block.0):
+  ; RV64IF-NEXT:   liveins: $x10, $x11, $x12
+  ; RV64IF-NEXT: {{  $}}
+  ; RV64IF-NEXT:   [[COPY:%[0-9]+]]:gpr = COPY $x12
+  ; RV64IF-NEXT:   [[COPY1:%[0-9]+]]:gpr = COPY $x11
+  ; RV64IF-NEXT:   [[COPY2:%[0-9]+]]:gpr = COPY $x10
+  ; RV64IF-NEXT:   [[FMV_W_X:%[0-9]+]]:fpr32 = FMV_W_X [[COPY]]
+  ; RV64IF-NEXT:   [[FMV_W_X1:%[0-9]+]]:fpr32 = FMV_W_X [[COPY1]]
+  ; RV64IF-NEXT:   [[FMV_W_X2:%[0-9]+]]:fpr32 = FMV_W_X [[COPY2]]
+  ; RV64IF-NEXT:   %6:fpr32 = nofpexcept FMADD_S killed [[FMV_W_X2]], killed [[FMV_W_X1]], killed [[FMV_W_X]], 7, implicit $frm
+  ; RV64IF-NEXT:   [[FMV_X_W:%[0-9]+]]:gpr = FMV_X_W killed %6
+  ; RV64IF-NEXT:   $x10 = COPY [[FMV_X_W]]
+  ; RV64IF-NEXT:   PseudoRET implicit $x10
+  %1 = call float @llvm.fma.f32(float %a, float %b, float %c)
+  ret float %1
+}
+
+; This uses rtz instead of dyn rounding mode so shouldn't have an FRM dependncy.
+define i32 @fcvt_w_s(float %a) nounwind {
+  ; RV32IF-LABEL: name: fcvt_w_s
+  ; RV32IF: bb.0 (%ir-block.0):
+  ; RV32IF-NEXT:   liveins: $x10
+  ; RV32IF-NEXT: {{  $}}
+  ; RV32IF-NEXT:   [[COPY:%[0-9]+]]:gpr = COPY $x10
+  ; RV32IF-NEXT:   [[FMV_W_X:%[0-9]+]]:fpr32 = FMV_W_X [[COPY]]
+  ; RV32IF-NEXT:   %2:gpr = nofpexcept FCVT_W_S killed [[FMV_W_X]], 1
+  ; RV32IF-NEXT:   $x10 = COPY %2
+  ; RV32IF-NEXT:   PseudoRET implicit $x10
+  ; RV64IF-LABEL: name: fcvt_w_s
+  ; RV64IF: bb.0 (%ir-block.0):
+  ; RV64IF-NEXT:   liveins: $x10
+  ; RV64IF-NEXT: {{  $}}
+  ; RV64IF-NEXT:   [[COPY:%[0-9]+]]:gpr = COPY $x10
+  ; RV64IF-NEXT:   [[FMV_W_X:%[0-9]+]]:fpr32 = FMV_W_X [[COPY]]
+  ; RV64IF-NEXT:   %2:gpr = nofpexcept FCVT_W_S killed [[FMV_W_X]], 1
+  ; RV64IF-NEXT:   $x10 = COPY %2
+  ; RV64IF-NEXT:   PseudoRET implicit $x10
+  %1 = fptosi float %a to i32
+  ret i32 %1
+}
+
+; This doesn't use a rounding mode since i32 can be represented exactly as a
+; double.
+define double @fcvt_d_w(i32 %a) nounwind {
+  ; RV32IF-LABEL: name: fcvt_d_w
+  ; RV32IF: bb.0 (%ir-block.0):
+  ; RV32IF-NEXT:   liveins: $x10
+  ; RV32IF-NEXT: {{  $}}
+  ; RV32IF-NEXT:   [[COPY:%[0-9]+]]:gpr = COPY $x10
+  ; RV32IF-NEXT:   %1:fpr64 = nofpexcept FCVT_D_W [[COPY]]
+  ; RV32IF-NEXT:   FSD killed %1, %stack.0, 0 :: (store (s64) into %stack.0)
+  ; RV32IF-NEXT:   [[LW:%[0-9]+]]:gpr = LW %stack.0, 0 :: (load (s32) from %stack.0, align 8)
+  ; RV32IF-NEXT:   [[LW1:%[0-9]+]]:gpr = LW %stack.0, 4 :: (load (s32) from %stack.0 + 4, basealign 8)
+  ; RV32IF-NEXT:   $x10 = COPY [[LW]]
+  ; RV32IF-NEXT:   $x11 = COPY [[LW1]]
+  ; RV32IF-NEXT:   PseudoRET implicit $x10, implicit $x11
+  ; RV64IF-LABEL: name: fcvt_d_w
+  ; RV64IF: bb.0 (%ir-block.0):
+  ; RV64IF-NEXT:   liveins: $x10
+  ; RV64IF-NEXT: {{  $}}
+  ; RV64IF-NEXT:   [[COPY:%[0-9]+]]:gpr = COPY $x10
+  ; RV64IF-NEXT:   %1:fpr64 = nofpexcept FCVT_D_W [[COPY]]
+  ; RV64IF-NEXT:   [[FMV_X_D:%[0-9]+]]:gpr = FMV_X_D killed %1
+  ; RV64IF-NEXT:   $x10 = COPY [[FMV_X_D]]
+  ; RV64IF-NEXT:   PseudoRET implicit $x10
+  %1 = sitofp i32 %a to double
+  ret double %1
+}