On Wed, 18 Nov 2020 at 19:13, Richard Biener <rguent...@suse.de> wrote:
>
> On Wed, 18 Nov 2020, Prathamesh Kulkarni wrote:
>
> > Hi,
> > For the following test-case (slightly reduced from PR)
> > int a, b, c;
> >
> > int g() {
> > char i = 0;
> > for (c = 0; c <= 8; c++)
> > --i;
> >
> > while (b) {
> > _Bool f = i <= 0;
> > a = (a == 0) ? 0 : f / a;
> > }
> > }
> >
> > The compiler segfaults with -O1 -march=armv8.2-a+sve in ifcvt_local_dce.
> >
> > IIUC, the issue here is that tree-if-conv.c:predicate_rhs_code
> > processes the following statement:
> > iftmp.2_7 = a_lsm.10_11 != 0 ? iftmp.2_13 : 0;
> > and records <iftmp.2_7, iftmp.2_13> mapping.
> >
> > However RPO VN eliminates iftmp.2_13:
> > Removing dead stmt iftmp.2_13 = .COND_DIV (_29, _4, a_lsm.10_11, 0);
> >
> > and we end up replacing iftmp.2_7 with a dead ssa_name in ifcvt_local_dce:
> > FOR_EACH_VEC_ELT (redundant_ssa_names, i, name_pair)
> > replace_uses_by (name_pair->first, name_pair->second);
> > redundant_ssa_names.release ();
> >
> > resulting in incorrect IR, and segfault down the line.
> >
> > To avoid clashing of RPO VN with redunant_ssa_names, the patch simply moves
> > ifcvt_local_dce before do_rpo_vn, which avoids the segfault.
> > Does that look OK ?
> > (Altho I guess, doing DCE after VN is better in principle)
>
> Yes, I'd say just moving
>
> FOR_EACH_VEC_ELT (redundant_ssa_names, i, name_pair)
> replace_uses_by (name_pair->first, name_pair->second);
> redundant_ssa_names.release ();
>
> before rpo_vn makes more sense, no?
Ah indeed, thanks for the suggestion.
Committed the attached patch after bootstrap+test on x86_64, and
cross testing on aarch64*-*-* and arm*-*-*.Thanks, Prathamesh > > OK with that change. > Thanks, > Richard. > > > Thanks, > > Prathamesh > > > > -- > Richard Biener <rguent...@suse.de> > SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg, > Germany; GF: Felix Imend
pr97849-3.diff
Description: Binary data
