[PATCH] D35498: [LoopVectorizer] Use two step casting for float to pointer type.

[Ayal Zaks via Phabricator via llvm-commits]

Ayal added a comment.

In https://reviews.llvm.org/D35498#813120, @rengolin wrote:

> ... Or maybe we just load them as "data" (i32/i64?) and then bitcast safely?
>
> Makes sense?


It does to me. The wide load and wide store are simply trying to move the packed bits together; each separate shuffle has its specific (possibly distinct) type.

(A follow-up issue may rise when attempting to interleave loads/stores of different sizes together; we're not there yet.)

A few additional comments below, mostly for completeness. They can be addressed by follow-up patches, if fixing the PR case (only) first is preferred.



================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:2922
           VectorType *OtherVTy = VectorType::get(Member->getType(), VF);
           StridedVec = Builder.CreateBitOrPointerCast(StridedVec, OtherVTy);
         }
----------------
Same floating-point-vs-pointer type casting issue may hold for interleaved loads here as well, right?


================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:2951
 
       // If this member has different type, cast it to an unified type.
+      if (StoredVec->getType() != SubVT) {
----------------
While you're at it, please correct this typo: "... cast it to a[n] unified type". Can continue and comment here what this unified type may be.


================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:2956
+          // StoredVec cannot be directly casted to SubVT type.
+          // May happen when StoredVec is Floating point vector but SubVT is a
+          // pointer type.
----------------
Or the other way around, when the last appearing store marking the insertion position of the final wide store has SubVT type of floating point, and another member has StoredVec type of a pointer. E.g., when the fields are swapped within the struct.


================
Comment at: lib/Transforms/Vectorize/LoopVectorize.cpp:2967
+            // Casting is not possible.
+            return;
+          }
----------------
At this stage assert that this 'else' is not reached, rather than return silently w/o generating the final wide store.

Suggest to add a condition to Legal checking that types are compatible when forming interleave-groups.


https://reviews.llvm.org/D35498