[PATCH] D49642: AMDGPU: Rework extract-lowbits test

Matt Arsenault via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Fri Jul 27 03:59:08 PDT 2018


arsenm added a comment.

In https://reviews.llvm.org/D49642#1172989, @jvesely wrote:

> In https://reviews.llvm.org/D49642#1172185, @arsenm wrote:
>
> > In https://reviews.llvm.org/D49642#1171179, @jvesely wrote:
> >
> > > In https://reviews.llvm.org/D49642#1171132, @arsenm wrote:
> > >
> > > > I'd rather stop trying to share tests with r600 at all. I would like to split out most of the shared tests as-i
> > >
> > >
> > > Any reason for that? Both bfe instructions use the same patterns so it'd be just a copy paste.
> >
> >
> > A lot of tests have too many run lines as is, and adding more for r600 increases the mess. In this case you are actually changing the tested content. The original used VGPR inputs for everything, and this changes everything to be SGPR inputs. Both would be useful as separate tests, but we don't try particular hard to match scalar BFEs currently. Also, I want to stop artificially sharing some of the intrinsics.
>
>
> Fair enough https://reviews.llvm.org/D49641 has been updated to include a copy of the file with EG/CM checks.
>  This sounds like chasing fools gold. Most RUN lines are added for new generations of gpu (gfx9, ...) and more for HSA, two lines for cayman and cypress barely make a difference.
>  The bit extract patterns should be recognized irrespective of whether it can be scalarized or not.


Part of the problem is I often try to add tests to the relevant files, and then I hit some crash or other issue in r

In https://reviews.llvm.org/D49642#1172989, @jvesely wrote:

> In https://reviews.llvm.org/D49642#1172185, @arsenm wrote:
>
> > In https://reviews.llvm.org/D49642#1171179, @jvesely wrote:
> >
> > > In https://reviews.llvm.org/D49642#1171132, @arsenm wrote:
> > >
> > > > I'd rather stop trying to share tests with r600 at all. I would like to split out most of the shared tests as-i
> > >
> > >
> > > Any reason for that? Both bfe instructions use the same patterns so it'd be just a copy paste.
> >
> >
> > A lot of tests have too many run lines as is, and adding more for r600 increases the mess. In this case you are actually changing the tested content. The original used VGPR inputs for everything, and this changes everything to be SGPR inputs. Both would be useful as separate tests, but we don't try particular hard to match scalar BFEs currently. Also, I want to stop artificially sharing some of the intrinsics.
>
>
> Fair enough https://reviews.llvm.org/D49641 has been updated to include a copy of the file with EG/CM checks.
>  This sounds like chasing fools gold. Most RUN lines are added for new generations of gpu (gfx9, ...) and more for HSA, two lines for cayman and cypress barely make a difference.
>  The bit extract patterns should be recognized irrespective of whether it can be scalarized or not.


Another reason would be I sometimes try to add new tests to files where they logically go, and then hit some problem in r600. For example failing to handle function return values, so then I have to split the tests in more arbitrary ways


Repository:
  rL LLVM

https://reviews.llvm.org/D49642





More information about the llvm-commits mailing list