[PATCH] D156633: [InstSimplify] Add some basic simplifications for `llvm.ptrmask`

[Alexander Richardson via Phabricator via llvm-commits]

arichardson added inline comments.


================
Comment at: llvm/test/Transforms/InstSimplify/ptrmask.ll:28
 ; CHECK-SAME: (ptr [[P:%.*]]) {
-; CHECK-NEXT:    [[R:%.*]] = call ptr @llvm.ptrmask.p0.i64(ptr [[P]], i64 0)
-; CHECK-NEXT:    ret ptr [[R]]
+; CHECK-NEXT:    ret ptr null
 ;
----------------
goldstein.w.n wrote:
> goldstein.w.n wrote:
> > goldstein.w.n wrote:
> > > goldstein.w.n wrote:
> > > > nikic wrote:
> > > > > I don't think this fold is correct, but alive2 accepts it. I've filed https://github.com/AliveToolkit/alive2/issues/929.
> > > > Oh, in that case a lot of this series is out the window.
> > > > Would have thought ptr -> null would be okay, but clearly need to read up on provenance.
> > > > 
> > > > No need to review the rest of the series as this was assumed in A LOT of places, will refactor.
> > > Is there a way to do this simplification w.o breaking provenance? Or do all the null returns need to be properly removed?
> > > 
> > Would the following be acceptable?
> > https://alive2.llvm.org/ce/z/yMmP6J
> > I.e outside of lets say load/store/inttoptr/inbounds-GEP replacing `%p` -> `null`?
> > AFAICT those are the only operations that meaningfully interact with provenance. 
> Does this being illegal imply that we don't need this nullness check here: https://github.com/llvm/llvm-project/blob/main/llvm/lib/Analysis/ValueTracking.cpp#L5789
Folding these to null could definitely result in miscompilations for CHERI targets such as Morello since pointers include bounds metadata. It is possible to have a pointer with e.g. full address space bounds. If I mask that with zero I'd want a dereference able pointer that still has those bounds rather than null which is non-dereferenceable. I could imagine this kind of pattern coming up followed by some more pointer arithmetic which would be equivalent to %p with address=X (there is no such intrinsic in upstream LLVM, but the CHERI fork adds one that retains provenance and just updated the pointed-to address).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156633