On Tue, 28 Apr 2020, Jakub Jelinek wrote: > On Tue, Apr 28, 2020 at 08:54:42AM +0200, Richard Biener wrote: > > > @@ -1311,7 +1311,7 @@ (define_operator_list COND_TERNARY > > > We combine the above two cases by using a conditional convert. */ > > > (for bitop (bit_and bit_ior bit_xor) > > > (simplify > > > - (bitop (convert @0) (convert? @1)) > > > + (bitop (convert@2 @0) (convert?@3 @1)) > > > (if (((TREE_CODE (@1) == INTEGER_CST > > > && INTEGRAL_TYPE_P (TREE_TYPE (@0)) > > > && int_fits_type_p (@1, TREE_TYPE (@0))) > > > @@ -1330,8 +1330,24 @@ (define_operator_list COND_TERNARY > > > || GET_MODE_CLASS (TYPE_MODE (type)) != MODE_INT > > > /* Or if the precision of TO is not the same as the precision > > > of its mode. */ > > > - || !type_has_mode_precision_p (type))) > > > - (convert (bitop @0 (convert @1)))))) > > > + || !type_has_mode_precision_p (type) > > > + /* In GIMPLE, getting rid of 2 conversions for one new results > > > + in smaller IL. */ > > > + || (GIMPLE > > > + && TREE_CODE (@1) != INTEGER_CST > > > + && tree_nop_conversion_p (type, TREE_TYPE (@0)) > > > + && single_use (@2) > > > + && single_use (@3)))) > > > > I think you don't need explicit single_use checks here but could use :s > > in the pattern since the result expression will be "complex" and thus > > the :s isn't elided. > > I would have used :s if I were to do a separate simplify for it, but I > didn't want to affect the other cases. So, do we want to use > :s even for those? E.g. not sure for the INTEGER_CST case what would > (convert?@3:s @1) do when the bitop has INTEGER_CST as second operand.
I think we generally want to use :s, but OTOH even for your new case, since you restrict to nop-conversions, :s isn't strictly needed since VN should know how to materialize nop-converted variants. I think it's different for the 2nd case you add since there we deal with bitops in different sign. So maybe just remove the single_use () calls? > Or should I e.g. move this GIMPLE rule as a separate simplify (under the > same for) and then use :s in there? That wouldn't work, if a structural pattern matched others are not considered so it wouldn't ever be used. genmatch even warns for this I think. Richard.
