[llvm-dev] SRET consistency between declaration and call site

[James Y Knight via llvm-dev]

On Oct 8, 2015, at 11:49 AM, Reid Kleckner via llvm-dev <llvm-dev at lists.llvm.org> wrote:
> On Thu, Oct 8, 2015 at 7:15 AM, Tim Northover via llvm-dev <llvm-dev at lists.llvm.org <mailto:llvm-dev at lists.llvm.org>> wrote:
> On 8 October 2015 at 03:31, Joerg Sonnenberger <joerg at britannica.bec.de <mailto:joerg at britannica.bec.de>> wrote:
> > I think we are talking about two different things here. What I am
> > advocating is that callsite and function signature must have matching
> > ABI. I think we already enforce that for things like argument type, so
> > enforcing sret doesn't seem to be any different. I am not saying that
> > you should not be able to override it via manual bitcasting, but that's
> > a completely different topic. If you do that, you are on your own.
> 
> Unfortunately the two aren't separable because the sret isn't part of
> the function's type. You can't even write down a bitcast that adds or
> removes it:
> 
> $ cat simple.ll
> define void @foo(void(i8*)* %fn) {
>   %tst = bitcast void(i8*)* %fn to void(i8* sret)*
>   ret void
> }
> $ opt -verify simple.ll -S
> opt: simple.ll:2:41: error: argument attributes invalid in function type
>   %tst = bitcast void(i8*)* %fn to void(i8* sret)*
> 
> So if you ban mismatches at call sites, you're banning the casts too.
> 
> I agree. Maybe a different analogy based on an FAQ[1] will work. sret is similar to the function calling convention. The function calling convention is, for reasons that I don't agree with, not part of the function type. Therefore the verifier accepts direct calls to functions with mismatched calling conventions, because it's not a type mismatch. The optimization passes, however, will transform that call to unreachable.
> 
> I argue the same applies to sret. Because the verifier does not reject it, we cannot assert on a mismatch later. We have to either make sret part of the type system, so that mismatches cannot occur without a cast, or we have to silently accept it and generate code that's probably incorrect.

Yuck. Yes, that's a very compelling argument. :) It really does seem that the attributes which are required to match should be part of the function type. Oh well -- removing the assert is definitely the way to go, then, in the current model.

(And fixing clang so it emits the proper declare would be nice too, but I guess it doesn't really matter, since that isn't actually used for anything.)

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20151013/6f4bdefc/attachment.html>