[llvm] r322181 - Avoid inlining if there is byval arguments with non-alloca address space

[Bjorn Pettersson via llvm-commits]

Author: bjope
Date: Wed Jan 10 05:01:18 2018
New Revision: 322181

URL: http://llvm.org/viewvc/llvm-project?rev=322181&view=rev
Log:
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

Modified:
    llvm/trunk/lib/Analysis/InlineCost.cpp
    llvm/trunk/test/Transforms/Inline/byval.ll

Modified: llvm/trunk/lib/Analysis/InlineCost.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Analysis/InlineCost.cpp?rev=322181&r1=322180&r2=322181&view=diff
==============================================================================
--- llvm/trunk/lib/Analysis/InlineCost.cpp (original)
+++ llvm/trunk/lib/Analysis/InlineCost.cpp Wed Jan 10 05:01:18 2018
@@ -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)) {

Modified: llvm/trunk/test/Transforms/Inline/byval.ll
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Transforms/Inline/byval.ll?rev=322181&r1=322180&r2=322181&view=diff
==============================================================================
--- llvm/trunk/test/Transforms/Inline/byval.ll (original)
+++ llvm/trunk/test/Transforms/Inline/byval.ll Wed Jan 10 05:01:18 2018
@@ -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
 }