[llvm] r203488 - For functions with ARM target specific calling convention, when simplify-libcall

[Evan Cheng]

On Mar 10, 2014, at 2:21 PM, Chandler Carruth <chandlerc at google.com> wrote:

> First off, thanks for the awesome description of the change. =]
> 
> On Mon, Mar 10, 2014 at 1:49 PM, Evan Cheng <evan.cheng at apple.com> wrote:
> I can think of 3 options to fix this.
> 
> 1. Make "C" calling convention just work since the target should know what CC
>    is being used.
> 
>    This doesn't work because each function can use different CC with the "pcs"
>    attribute.
> 
> 2. Have Clang add the right CC keyword on the calls to LLVM builtin.
> 
>    This will work but it doesn't match the LLVM IR specification which states
>    these are "Standard C Library Intrinsics".
> 
> 3. Fix simplify libcall so the resulting calls to the C routines will have the
>    proper CC keyword. e.g.
>    %__exp10 = call arm_aapcs_vfpcc double @__exp10(double %x) #1
> 
>    This works and is the solution I implemented here.
> 
> Both solutions #2 and #3 would work. After carefully considering the pros and
> cons, I decided to implement #3 for the following reasons.
> 
> 1. It doesn't change the "spec" of the intrinsics.
> 2. It's a self-contained fix.
> 
> There are a couple of potential downsides.
> 1. There could be other places in the optimizer that is broken in the same way
>    that's not addressed by this.
> 2. There could be other calling conventions that need to be propagated by
>    simplify-libcall that's not handled.
> 
> But for now, this is the fix that I'm most comfortable with.
> 
> Agreed.
> 
> Here is a suggested option #4 which only makes sense in the long term, but might be more palatable than #1 in the long term.
> 
> We could require that the target provide a libcall description for all library functions which have an IR intrinsic form. These could look not entirely dissimilar from the existing hooks for a target to inform IR passes about available library calls on a specific platform, and could include such details as what calling conventions should be used. Then this code (and any other optimizer code) could use these target-provided libcall descriptions to write correct calls into the library. This would also give us a nice way of modeling freestanding implementations where there *is* no libc function of note.
> 
> Other than this, I like option #2 the best. Clang already knows how to call the libc function correctly; having it encode this in the call to the intrinsic seems natural and easy. It is a spec change, but it seems a good spec change to me at least.

I should correct my description. Actually both #1 and #3 are implemented. 

Currently isel nows the default C calling convention maps to aapcs or aapcs-vfp. So this specific problem would only cause a misompilation if simplify-libcall changed a call in a function that has the "apcs" attribute which changed the default calling convention. Given that, I would argue this patch is the right way to go since it fixes the issue where "apcs" attribute is used.

> 
> 
> Maybe file a PR to revisit this and go after one of these solutions?

Sure.

Evan

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20140310/16e7d5bc/attachment.html>