[PATCH] D41898: Avoid inlining if there is byval arguments with non-alloca address space

[Bjorn Pettersson via Phabricator via llvm-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL322181: Avoid inlining if there is byval arguments with non-alloca address space (authored by bjope, committed by ).

Repository:
  rL LLVM

https://reviews.llvm.org/D41898

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


Index: llvm/trunk/lib/Analysis/InlineCost.cpp
===================================================================
--- llvm/trunk/lib/Analysis/InlineCost.cpp
+++ llvm/trunk/lib/Analysis/InlineCost.cpp
@@ -1960,6 +1960,19 @@
   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)) {
Index: llvm/trunk/test/Transforms/Inline/byval.ll
===================================================================
--- llvm/trunk/test/Transforms/Inline/byval.ll
+++ llvm/trunk/test/Transforms/Inline/byval.ll
@@ -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 @@
 ; 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 @@
 	%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
 }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D41898.129249.patch
Type: text/x-patch
Size: 2743 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20180110/7d14ca25/attachment.bin>