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

[Akira Hatanaka via cfe-commits]

I see. Perhaps this should be an assert?

> On May 2, 2016, at 3:05 PM, Richard Smith <richard at metafoo.co.uk> wrote:
> 
> On Mon, May 2, 2016 at 2:52 PM, Akira Hatanaka via cfe-commits <cfe-commits at lists.llvm.org <mailto: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 <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 <http://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 <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 <http://block-default-arg.mm/>
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGenObjCXX/block-default-arg.mm?rev=268314&view=auto <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 <http://block-default-arg.mm/> (added)
> +++ cfe/trunk/test/CodeGenObjCXX/block-default-arg.mm <http://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 <mailto:cfe-commits at lists.llvm.org>
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits <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/1d0cb087/attachment-0001.html>