[PATCH] D43674: [TargetLowering] Add vector BITCAST support to SimplifyDemandedVectorElts

[Sanjay Patel via Phabricator via llvm-commits]

spatel added inline comments.


================
Comment at: lib/CodeGen/SelectionDAG/TargetLowering.cpp:1359
+    APInt SrcZero, SrcUndef;
+    APInt SrcElts = APInt::getNullValue(NumSrcElts);
+
----------------
rename to SrcDemandedElts?


================
Comment at: lib/CodeGen/SelectionDAG/TargetLowering.cpp:1361-1362
+
+    // Bitcast 'small element' vector from 'large element' vector, we must
+    // demand a source element if any DemandedElt maps to it.
+    if ((NumElts % NumSrcElts) == 0) {
----------------
I think this would read more clearly as:
"Bitcast large element source vector to small element vector."
because that matches how it would be written in LLVM:
bitcast <4 x i32> to <16 x i8>
and:
"We demand any larger source element if any smaller demanded element of this vector maps to it."


================
Comment at: lib/CodeGen/SelectionDAG/TargetLowering.cpp:1385-1386
+
+    // Bitcast 'large element' vector from 'small element' vector, demand all
+    // source elements that a DemandedElt covers.
+    if ((NumSrcElts % NumElts) == 0) {
----------------
Similar to above:
"Bitcast small element source vector to large element vector (eg, bitcast <16 x i8> to <4 x i32>). We demand all smaller source elements covered by the larger demanded element of this vector."


================
Comment at: lib/CodeGen/SelectionDAG/TargetLowering.cpp:1398
+      // If all the src elements covering an output element are zero/undef, then
+      // the output element will be as well, assuming if was demanded.
+      for (unsigned i = 0; i != NumElts; ++i) {
----------------
if -> it


================
Comment at: test/CodeGen/X86/vector-shuffle-256-v16.ll:4606-4609
 ; AVX1-NEXT:    vmovd {{.*#+}} xmm0 = mem[0],zero,zero,zero
-; AVX1-NEXT:    vpshufb {{.*#+}} xmm0 = xmm0[0,1,0,1,0,1,0,1,0,1,0,1,0,1,0,1]
+; AVX1-NEXT:    vpshuflw {{.*#+}} xmm0 = xmm0[0,0,2,3,4,5,6,7]
+; AVX1-NEXT:    vpshufd {{.*#+}} xmm0 = xmm0[0,0,0,0]
 ; AVX1-NEXT:    vinsertf128 $1, %xmm0, %ymm0, %ymm0
----------------
I haven't looked at this in a while, so not sure what's happening here, but this should just be:
vbroadcastss	(%rdi), %ymm0 ?


Repository:
  rL LLVM

https://reviews.llvm.org/D43674