[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 3461c692f3a3e8f7109d3d0661d145735f802718..9413b39978e3fb36e931082587b66431f87c1253 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 e9d39a606b7f15e9cfef5be72a9149bf9e2cfb10..9bb7c86d26caf4729735d3f0c1cb06c65647d9bc 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 8a5242ec1ffc09d8f1a054ab25c49b0b990b8661..58ccf2bd664be0d72671de848fd292dd4bbf3b3d 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 669464b88b5d8e037d48829271b489d10729da51..d66f1e2617801ade91616e4bfc1be0f6a5d3bdec 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 9947ef602be0f6f4f1a119d2c3918f89c52931cd..fcda674fd2542b157ea3627706827f71285ad522 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 06918236bd87094a1e0e7ec26b2c693c4e330f5a..694e805a8fc154e4f3816b75ffa1725a404a30d4 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 bdf6d3a07ca9b9299c106a8a07130d902a7761b3..658d73804c174799e64e4d956de10cec8463c6b9 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 a6744932b07beccee4ece5175b536bf90983ba0d..2b6969bc3c847b7f17ee1e41d2663bff1c75f5e2 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 19e490718231d7dbbf0666a4e7b9b8b299a9bc23..999a4249eefe69008d3097833c051767e97e670f 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