[PATCH] D93529: [GVN][BasicAA] Enable clobbering in GVN.

Daniil Fukalov via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Jan 18 15:56:00 PST 2021


dfukalov added a comment.

In D93529#2504303 <https://reviews.llvm.org/D93529#2504303>, @fhahn wrote:

> IIUC this is similar to the handling of partial overwrites in DSE (https://github.com/llvm/llvm-project/blob/master/llvm/lib/Transforms/Scalar/DeadStoreElimination.cpp#L548)?
>
> If so, could we just generalize the helper there, rather than threading this through the different caches from BasicAA to GVN? Yes, we have to do some extra work in GVN, but there would be no need to maintain a cached value not used by other clients, it will work with any AA implementations returning partial alias & there will be no extra work when moving away from `MemDepAnalysis`.

GVN has similar partial aliased memory objects functionality in https://github.com/llvm/llvm-project/blob/master/llvm/lib/Transforms/Utils/VNCoercion.cpp but it has the same issue: they use `GetPointerBaseWithConstantOffset()` and give up in a case when GEP offset is not constant (please refer the trivial test case in the patch). But BasicAA has an ability to go further and prove partial alias plus estimate overlapping offsets. It seems all these operations (including PHIs processing) should not be re-applied in an AA user.

The issue was masked for different targets because usually vector types are splitted before GVN so it's just see `MustAlias` parts of vectors and successfully eliminate them.
Actually current DSE fails with the same pattern of stores, I planned to fix it after the patch.

I'm going to split the change by Roman request and update the description to clarify these circumstances.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93529



More information about the llvm-commits mailing list