Re: [PATCH V2] Generalized value pass-through for self-recursive function (ipa/pr93203)

[Feng Xue OS <fxue@os.amperecomputing.com>]

Hi Tarmar,

  Since I could not reproduce this ICE with my local testing on Spec2017, would you share your config, or command line options extracted from compilation of perlbench?

Thanks,
Feng

________________________________________
From: gcc-patches-owner@gcc.gnu.org <gcc-patches-owner@gcc.gnu.org> on behalf of Tamar Christina <Tamar.Christina@arm.com>
Sent: Tuesday, February 11, 2020 6:05 PM
To: Feng Xue OS; Martin Jambor; Jan Hubicka; gcc-patches@gcc.gnu.org
Cc: nd
Subject: RE: [PATCH V2] Generalized value pass-through for self-recursive function (ipa/pr93203)

Hi Feng,

This patch (commit a0f6a8cb414b687f22c9011a894d5e8e398c4be0) is causing ICEs in the GCC and perlbench benchmark in Spec2017.

during IPA pass: cp
lto1: internal compiler error: in find_more_scalar_values_for_callers_subset, at ipa-cp.c:4709
0x1698187 find_more_scalar_values_for_callers_subset
        ../.././gcc/ipa-cp.c:4709
0x169f7d3 decide_about_value<tree_node*>
        ../.././gcc/ipa-cp.c:5490
0x169fdc3 decide_whether_version_node
        ../.././gcc/ipa-cp.c:5537
0x169fdc3 ipcp_decision_stage
        ../.././gcc/ipa-cp.c:5718
0x169fdc3 ipcp_driver
        ../.././gcc/ipa-cp.c:5901
Please submit a full bug report,

Thanks,
Tamar

> -----Original Message-----
> From: gcc-patches-owner@gcc.gnu.org <gcc-patches-owner@gcc.gnu.org>
> On Behalf Of Feng Xue OS
> Sent: Monday, February 10, 2020 03:29
> To: Martin Jambor <mjambor@suse.cz>; Jan Hubicka <hubicka@ucw.cz>;
> gcc-patches@gcc.gnu.org
> Subject: Re: [PATCH V2] Generalized value pass-through for self-recursive
> function (ipa/pr93203)
>
> >> -           gcc_checking_assert (item->value);
>
> > I've been staring at this for quite a while, trying to figure out how
> > your patch can put NULL here before I realized it was just a clean-up
> > :-)  Sending such changes independently or pointing them out in the
> > email/ChangeLog makes review easier.
>
> Ok. I'll add some description on this cleanup on ChangeLog.
>
> >> @@ -5564,7 +5610,6 @@ decide_whether_version_node (struct
> cgraph_node *node)
> >>       }
> >>        clone = create_specialized_node (node, known_csts, known_contexts,
> >>                                      aggvals, callers);
> >> -      info = IPA_NODE_REF (node);
>
> > please either drop this change or change it to:
> >
> >   gcc_checking_assert (info == IPA_NODE_REF (node));
>
> > this line of code was actually necessary when adding nodes possibly
> > invalidated addresses of all summaries - like fast_function_summary
> > classes still do.  So if we ever decide to use fast summaries we need
> > a test to remind us that info address must be obtained again.
> Ok. I'm not aware that, will keep the line as original.
>
> Thanks,
> Feng