"duanbo (C)" <duan...@huawei.com> writes: >> -----Original Message----- >> From: Richard Sandiford [mailto:richard.sandif...@arm.com] >> Sent: Wednesday, September 30, 2020 6:38 PM >> To: duanbo (C) <duan...@huawei.com> >> Cc: GCC Patches <gcc-patches@gcc.gnu.org> >> Subject: Re: [PATCH PR96757] aarch64: ICE during GIMPLE pass: vect >> >> Thanks for the update, looks good apart from… >> >> "duanbo (C)" <duan...@huawei.com> writes: >> > @@ -4361,7 +4391,7 @@ vect_recog_mask_conversion_pattern (vec_info >> *vinfo, >> > if (known_eq (TYPE_VECTOR_SUBPARTS (vectype1), >> > TYPE_VECTOR_SUBPARTS (vectype2)) >> > && (TREE_CODE (rhs1) == SSA_NAME >> > - || rhs1_type == TREE_TYPE (TREE_OPERAND (rhs1, 0)))) >> > + || !rhs1_op0_type || !rhs1_op1_type)) >> > return NULL; >> >> …I think this should be: >> >> && (TREE_CODE (rhs1) == SSA_NAME >> || (!rhs1_op0_type && !rhs1_op1_type)) >> >> i.e. punt only if both types are already OK. If one operand wants a specific >> mask type, we should continue to the code below and attach the chosen >> type to the comparison. >> >> Although I guess this simplifies to: >> >> if (known_eq (TYPE_VECTOR_SUBPARTS (vectype1), >> TYPE_VECTOR_SUBPARTS (vectype2)) >> && !rhs1_op0_type >> && !rhs1_op1_type) >> return NULL; >> >> (I think the comment above the code is still accurate with this change.) >> >> > @@ -4393,7 +4423,16 @@ vect_recog_mask_conversion_pattern >> (vec_info *vinfo, >> > if (TREE_CODE (rhs1) != SSA_NAME) >> > { >> > tmp = vect_recog_temp_ssa_var (TREE_TYPE (rhs1), NULL); >> > - pattern_stmt = gimple_build_assign (tmp, rhs1); >> > + if (rhs1_op0_type && TYPE_PRECISION (rhs1_op0_type) >> > + != TYPE_PRECISION (rhs1_type)) >> > + rhs1_op0 = build_mask_conversion (vinfo, rhs1_op0, >> > + vectype2, stmt_vinfo); >> > + if (rhs1_op1_type && TYPE_PRECISION (rhs1_op1_type) >> > + != TYPE_PRECISION (rhs1_type)) >> >> Very minor -- I would have fixed this up before committing if it wasn't for >> the >> above -- but: GCC formatting is instead: >> >> if (rhs1_op1_type >> && TYPE_PRECISION (rhs1_op1_type) != TYPE_PRECISION >> (rhs1_type)) >> >> LGTM with those changes, thanks. >> >> Richard >> >> > + rhs1_op1 = build_mask_conversion (vinfo, rhs1_op1, >> > + vectype2, stmt_vinfo); >> > + pattern_stmt = gimple_build_assign (tmp, TREE_CODE (rhs1), >> > + rhs1_op0, rhs1_op1); >> > rhs1 = tmp; >> > append_pattern_def_seq (vinfo, stmt_vinfo, pattern_stmt, >> vectype2, >> > rhs1_type); > > Sorry for the late reply. > I have modified the patch according to your suggestion, and it works well.
Looks good, thanks. Pushed to trunk. Richard
