[llvm] 209496b - [Core] Allow `hasAddressTaken` to ignore "casted direct calls"

[Johannes Doerfert via llvm-commits]

Author: Johannes Doerfert
Date: 2023-08-31T19:32:52-07:00
New Revision: 209496b766bb08f43155d38dc8015804fb6c0c96

URL: https://github.com/llvm/llvm-project/commit/209496b766bb08f43155d38dc8015804fb6c0c96
DIFF: https://github.com/llvm/llvm-project/commit/209496b766bb08f43155d38dc8015804fb6c0c96.diff

LOG: [Core] Allow `hasAddressTaken` to ignore "casted direct calls"

A direct call to a function casted to a different type is still not
really an address taken event. We allow the user to opt out of these
now.

Reviewed By: arsenm

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

Added: 
    

Modified: 
    llvm/include/llvm/IR/Function.h
    llvm/lib/IR/Function.cpp
    llvm/lib/Transforms/IPO/Attributor.cpp
    llvm/test/Transforms/Attributor/callgraph.ll

Removed: 
    


################################################################################
diff  --git a/llvm/include/llvm/IR/Function.h b/llvm/include/llvm/IR/Function.h
index 93cf0d27e9a73e..0feb536620e599 100644
--- a/llvm/include/llvm/IR/Function.h
+++ b/llvm/include/llvm/IR/Function.h
@@ -888,13 +888,14 @@ class LLVM_EXTERNAL_VISIBILITY Function : public GlobalObject,
   /// other than direct calls or invokes to it, or blockaddress expressions.
   /// Optionally passes back an offending user for diagnostic purposes,
   /// ignores callback uses, assume like pointer annotation calls, references in
-  /// llvm.used and llvm.compiler.used variables, and operand bundle
-  /// "clang.arc.attachedcall".
-  bool hasAddressTaken(const User ** = nullptr,
-                       bool IgnoreCallbackUses = false,
+  /// llvm.used and llvm.compiler.used variables, operand bundle
+  /// "clang.arc.attachedcall", and direct calls with a 
diff erent call site
+  /// signature (the function is implicitly casted).
+  bool hasAddressTaken(const User ** = nullptr, bool IgnoreCallbackUses = false,
                        bool IgnoreAssumeLikeCalls = true,
                        bool IngoreLLVMUsed = false,
-                       bool IgnoreARCAttachedCall = false) const;
+                       bool IgnoreARCAttachedCall = false,
+                       bool IgnoreCastedDirectCall = false) const;
 
   /// isDefTriviallyDead - Return true if it is trivially safe to remove
   /// this function definition from the module (because it isn't externally

diff  --git a/llvm/lib/IR/Function.cpp b/llvm/lib/IR/Function.cpp
index a9beb0be3de0a4..b1b8404157c3b2 100644
--- a/llvm/lib/IR/Function.cpp
+++ b/llvm/lib/IR/Function.cpp
@@ -1752,7 +1752,8 @@ std::optional<Function *> Intrinsic::remangleIntrinsicFunction(Function *F) {
 bool Function::hasAddressTaken(const User **PutOffender,
                                bool IgnoreCallbackUses,
                                bool IgnoreAssumeLikeCalls, bool IgnoreLLVMUsed,
-                               bool IgnoreARCAttachedCall) const {
+                               bool IgnoreARCAttachedCall,
+                               bool IgnoreCastedDirectCall) const {
   for (const Use &U : uses()) {
     const User *FU = U.getUser();
     if (isa<BlockAddress>(FU))
@@ -1801,7 +1802,8 @@ bool Function::hasAddressTaken(const User **PutOffender,
           continue;
     }
 
-    if (!Call->isCallee(&U) || Call->getFunctionType() != getFunctionType()) {
+    if (!Call->isCallee(&U) || (!IgnoreCastedDirectCall &&
+                                Call->getFunctionType() != getFunctionType())) {
       if (IgnoreARCAttachedCall &&
           Call->isOperandBundleOfType(LLVMContext::OB_clang_arc_attachedcall,
                                       U.getOperandNo()))

diff  --git a/llvm/lib/Transforms/IPO/Attributor.cpp b/llvm/lib/Transforms/IPO/Attributor.cpp
index 86a9f22ea32e2f..029f314b96901c 100644
--- a/llvm/lib/Transforms/IPO/Attributor.cpp
+++ b/llvm/lib/Transforms/IPO/Attributor.cpp
@@ -1072,7 +1072,9 @@ Attributor::Attributor(SetVector<Function *> &Functions,
     if (Fn->hasAddressTaken(/*PutOffender=*/nullptr,
                             /*IgnoreCallbackUses=*/false,
                             /*IgnoreAssumeLikeCalls=*/true,
-                            /*IgnoreLLVMUsed=*/true))
+                            /*IgnoreLLVMUsed=*/true,
+                            /*IgnoreARCAttachedCall=*/false,
+                            /*IgnoreCastedDirectCall=*/true))
       InfoCache.IndirectlyCallableFunctions.push_back(Fn);
 }
 

diff  --git a/llvm/test/Transforms/Attributor/callgraph.ll b/llvm/test/Transforms/Attributor/callgraph.ll
index 547137046ca93f..08a294318f8bcc 100644
--- a/llvm/test/Transforms/Attributor/callgraph.ll
+++ b/llvm/test/Transforms/Attributor/callgraph.ll
@@ -459,6 +459,19 @@ define i32 @non_matching_unknown(i1 %c, ptr %fn) {
   ret i32 %call
 }
 
+; This function is used in a "direct" call but with a 
diff erent signature.
+; We check that it does not show up above in any of the if-cascades because
+; the address is not actually taken.
+declare void @usedOnlyInCastedDirectCall(i32)
+define void @usedOnlyInCastedDirectCallCaller() {
+; CHECK-LABEL: @usedOnlyInCastedDirectCallCaller(
+; CHECK-NEXT:    call void @usedOnlyInCastedDirectCall()
+; CHECK-NEXT:    ret void
+;
+  call void @usedOnlyInCastedDirectCall()
+  ret void
+}
+
 define void @broker(ptr %unknown) !callback !0 {
 ; OWRDL-LABEL: @broker(
 ; OWRDL-NEXT:    call void [[UNKNOWN:%.*]]()