[llvm-commits] [llvm] r122959 - in /llvm/trunk: lib/Target/README.txt lib/Transforms/InstCombine/InstCombineCalls.cpp test/Transforms/InstCombine/objsize.ll

[Benjamin Kramer]

On 07.01.2011, at 22:00, Frits van Bommel wrote:

> On Fri, Jan 7, 2011 at 9:49 PM, Benjamin Kramer
> <benny.kra at googlemail.com> wrote:
>> On 07.01.2011, at 21:00, Bob Wilson wrote:
>>> On Jan 6, 2011, at 5:11 AM, Benjamin Kramer wrote:
>>> Before this change, codegenprepare folded the strcpy_chk to strcpy because it treated the malloc size as unknown.  Now, it knows the malloc size but not the source size, so it's keeping the strcpy_chk.  The ds() function is inlined to lots of places, so the effect is greatly magnified.  In this case, it ought to be possible to recognize that the source is the same size as the destination and have instcombine fold to strcpy.  Could you look into doing that, or at least add a comment to the README file about it?
>> 
>> I reverted the change for now, there is another issue with this as it changes the semantics of __builtin_objectsize.
>> Unless we want to extend this GCC extension even more my patch is not safe in it's current form :(
> 
> The "changes the semantics" issue should be easily avoidable by adding
> an isa<Constant>() check. That would allow it to work for simpler
> cases like 'malloc(sizeof(Struct))'.

That worked without this patch and still does its job now :)