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

[Akira Hatanaka via cfe-commits]

It looks like turning it to an assert wouldn’t be correct, I’ll probably just remove it.

> On May 2, 2016, at 3:12 PM, Akira Hatanaka via cfe-commits <cfe-commits at lists.llvm.org> wrote:
> 
> I see. Perhaps this should be an assert?
> 
>> On May 2, 2016, at 3:05 PM, Richard Smith <richard at metafoo.co.uk <mailto: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 <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>
> _______________________________________________
> 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/09cfd61a/attachment.html>