[CallGraph][FIX] Ensure generic intrinsics are represented in the CG

Intrinsics have historically been excluded from the call graph with an
exception of 3 special ones added at some point. This meant that passes
depending on the call graph needed to handle intrinsics explicitly as
the underlying assumption, namely that intrinsics can't call or modify
things, doesn't hold. We are slowly moving away from special handling of
intrinsics, or at least towards explicitly checking what intrinsics we
want to handle differently.

This patch:
 - Includes most intrinsics in the call graph. Debug intrinsics are
   still excluded.
 - Removes the special handling of intrinsics in the GlobalsAA pass.
 - Removes the `IntrinsicInst::isLeaf` method.

Properly
Fixes: https://github.com/llvm/llvm-project/issues/52706

See also:
https://discourse.llvm.org/t/intrinsics-are-not-special-stop-pretending-i-mean-it/67545

Differential Revision: https://reviews.llvm.org/D14119

diff --git a/llvm/include/llvm/Analysis/CallGraph.h b/llvm/include/llvm/Analysis/CallGraph.h
index 3461c69..9413b39 100644 (file)
--- a/llvm/include/llvm/Analysis/CallGraph.h
+++ b/llvm/include/llvm/Analysis/CallGraph.h
@@ -240,9 +240,6 @@ public:
 
   /// Adds a function to the list of functions called by this one.
   void addCalledFunction(CallBase *Call, CallGraphNode *M) {
-    assert(!Call || !Call->getCalledFunction() ||
-           !Call->getCalledFunction()->isIntrinsic() ||
-           !Intrinsic::isLeaf(Call->getCalledFunction()->getIntrinsicID()));
     CalledFunctions.emplace_back(Call ? std::optional<WeakTrackingVH>(Call)
                                       : std::optional<WeakTrackingVH>(),
                                  M);

diff --git a/llvm/include/llvm/IR/Intrinsics.h b/llvm/include/llvm/IR/Intrinsics.h
index e9d39a6..9bb7c86 100644 (file)
--- a/llvm/include/llvm/IR/Intrinsics.h
+++ b/llvm/include/llvm/IR/Intrinsics.h
@@ -79,11 +79,6 @@ namespace Intrinsic {
   /// Returns true if the intrinsic can be overloaded.
   bool isOverloaded(ID id);
 
-  /// Returns true if the intrinsic is a leaf, i.e. it does not make any calls
-  /// itself.  Most intrinsics are leafs, the exceptions being the patchpoint
-  /// and statepoint intrinsics. These call (or invoke) their "target" argument.
-  bool isLeaf(ID id);
-
   /// Return the attributes for an intrinsic.
   AttributeList getAttributes(LLVMContext &C, ID id);
 

diff --git a/llvm/lib/Analysis/CallGraph.cpp b/llvm/lib/Analysis/CallGraph.cpp
index 8a5242e..58ccf2b 100644 (file)
--- a/llvm/lib/Analysis/CallGraph.cpp
+++ b/llvm/lib/Analysis/CallGraph.cpp
@@ -14,7 +14,6 @@
 #include "llvm/IR/AbstractCallSite.h"
 #include "llvm/IR/Function.h"
 #include "llvm/IR/IntrinsicInst.h"
-#include "llvm/IR/Intrinsics.h"
 #include "llvm/IR/Module.h"
 #include "llvm/IR/PassManager.h"
 #include "llvm/InitializePasses.h"
@@ -92,7 +91,7 @@ void CallGraph::populateCallGraphNode(CallGraphNode *Node) {
 
   // If this function is not defined in this translation unit, it could call
   // anything.
-  if (F->isDeclaration() && !F->isIntrinsic())
+  if (F->isDeclaration() && !F->hasFnAttribute(Attribute::NoCallback))
     Node->addCalledFunction(nullptr, CallsExternalNode.get());
 
   // Look for calls by this function.
@@ -100,12 +99,9 @@ void CallGraph::populateCallGraphNode(CallGraphNode *Node) {
     for (Instruction &I : BB) {
       if (auto *Call = dyn_cast<CallBase>(&I)) {
         const Function *Callee = Call->getCalledFunction();
-        if (!Callee || !Intrinsic::isLeaf(Callee->getIntrinsicID()))
-          // Indirect calls of intrinsics are not allowed so no need to check.
-          // We can be more precise here by using TargetArg returned by
-          // Intrinsic::isLeaf.
+        if (!Callee)
           Node->addCalledFunction(Call, CallsExternalNode.get());
-        else if (!Callee->isIntrinsic())
+        else if (!isDbgInfoIntrinsic(Callee->getIntrinsicID()))
           Node->addCalledFunction(Call, getOrInsertFunction(Callee));
 
         // Add reference to callback functions.

diff --git a/llvm/lib/Analysis/CallGraphSCCPass.cpp b/llvm/lib/Analysis/CallGraphSCCPass.cpp
index 669464b..d66f1e2 100644 (file)
--- a/llvm/lib/Analysis/CallGraphSCCPass.cpp
+++ b/llvm/lib/Analysis/CallGraphSCCPass.cpp
@@ -267,15 +267,7 @@ bool CGPassManager::RefreshCallGraph(const CallGraphSCC &CurSCC, CallGraph &CG,
           // If we've already seen this call site, then the FunctionPass RAUW'd
           // one call with another, which resulted in two "uses" in the edge
           // list of the same call.
-          Calls.count(Call) ||
-
-          // If the call edge is not from a call or invoke, or it is a
-          // intrinsic call, then the function pass RAUW'd a call with
-          // another value. This can happen when constant folding happens
-          // of well known functions etc.
-          (Call->getCalledFunction() &&
-           Call->getCalledFunction()->isIntrinsic() &&
-           Intrinsic::isLeaf(Call->getCalledFunction()->getIntrinsicID()))) {
+          Calls.count(Call)) {
         assert(!CheckingMode &&
                "CallGraphSCCPass did not update the CallGraph correctly!");
 

diff --git a/llvm/lib/Analysis/GlobalsModRef.cpp b/llvm/lib/Analysis/GlobalsModRef.cpp
index 9947ef6..fcda674 100644 (file)
--- a/llvm/lib/Analysis/GlobalsModRef.cpp
+++ b/llvm/lib/Analysis/GlobalsModRef.cpp
@@ -23,7 +23,6 @@
 #include "llvm/Analysis/ValueTracking.h"
 #include "llvm/IR/InstIterator.h"
 #include "llvm/IR/Instructions.h"
-#include "llvm/IR/IntrinsicInst.h"
 #include "llvm/IR/Module.h"
 #include "llvm/IR/PassManager.h"
 #include "llvm/InitializePasses.h"
@@ -593,20 +592,8 @@ void GlobalsAAResult::AnalyzeCallGraph(CallGraph &CG, Module &M) {
 
         // We handle calls specially because the graph-relevant aspects are
         // handled above.
-        if (auto *Call = dyn_cast<CallBase>(&I)) {
-          if (Function *Callee = Call->getCalledFunction()) {
-            // The callgraph doesn't include intrinsic calls.
-            if (Callee->isIntrinsic()) {
-              if (isa<DbgInfoIntrinsic>(Call))
-                // Don't let dbg intrinsics affect alias info.
-                continue;
-
-              MemoryEffects Behaviour = AAResultBase::getMemoryEffects(Callee);
-              FI.addModRefInfo(Behaviour.getModRef());
-            }
-          }
+        if (auto *Call = dyn_cast<CallBase>(&I))
           continue;
-        }
 
         // All non-call instructions we use the primary predicates for whether
         // they read or write memory.

diff --git a/llvm/lib/IR/Function.cpp b/llvm/lib/IR/Function.cpp
index 0691823..694e805 100644 (file)
--- a/llvm/lib/IR/Function.cpp
+++ b/llvm/lib/IR/Function.cpp
@@ -1499,18 +1499,6 @@ bool Intrinsic::isOverloaded(ID id) {
 #undef GET_INTRINSIC_OVERLOAD_TABLE
 }
 
-bool Intrinsic::isLeaf(ID id) {
-  switch (id) {
-  default:
-    return true;
-
-  case Intrinsic::experimental_gc_statepoint:
-  case Intrinsic::experimental_patchpoint_void:
-  case Intrinsic::experimental_patchpoint_i64:
-    return false;
-  }
-}
-
 /// This defines the "Intrinsic::getAttributes(ID id)" method.
 #define GET_INTRINSIC_ATTRIBUTES
 #include "llvm/IR/IntrinsicImpl.inc"

diff --git a/llvm/test/Analysis/CallGraph/ignore-assumelike-calls.ll b/llvm/test/Analysis/CallGraph/ignore-assumelike-calls.ll
index bdf6d3a..658d738 100644 (file)
--- a/llvm/test/Analysis/CallGraph/ignore-assumelike-calls.ll
+++ b/llvm/test/Analysis/CallGraph/ignore-assumelike-calls.ll
@@ -10,19 +10,23 @@
 ; CHECK-EMPTY:
 ; CHECK-NEXT:   Call graph node for function: 'bitcast_only'<<{{.*}}>>  #uses=0
 ; CHECK-EMPTY:
-; CHECK-NEXT:   Call graph node for function: 'llvm.lifetime.start.p0'<<{{.*}}>>  #uses=1
+; CHECK-NEXT:   Call graph node for function: 'llvm.lifetime.start.p0'<<{{.*}}>>  #uses=3
 ; CHECK-EMPTY:
-; CHECK-NEXT:   Call graph node for function: 'llvm.memset.p0.i64'<<{{.*}}>>  #uses=1
+; CHECK-NEXT:   Call graph node for function: 'llvm.memset.p0.i64'<<{{.*}}>>  #uses=2
 ; CHECK-EMPTY:
-; CHECK-NEXT:   Call graph node for function: 'llvm.memset.p1.i64'<<{{.*}}>>  #uses=1
+; CHECK-NEXT:   Call graph node for function: 'llvm.memset.p1.i64'<<{{.*}}>>  #uses=2
 ; CHECK-EMPTY:
 ; CHECK-NEXT:   Call graph node for function: 'other_cast_intrinsic_use'<<{{.*}}>>  #uses=1
+; CHECK-NEXT:   CS<{{.*}}> calls function 'llvm.memset.p1.i64'
 ; CHECK-EMPTY:
 ; CHECK-NEXT:   Call graph node for function: 'other_intrinsic_use'<<{{.*}}>>  #uses=1
+; CHECK-NEXT:   CS<{{.*}}> calls function 'llvm.memset.p0.i64'
 ; CHECK-EMPTY:
 ; CHECK-NEXT:   Call graph node for function: 'used_by_lifetime'<<{{.*}}>>  #uses=0
+; CHECK-NEXT:   CS<{{.*}}> calls function 'llvm.lifetime.start.p0'
 ; CHECK-EMPTY:
 ; CHECK-NEXT:   Call graph node for function: 'used_by_lifetime_cast'<<{{.*}}>>  #uses=0
+; CHECK-NEXT:   CS<{{.*}}> calls function 'llvm.lifetime.start.p0'
 ; CHECK-EMPTY:
 
 define internal void @used_by_lifetime() {

diff --git a/llvm/test/Analysis/CallGraph/non-leaf-intrinsics.ll b/llvm/test/Analysis/CallGraph/non-leaf-intrinsics.ll
index a674493..2b6969b 100644 (file)
--- a/llvm/test/Analysis/CallGraph/non-leaf-intrinsics.ll
+++ b/llvm/test/Analysis/CallGraph/non-leaf-intrinsics.ll
@@ -25,7 +25,13 @@ entry:
 ; CHECK:  CS<None> calls function 'f'
 
 ; CHECK: Call graph node for function: 'calls_patchpoint'
-; CHECK-NEXT:  CS<[[addr_1:[^>]+]]> calls external node
+; CS<{{.*}}> calls function 'llvm.experimental.patchpoint.void'
 
 ; CHECK: Call graph node for function: 'calls_statepoint'
-; CHECK-NEXT:  CS<[[addr_0:[^>]+]]> calls external node
+; CS<{{.*}}> calls function 'llvm.experimental.gc.statepoint.p0'
+
+; CHECK: Call graph node for function: 'llvm.experimental.gc.statepoint.p0'<<{{.*}}>>  #uses=2
+; CHECK-NEXT:  CS<[[addr_1:[^>]+]]> calls external node
+
+; CHECK: Call graph node for function: 'llvm.experimental.patchpoint.void'<<{{.*}}>>  #uses=2
+; CHECK-NEXT:  CS<[[addr_1:[^>]+]]> calls external node

diff --git a/llvm/test/Transforms/GVN/intrinsics_in_cg.ll b/llvm/test/Transforms/GVN/intrinsics_in_cg.ll
index 19e4907..999a424 100644 (file)
--- a/llvm/test/Transforms/GVN/intrinsics_in_cg.ll
+++ b/llvm/test/Transforms/GVN/intrinsics_in_cg.ll
@@ -2,7 +2,6 @@
 ; RUN: opt < %s -passes='require<globals-aa>,gvn' -S | FileCheck %s
 
 ; Ensure we do not hoist the load over the call.
-; FIXME: Currently broken until D141190 or similar lands.
 
 @G1 = internal global i32 1
 @G2 = internal global i32 1
@@ -32,16 +31,13 @@ check:
 define i32 @indirect_intrinsic(i1 %c) {
 ; CHECK-LABEL: define {{[^@]+}}@indirect_intrinsic
 ; CHECK-SAME: (i1 [[C:%.*]]) {
-; CHECK-NEXT:    br i1 [[C]], label [[INIT:%.*]], label [[DOTCHECK_CRIT_EDGE:%.*]]
-; CHECK:       .check_crit_edge:
-; CHECK-NEXT:    [[V_PRE:%.*]] = load i32, ptr @G2, align 4
-; CHECK-NEXT:    br label [[CHECK:%.*]]
+; CHECK-NEXT:    br i1 [[C]], label [[INIT:%.*]], label [[CHECK:%.*]]
 ; CHECK:       init:
 ; CHECK-NEXT:    store i32 0, ptr @G2, align 4
 ; CHECK-NEXT:    br label [[CHECK]]
 ; CHECK:       check:
-; CHECK-NEXT:    [[V:%.*]] = phi i32 [ [[V_PRE]], [[DOTCHECK_CRIT_EDGE]] ], [ 0, [[INIT]] ]
 ; CHECK-NEXT:    call void @intrinsic_caller()
+; CHECK-NEXT:    [[V:%.*]] = load i32, ptr @G2, align 4
 ; CHECK-NEXT:    ret i32 [[V]]
 ;
   br i1 %c, label %init, label %check