RFR: 8290169: adlc: Improve child constraints for vector unary operations [v2]

[Hao Sun]

On Thu, 8 Sep 2022 11:20:57 GMT, Tobias Hartmann <thartmann at openjdk.org> wrote:

>> Hao Sun has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains two commits:
>> 
>>  - Merge branch 'master' into 8290169-adlc
>>    
>>    Resolve the conflicts.
>>  - 8290169: adlc: Improve child constraints for vector unary operations
>>    
>>    As demonstrated in [1], the child constrait generated for *predicated
>>    vector unary operation* is the super set of that generated for the
>>    *unpredicated* version. As a result, there exists a risk for predicated
>>    vector unary operaions to match the unpredicated rules by accident.
>>    
>>    In this patch, we resolve this issue by generating one extra check
>>    "rChild == NULL" ONLY for vector unary operations. In this way, the
>>    child constraints for predicated/unpredicated vector unary operations
>>    are exclusive now.
>>    
>>    Following the example in [1], the dfa state generated for AbsVI is shown
>>    below.
>>    
>>    ```
>>    void  State::_sub_Op_AbsVI(const Node *n){
>>      if( STATE__VALID_CHILD(_kids[0], VREG) && STATE__VALID_CHILD(_kids[1], PREGGOV) &&
>>          ( UseSVE > 0 ) )
>>      {
>>        unsigned int c = _kids[0]->_cost[VREG]+_kids[1]->_cost[PREGGOV] + SVE_COST;
>>          DFA_PRODUCTION(VREG, vabsI_masked_rule, c)
>>      }
>>      if( STATE__VALID_CHILD(_kids[0], VREG) && _kids[1] == NULL &&  <---- 1
>>          ( UseSVE > 0) )
>>      {
>>        unsigned int c = _kids[0]->_cost[VREG] + SVE_COST;
>>        if (STATE__NOT_YET_VALID(VREG) || _cost[VREG] > c) {
>>          DFA_PRODUCTION(VREG, vabsI_rule, c)
>>        }
>>      }
>>      ...
>>    ```
>>    
>>    We can see that the constraint at line 1 cannot be matched for
>>    predicated AbsVI node now.
>>    
>>    The main updates are made in adlc/dfa part. Ideally, we should only
>>    add the extra check for affected platforms, i.e. AVX-512 and SVE. But we
>>    didn't do that because it would be better not to introduce any
>>    architecture dependent implementation here.
>>    
>>    Besides, workarounds in both aarch64_sve.ad and x86.ad are removed. 1)
>>    Many "is_predicated_vector()" checks can be removed in aarch64_sve.ad
>>    file. 2) Default instruction cost is used for involving rules in x86.ad
>>    file.
>>    
>>    [1]. https://github.com/shqking/jdk/commit/50ec9b19
>
> Looks reasonable to me but I'm not an expert in that area. @jatin-bhateja, @sviswa7, @iwanowww could you have a look?

@TobiHartmann Thanks for your comment.

As some shared code, i.e. `share/adlc`, gets changed in this patch, I think it would be better for this patch to pass the Oracle CI tests. I wonder if you could help to launch the test? Thanks.

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

PR: https://git.openjdk.org/jdk/pull/9534