[llvm-commits] [llvm] r37940 - in /llvm/trunk: include/llvm/CodeGen/SelectionDAGNodes.h include/llvm/ParameterAttributes.h lib/AsmParser/Lexer.l lib/AsmParser/llvmAsmParser.y lib/CodeGen/SelectionDAG/SelectionDAGISel.cpp lib/Target/TargetCallingConv.td lib/Target/X86/X86CallingConv.td lib/VMCore/Function.cpp lib/VMCore/Verifier.cpp utils/TableGen/CallingConvEmitter.cpp

[Chris Lattner]

>> Okay, so now I'm really confused. :)
>>
>> The Verifier is checking to make sure that the argument type to which
>> ByVal is applied is a pointer. It doesn't check any further so I  
>> assume
>> a pointer to anything is valid. So, consider the following:
>>
>> 1. i32*              ; i32 is passed?
>> 2. i1024*            ; We pass a 128-byte long integer?
>> 3. f64*              ; We pass a double?
>> 4. {i32,i32,i32}*    ; We pass 3 i32 integers?
>> 5. <4xi32>*          ; We pass 4 i32 integers?
>> 6. [1024xi32>*       ; We pass 1024 i32 integers?
>>
>>> From an assembly language point of view, this seems redundant with
>> existing capabilities as well as violating one of the tenets of the
>> language (can only pass first class types).
>>
>> In cases 1, 3, 4 and 5 the front end could have just done the
>> getelementptr and load(s) necessary. For cases 2 and 6, the implied
>> loads are a performance issue so it would be better to pass the  
>> pointer.
>> Also, in all these cases, the implied signature of the function is
>> different. How does this play with calling conventions?
>>
>> Am I missing something here? Because if I'm not then I'm -1 on this
>> feature as a whole. What's the objective of this?
>
> first let me point out that much of what you say also applies to
> StructReturn, which probably can also be used for returning arrays,
> ints etc.

Yes.

> And like StructReturn this is all about the ABI: my
> understanding is that on some platforms the ABI allows for passing
> (small) structs by value.

Yes.

> We need to be able to support this, but
> how?  The idea is to treat such parameters as passed by reference,
> and add an attribute to tell the code generators to in fact pass them
> in whatever way the ABI says they should be.  See PR1521.

Yep.

> Thus in the IR the function is declared to take a pointer to the
> struct as an argument, and the ByVal marker is placed for the
> benefit of the code generators.  However it is not clear to me
> that this is semantically correct.  Doesn't ByVal mean what it
> says: a copy of the struct is made, and if this copy is modified
> inside the called function then this does not change the original
> struct?  But if in the IR we pass in a pointer to the struct, then
> any writes to the struct will modify the original, which is wrong.

Yes, this is an issue.  Transformations that care will need to be  
updated to be aware of byval.  For example, the inliner should insert  
a copy unless it can prove that the callee only reads the struct  
passed in.

> Also, I agree that there needs to be more clarity about what is
> allowed to be passed ByVal and returned by SRet.
>
> Finally, if StructReturn can also return arrays, then I don't mind
> to much if ByVal is called StructByVal or something like that.

I think it should be fully general.  The semantics of LLVM are  
independent of C.  If another language wants to pass a [2 x i8] by  
value, go for it.  Whether a target supports this or not is a  
different question though :).

-Chris