[llvm] 1041053 - [CallGraph][FIX] Ensure generic intrinsics are represented in the CG
Johannes Doerfert via llvm-commits
llvm-commits at lists.llvm.org
Tue Jan 10 11:39:32 PST 2023
Author: Johannes Doerfert
Date: 2023-01-10T11:38:58-08:00
New Revision: 10410534696e1922cbed56b229d123b2db4acd8e
URL: https://github.com/llvm/llvm-project/commit/10410534696e1922cbed56b229d123b2db4acd8e
DIFF: https://github.com/llvm/llvm-project/commit/10410534696e1922cbed56b229d123b2db4acd8e.diff
LOG: [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
Added:
Modified:
llvm/include/llvm/Analysis/CallGraph.h
llvm/include/llvm/IR/Intrinsics.h
llvm/lib/Analysis/CallGraph.cpp
llvm/lib/Analysis/CallGraphSCCPass.cpp
llvm/lib/Analysis/GlobalsModRef.cpp
llvm/lib/IR/Function.cpp
llvm/test/Analysis/CallGraph/ignore-assumelike-calls.ll
llvm/test/Analysis/CallGraph/non-leaf-intrinsics.ll
llvm/test/Transforms/GVN/intrinsics_in_cg.ll
Removed:
################################################################################
diff --git a/llvm/include/llvm/Analysis/CallGraph.h b/llvm/include/llvm/Analysis/CallGraph.h
index 3461c692f3a3..9413b39978e3 100644
--- a/llvm/include/llvm/Analysis/CallGraph.h
+++ b/llvm/include/llvm/Analysis/CallGraph.h
@@ -240,9 +240,6 @@ class CallGraphNode {
/// 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 e9d39a606b7f..9bb7c86d26ca 100644
--- 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 8a5242ec1ffc..58ccf2bd664b 100644
--- 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 669464b88b5d..d66f1e261780 100644
--- 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 9947ef602be0..fcda674fd254 100644
--- 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 06918236bd87..694e805a8fc1 100644
--- 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 bdf6d3a07ca9..658d73804c17 100644
--- 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 a6744932b07b..2b6969bc3c84 100644
--- 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 19e490718231..999a4249eefe 100644
--- 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
More information about the llvm-commits
mailing list