[AssumeBundles] Fix Bug in Assume Queries

[Tyker]

Summary

this bug was causing miscompile.
now clang cant properly selfhost with -mllvm --enable-knowledge-retention

[lebedev.ri]

Test?

[Tyker]

In D83507#2142627, @lebedev.ri wrote:

Test?

i would also like to add a test for it, but the smallest reproduction example is still very big 30k+ line of IR
and depend on what is present in the AssumptionCache so i could only reproduce it under -O3 run.
it isn't minimized at all but minimizing it in a way that still exibit the bug is quite hard

[lebedev.ri]

In D83507#2142683, @Tyker wrote:

In D83507#2142627, @lebedev.ri wrote:

Test?

i would also like to add a test for it, but the smallest reproduction example is still very big 30k+ line of IR
and depend on what is present in the AssumptionCache so i could only reproduce it under -O3 run.
it isn't minimized at all but minimizing it in a way that still exibit the bug is quite hard

Can you at least post the reproducer+steps as-is?

[lebedev.ri]

Also, this either needs a rebase, or this isn't against master but the stack doesn't say so.

[Tyker]

In D83507#2145353, @lebedev.ri wrote:

Also, this either needs a rebase, or this isn't against master but the stack doesn't say so.

The patch should apply cleanly now. it was conflicting with an other path that landed.

In D83507#2145318, @lebedev.ri wrote:

In D83507#2142683, @Tyker wrote:

In D83507#2142627, @lebedev.ri wrote:

Test?

i would also like to add a test for it, but the smallest reproduction example is still very big 30k+ line of IR
and depend on what is present in the AssumptionCache so i could only reproduce it under -O3 run.
it isn't minimized at all but minimizing it in a way that still exibit the bug is quite hard

Can you at least post the reproducer+steps as-is?

one way to see the effect of the bug is to try to run ADT unitest compiled with stage1 clang with -O3 -mllvm --enable-knowledge-retention.
this test should have test failures caused by this bug.

one of the files in which a miscompile occurs is /home/tyker/opensource/llvm-project/llvm/unittests/ADT/ImmutableSetTest.cpp
here is the unoptimized IR that triggers the bug.

tmp.ll2 MBDownload

running: opt -O3 --enable-knowledge-retention tmp.ll

[lebedev.ri]

Please rebase and update the affected test :)

[Tyker]

In D83507#2215586, @lebedev.ri wrote:

Please rebase and update the affected test :)

thank you, but how did you reduced it ?

[lebedev.ri]

LG
@jdoerfert ?

In D83507#2219127, @Tyker wrote:

In D83507#2215586, @lebedev.ri wrote:

Please rebase and update the affected test :)

thank you, but how did you reduced it ?

By not treating compiler as a black box - there is a (an unreduced) testcase already, and a fix, that changes the IR after optimizations.
So one just has to write an interestingness test that checks that 1. the bug is still triggered, and 2. the final IR differs with/without the fix.

I would be lying if i said that llvm-reduce got those 2mb down to these 40 lines all by itself,
there are still some rough edges that required manual intervention..

[jdoerfert]

This seems to fix a problem.

llvm/lib/Analysis/AssumeBundleQueries.cpp

llvm::getKnowledgeForValue(const Value *V,
   if (!DebugCounter::shouldExecute(AssumeQueryCounter))
     return RetainedKnowledge::none();
   if (AC) {
     for (AssumptionCache::ResultElem &Elem : AC->assumptionsFor(V)) {
       IntrinsicInst *II = cast_or_null<IntrinsicInst>(Elem.Assume);
       if (!II || Elem.Index == AssumptionCache::ExprResultIdx)
         continue;
       if (RetainedKnowledge RK = getKnowledgeFromBundle(
-              *II, II->bundle_op_info_begin()[Elem.Index]))
+              *II, II->bundle_op_info_begin()[Elem.Index])) {
+        if (V != RK.WasOn)
+          continue;
         if (is_contained(AttrKinds, RK.AttrKind) &&
             Filter(RK, II, &II->bundle_op_info_begin()[Elem.Index])) {
           NumUsefullAssumeQueries++;
           return RK;
         }
       }
+    }
     return RetainedKnowledge::none();
   }
   for (const auto &U : V->uses()) {
     CallInst::BundleOpInfo* Bundle = getBundleFromUse(&U);
     if (!Bundle)
       continue;
     if (RetainedKnowledge RK =
             getKnowledgeFromBundle(*cast<CallInst>(U.getUser()), *Bundle))

llvm/test/Transforms/PhaseOrdering/d83507-knowledge-retention-bug.ll

 ; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
 ; RUN: opt -O1                   -enable-knowledge-retention -S < %s  | FileCheck %s --check-prefixes=ANY,OLDPM
 ; RUN: opt -passes='default<O1>' -enable-knowledge-retention -S < %s  | FileCheck %s --check-prefixes=ANY,NEWPM
 
 %0 = type { %0* }
 
 define %0* @f1() local_unnamed_addr {
 ; ANY-LABEL: @f1(
 ; ANY-NEXT:  bb:
 ; ANY-NEXT:    br label [[BB3:%.*]]
 ; ANY:       bb3:
 ; ANY-NEXT:    [[I1:%.*]] = phi %0* [ [[I5:%.*]], [[BB3]] ], [ undef, [[BB:%.*]] ]
 ; ANY-NEXT:    call void @llvm.assume(i1 true) [ "nonnull"(%0* [[I1]]) ]
 ; ANY-NEXT:    [[I4:%.*]] = getelementptr inbounds [[TMP0:%.*]], %0* [[I1]], i64 0, i32 0
 ; ANY-NEXT:    [[I5]] = load %0*, %0** [[I4]], align 8
-; ANY-NEXT:    br label [[BB3]]
+; ANY-NEXT:    [[I2:%.*]] = icmp eq %0* [[I5]], null
+; ANY-NEXT:    br i1 [[I2]], label [[BB6:%.*]], label [[BB3]]
+; ANY:       bb6:
+; ANY-NEXT:    ret %0* undef
 ;
 bb:
   br label %bb1
 
 bb1:
   %i = phi %0* [ undef, %bb ], [ %i5, %bb3 ]
   %i2 = icmp eq %0* %i, null
   br i1 %i2, label %bb6, label %bb3