RFR: 8354242: VectorAPI: combine vector not operation with compare [v3]
erifan
duke at openjdk.org
Tue Apr 29 02:45:46 UTC 2025
On Mon, 28 Apr 2025 14:13:43 GMT, Emanuel Peter <epeter at openjdk.org> wrote:
> > > > > Just a drive-by comment for now, I may review this later more fully.
> > > > > > I would also prefer if you added the IR restrictions rather than the JTREG requires.
> > > > > > The benefit is that we can still run the tests on all platforms, at least for result verification.
> > > > > > Imagine someone adds optimizations to a new platform, but does not know about this test here. They make a mistake, and there is a bug, leading either to a crash or wrong result. With the requires, you test would never even run, and we would not catch it. With the IR applyIf, we would catch the bug.
> > > > >
> > > > >
> > > > > Just copy pasting the IR applyIf everywhere is not that much work, and adding in a new platform later is not really hard either.
> > > >
> > > >
> > > > Thanks! The problem is that when a new platform is added, people may not even know there is a test.
> > >
> > >
> > > @erifan That is true. But we have that problem either way. If you use `@require`, then the person does not realize there is a test AND the test is not run. If you use `applyIf`, the person does not realize there is a test, but it is run at least for result verifiation - and then the person MIGHT realize if the test catches a wrong result / crash.
> >
> >
> > This test will run on new platforms when we use @requires. I explained the meaning of the @requires in the previous comment, it only excludes one case: when -XX:UseAVX=0 is specified on x86 platforms.
>
> I see. You should probably add a comment there, to say that you are only excluding `AVX=0`. But even `UseAVX = 0` would profit from result verification.
@requires is a special comment itself. I feel like it's a bit weird to add a comment to a comment, and I don't think the @requires is hard to understand.
If we want to verify the correctness of AVX=0, we have to use ApplyIf. This is back to the beginning of the question, should we use @requires or ApplyIf? Personally I tend to use the former. By the way, I have tested the correctness of AVX=0 locally.
-------------
PR Comment: https://git.openjdk.org/jdk/pull/24674#issuecomment-2837292655
More information about the hotspot-compiler-dev
mailing list