Re: PR97849: aarch64: ICE (segfault) during GIMPLE pass: ifcvt

[Richard Biener <rguenther@suse.de>]

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?

OK with that change.
Thanks,
Richard.

> Thanks,
> Prathamesh
> 

-- 
Richard Biener <rguenther@suse.de>
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Felix Imend