[PATCH] D101507: [SimplifyCFG] Ignore llvm.assume when looking for side effects

[Teresa Johnson via Phabricator via llvm-commits]

tejohnson created this revision.
tejohnson added a reviewer: xbolva00.
Herald added subscribers: wenlei, hiraditya.
tejohnson requested review of this revision.
Herald added a project: LLVM.

When passingValueIsAlwaysUndefined scans for an instruction between an
inst with a null or undef argument and its first use, explicitly ignore
llvm.assume which do not appear side effect free but in fact are. This
intrinsic is marked with the InaccessibleMemOnly attribute solely to
ensure it maintains proper control dependencies.

Without this change, when enabling -fwhole-program-vtables, which causes
assumes to be inserted by clang, we can get different simplification
decisions. In particular, when building with instrumentation FDO it can
affect the optimizations decisions before FDO matching, leading to some
mismatches.

I had to modify d83507-knowledge-retention-bug.ll since this fix enables
more aggressive optimization of that code such that it no longer tested
the original bug it was meant to test. I changed it to invoke the passes
of interest, instead of -O1, and confirmed that if I revert the fix it
is testing it still catches it.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D101507

Files:
  llvm/lib/Transforms/Utils/SimplifyCFG.cpp
  llvm/test/Transforms/PhaseOrdering/d83507-knowledge-retention-bug.ll
  llvm/test/Transforms/SimplifyCFG/UnreachableEliminate.ll


Index: llvm/test/Transforms/SimplifyCFG/UnreachableEliminate.ll
===================================================================
--- llvm/test/Transforms/SimplifyCFG/UnreachableEliminate.ll
+++ llvm/test/Transforms/SimplifyCFG/UnreachableEliminate.ll
@@ -84,6 +84,41 @@
   ret void
 }
 
+declare void @llvm.assume(i1)
+declare i1 @llvm.type.test(i8*, metadata) nounwind readnone
+
+;; Same as the above test but make sure the unreachable control flow is still
+;; removed in the presence of a type test / assume sequence.
+
+define void @test5_type_test_assume(i1 %cond, i8* %ptr, [3 x i8*]* %vtable) {
+; CHECK-LABEL: @test5_type_test_assume(
+; CHECK-NEXT:  entry:
+; CHECK-NEXT:    [[TMP0:%.*]] = xor i1 [[COND:%.*]], true
+; CHECK-NEXT:    call void @llvm.assume(i1 [[TMP0]])
+; CHECK-NEXT:    [[VTABLE:%.*]] = bitcast [3 x i8*]* %vtable to i8*
+; CHECK-NEXT:    [[P:%.*]] = call i1 @llvm.type.test(i8* [[VTABLE]], metadata !"foo")
+; CHECK-NEXT:    tail call void @llvm.assume(i1 [[P]])
+; CHECK-NEXT:    store i8 2, i8* [[PTR:%.*]], align 8
+; CHECK-NEXT:    ret void
+;
+entry:
+  br i1 %cond, label %bb1, label %bb3
+
+bb3:
+  br label %bb2
+
+bb1:
+  br label %bb2
+
+bb2:
+  %ptr.2 = phi i8* [ %ptr, %bb3 ], [ null, %bb1 ]
+  %vtablei8 = bitcast [3 x i8*]* %vtable to i8*
+  %p = call i1 @llvm.type.test(i8* %vtablei8, metadata !"foo")
+  tail call void @llvm.assume(i1 %p)
+  store i8 2, i8* %ptr.2, align 8
+  ret void
+}
+
 define void @test5_no_null_opt(i1 %cond, i8* %ptr) #0 {
 ; CHECK-LABEL: @test5_no_null_opt(
 ; CHECK-NEXT:  entry:
Index: llvm/test/Transforms/PhaseOrdering/d83507-knowledge-retention-bug.ll
===================================================================
--- llvm/test/Transforms/PhaseOrdering/d83507-knowledge-retention-bug.ll
+++ llvm/test/Transforms/PhaseOrdering/d83507-knowledge-retention-bug.ll
@@ -1,15 +1,15 @@
 ; NOTE: Assertions have been autogenerated by utils/update_test_checks.py
-; RUN: opt -O1                   -enable-knowledge-retention -S < %s  | FileCheck %s
-; RUN: opt -passes='default<O1>' -enable-knowledge-retention -S < %s  | FileCheck %s
+; RUN: opt -loop-rotate -instcombine -enable-knowledge-retention -S < %s  | FileCheck %s
+; RUN: opt -passes='loop(loop-rotate),instcombine' -enable-knowledge-retention -S < %s  | FileCheck %s
 
 %0 = type { %0* }
 
 define %0* @f1() local_unnamed_addr {
 ; CHECK-LABEL: @f1(
 ; CHECK-NEXT:  bb:
-; CHECK-NEXT:    br label [[BB3:%.*]]
+; CHECK:         br label [[BB3:%.*]]
 ; CHECK:       bb3:
-; CHECK-NEXT:    [[I1:%.*]] = phi %0* [ [[I5:%.*]], [[BB3]] ], [ undef, [[BB:%.*]] ]
+; CHECK-NEXT:    [[I1:%.*]] = phi %0* [ undef, [[BB:%.*]] ], [ [[I5:%.*]], [[BB3]] ]
 ; CHECK-NEXT:    call void @llvm.assume(i1 true) [ "nonnull"(%0* [[I1]]) ]
 ; CHECK-NEXT:    [[I4:%.*]] = getelementptr inbounds [[TMP0:%.*]], %0* [[I1]], i64 0, i32 0
 ; CHECK-NEXT:    [[I5]] = load %0*, %0** [[I4]], align 8
Index: llvm/lib/Transforms/Utils/SimplifyCFG.cpp
===================================================================
--- llvm/lib/Transforms/Utils/SimplifyCFG.cpp
+++ llvm/lib/Transforms/Utils/SimplifyCFG.cpp
@@ -6565,7 +6565,12 @@
              i = ++BasicBlock::iterator(I),
              UI = BasicBlock::iterator(dyn_cast<Instruction>(Use));
          i != UI; ++i)
-      if (i == I->getParent()->end() || i->mayHaveSideEffects())
+      if (i == I->getParent()->end() ||
+          // Explicitly check for and ignore llvm.assume intrinsics, which are
+          // marked as accessing InaccessibleMemOnly in order to maintain
+          // control dependencies, and therefore not detected as side effect
+          // free.
+          (i->mayHaveSideEffects() && !isa<AssumeInst>(i)))
         return false;
 
     // Look through GEPs. A load from a GEP derived from NULL is still undefined


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D101507.341376.patch
Type: text/x-patch
Size: 3803 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20210429/65aa23cf/attachment.bin>