CodeGen: Insert addr space cast for automatic/temp var at right position

[yaxunl]

Summary

The uses of alloca may be in different blocks other than the block containing the alloca.
Therefore if the alloca addr space is non-zero and it needs to be casted to default
address space, the cast needs to be inserted in the same BB as the alloca insted of
the current builder insert point since the current insert point may be in a different BB.

[rjmccall]

Hmm. It doesn't seem unreasonable for this to require an insertion point as a precondition. In what situation do we emit addrspace casts after a return?

[yaxunl]

In D35438#810234, @rjmccall wrote:

Hmm. It doesn't seem unreasonable for this to require an insertion point as a precondition. In what situation do we emit addrspace casts after a return?

When the alloca address space is not zero and we declare an automatic variable after return. Since we need to cast the automatic variable to default address space, we need to have a new basic block.

[rjmccall]

Oh, of course. Sadly, in C/C++ the declaration point of a variable does not necessarily dominate all uses of the variable, so you need to emit the cast immediately after the alloca. Just temporarily move CGF.Builder to that point; hopefully we can assume that this function will never try to add control flow.

Test case:

extern void use(int*);
void foo() {
  goto later;
  int x;
  later:
  use(&x);
}

[yaxunl]

In D35438#810385, @rjmccall wrote:

Oh, of course. Sadly, in C/C++ the declaration point of a variable does not necessarily dominate all uses of the variable, so you need to emit the cast immediately after the alloca. Just temporarily move CGF.Builder to that point; hopefully we can assume that this function will never try to add control flow.

Test case:

extern void use(int*);
void foo() {
  goto later;
  int x;
  later:
  use(&x);
}

Right. performAddressSpaceCast could be used in general cases. We only need to change insert position when emitting automatic or temporary variables. I will move this logic to CodeGenFunction::CreateTempAlloca.

[yaxunl]

Insert addr space cast in the same basic block as alloca.

[rjmccall]

lib/CodeGen/CGExpr.cpp

76 ↗

