RFR: 8332394: Add friendly output when @IR rule missing value

Christian Hagedorn chagedorn at openjdk.org
Thu May 16 14:39:02 UTC 2024


On Thu, 16 May 2024 13:59:16 GMT, Hamlin Li <mli at openjdk.org> wrote:

> Hi,
> Can you help to review this simple patch?
> Currently, when a @IR rule like "applyIfPlatform" or "applyIfCPUFeature" miss a value, it will just throw ArrayIndexOutOfBoundsException, with no other information. This is confusing unless you dig into the test frame code.
> It's helpful to output more meaningful information.
> Thanks

Good catch! Only a small improvement suggestion, otherwise, looks good.

Just noticed that we are actually missing tests that trigger a format violation in  `TestBadFormat` for `applyIfCPUFeature*` and `applyIfPlatform*`. We should probably add some at some point, analogously to the ones already there for `applyIf*` for flags. But that could be done separately.

test/hotspot/jtreg/compiler/lib/ir_framework/test/IREncodingPrinter.java line 293:

> 291:             String platform = andRules[i].trim();
> 292:             i++;
> 293:             TestFormat.check(i < andRules.length, "Missing value for platform " + platform + failAt());

I suggest to also add the `ruleType` as in `hasAllRequiredFlags()`, for example. Then it is even more precise. For even more readability you could add some `""`:
Current:

 Missing value for platform xyz in @IR rule 1 at foo()

vs.
Improved:

 Missing value for platform "xyz" in @IR rule 1 in "applyIfPlatform" at foo()

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

Marked as reviewed by chagedorn (Reviewer).

PR Review: https://git.openjdk.org/jdk/pull/19270#pullrequestreview-2060974799
PR Review Comment: https://git.openjdk.org/jdk/pull/19270#discussion_r1603482668


More information about the hotspot-compiler-dev mailing list