On Wed, Jan 25, 2017 at 10:49 AM, Richard Biener <rguent...@suse.de> wrote:
> On Wed, 25 Jan 2017, Yuri Gribov wrote:
>
>> Hi all,
>>
>> This fixes inefficient bitfield code reported in
>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=67328
>>
>> Bootstrapped and regtested on x86_64.
>>
>> Ok for trunk?
>
> This isn't a regression fix and thus not appropriate at this stage.

Definitely, just wanted to get initial round of comments. Thanks for
review, will get back with updated patch at stage 1.

> Some comments on the patch:
>
> +/* A & (2**N - 1) <= 2**K - 1 -> ~(A & (2**N - 2**K)
> +   A & (2**N - 1) <  2**K     -> ~(A & (2**N - 2**K)
> +   A & (2**N - 1) >= 2**K     -> A & (2**N - 2**K)
> +   A & (2**N - 1) >  2**K - 1 -> A & (2**N - 2**K)
> + */
>
> you miss the != 0/== 0 in the result (and the ~ is redundant then).
>
> Note that A & (2**N - 1) >= 2**K should already have been simplified
> to A & (2**N - 1) >  2**K - 1 (we canonicalize to smaller constants).
>
> +  (if (TYPE_UNSIGNED (TREE_TYPE (@0)) && tree_fits_uhwi_p (@2) &&
> tree_fits_uhwi_p (@3))
> +   (with
> +    {
>
> I think you should restrict this to INTEGRAL_TYPE_P types.
>
> Please use wide-ints so you do not restrict yourself to fits_uhwi_p
> values.
>
> Thanks,
> Richard.

Reply via email to