[llvm] [polly] [AST] Don't merge memory locations in AliasSetTracker (PR #65731)

Bruno De Fraine via llvm-commits llvm-commits at lists.llvm.org
Mon Dec 25 16:59:10 PST 2023


================
@@ -307,7 +173,7 @@ class AliasSetTracker {
   BatchAAResults &AA;
   ilist<AliasSet> AliasSets;
 
-  using PointerMapType = DenseMap<AssertingVH<Value>, AliasSet::PointerRec *>;
+  using PointerMapType = DenseMap<MemoryLocation, AliasSet *>;
----------------
brunodf-snps wrote:

I agree that when using a map from pointer values, it is more logical to simply place MemLocs in an existing set of the pointer value, even if AA reports NoAlias for this set. I've updated the ast-pointer-map branch to this effect, in particular, see:
https://github.com/brunodf-snps/llvm-project/blob/9e9ebcd53aff011cff2b4d7ebdf439855806894b/llvm/lib/Analysis/AliasSetTracker.cpp#L312-L319

But be aware that in a program where AA indicates that two instruction using an undef pointer are not aliasing, _putting them in the same alias set is information loss_ (although it may not be relevant):

```
%val = load i32, ptr undef
...
store ptr null, ptr undef
``` 

So I would disagree with the cited code comment, or at least find it misleading:

>> Since alias(undef, undef) is NoAlias, mergeAliasSetsForPointer(undef, ...) will not find the the right set for undef, even if it exists. 

`mergeAliasSetsForPointer` is correctly indicating (with a `nullptr` result) that there is no set that the second memory location aliases with, according to AA. The existing set for the first memory location with an undef pointer is only a "right" set insofar that the implementation with a pointer map cannot accurately represent the situation. (But it may be a fine tradeoff to be inaccurate here.)

> It seems like you are going out of your way to have undefs in different sets -- why?

Well, I have roughly 2 objectives for AST with this patch: (1) avoid information loss and (2) minimal knowledge about AA or MemoryLocation internals.  So I am reluctant to add information loss for an implementation organization (the pointer map). But you seem to favor the pointer map for conceptual reasons too, so then I would propose what is on my ast-pointer-map branch.

https://github.com/llvm/llvm-project/pull/65731


More information about the llvm-commits mailing list