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

Aditya Nandakumar via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Jul 22 09:55:05 PDT 2019


aditya_nandakumar added a comment.

In D65048#1595839 <https://reviews.llvm.org/D65048#1595839>, @aemerson wrote:

> 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.


It should be quite trivial to add caching support to compute known bits while operating on demand similar to DAG (which is pretty much bottom up).


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

https://reviews.llvm.org/D65048





More information about the llvm-commits mailing list