[PATCH] D20077: [Inliner] don't assume that a Constant alloca size is a ConstantInt (PR27277)

Mehdi AMINI via llvm-commits llvm-commits at lists.llvm.org
Mon May 9 12:54:43 PDT 2016


joker.eph added inline comments.

================
Comment at: lib/Analysis/InlineCost.cpp:335
@@ -337,1 +334,3 @@
+    if (ConstantInt *AllocSize = dyn_cast_or_null<ConstantInt>(
+            SimplifiedValues.lookup(I.getArraySize()))) {
       Type *Ty = I.getAllocatedType();
----------------
We're losing the assertion, that could be seen as somehow not totally useless: if the only special case is undef, maybe it could be spelled out explicitly?
(I'm not having a strong opinion either way)

================
Comment at: lib/Analysis/InlineCost.cpp:337
@@ -337,2 +336,3 @@
       Type *Ty = I.getAllocatedType();
+      // FIXME: This can't be right? AllocatedSize is in *bytes*.
       AllocatedSize += Ty->getPrimitiveSizeInBits() * AllocSize->getZExtValue();
----------------
Indeed...

My impression is that this is the kind of things that can only be reliably tested using a c++ unitest. I doubt that InlineCost has much unittest coverage though.


http://reviews.llvm.org/D20077





More information about the llvm-commits mailing list