[PATCH] D76157: [SelectionDAGBuilder] Don't set MachinePointerInfo for gather when we find a uniform base

Fri Mar 13 14:34:19 PDT 2020

craig.topper added a comment.

In D76157#1922361 <https://reviews.llvm.org/D76157#1922361>, @efriedma wrote:

> We want to try to create the best MachineMemOperand we can.  That means:
>
> 1. In general, we can't express the semantics in MachinePointerInfo; the Value has to be a pointer, not a vector of pointers.  So we have to create an "empty" MachinePointerInfo.  (But we should make sure to note the address-space correctly.)

Ok I'll fix the address space.

> 2. If we find a uniform base, we should be able to express the notion that each loaded value has to be "based on" that pointer.  I'm not sure we can just pass it to the MachinePointerInfo; probably we need some way to express that the offsets could be negative.  But it shouldn't be a big extension.  Even if the access offset/size is unknown, we can still figure out some useful aliasing information. (Two pointers to distinct objects can't alias.)
> 3. The size of the access should reflect the actual distance between the accesses, not the number of bytes accessed.  If we don't have any way to express an access of unknown size, we should add it (it would be useful in other contexts).  Maybe -1ULL just works?

I suspect the type legalization code for splitting gathers/scatters is also miscalculating the size. It just calls getStoreSize() on the split VT pieces.

> 
> 
>  ----
> 
> I'm not sure what you think is wrong with the pointsToConstantMemory check.  If a gather loads from constant memory, it obviously can't alias anything.

I wasn't sure if passing just the "base" pointer was correct.

> 
> 
>  ----
> 
> We should be able to write testcases for all of this, I think; there are DAGCombines that depend on whether we can prove aliasing between a load and a store.

Do any of those DAG combines even consider gather/scatter nodes or do they just look at  loadsdnode/storesdnode?

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

https://reviews.llvm.org/D76157