WholeProgramDevirt: Examine the function body when deciding whether functions are readnone.

[pcc]

Summary

The goal is to get an analysis result even for de-refineable functions.
The justification is the same as for D29701.

Depends on D29695

Depends on D29782

[pcc]

• Refresh

[tejohnson]

LGTM, one suggestion below

llvm/test/Transforms/WholeProgramDevirt/vcp-accesses-memory.ll

46 ↗

(On Diff #88633)

Can you add comments about what these different functions are testing?

llvm/trunk/lib/Transforms/IPO/WholeProgramDevirt.cpp

 #include "llvm/Transforms/IPO/WholeProgramDevirt.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/DenseMapInfo.h"
 #include "llvm/ADT/DenseSet.h"
 #include "llvm/ADT/iterator_range.h"
 #include "llvm/ADT/MapVector.h"
 #include "llvm/ADT/SmallVector.h"
+#include "llvm/Analysis/AliasAnalysis.h"
+#include "llvm/Analysis/BasicAliasAnalysis.h"
 #include "llvm/Analysis/TypeMetadataUtils.h"
 #include "llvm/IR/CallSite.h"
 #include "llvm/IR/Constants.h"
 #include "llvm/IR/DataLayout.h"
 #include "llvm/IR/DebugInfoMetadata.h"
 #include "llvm/IR/DebugLoc.h"
 #include "llvm/IR/DerivedTypes.h"
 #include "llvm/IR/DiagnosticInfo.h"

 #include "llvm/Pass.h"
 #include "llvm/PassRegistry.h"
 #include "llvm/PassSupport.h"
 #include "llvm/Support/Casting.h"
 #include "llvm/Support/Error.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/MathExtras.h"
 #include "llvm/Transforms/IPO.h"
+#include "llvm/Transforms/IPO/FunctionAttrs.h"
 #include "llvm/Transforms/Utils/Evaluator.h"
 #include <algorithm>
 #include <cstddef>
 #include <map>
 #include <set>
 #include <string>
 
 using namespace llvm;

 
 void VTableSlotInfo::addCallSite(Value *VTable, CallSite CS,
                                  unsigned *NumUnsafeUses) {
   findCallSiteInfo(CS).CallSites.push_back({VTable, CS, NumUnsafeUses});
 }
 
 struct DevirtModule {
   Module &M;
+  function_ref<AAResults &(Function &)> AARGetter;
 
   PassSummaryAction Action;
   ModuleSummaryIndex *Summary;
 
   IntegerType *Int8Ty;
   PointerType *Int8PtrTy;
   IntegerType *Int32Ty;
   IntegerType *Int64Ty;
 
   bool RemarksEnabled;
 
   MapVector<VTableSlot, VTableSlotInfo> CallSlots;
 
   // This map keeps track of the number of "unsafe" uses of a loaded function
   // pointer. The key is the associated llvm.type.test intrinsic call generated
   // by this pass. An unsafe use is one that calls the loaded function pointer
   // directly. Every time we eliminate an unsafe use (for example, by
   // devirtualizing it or by applying virtual constant propagation), we
   // decrement the value stored in this map. If a value reaches zero, we can
   // eliminate the type check by RAUWing the associated llvm.type.test call with
   // true.
   std::map<CallInst *, unsigned> NumUnsafeUsesForTypeTest;
 
-  DevirtModule(Module &M, PassSummaryAction Action, ModuleSummaryIndex *Summary)
-      : M(M), Action(Action), Summary(Summary),
+  DevirtModule(Module &M, function_ref<AAResults &(Function &)> AARGetter,
+               PassSummaryAction Action, ModuleSummaryIndex *Summary)
+      : M(M), AARGetter(AARGetter), Action(Action), Summary(Summary),
         Int8Ty(Type::getInt8Ty(M.getContext())),
         Int8PtrTy(Type::getInt8PtrTy(M.getContext())),
         Int32Ty(Type::getInt32Ty(M.getContext())),
         Int64Ty(Type::getInt64Ty(M.getContext())),
         RemarksEnabled(areRemarksEnabled()) {}
 
   bool areRemarksEnabled();
 
bool tryVirtualConstProp(MutableArrayRef<VirtualCallTarget> TargetsForSlot,
                            VTableSlotInfo &SlotInfo);
 
   void rebuildGlobal(VTableBits &B);
 
   bool run();
 
   // Lower the module using the action and summary passed as command line
   // arguments. For testing purposes only.
-  static bool runForTesting(Module &M);
+  static bool runForTesting(Module &M,
+                            function_ref<AAResults &(Function &)> AARGetter);
 };
 
 struct WholeProgramDevirt : public ModulePass {
   static char ID;
 
   bool UseCommandLine = false;
 
   PassSummaryAction Action;
   ModuleSummaryIndex *Summary;
 
   WholeProgramDevirt() : ModulePass(ID), UseCommandLine(true) {
     initializeWholeProgramDevirtPass(*PassRegistry::getPassRegistry());
   }
 
   WholeProgramDevirt(PassSummaryAction Action, ModuleSummaryIndex *Summary)
       : ModulePass(ID), Action(Action), Summary(Summary) {
     initializeWholeProgramDevirtPass(*PassRegistry::getPassRegistry());
   }
 
   bool runOnModule(Module &M) override {
     if (skipModule(M))
       return false;
     if (UseCommandLine)
-      return DevirtModule::runForTesting(M);
-    return DevirtModule(M, Action, Summary).run();
+      return DevirtModule::runForTesting(M, LegacyAARGetter(*this));
+    return DevirtModule(M, LegacyAARGetter(*this), Action, Summary).run();
+  }
+
+  void getAnalysisUsage(AnalysisUsage &AU) const override {
+    AU.addRequired<AssumptionCacheTracker>();
+    AU.addRequired<TargetLibraryInfoWrapperPass>();
   }
 };
 
 } // end anonymous namespace
 
