[PATCH] Pass -mglobal-merge as a module flag metadata.

[Duncan P. N. Exon Smith]

+echristo

> On 2015-Mar-02, at 09:55, Ahmed Bougacha <ahmed.bougacha at gmail.com> wrote:
> 
> - Only emit flag when GlobalMerge is disabled (per Akira and Duncan's reviews)
> 
> I added a target check to only emit the flags on ARM/AArch64.  I'm fine with emitting the flag no matter the target, if desired (less brittle), but I personally don't like adding noise to every module ever, hence hiding it.

Now the logic is what I expected, but I'm less confident that it's the
best path.

The logic shakes out like this:

  - Change the driver to *always* specify -m{no-,}global-merge.
  - In -cc1:
      - If -mglobal-merge: don't set any flags.
      - If -mno-global-merge: add `Error` flag to explicitly disable it,
        if this is a relevant target.
  - When linking modules:
      - If any module has the flag, it's disabled in the linked module.
  - When running codegen: run global-merge (if possible) unless it has
    been disabled.

Maybe I haven't had enough sleep, but this other logic seems more
appealing to me now.

  - Use the old driver logic (pass the option through only if
    specified).
  - In -cc1:
      - -mno-global-merge: add `Override` flag to disable it.
      - -mglobal-merge: add `Error` flag to enable it.
      - (Otherwise, set the flag based on optimization level?  Or don't
        set a flag?)
  - When linking modules:
      - If any module disabled it, it's disabled in the linked module.
      - Else, if any module has enabled it, it's enabled in the linked
        module.
      - Else, the linked module has neither flag.
  - When running codegen: respect the flag if present; otherwise, use
    old logic (optimization level?).

@Eric/Akira/Ahmed, any thoughts?

@Ahmed, if we stick with this logic, I have some nitpicks below on the
spelling.

Also, was there an LLVM patch that went with this?  Can you attach it to
the same thread?


> Index: lib/CodeGen/CodeGenModule.cpp
> ===================================================================
> --- lib/CodeGen/CodeGenModule.cpp
> +++ lib/CodeGen/CodeGenModule.cpp
> @@ -418,6 +418,8 @@
>    EmitVersionIdentMetadata();
>  
>    EmitTargetMetadata();
> +
> +  EmitBackendOptionsMetadata();
>  }
>  
>  void CodeGenModule::UpdateCompletedType(const TagDecl *TD) {
> @@ -3609,6 +3611,18 @@
>    }
>  }
>  
> +void CodeGenModule::EmitBackendOptionsMetadata() {
> +  // Only emit the "Global Merge" flag when it disables it, and only on targets
> +  // that support it (ARM/AArch64).
> +  llvm::Triple::ArchType Arch = Context.getTargetInfo().getTriple().getArch();
> +  if ((Arch == llvm::Triple::arm || Arch == llvm::Triple::armeb ||
> +       Arch == llvm::Triple::thumb || Arch == llvm::Triple::thumbeb ||
> +       Arch == llvm::Triple::aarch64 || Arch == llvm::Triple::aarch64_be) &&
> +      getCodeGenOpts().NoGlobalMerge)
> +    getModule().addModuleFlag(llvm::Module::Error, "Enable Global Merge",
> +                              llvm::MDString::get(VMContext, "false"));

I think this should be:

    getModule().addModuleFlag(llvm::Module::Error, "Disable Global Merge", 1u);

> +}
> +
>  void CodeGenModule::EmitCoverageFile() {
>    if (!getCodeGenOpts().CoverageFile.empty()) {
>      if (llvm::NamedMDNode *CUNode = TheModule.getNamedMetadata("llvm.dbg.cu")) {