r234499 - [CodeGen] When promoting a reference temporary to a global use the inner type to fold it.

[Richard Smith]

On Thu, Apr 9, 2015 at 9:09 AM, Benjamin Kramer <benny.kra at googlemail.com>
wrote:

> Author: d0k
> Date: Thu Apr  9 11:09:29 2015
> New Revision: 234499
>
> URL: http://llvm.org/viewvc/llvm-project?rev=234499&view=rev
> Log:
> [CodeGen] When promoting a reference temporary to a global use the inner
> type to fold it.
>
> The MaterializeTemporaryExpr can have a different type than the inner
> expression, miscompiling the constant. PR23165.
>
> Modified:
>     cfe/trunk/lib/CodeGen/CGExpr.cpp
>     cfe/trunk/test/CodeGenCXX/cxx0x-initializer-references.cpp
>
> Modified: cfe/trunk/lib/CodeGen/CGExpr.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGExpr.cpp?rev=234499&r1=234498&r2=234499&view=diff
>
> ==============================================================================
> --- cfe/trunk/lib/CodeGen/CGExpr.cpp (original)
> +++ cfe/trunk/lib/CodeGen/CGExpr.cpp Thu Apr  9 11:09:29 2015
> @@ -309,12 +309,13 @@ createReferenceTemporary(CodeGenFunction
>          (M->getType()->isArrayType() || M->getType()->isRecordType()) &&
>          CGF.CGM.isTypeConstant(M->getType(), true))
>

This reference to M->getType() should be changed to Inner->getType() too.
Consider a case like:

  struct S { struct T { int a; } t; mutable int b; };
  void f() { const S::T &r = S().t; }

Here, we shouldn't emit the temporary as a global constant due to the
mutable member.

The same is probably true of the array type / record type check -- if the
reference temporary itself is of array or reference type, and we're binding
a reference to a non-array/non-reference subobject, we should probably
still promote to a global.

       if (llvm::Constant *Init =
> -              CGF.CGM.EmitConstantExpr(Inner, M->getType(), &CGF)) {
> +              CGF.CGM.EmitConstantExpr(Inner, Inner->getType(), &CGF)) {
>          auto *GV = new llvm::GlobalVariable(
>              CGF.CGM.getModule(), Init->getType(), /*isConstant=*/true,
>              llvm::GlobalValue::PrivateLinkage, Init, ".ref.tmp");
> -        GV->setAlignment(
> -
> CGF.getContext().getTypeAlignInChars(M->getType()).getQuantity());
> +        GV->setAlignment(CGF.getContext()
> +                             .getTypeAlignInChars(Inner->getType())
> +                             .getQuantity());
>          // FIXME: Should we put the new global into a COMDAT?
>          return GV;
>        }
>
> Modified: cfe/trunk/test/CodeGenCXX/cxx0x-initializer-references.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenCXX/cxx0x-initializer-references.cpp?rev=234499&r1=234498&r2=234499&view=diff
>
> ==============================================================================
> --- cfe/trunk/test/CodeGenCXX/cxx0x-initializer-references.cpp (original)
> +++ cfe/trunk/test/CodeGenCXX/cxx0x-initializer-references.cpp Thu Apr  9
> 11:09:29 2015
> @@ -1,5 +1,7 @@
>  // RUN: %clang_cc1 -std=c++11 -S -triple armv7-none-eabi -emit-llvm -o -
> %s | FileCheck %s
>
> +// CHECK: private constant { i8** } { i8** getelementptr inbounds ([3 x
> i8*], [3 x i8*]* @_ZTVN7PR2316510ChildClassE, i64 0, i64 2) }, align 4
> +
>  namespace reference {
>    struct A {
>      int i1, i2;
> @@ -79,3 +81,22 @@ namespace reference {
>    }
>
>  }
> +
> +namespace PR23165 {
> +struct AbstractClass {
> +  virtual void foo() const = 0;
> +};
> +
> +struct ChildClass : public AbstractClass {
> +  virtual void foo() const {}
> +};
> +
> +void helper(const AbstractClass &param) {
> +  param.foo();
> +}
> +
> +void foo() {
> +// CHECK: call void @_ZN7PR231656helperERKNS_13AbstractClassE(%{{.*}}
> bitcast ({ i8** }* @{{.*}} to %{{.*}}*))
> +  helper(ChildClass());
> +}
> +}
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20150409/b8acdb45/attachment.html>