RFR: 8335444: Generalize implementation of AndNode mul_ring

[Christian Hagedorn]

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.

Thanks for the investigation and the update! That sounds reasonable. Let me submit some testing again for that test.

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

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