[PATCH] D62831: [CodeGen][ObjC] Add attribute "arc_retain_agnostic" to ObjC globals that are retain-agnostic

[Erik Pilkington via Phabricator via cfe-commits]

erik.pilkington added inline comments.


================
Comment at: lib/CodeGen/CGBlocks.cpp:1441
 
+  cast<llvm::GlobalVariable>(literal)->addAttribute("arc_retain_agnostic");
+
----------------
Can't you just declare `literal` as a GlobalVariable instead of immediately casting it?


================
Comment at: lib/CodeGen/CodeGenModule.cpp:4654
                                     llvm::GlobalVariable::PrivateLinkage);
+  cast<llvm::GlobalVariable>(GV)->addAttribute("arc_retain_agnostic");
   switch (Triple.getObjectFormat()) {
----------------
`cast` seems unnecessary. 


================
Comment at: test/CodeGenObjC/local-static-block.m:5
 
+// CHECK: @{{.*}} = internal constant { i8**, i32, i32, i8*, %struct.__block_descriptor* } { i8** @_NSConcreteGlobalBlock, i32 1342177280, i32 0, i8* bitcast (%0* (i8*, %0*, i64)* @{{.*}} to i8*), %{{.*}}* bitcast ({ i64, i64, i8*, i8* }* @{{.*}} to %{{.*}}) }, align 8 #[[ATTRNUM0:.*]]
+
----------------
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. 


================
Comment at: test/CodeGenObjC/local-static-block.m:59-60
 // CHECK-LABEL-LP64: define void @FUNC2(
 // CHECK: define internal void @_block_invoke{{.*}}(
 // CHECK: call void %{{.*}}(i8* bitcast ({ i8**, i32, i32, i8*, %struct.__block_descriptor* }* @__block_literal_global{{.*}} to i8*), i32 %{{.*}})
 
----------------
(Likewise, these should be CHECK-LP64, or this test shouldn't be using a check-prefix. Would you mind updating this?)


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62831/new/

https://reviews.llvm.org/D62831