Option to ignore llvm[.compiler].used uses in hasAddressTaken()

[madhur13490]

llvm/lib/Analysis/CallGraph.cpp

85

Typo: Ingore

llvm/lib/IR/Function.cpp

1619

Can you please add a test which has non-bitcast value of function pointers? Something like below;

@llvm.used = appending global [2 x float()*] [void()* @foo, void()* @bar]

IIRC, the above simple cast to GlobalVariable is not sufficient in this case. You need to handle it with some more extra checks.

[rampitec]

Fixed typo.

[rampitec]

llvm/lib/IR/Function.cpp

1619

Does this really happen? My understanding these arrays shall have i8* type, so a function pointer always needs a bitcast.

[rampitec]

Handle the case when function pointer is used directly.

[dexonsmith]

(Passing comment not really related to the patch, but I wonder how hard / useful it would be to convert @llvm.used and @llvm.compiler.used to attributes (or similar)...?)

[rampitec]

In D96087#2552603, @dexonsmith wrote:

(Passing comment not really related to the patch, but I wonder how hard / useful it would be to convert @llvm.used and @llvm.compiler.used to attributes (or similar)...?)

At the very least these variables are defined in the LLVM IR specification, so it would be a compatibility break.

[dexonsmith]

In D96087#2552612, @rampitec wrote:

In D96087#2552603, @dexonsmith wrote:

(Passing comment not really related to the patch, but I wonder how hard / useful it would be to convert @llvm.used and @llvm.compiler.used to attributes (or similar)...?)

At the very least these variables are defined in the LLVM IR specification, so it would be a compatibility break.

Right, it'd certainly be a semantic change; we'd need an auto-upgrade for bitcode (surely no compatibility break there) and a transition of some sort for the C API (I imagine there's a reasonable way of handling both, but maybe you're seeing some difficulty?). Probably it could / should be modelled as part of the linkage somehow, rather than an attribute, but it might not be worth a bit on Value. (But probably this isn't the right venue to discuss this; carry on...)

[madhur13490]

llvm/lib/IR/Function.cpp

1583–1584

Need to enhance this comment to accommodate new arguments.

1588

%s/Ingore/Ignore

1619

Yes, as per the spec, "This array contains a list of pointers to named global variables, functions and aliases which may optionally have a pointer cast formed of bitcast or getelementptr". So the pointer cast is optional. You can have plain function pointers in the array. The newly added code will handle only bitcast cast case but not general. You need to handle the general case and find if one of the users if GlobalVariable.

@llvm.used = appending global [2 x void()*] [void()* @foo, void()* @bar]

[rampitec]

Fixed typo.

llvm/lib/IR/Function.cpp

1619

This is already done in the previous patch. I still think that does not practically happen because you cannot have elements of different types in an array. The words "may optionally have a pointer cast" shall apply if your pointer is not of the i8* type (since this variable is not only used for functions). For sure one can artificially forge such initializer (like I did in the test), but that does not have much practical sense.

[madhur13490]

llvm/lib/IR/Function.cpp

1619

Two questions:

1. I don't understand why the below the initialization of llvm.used is invalid?

@llvm.used = appending global [2 x void()*] [void()* @foo, void()* @bar]

I don't see verifier complain about it, which make me believe that it is a valid LLVM construct. Are you saying that it is not common so you'd like to skip it?

2. Assumming that 1 is valid, does the patch succeed to determine if one of the user of function symbol is GlobalVariable?

[rampitec]

llvm/lib/IR/Function.cpp

1619

1. The initialization is not invalid, it just useless. If you have functions with different types in a module you will not be able to to initialize it. You also will be unable link different modules with such initializations.
2. Yet it works with the patch. See the test for llvm.compiler.used.

[madhur13490]

Accept with the condition that the comment is enhanced.

[rampitec]

Updated comment.

llvm/include/llvm/IR/Function.h

/// @}
   /// function, dropping all references deletes the entire body of the function,
   /// including any contained basic blocks.
   ///
   void dropAllReferences();
 
   /// hasAddressTaken - returns true if there are any uses of this function
   /// other than direct calls or invokes to it, or blockaddress expressions.
   /// Optionally passes back an offending user for diagnostic purposes,
-  /// ignores callback uses, and assume like pointer annotation calls.
+  /// ignores callback uses, assume like pointer annotation calls, and
+  /// references in llvm.used and llvm.compiler.used variables.
   ///
   bool hasAddressTaken(const User ** = nullptr,
                        bool IgnoreCallbackUses = false,
-                       bool IgnoreAssumeLikeCalls = false) const;
+                       bool IgnoreAssumeLikeCalls = false,
+                       bool IngoreLLVMUsed = false) const;
 
   /// isDefTriviallyDead - Return true if it is trivially safe to remove
   /// this function definition from the module (because it isn't externally
   /// visible, does not have its address taken, and has no callers).  To make
   /// this more accurate, call removeDeadConstantUsers first.
   bool isDefTriviallyDead() const;
 
   /// callsFunctionThatReturnsTwice - Return true if the function has a call to

