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

[Chris Lattner]

On Jan 7, 2011, at 12:49 PM, 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 :(

What exactly is the issue here?  While I'm sympathetic to the "cdecl.c/ds" regression, I still think that your patch is a good route forward.  Optimizing to "know" the strlen size seems like a nice second step.

However, if your patch wasn't safe, then it clearly isn't good.  What is the issue?

-Chris