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

[Noah Goldstein via Phabricator via llvm-commits]

goldstein.w.n 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:
> > > 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


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156633