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

[Liu, Xin]

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.