[PATCH] D78887: [VE] Support floating point immediate values

[Simon Moll via Phabricator via llvm-commits]

simoll added a comment.

Thanks! Minor comments inline.



================
Comment at: llvm/lib/Target/VE/VEInstrInfo.td:100
+  if (M0Flag)
+    Val = countLeadingZeros(Val) | 0x40;
+  else
----------------
Could you document the purpose of this constant with a comment?


================
Comment at: llvm/lib/Target/VE/VEInstrInfo.td:107
+    const APInt& Imm = N->getValueAPF().bitcastToAPInt();
+    return isMask_64(Imm.getSExtValue()) ||
+           ((Imm.getSExtValue() & (1UL << 63)) &&
----------------
Maybe cache the `Imm.getSExtValue()` result as done above and use that variable instead.


================
Comment at: llvm/lib/Target/VE/VEInstrInfo.td:456
+               Operand immOp = simm7, Operand mOp = mimm> {
+  defm "" : RRbm<opcStr, opc, RC, Ty, RC, Ty, OpNode, immOp, mOp>;
+}
----------------
same as below


================
Comment at: llvm/lib/Target/VE/VEInstrInfo.td:465
+                 Operand immOp = simm7, Operand mOp = mimm> {
+  defm "" : RRNCbm<opcStr, opc, RC, Ty, RC, Ty, OpNode, immOp, mOp>;
+}
----------------
I suppose you could write this more naturally as `multiclass RRNCm<...> : RRNCbm<...>;`


================
Comment at: llvm/test/CodeGen/VE/fp_mul.ll:97
+; CHECK-NEXT:    or %s11, 0, %s9
+  %r = fmul float %a, 1.175494210692441075487029444849287348827052428745893333857174530571588870475618904265502351336181163787841796875E-38
+  ret float %r
----------------
I guess this is one of those cases where it makes sense to use a fp hex constant: it is shorter and the expected output relates to the bitencoding anyway.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D78887/new/

https://reviews.llvm.org/D78887