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

[Pan, Wei via llvm-dev]

Hi Mikael,

This patch to stop inlining may hurt performance badly. My context is to support OpenCL generic address space. Inlining is crucial to bring generic address space to concrete address spaces during compilation.  I also do not have a proper fix to inliner. Adding an address space cast would fix the crash, however it will use loads from address space one to read a memory location from address space zero. 

Thanks,
Wei


-----Original Message-----
From: Mikael Holmén [mailto:mikael.holmen at ericsson.com] 
Sent: Wednesday, September 26, 2018 2:14 AM
To: Pan, Wei <wei.pan at intel.com>; Friedman, Eli <efriedma at codeaurora.org>; Reid Kleckner <rnk at google.com>
Cc: llvm-dev <llvm-dev at lists.llvm.org>; Björn Pettersson A <bjorn.a.pettersson at ericsson.com>
Subject: Re: [llvm-dev] byval argument causes llvm to crash after inlining

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
>