-INITIALIZE_PASS(WholeProgramDevirt, "wholeprogramdevirt",
+INITIALIZE_PASS_BEGIN(WholeProgramDevirt, "wholeprogramdevirt",
+                      "Whole program devirtualization", false, false)
+INITIALIZE_PASS_DEPENDENCY(AssumptionCacheTracker)
+INITIALIZE_PASS_DEPENDENCY(TargetLibraryInfoWrapperPass)
+INITIALIZE_PASS_END(WholeProgramDevirt, "wholeprogramdevirt",
                     "Whole program devirtualization", false, false)
 char WholeProgramDevirt::ID = 0;
 
 ModulePass *llvm::createWholeProgramDevirtPass(PassSummaryAction Action,
                                                ModuleSummaryIndex *Summary) {
   return new WholeProgramDevirt(Action, Summary);
 }
 
 PreservedAnalyses WholeProgramDevirtPass::run(Module &M,
-                                              ModuleAnalysisManager &) {
-  if (!DevirtModule(M, PassSummaryAction::None, nullptr).run())
+                                              ModuleAnalysisManager &AM) {
+  auto &FAM = AM.getResult<FunctionAnalysisManagerModuleProxy>(M).getManager();
+  auto AARGetter = [&](Function &F) -> AAResults & {
+    return FAM.getResult<AAManager>(F);
+  };
+  if (!DevirtModule(M, AARGetter, PassSummaryAction::None, nullptr).run())
     return PreservedAnalyses::all();
   return PreservedAnalyses::none();
 }
 
-bool DevirtModule::runForTesting(Module &M) {
+bool DevirtModule::runForTesting(
+    Module &M, function_ref<AAResults &(Function &)> AARGetter) {
   ModuleSummaryIndex Summary;
 
   // Handle the command-line summary arguments. This code is for testing
   // purposes only, so we handle errors directly.
   if (!ClReadSummary.empty()) {
     ExitOnError ExitOnErr("-wholeprogramdevirt-read-summary: " + ClReadSummary +
                           ": ");
     auto ReadSummaryFile =
         ExitOnErr(errorOrToExpected(MemoryBuffer::getFile(ClReadSummary)));
 
     yaml::Input In(ReadSummaryFile->getBuffer());
     In >> Summary;
     ExitOnErr(errorCodeToError(In.error()));
   }
 
-  bool Changed = DevirtModule(M, ClSummaryAction, &Summary).run();
+  bool Changed = DevirtModule(M, AARGetter, ClSummaryAction, &Summary).run();
 
   if (!ClWriteSummary.empty()) {
     ExitOnError ExitOnErr(
         "-wholeprogramdevirt-write-summary: " + ClWriteSummary + ": ");
     std::error_code EC;
     raw_fd_ostream OS(ClWriteSummary, EC, sys::fs::F_Text);
     ExitOnErr(errorCodeToError(EC));
 
if (!RetType)
     return false;
   unsigned BitWidth = RetType->getBitWidth();
   if (BitWidth > 64)
     return false;
 
   // Make sure that each function is defined, does not access memory, takes at
   // least one argument, does not use its first argument (which we assume is
   // 'this'), and has the same return type.
+  //
+  // Note that we test whether this copy of the function is readnone, rather
+  // than testing function attributes, which must hold for any copy of the
+  // function, even a less optimized version substituted at link time. This is
+  // sound because the virtual constant propagation optimizations effectively
+  // inline all implementations of the virtual function into each call site,
+  // rather than using function attributes to perform local optimization.
   for (VirtualCallTarget &Target : TargetsForSlot) {
-    if (Target.Fn->isDeclaration() || !Target.Fn->doesNotAccessMemory() ||
+    if (Target.Fn->isDeclaration() ||
+        computeFunctionBodyMemoryAccess(*Target.Fn, AARGetter(*Target.Fn)) !=
+            MAK_ReadNone ||
         Target.Fn->arg_empty() || !Target.Fn->arg_begin()->use_empty() ||
         Target.Fn->getReturnType() != RetType)
       return false;
   }
 
   for (auto &&CSByConstantArg : SlotInfo.ConstCSInfo) {
     if (!tryEvaluateFunctionsWithArgs(TargetsForSlot, CSByConstantArg.first))
       continue;

llvm/trunk/test/Transforms/WholeProgramDevirt/vcp-accesses-memory.ll

 ; RUN: opt -S -wholeprogramdevirt %s | FileCheck %s
+; RUN: opt -S -passes=wholeprogramdevirt %s | FileCheck %s
 
 target datalayout = "e-p:64:64"
 target triple = "x86_64-unknown-linux-gnu"
 
-@vt1 = constant [1 x i8*] [i8* bitcast (i32 (i8*, i32)* @vf1 to i8*)], !type !0
-@vt2 = constant [1 x i8*] [i8* bitcast (i32 (i8*, i32)* @vf2 to i8*)], !type !0
+@vt1 = constant [2 x i8*] [i8* bitcast (i32 (i8*, i32)* @vf1a to i8*), i8* bitcast (i32 (i8*, i32)* @vf1b to i8*)], !type !0
+@vt2 = constant [2 x i8*] [i8* bitcast (i32 (i8*, i32)* @vf2a to i8*), i8* bitcast (i32 (i8*, i32)* @vf2b to i8*)], !type !0
 
-define i32 @vf1(i8* %this, i32 %arg) {
+@sink = external global i32
+
+define i32 @vf1a(i8* %this, i32 %arg) {
+  store i32 %arg, i32* @sink
+  ret i32 %arg
+}
+
+define i32 @vf2a(i8* %this, i32 %arg) {
+  store i32 %arg, i32* @sink
+  ret i32 %arg
+}
+
+define i32 @vf1b(i8* %this, i32 %arg) {
   ret i32 %arg
 }
 
-define i32 @vf2(i8* %this, i32 %arg) {
+define i32 @vf2b(i8* %this, i32 %arg) {
   ret i32 %arg
 }
 
-; CHECK: define i32 @call
-define i32 @call(i8* %obj) {
+; Test that we don't apply VCP if the virtual function body accesses memory,
+; even if the function returns a constant.
+
+; CHECK: define i32 @call1
+define i32 @call1(i8* %obj) {
   %vtableptr = bitcast i8* %obj to [1 x i8*]**
   %vtable = load [1 x i8*]*, [1 x i8*]** %vtableptr
   %vtablei8 = bitcast [1 x i8*]* %vtable to i8*
   %p = call i1 @llvm.type.test(i8* %vtablei8, metadata !"typeid")
   call void @llvm.assume(i1 %p)
   %fptrptr = getelementptr [1 x i8*], [1 x i8*]* %vtable, i32 0, i32 0
   %fptr = load i8*, i8** %fptrptr
   %fptr_casted = bitcast i8* %fptr to i32 (i8*, i32)*
   ; CHECK: call i32 %
   %result = call i32 %fptr_casted(i8* %obj, i32 1)
   ret i32 %result
 }
 
+; Test that we can apply VCP regardless of the function attributes by analyzing
+; the function body itself.
+
+; CHECK: define i32 @call2
+define i32 @call2(i8* %obj) {
+  %vtableptr = bitcast i8* %obj to [1 x i8*]**
+  %vtable = load [1 x i8*]*, [1 x i8*]** %vtableptr
+  %vtablei8 = bitcast [1 x i8*]* %vtable to i8*
+  %p = call i1 @llvm.type.test(i8* %vtablei8, metadata !"typeid")
+  call void @llvm.assume(i1 %p)
+  %fptrptr = getelementptr [1 x i8*], [1 x i8*]* %vtable, i32 0, i32 1
+  %fptr = load i8*, i8** %fptrptr
+  %fptr_casted = bitcast i8* %fptr to i32 (i8*, i32)*
+  %result = call i32 %fptr_casted(i8* %obj, i32 1)
+  ; CHECK: ret i32 1
+  ret i32 %result
+}
+
 declare i1 @llvm.type.test(i8*, metadata)
 declare void @llvm.assume(i1)
 
 !0 = !{i32 0, !"typeid"}