byval argument causes llvm to crash after inlining

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

}

Well, they are from address space zero, just like all allocas and other stack memory, right?

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

Probably, yes: lowering a byval parameter requires allocating space on the stack, so it has to be in the same address space.

-Eli

This sounds right to me. If there is no objection, I will implement a patch to enforce this in langref and IR verifier.

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

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