[PATCH] Teach instcombine to canonicalize "element extraction" from a load of an integer and "element insertion" into a store of an integer into actual element extraction, element insertion, and vector loads and stores.
hfinkel at anl.gov
hfinkel at anl.gov
Mon Dec 8 00:26:01 PST 2014
LGTM.
This will obviously lead to the formation of more vector types, many/most of which will be scalarized. We'll need to be careful re: code quality here, as the default expansion of extract_vector_elt, for example, stores the vector to the stack and loads one element (and will do this separately for each extraction). Nevertheless, this does seem to make sense as a canonical form.
It seems like the two primary types affected by this change will be floating-point types and pointers. You have tests with fp types, but none with pointers. Can you please add a test case with pointers? [The underlying logic is obviously the same, but we should have regression tests covering this basic facet of our canonical form]
================
Comment at: lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp:390
@@ +389,3 @@
+
+ // Check that the element type matches.
+ if (!ElementTy) {
----------------
You have a more-explanatory comment for the store version, perhaps you could copy that here, "All of the elements extracted need to be the same type...."
================
Comment at: lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp:395
@@ +394,3 @@
+ if (!ElementTy->isSingleValueType() || ElementTy->isIntegerTy() ||
+ ElementTy->isVectorTy())
+ return false;
----------------
So you will catch pointer types here and form vectors of pointers; this is likely worth a comment somewhere.
http://reviews.llvm.org/D6548
More information about the llvm-commits
mailing list