This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH, PR69110] Don't return NULL access_fns in dr_analyze_indices


On 13/01/16 09:42, Richard Biener wrote:
On Tue, 12 Jan 2016, Tom de Vries wrote:

>On 12/01/16 14:04, Richard Biener wrote:
> >On Tue, 12 Jan 2016, Tom de Vries wrote:
> >
> > >On 12/01/16 12:22, Richard Biener wrote:
> > > >Doesnt' the same issue apply to
> > > >
> > > > > >unsigned int *p;
> > > > > >
> > > > > >static void __attribute__((noinline, noclone))
> > > > > >foo (void)
> > > > > >{
> > > > > >    unsigned int z;
> > > > > >
> > > > > >    for (z = 0; z < N; ++z)
> > > > > >      ++(*p);
> > > > > >}
> > > >thus when we have a MEM_REF[p_1]?  SCEV will not analyze
> > > >its evolution to a POLYNOMIAL_CHREC and thus access_fns will
> > > >be NULL again.
> > > >
> > >
> > >I didn't manage to trigger this scenario, though I could probably make it
> > >happen by modifying ftree-loop-im to work in one case (the load of the
> > >value
> > >of p) but not the other (the *p load and store).
> > >
> > > >I think avoiding a NULL access_fns is ok but it should be done
> > > >unconditionally, not only for the DECL_P case.
> > >
> > >Ok, I'll retest and commit this patch.
> >
> >Please add a comment as well.
>
>Patch updated with comment.
>
>During testing however, I ran into two testsuite regressions:
>
>1.
>
>-PASS: gfortran.dg/graphite/pr39516.f   -O  (test for excess errors)
>+FAIL: gfortran.dg/graphite/pr39516.f   -O  (internal compiler error)
>+FAIL: gfortran.dg/graphite/pr39516.f   -O  (test for excess errors)
>
>AFAIU, this is a duplicate of PR68976.
>
>Should I wait with committing the patch until PR68976 is fixed?
Yes - we shouldn't introduce new testsuite regressions willingly at this
point.


I've looked in more detail at both PR68976 and the pr39516.f regression using this patch, and I now think they are independent.

Furthermore, I managed to reproduce the pr39516.f regression without the patch (filed as PR69292 - '[graphite] ICE with -floop-nest-optimize').

So, AFAIU, committing this patch does not introduce a new type of ICE, but it makes it more likely that you run into it.

Does that still mean we need to wait with committing, and first fix PR69292?

Thanks,
- Tom


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]