[cfe-commits] r155305 - in /cfe/trunk: lib/CodeGen/CodeGenFunction.cpp test/CodeGenCXX/global-array-destruction.cpp

[Meador Inge]

Hi Richard,

On Sun, Apr 22, 2012 at 12:51 AM, Richard Smith
<richard-llvm at metafoo.co.uk> wrote:
> Author: rsmith
> Date: Sun Apr 22 00:51:36 2012
> New Revision: 155305
>
> URL: http://llvm.org/viewvc/llvm-project?rev=155305&view=rev
> Log:
> PR12571: Objects of type clang::ConstantArrayType aren't always emitted with
> type llvm::ArrayType -- sometimes we emit them as packed structs. Don't assert
> if such a global array has an element type with a non-trivial destructor.

I was poking around at PR 12571 and am curious about something.  My first
inclination was to do exactly what did: fix 'CodeGenFunction::emitArrayLength'
to handle structures.  Then I got to wondering how structures ever end up there
to begin with.

This is what found:

1. 'CodeGenModule::EmitGlobalVarDefinition' calls 'EmitConstantInit'
    to create the intialization constant for 'A a[] = { 0 };'.

2. When calling 'EmitConstantInit' we end up in 'ConstStructBuilder::Finalize'
    which decides to use '{ i32*, i32, [4 x i8] }' (the one the builder created)
    for the constant type instead of '%class.A = type { i32*, i32 }'.

The '{ i32*, i32, [4 x i8] }' and '%class.A = type { i32*, i32 }' types are seen
as having different layouts because the tail padding is explicit in the latter.
Is this right?  I would think that class.A should have the tail padding too and
that the layouts should be identical.  (NOTE: because of this the original ICE
can be reproduced using any case similar to the original that causes the tail
padding to be added).

> Modified:
>    cfe/trunk/lib/CodeGen/CodeGenFunction.cpp
>    cfe/trunk/test/CodeGenCXX/global-array-destruction.cpp
>
> Modified: cfe/trunk/lib/CodeGen/CodeGenFunction.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CodeGenFunction.cpp?rev=155305&r1=155304&r2=155305&view=diff
> ==============================================================================
> --- cfe/trunk/lib/CodeGen/CodeGenFunction.cpp (original)
> +++ cfe/trunk/lib/CodeGen/CodeGenFunction.cpp Sun Apr 22 00:51:36 2012
> @@ -881,33 +881,49 @@
>   llvm::ConstantInt *zero = Builder.getInt32(0);
>   gepIndices.push_back(zero);
>
> -  // It's more efficient to calculate the count from the LLVM
> -  // constant-length arrays than to re-evaluate the array bounds.
>   uint64_t countFromCLAs = 1;
> +  QualType eltType;
>
>   llvm::ArrayType *llvmArrayType =
> -    cast<llvm::ArrayType>(
> +    dyn_cast<llvm::ArrayType>(
>       cast<llvm::PointerType>(addr->getType())->getElementType());
> -  while (true) {
> +  while (llvmArrayType) {
>     assert(isa<ConstantArrayType>(arrayType));
>     assert(cast<ConstantArrayType>(arrayType)->getSize().getZExtValue()
>              == llvmArrayType->getNumElements());
>
>     gepIndices.push_back(zero);
>     countFromCLAs *= llvmArrayType->getNumElements();
> +    eltType = arrayType->getElementType();
>
>     llvmArrayType =
>       dyn_cast<llvm::ArrayType>(llvmArrayType->getElementType());
> -    if (!llvmArrayType) break;
> -
>     arrayType = getContext().getAsArrayType(arrayType->getElementType());
> -    assert(arrayType && "LLVM and Clang types are out-of-synch");
> +    assert((!llvmArrayType || arrayType) &&
> +           "LLVM and Clang types are out-of-synch");
>   }
>
> -  baseType = arrayType->getElementType();
> +  if (arrayType) {
> +    // From this point onwards, the Clang array type has been emitted
> +    // as some other type (probably a packed struct). Compute the array
> +    // size, and just emit the 'begin' expression as a bitcast.
> +    while (arrayType) {
> +      countFromCLAs *=
> +          cast<ConstantArrayType>(arrayType)->getSize().getZExtValue();
> +      eltType = arrayType->getElementType();
> +      arrayType = getContext().getAsArrayType(eltType);
> +    }
> +
> +    unsigned AddressSpace =
> +        cast<llvm::PointerType>(addr->getType())->getAddressSpace();
> +    llvm::Type *BaseType = ConvertType(eltType)->getPointerTo(AddressSpace);
> +    addr = Builder.CreateBitCast(addr, BaseType, "array.begin");
> +  } else {
> +    // Create the actual GEP.
> +    addr = Builder.CreateInBoundsGEP(addr, gepIndices, "array.begin");
> +  }
>
> -  // Create the actual GEP.
> -  addr = Builder.CreateInBoundsGEP(addr, gepIndices, "array.begin");
> +  baseType = eltType;
>
>   llvm::Value *numElements
>     = llvm::ConstantInt::get(SizeTy, countFromCLAs);
>
> Modified: cfe/trunk/test/CodeGenCXX/global-array-destruction.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/global-array-destruction.cpp?rev=155305&r1=155304&r2=155305&view=diff
> ==============================================================================
> --- cfe/trunk/test/CodeGenCXX/global-array-destruction.cpp (original)
> +++ cfe/trunk/test/CodeGenCXX/global-array-destruction.cpp Sun Apr 22 00:51:36 2012
> @@ -1,6 +1,4 @@
> -// REQUIRES: x86-64-registered-target
> -// RUN: %clang_cc1 -triple x86_64-apple-darwin -std=c++11 -S %s -o %t-64.s
> -// RUN: FileCheck -check-prefix LP64 --input-file=%t-64.s %s
> +// RUN: %clang_cc1 -triple x86_64-apple-darwin -std=c++11 -emit-llvm %s -o - | FileCheck %s
>
>  extern "C" int printf(...);
>
> @@ -24,11 +22,24 @@
>  S s2;
>  S arr3[3];
>
> -// CHECK-LP64: callq    ___cxa_atexit
> -// CHECK-LP64: callq    ___cxa_atexit
> -// CHECK-LP64: callq    ___cxa_atexit
> -// CHECK-LP64: callq    ___cxa_atexit
> -// CHECK-LP64: callq    ___cxa_atexit
> -// CHECK-LP64: callq    ___cxa_atexit
> -// CHECK-LP64: callq    ___cxa_atexit
> -// CHECK-LP64: callq    ___cxa_atexit
> +// CHECK: call {{.*}} @__cxa_atexit
> +// CHECK: call {{.*}} @__cxa_atexit
> +// CHECK: call {{.*}} @__cxa_atexit
> +// CHECK: call {{.*}} @__cxa_atexit
> +// CHECK: call {{.*}} @__cxa_atexit
> +// CHECK: call {{.*}} @__cxa_atexit
> +// CHECK: call {{.*}} @__cxa_atexit
> +// CHECK: call {{.*}} @__cxa_atexit
> +
> +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 ({{.*}} bitcast {{.*}}* @t to %struct.T*), i64 6
> +// CHECK: call void @_ZN1TD1Ev
> +// CHECK: icmp eq {{.*}} @t
> +// CHECK: br i1 {{.*}}
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits



-- 
# Meador