[PATCH] D34311: [InstCombine] Don't replace allocas with globals if we can't prove that it's large enough.

Eli Friedman via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Jun 19 12:44:16 PDT 2017


efriedma added a reviewer: efriedma.
efriedma added a comment.

I wonder if it makes sense to more aggressively transform the IR in certain cases rather than bail out, since we can prove that the IR is reading uninitialized memory.  Something for a future patch, I guess, if someone has a testcase where it matters in practice.



================
Comment at: lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp:169
+  if (!ConstArraySize)
+    return false;
+
----------------
Messing with getArraySize() is unnecessary; just return false if isArrayAllocation() is true.


================
Comment at: lib/Transforms/InstCombine/InstCombineLoadStoreAlloca.cpp:193
 static MemTransferInst *
 isOnlyCopiedFromConstantGlobal(AllocaInst *AI,
                                SmallVectorImpl<Instruction *> &ToDelete) {
----------------
If you're going to change the functionality here, you also need to change the name.


================
Comment at: test/Transforms/InstCombine/memcpy-from-global.ll:211
+
+define void @test9_small_global() {
+; CHECK-LABEL: @test9_small_global(
----------------
Testcase needs comment explaining what it's checking for.


https://reviews.llvm.org/D34311





More information about the llvm-commits mailing list