[CodeGen][ObjC] Add attribute "objc_arc_intert" to ObjC globals that are retain-agnostic

[ahatanak]

Summary

This is needed to enable the optimization in this patch: https://reviews.llvm.org/D62433

[erik.pilkington]

lib/CodeGen/CGBlocks.cpp

1441 ↗

(On Diff #202820)

Can't you just declare literal as a GlobalVariable instead of immediately casting it?

lib/CodeGen/CodeGenModule.cpp

4654 ↗

(On Diff #202820)

cast seems unnecessary.

test/CodeGenObjC/local-static-block.m

5 ↗

(On Diff #202820)

I believe FileCheck ignores CHECK lines when there is a custom check-prefix, so this should be CHECK-LP64. I think this makes more sense regardless in its own file though, its weird to piggyback on an unrelated test IMO.

59–60 ↗

(On Diff #202820)

(Likewise, these should be CHECK-LP64, or this test shouldn't be using a check-prefix. Would you mind updating this?)

[ahatanak]

test/CodeGenObjC/local-static-block.m

59–60 ↗

(On Diff #202820)

Fixed in r362651.

[ahatanak]

Address review comments.

[ahatanak]

test/CodeGenObjC/local-static-block.m

5 ↗

(On Diff #202820)

I added a file that just tests this attribute.

[erik.pilkington]

I think this looks good from a clang perspective. No reason to land it before the dust settles on D62433 though.

Do you think we can also add this attribute (or something like it) onto allocas of _NSConcreteStackBlocks? They also have this retain-swallowing behaviour.

[gottesmm]

This is exactly what I was imagining! This will enable the frontend to opt into this optimization without having to touch the optimizer. One nit: can we use a different name than "arc_retain_agnostic". Have you considered something like "arc_inert"? My fear is that at a glance (without thinking), you would think that the attribute would have something only to do with retain when we are really talking about ARC value operations. Beyond that looks great!

[ahatanak]

I agree. Something like arc_inert is probably a better name in this case for the reason you mentioned.

[rjmccall]

Should objc be in the attribute somewhere to avoid any future awkwardness? I don't remember what we've called similar attributes.

[ahatanak]

There are many attributes that start with objc_ including objc_arc_weak_reference_unavailable.

[rjmccall]

Please rename the test. There's also a typo in the differential title; please make sure that doesn't get into the commit message. Otherwise LGTM.

cfe/trunk/lib/CodeGen/CGBlocks.cpp

for (auto I : Helper->getCustomFieldValues(CGM, blockInfo)) {
       fields.add(I);
     }
   }
 
   unsigned AddrSpace = 0;
   if (CGM.getContext().getLangOpts().OpenCL)
     AddrSpace = CGM.getContext().getTargetAddressSpace(LangAS::opencl_global);
 
-  llvm::Constant *literal = fields.finishAndCreateGlobal(
+  llvm::GlobalVariable *literal = fields.finishAndCreateGlobal(
       "__block_literal_global", blockInfo.BlockAlign,
       /*constant*/ !IsWindows, llvm::GlobalVariable::InternalLinkage, AddrSpace);
 
+  literal->addAttribute("objc_arc_inert");
+
   // Windows does not allow globals to be initialised to point to globals in
   // different DLLs.  Any such variables must run code to initialise them.
   if (IsWindows) {
     auto *Init = llvm::Function::Create(llvm::FunctionType::get(CGM.VoidTy,
           {}), llvm::GlobalValue::InternalLinkage, ".block_isa_init",
         &CGM.getModule());
     llvm::IRBuilder<> b(llvm::BasicBlock::Create(CGM.getLLVMContext(), "entry",
           Init));

cfe/trunk/lib/CodeGen/CodeGenModule.cpp

CodeGenModule::GetAddrOfConstantCFString(const StringLiteral *Literal) {
   Fields.addInt(LengthTy, StringLength);
 
   CharUnits Alignment = getPointerAlign();
 
   // The struct.
   GV = Fields.finishAndCreateGlobal("_unnamed_cfstring_", Alignment,
                                     /*isConstant=*/false,
                                     llvm::GlobalVariable::PrivateLinkage);
+  GV->addAttribute("objc_arc_inert");
   switch (Triple.getObjectFormat()) {
   case llvm::Triple::UnknownObjectFormat:
     llvm_unreachable("unknown file format");
   case llvm::Triple::XCOFF:
     llvm_unreachable("XCOFF is not yet implemented");
   case llvm::Triple::COFF:
   case llvm::Triple::ELF:
   case llvm::Triple::Wasm:

cfe/trunk/test/CodeGenCXX/static-local-in-local-class.cpp

extern "C" int call_block_deduced_return() {
   return *decltype(block_deduced_return())()();
 }
 
 // CHECK-LABEL: define i32 @call_block_deduced_return()
 // CHECK: call i32* @_ZZZL20block_deduced_returnvEUb_EN1SclEv(
 // CHECK: load i32, i32* %
 // CHECK: ret i32 %
 
-// CHECK-LABEL: define internal i32* @_ZZZL20block_deduced_returnvEUb_EN1SclEv(%struct.S.6* %this) #0 align 2 {
+// CHECK-LABEL: define internal i32* @_ZZZL20block_deduced_returnvEUb_EN1SclEv(%struct.S.6* %this) #1 align 2 {
 // CHECK: ret i32* @_ZZZL20block_deduced_returnvEUb_E1n
 
 inline auto static_local_label(void *p) {
   if (p)
     goto *p;
   static void *q = &&label;
   struct S { static void *get() { return q; } };
   return S();

cfe/trunk/test/CodeGenObjC/attr-objc-arc-inert.m

+// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -fblocks -emit-llvm -o - %s | FileCheck %s
+
+// CHECK: @{{.*}} = private global %{{.*}} { i32* getelementptr inbounds ([0 x i32], [0 x i32]* @__CFConstantStringClassReference, i32 0, i32 0), i32 1992, i8* getelementptr inbounds ([4 x i8], [4 x i8]* @{{.*}}, i32 0, i32 0), i64 3 }, section "__DATA,__cfstring", align 8 #[[ATTRNUM0:[0-9]+]]
+// CHECK: @{{.*}} = internal constant { i8**, i32, i32, i8*, %{{.*}}* } { i8** @_NSConcreteGlobalBlock, i32 1342177280, i32 0, i8* bitcast (i32 (i8*)* @{{.*}} to i8*), %{{.*}}* bitcast ({ i64, i64, i8*, i8* }* @{{.*}} to %{{.*}}*) }, align 8 #[[ATTRNUM0]]
+
+@class NSString;
+
+NSString *testStringLiteral(void) {
+  return @"abc";
+}
+
+int testGlobalBlock(int a) {
+  return ^{ return 123; }();
+}
+
+// CHECK: attributes #[[ATTRNUM0]] = { "objc_arc_inert" }