[PATCH] D150526: [X86] Add X86FixupVectorConstantsPass to re-fold AVX512 vector load folds as broadcast folds

[Simon Pilgrim via Phabricator via llvm-commits]

RKSimon added inline comments.


================
Comment at: llvm/lib/Target/X86/X86FixupVectorConstants.cpp:218
+    RawBits.push_back(Splat->extractBits(64, I).getZExtValue());
+  if (SclTy->isDoubleTy())
+    return ConstantDataVector::getFP(SclTy, RawBits);
----------------
pengfei wrote:
> Can we use `if (!SclTy->isIntegerTy())` so that we can unify and put them in one loop?
ConstantDataVector get/getFP helpers use the width of the raw bits to help determine the type so we wouldn't be able to merge the loops.


================
Comment at: llvm/lib/Target/X86/X86FixupVectorConstants.cpp:273
+      case 32:
+        if (ConvertToBroadcast(0, 0, 0, Mem2Bcst->DstOp, 0, 0, OpNo))
+          return true;
----------------
pengfei wrote:
> Do we have chance to pass one more OpBcstN at the same time? Otherwise, we can simply pass `BitWidth` and a single `OpBcst`.
I'll see if I can refactor the loop so that we only have a single call to ConvertToBroadcast

We're going to need this layout of ConvertToBroadcast for followup patches which adds some of the basic load -> broadcast support from D150143, so I'd prefer not to change it unless you think it absolutely necessary?


================
Comment at: llvm/lib/Target/X86/X86InstrFoldTables.cpp:37
+  { X86::VANDNPSZ128rr,  X86::VANDNPSZ128rmb,  TB_BCAST_SS },
+  { X86::VANDNPSZ256rr,  X86::VANDNPSZ256rmb,  TB_BCAST_SS },
+  { X86::VANDPDZ128rr,   X86::VANDPDZ128rmb,   TB_BCAST_SD },
----------------
pengfei wrote:
> Missing `X86::VANDNPSZrr`
Good catch!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150526