RFR: 8335444: Generalize implementation of AndNode mul_ring

[Jasmine Karthikeyan]

On Wed, 31 Jul 2024 18:38:48 GMT, Christian Hagedorn <chagedorn at openjdk.org> wrote:

>> Not a review but I quickly ran it through our testing and the following test fails with `-XX:UseAVX=3` on linux-x64-debug and windows-x64-debug and without any flags on windows-x64-debug which seems to be related to your patch:
>> 
>> Test: compiler/vectorization/runner/BasicBooleanOpTest.java
>> 
>> Output:
>> 
>> Failed IR Rules (1) of Methods (1)
>> ----------------------------------
>> 1) Method "public boolean[] compiler.vectorization.runner.BasicBooleanOpTest.vectorAnd()" - [Failed IR rules: 1]:
>>    * @IR rule 3: "@compiler.lib.ir_framework.IR(phase={DEFAULT}, applyIfPlatformAnd={}, applyIfCPUFeatureOr={}, counts={"_#MACRO_LOGIC_V#_", ">0"}, failOn={}, applyIfPlatform={}, applyIfPlatformOr={}, applyIfOr={}, applyIfCPUFeatureAnd={}, applyIf={}, applyIfCPUFeature={"avx512f", "true"}, applyIfAnd={}, applyIfNot={})"
>>      > Phase "PrintIdeal":
>>        - counts: Graph contains wrong number of nodes:
>>          * Constraint 1: "MacroLogicV"
>>            - Failed comparison: [found] 0 > 0 [given]
>>            - No nodes matched!
>
>> Thank you for running testing @chhagedorn! I think I didn't run into this because my device doesn't support AVX-512. Does the failure have an ideal node printout as well? I think that could help in diagnosing the issue. Thanks!
> 
> Sure, here is the log file for linux-x64-debug: [test_failure.log](https://github.com/user-attachments/files/16445886/test_failure.log)

@chhagedorn I've had a chance to investigate the IR test failure, and it seems it's because with the patch we can find a sharper type during IGVN than with the baseline. After parsing, the code shape inside the loop looks like this: `StoreB[idx] = (a & b) & 1;` As the StoreB is storing a boolean type, we perform a bitwise-and of 1 to ensure the boolean value is in bounds. With the patch, the type of `(a & b)` is `bool` so the `& 1` is removed in `AndINode::Identity`. In the baseline, the type of `(a & b)` is `int`, so the redundant AndNode isn't removed. Later on, the logic chain is transformed into a `MacroLogicVNode`.

Since the IR output of the operation has changed I've updated the IR checks, bringing it in line with the other operations in the test.

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

PR Comment: https://git.openjdk.org/jdk/pull/20066#issuecomment-2270323255