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

[bjope]

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.

llvm/trunk/lib/Analysis/InlineCost.cpp

InlineCost llvm::getInlineCost(
     std::function<AssumptionCache &(Function &)> &GetAssumptionCache,
     Optional<function_ref<BlockFrequencyInfo &(Function &)>> GetBFI,
     ProfileSummaryInfo *PSI, OptimizationRemarkEmitter *ORE) {
 
   // Cannot inline indirect calls.
   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)) {
     if (isInlineViable(*Callee))
       return llvm::InlineCost::getAlways();
     return llvm::InlineCost::getNever();
   }
 

llvm/trunk/test/Transforms/Inline/byval.ll

 ; 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.
 
   %struct.ss = type { i32, i64 }
 @.str = internal constant [10 x i8] c"%d, %lld\0A\00"    ; <[10 x i8]*> [#uses=1]
 
 define internal void @f(%struct.ss* byval  %b) nounwind  {
entry:
   tail call void @f5(%struct.S0* byval align 4 @b)
   %0 = load i32, i32* @a, align 4
   ret i32 %0
 ; CHECK: @test5()
 ; CHECK: store i32 0, i32* getelementptr inbounds (%struct.S0, %struct.S0* @b, i64 0, i32 0), align 4
 ; 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
 @c = common addrspace(1) global i32 0, align 4
 
 define internal void @f5_as1(%struct.S1 addrspace(1)* byval nocapture readonly align 4 %p) {
 entry:
   store i32 0, i32 addrspace(1)* getelementptr inbounds (%struct.S1, %struct.S1 addrspace(1)* @d, i64 0, i32 0), align 4
   %f2 = getelementptr inbounds %struct.S1, %struct.S1 addrspace(1)* %p, i64 0, i32 0
   %0 = load i32, i32 addrspace(1)* %f2, align 4
   store i32 %0, i32 addrspace(1)* @c, align 4
   ret void
 }
 
 define i32 @test5_as1() {
 entry:
   tail call void @f5_as1(%struct.S1 addrspace(1)* byval align 4 @d)
   %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
 }