r268314 - [CodeGenObjCXX] Don't rematerialize default arguments of function

[Richard Smith via cfe-commits]

On Mon, May 2, 2016 at 2:52 PM, Akira Hatanaka via cfe-commits <
cfe-commits at lists.llvm.org> wrote:

> Author: ahatanak
> Date: Mon May  2 16:52:57 2016
> New Revision: 268314
>
> URL: http://llvm.org/viewvc/llvm-project?rev=268314&view=rev
> Log:
> [CodeGenObjCXX] Don't rematerialize default arguments of function
> parameters in the body of a block.
>
> This fixes a bug where clang would materialize the default argument
> inside the body of a block instead of passing the value via the block
> descriptor.
>
> For example, in the code below, foo1 would always print 42 regardless
> of the value of argument "a" passed to foo1.
>
> void foo1(const int a = 42 ) {
>   auto block = ^{
>     printf("%d\n", a);
>   };
>   block();
> }
>
> rdar://problem/24449235
>
> Added:
>     cfe/trunk/test/CodeGenObjCXX/block-default-arg.mm
> Modified:
>     cfe/trunk/lib/CodeGen/CGBlocks.cpp
>
> Modified: cfe/trunk/lib/CodeGen/CGBlocks.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGBlocks.cpp?rev=268314&r1=268313&r2=268314&view=diff
>
> ==============================================================================
> --- cfe/trunk/lib/CodeGen/CGBlocks.cpp (original)
> +++ cfe/trunk/lib/CodeGen/CGBlocks.cpp Mon May  2 16:52:57 2016
> @@ -262,6 +262,11 @@ static bool isSafeForCXXConstantCapture(
>  static llvm::Constant *tryCaptureAsConstant(CodeGenModule &CGM,
>                                              CodeGenFunction *CGF,
>                                              const VarDecl *var) {
> +  // Don't rematerialize default arguments of function parameters.
> +  if (auto *PD = dyn_cast<ParmVarDecl>(var))
> +    if (PD->hasDefaultArg())
>

I don't think you need this test, and I think it somewhat confuses the
intent here. (A reader would wonder why you want to keep going for
ParmVarDecls that don't have default arguments.)


> +      return nullptr;
> +
>    QualType type = var->getType();
>
>    // We can only do this if the variable is const.
>
> Added: cfe/trunk/test/CodeGenObjCXX/block-default-arg.mm
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenObjCXX/block-default-arg.mm?rev=268314&view=auto
>
> ==============================================================================
> --- cfe/trunk/test/CodeGenObjCXX/block-default-arg.mm (added)
> +++ cfe/trunk/test/CodeGenObjCXX/block-default-arg.mm Mon May  2 16:52:57
> 2016
> @@ -0,0 +1,16 @@
> +// RUN: %clang_cc1 -triple x86_64-apple-darwin10.0.0 -emit-llvm -o - %s
> -std=c++11 -fblocks -fobjc-arc | FileCheck  %s
> +
> +// CHECK: define internal void @___Z16test_default_argi_block_invoke(i8*
> %[[BLOCK_DESCRIPTOR:.*]])
> +// CHECK: %[[BLOCK:.*]] = bitcast i8* %[[BLOCK_DESCRIPTOR]] to <{ i8*,
> i32, i32, i8*, %struct.__block_descriptor*, i32 }>*
> +// CHECK: %[[BLOCK_CAPTURE_ADDR:.*]] = getelementptr inbounds <{ i8*,
> i32, i32, i8*, %struct.__block_descriptor*, i32 }>, <{ i8*, i32, i32, i8*,
> %struct.__block_descriptor*, i32 }>* %[[BLOCK]], i32 0, i32 5
> +// CHECK: %[[V0:.*]] = load i32, i32* %[[BLOCK_CAPTURE_ADDR]]
> +// CHECK: call void @_Z4foo1i(i32 %[[V0]])
> +
> +void foo1(int);
> +
> +void test_default_arg(const int a = 42) {
> +  auto block = ^{
> +    foo1(a);
> +  };
> +  block();
> +}
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20160502/50de1d00/attachment.html>