[PATCH] Update InstCombine to transform aggregate loads into scalar loads.

David Majnemer david.majnemer at gmail.com
Sat Apr 4 21:47:19 PDT 2015


================
Comment at: lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp:525
@@ +524,3 @@
+
+  if (StructType *ST = dyn_cast<StructType>(T)) {
+    // If the struct only have one element, we unpack.
----------------
We know the type from the right hand side, please change this to `auto *ST`.

================
Comment at: lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp:527
@@ +526,3 @@
+    // If the struct only have one element, we unpack.
+    if (ST->getNumElements() == 1) {
+      Ptr = IC.Builder->CreateStructGEP(Ptr, 0);
----------------
Is there any reason why you only go one level deep?  What if we peel the struct type away just to find another single element struct type?  Perhaps the InstCombine worklist will naturally get this... In any case, can you please have a test for this case?

================
Comment at: test/Transforms/InstCombine/unpack-fca.ll:33
@@ +32,3 @@
+
+define %A @loadA() {
+body:
----------------
Please use `CHECK-LABEL` with the IR function's name for each test.

http://reviews.llvm.org/D8339

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/






More information about the llvm-commits mailing list