[CodeGen] Make element type in emitArrayDestroy() predictable

[nikic]

Summary

When calling emitArrayDestroy(), the pointer will usually have ConvertTypeForMem(EltType) as the element type, as one would expect. However, globals with initializers sometimes don't use the same types as values normally would, e.g. here the global uses { double, i32 } rather than %struct.T as element type.

Add an early cast to the global destruction path to avoid this special case. The cast would happen lateron anyway, it only gets moved to an earlier point.

[nikic]

Merge fixes for element type uses into this patch.

[rjmccall]

clang/lib/CodeGen/CGDecl.cpp

2257

Hmm. I would expect the caller to be responsible for passing in a correctly-typed pointer here.

clang/lib/CodeGen/CodeGenFunction.cpp

2115 ↗

(On Diff #397018)

I think I have a similar question here. I understand that globals don't always have the right type because of initializers, but the code that's emitting the reference to the global generally ought to be casting it to the right type instead of forcing all the downstream code to cast.

[nikic]

Perform cast earlier, in EmitDeclDestroy().

[nikic]

clang/lib/CodeGen/CGDecl.cpp

2257

The caller is responsible at this point. This only explicitly computes the type to avoid the getPointerElementType() call.

clang/lib/CodeGen/CodeGenFunction.cpp

2115 ↗

(On Diff #397018)

I've moved the cast into EmitDeclDestroy() and left this code alone. Does this look better?

[nikic]

Rebase over D116587 to get rid of the double geps.

[rjmccall]

clang/lib/CodeGen/CGDecl.cpp

2257

Okay. Is it excessively annoying to pass in an Address for begin instead of separately passing elementAlign? We'd get the element type for free then.

[nikic]

clang/lib/CodeGen/CGDecl.cpp

2257

Yeah, I did give that a try -- accepting Address in emitArrayDestroy() in particular is easy, but once you try to propagate it up the call chain it turns out that there are a lot of non-trivial callers.

[rjmccall]

Alright. We can do this for now, then.

clang/lib/CodeGen/CGDecl.cpp

void CodeGenFunction::emitArrayDestroy(llvm::Value *begin,
   llvm::BasicBlock *entryBB = Builder.GetInsertBlock();
   EmitBlock(bodyBB);
   llvm::PHINode *elementPast =
     Builder.CreatePHI(begin->getType(), 2, "arraydestroy.elementPast");
   elementPast->addIncoming(end, entryBB);
 
   // Shift the address back by one element.
   llvm::Value *negativeOne = llvm::ConstantInt::get(SizeTy, -1, true);
+  llvm::Type *llvmElementType = ConvertTypeForMem(elementType);
   llvm::Value *element = Builder.CreateInBoundsGEP(
-      elementPast->getType()->getPointerElementType(), elementPast, negativeOne,
-      "arraydestroy.element");
+      llvmElementType, elementPast, negativeOne, "arraydestroy.element");
 
   if (useEHCleanup)
     pushRegularPartialArrayCleanup(begin, element, elementType, elementAlign,
                                    destroyer);
 
   // Perform the actual destruction there.
-  destroyer(*this, Address(element, elementAlign), elementType);
+  destroyer(*this, Address(element, llvmElementType, elementAlign),
+            elementType);
 
   if (useEHCleanup)
     PopCleanupBlock();
 
   // Check whether we've reached the end.
   llvm::Value *done = Builder.CreateICmpEQ(element, begin, "arraydestroy.done");
   Builder.CreateCondBr(done, doneBB, bodyBB);
   elementPast->addIncoming(element, Builder.GetInsertBlock());

clang/lib/CodeGen/CGDeclCXX.cpp

if (CGF.getContext().getLangOpts().OpenCL) {
         // of the global destructor function should be adjusted accordingly.
         Argument = llvm::ConstantPointerNull::get(DestTy);
     } else {
       Argument = llvm::ConstantExpr::getBitCast(
           Addr.getPointer(), CGF.getTypes().ConvertType(Type)->getPointerTo());
     }
   // Otherwise, the standard logic requires a helper function.
   } else {
+    Addr = Addr.getElementBitCast(CGF.ConvertTypeForMem(Type));
     Func = CodeGenFunction(CGM)
            .generateDestroyHelper(Addr, Type, CGF.getDestroyer(DtorKind),
                                   CGF.needsEHCleanup(DtorKind), &D);
     Argument = llvm::Constant::getNullValue(CGF.Int8PtrTy);
   }
 
   CGM.getCXXABI().registerGlobalDtor(CGF, D, Func, Argument);
 }

clang/test/CodeGenCXX/global-array-destruction.cpp

 struct T {
   double d;
   int n;
   ~T();
 };
 T t[2][3] = { 1.0, 2, 3.0, 4, 5.0, 6, 7.0, 8, 9.0, 10, 11.0, 12 };
 
 // CHECK: call {{.*}} @__cxa_atexit
-// CHECK: getelementptr inbounds ([2 x [3 x {{.*}}]], [2 x [3 x {{.*}}]]* @t, i64 1, i64 0, i64 0)
+// CHECK: getelementptr inbounds ([2 x [3 x %struct.T]], [2 x [3 x %struct.T]]* bitcast ([2 x [3 x { double, i32 }]]* @t to [2 x [3 x %struct.T]]*), i64 1, i64 0, i64 0)
 // CHECK: call void @_ZN1TD1Ev
 // CHECK: icmp eq {{.*}} @t
 // CHECK: br i1 {{.*}}
 
 static T t2[2][3] = { 1.0, 2, 3.0, 4, 5.0, 6, 7.0, 8, 9.0, 10, 11.0, 12 };
 
 // CHECK: call {{.*}} @__cxa_atexit
-// CHECK: getelementptr inbounds ([2 x [3 x {{.*}}]], [2 x [3 x {{.*}}]]* @_ZL2t2, i64 1, i64 0, i64 0)
+// CHECK: getelementptr inbounds ([2 x [3 x %struct.T]], [2 x [3 x %struct.T]]* bitcast ([2 x [3 x { double, i32 }]]* @_ZL2t2 to [2 x [3 x %struct.T]]*), i64 1, i64 0, i64 0)
 // CHECK: call void @_ZN1TD1Ev
 // CHECK: icmp eq {{.*}} @_ZL2t2
 // CHECK: br i1 {{.*}}
 
 using U = T[2][3];
 U &&u = U{ {{1.0, 2}, {3.0, 4}, {5.0, 6}}, {{7.0, 8}, {9.0, 10}, {11.0, 12}} };
 
 // CHECK: call {{.*}} @__cxa_atexit
-// CHECK: getelementptr inbounds ([2 x [3 x {{.*}}]], [2 x [3 x {{.*}}]]* @_ZGR1u_, i64 1, i64 0, i64 0)
+// CHECK: getelementptr inbounds ([2 x [3 x %struct.T]], [2 x [3 x %struct.T]]* @_ZGR1u_, i64 1, i64 0, i64 0)
 // CHECK: call void @_ZN1TD1Ev
 // CHECK: icmp eq {{.*}} @_ZGR1u_
 // CHECK: br i1 {{.*}}