[PATCH] D22840: [InstCombine] Split int64 store into separate int32 stores

[Chandler Carruth via llvm-commits]

On Wed, Jul 27, 2016 at 12:15 AM Xinliang David Li <davidxl at google.com>
wrote:

> On Tue, Jul 26, 2016 at 11:27 PM, Chandler Carruth <chandlerc at google.com>
> wrote:
>
>> On Tue, Jul 26, 2016 at 11:19 PM Xinliang David Li <davidxl at google.com>
>> wrote:
>>
>>> On Tue, Jul 26, 2016 at 11:11 PM, Chandler Carruth <chandlerc at google.com
>>> > wrote:
>>>
>>>> On Tue, Jul 26, 2016 at 11:08 PM Xinliang David Li via llvm-commits <
>>>> llvm-commits at lists.llvm.org> wrote:
>>>>
>>>>> Hmm, why does Clang change the return type from from struct to i64?
>>>>> Set aside whether it is always beneficial to do it or not, it certainly
>>>>> feels like the wrong place to make such a change.
>>>>>
>>>>
>>>> It's necessary to get the right ABI out of LLVM.
>>>>
>>>>
>>> what ABI issue and what can be wrong without using i64? I might have
>>> missed something obvious here.
>>>
>>
>> The ABI specifies that a struct such as this is returned in a single
>> 64-bit integer register.
>>
>
> yes, at least for system V ABI for x86_64.
>
>
>>
>> Consider this C++ test case:
>> long long a();
>>
>> struct S { int i; float f; };
>> S b();
>>
>> void f(long long *ptr1, S *ptr2) {
>>   *ptr1 = a();
>>   *ptr2 = b();
>> }
>>
>> It compiles to:
>>         callq   _Z1av
>>         movq    %rax, (%rbx)
>>         callq   _Z1bv
>>         movq    %rax, (%r14)
>>
>> Both need to be in the single register.
>>
>> But if you consider the following IR:
>> declare i64 @a()
>> declare {i32, float} @b()
>>
>> define void @f(i64* %ptr1, {i32, float}* %ptr2) {
>> entry:
>>   %calla = call i64 @a()
>>   store i64 %calla, i64* %ptr1
>>   %callb = call {i32, float} @b()
>>   store {i32, float} %callb, {i32, float}* %ptr2
>>   ret void
>> }
>>
>> The LLVM lowering of the call to @b isn't what you want:
>>         callq   a
>>         movq    %rax, (%r14)
>>         callq   b
>>         movl    %eax, (%rbx)
>>         movss   %xmm0, 4(%rbx)
>>
>> In order to get the ABI's semantics, we have to map the type into
>> something that will actually get lowered in that exact manner.
>>
>
> Still, the FE does not seem to be the right place to fix the bug
> (considering multiple different FEs targeting LLVM). It seems to me this
> bug is either due to the call return lowering either happens too early or
> needs to be target dependent.
>

I'm confused... What do you mean by "fix the bug"?

The performance bug? I don't think anyone has suggested the frontend -- the
advice was to fix the performance issue *later* in the pipeline.

And I don't see a bug with the ABI lowering though, so I don't understand
what you would mean by "fix the bug" in that context...
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20160727/8b515acb/attachment.html>