[aarch64-port-dev ] RFR(L): 8231441: AArch64: Initial SVE backend support

[Ningsheng Jian]

Hi Andrew,

On 8/19/20 9:01 PM, Andrew Dinn wrote:
> Hi Ningsheng,
> 
> On 19/08/2020 10:53, Ningsheng Jian wrote:
>> I have updated the patch based on the review comments. Would you mind
>> taking another look? Thanks!
>>
>> Full:
>> http://cr.openjdk.java.net/~njian/8231441/webrev.04/
>>
>> Incremental:
>> http://cr.openjdk.java.net/~njian/8231441/webrev.04-vs-03/
> 
> That looks ok. A few suggested tweaks:
> 

Thanks!

> aarch64.ad:168
> 
> I think the following comment explains more clearly what is going on:
> 
> // For SVE vector registers, we simply extend vector register size to 8
> // 'logical' slots. This is nominally 256 bits but it actually covers
> // all possible 'physical' SVE vector register lengths from 128 ~ 2048 bits.
> // The 'physical' SVE vector register length is detected during startup
> // so the register allocator is able to identify the correct number of
> // bytes needed for an SVE spill/unspill.
> // Note that a vector register with 4 slots, denotes a 128-bit NEON
> // register allowing it to be distinguished from the
> //  corresponding SVE vector register when the SVE vector length
> // is 128 bits.
> 

This looks better than mine. Thanks! :-)

> postaloc.cpp:312 & 322
> 
> 311     if (lrgs(val_idx).is_scalable()) {
> 312       assert(val->ideal_reg() == Op_VecA, "scalable vector register");
> 
>          . . .
> 
> 321       if (lrgs(val_idx).is_scalable()) {
> 322         assert(val->ideal_reg() == Op_VecA, "scalable vector register");
> 
> You don't strictly need the asserts here as this is already asserted in
> the call to is_scalable().
> 

The assertion in LRG::is_scalable() is different, while this is an 
assertion for ideal_reg of a given node.

> 
>> JTreg tests are still running, and so far no new failure found.
> Ok, well assuming they pass I am happy with this latest patch modulo the
> tweaks above.
> 

Will report back once the tests on real hardware passed.

Thanks,
Ningsheng