[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