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