This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
Re: [PATCH][Middle-end]change char type to unsigned char type when expanding strcmp/strncmp
- From: Qing Zhao <qing dot zhao at oracle dot com>
- To: Jakub Jelinek <jakub at redhat dot com>
- Cc: gcc Patches <gcc-patches at gcc dot gnu dot org>, jeff Law <law at redhat dot com>, richard Biener <rguenther at suse dot de>
- Date: Thu, 19 Jul 2018 15:48:58 -0500
- Subject: Re: [PATCH][Middle-end]change char type to unsigned char type when expanding strcmp/strncmp
- References: <F0A510E7-248E-4EAF-81D4-1688B8C947B6@oracle.com> <20180719173100.GU7166@tucnak> <94C14E05-9285-4B91-8073-9501CEDC2A15@oracle.com> <20180719192438.GW7166@tucnak>
> On Jul 19, 2018, at 2:24 PM, Jakub Jelinek <jakub@redhat.com> wrote:
>
> On Thu, Jul 19, 2018 at 02:06:16PM -0500, Qing Zhao wrote:
>>> If you expand it as (int) ((unsigned char *)p)[n] - (int) ((unsigned char *)q)[n]
>>> then aren't you relying on int type to have wider precision than unsigned char
>>> (or unit_mode being narrower than mode)?
>>
>> do you imply that we should only expand it as (int) ((unsigned char *)p)[n] - (int) ((unsigned char *)q)[n] when we are sure
>> int type is wider than unsigned char?
>
> Yes.
>
>>> I don't see anywhere where you'd
>>> give up on doing the inline expansion on targets where e.g. lowest
>>> addressable unit would be 16-bit and int would be 16-bit too.
>>
>> even on this targets, is char type still 8-bit?
>> then int type is still wider than char?
>
> C requires that int is at least 16-bit wide, so the sizeof (int) == sizeof
> (char) case is only possible say with 16-bit char and 16-bit int, or 32-bit
> char and 32-bit int etc.
>
>>> On targets where int is as wide as char, one would need to expand it instead
>>> as something like:
>>> if (((unsigned char *)p)[n] == ((unsigned char *)q)[n]) loop;
>>> ret = ((unsigned char *)p)[n] < ((unsigned char *)q)[n] ? -1 : 1;
>>> or similar or just use the library routine.
>>
>>
>> even when int type is as wide as char, expand it as (int) ((unsigned char *)p)[n] - (int) ((unsigned char *)q)[n]
>> should still be correct (even though not optimal), doesn’t it?
>
> No. Consider p[n] being e.g. 1 and q[n] being __SCHAR_MAX__ + 3U and 16-bit
> int and 16-bit char. Then (unsigned char) 0x0001 < (unsigned char) 0x8002,
> so it should return a negative number. But (int) (0x0001U - 0x8002U) is
> 0x7fff, which is a positive int. Now, if int is 17-bit and char is 16-bit,
> this works fine, because is then -0x8001 and thus negative.
Okay, I see now.
really appreciate for your detailed explanation.
>
> The above really works only if int is at least one bit wider than unsigned
> char.
Then, I will add a check to exclude the inlining when int is NOT wider than unsigned char on the target.
is the following the correct check: (exp is the call to strcmp)
if (CHAR_TYPE_SIZE >= TYPE_PRECISION (TREE_TYPE (exp)))
Thanks.
Qing