RFR: 8346664: C2: Optimize mask check with constant offset [v8]

[Emanuel Peter]

On Thu, 30 Jan 2025 14:46:06 GMT, Matthias Ernst <duke at openjdk.org> wrote:

>> About the vectorization tests.
>> 
>> --------------
>> `TestEquivalentInvariants`:
>> Here we can adjust the `@IR` rules. You can see other IR rules in the file for examples.
>> `testMemorySegmentIInvarL3d` fails because it expects no vector nodes in the IR, hence the `= 0`.
>> There are other tests that have rules like these, i.e. with `> 0`:
>> 
>>     @Test
>>     @IR(counts = {IRNode.LOAD_VECTOR_I, "> 0",
>>                   IRNode.ADD_VI,        "> 0",
>>                   IRNode.STORE_VECTOR,  "> 0"},
>>         applyIfPlatform = {"64-bit", "true"},
>>         applyIf = {"AlignVector", "false"},
>>         applyIfCPUFeatureOr = {"sse4.1", "true", "asimd", "true"})
>> 
>> Why don't you try tweaking the IR rules until they pass on your machine.
>> 
>> ------------------
>> 
>> About `compiler/vectorization/TestPopulateIndex.java`.
>> 
>> I think we need to consider a generalization of `PopulateIndex`. Currently, it only allows the pattern:
>> `i+0, i+1, i+2, i+3, ...`.
>> 
>> This allows us to do this in the backend, see e.g. `src/hotspot/cpu/x86/x86.ad`:
>> 
>> instruct VectorPopulateIndex(vec dst, rRegI src1, immI_1 src2, vec vtmp) %{
>>   match(Set dst (PopulateIndex src1 src2));
>>   effect(TEMP dst, TEMP vtmp);
>>   format %{ "vector_populate_index $dst $src1 $src2\t! using $vtmp as TEMP" %}
>>   ins_encode %{
>>      assert($src2$$constant == 1, "required");
>>      int vlen_in_bytes = Matcher::vector_length_in_bytes(this);
>>      int vlen_enc = vector_length_encoding(this);
>>      BasicType elem_bt = Matcher::vector_element_basic_type(this);
>>      __ vpbroadcast(elem_bt, $vtmp$$XMMRegister, $src1$$Register, vlen_enc);
>>      __ load_iota_indices($dst$$XMMRegister, vlen_in_bytes, elem_bt);
>>      __ vpadd(elem_bt, $dst$$XMMRegister, $dst$$XMMRegister, $vtmp$$XMMRegister, vlen_enc);
>>   %}
>>   ins_pipe( pipe_slow );
>> %}
>> 
>> We take the `i`, broadcast it to every vector lane, and then add the `iota_indices` to it: `i,i,i,i` + `0,1,2,3`.
>> 
>> We could just introduce some constants vector, and add this instead of the `iota_indices`.
>> 
>> That would allow us to generalize it to any `i+c0, i+c1, i+c2, ...`.
>> 
>> I've wanted to have vector constants for other reasons anyway.
>> 
>> TLDR:
>> - I'm ok with disabling the IR rules in `TestPopulateIndex.java` for `exprWithIndex1`.
>> - But create another IR test with `i | 7` instead of `i & 7`, which should still vectorize and pass with IR rules, as it does not trigger your optimization.
>> - We will file an RFE for the generalization of ...
>
>> Why don't you try tweaking the IR rules until they pass on your machine.
> 
> Done. This affected three scenarios int/long 3d 3d2 and 3e, and only under -XX:-AlignVector. I have no insight why that is, my changes (split each scenario) only reflect what I observed. Also, 3e's setup says it should actually never vectorize; this needs special attention, something must be off.
> 
>> * I'm ok with disabling the IR rules in `TestPopulateIndex.java` for `exprWithIndex1`.
>> * But create another IR test with `i | 7` instead of `i & 7`, which should still vectorize and pass with IR rules, as it does not trigger your optimization.
> 
> Done, I verified that it compiles/checks, but IR tests @require something I don't seem to have.

@mernst-github It looks like GitHub actions are not being run. That is usually a good way to get some automatic verification / basic testing on different platforms for external contributors.

I think that way you get at least tier1 on x64 and aarch64, which is a great start.

https://wiki.openjdk.org/display/SKARA/Testing

Configuring workflows to run
When you create a new personal fork of the jdk repository, GitHub Actions are disabled by default. When you visit the Actions tab for your personal fork, you will see a message similar to this: "Workflows aren't being run on this forked repository". Click on the green button labelled "I understand my workflows, go ahead and enable them" to allow the jdk builds and tests to execute for branches in your personal fork.

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

PR Comment: https://git.openjdk.org/jdk/pull/22856#issuecomment-2624826963