llvm/lib/Analysis/CallGraph.cpp

 
 void CallGraph::addToCallGraph(Function *F) {
   CallGraphNode *Node = getOrInsertFunction(F);
 
   // If this function has external linkage or has its address taken and
   // it is not a callback, then anything could call it.
   if (!F->hasLocalLinkage() ||
       F->hasAddressTaken(nullptr, /*IgnoreCallbackUses=*/true,
-                         /* IgnoreAssumeLikeCalls */ true))
+                         /* IgnoreAssumeLikeCalls */ true,
+                         /* IgnoreLLVMUsed */ true))
     ExternalCallingNode->addCalledFunction(nullptr, Node);
 
   populateCallGraphNode(Node);
 }
 
 void CallGraph::populateCallGraphNode(CallGraphNode *Node) {
   Function *F = Node->getFunction();
 

llvm/lib/IR/Function.cpp

 #include "llvm/IR/IntrinsicsVE.h"
 #include "llvm/IR/IntrinsicsWebAssembly.h"
 #include "llvm/IR/IntrinsicsX86.h"
 #include "llvm/IR/IntrinsicsXCore.h"
 #include "llvm/IR/LLVMContext.h"
 #include "llvm/IR/MDBuilder.h"
 #include "llvm/IR/Metadata.h"
 #include "llvm/IR/Module.h"
+#include "llvm/IR/Operator.h"
 #include "llvm/IR/SymbolTableListTraits.h"
 #include "llvm/IR/Type.h"
 #include "llvm/IR/Use.h"
 #include "llvm/IR/User.h"
 #include "llvm/IR/Value.h"
 #include "llvm/IR/ValueSymbolTable.h"
 #include "llvm/Support/Casting.h"
 #include "llvm/Support/Compiler.h"
Optional<Function *> Intrinsic::remangleIntrinsicFunction(Function *F) {
   NewDecl->setCallingConv(F->getCallingConv());
   assert(NewDecl->getFunctionType() == F->getFunctionType() &&
          "Shouldn't change the signature");
   return NewDecl;
 }
 
 /// hasAddressTaken - returns true if there are any uses of this function
 /// other than direct calls or invokes to it. Optionally ignores callback
-/// uses and assume like pointer annotation calls.
+/// uses, assume like pointer annotation calls, and references in llvm.used
+/// and llvm.compiler.used variables.
 bool Function::hasAddressTaken(const User **PutOffender,
                                bool IgnoreCallbackUses,
-                               bool IgnoreAssumeLikeCalls) const {
+                               bool IgnoreAssumeLikeCalls,
+                               bool IgnoreLLVMUsed) const {
   for (const Use &U : uses()) {
     const User *FU = U.getUser();
     if (isa<BlockAddress>(FU))
       continue;
 
     if (IgnoreCallbackUses) {
       AbstractCallSite ACS(&U);
       if (ACS && ACS.isCallbackCall())
         continue;
     }
 
     const auto *Call = dyn_cast<CallBase>(FU);
     if (!Call) {
       if (IgnoreAssumeLikeCalls) {
         if (const auto *FI = dyn_cast<Instruction>(FU)) {
           if (FI->isCast() && !FI->user_empty() &&
               llvm::all_of(FU->users(), [](const User *U) {
                 if (const auto *I = dyn_cast<IntrinsicInst>(U))
                   return I->isAssumeLikeIntrinsic();
                 return false;
               }))
             continue;
         }
       }
+      if (IgnoreLLVMUsed && !FU->user_empty()) {
+        const User *FUU = FU;
+        if (isa<BitCastOperator>(FU) && FU->hasOneUse() &&
+            !FU->user_begin()->user_empty())
+          FUU = *FU->user_begin();
+        if (llvm::all_of(FUU->users(), [](const User *U) {
+              if (const auto *GV = dyn_cast<GlobalVariable>(U))
+                return GV->hasName() &&
+                       (GV->getName().equals("llvm.compiler.used") ||
+                        GV->getName().equals("llvm.used"));
+              return false;
+            }))
+          continue;
+      }
       if (PutOffender)
         *PutOffender = FU;
       return true;
     }
     if (!Call->isCallee(&U)) {
       if (PutOffender)
         *PutOffender = FU;
       return true;

llvm/test/Analysis/CallGraph/ignore-llvm-used.ll

+; RUN: opt < %s -print-callgraph -disable-output 2>&1 | FileCheck %s
+; CHECK: Call graph node <<null function>><<{{.*}}>>  #uses=0
+; CHECK-NEXT:  CS<None> calls function 'unused'
+; CHECK-EMPTY:
+; CHECK-NEXT:   Call graph node for function: 'unused'<<{{.*}}>>  #uses=1
+; CHECK-EMPTY:
+; CHECK-NEXT:   Call graph node for function: 'used1'<<{{.*}}>>  #uses=0
+; CHECK-EMPTY:
+; CHECK-NEXT:   Call graph node for function: 'used2'<<{{.*}}>>  #uses=0
+; CHECK-EMPTY:
+
[e-mail] = appending global [1 x i8*] [i8* bitcast (void ()* @used1 to i8*)]
[e-mail] = appending global [1 x void()*] [void ()* @used2]
+@array = appending global [1 x i8*] [i8* bitcast (void ()* @unused to i8*)]
+
+define internal void @used1() {
+entry:
+  ret void
+}
+
+define internal void @used2() {
+entry:
+  ret void
+}
+
+define internal void @unused() {
+entry:
+  ret void
+}