SpeculativeExecution: Fix for logic change introduced in D81730.

[dfukalov]

Summary

The test case started to hoist bitcasts to upper BB after D81730.
Reverted unintentional logic change. Some instructions may have zero cost but
will not be hoisted by different limitation so should be counted for threshold.

[nhaehnle]

Seems reasonable to me based on the original intention here.

[arsenm]

llvm/test/Transforms/SpeculativeExecution/PR46267.ll

6

I don't understand what this is checking. Can you ad a comment and/or rename the function?

[aprantl]

llvm/test/Transforms/SpeculativeExecution/PR46267.ll

10

Yeah I would have expected a hoisted debug intrinsic somewhere in the test?

[dfukalov]

Test updated

[dfukalov]

llvm/test/Transforms/SpeculativeExecution/PR46267.ll

6

This is a reduced buildbot testcase started to fail after D81730. The change started to hoist two bitcasts to upper BB although it was not intended in fix for PR46267.

This test checks that bitscasts stayed in their BB.

[dfukalov]

Ping.

llvm/lib/Transforms/Scalar/SpeculativeExecution.cpp

bool SpeculativeExecutionPass::considerHoistingFromTo(
   for (const auto &I : FromBlock) {
     const unsigned Cost = ComputeSpeculationCost(&I, *TTI);
     if (Cost != UINT_MAX && isSafeToSpeculativelyExecute(&I) &&
         AllPrecedingUsesFromBlockHoisted(&I)) {
       TotalSpeculationCost += Cost;
       if (TotalSpeculationCost > SpecExecMaxSpeculationCost)
         return false;  // too much to hoist
     } else {
-      // If the instruction cannot be hoisted but has zero cost suppose it's
-      // a special case e.g. debug info instrinsics that should not be counted
-      // for threshold.
-      if (Cost)
+      // Debug info instrinsics should not be counted for threshold.
+      if (!isa<DbgInfoIntrinsic>(I))
         NotHoistedInstCount++;
       if (NotHoistedInstCount > SpecExecMaxNotHoisted)
         return false; // too much left behind
       NotHoisted.insert(&I);
     }
   }
 
   for (auto I = FromBlock.begin(); I != FromBlock.end();) {

llvm/test/Transforms/SpeculativeExecution/PR46267.ll

 ; RUN: opt < %s -S -speculative-execution | FileCheck %s
 ; RUN: opt < %s -S -passes='speculative-execution' | FileCheck %s
 
+%class.B = type { i32 (...)** }
+
+; Testing that two bitcasts are not hoisted to the first BB
+define i8* @foo(%class.B* readonly %b) {
+; CHECK-LABEL: foo
+; CHECK-LABEL: entry
+; CHECK-NEXT: %i = icmp eq %class.B* %b, null
+; CHECK-NEXT: br i1 %i, label %end, label %notnull
+entry:
+  %i = icmp eq %class.B* %b, null
+  br i1 %i, label %end, label %notnull
+
+; CHECK-LABEL: notnull:
+; CHECK-NEXT: %i1 = bitcast %class.B* %b to i32**
+; CHECK: %i3 = bitcast %class.B* %b to i8*
+notnull:                             ; preds = %entry
+  %i1 = bitcast %class.B* %b to i32**
+  %vtable = load i32*, i32** %i1, align 8
+  %i2 = getelementptr inbounds i32, i32* %vtable, i64 -2
+  %offset.to.top = load i32, i32* %i2, align 4
+  %i3 = bitcast %class.B* %b to i8*
+  %i4 = sext i32 %offset.to.top to i64
+  %i5 = getelementptr inbounds i8, i8* %i3, i64 %i4
+  br label %end
+
+end:                                 ; preds = %notnull, %entry
+  %i6 = phi i8* [ %i5, %notnull ], [ null, %entry ]
+  ret i8* %i6
+}
+
 define void @f(i32 %i) {
 entry:
 ; CHECK-LABEL: @f(
 ; CHECK:  %a2 = add i32 %i, 0
 ; CHECK-NEXT:  call void @llvm.dbg.value(metadata i32 %a2
   br i1 undef, label %land.rhs, label %land.end
 
 land.rhs:                                         ; preds = %entry