RFR: 8327381: Refactor type-improving transformations in BoolNode::Ideal to BoolNode::Value [v10]
Kangcheng Xu
kxu at openjdk.org
Mon Jul 8 16:26:39 UTC 2024
On Mon, 8 Jul 2024 13:59:59 GMT, Tobias Hartmann <thartmann at openjdk.org> wrote:
>> PR updated according to requests. It looked like GHA previously had a problem with serviceability tests across the repo. Let's wait until the new round of testing is complete.
>
> @tabjy any update on this? Let us know if you need help with reproducing the issue that Christian reported. Thanks!
@TobiHartmann Thanks for the reminder.
I had no problem reproducing this issue, but what ultimately caused it is more involved and has something to do with a *possible* bug in the IR test framework itself. Let me explain:
First, enabling/disabling TieredCompilation does make a difference in produced IR. This has nothing to do # of executions during warm-up but rather the discrepancy in profiling: with TieredCompilation **enlabled**, profiling is done at C1 level, and there are 2 additional `Cmp_U3` nodes and uncommon traps.
Second, the IR framework seems to be incorrectly matching `Cmp_U3` as `Cmp_U` nodes. Afaik, the framework uses regex to match strings and determine the type of a node [[0]](https://github.com/openjdk/jdk/blob/d8c1c6ab0543c986280dcfa1c6c79e010a7b35fb/test/hotspot/jtreg/compiler/lib/ir_framework/IRNode.java#L442). Because `CmpU` is also a prefix of `CmpU3`, there could be a problem with pattern matching. However, I've not confirmed this. If this is the case, it might deserve a separate issue.
Third, I made incorrect assumption on the number of `Cmp_U` nodes to begin with:
return !(Integer.compareUnsigned((x & m), m - 1) > 0) |
!(Integer.compareUnsigned((m & x), m - 1) > 0) |
Integer.compareUnsigned((x & m), m + 2) < 0 |
Integer.compareUnsigned((m & x), m + 2) < 0;
I was under the assumption that the 4 cases would produce 4 `Cmp_U` nodes; however, the first and the second are semantically equivelent and optmized into 1. The same goes for the latter two. With the additional 2 miscounted `Cmp_U3`, they coincidently add to 4 and passed test with TieredCompilation enabled. (But not with `-XX:-TieredCompilation`).
-------------
PR Comment: https://git.openjdk.org/jdk/pull/18198#issuecomment-2214599025
More information about the hotspot-compiler-dev
mailing list