[8u] RFR 8248901: Signed immediate support in .../share/assembler.hpp is broken.

[Zhengyu Gu]

I am echoing Andrew Dinn's point: it is not clear-cut.

I am not sure it is a good idea to take already diverged 11u backport, 
then further diverge it into 8u.

Analyising how code evolved and its dependencies, is a big part of 
backport process. Usually, the closest version gives shortest path, but 
not always.

This particular 11u backport, combined 3 patches, (especially 
JDK-8215144, which I believe it is irrelevant to this backport and 
should be done in separate backport) which is going to confuse future 
backporter.


Thanks,

-Zhengyu



On 6/24/21 5:48 AM, Andrew Dinn wrote:
> On 23/06/2021 17:47, Andrew Hughes wrote:
>> This seems to be missing changes that were made for 11u. With backports,
>> we should work from the closest version that has the fix, so to try and
>> avoid repeating analysis that has already been done in earlier reviews.
>> For 8u, this should mean working from 11u, unless there is a good reason
>> why the fix is not applicable there.
> 
> All that is missing is
> 
>    i) the arm.ad change (this port does not exist in jdk8u)
>    ii) use of the precond macro (replaced with assert)
> 
>> In the 11u changeset:
>>
>> https://hg.openjdk.java.net/jdk-updates/jdk11u/rev/d21f0f95e6b4
>>
>> the precond macros are introduced from JDK-8223140, which, given their
>> simplicity, I think makes sense and will help any future backports that
>> use these macros.
> 
> I'm not sure this is clear-cut. That patch changes code in the opto 
> compiler. While it looks like those changes would apply cleanly and 
> correctly, backporting them just to obtain a macro equivalent to the 
> assert used by Zhengyu's patch is a risk. Replacing the calls to precond 
> with equivalent calls to assert is clearly a lower risk. It also does 
> not stop JDK-8223140 from being back-ported later should need for that 
> arise.
> 
>> It also includes the ppc64 fix, JDK-8215144, though uncredited, and some
>> sparc changes (there is no sparc port in trunk).
> 
> If the ppc patch is needed then it can be back-ported independent of 
> this one.
> 
>> If we're going to backport this to 8u, I'd like to see the backport based
>> on 11u rather than diverging. It should also credit both JDK-8248901 and
>> JDK-8215144 (I've fixed the missing backport bug for 11u there).
> It is based on jdk11u. It has diverged from that patch but only in order 
> to avoid pulling in compiler changes that have not been shown to be needed.
> 
> 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
>