[ThinLTO] Record references to aliases to calls

[tejohnson]

Summary

When there is a call to an alias in the same module, we were not
adding either a reference or call edge. So we could incorrectly think
that the alias was dead if it was inlined in that function, despite
having a reference imported elsewhere. This resulted in unsats at
link time.

Add a reference edge when the call is to an alias, since we won't
add a call edge for it.

[davide]

LGTM. Thanks Teresa!

[mehdi_amini]

lib/Analysis/ModuleSummaryAnalysis.cpp

58

I don't understand this change. I seem to me that you're adding a reference when we're calling an alias. Why isn't the alias added as a callee?

[tejohnson]

lib/Analysis/ModuleSummaryAnalysis.cpp

58

Because there is no called function (it's null). Note we can still find the aliasee function in the ref graph via the alias summary. Maybe we should add a call edge to the alias regardless, so we could import via the alias summary...let me try that .

[mehdi_amini]

lib/Analysis/ModuleSummaryAnalysis.cpp

58

I have a doubt about what we generate (and should generate) on you example, is it:

• main reference analias
• main calls aliasee
• analias reference aliasee

Or:

• main calls analias
• analias reference aliasee

I tend to lean for the second one.

[tejohnson]

lib/Analysis/ModuleSummaryAnalysis.cpp

58

Aliases are represented explicitly in the summary, although they are not created here.

What we will generate with my new fix is the same as what we are already generating if the call to the alias was intra-module. This is simply:

• main calls analias

Later on we will generate an ALIAS record in the summary to record the relationship:

• analias aliases aliasee

(in the bitcode writer)

Then in the bitcode reader we will turn that into an AliasSummary so that we can follow the call from main to analias (recorded in the FunctionSummary for main) through to aliasee via the AliasSummmary.

[tejohnson]

Create a call edge to the alias instead of a ref edge.

[tejohnson]

lib/Analysis/ModuleSummaryAnalysis.cpp

58

Aliases are represented explicitly in the summary, although they are not created here.

Meaning the "edge" (AliasSummary) is not created here

What we will generate with my new fix is the same as what we are already generating if the call to the alias was intra-module.

I meant inter-module.

[mehdi_amini]

lib/Analysis/ModuleSummaryAnalysis.cpp

116

What if we have an alias to an unnamed function?

[tejohnson]

lib/Analysis/ModuleSummaryAnalysis.cpp

116

I thought these would all get named in the NameAnonGlobals pass?

In fact, perhaps this should be an assert that we have a name?

[mehdi_amini]

LGTM now!

Thanks.

lib/Analysis/ModuleSummaryAnalysis.cpp

116

Oh right, I forgot about that.

lib/Analysis/ModuleSummaryAnalysis.cpp

for (const auto &OI : U->operands()) {
       if (!Operand)
         continue;
       if (isa<BlockAddress>(Operand))
         continue;
       if (isa<GlobalValue>(Operand)) {
         // We have a reference to a global value. This should be added to
         // the reference set unless it is a callee. Callees are handled
         // specially by WriteFunction and are added to a separate list.
         if (!(CS && CS.isCallee(&OI)))
           RefEdges.insert(Operand);
         continue;
       }
       Worklist.push_back(Operand);
     }
   }
 }
 
for (const BasicBlock &BB : F)
     for (const Instruction &I : BB) {
       if (isa<DbgInfoIntrinsic>(I))
         continue;
       ++NumInsts;
       findRefEdges(&I, RefEdges, Visited);
       auto CS = ImmutableCallSite(&I);
       if (!CS)
         continue;
+      auto *CalledValue = CS.getCalledValue();
       auto *CalledFunction = CS.getCalledFunction();
+      // Check if this is an alias to a function. If so, get the
+      // called aliasee for the checks below.
+      if (auto *GA = dyn_cast<GlobalAlias>(CalledValue)) {
+        assert(!CalledFunction && "Expected null called function in callsite for alias");
+        CalledFunction = dyn_cast<Function>(GA->getBaseObject());
+      }
       // Check if this is a direct call to a known function.
       if (CalledFunction) {
         // Skip nameless and intrinsics.
         if (!CalledFunction->hasName() || CalledFunction->isIntrinsic())
           continue;
         auto ScaledCount = BFI ? BFI->getBlockProfileCount(&BB) : None;
+        // Use the original CalledValue, in case it was an alias. We want
+        // to record the call edge to the alias in that case. Eventually
+        // an alias summary will be created to associate the alias and
+        // aliasee.
         auto *CalleeId =
-            M.getValueSymbolTable().lookup(CalledFunction->getName());
+            M.getValueSymbolTable().lookup(CalledValue->getName());
 
         auto Hotness = ScaledCount ? getHotness(ScaledCount.getValue(), PSI)
                                    : CalleeInfo::HotnessType::Unknown;
         CallGraphEdges[CalleeId].updateHotness(Hotness);
       } else {
         const auto *CI = dyn_cast<CallInst>(&I);
         // Skip inline assembly calls.
         if (CI && CI->isInlineAsm())

test/Bitcode/thinlto-alias2.ll

+; Test to check the callgraph for call to alias in module.
+; RUN: opt -module-summary %s -o %t.o
+; RUN: llvm-bcanalyzer -dump %t.o | FileCheck %s
+
+; CHECK:       <GLOBALVAL_SUMMARY_BLOCK
+; CHECK-NEXT:    <VERSION
+; CHECK-NEXT:    <PERMODULE {{.*}} op3=0 op4=[[ALIASID:[0-9]+]]/>
+; CHECK-NEXT:    <PERMODULE {{.*}} op0=[[ALIASEEID:[0-9]+]]
+; CHECK-NEXT:    <ALIAS {{.*}} op0=[[ALIASID]] {{.*}} op2=[[ALIASEEID]]/>
+; CHECK-NEXT:  </GLOBALVAL_SUMMARY_BLOCK>
+
+; ModuleID = 'thinlto-alias2.ll'
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+target triple = "x86_64-unknown-linux-gnu"
+
+define i32 @main() {
+entry:
+    call void (...) @analias()
+    ret i32 0
+}
+
+@analias = alias void (...), bitcast (void ()* @aliasee to void (...)*)
+
+define void @aliasee() #0 {
+entry:
+    ret void
+}
+