On Thu, 8 Oct 2020, Jakub Jelinek wrote:
> On Thu, Oct 08, 2020 at 11:46:36AM +0200, Aldy Hernandez via Gcc-patches
> wrote:
> > This was a pasto by yours truly while porting the __builtin code from
> > vr_values::extract_range_basic.
> >
> > I've tested Andrew's patch, and pushed it.
>
> Note for next time, I think you should use --author when committing a patch
> on someone else's behalf.
>
> And no testcase was included, I'm including one below.
>
> Anyway, this PR and the other CTZ related discussions led me to discover a
> bug I've made earlier, CLZ/CTZ builtins have unsigned arguments and e.g.
> both the vr-values.cc and now gimple-range.cc code heavily relies on that,
> but __builtin_ffs has a signed operand and this optimization was incorrectly
> making the operand signed too, so I guess it would greatly confuse VRP in
> some cases.
>
> Ok for trunk if it passes bootstrap/regtest?
OK.
Richard.
> 2020-10-08 Jakub Jelinek <ja...@redhat.com>
>
> PR tree-optimization/97325
> * match.pd (FFS(nonzero) -> CTZ(nonzero) + 1): Cast argument to
> corresponding unsigned type.
>
> * gcc.c-torture/execute/pr97325.c: New test.
>
> --- gcc/match.pd.jj 2020-10-01 10:40:10.057756683 +0200
> +++ gcc/match.pd 2020-10-08 14:39:27.347249559 +0200
> @@ -6196,7 +6196,8 @@ (define_operator_list COND_TERNARY
> (if (INTEGRAL_TYPE_P (TREE_TYPE (@0))
> && direct_internal_fn_supported_p (IFN_CTZ, TREE_TYPE (@0),
> OPTIMIZE_FOR_SPEED))
> - (plus (CTZ:type @0) { build_one_cst (type); })))
> + (with { tree utype = unsigned_type_for (TREE_TYPE (@0)); }
> + (plus (CTZ:type (convert:utype @0)) { build_one_cst (type); }))))
> #endif
>
> (for ffs (BUILT_IN_FFS BUILT_IN_FFSL BUILT_IN_FFSLL
> --- gcc/testsuite/gcc.c-torture/execute/pr97325.c.jj 2020-10-08
> 14:29:59.740440126 +0200
> +++ gcc/testsuite/gcc.c-torture/execute/pr97325.c 2020-10-08
> 14:27:22.518708841 +0200
> @@ -0,0 +1,15 @@
> +/* PR tree-optimization/97325 */
> +
> +unsigned long long
> +foo (unsigned long long c)
> +{
> + return c ? __builtin_ffs (-(unsigned short) c) : 0;
> +}
> +
> +int
> +main ()
> +{
> + if (foo (2) != 2)
> + __builtin_abort ();
> + return 0;
> +}
>
>
> Jakub
>
>
--
Richard Biener <rguent...@suse.de>
SUSE Software Solutions Germany GmbH, Maxfeldstrasse 5, 90409 Nuernberg,
Germany; GF: Felix Imend
