[PATCH] D54882: [AMDGPU] Add sdwa support for ADD|SUB U64 decomposed Pseudos

[Ron Lieberman via Phabricator via llvm-commits]

ronlieb added a comment.

In D54882#1309605 <https://reviews.llvm.org/D54882#1309605>, @rampitec wrote:

> In D54882#1309588 <https://reviews.llvm.org/D54882#1309588>, @rampitec wrote:
>
> > In D54882#1309583 <https://reviews.llvm.org/D54882#1309583>, @ronlieb wrote:
> >
> > > In D54882#1308240 <https://reviews.llvm.org/D54882#1308240>, @rampitec wrote:
> > >
> > > > Essentially this is a limited version of shrinking. So I have several questions:
> > > >
> > > > 1. Why not to run shrink pass before sdwa instead?
> > >
> > >
> > > I tried adding Shrink pass before PeepholeSDWA and observed 88 lit test failures.
> > >  i tried moving Shrink pass before Peephole SDWA and observed 25 lit test failures
> >
> >
> > Which may be a good thing if these failures are progressions (as I suspect) and not regressions. Are they progressions?
> >  That is the point of other comments too, this patch is limited to handle just two instructions while there is a clear possibility to do it for almost any VOP3.
>
>
> I would also assume many of these failures are just commute which is attempted by shrink pass. That is normal and would only need to change the tests.


i tried an experiment of simply invoking the Shrink pass a 2nd time.

  addPass(createSIShrinkInstructionsPass());
  addPass(createSIShrinkInstructionsPass());

which resulted in 74 failures, and they do seem to be commute changes primarily (did not look at them all)
So then, i added a 3rd invocation and zero failures (i'm still laughing at this one).


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

https://reviews.llvm.org/D54882