[PATCH] D150143: [X86] Add X86FixupVectorConstantsPass to fold vectors constant loads as broadcasts (WIP)

[Phoebe Wang via Phabricator via llvm-commits]

pengfei added inline comments.


================
Comment at: llvm/lib/Target/X86/X86FixupVectorConstants.cpp:208
+  case X86::VMOVDQUrm:
+    if (ST->hasInt256())
+      return ConvertToBroadcastInt(X86::VPBROADCASTQrm, X86::VPBROADCASTDrm,
----------------
What's the reason to check `hasInt256` rather than `hasAVX2`?


================
Comment at: llvm/lib/Target/X86/X86FixupVectorConstants.cpp:211
+                                   X86::VPBROADCASTWrm, X86::VPBROADCASTBrm);
+    return false; // TODO: Add AVX VMOVDDUP/VBROADCASTSS.
+  case X86::VMOVDQAYrm:
----------------
Should avoid FP instruction for integer?


================
Comment at: llvm/lib/Target/X86/X86FixupVectorConstants.cpp:227
+  case X86::VMOVUPSrm:
+    return ConvertToBroadcastFP(X86::VMOVDDUPrm, X86::VBROADCASTSSrm);
+  case X86::VMOVAPDYrm:
----------------
Why don't use `VBROADCASTSD` here?


================
Comment at: llvm/test/CodeGen/X86/combine-concatvectors.ll:67-68
 ; AVX2-NEXT:    vmovups %ymm0, 48296(%rax)
-; AVX2-NEXT:    vmovlps %xmm0, 47372(%rax)
+; AVX2-NEXT:    vmovsd {{.*#+}} xmm0 = mem[0],zero
+; AVX2-NEXT:    vmovsd %xmm0, 47372(%rax)
 ; AVX2-NEXT:    vzeroupper
----------------
Regression?


================
Comment at: llvm/test/CodeGen/X86/horizontal-reduce-umin.ll:1170-1171
 ; X86-AVX2-NEXT:    vextractf128 $1, %ymm0, %xmm1
+; X86-AVX2-NEXT:    vmovddup {{.*#+}} xmm2 = [9223372036854775808,9223372036854775808]
+; X86-AVX2-NEXT:    ## xmm2 = mem[0,0]
 ; X86-AVX2-NEXT:    vxorpd %xmm2, %xmm0, %xmm3
----------------
This looks like regression too.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D150143