On 2018-03-20 14:54, Edward Nevill wrote:
On Tue, 2018-03-20 at 08:39 +0100, Erik Helin wrote:
Please review the following webrev
Bugid: https://bugs.openjdk.java.net/browse/JDK-8199138
Webrev: http://cr.openjdk.java.net/~enevill/8199138/webrev.00
    32 # First, filter out everything that doesn't begin with "aarch64-"
    33 if ! echo $* | grep '^aarch64-\|^riscv64-' >/dev/null ; then

Could you please update the comment on line 32 to say the same thing as
the code?

Hi Eirk,

Thanks for this. I have updated the webrev with the above comment.

http://cr.openjdk.java.net/~enevill/8199138/webrev.01
I note that in platform.m4 (sorry I didn't say this earlier), you set the CPU_ARCH to riscv64 as well, and not just riscv. Now I don't know how likely it is that OpenJDK will ever support the 32-bit version of riscv, but it seems like it would make more sense to define the CPU_ARCH as "riscv", and the CPU as "riscv64".

It's just a minor thing, if you like it the way it is, keep it.

/Magnus


I have also fixed a problem encountered with the submit-hs repo where the build 
machine had older headers which did not define EM_RISCV.

The solution is to define EM_RISCV if not already defined as is done for 
aarch64.

IE.

  #ifndef EM_AARCH64
    #define EM_AARCH64    183               /* ARM AARCH64 */
  #endif
+#ifndef EM_RISCV
+  #define EM_RISCV      243
+#endif

This now passes the submit-hs tests.

Does this look OK to push now?

Thanks,
Ed.


Reply via email to