[PATCH] D90173: [PowerPC] Exploit splat instruction xxsplti32dx in Power10
Nemanja Ivanovic via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Fri Dec 11 07:06:05 PST 2020
nemanjai requested changes to this revision.
nemanjai added a comment.
This revision now requires changes to proceed.
This is not functionally correct.
================
Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:9345
+ return DAG.getBitcast(Op.getValueType(), SplatNode);
+ } else { // we may lose precision, so we have to use XXSPLTI32DX.
+
----------------
Nit: full sentences in comments. Capitalize the first word.
================
Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:9349
+ (uint32_t)((APSplatBits.getZExtValue() & 0xFFFFFFFF00000000LL) >> 32);
+ uint32_t Lo =
+ (uint32_t)(APSplatBits.getZExtValue() & 0xFFFFFFFF00000000LL);
----------------
Won't this just produce a zero?
================
Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:9351
+ (uint32_t)(APSplatBits.getZExtValue() & 0xFFFFFFFF00000000LL);
+ SDValue SplatNode;
+
----------------
Initialize to `undef` to simplify the logic below.
================
Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:9359
+ SplatNode =
+ DAG.getNode(PPCISD::XXSPLTI32DX, !Hi ? SDLoc(SplatNode) : dl,
+ MVT::v2i64, !Hi ? SplatNode : DAG.getUNDEF(MVT::v2i64),
----------------
The debug location comes from the original node (i.e. `dl` in this case). Just use that - no ternary operator.
================
Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:9361
+ MVT::v2i64, !Hi ? SplatNode : DAG.getUNDEF(MVT::v2i64),
+ DAG.getTargetConstant(isLE ? 0 : 1, dl, MVT::i32),
+ DAG.getTargetConstant(Lo, dl, MVT::i32));
----------------
Why does the index change depending on endianness? You are trying to produce a 64-bit pattern that is in no way dependent on endianness.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D90173/new/
https://reviews.llvm.org/D90173
More information about the cfe-commits
mailing list