[WPD] Filter out intrinsics inserted from whole-program-vtables

[xur]

Summary

These intrinsics should be counted in the heuristics in
InstCombine and CallsiteSplit.

They are causing FDO hash mismatches when whole-program-vtables
is enabled in optimized build but not in instrumentation build.

[aeubanks]

this doesn't seem very principled, it's seemingly adding checks in random places hoping that passes won't look too much at assumes/type tests
is there anything wrong with making the instrumented build use -fwhole-program-vtables?

llvm/test/Transforms/InstCombine/wpvt.ll

1

this test is very noisy and doesn't really show what's going on, could you reduce it a bit, and also maybe run it through -passes=metarenamer (and maybe convert to opaque pointers to remove some unnecessary pointer bitcasts via opt -S -opaque-pointers foo.ll)

[davidxl]

In the summary, do you mean 'should not be counted'? Also agree that the test should be cleaned up.

llvm/lib/Analysis/Loads.cpp

619

Add a wrapper interface 'isNopInst()' instead?

These instructions should not be counted in inline size estimation either.

[aeubanks]

llvm/lib/Analysis/Loads.cpp

619

the assume and type test should be already handled in CallAnalyzer::analyzeBlock by being ephemeral values (analyzed by AssumptionCache/CodeMetrics::collectEphemeralValues), but it'd be good to double check

[nikic]

This should be split into a patch for CallSiteSplitting and one for Loads. For CallSiteSplitting, you'll probably want to implement ephemeral value handling along the lines of https://github.com/llvm/llvm-project/blob/main/llvm/lib/Transforms/Utils/SimplifyCFG.cpp#L2941. For Loads, we'd want something along the lines of D101553.

[davidxl]

In D129003#3625953, @nikic wrote:

This should be split into a patch for CallSiteSplitting and one for Loads. For CallSiteSplitting, you'll probably want to implement ephemeral value handling along the lines of https://github.com/llvm/llvm-project/blob/main/llvm/lib/Transforms/Utils/SimplifyCFG.cpp#L2941. For Loads, we'd want something along the lines of D101553.

You may want to spell out the interface changes you think is right more clearly :)

llvm/lib/Analysis/Loads.cpp

619

It is probably better to unify the interface that does this though (an old comment -- forgot to click the submit button).

llvm/include/llvm/IR/Instruction.h

public:
 
   /// Return true if the instruction is a llvm.launder.invariant.group or
   /// llvm.strip.invariant.group.
   bool isLaunderOrStripInvariantGroup() const;
 
   /// Return true if the instruction is a DbgInfoIntrinsic or PseudoProbeInst.
   bool isDebugOrPseudoInst() const;
 
