[NFC][Clang] Remove redundant function definitions

[jmmartinez]

Summary

There were 3 definitions of the mergeDefaultFunctionDefinitionAttributes
function: A private implementation, a version exposed in CodeGen, a
version exposed in CodeGenModule.

This patch removes the private and the CodeGenModule versions and keeps
a single definition in CodeGen.

[jmmartinez]

@jhuber6 I was wondering if there is a reason you kept 3 versions of mergeDefaultFunctionDefinitionAttributes in https://reviews.llvm.org/D152391 ?

[jhuber6]

In D159256#4630410, @jmmartinez wrote:

@jhuber6 I was wondering if there is a reason you kept 3 versions of mergeDefaultFunctionDefinitionAttributes in https://reviews.llvm.org/D152391 ?

I believe it's because one was a freestanding function, the other was a member function, and the last was a common implementation.

[jmmartinez]

In D159256#4630876, @jhuber6 wrote:

In D159256#4630410, @jmmartinez wrote:

@jhuber6 I was wondering if there is a reason you kept 3 versions of mergeDefaultFunctionDefinitionAttributes in https://reviews.llvm.org/D152391 ?

I believe it's because one was a freestanding function, the other was a member function, and the last was a common implementation.

Would it be ok if I keep only one? It seems that the member function is not used (I was not sure if there was some external code using it).

If not, I can also keep just 2 versions (the freestanding function and the member function), move the implementation to the freestanding one, and drop the static function since it is redundant.

[jhuber6]

In D159256#4630915, @jmmartinez wrote:

In D159256#4630876, @jhuber6 wrote:

In D159256#4630410, @jmmartinez wrote:

@jhuber6 I was wondering if there is a reason you kept 3 versions of mergeDefaultFunctionDefinitionAttributes in https://reviews.llvm.org/D152391 ?

I believe it's because one was a freestanding function, the other was a member function, and the last was a common implementation.

Would it be ok if I keep only one? It seems that the member function is not used (I was not sure if there was some external code using it).

If not, I can also keep just 2 versions (the freestanding function and the member function), move the implementation to the freestanding one, and drop the static function since it is redundant.

Yeah I think I noticed that when I was doing the patch but I just left it because I figured it would be less disruptive. It should be fine since I'm not aware of any other users.

[jmmartinez]

Thanks for the review!

clang/lib/CodeGen/CGCall.h

public:
 
   bool isNull() const { return !Addr.isValid(); }
   bool isVolatile() const { return IsVolatile; }
   Address getValue() const { return Addr; }
   bool isUnused() const { return IsUnused; }
   bool isExternallyDestructed() const { return IsExternallyDestructed; }
 };
 
