[PATCH] D95543: [GVN] Clobber partially aliased loads.

[Nikita Popov via Phabricator via llvm-commits]

nikic added inline comments.


================
Comment at: llvm/lib/Analysis/MemoryDependenceAnalysis.cpp:518
-      // at the clobbering load directly, it doesn't know about any
-      // phi translation that may have happened along the way.
-
----------------
Do you have any information on why this is not a concern anymore? The comment was introduced in https://github.com/llvm/llvm-project/commit/a471751c24324e7ba6ac5c612dbedb16c644fc44, unfortunately without a test case :(


================
Comment at: llvm/lib/Analysis/MemoryDependenceAnalysis.cpp:522
+            ClobberOffset = None;
           return MemDepResult::getClobber(Inst);
+        }
----------------
Say we have a load from X, then a load from PartialAlias of X without offset, then another load from X. I think after your change, we will no longer forward from the first load, because we'll return a clobber for the PartialAlias in between, even though it cannot be used.

Can you please test this situation? Maybe we should not return a clobber if no offset is available?


================
Comment at: llvm/test/Transforms/GVN/clobber-partial-alias.ll:1
+; RUN: opt -gvn -S < %s | FileCheck %s
+
----------------
update_test_checks please.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95543