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

[Andrew Dinn]

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