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

Hao Sun haosun at openjdk.org
Wed Sep 14 07:06:47 UTC 2022


> 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~aarch64_vector.ad and x86.ad are removed. 1)
> Many "is_predicated_vector()" checks can be removed in ~aarch64_sve.ad~aarch64_vector.ad
> file. 2) Default instruction cost is used for involving rules in x86.ad
> file.
> 
> ~[1]. https://github.com/shqking/jdk/commit/50ec9b19~
> [1]. https://github.com/shqking/jdk/commit/f7d9621e2

Hao Sun has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains four commits:

 - Remove the "is_predicated_vector()" check introduced in JDK-8292587
 - Merge branch 'master' into 8290169-adlc
 - 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

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

Changes: https://git.openjdk.org/jdk/pull/9534/files
 Webrev: https://webrevs.openjdk.org/?repo=jdk&pr=9534&range=02
  Stats: 154 lines in 5 files changed: 28 ins; 81 del; 45 mod
  Patch: https://git.openjdk.org/jdk/pull/9534.diff
  Fetch: git fetch https://git.openjdk.org/jdk pull/9534/head:pull/9534

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


More information about the hotspot-compiler-dev mailing list