-/// Helper to add attributes to \p F according to the CodeGenOptions and
-/// LangOptions without requiring a CodeGenModule to be constructed.
+/// Adds attributes to \p F according to our \p CodeGenOpts and \p LangOpts, as
+/// though we had emitted it ourselves. We remove any attributes on F that
+/// conflict with the attributes we add here.
+///
+/// This is useful for adding attrs to bitcode modules that you want to link
+/// with but don't control, such as CUDA's libdevice.  When linking with such
+/// a bitcode library, you might want to set e.g. its functions'
+/// "unsafe-fp-math" attribute to match the attr of the functions you're
+/// codegen'ing.  Otherwise, LLVM will interpret the bitcode module's lack of
+/// unsafe-fp-math attrs as tantamount to unsafe-fp-math=false, and then LLVM
+/// will propagate unsafe-fp-math=false up to every transitive caller of a
+/// function in the bitcode library!
+///
+/// With the exception of fast-math attrs, this will only make the attributes
+/// on the function more conservative.  But it's unsafe to call this on a
+/// function which relies on particular fast-math attributes for correctness.
+/// It's up to you to ensure that this is safe.
 void mergeDefaultFunctionDefinitionAttributes(llvm::Function &F,
-                                              const CodeGenOptions CodeGenOpts,
+                                              const CodeGenOptions &CodeGenOpts,
                                               const LangOptions &LangOpts,
                                               const TargetOptions &TargetOpts,
                                               bool WillInternalize);
 
 enum class FnInfoOpts {
   None = 0,
   IsInstanceMethod = 1 << 0,
   IsChainCall = 1 << 1,

clang/lib/CodeGen/CGCall.cpp

static void getTrivialDefaultFunctionAttributes(
 
   for (StringRef Attr : CodeGenOpts.DefaultFunctionAttrs) {
     StringRef Var, Value;
     std::tie(Var, Value) = Attr.split('=');
     FuncAttrs.addAttribute(Var, Value);
   }
 }
 
-/// Adds attributes to \p F according to our \p CodeGenOpts and \p LangOpts, as
-/// though we had emitted it ourselves. We remove any attributes on F that
-/// conflict with the attributes we add here.
-static void mergeDefaultFunctionDefinitionAttributes(
+void CodeGen::mergeDefaultFunctionDefinitionAttributes(
     llvm::Function &F, const CodeGenOptions &CodeGenOpts,
     const LangOptions &LangOpts, const TargetOptions &TargetOpts,
     bool WillInternalize) {
 
   llvm::AttrBuilder FuncAttrs(F.getContext());
   // Here we only extract the options that are relevant compared to the version
   // from GetCPUAndFeaturesAttributes.
   if (!TargetOpts.CPU.empty())
FuncAttrs.addAttribute("denormal-fp-math-f32",
                            CodeGenOpts.FP32DenormalMode.str());
   }
 
   F.removeFnAttrs(AttrsToRemove);
   addDenormalModeAttrs(Merged, MergedF32, FuncAttrs);
   F.addFnAttrs(FuncAttrs);
 }
 
-void clang::CodeGen::mergeDefaultFunctionDefinitionAttributes(
-    llvm::Function &F, const CodeGenOptions CodeGenOpts,
-    const LangOptions &LangOpts, const TargetOptions &TargetOpts,
-    bool WillInternalize) {
-
-  ::mergeDefaultFunctionDefinitionAttributes(F, CodeGenOpts, LangOpts,
-                                             TargetOpts, WillInternalize);
-}
-
 void CodeGenModule::getTrivialDefaultFunctionAttributes(
     StringRef Name, bool HasOptnone, bool AttrOnCallSite,
     llvm::AttrBuilder &FuncAttrs) {
   ::getTrivialDefaultFunctionAttributes(Name, HasOptnone, getCodeGenOpts(),
                                         getLangOpts(), AttrOnCallSite,
                                         FuncAttrs);
 }
 
 void CodeGenModule::getDefaultFunctionAttributes(StringRef Name,
                                                  bool HasOptnone,
                                                  bool AttrOnCallSite,
                                                  llvm::AttrBuilder &FuncAttrs) {
   getTrivialDefaultFunctionAttributes(Name, HasOptnone, AttrOnCallSite,
                                       FuncAttrs);
   // If we're just getting the default, get the default values for mergeable
   // attributes.
   if (!AttrOnCallSite)
     addMergableDefaultFunctionAttributes(CodeGenOpts, FuncAttrs);
 }
 
-/// Apply default attributes to \p F, accounting for merge semantics of
-/// attributes that should not overwrite existing attributes.
-void CodeGenModule::mergeDefaultFunctionDefinitionAttributes(
-    llvm::Function &F, bool WillInternalize) {
-  ::mergeDefaultFunctionDefinitionAttributes(F, getCodeGenOpts(), getLangOpts(),
-                                             getTarget().getTargetOpts(),
-                                             WillInternalize);
-}
-
 void CodeGenModule::addDefaultFunctionDefinitionAttributes(
     llvm::AttrBuilder &attrs) {
   getDefaultFunctionAttributes(/*function name*/ "", /*optnone*/ false,
                                /*for call*/ false, attrs);
   GetCPUAndFeaturesAttributes(GlobalDecl(), attrs);
 }
 
 static void addNoBuiltinAttributes(llvm::AttrBuilder &FuncAttrs,

clang/lib/CodeGen/CodeGenModule.h

public:
   /// contribute to the function attributes and calling convention.
   /// \param Attrs [out] - On return, the attribute list to use.
   /// \param CallingConv [out] - On return, the LLVM calling convention to use.
   void ConstructAttributeList(StringRef Name, const CGFunctionInfo &Info,
                               CGCalleeInfo CalleeInfo,
                               llvm::AttributeList &Attrs, unsigned &CallingConv,
                               bool AttrOnCallSite, bool IsThunk);
 
-  /// Adds attributes to F according to our CodeGenOptions and LangOptions, as
-  /// though we had emitted it ourselves.  We remove any attributes on F that
-  /// conflict with the attributes we add here.
-  ///
-  /// This is useful for adding attrs to bitcode modules that you want to link
-  /// with but don't control, such as CUDA's libdevice.  When linking with such
-  /// a bitcode library, you might want to set e.g. its functions'
-  /// "unsafe-fp-math" attribute to match the attr of the functions you're
-  /// codegen'ing.  Otherwise, LLVM will interpret the bitcode module's lack of
-  /// unsafe-fp-math attrs as tantamount to unsafe-fp-math=false, and then LLVM
-  /// will propagate unsafe-fp-math=false up to every transitive caller of a
-  /// function in the bitcode library!
-  ///
-  /// With the exception of fast-math attrs, this will only make the attributes
-  /// on the function more conservative.  But it's unsafe to call this on a
-  /// function which relies on particular fast-math attributes for correctness.
-  /// It's up to you to ensure that this is safe.
-  void mergeDefaultFunctionDefinitionAttributes(llvm::Function &F,
-                                                bool WillInternalize);
-
   /// Like the overload taking a `Function &`, but intended specifically
   /// for frontends that want to build on Clang's target-configuration logic.
   void addDefaultFunctionDefinitionAttributes(llvm::AttrBuilder &attrs);
 
   StringRef getMangledName(GlobalDecl GD);
   StringRef getBlockMangledName(GlobalDecl GD, const BlockDecl *BD);
   const GlobalDecl getMangledNameDecl(StringRef);