[PATCH] Add ObjC ARC module flag and use it to control arc-contract pass

[Duncan P. N. Exon Smith]

> On 2015-Feb-20, at 14:17, Steven Wu <stevenwu at apple.com> wrote:
> 
> Hi dexonsmith, gottesmm,
> 
> Currently, there is no good way to tell if the bitcode is compiled
> with ARC or not. However, it is required to run objc-arc-contract
> pass for ObjC ARC. Introduces a new module flag for ObjC-ARC and
> use that to decide if objc-arc-contract pass is required or not.
> 

This makes sense to me.  You're missing testcases:

  - Test that the flag gets added correctly (only when it should).
  - Test that the pass gets run based on the flag.

Some other comments inline.

> http://reviews.llvm.org/D7799
> 
> Files:
>  lib/CodeGen/BackendUtil.cpp
>  lib/CodeGen/CGObjCMac.cpp
> 
> Index: lib/CodeGen/BackendUtil.cpp
> ===================================================================
> --- lib/CodeGen/BackendUtil.cpp
> +++ lib/CodeGen/BackendUtil.cpp
> @@ -568,7 +568,7 @@
>   // Add ObjC ARC final-cleanup optimizations. This is done as part of the
>   // "codegen" passes so that it isn't run multiple times when there is
>   // inlining happening.
> -  if (LangOpts.ObjCAutoRefCount &&
> +  if (TheModule->getModuleFlag("Objective-C ARC") &&
>       CodeGenOpts.OptimizationLevel > 0)
>     PM->add(createObjCARCContractPass());
> 
> Index: lib/CodeGen/CGObjCMac.cpp
> ===================================================================
> --- lib/CodeGen/CGObjCMac.cpp
> +++ lib/CodeGen/CGObjCMac.cpp
> @@ -4283,6 +4283,13 @@
>     }
>   }
> 
> +  // Add ObjCAutoRefCount as a module flag

I don't think this comment is helpful.  It's clear from the code
what's happening.

Be sure the commit message explains *why* the logic is this way,
though.

> +  if (CGM.getLangOpts().ObjCAutoRefCount) {
> +    // AutoRefCount will overwrites files which don't have ARC.

I don't think this comment is helpful.

> +    Mod.addModuleFlag(llvm::Module::Override,
> +                      "Objective-C ARC", (uint32_t)1);

I think `1u` is easier to read than `(uint32_t)1` here.

Also, I think `Error` should be sufficient.  There's nothing to
override here (we never set it to anything but `1`).

> +  }
> +
>   // Indicate whether we're compiling this to run on a simulator.
>   const llvm::Triple &Triple = CGM.getTarget().getTriple();
>   if (Triple.isiOS() &&