[PATCH] D21920: Fix for Bug 26903 adds support to inline __builtin_mempcpy

[Sunita_Marathe via llvm-commits]

Sunita_Marathe added a comment.

Thank you for raising the good points.

Clang could do the replacement, just as it does for memcpy.  
The replacement would have to be done conditionally, only in the absence of options such as -ffreestanding.  Because  if -ffreestanding   is specified then the compiler must lower the call as a call to mempcpy, not to memcpy.  The current approach works correctly for this case.

But the reason I chose to go the special function way is that I thought that it might be considered desirable to distinguish memcpy from mempcpy in the IR due to the fact that the memcpy intrinsic is the canonical form to represent copies. In addition to explicit memcpy calls it can also result from idiom recognition etc. and there might be passes which depend on the exact semantics of memcpy including return value etc. So it might be useful to keep the two distinct.
I am not at all sure how compelling this reason is.

The tail call point is a good one:
While we cannot do a tail call for  "return mempcpy(...)" , a tail call could be supported when the call is in a tail call position (but without "return").  I had considered checking whether the return value of the mempcpy is used or not in order to determine a value for the isTailCall argument to getMemcpy() .  But I dropped the idea because generally if  mempcpy is used instead of memcpy then very likely the user will also be interested in the return value.

Dead code removal when return value is not used:
This actually is something that does happen even with the current approach. If there are no users of the ADD dag node then it gets discarded.

Thanks
Sunita


http://reviews.llvm.org/D21920