+  /// Return true if the instruction is an intrinsic inserted by
+  /// -fwhole-program-vtables.
+  bool isWholeProgramVTIntrinsic() const;
+
   /// Return a pointer to the next non-debug instruction in the same basic
   /// block as 'this', or nullptr if no such instruction exists. Skip any pseudo
   /// operations if \c SkipPseudoOp is true.
   const Instruction *
   getNextNonDebugInstruction(bool SkipPseudoOp = false) const;
   Instruction *getNextNonDebugInstruction(bool SkipPseudoOp = false) {
     return const_cast<Instruction *>(
         static_cast<const Instruction *>(this)->getNextNonDebugInstruction(

llvm/lib/Analysis/Loads.cpp

Value *llvm::findAvailablePtrLoadStore(
 
   const DataLayout &DL = ScanBB->getModule()->getDataLayout();
   const Value *StrippedPtr = Loc.Ptr->stripPointerCasts();
 
   while (ScanFrom != ScanBB->begin()) {
     // We must ignore debug info directives when counting (otherwise they
     // would affect codegen).
     Instruction *Inst = &*--ScanFrom;
-    if (Inst->isDebugOrPseudoInst())
+    if (Inst->isDebugOrPseudoInst() || Inst->isWholeProgramVTIntrinsic())
       continue;
 
     // Restore ScanFrom to expected value in case next test succeeds
     ScanFrom++;
 
     if (NumScanedInst)
       ++(*NumScanedInst);
 
if (!Load->isUnordered())
     return nullptr;
 
   // Try to find an available value first, and delay expensive alias analysis
   // queries until later.
   Value *Available = nullptr;;
   SmallVector<Instruction *> MustNotAliasInsts;
   for (Instruction &Inst : make_range(++Load->getReverseIterator(),
                                       ScanBB->rend())) {
-    if (Inst.isDebugOrPseudoInst())
+    if (Inst.isDebugOrPseudoInst() || Inst.isWholeProgramVTIntrinsic())
       continue;
 
     if (MaxInstsToScan-- == 0)
       return nullptr;
 
     Available = getAvailableLoadStore(&Inst, StrippedPtr, AccessTy,
                                       AtLeastAtomic, DL, IsLoadCSE);
     if (Available)

llvm/lib/IR/Instruction.cpp

bool Instruction::isLaunderOrStripInvariantGroup() const {
   return ID == Intrinsic::launder_invariant_group ||
          ID == Intrinsic::strip_invariant_group;
 }
 
 bool Instruction::isDebugOrPseudoInst() const {
   return isa<DbgInfoIntrinsic>(this) || isa<PseudoProbeInst>(this);
 }
 
+bool Instruction::isWholeProgramVTIntrinsic() const {
+  auto *II = dyn_cast<IntrinsicInst>(this);
+  if (!II)
+    return false;
+  Intrinsic::ID ID = II->getIntrinsicID();
+  return (ID == Intrinsic::assume || ID == Intrinsic::type_test);
+}
+
 const Instruction *
 Instruction::getNextNonDebugInstruction(bool SkipPseudoOp) const {
   for (const Instruction *I = getNextNode(); I; I = I->getNextNode())
     if (!isa<DbgInfoIntrinsic>(I) && !(SkipPseudoOp && isa<PseudoProbeInst>(I)))
       return I;
   return nullptr;
 }
 

llvm/lib/Transforms/Scalar/CallSiteSplitting.cpp

static bool canSplitCallSite(CallBase &CB, TargetTransformInfo &TTI) {
 
   // Allow splitting a call-site only when the CodeSize cost of the
   // instructions before the call is less then DuplicationThreshold. The
   // instructions before the call will be duplicated in the split blocks and
   // corresponding uses will be updated.
   InstructionCost Cost = 0;
   for (auto &InstBeforeCall :
        llvm::make_range(CallSiteBB->begin(), CB.getIterator())) {
+    if (InstBeforeCall.isWholeProgramVTIntrinsic())
+      continue;
     Cost += TTI.getInstructionCost(&InstBeforeCall,
                                    TargetTransformInfo::TCK_CodeSize);
     if (Cost >= DuplicationThreshold)
       return false;
   }
 
   return true;
 }

llvm/test/Transforms/InstCombine/wpvt.ll

+; RUN: opt < %s -passes=instcombine -S | FileCheck %s
+target triple = "x86_64-grtev4-linux-gnu"
+
+%"class.keyres::ByteArray" = type { i32 (...)** }
+%"class.absl::Span" = type { i8*, i64 }
+%"class.keyres::(anonymous namespace)::ByteArrayImpl" = type { %"class.keyres::ByteArray", %"class.std::__u::unique_ptr.4", i64 }
+%"class.std::__u::unique_ptr.4" = type { %"class.std::__u::__compressed_pair.5" }
+%"class.std::__u::__compressed_pair.5" = type { %"struct.std::__u::__compressed_pair_elem.6" }
+%"struct.std::__u::__compressed_pair_elem.6" = type { i8* }
+%"class.keyres::MemoryEncrypter" = type { i32 (...)** }
+
+$_ZTSN6keyres9ByteArrayE = comdat any
+$_ZTIN6keyres9ByteArrayE = comdat any
+$_ZTVN6keyres9ByteArrayE = comdat any
+
+@_ZTVN6keyres12_GLOBAL__N_113ByteArrayImplE = internal unnamed_addr constant { [6 x i8*] } { [6 x i8*] [i8* null, i8* bitcast ({ i8*, i8*, i8* }* @_ZTIN6keyres12_GLOBAL__N_113ByteArrayImplE to i8*), i8* bitcast (void (%"class.keyres::(anonymous namespace)::ByteArrayImpl"*)* @_ZN6keyres12_GLOBAL__N_113ByteArrayImplD2Ev to i8*), i8* bitcast (void (%"class.keyres::(anonymous namespace)::ByteArrayImpl"*)* @_ZN6keyres12_GLOBAL__N_113ByteArrayImplD0Ev to i8*), i8* bitcast (void (%"class.keyres::(anonymous namespace)::ByteArrayImpl"*, i8*, i64)* @_ZNK6keyres12_GLOBAL__N_113ByteArrayImpl3GetEN4absl4SpanIhEE to i8*), i8* bitcast (i64 (%"class.keyres::(anonymous namespace)::ByteArrayImpl"*)* @_ZNK6keyres12_GLOBAL__N_113ByteArrayImpl4SizeEv to i8*)] }, align 8, !type !0, !type !2, !type !4, !type !6, !type !7, !type !8
+@_ZTVN10__cxxabiv120__si_class_type_infoE = external dso_local global i8*
+@_ZTSN6keyres12_GLOBAL__N_113ByteArrayImplE = internal constant [39 x i8] c"N6keyres12_GLOBAL__N_113ByteArrayImplE\00", align 1
+@_ZTVN10__cxxabiv117__class_type_infoE = external dso_local global i8*
+@_ZTSN6keyres9ByteArrayE = linkonce_odr dso_local constant [20 x i8] c"N6keyres9ByteArrayE\00", comdat, align 1
+@_ZTIN6keyres9ByteArrayE = linkonce_odr dso_local constant { i8*, i8* } { i8* bitcast (i8** getelementptr inbounds (i8*, i8** @_ZTVN10__cxxabiv117__class_type_infoE, i64 2) to i8*), i8* getelementptr inbounds ([20 x i8], [20 x i8]* @_ZTSN6keyres9ByteArrayE, i32 0, i32 0) }, comdat, align 8
+@_ZTIN6keyres12_GLOBAL__N_113ByteArrayImplE = internal constant { i8*, i8*, i8* } { i8* bitcast (i8** getelementptr inbounds (i8*, i8** @_ZTVN10__cxxabiv120__si_class_type_infoE, i64 2) to i8*), i8* getelementptr inbounds ([39 x i8], [39 x i8]* @_ZTSN6keyres12_GLOBAL__N_113ByteArrayImplE, i32 0, i32 0), i8* bitcast ({ i8*, i8* }* @_ZTIN6keyres9ByteArrayE to i8*) }, align 8
+
+define %"class.keyres::(anonymous namespace)::ByteArrayImpl"* @_ZNSt3__u11make_uniqueIN6keyres12_GLOBAL__N_113ByteArrayImplEJRN4absl4SpanIKhEEEEENS_11__unique_ifIT_E15__unique_singleEDpOT0_(%"class.absl::Span"* noundef nonnull align 8 dereferenceable(16) %__args) unnamed_addr align 32 {
+entry:
+  %call = call noalias noundef nonnull dereferenceable(24) i8* @_Znwm(i64 noundef 24)
+  %0 = bitcast i8* %call to %"class.keyres::(anonymous namespace)::ByteArrayImpl"*
+  %agg.tmp.sroa.0.0..sroa_idx = getelementptr inbounds %"class.absl::Span", %"class.absl::Span"* %__args, i64 0, i32 0
+  %agg.tmp.sroa.0.0.copyload = load i8*, i8** %agg.tmp.sroa.0.0..sroa_idx, align 8
+  %agg.tmp.sroa.2.0..sroa_idx3 = getelementptr inbounds %"class.absl::Span", %"class.absl::Span"* %__args, i64 0, i32 1
+  %agg.tmp.sroa.2.0.copyload = load i64, i64* %agg.tmp.sroa.2.0..sroa_idx3, align 8
+  %1 = getelementptr inbounds %"class.keyres::(anonymous namespace)::ByteArrayImpl", %"class.keyres::(anonymous namespace)::ByteArrayImpl"* %0, i64 0, i32 0, i32 0
+  store i32 (...)** bitcast (i8** getelementptr inbounds ({ [6 x i8*] }, { [6 x i8*] }* @_ZTVN6keyres12_GLOBAL__N_113ByteArrayImplE, i64 0, inrange i32 0, i64 2) to i32 (...)**), i32 (...)*** %1, align 8
+  %__value_.i.i.i.i = getelementptr inbounds %"class.keyres::(anonymous namespace)::ByteArrayImpl", %"class.keyres::(anonymous namespace)::ByteArrayImpl"* %0, i64 0, i32 1, i32 0, i32 0, i32 0
+  store i8* null, i8** %__value_.i.i.i.i, align 8
+  %size_.i = getelementptr inbounds %"class.keyres::(anonymous namespace)::ByteArrayImpl", %"class.keyres::(anonymous namespace)::ByteArrayImpl"* %0, i64 0, i32 2
+  store i64 %agg.tmp.sroa.2.0.copyload, i64* %size_.i, align 8
+  %2 = call i1 @llvm.type.test(i8* bitcast (i8** getelementptr inbounds ({ [6 x i8*] }, { [6 x i8*] }* @_ZTVN6keyres12_GLOBAL__N_113ByteArrayImplE, i64 0, inrange i32 0, i64 2) to i8*), metadata !1)
+  call void @llvm.assume(i1 %2)
+  %add.i = add i64 %agg.tmp.sroa.2.0.copyload, 28
+  %call.i.i = call noalias noundef nonnull i8* @_Znam(i64 noundef %add.i)
+  call void @llvm.memset.p0i8.i64(i8* nonnull align 1 %call.i.i, i8 0, i64 %add.i, i1 false)
+  %3 = load i8*, i8** %__value_.i.i.i.i, align 8
+  store i8* %call.i.i, i8** %__value_.i.i.i.i, align 8
+  %tobool.not.i.i.i = icmp eq i8* %3, null
+  br i1 %tobool.not.i.i.i, label %_ZN6keyres12_GLOBAL__N_113ByteArrayImplC2EN4absl4SpanIKhEE.exit, label %delete.notnull.i.i.i.i
+; CHECK: br i1 true, label %[[TRUE_LABEL:.*]],
+; CHECK-SAME: label %[[FALSE_LABEL:.*]]
+
+delete.notnull.i.i.i.i:
+  call void @_ZdaPv(i8* noundef %3)
+  br label %_ZN6keyres12_GLOBAL__N_113ByteArrayImplC2EN4absl4SpanIKhEE.exit
+; CHECK: [[FALSE_LABEL]]:
+; CHECk-NOT: call void
+; CHECK: br
+
+_ZN6keyres12_GLOBAL__N_113ByteArrayImplC2EN4absl4SpanIKhEE.exit:
+  %call8.i = call noundef nonnull align 8 dereferenceable(8) %"class.keyres::MemoryEncrypter"* @_ZN6keyres15MemoryEncrypter3GetEv()
+; CHECK: [[TRUE_LABEL]]:
+; CHECK: %call8
+  %4 = load i8*, i8** %__value_.i.i.i.i, align 8
+  %5 = load i64, i64* %size_.i, align 8
+  %add.i.i = add i64 %5, 28
+  %6 = bitcast %"class.keyres::MemoryEncrypter"* %call8.i to void (%"class.keyres::MemoryEncrypter"*, i8*, i64, i8*, i64)***
+  %vtable11.i = load void (%"class.keyres::MemoryEncrypter"*, i8*, i64, i8*, i64)**, void (%"class.keyres::MemoryEncrypter"*, i8*, i64, i8*, i64)*** %6, align 8
+  %7 = bitcast void (%"class.keyres::MemoryEncrypter"*, i8*, i64, i8*, i64)** %vtable11.i to i8*
+  %8 = call i1 @llvm.type.test(i8* %7, metadata !"_ZTSN6keyres15MemoryEncrypterE")
+  call void @llvm.assume(i1 %8)
+  %vfn12.i = getelementptr inbounds void (%"class.keyres::MemoryEncrypter"*, i8*, i64, i8*, i64)*, void (%"class.keyres::MemoryEncrypter"*, i8*, i64, i8*, i64)** %vtable11.i, i64 2
+  %9 = load void (%"class.keyres::MemoryEncrypter"*, i8*, i64, i8*, i64)*, void (%"class.keyres::MemoryEncrypter"*, i8*, i64, i8*, i64)** %vfn12.i, align 8
+  call void %9(%"class.keyres::MemoryEncrypter"* noundef nonnull align 8 dereferenceable(8) %call8.i, i8* %agg.tmp.sroa.0.0.copyload, i64 %agg.tmp.sroa.2.0.copyload, i8* %4, i64 %add.i.i)
+  ret %"class.keyres::(anonymous namespace)::ByteArrayImpl"* %0
+}
+
+declare dso_local noundef nonnull i8* @_Znwm(i64 noundef) local_unnamed_addr
+declare i1 @llvm.type.test(i8*, metadata)
+declare void @llvm.assume(i1 noundef)
+declare dso_local noundef nonnull align 8 dereferenceable(8) %"class.keyres::MemoryEncrypter"* @_ZN6keyres15MemoryEncrypter3GetEv() local_unnamed_addr
+declare void @_ZN6keyres12_GLOBAL__N_113ByteArrayImplD2Ev(%"class.keyres::(anonymous namespace)::ByteArrayImpl"* noundef nonnull align 8 dereferenceable(24) %this) unnamed_addr align 32
+declare void @_ZN6keyres12_GLOBAL__N_113ByteArrayImplD0Ev(%"class.keyres::(anonymous namespace)::ByteArrayImpl"* noundef nonnull align 8 dereferenceable(24) %this) unnamed_addr align 32
+declare void @_ZNK6keyres12_GLOBAL__N_113ByteArrayImpl3GetEN4absl4SpanIhEE(%"class.keyres::(anonymous namespace)::ByteArrayImpl"* noundef nonnull align 8 dereferenceable(24) %this, i8* %output.coerce0, i64 %output.coerce1) unnamed_addr align 32
+declare i64 @_ZNK6keyres12_GLOBAL__N_113ByteArrayImpl4SizeEv(%"class.keyres::(anonymous namespace)::ByteArrayImpl"* noundef nonnull align 8 dereferenceable(24) %this) unnamed_addr align 32
+declare dso_local noundef nonnull i8* @_Znam(i64 noundef) local_unnamed_addr
+declare void @llvm.memset.p0i8.i64(i8* nocapture writeonly, i8, i64, i1 immarg)
+declare dso_local void @_ZdaPv(i8* noundef) local_unnamed_addr
+
+!0 = !{i64 16, !1}
+!1 = distinct !{}
+!2 = !{i64 32, !3}
+!3 = distinct !{}
+!4 = !{i64 40, !5}
+!5 = distinct !{}
+!6 = !{i64 16, !"_ZTSN6keyres9ByteArrayE"}
+!7 = !{i64 32, !"_ZTSMN6keyres9ByteArrayEKFvN4absl4SpanIhEEE.virtual"}
+!8 = !{i64 40, !"_ZTSMN6keyres9ByteArrayEKFmvE.virtual"}