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: 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. 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(). > 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. regards, Andrew Dinn ----------- Red Hat Distinguished Engineer Red Hat UK Ltd Registered in England and Wales under Company Registration No. 03798903 Directors: Michael Cunningham, Michael ("Mike") O'Neill