From: Jakub Jelinek <jakub@redhat.com>
To: Wilco Dijkstra <Wilco.Dijkstra@arm.com>
Cc: GCC Patches <gcc-patches@gcc.gnu.org>
Subject: Re: [PATCH] Fix ctz issues (PR93231)
Date: Wed, 15 Jan 2020 16:59:00 -0000 [thread overview]
Message-ID: <20200115161405.GV10088@tucnak> (raw)
In-Reply-To: <AM5PR0801MB20354370D3C5368CD426169083370@AM5PR0801MB2035.eurprd08.prod.outlook.com>
On Wed, Jan 15, 2020 at 03:50:01PM +0000, Wilco Dijkstra wrote:
> The multiply can be signed or unsigned, and the immediate can be positive or
> negative, but the shift must be unsigned indeed. I thought the match.pd
> pattern only allows unsigned shifts, but the shift operator allows signed shifts
> too, so I've added an unsigned check on the input_type.
I guess in theory if there is some multiplication constant that for all the
power of twos + 0 would give a positive result even arithetic right shift
would be ok, and we could also in the match.pd pattern use convert? or
nop_convert? to allow signed multiplication with a cast to unsigned after
it, but guess it isn't that important and we can just go with TYPE_UNSIGNED
requirement for now.
> >> - if (TREE_CODE (ctor) == STRING_CST)
> >> + if (TREE_CODE (ctor) == STRING_CST && TYPE_PRECISION (type) == 8)
> >
> > Isn't another precondition that BITS_PER_UNIT is 8 (because STRING_CSTs are
> > really bytes)?
>
> I've used CHAR_TYPE_SIZE instead of 8, but I don't think GCC supports anything other
> than BITS_PER_UNIT == 8 and CHAR_TYPE_SIZE == 8. GCC uses memcmp/strlen
> on STRING_CST (as well as direct accesses as 'char') which won't work if the host and
> target chars are not the same size.
Yes, we even don't support anything else ATM, but are trying to document in
the source when we rely on it.
So e.g. VIEW_CONVERT_EXPR folding has
/* Check that the host and target are sane. */
if (CHAR_BIT != 8 || BITS_PER_UNIT != 8)
return NULL_TREE;
etc.
> 2020-01-15 Wilco Dijkstra <wdijkstr@arm.com>
>
> PR tree-optimization/93231
> * tree-ssa-forwprop.c
> (optimize_count_trailing_zeroes): Check input_type is unsigned.
No need to wrap line before (optimize_count_trailing_zeroes), please use:
* tree-ssa-forwprop.c (optimize_count_trailing_zeroes): Check
input_type is unsigned.
> +int ctz2 (unsigned x)
> +{
> + const int u = 0;
> + static short table[64] =
> + {
> + 32, 0, 1,12, 2, 6, u,13, 3, u, 7, u, u, u, u,14,
> + 10, 4, u, u, 8, u, u,25, u, u, u, u, u,21,27,15,
> + 31,11, 5, u, u, u, u, u, 9, u, u,24, u, u,20,26,
> + 30, u, u, u, u,23, u,19,29, u,22,18,28,17,16, u
This is just a GNU extension that we accept it in C, more portable would be
enum U { u = 0; };
or #define u 0
But no big deal.
> @@ -1894,7 +1894,7 @@ optimize_count_trailing_zeroes (tree array_ref, tree x, tree mulc,
> if (TREE_CODE (ctor) == CONSTRUCTOR)
> return check_ctz_array (ctor, val, zero_val, shiftval, input_bits);
>
> - if (TREE_CODE (ctor) == STRING_CST)
> + if (TREE_CODE (ctor) == STRING_CST && TYPE_PRECISION (type) == CHAR_TYPE_SIZE)
Too long line, please wrap.
Ok for trunk with those nits tweaked.
Jakub
prev parent reply other threads:[~2020-01-15 16:14 UTC|newest]
Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-01-13 17:53 Wilco Dijkstra
2020-01-13 18:16 ` Jakub Jelinek
2020-01-15 16:46 ` Wilco Dijkstra
2020-01-15 16:59 ` Jakub Jelinek [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20200115161405.GV10088@tucnak \
--to=jakub@redhat.com \
--cc=Wilco.Dijkstra@arm.com \
--cc=gcc-patches@gcc.gnu.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).