maxjhandsome
/
llvm-project
forked from OSchip/llvm-project
1
0
Fork 0
Code Issues Pull Requests Packages Releases Wiki Activity

Avoid inlining if there is byval arguments with non-alloca address space 

Summary:
After teaching InlineCost more about address spaces ()
another fault was detected in the inliner. If an argument has
the byval attribute the parameter might be copied to an alloca.
That part seems to work fine even if the argument has a different
address space than the alloca address space. However, if the
address spaces differ, then the inlined function still might
refer to the parameter using the original address space (the
inliner does not handle that situation very well).

This patch avoids the problem by simply disallowing inlining
when there are byval arguments with address space that differs
from the alloca address space.

I'm not really sure how to transform the code if we want to
get inlining for this situation. I assume that it never has
been working, and that the fixes in r321809 just exposed an
old problem.

Fault found by skatkov (Serguei Katkov). It is mentioned in
follow up comments to https://reviews.llvm.org/D40455.

Reviewers: skatkov

Reviewed By: skatkov

Subscribers: uabelho, eraman, llvm-commits, haicheng

Differential Revision: https://reviews.llvm.org/D41898

llvm-svn: 322181
This commit is contained in:
Bjorn Pettersson 2018-01-10 13:01:18 +00:00
parent 6ed7f00e49
commit 3851496e6e
2 changed files with 24 additions and 2 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

13
llvm/lib/Analysis/InlineCost.cpp
View File

@ -1960,6 +1960,19 @@ InlineCost llvm::getInlineCost(
if (!Callee)
return llvm::InlineCost::getNever();
// Never inline calls with byval arguments that does not have the alloca
// address space. Since byval arguments can be replaced with a copy to an
// alloca, the inlined code would need to be adjusted to handle that the
// argument is in the alloca address space (so it is a little bit complicated
// to solve).
unsigned AllocaAS = Callee->getParent()->getDataLayout().getAllocaAddrSpace();
for (unsigned I = 0, E = CS.arg_size(); I != E; ++I)
if (CS.isByValArgument(I)) {
PointerType *PTy = cast<PointerType>(CS.getArgument(I)->getType());
if (PTy->getAddressSpace() != AllocaAS)
return llvm::InlineCost::getNever();
}
// Calls to functions with always-inline attributes should be inlined
// whenever possible.
if (CS.hasFnAttr(Attribute::AlwaysInline)) {

13
llvm/test/Transforms/Inline/byval.ll
View File

@ -1,6 +1,11 @@
; RUN: opt < %s -inline -S | FileCheck %s
; RUN: opt < %s -passes='cgscc(inline)' -S | FileCheck %s
; The verifier does catch problems with inlining of byval arguments that has a
; different address space compared to the alloca. But running instcombine
; after inline used to trigger asserts unless we disallow such inlining.
; RUN: opt < %s -inline -instcombine -disable-output 2>/dev/null
target datalayout = "p:32:32-p1:64:64-p2:16:16-n16:32:64"
; Inlining a byval struct should cause an explicit copy into an alloca.
@ -131,6 +136,11 @@ entry:
; CHECK-NOT: load i32, i32* getelementptr inbounds (%struct.S0, %struct.S0* @b, i64 0, i32 0), align 4
}
; Inlining a byval struct that is in a different address space compared to the
; alloca address space is at the moment not expected. That would need
; adjustments inside the inlined function since the address space attribute of
; the inlined argument changes.
%struct.S1 = type { i32 }
@d = addrspace(1) global %struct.S1 { i32 1 }, align 4
@ -151,6 +161,5 @@ entry:
%0 = load i32, i32 addrspace(1)* @c, align 4
ret i32 %0
; CHECK: @test5_as1()
; CHECK: store i32 0, i32 addrspace(1)* getelementptr inbounds (%struct.S1, %struct.S1 addrspace(1)* @d, i64 0, i32 0), align 4
; CHECK-NOT: load i32, i32 addrspace(1)* getelementptr inbounds (%struct.S1, %struct.S1 addrspace(1)* @d, i64 0, i32 0), align 4
; CHECK: call void @f5_as1
}