[PATCH] D78887: [VE] Support floating point immediate values
Simon Moll via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Mon Apr 27 01:01:26 PDT 2020
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
More information about the llvm-commits
mailing list