[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