[14] RFR (L): 8234391: C2: Generic vector operands

[Tobias Hartmann]

Hi Vladimir,

thanks for the additional details. Looks all good to me.

Best regards,
Tobias

On 26.11.19 17:52, Vladimir Ivanov wrote:
> Thanks, Tobias.
> 
>> hard to review the .ad file changes but this looks good to me.
> 
> Yes, the changes are massive, but mostly straightforward. In addition to the code needed for generic
> vector operand, the changes are:
> 
>   (1) (x86.ad) switching vec[SDXYZ] => vec and legVec[SDXYZ] => legVec;
> 
>   (2) (x86.ad) after the switch reduction instructions need additional checks on input vector size
> (example [1]);
> 
>   (3) (x86_64.ad/x86_32.ad) rename operands with "vec" name to avoid name conflicts with vec operand;
> 
>   (4) (x86_64.ad) migrate compressed string instructions from legVecS to legRegD to keep it working
> when vector support is explicitly disabled (e.g., -XX:MaxVectorSize=0)
> 
> (1) is needed to avoid explicit moves between concrete (legVec/vec[SDXYZ]) and generic vectors
> (vec/legVec).
> 
> (2)-(4) could have been reviewed/integrated separately, but they did look trivial enough to avoid
> the effort.
> 
>> Just noticed some code style issues:
>> - x86_64.ad:11284, 11346, 11410, 11426: indentation is wrong (already before your fix)
>> - whitespace in matcher.cpp:2598/2601 can be removed
> 
> Good catch.
> 
> Best regards,
> Vladimir Ivanov
> 
> [1]
> -instruct rvadd2F_reduction_reg(regF dst, vecD src2, vecD tmp) %{
> -  predicate(UseAVX > 0);
> +instruct rvadd2F_reduction_reg(regF dst, vec src2, vec tmp) %{
> +  predicate(UseAVX > 0 && n->in(2)->bottom_type()->is_vect()->length() == 2);
> 
>> On 19.11.19 15:30, Vladimir Ivanov wrote:
>>> http://cr.openjdk.java.net/~vlivanov/jbhateja/8234391/webrev.00/
>>> https://bugs.openjdk.java.net/browse/JDK-8234391
>>>
>>> Introduce generic vector operands and migrate existing usages from fixed sized operands (vec[SDXYZ])
>>> to generic ones.
>>>
>>> (It's an updated version of generic vector support posted for review in August, 2019 [1] [2]. AD
>>> instruction merges will be handled separately.)
>>>
>>> On a high-level it is organized as follows:
>>>
>>>    (1) all AD instructions in x86.ad/x86_64.ad/x86_32.ad use vec/legVec;
>>>
>>>    (2) at runtime, right after matching is over, a special pass is performed which does:
>>>
>>>        * replaces vecOper with vec[SDXYZ] depending on mach node type
>>>           - vector mach nodes capute bottom_type() of their ideal prototype;
>>>
>>>        * eliminates redundant reg-to-reg vector moves (MoveVec2Leg /MoveLeg2Vec)
>>>           - matcher needs them, but they are useless for register allocator (moreover, may cause
>>> additional spills);
>>>
>>>
>>>     (3) after post-selection pass is over, all mach nodes should have fixed-size vector operands.
>>>
>>>
>>> Some details:
>>>
>>>     (1) vec and legVec are marked as "dynamic" operands, so post-selection rewriting works
>>>
>>>
>>>     (2) new logic is guarded by new matcher flag (Matcher::supports_generic_vector_operands)
>>> which is
>>> enabled only on x86
>>>
>>>
>>>     (3) post-selection analysis is implemented as a single pass over the graph and processing
>>> individual nodes using their own (for DEF operands) or their inputs (USE operands) bottom_type()
>>> (which is an instance of TypeVect)
>>>
>>>
>>>     (4) most of the analysis is cross-platform and interface with platform-specific code through 3
>>> methods:
>>>
>>>       static bool is_generic_reg2reg_move(MachNode* m);
>>>       // distinguishes MoveVec2Leg/MoveLeg2Vec nodes
>>>
>>>       static bool is_generic_vector(MachOper* opnd);
>>>       // distinguishes vec/legVec operands
>>>
>>>       static MachOper* clone_generic_vector_operand(MachOper* generic_opnd, uint ideal_reg);
>>>       // constructs fixed-sized vector operand based on ideal reg
>>>       //   vec    + Op_Vec[SDXYZ] =>    vec[SDXYZ]
>>>       //   legVec + Op_Vec[SDXYZ] => legVec[SDXYZ]
>>>
>>>
>>>     (5) TEMP operands are handled specially:
>>>       - TEMP uses max_vector_size() to determine what fixed-sized operand to use
>>>           * it is needed to cover reductions which don't produce vectors but scalars
>>>       - TEMP_DEF inherits fixed-sized operand type from DEF;
>>>
>>>
>>>     (6) there is limited number of special cases for mach nodes in
>>> Matcher::get_vector_operand_helper:
>>>
>>>         - RShiftCntV/RShiftCntV: though it reports wide vector type as Node::bottom_type(), its
>>> ideal_reg is VecS! But for vector nodes only Node::bottom_type() is captured during matching and not
>>> ideal_reg().
>>>
>>>         - vshiftcntimm: chain instructions which convert scalar to vector don't have vector type.
>>>
>>>
>>>     (7) idealreg2regmask initialization logic is adjusted to handle generic vector operands (see
>>> Matcher::get_vector_regmask)
>>>
>>>
>>>     (8) operand renaming in x86_32.ad & x86_64.ad to avoid name conflicts with new vec/legVec
>>> operands
>>>
>>>
>>>     (9) x86_64.ad: all TEMP usages of vecS/legVecS are replaced with regD/legRegD
>>>        - it aligns the code between x86_64.ad and x86_32.ad
>>>        - strictly speaking, it's illegal to use vector operands on a non-vector node (e.g.,
>>> string_inflate) unless its usage is guarded by C2 vector support checks (-XX:MaxVectorSize=0)
>>>
>>>
>>> Contributed-by: Jatin Bhateja <jatin.bhateja at intel.com>
>>> Reviewed-by: vlivanov, sviswanathan, ?
>>>
>>> Testing: tier1-tier4, jtreg compiler tests on KNL and SKL,
>>>           performance testing (SPEC* + Octane + micros / G1 + ParGC).
>>>
>>> Best regards,
>>> Vladimir Ivanov
>>>
>>> [1] https://mail.openjdk.java.net/pipermail/hotspot-compiler-dev/2019-August/034822.html
>>>
>>> [2] http://cr.openjdk.java.net/~jbhateja/genericVectorOperands/generic_operands_support_v1.0.pdf