[PATCH] D43716: [EarlyCSE] Exploit open ended invariant.start scopes

Anna Thomas via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Mar 9 12:34:32 PST 2018

anna added a comment.

Patch looks good to me. Let's wait if anyone else has any comments.

Comment at: lib/Transforms/Scalar/EarlyCSE.cpp:695
+  if (!MemLocOpt)
+    // "target" intrinsic forms of loads aren't currently known to
+    // MemoryLocation::get.  TODO
I couldn't parse this comment.

Comment at: lib/Transforms/Scalar/EarlyCSE.cpp:803
+    if (match(Inst, m_Intrinsic<Intrinsic::invariant_start>())) {
+      if (!Inst->use_empty())
+        continue;
Pls add a comment here that this handles invariant end. So, `AvailableInvariants` are guaranteed to be invariant indefinitely from `CurrentGeneration` onwards.

Comment at: test/Transforms/EarlyCSE/invariant-loads.ll:99
+define void @test_false_negative_dse(i32* %p, i1 %cnd) {
+; CHECK-LABEL: @test_false_negative_dse
+; CHECK: store
why are we not able to catch this case? `invariant.load` is more powerful than invariant.start and we unconditionally return true for `isOperatingOnInvariantMemAt`

Comment at: test/Transforms/EarlyCSE/invariant.start.ll:257
+define i32 @test_false_negative_scope(i32* %p) {
+; CHECK-LABEL: @test_false_negative_scope
Any idea if MemorySSA will catch this case?



More information about the llvm-commits mailing list