RFR: 8291809: Convert compiler/c2/cr7200264/TestSSE2IntVect.java to IR verification test [v2]

Emanuel Peter epeter at openjdk.org
Thu Jan 18 14:55:15 UTC 2024


On Thu, 18 Jan 2024 14:05:09 GMT, Daniel Lundén <dlunden at openjdk.org> wrote:

>> Ok then, let me explain IR Framework whitelists, and `runWithFlags`:
>> Generally, we want a test to be run from the outside with as many flag combinations as possible, since some bugs only trigger with strange combinations.
>> On the other hand, we need some way to say under what conditions an IR rule should be checked, because many flags might have side-effects on the IR, and disable all sorts of optimizations.
>> We chose the whitelist approach: those flags are expected generally not to change the IR, or change it in a way that could also happen on other machines, and therefore must be allowed to simulate those machines (e.g. UseAVX).
>> If you have a test that is not ok with any combination of the whitelisted flags, then the test must further restrict the IR rule with `applyIf` statements.
>> 
>> Sometimes, we want to make sure that IR rules are ok with flag settings that are not allowed by the whitelist, for example `OptimizeFill` (it has an effect, but maybe an effect we want to check for in a specific test). Or maybe we want to overwrite some flag setting for specific reasons (put up some node limit, etc). In those cases, it can make sense to use `runWithFlags`.
>> 
>> In the case of this test, I don't see why you would want to set `-XX:LoopUnrollLimit=0` via `runWithFlags`. Of course this makes all tests fail, because it messes up unrolling, and without unrolling you have no SuperWord, and without SuperWord you have no vectorization.
>> But if anybody were to set this flag from the outside (via JTREG), then it would disable the IR rules implicitly (because the flag is not on the whitelist), and the jtreg test would pass as a whole.
>> 
>> Does that make sense, and help?
>
> Yes, thanks, that corresponds to my understanding of the framework. Sorry if I was unclear, we of course do not want to include `-XX:LoopUnrollLimit=0`, that was just an example to illustrate the difference between whitelisting and `runWithFlags` (and what originally spawned this issue: https://bugs.openjdk.org/browse/JDK-8291809).
> 
> The question is if we want to include `-XX:StressLongCountedLoop=0` in `runWithFlags`, as that flag was part of the original test. I'm fine with removing it if it no longer makes sense in the IR framework.

Ok, great :)

I assume that `-XX:StressLongCountedLoop=0` was there before, because if it was not, then one could make the test fail with JTREG, by for example passing `-XX:StressLongCountedLoop=1000`, right?

But of course you would have to guard against all sorts of flags like this. For example `-Xint`. I just ran it with that flag, and it failed! :rofl: 

TLDR: this is an old mechanism, and a bad one. So please remove the flag ;)

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

PR Review Comment: https://git.openjdk.org/jdk/pull/17428#discussion_r1457554234


More information about the hotspot-compiler-dev mailing list