RFR(XS): 8245051: c1 is broken if it is compiled by gcc without -fno-lifetime-dse

[Tobias Hartmann]

Looks good to me.

Best regards,
Tobias

On 17.05.20 23:34, Liu, Xin wrote:
> Hi, Kim,
> 
> Thank you to review my patch. I have removed the friend class BlockBegin.  
> Here is the new revision:  http://cr.openjdk.java.net/~xliu/8245051/01/webrev/
> 
> About --with-extra-cflags, I completely agree. However,  how to configure OpenJDK is not under control. There're so many linux distributions.
> In addition, it might cause subtle bugs if the toolchain is not gcc. From my side, I'd like to get rid of undefine behaviors as many as we can.  
> 
> Yes, I still need a reviewer and a sponsor.
>  
> Thanks,
> --lx
> 
> 
> On 5/16/20, 4:29 PM, "Kim Barrett" <kim.barrett at oracle.com> wrote:
> 
>     CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you can confirm the sender and know the content is safe.
> 
> 
> 
>     > On May 15, 2020, at 2:49 PM, Liu, Xin <xxinliu at amazon.com> wrote:
>     >
>     > Hi,
>     >
>     > Please review the following patch.
>     > Jbs: https://bugs.openjdk.java.net/browse/JDK-8245051
>     > Webrev: http://cr.openjdk.java.net/~xliu/8245051/00/webrev/
>     >
>     > I found this problem from centos’s java-11-openjdk. https://git.centos.org/rpms/java-11-openjdk/blob/c7/f/SPECS/java-11-openjdk.spec#_1327
>     > '-std=gnu++98' is not a valid option for cc1. As a result, configure will fail to determine gcc supports -fno-lifetime-dse or not.
>     > C1 acts weird if it is compiled by GCC without that flag.
>     >
>     > After then, I built hotspot with -fsanitize=undefined. I found some interesting points for c1.  With this patch, I can build the whole openjdk without -fno-lifetime-dse.
>     > I've tested hotspot:tier1 and "gtest:all". Even though everything looks fine, I don't think we reach a point to lift "-fno-lifetime-dse".  This patch just attempts to fix C1.
>     >
>     > Thanks,
>     > --lx
> 
>     Thanks for finding these.
> 
>     The change to c1_ValueMap.cpp looks good to me.
> 
>     The changes to c1_Instruction.hpp also look good to me.  In addition,
>     Instruction no longer needs to befriend BlockBegin.  Please make that
>     change before pushing.
> 
>     However, I have to question the configure options being used. Some
>     (many? all?) of the options being passed in as --with-extra-cflags and
>     friends are already used properly by the build system, and don't need
>     to be added that way, and may even cause problems. That's what's
>     happening with -std=gnu++98, which is not a valid option for compling
>     C code, but the build system will use it when compiling C++ code.
>     That's been true since JDK 9 (JDK-8156980).
> 
>     We definitely cannot remove -fno-lifetime-dse.  There are a number of
>     other places where we're doing weird things in allocators/deallocators
>     that are similarly bogus.  The ResourceObj allocation_type stuff is
>     just one example; I'm pretty sure there are others.
> 
>     I'm running these changes (including the friendship removal) through
>     more extensive testing. I'll report back when done, but I don't expect
>     that to find anything.
> 
>     It looks like you will need a sponsor? Hopefully a compiler person
>     will also review and sponsor.
> 
>