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

[Andrew Dinn]

On 25/06/2021 14:54, Andrew Hughes wrote:
> Sorry if I was unclear here. I'm not suggesting backporting the whole
> of 8223140. I'm suggesting to follow 11u in adding this hunk:
> 
> --- a/src/hotspot/share/utilities/debug.hpp	Sat Jul 04 08:18:17 2020 +0800
> +++ b/src/hotspot/share/utilities/debug.hpp	Mon Jul 06 21:29:51 2020 +0200
> @@ -67,6 +67,9 @@
>   // For backward compatibility.
>   #define assert(p, ...) vmassert(p, __VA_ARGS__)
>   
> +#define precond(p)   assert(p, "precond")
> +#define postcond(p)  assert(p, "postcond")
> +
>   #ifndef ASSERT
>   #define vmassert_status(p, status, msg)
>   #else
> 
> See:
> 
> https://hg.openjdk.java.net/jdk-updates/jdk11u/rev/d21f0f95e6b4
> 
> This should achieve the same as replacing precond with assert, but also provide
> precond for future backports.

I'm happy for that specific change to be included given that it was 
included in jdk11u. However, cherry-picking bits of one patch and 
placing them in another patch seems like an anti-pattern to me.

>  From a search of the code, it appears only is_simm26, min_simm, max_simm
> and min_simm16 are actually safe to be completely removed in 8u.
> 
> is_simm5, is_simm11, is_simm12, is_simm13 and min_simm13 are indeed all
> used by the SPARC assembler code, as the 11u review already identified and
> fixed. This is clearly not the case in trunk, as the SPARC port has been
> removed.

Oh, indeed, that's my error. I did look at the jdk11u change set but 
then I completely forgot we needed the SPARC changes when I reviewed 
Zhengyu's patch.

> However, I can't approve this, knowing it will break SPARC. So, I
> suggest either those functions are left untouched or (preferably) we
> follow 11u in localising them to the only consumer, the SPARC port.
Yes, I think the patch should be updated to include the jdk11u additions 
to the SPARC assembler.

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