RFR: 8327381 Refactor type-improving transformations in BoolNode::Ideal to BoolNode::Value [v4]

[Quan Anh Mai]

On Tue, 19 Mar 2024 05:08:53 GMT, Jasmine Karthikeyan <jkarthikeyan at openjdk.org> wrote:

>> Kangcheng Xu has updated the pull request incrementally with one additional commit since the last revision:
>> 
>>   update the package name for tests
>
> Ah, I had assumed the transformation was valid beforehand, since it had existed for a while 😅 The issue only impacts `-1`, right? Since the comparison should succeed for both `m >=0` and `m < -1`. I think it would be good to address it in this patch, as it's refactoring the existing code.
> 
> [The original patch](https://github.com/openjdk/jdk/commit/415eda1274fce4ddc78dd2221abe2ce61f7ab7f2) seems to primarily test with `array.length` as the `m` value, so the value set was nonnegative. I think we can limit the `((x & m) u< m + 1)` transform to cases where `m` is known to be nonnegative and maintain the intent behind the transform. Something like:
> 
> -} else if (_test._test == BoolTest::lt && cmp2->Opcode() == Op_AddI && cmp2->in(2)->find_int_con(0) == 1) {
> +} else if (_test._test == BoolTest::lt && cmp2->Opcode() == Op_AddI && cmp2->in(2)->find_int_con(0) == 1 && phase->type(cmp2->in(1))->is_int()->_lo >= 0) {
> 
> With the IR test being modified accordingly. It'd also be good to write an IR test method that verifies that the transform doesn't take place if `m` doesn't succeed the `_lo >= 0` test.

@jaskarth Optimally, `(x & m) u< m + 1` can be transformed into `m != -1` but I think limiting it to non-negative `m` seems to be a reasonable approach.

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

PR Comment: https://git.openjdk.org/jdk/pull/18198#issuecomment-2007221375