RFR: 8372266: Relax store matchers in compiler/escapeAnalysis/TestRematerializeObjects.java test

[Aleksey Shipilev]

On Mon, 24 Nov 2025 16:41:54 GMT, Aleksey Shipilev <shade at openjdk.org> wrote:

>> But how do you now know that the `StoreL` is really coming from the merged `StoreI`, and that it is not some other unrelated `StoreL`? The info that it comes from an int-array is relevant here `int[int:4]`, don't you think?
>> 
>> What about all the other MergeStores IR tests? For consistency you would now have to adjust those too, but I hope you don't do that ;)
>> `./test/hotspot/jtreg/compiler/c2/TestMergeStores.java`
>> 
>> The motivation seems to be that printing of store nodes was a bit different in JDK25. But then we just have to adjust the matching a bit, maybe weaken the IR rule for backports. But I'd prefer not to weaken the IR rule on mainline.
>> 
>> What do you think?
>
>> But how do you now know that the `StoreL` is really coming from the merged `StoreI`, and that it is not some other unrelated `StoreL`?
> 
> Well, because there are no `long` stores in Java code at all, so whatever that `StoreL` came from, it is JIT-generated? So then the IR test verifies that whatever happens with EA and MergeStores makes sure the store either goes away, or some merged store remains. I personally dislike overly-specific tests that rely on particulars of optimization sequencing or some such, and would rather have a test that checks the generic final state, without over-specificity.
> 
>> The motivation seems to be that printing of store nodes was a bit different in JDK25. But then we just have to adjust the matching a bit, maybe weaken the IR rule for backports. But I'd prefer not to weaken the IR rule on mainline.
> 
> Yes. I mean, there is a tradeoff somewhere here: either mainline relaxes the test and then JDK 25 matches the test version, or JDK 25 diverges. We _usually_ try to avoid divergences, if we can, because they continuously bite us. If your preference about not relaxing the mainline version is strong, then I can yield and diverge JDK 25. It would likely be literally the same fix I have here.

> @shipilev The IR rule failed on JDK25, and must have printed the StoreL, right? How does that line look like? Maybe it is a really minor difference, and we just adjust the IR rules for the backport ever so slightly

It is `STORE_L`, but not `STORE_L_OF_CLASS`, AFAICS, because there is apparently some subtlety in optimizations.

> Maybe @shipilev is right here: maybe we should not use IR rules in regression tests where a very specific ordering of optimizations was required. However, the IR rules would give us a hint that the regression test is still likely to test what we think it tests, so it would increase our confidence. But the question is what one is supposed to do when the IR rule fails...

To be precise, I think IR rules that depend on _overly precise_ node attributes are borderline flaky. It would not help when things like [JDK-8371789](https://bugs.openjdk.org/browse/JDK-8371789) improve node attribute reporting, and we accidental rely on them in IR tests.

I really do think that IR tests are useful, but are only solid for the clear-cut cases of "this node type is definitely not present in the final graph shape" vs "this node is expected to be there in the graph at least once". Everything else is relying that the dice keeps landing in a particular way.

Then again, in this particular case, it is not that important at all. I was just following SOP that if we figure the test is flaky (for some definition of it) during backports, we do the test fix in mainline, and then backport the test fix along. If we disagree this one is the flaky test, that's also fine, I'll just diverge JDK 25.

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

PR Comment: https://git.openjdk.org/jdk/pull/28437#issuecomment-3574404864