Looks good to me. 

/Magnus

> 4 sep. 2018 kl. 12:11 skrev Andrew Leonard <andrew_m_leon...@uk.ibm.com>:
> 
> Hi Brian/Goetz, 
> Yes, that seems sensible. I have created a new webrev with fdlibm compiler 
> option disabled, and mediaLib code fixed: 
> http://cr.openjdk.java.net/~aleonard/8209786/webrev.02/ 
> I've built and tested on xLinux and zLinux, with gcc 4.8.4 and 7.3. 
> 
> Are we good to go? 
> Thanks 
> Andrew 
> 
> hg export: 
> # HG changeset patch 
> # User aleonard 
> # Date 1536055438 -3600 
> #      Tue Sep 04 11:03:58 2018 +0100 
> # Node ID c2523f285c503e8f82f1212b38de1cb54093255e 
> # Parent  3ee91722550680c18b977f0e00b1013323b5c9ef 
> 8209786: JDK12 fails to build on s390x with gcc 7.3 
> 
> diff -r 3ee917225506 -r c2523f285c50 make/lib/CoreLibraries.gmk 
> --- a/make/lib/CoreLibraries.gmk        Tue Sep 04 14:47:55 2018 +0800 
> +++ b/make/lib/CoreLibraries.gmk        Tue Sep 04 11:03:58 2018 +0100 
> @@ -68,7 +68,7 @@ 
>        CFLAGS_linux_ppc64le := -ffp-contract=off, \ 
>        CFLAGS_linux_s390x := -ffp-contract=off, \ 
>        CFLAGS_linux_aarch64 := -ffp-contract=off, \ 
> -      DISABLED_WARNINGS_gcc := sign-compare misleading-indentation, \ 
> +      DISABLED_WARNINGS_gcc := sign-compare misleading-indentation 
> array-bounds, \ 
>        DISABLED_WARNINGS_microsoft := 4146 4244 4018, \ 
>        ARFLAGS := $(ARFLAGS), \ 
>        OBJECT_DIR := $(SUPPORT_OUTPUTDIR)/native/$(MODULE)/libfdlibm, \ 
> diff -r 3ee917225506 -r c2523f285c50 
> src/java.desktop/share/native/libmlib_image/mlib_ImageLookUp_Bit.c 
> --- a/src/java.desktop/share/native/libmlib_image/mlib_ImageLookUp_Bit.c      
>   Tue Sep 04 14:47:55 2018 +0800 
> +++ b/src/java.desktop/share/native/libmlib_image/mlib_ImageLookUp_Bit.c      
>   Tue Sep 04 11:03:58 2018 +0100 
> @@ -1,5 +1,5 @@ 
>  /* 
> - * Copyright (c) 2003, Oracle and/or its affiliates. All rights reserved. 
> + * Copyright (c) 2003, 2018, Oracle and/or its affiliates. All rights 
> reserved. 
>   * DO NOT ALTER OR REMOVE COPYRIGHT NOTICES OR THIS FILE HEADER. 
>   * 
>   * This code is free software; you can redistribute it and/or modify it 
> @@ -259,18 +259,18 @@ 
>        } 
>   
>  #ifdef _LITTLE_ENDIAN 
> -      emask = (mlib_u32)((mlib_s32)(-1)) >> ((4 - (size - i)) * 8); 
> +      emask = (~(mlib_u32)0) >> ((4 - (size - i)) * 8); 
>  #else 
> -      emask = (mlib_s32)(-1) << ((4 - (size - i)) * 8); 
> +      emask = (~(mlib_u32)0) << ((4 - (size - i)) * 8); 
>  #endif /* _LITTLE_ENDIAN */ 
>        ((mlib_u32*)da)[0] = (val1 & emask) | (((mlib_u32*)da)[0] &~ emask); 
>   
>  #else /* _NO_LONGLONG */ 
>   
>  #ifdef _LITTLE_ENDIAN 
> -      mlib_u64 emask = (mlib_u64)((mlib_s64)(-1)) >> ((8 - (size - i)) * 8); 
> +      mlib_u64 emask = (~(mlib_u64)0) >> ((8 - (size - i)) * 8); 
>  #else 
> -      mlib_u64 emask = (mlib_s64)(-1) << ((8 - (size - i)) * 8); 
> +      mlib_u64 emask = (~(mlib_u64)0) << ((8 - (size - i)) * 8); 
>  #endif /* _LITTLE_ENDIAN */ 
>   
>        ((mlib_u64*)da)[0] = (((mlib_u64*)dd_array)[sa[0]] & emask) | 
> (((mlib_u64*)da)[0] &~ emask); 
> @@ -395,9 +395,9 @@ 
>        } 
>   
>  #ifdef _LITTLE_ENDIAN 
> -      emask = (mlib_u32)((mlib_s32)(-1)) >> ((4 - (size - i)) * 8); 
> +      emask = (~(mlib_u32)0) >> ((4 - (size - i)) * 8); 
>  #else 
> -      emask = (mlib_s32)(-1) << ((4 - (size - i)) * 8); 
> +      emask = (~(mlib_u32)0) << ((4 - (size - i)) * 8); 
>  #endif /* _LITTLE_ENDIAN */ 
>        ((mlib_u32*)da)[0] = (dd1 & emask) | (((mlib_u32*)da)[0] &~ emask); 
>   
> @@ -413,9 +413,9 @@ 
>        } 
>   
>  #ifdef _LITTLE_ENDIAN 
> -      emask = (mlib_u64)((mlib_s64)(-1)) >> ((8 - (size - i)) * 8); 
> +      emask = (~(mlib_u64)0) >> ((8 - (size - i)) * 8); 
>  #else 
> -      emask = (mlib_s64)(-1) << ((8 - (size - i)) * 8); 
> +      emask = (~(mlib_u64)0) << ((8 - (size - i)) * 8); 
>  #endif /* _LITTLE_ENDIAN */ 
>        ((mlib_u64*)da)[0] = (dd & emask) | (((mlib_u64*)da)[0] &~ emask); 
>   
> @@ -565,9 +565,9 @@ 
>        } 
>   
>  #ifdef _LITTLE_ENDIAN 
> -      emask = (mlib_u32)((mlib_s32)(-1)) >> ((4 - (size - i)) * 8); 
> +      emask = (~(mlib_u32)0) >> ((4 - (size - i)) * 8); 
>  #else 
> -      emask = (mlib_s32)(-1) << ((4 - (size - i)) * 8); 
> +      emask = (~(mlib_u32)0) << ((4 - (size - i)) * 8); 
>  #endif /* _LITTLE_ENDIAN */ 
>        da[0] = (dd & emask) | (da[0] &~ emask); 
>      } 
> 
> 
> 
> 
> 
> 
> 
> Andrew Leonard
> Java Runtimes Development
> IBM Hursley
> IBM United Kingdom Ltd
> Phone internal: 245913, external: 01962 815913
> internet email: andrew_m_leon...@uk.ibm.com 
> 
> 
> 
> 
> From:        Brian Burkhalter <brian.burkhal...@oracle.com> 
> To:        Magnus Ihse Bursie <magnus.ihse.bur...@oracle.com> 
> Cc:        Andrew Leonard <andrew_m_leon...@uk.ibm.com>, "Lindenmaier, Goetz" 
> <goetz.lindenma...@sap.com>, 2d-dev <2d-...@openjdk.java.net>, build-dev 
> <build-...@openjdk.java.net>, core-libs-dev <core-libs-dev@openjdk.java.net>, 
> Philip Race <philip.r...@oracle.com> 
> Date:        31/08/2018 15:44 
> Subject:        Re: [OpenJDK 2D-Dev] RFR JDK-8209786: gcc 7.3 compiler errors 
> on zLinux 
> 
> 
> 
> 
> On Aug 31, 2018, at 2:28 AM, Magnus Ihse Bursie 
> <magnus.ihse.bur...@oracle.com> wrote: 
> 
> Magnus, Philip, Brian, Goetz, can we have a vote? => "Fix" or 
> "DisableWarnings" ? 
> 
> Note that this decision can be different for the two libraries. I'd argue 
> that the maintainer of each library decides. And if so, it seems to be 
> "compiler fix" for libfdlibm, and "source fix" for libmlib_image. 
> 
> I think we can safely say “disable compiler errors” for fdlibm as indicated 
> by Joe Darcy’s comment in the issue (he owns fdlibm), and source code change 
> for mediaLib as Phil indicated in e-mail. 
> 
> Thanks, 
> 
> Brian 
> 
> 
> 
> Unless stated otherwise above:
> IBM United Kingdom Limited - Registered in England and Wales with number 
> 741598. 
> Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU

Reply via email to