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

[Phoebe Wang via Phabricator via llvm-commits]

pengfei added a comment.

Will it increase compile time much?



================
Comment at: llvm/lib/Target/X86/X86.h:67
 
+/// Return aspass that reduces the size of vector constant pool loads.
+FunctionPass *createX86FixupVectorConstants();
----------------
a pass


================
Comment at: llvm/lib/Target/X86/X86FixupVectorConstants.cpp:104
+    bool IsInteger = CDS->getElementType()->isIntegerTy();
+    bool IsFloat = CDS->getElementType()->isHalfTy() ||
+                   CDS->getElementType()->isFloatTy() ||
----------------
Should check `isBFloatTy` too?


================
Comment at: llvm/lib/Target/X86/X86FixupVectorConstants.cpp:200
+      RawBits.push_back(Splat->extractBits(16, I).getZExtValue());
+    if (SclTy->isHalfTy() || SclTy->isBFloatTy())
+      return ConstantDataVector::getFP(SclTy, RawBits);
----------------
SclTy->is16bitFPTy()


================
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);
----------------
Can we use `if (!SclTy->isIntegerTy())` so that we can unify and put them in one loop?


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


================
Comment at: llvm/lib/Target/X86/X86FixupVectorConstants.cpp:295
+  for (MachineBasicBlock &MBB : MF) {
+    for (MachineBasicBlock::iterator I = MBB.begin(); I != MBB.end(); ++I) {
+      if (processInstruction(MF, MBB, I)) {
----------------
Why not `MachineInstr &MI : MBB`?


================
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 },
----------------
Missing `X86::VANDNPSZrr`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150526