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

[Hohensee, Paul]

I'll push it for Xin once Kim comes back with an approval.

Thanks,
Paul

On 5/18/20, 5:22 AM, "hotspot-compiler-dev on behalf of Tobias Hartmann" <hotspot-compiler-dev-bounces at openjdk.java.net on behalf of tobias.hartmann at oracle.com> wrote:

    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.
    >
    >