[PATCH] D73832: Ignore/Drop droppable uses for code-sinking in InstCombine

[Johannes Doerfert via Phabricator via llvm-commits]

jdoerfert added inline comments.


================
Comment at: llvm/lib/Transforms/Utils/KnowledgeRetention.cpp:263
+      Assume.bundle_op_infos(), [&Assume](const CallBase::BundleOpInfo &BOI) {
+        return BOI.Begin == BOI.End ||
+               getValueFromBundleOpInfo(Assume, BOI, BOIE_WasOn) != nullptr;
----------------
We do we disallow empty BundleOpInfo here?


================
Comment at: llvm/test/Transforms/InstCombine/assume.ll:274
 ; CHECK:       taken:
-; CHECK-NEXT:    [[CMP:%.*]] = icmp ne i32* [[LOAD]], null
 ; CHECK-NEXT:    tail call void @llvm.assume(i1 [[CMP]])
----------------
Why don't we sink this one anymore?


================
Comment at: llvm/test/Transforms/InstCombine/assume.ll:425
+  ret i1 %rval.2
+}
+
----------------
This test opens up the question if we want to sink simple instructions when we need to destroy assumes. I guess that is unrelated to this patch though.


================
Comment at: llvm/test/Transforms/InstCombine/assume.ll:450
+}
+
 declare void @llvm.dbg.value(metadata, metadata, metadata)
----------------
Could you add a test case with a FIXME that show we drop too many assumes when we sink, e.g. the ones that are still dominated.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D73832/new/

https://reviews.llvm.org/D73832