[PATCH] D65048: [GISel]: Attach missing range metadata while translating G_LOADs

[Amara Emerson via Phabricator via llvm-commits]

aemerson added a subscriber: paquette.
aemerson added a comment.

In D65048#1594839 <https://reviews.llvm.org/D65048#1594839>, @arsenm wrote:

> In D65048#1594810 <https://reviews.llvm.org/D65048#1594810>, @aditya_nandakumar wrote:
>
> > In D65048#1594784 <https://reviews.llvm.org/D65048#1594784>, @arsenm wrote:
> >
> > > LGTM. Are you planning on adding a MI computeKnownBits to use this? I will soon have a need for one and don't want to repeat work
> >
> >
> > Yes - that's how I stumbled on this.
> >  We have an out of tree port of most of DAG's ComputeKnownBits and SimplifyDemandedBits already implemented. It hasn't been upstreamed yet because
> >
> > 1. There was no need until now.
> > 2. Writing tests were/are pain and are dependent on combines which upstream didn't have a lot of.
> >
> >   I'd be happy to create a patch for that assuming we decide to go down the route of a direct port of DAG like implementation.
>
>
> I don't know what could be significantly different


@paquette is going to look at this from a design perspective but we haven't so far had a strong immediate need or the time to do this.

My thoughts are that computeKnownBits in SelectionDAG is a potentially very costly analysis, and we currently have magic recursion depth limits to prevent extremely long compile times. We don't have any evidence to support an alternative yet, but it would be nice to prototype different approaches for the GlobalISel equivalent. Maybe add caching support for the start, and/or implement it using a bottom up approach.


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

https://reviews.llvm.org/D65048