On Mon, 28 Apr 2025 09:51:10 GMT, erifan <d...@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.

-------------

PR Comment: https://git.openjdk.org/jdk/pull/24674#issuecomment-2835397926

Reply via email to