[PATCH] D20217: Add direct control of whether or not a symbol is preemtable at runtime

[Rafael Avila de Espindola via llvm-commits]

The description is now out of date as it still refers to a bit that
flips the meaning.

I agree with Reid Kleckner, please split the patch so that this one has
as few user visible changes as possible.

Sean Fertile via Phabricator <reviews at reviews.llvm.org> writes:


> Index: lib/Target/PowerPC/PPCISelLowering.cpp
> ===================================================================
> --- lib/Target/PowerPC/PPCISelLowering.cpp
> +++ lib/Target/PowerPC/PPCISelLowering.cpp
> @@ -4232,6 +4232,37 @@
>  }
>  
>  static bool
> +isCodeModelMediumOrLarge(CodeModel::Model CM, const Triple& TT) {
> +  switch (CM) {
> +  case CodeModel::Medium:
> +  case CodeModel::Large:
> +    return true;
> +  case CodeModel::Default:
> +    return !TT.isOSDarwin() &&
> +           (TT.getArch() == Triple::ppc64 || TT.getArch() == Triple::ppc64le);
> +  default:
> +    return false;
> +  }

I would probably fully cover the enum and remove the default:

> Index: include/llvm/IR/GlobalValue.h
> ===================================================================
> --- include/llvm/IR/GlobalValue.h
> +++ include/llvm/IR/GlobalValue.h
> @@ -73,20 +73,26 @@
>      DLLExportStorageClass = 2  ///< Function to be accessible from DLL.
>    };
>  
> +  enum DSO_Location {
> +    DSO_Default = 0,
> +    DSO_Local,
> +    DSO_Preemptable
> +  };

Is the intention to have the 3 values in the end? I can see the point of
having defaults in the .ll to make it less verbose, but once parsed
every GV should be local or preemtable, no?

I am definitely ok with having 3 values for now to make an easier
transition.

Cheers,
Rafael