RFR: 8280378: [IR Framework] Support IR matching for different compile phases [v6]
Christian Hagedorn
chagedorn at openjdk.org
Tue Nov 1 15:42:48 UTC 2022
On Mon, 31 Oct 2022 08:26:25 GMT, Christian Hagedorn <chagedorn at openjdk.org> wrote:
>> This patch extends the IR framework with the capability to perform IR matching not only on the `PrintIdeal` and `PrintOptoAssembly` flag outputs but also on the `PrintIdeal` output of the different compile phases defined by the `COMPILER_PHASES` macro in `phasetype.hpp`:
>>
>> https://github.com/openjdk/jdk/blob/fba763f82528d2825831a26b4ae4e090c602208f/src/hotspot/share/opto/phasetype.hpp#L28-L29
>>
>> The IR framework uses the same compile phases with the same names (it only drops those which are part of a normal execution like `PHASE_DEBUG` or `PHASE_FAILURE`). A matching on the `PrintIdeal` and `PrintOptoAssembly` flags is still possible. The IR framework creates an own `CompilePhase` enum entry for them to simplify the implementation.
>>
>> ## How does it work?
>>
>> ### Basic idea
>> There is a new `phase` attribute for `@IR` annotations that allows the user to specify a list of compile phases on which the IR rule should be applied on:
>>
>>
>> int iFld;
>>
>> @Test
>> @IR(counts = {IRNode.STORE_I, "1"},
>> phase = {CompilePhase.AFTER_PARSING, // Fails
>> CompilePhase.ITER_GVN1}) // Works
>> public void optimizeStores() {
>> iFld = 42;
>> iFld = 42 + iFld; // Removed in first IGVN iteration and replaced by iFld = 84
>> }
>>
>> In this example, we apply the IR rule on the compile phases `AFTER_PARSING` and `ITER_GVN1`. Since the first store to `iFld` is only removed in the first IGVN iteration, the IR rule fails for `AFTER_PARSING` while it passes for `ITER_GVN1`:
>>
>> 1) Method "public void ir_framework.examples.IRExample.optimizeStores()" - [Failed IR rules: 1]:
>> * @IR rule 1: "@compiler.lib.ir_framework.IR(phase={AFTER_PARSING, ITER_GVN1}, applyIfCPUFeatureAnd={}, applyIf={}, applyIfCPUFeatureOr={}, applyIfCPUFeature={}, counts={"_#STORE_I#_", "1"}, failOn={}, applyIfAnd={}, applyIfOr={}, applyIfNot={})"
>> > Phase "After Parsing":
>> - counts: Graph contains wrong number of nodes:
>> * Constraint 1: "(\d+(\s){2}(StoreI.*)+(\s){2}===.*)"
>> - Failed comparison: [found] 2 = 1 [given]
>> - Matched nodes (2):
>> * 25 StoreI === 5 7 24 21 [[ 31 ]] @ir_framework/examples/IRExample+12 *, name=iFld, idx=4; Memory: @ir_framework/examples/IRExample:NotNull+12 *, name=iFld, idx=4; !jvms: IRExample::optimizeStores @ bci:3 (line 123)
>> * 31 StoreI === 5 25 24 29 [[ 16 ]] @ir_framework/examples/IRExample+12 *, name=iFld, idx=4; Memory: @ir_framework/examples/IRExample:NotNull+12 *, name=iFld, idx=4; !jvms: IRExample::optimizeStores @ bci:14 (line 124)
>>
>>
>> More examples are shown in `IRExample.java` and also in the new test `TestPhaseIRMatching.java`.
>>
>> ### CompilePhase.DEFAULT - default compile phase
>> The existing IR tests either match on the `PrintIdeal` and/or `PrintOptoAssembly` flag. Looking closer at the individual `@IR` rules, we can see that a single regex only matches **either** on `PrintIdeal` **or** on `PrintOptoAssembly` but never on both. Most of these regexes are taken from the class `IRNode` which currently provides default regexes for various C2 IR nodes (a default regex either matches on `PrintIdeal` or `PrintOptoAssembly`).
>>
>> Not only the existing IR tests but also the majority of future test do not need this new flexibility - they simply want to default match on the `PrintIdeal` flag. To avoid having to specify `phase = CompilePhase.PRINT_IDEAL` for each new rule (and also to avoid updating the large number of existing IR tests), we introduce a new compile phase `CompilePhase.DEFAULT` which is used by default if the user does not specify the `phase` attribute.
>>
>> Each entry in class `IRNode` now needs to define a default phase such that the IR framework knows which compile phase an `IRNode` should be matched on if `CompilePhase.DEFAULT` is selected.
>>
>> ### Different regexes for the same IRNode entry
>> A regex for an IR node might look different for certain compile phases. For example, we can directly match the node name `Allocate` before macro expansion for an `Allocate` node. But we are only able to match allocations again on the `PrintOptoAssembly` output (the current option) which requires a different regex. Therefore, the `IRNode` class is redesigned in the following way:
>>
>> - `IRNode` entries which currently represent direct regexes are replaced by "IR node placeholder strings". These strings are just encodings recognized by the IR framework as IR nodes:
>>
>> public static final String ALLOC = PREFIX + "ALLOC" + POSTFIX; // Normal IR node
>> public static final String ALLOC_OF = COMPOSITE_PREFIX + "ALLOC_OF" + POSTFIX; // Composite IR node
>>
>> - For each _IR node placeholder string_, we need to define which regex should be used for which compile phase. Additionally, we need to specify the default compile phase for this IR node. This is done in a static block immediately following the IR node placeholder string where we add a mapping to `IRNode.IR_NODE_MAPPINGS` (by using helper methods such als `allocNodes()`):
>>
>> public static final String ALLOC = PREFIX + "ALLOC" + POSTFIX;
>> static {
>> String idealIndependentRegex = START + "Allocate" + MID + END;
>> String optoRegex = "(.*precise .*\\R((.*(?i:mov|xorl|nop|spill).*|\\s*|.*LGHI.*)\\R)*.*(?i:call,static).*wrapper for: _new_instance_Java" + END;
>> allocNodes(ALLOC, idealIndependentRegex, optoRegex);
>> }
>>
>> **Thus, adding a new IR node now requires two steps: Defining an IR node placeholder string and an associated regex-compile-phase-mapping in the static block immediately following the IR node placeholder string. This mapping should reflect in which compile phases the new IR node can be found.**
>>
>> ### Using the IRNode entries correctly
>> The IR framework enforces the correct usage of the new IR node placeholder strings. It reports format violations if:
>> - An `IRNode` entry is used for a compile phase which is not supported (e.g. trying to match a `LoopNode` on the output of `CompilePhase.AFTER_PARSING`).
>> - An IR node entry is used without a specifying a mapping in a static block (e.g. when creating a new entry and forgetting to set up the mapping).
>> - Using a user-defined regex in `@IR` without specifying a non-default compile phase in `phase`. `CompilePhase.DEFAULT` only works with IR node placeholder strings. In all other cases, the user must explicitly specify one or more non-default compile phases.
>>
>> ## General Changes
>> The patch became larger than I've intended it to be. I've tried to split it into smaller patches but that was quite difficult and I eventually gave up. I therefore try to summarize the changes to make reviewing this simpler:
>>
>> - Added more packages to better group related classes together.
>> - Removed unused IGV phases in `phasetype.hpp` and sorted them in the order how they are used throughout a compilation.
>> - Updated existing, now failing, IR tests that use user-defined regexes. These now require a non-default phase. I've fixed that by replacing the user-defined regexes with `IRNode` entries (required some new `IRNode` entries for missing IR nodes).
>> - Introduced a new interface `Matchable.java` (for all classes representing a part on which IR matching can be done such as `IRMethod` or `IRRule`) to use it throughout the code instead of concrete class references. This allows better code sharing together with the already added `MatchResult.java` interface. Using interface types also simplifies testing as everything is substitutable. Each `Matchable`/`MatchResult` object now just contains a list of `Matchable`/`MatchResult` objects (e.g. an `IRMethod`/`IRMethodResult` contains a list of `IRRule`/`IRRuleMatchResult` objects to represent all IR rules/IR rule match results of this method etc.)
>> - Cleaned up and refactored a lot of code to use this new design.
>> - Using visitor pattern to visit the `MatchResult` objects. This simplifies the failure message and compilation output printing of the different compile phases. It allows us to perform specific operations at each level (i.e. on a method level, an IR rule level etc.). Visitors are also used in testing.
>> - New compile phase related classes in package `phase`. The idea is that each `IRRule`/`IRRuleMatchResult` class contains a list of `CompilePhaseIRRule`/`CompilePhaseIRRuleMatchResult` objects for each compile phase. When `CompilePhase.DEFAULT` is used, we create or re-use `CompilePhaseIRRule` objects which represent the specified default phases of each `IRNode` entry.
>> - The `FlagVM` now collects all the needed compile phases and creates a compiler directives file for the `TestVM` accordingly. We now only print the output which is also used for IR matching. Before this change, we always emitted `PrintIdeal` and `PrintOptoAssembly`. Even when not using the new matching on compile phases, we still get a performance benefit when only using `IRNode` entries which match on either `PrintIdeal` or `PrintOptoAssembly`.
>> - The failure message and compilation output is sorted alphabetically by method names and by the enum definition order of `CompilerPhase`.
>> - Replaced implementation inheritance by interfaces.
>> - Improved encapsulation of object data.
>> - Updated README and many comments/class descriptions to reflect this new feature.
>> - Added new IR framework tests
>>
>> ## Testing
>> - Normal tier testing.
>> - Applying the patch to Valhalla to perform tier testing.
>> - Testing with a ZGC branch by @robcasloz which does IR matching on mach graph compile phases. This requirement was the original motivation for this RFE. Thanks Roberto for helping to test this!
>>
>> Thanks,
>> Christian
>
> Christian Hagedorn has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 85 commits:
>
> - Merge branch 'master' into JDK-8280378
> - Merge branch 'master' into JDK-8280378
> - Fix TestVectorConditionalMove
> - Merge branch 'master' into JDK-8280378
> - Hao's patch to address review comments
> - Roberto's review comments
> - Update test/hotspot/jtreg/compiler/lib/ir_framework/driver/irmatching/NonIRTestClass.java
>
> Co-authored-by: Roberto Castañeda Lozano <robcasloz at users.noreply.github.com>
> - Update test/hotspot/jtreg/compiler/lib/ir_framework/driver/irmatching/irrule/constraint/raw/RawConstraint.java
>
> Co-authored-by: Roberto Castañeda Lozano <robcasloz at users.noreply.github.com>
> - Update test/hotspot/jtreg/compiler/lib/ir_framework/driver/irmatching/irrule/phase/CompilePhaseIRRuleBuilder.java
>
> Co-authored-by: Roberto Castañeda Lozano <robcasloz at users.noreply.github.com>
> - Merge branch 'master' into JDK-8280378
> - ... and 75 more: https://git.openjdk.org/jdk/compare/9b9be88b...8d330790
Another test iteration was successful. I'm therefore integrating this now. Thanks all for your feedback again!
-------------
PR: https://git.openjdk.org/jdk/pull/10695
More information about the hotspot-compiler-dev
mailing list