[CFLAA] Move a common function to the utils header to reduce duplication

[davide]

Summary

In the same vein as my last change. I'm not convinced about using inline in the header to force the function to live in a COMDAT, so, suggestions welcome.

[george.burgess.iv]

LGTM. Thanks!

include/llvm/Analysis/CFLAliasAnalysisUtils.h

23

Now that I look at it, this should probably live in the llvm::cflaa namespace, rather than the global namespace (unless you're planning on making this more general?) As an added bonus, that may let us get rid of the using namespace llvm above?

I'm indifferent about whether that's included in this patch, or if it's done in a follow-up.

46

I'm not convinced about using inline in the header to force the function to live in a COMDAT, so, suggestions welcome

Honestly, given how small this function will end up being and that it should only ever be used in 2 TUs, I'd be content if we just made it static inline.

I'm happy with either approach ¯\_(ツ)_/¯.

56

If we expand these namespaces to include FunctionHandle, please add // namespace cflaa and // namespace llvm to their respective close braces.

[davide]

r306354/r306355, thanks

include/llvm/Analysis/CFLAliasAnalysisUtils.h

 #ifndef LLVM_ANALYSIS_CFLALIASANALYSISUTILS_H
 #define LLVM_ANALYSIS_CFLALIASANALYSISUTILS_H
 
 #include "llvm/IR/Function.h"
 #include "llvm/IR/ValueHandle.h"
 
 using namespace llvm;
 
 template <typename AAResult> struct FunctionHandle final : public CallbackVH {
   FunctionHandle(Function *Fn, AAResult *Result)
       : CallbackVH(Fn), Result(Result) {
     assert(Fn != nullptr);
     assert(Result != nullptr);
   }
 
   void deleted() override { removeSelfFromCache(); }
   void allUsesReplacedWith(Value *) override { removeSelfFromCache(); }
 
 private:
   AAResult *Result;
 
   void removeSelfFromCache() {
     assert(Result != nullptr);
     auto *Val = getValPtr();
     Result->evict(cast<Function>(Val));
     setValPtr(nullptr);
   }
 };
 
+namespace llvm {
+namespace cflaa {
+inline const Function *parentFunctionOfValue(const Value *Val) {
+  if (auto *Inst = dyn_cast<Instruction>(Val)) {
+    auto *Bb = Inst->getParent();
+    return Bb->getParent();
+  }
+
+  if (auto *Arg = dyn_cast<Argument>(Val))
+    return Arg->getParent();
+  return nullptr;
+}
+}
+}
+
 #endif // LLVM_ANALYSIS_CFLALIASANALYSISUTILS_H

lib/Analysis/CFLAndersAliasAnalysis.cpp

 
 #define DEBUG_TYPE "cfl-anders-aa"
 
 CFLAndersAAResult::CFLAndersAAResult(const TargetLibraryInfo &TLI) : TLI(TLI) {}
 CFLAndersAAResult::CFLAndersAAResult(CFLAndersAAResult &&RHS)
     : AAResultBase(std::move(RHS)), TLI(RHS.TLI) {}
 CFLAndersAAResult::~CFLAndersAAResult() {}
 
-static const Function *parentFunctionOfValue(const Value *Val) {
-  if (auto *Inst = dyn_cast<Instruction>(Val)) {
-    auto *Bb = Inst->getParent();
-    return Bb->getParent();
-  }
-
-  if (auto *Arg = dyn_cast<Argument>(Val))
-    return Arg->getParent();
-  return nullptr;
-}
-
 namespace {
 
 enum class MatchState : uint8_t {
   // The following state represents S1 in the paper.
   FlowFromReadOnly = 0,
   // The following two states together represent S2 in the paper.
   // The 'NoReadWrite' suffix indicates that there exists an alias path that
   // does not contain assignment and reverse assignment edges.

lib/Analysis/CFLSteensAliasAnalysis.cpp

 
 //===----------------------------------------------------------------------===//
 // Function declarations that require types defined in the namespace above
 //===----------------------------------------------------------------------===//
 
 /// Determines whether it would be pointless to add the given Value to our sets.
 static bool canSkipAddingToSets(Value *Val);
 
-static Function *parentFunctionOfValue(Value *Val) {
-  if (auto *Inst = dyn_cast<Instruction>(Val)) {
-    auto *Bb = Inst->getParent();
-    return Bb->getParent();
-  }
-
-  if (auto *Arg = dyn_cast<Argument>(Val))
-    return Arg->getParent();
-  return nullptr;
-}
-
 static bool canSkipAddingToSets(Value *Val) {
   // Constants can share instances, which may falsely unify multiple
   // sets, e.g. in
   // store i32* null, i32** %ptr1
   // store i32* null, i32** %ptr2
   // clearly ptr1 and ptr2 should not be unified into the same set, so
   // we should filter out the (potentially shared) instance to
   // i32* null.
AliasResult CFLSteensAAResult::query(const MemoryLocation &LocA,
                                      const MemoryLocation &LocB) {
   auto *ValA = const_cast<Value *>(LocA.Ptr);
   auto *ValB = const_cast<Value *>(LocB.Ptr);
 
   if (!ValA->getType()->isPointerTy() || !ValB->getType()->isPointerTy())
     return NoAlias;
 
   Function *Fn = nullptr;
-  Function *MaybeFnA = parentFunctionOfValue(ValA);
-  Function *MaybeFnB = parentFunctionOfValue(ValB);
+  Function *MaybeFnA = const_cast<Function *>(parentFunctionOfValue(ValA));
+  Function *MaybeFnB = const_cast<Function *>(parentFunctionOfValue(ValB));
   if (!MaybeFnA && !MaybeFnB) {
     // The only times this is known to happen are when globals + InlineAsm are
     // involved
     DEBUG(dbgs()
           << "CFLSteensAA: could not extract parent function information.\n");
     return MayAlias;
   }