[llvm-dev] byval argument causes llvm to crash after inlining

[Mikael Holmén via llvm-dev]

Hi,

The normal inliner avoids inlining in cases like this, see the fix in 
r322181.

Now I see that you lean towards making some use of byval illegal, but I 
suppose a similar change could be done somewhere in the AlwaysInliner as 
well?

What exactly is it that should be disallowed?

If we do make something illegal I suppose there needs to be fixes in 
other places too, since someone seems to produce such code?

Regards,
Mikael

On 09/26/2018 12:11 AM, Pan, Wei via llvm-dev wrote:
> This sounds right to me. If there is no objection, I will implement a patch to enforce this in langref and IR verifier.
> 
> -----Original Message-----
> From: Friedman, Eli [mailto:efriedma at codeaurora.org]
> Sent: Tuesday, September 25, 2018 3:07 PM
> To: Pan, Wei <wei.pan at intel.com>; Reid Kleckner <rnk at google.com>
> Cc: llvm-dev <llvm-dev at lists.llvm.org>
> Subject: Re: [llvm-dev] byval argument causes llvm to crash after inlining
> 
> Probably, yes: lowering a byval parameter requires allocating space on the stack, so it has to be in the same address space.
> 
> -Eli
> 
> On 9/25/2018 3:00 PM, Pan, Wei via llvm-dev wrote:
>> It is problematic when byval argument is not from address space 0.   When the default alloca address space is 0, should we consider this IR illegal?
>>
>> define internal i32 @bar(i32 addrspace(1)* byval %a) alwaysinline
>>
>>
>> From: Reid Kleckner [mailto:rnk at google.com]
>> Sent: Tuesday, September 25, 2018 2:38 PM
>> To: Pan, Wei <wei.pan at intel.com>
>> Cc: llvm-dev <llvm-dev at lists.llvm.org>
>> Subject: Re: [llvm-dev] byval argument causes llvm to crash after
>> inlining
>>
>> Well, they are from address space zero, just like all allocas and other stack memory, right?
>>
>> On Tue, Sep 25, 2018 at 1:45 PM Pan, Wei via llvm-dev <llvm-dev at lists.llvm.org> wrote:
>> Hello,
>>    
>> With the following reduced test case, cmd “opt -always-inline t.ll” crashes after inlining. Notice that byval argument %a will be remapped to %1 below, and consequently produces an illegal store.
>>    
>> %1 = alloca i32, align 4
>> store i32 * %1, i32 addrspace(1)** %a.addr, align 8
>>    
>> Looks like Inliner assumes that byval arguments are from address space 0. Or this is just a bug in inliner?
>>    
>> Thanks,
>> Wei
>>    
>> t.ll:
>>    
>> define i32 @foo(i32 addrspace(1)* %x) {
>> entry:
>>     %y = call i32 @bar(i32 addrspace(1)* %x)
>>     ret i32 %y
>> }
>>    
>> define internal i32 @bar(i32 addrspace(1)* byval %a) alwaysinline {
>>     %a.addr = alloca i32 addrspace(1)*, align 8
>>     store i32 addrspace(1)* %a, i32 addrspace(1)** %a.addr, align 8
>>     %a1 = load i32 addrspace(1)*  , i32 addrspace(1)** %a.addr, align 8
>>     %b = load i32, i32 addrspace(1)* %a1, align 4
>>     ret i32 %b
>> }
>>    
>> _______________________________________________
>> LLVM Developers mailing list
>> llvm-dev at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>> _______________________________________________
>> LLVM Developers mailing list
>> llvm-dev at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
> 
> 
> --
> Employee of Qualcomm Innovation Center, Inc.
> Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, a Linux Foundation Collaborative Project
> 
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>