On Wed, Jun 30, 2021 at 9:15 PM Qing Zhao via Gcc-patches
<gcc-patches@gcc.gnu.org> wrote:
>
>
>
> > On Jun 30, 2021, at 1:59 PM, Richard Biener <rguent...@suse.de> wrote:
> >
> > On June 30, 2021 8:07:43 PM GMT+02:00, Qing Zhao <qing.z...@oracle.com>
> > wrote:
> >>
> >>
> >>> On Jun 30, 2021, at 12:36 PM, Richard Biener <rguent...@suse.de>
> >> wrote:
> >>>
> >>> On June 30, 2021 7:20:18 PM GMT+02:00, Andrew Pinski
> >> <pins...@gmail.com> wrote:
> >>>> On Wed, Jun 30, 2021 at 8:47 AM Qing Zhao via Gcc-patches
> >>>> <gcc-patches@gcc.gnu.org> wrote:
> >>>>>
> >>>>> I came up with a very simple testing case that can repeat the same
> >>>> issue:
> >>>>>
> >>>>> [qinzhao@localhost gcc]$ cat t.c
> >>>>> extern void bar (int);
> >>>>> void foo (int a)
> >>>>> {
> >>>>> int i;
> >>>>> for (i = 0; i < a; i++) {
> >>>>> if (__extension__({int size2;
> >>>>> size2 = 4;
> >>>>> size2 > 5;}))
> >>>>> bar (a);
> >>>>> }
> >>>>> }
> >>>>
> >>>> You should show the full dump,
> >>>> What we have is the following:
> >>>>
> >>>>
> >>>>
> >>>> size2_3 = PHI <size2_1(D), size2_13>
> >>>> <bb 3> :
> >>>>
> >>>> size2_12 = .DEFERRED_INIT (size2_3, 2);
> >>>> size2_13 = 4;
> >>>>
> >>>> So CCP decides to propagate 4 into the PHI and then decides
> >> size2_1(D)
> >>>> is undefined so size2_3 is then considered 4 and propagates it into
> >>>> the .DEFERRED_INIT.
> >>>
> >>> Which means the DEFERED_INIT is inserted at the wrong place.
> >>
> >> Then, where is the correct place for “.DEFERRED_INIT(size2,2)?
> >>
> >> The variable “size2” is a block scope variable which is declared inside
> >> the “if” condition:
> >
> > But that's obviously not how it behaves
> > During into SSA phase since we're inserting a PHI for it - and we're
> > inserting it because of the use in the DEFERED_INIT call. I suppose you
> > need to fiddle with the SSA rewrite and avoid treating the use as a use but
> > only for the purpose of inserting PHIs...
>
> Please see my other email on the new small testing case without
> -ftrivial-auto-var-init. The same issue in SSA with that testing case even
> without -ftrivial-auto-var-init.
> It looks like an existing bug to me in SSA.
It's not a bug in SSA, it's at most a missed optimization there. But
with -ftrivial-auto-var-init it
becomes a correctness issue. I think the idea avoiding the USE of the
variable in .DEFERRED_INIT
and instead passing the init size is a good one and should avoid this
case (hopefully).
Richard.
> Let me know if I still miss anything
>
> Qing
> >
> > You might be able to construct a testcase which has a use before the real
> > init where then the optimistic CCP propagation will defeat the DEFERED_INIT
> > otherwise.
> >
> > I'd need to play with the actual patch to find a good solution to this
> > problem.
> >
> > Richard.
> >
>
