[PATCH] D22710: Add option to CodeExtractor for hoisting static allocas with uses contained within extracted region

[Davide Italiano via Phabricator via llvm-commits]

davide added a comment.

Some comments online. Also please clang-format this patch.



================
Comment at: lib/Transforms/IPO/PartialInlining.cpp:138-139
   Function *ExtractedFunction =
-      CodeExtractor(ToExtract, &DT).extractCodeRegion();
+      CodeExtractor(ToExtract, &DT, /*AggregateArgs*/false,
+                    /*ExtractContainedStaticAllocas*/true).extractCodeRegion();
 
----------------
is this clang-formatted?


================
Comment at: lib/Transforms/Utils/CodeExtractor.cpp:176-177
+      continue;
+    if (!Blocks.count(User->getParent()))
+      return true;
+    if (!I->getType()->isPointerTy())
----------------
this needs a comment.


================
Comment at: lib/Transforms/Utils/CodeExtractor.cpp:180-187
+    // Check to see if there is a gep of this instruction that geps this
+    // instruction.
+    if (GetElementPtrInst *GEP = dyn_cast<GetElementPtrInst>(User)) {
+      if (GEP->getPointerOperand() != I)
+        continue;
+      if (hasOutsideUses(Blocks, GEP))
+        return true;
----------------
Can you add a test exercising this behaviour? 


================
Comment at: lib/Transforms/Utils/CodeExtractor.cpp:189-193
+    else if (BitCastInst *BC = dyn_cast<BitCastInst>(User)) {
+      if (hasOutsideUses(Blocks, BC))
+        return true;
+    }
+  }
----------------
same.


https://reviews.llvm.org/D22710