[PATCH] D83311: [CodeMoverUtils] Add optional data dependence checks using MSSA

[Bardia Mahjour via Phabricator via llvm-commits]

bmahjour added inline comments.


================
Comment at: llvm/lib/Transforms/Utils/CodeMoverUtils.cpp:264
+      // read after write dependency on same memory location
+      if (InstDefMA == CurInstLastDefMA || InstDefMA == CurInstMA)
+        return true;
----------------
Whitney wrote:
> bmahjour wrote:
> > Do we really need `InstDefMA == CurInstLastDefMA ||`? See my comment above.
> This is needed for cases like
> ```
> store 1, ptr
> x = load ptr
> store 0, ptr
> ```
> check if `x` is safe to move after `store 0, ptr`. 
> e.g. in https://reviews.llvm.org/D83543
> ```
> // Anti forward dependency
> EXPECT_FALSE(isSafeToMoveBefore(*LoadA1, *StoreB1, DT, &PDT, &DI));
> ```
> the defining access of `x` is `store 1, ptr`, the `getClobberingMemoryAccess` of `store 0, ptr` is `x`.
> Not sure if this works if there is no store before the memory use.
> I wonder if there is a better to do this, like if there is a way to directly get `x` from `store 0, ptr`.
That still won't work if the first store is after the load or is absent all together. I don't think there is a way to get `x` from `store 0, ptr` because there is no def-use relationship between them. This highlights the need to use Alias Analysis instead or in combination with MSSA. I see that current LICM has similar issues, for example see comment in `pointerInvalidatedByLoopWithMSSA()`.

I suppose improvements can be made in steps. Currently LICM uses an over-approximation to deal with this situation by looking for any definitions that are locally dominated by the sinking use. Perhaps we can start with that and gradually add aliasing queries into the equation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D83311