(On Diff #106879)

IRBuilder already has saveIP() and restoreIP() methods for this purpose that deal correctly with the current insertion point being null.

[yaxunl]

lib/CodeGen/CGExpr.cpp

76 ↗

(On Diff #106879)

Thanks. I will use those.

[yaxunl]

Use saveIP/restoreIP.

[rjmccall]

LGTM.

cfe/trunk/lib/CodeGen/CGExpr.cpp

Address CodeGenFunction::CreateTempAlloca(llvm::Type *Ty, CharUnits Align,
   Alloca->setAlignment(Align.getQuantity());
   llvm::Value *V = Alloca;
   // Alloca always returns a pointer in alloca address space, which may
   // be different from the type defined by the language. For example,
   // in C++ the auto variables are in the default address space. Therefore
   // cast alloca to the default address space when necessary.
   if (CastToDefaultAddrSpace && getASTAllocaAddressSpace() != LangAS::Default) {
     auto DestAddrSpace = getContext().getTargetAddressSpace(LangAS::Default);
+    auto CurIP = Builder.saveIP();
+    Builder.SetInsertPoint(AllocaInsertPt);
     V = getTargetHooks().performAddrSpaceCast(
         *this, V, getASTAllocaAddressSpace(), LangAS::Default,
         Ty->getPointerTo(DestAddrSpace), /*non-null*/ true);
+    Builder.restoreIP(CurIP);
   }
 
   return Address(V, Align);
 }
 
 /// CreateTempAlloca - This creates an alloca and inserts it into the entry
 /// block if \p ArraySize is nullptr, otherwise inserts it at the current
 /// insertion point of the builder.

cfe/trunk/test/CodeGenCXX/amdgcn-automatic-variable.cpp

 // RUN: %clang_cc1 -O0 -triple amdgcn---amdgiz -emit-llvm %s -o - | FileCheck %s
 
 // CHECK-LABEL: define void @_Z5func1Pi(i32* %x)
 void func1(int *x) {
   // CHECK: %[[x_addr:.*]] = alloca i32*{{.*}}addrspace(5)
   // CHECK: %[[r0:.*]] = addrspacecast i32* addrspace(5)* %[[x_addr]] to i32**
   // CHECK: store i32* %x, i32** %[[r0]]
   // CHECK: %[[r1:.*]] = load i32*, i32** %[[r0]]
   // CHECK: store i32 1, i32* %[[r1]]
   *x = 1;
 }
 
 // CHECK-LABEL: define void @_Z5func2v()
 void func2(void) {
   // CHECK: %lv1 = alloca i32, align 4, addrspace(5)
+  // CHECK: %[[r0:.*]] = addrspacecast i32 addrspace(5)* %lv1 to i32*
   // CHECK: %lv2 = alloca i32, align 4, addrspace(5)
+  // CHECK: %[[r1:.*]] = addrspacecast i32 addrspace(5)* %lv2 to i32*
   // CHECK: %la = alloca [100 x i32], align 4, addrspace(5)
+  // CHECK: %[[r2:.*]] = addrspacecast [100 x i32] addrspace(5)* %la to [100 x i32]*
   // CHECK: %lp1 = alloca i32*, align 8, addrspace(5)
+  // CHECK: %[[r3:.*]] = addrspacecast i32* addrspace(5)* %lp1 to i32**
   // CHECK: %lp2 = alloca i32*, align 8, addrspace(5)
+  // CHECK: %[[r4:.*]] = addrspacecast i32* addrspace(5)* %lp2 to i32**
   // CHECK: %lvc = alloca i32, align 4, addrspace(5)
+  // CHECK: %[[r5:.*]] = addrspacecast i32 addrspace(5)* %lvc to i32*
 
-  // CHECK: %[[r0:.*]] = addrspacecast i32 addrspace(5)* %lv1 to i32*
   // CHECK: store i32 1, i32* %[[r0]]
   int lv1;
   lv1 = 1;
-  // CHECK: %[[r1:.*]] = addrspacecast i32 addrspace(5)* %lv2 to i32*
   // CHECK: store i32 2, i32* %[[r1]]
   int lv2 = 2;
 
-  // CHECK: %[[r2:.*]] = addrspacecast [100 x i32] addrspace(5)* %la to [100 x i32]*
   // CHECK: %[[arrayidx:.*]] = getelementptr inbounds [100 x i32], [100 x i32]* %[[r2]], i64 0, i64 0
   // CHECK: store i32 3, i32* %[[arrayidx]], align 4
   int la[100];
   la[0] = 3;
 
-  // CHECK: %[[r3:.*]] = addrspacecast i32* addrspace(5)* %lp1 to i32**
   // CHECK: store i32* %[[r0]], i32** %[[r3]], align 8
   int *lp1 = &lv1;
 
-  // CHECK: %[[r4:.*]] = addrspacecast i32* addrspace(5)* %lp2 to i32**
   // CHECK: %[[arraydecay:.*]] = getelementptr inbounds [100 x i32], [100 x i32]* %[[r2]], i32 0, i32 0
   // CHECK: store i32* %[[arraydecay]], i32** %[[r4]], align 8
   int *lp2 = la;
 
   // CHECK: call void @_Z5func1Pi(i32* %[[r0]])
   func1(&lv1);
 
-  // CHECK: %[[r5:.*]] = addrspacecast i32 addrspace(5)* %lvc to i32*
   // CHECK: store i32 4, i32* %[[r5]]
   // CHECK: store i32 4, i32* %[[r0]]
   const int lvc = 4;
   lv1 = lvc;
 }
 
 void destroy(int x);
 

 void func4(int x) {
   // CHECK: %[[x_addr:.*]] = alloca i32, align 4, addrspace(5)
   // CHECK: %[[r0:.*]] = addrspacecast i32 addrspace(5)* %[[x_addr]] to i32*
   // CHECK: store i32 %x, i32* %[[r0]], align 4
   // CHECK: call void @_Z5func1Pi(i32* %[[r0]])
   func1(&x);
 }
 
+// CHECK-LABEL: define void @_Z5func5v
+void func5() {
+  return;
+  int x = 0;
+}
+
+// CHECK-LABEL: define void @_Z5func6v
+void func6() {
+  return;
+  int x;
+}
+
+// CHECK-LABEL: define void @_Z5func7v
+extern void use(int *);
+void func7() {
+  goto later;
+  int x;
+later:
+  use(&x);
+}
+
 // CHECK-NOT: !opencl.ocl.version