RFR: 8283894: Intrinsify compress and expand bits on x86 [v2]

[Jatin Bhateja]

On Tue, 3 May 2022 00:04:15 GMT, John R Rose <jrose at openjdk.org> wrote:

>> Jatin Bhateja has updated the pull request with a new target base due to a merge or a rebase. The incremental webrev excludes the unrelated changes brought in by the merge/rebase. The pull request contains four additional commits since the last revision:
>> 
>>  - 8283894: Review comments resolutions.
>>  - Merge branch 'master' of http://github.com/openjdk/jdk into JDK-8283894
>>  - 8283894: Extending IR framework testcase with some functional test points.
>>  - 8283894: Intrinsify compress and expand bits on x86
>
> src/hotspot/share/opto/intrinsicnode.cpp line 155:
> 
>> 153:       return new AndLNode(compr, src->in(1));
>> 154:     }
>> 155:   }
> 
> I think a further rule for `compress(m, m)` could be in order.
> 
> 
> compress(m, m) = m==-1 ? m : (1L << PopCount[IL](m))-1
> 
> 
> This should be its own path through `Ideal`, not special logic at this particular point.
> 
> Don't use it unless `Matcher::match_rule_supported(Op_PopCount[IL])` is true.

This is a special case when both source and mask are same and we cannot take this call from C2Compiler::is_intrinsic_supported method. So currently  if target does not support compress/expand bits then we do not create intrinsic in the first place.   

> Can you update the jtreg tests:
> 
> 1. Modify `CompressExpandTest` to run with and without the intrinsic enabled
> 2. Disable  (by default) `CompressExpandSanityTest`
>    ?

By disable you mean move it to ProblemList.txt.

> src/hotspot/share/opto/intrinsicnode.cpp line 213:
> 
>> 211: }
>> 212: 
>> 213: Node* ExpandBitsNode::Identity(PhaseGVN* phase) {
> 
> I also suggest adding a boolean if `compress_expand_identity` if you add rules which don't apply to both equally.
> 
> Here is possible type-propagation logic for compress and expand:
> 
> 
> let SIGN_BIT = (((IntOrLong)-1)>>>1)+1  (bit 31 or 63)
> let MAX_POS  = (((IntOrLong)-1)>>>1)
> lot BITS = 1+bitCount(MAX_POS)  (32 or 64)
> if (both x, m are con) {
>   // maybe use these rules, by porting the Java code to C++
>   compress(CON[x], CON[m]) ] = CON[portable_compress(x,m)]
>   expand(CON[x], CON[m]) ] = CON[portable_expand(x,m)]
>   // see also https://stackoverflow.com/questions/38938911/portable-efficient-alternative-to-pdep-without-using-bmi2
> } else if (m is CON[m] && m != -1) {
>   //compress(x, -1) = x   //identity handled elsewhere
>   //expand(x, -1) = x     //identity handled elsewhere
>   let bitc = bitCount(m)
>   LO[ compress(x, CON[m]) ] = 0  //sign bit is never set
>   HI[ compress(x, CON[m]) ] = ((1L<<bitc)-1)  //unsigned bit-support
>   LO[ expand(x, CON[m]) ] = (m >= 0) ? 0 : SIGN_BIT //sign bit might be set alone
>   HI[ expand(x, CON[m]) ] = (m >= 0) ? m : m ^ SIGN_BIT
>   // could improve a little by looking TYPE[x], but do not bother
> } else {
>   // estimate maximum possible weight of m (in 0..63)
>   let  maxbitc = BITS   if (LO[m] < 0 && HI[m] >= -1)  // could be -1
>   else maxbitc = BITS-1 if (LO[m] < 0 || HI[m] == MAX_POS)  // <0 or maxint
>   else maxbitc = BITS-1 - numberOfLeadingZeros(HI[m])
>   LO[ compress(x, m) ] = (maxbitc == 64 && LO[x] < 0) ? SIGN_BIT : 0
>   HI[ compress(x, m) ] = (maxbitc >= 63) ? HI[x] : MIN(HI[x], (1L<<maxbitc)-1)
>   LO[ expand(x, m) ] = (LO[m] >= 0) ? 0 : SIGN_BIT
>   HI[ expand(x, m) ] = (LO[m] >= 0) ? HI[m] : MAX_POS
> }
> 
> 
> The operands of compress and expand are inherently unsigned bitmasks, so the signed type system of C2 gets in the way.  In the future, a somewhat more thorough job could be done if we had bitwise types as well in C2.  For that that would mean, see https://bugs.openjdk.java.net/browse/JDK-8001436

I have handled these transformation separately in ideal/identity and value routines.

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

PR: https://git.openjdk.java.net/jdk/pull/8498