* [PATCH] small _BitInt tweaks
@ 2023-09-11 17:42 Jakub Jelinek
2023-09-12 10:27 ` Richard Biener
0 siblings, 1 reply; 6+ messages in thread
From: Jakub Jelinek @ 2023-09-11 17:42 UTC (permalink / raw)
To: Richard Biener; +Cc: gcc-patches
Hi!
When discussing PR111369 with Andrew Pinski, I've realized that
I haven't added BITINT_TYPE handling to range_check_type. Right now
(unsigned) max + 1 == (unsigned) min for signed _BitInt,l so I think we
don't need to do the extra hops for BITINT_TYPE (though possibly we don't
need them for INTEGER_TYPE either in the two's complement word and we don't
support anything else, though I really don't know if Ada or some other
FEs don't create weird INTEGER_TYPEs).
And, also I think it is undesirable when being asked for signed_type_for
of unsigned _BitInt(1) (which is valid) to get signed _BitInt(1) (which is
invalid, the standard only allows signed _BitInt(2) and larger), so the
patch returns 1-bit signed INTEGER_TYPE for those cases.
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
2023-09-11 Jakub Jelinek <jakub@redhat.com>
gcc/
* tree.cc (signed_or_unsigned_type_for): Return INTEGER_TYPE for
signed variant of unsigned _BitInt(1).
* fold-const.cc (range_check_type): Handle BITINT_TYPE like
OFFSET_TYPE.
gcc/c-family/
* c-common.cc (c_common_signed_or_unsigned_type): Return INTEGER_TYPE
for signed variant of unsigned _BitInt(1).
--- gcc/tree.cc.jj 2023-09-06 17:50:30.707589026 +0200
+++ gcc/tree.cc 2023-09-11 16:24:58.749625569 +0200
@@ -11096,7 +11096,7 @@ signed_or_unsigned_type_for (int unsigne
else
return NULL_TREE;
- if (TREE_CODE (type) == BITINT_TYPE)
+ if (TREE_CODE (type) == BITINT_TYPE && (unsignedp || bits > 1))
return build_bitint_type (bits, unsignedp);
return build_nonstandard_integer_type (bits, unsignedp);
}
--- gcc/c-family/c-common.cc.jj 2023-09-06 17:34:24.467254960 +0200
+++ gcc/c-family/c-common.cc 2023-09-11 16:24:07.873300311 +0200
@@ -2739,7 +2739,9 @@ c_common_signed_or_unsigned_type (int un
|| TYPE_UNSIGNED (type) == unsignedp)
return type;
- if (TREE_CODE (type) == BITINT_TYPE)
+ if (TREE_CODE (type) == BITINT_TYPE
+ /* signed _BitInt(1) is invalid, avoid creating that. */
+ && (unsignedp || TYPE_PRECISION (type) > 1))
return build_bitint_type (TYPE_PRECISION (type), unsignedp);
#define TYPE_OK(node) \
--- gcc/fold-const.cc.jj 2023-09-11 11:05:47.473728473 +0200
+++ gcc/fold-const.cc 2023-09-11 16:28:06.052141516 +0200
@@ -5565,7 +5565,12 @@ range_check_type (tree etype)
else
return NULL_TREE;
}
- else if (POINTER_TYPE_P (etype) || TREE_CODE (etype) == OFFSET_TYPE)
+ else if (POINTER_TYPE_P (etype)
+ || TREE_CODE (etype) == OFFSET_TYPE
+ /* Right now all BITINT_TYPEs satisfy
+ (unsigned) max + 1 == (unsigned) min, so no need to verify
+ that like for INTEGER_TYPEs. */
+ || TREE_CODE (etype) == BITINT_TYPE)
etype = unsigned_type_for (etype);
return etype;
}
Jakub
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] small _BitInt tweaks
2023-09-11 17:42 [PATCH] small _BitInt tweaks Jakub Jelinek
@ 2023-09-12 10:27 ` Richard Biener
2023-09-12 11:26 ` Jakub Jelinek
0 siblings, 1 reply; 6+ messages in thread
From: Richard Biener @ 2023-09-12 10:27 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: gcc-patches
On Mon, 11 Sep 2023, Jakub Jelinek wrote:
> Hi!
>
> When discussing PR111369 with Andrew Pinski, I've realized that
> I haven't added BITINT_TYPE handling to range_check_type. Right now
> (unsigned) max + 1 == (unsigned) min for signed _BitInt,l so I think we
> don't need to do the extra hops for BITINT_TYPE (though possibly we don't
> need them for INTEGER_TYPE either in the two's complement word and we don't
> support anything else, though I really don't know if Ada or some other
> FEs don't create weird INTEGER_TYPEs).
> And, also I think it is undesirable when being asked for signed_type_for
> of unsigned _BitInt(1) (which is valid) to get signed _BitInt(1) (which is
> invalid, the standard only allows signed _BitInt(2) and larger), so the
> patch returns 1-bit signed INTEGER_TYPE for those cases.
I think the last bit is a bit surprising - do the frontends use
signed_or_unsigned_type_for and would they be confused if getting
back an INTEGER_TYPE here?
The range_check_type bits are OK. For the tree.cc part I think
the middle-end can just handle signed 1-bit BITINT fine?
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
>
> 2023-09-11 Jakub Jelinek <jakub@redhat.com>
>
> gcc/
> * tree.cc (signed_or_unsigned_type_for): Return INTEGER_TYPE for
> signed variant of unsigned _BitInt(1).
> * fold-const.cc (range_check_type): Handle BITINT_TYPE like
> OFFSET_TYPE.
> gcc/c-family/
> * c-common.cc (c_common_signed_or_unsigned_type): Return INTEGER_TYPE
> for signed variant of unsigned _BitInt(1).
>
> --- gcc/tree.cc.jj 2023-09-06 17:50:30.707589026 +0200
> +++ gcc/tree.cc 2023-09-11 16:24:58.749625569 +0200
> @@ -11096,7 +11096,7 @@ signed_or_unsigned_type_for (int unsigne
> else
> return NULL_TREE;
>
> - if (TREE_CODE (type) == BITINT_TYPE)
> + if (TREE_CODE (type) == BITINT_TYPE && (unsignedp || bits > 1))
> return build_bitint_type (bits, unsignedp);
> return build_nonstandard_integer_type (bits, unsignedp);
> }
> --- gcc/c-family/c-common.cc.jj 2023-09-06 17:34:24.467254960 +0200
> +++ gcc/c-family/c-common.cc 2023-09-11 16:24:07.873300311 +0200
> @@ -2739,7 +2739,9 @@ c_common_signed_or_unsigned_type (int un
> || TYPE_UNSIGNED (type) == unsignedp)
> return type;
>
> - if (TREE_CODE (type) == BITINT_TYPE)
> + if (TREE_CODE (type) == BITINT_TYPE
> + /* signed _BitInt(1) is invalid, avoid creating that. */
> + && (unsignedp || TYPE_PRECISION (type) > 1))
> return build_bitint_type (TYPE_PRECISION (type), unsignedp);
>
> #define TYPE_OK(node) \
> --- gcc/fold-const.cc.jj 2023-09-11 11:05:47.473728473 +0200
> +++ gcc/fold-const.cc 2023-09-11 16:28:06.052141516 +0200
> @@ -5565,7 +5565,12 @@ range_check_type (tree etype)
> else
> return NULL_TREE;
> }
> - else if (POINTER_TYPE_P (etype) || TREE_CODE (etype) == OFFSET_TYPE)
> + else if (POINTER_TYPE_P (etype)
> + || TREE_CODE (etype) == OFFSET_TYPE
> + /* Right now all BITINT_TYPEs satisfy
> + (unsigned) max + 1 == (unsigned) min, so no need to verify
> + that like for INTEGER_TYPEs. */
> + || TREE_CODE (etype) == BITINT_TYPE)
> etype = unsigned_type_for (etype);
> return etype;
> }
>
> Jakub
>
>
--
Richard Biener <rguenther@suse.de>
SUSE Software Solutions Germany GmbH,
Frankenstrasse 146, 90461 Nuernberg, Germany;
GF: Ivo Totev, Andrew McDonald, Werner Knoblich; (HRB 36809, AG Nuernberg)
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] small _BitInt tweaks
2023-09-12 10:27 ` Richard Biener
@ 2023-09-12 11:26 ` Jakub Jelinek
2023-09-12 17:27 ` Joseph Myers
0 siblings, 1 reply; 6+ messages in thread
From: Jakub Jelinek @ 2023-09-12 11:26 UTC (permalink / raw)
To: Richard Biener, Joseph S. Myers; +Cc: gcc-patches
On Tue, Sep 12, 2023 at 10:27:18AM +0000, Richard Biener wrote:
> On Mon, 11 Sep 2023, Jakub Jelinek wrote:
> > And, also I think it is undesirable when being asked for signed_type_for
> > of unsigned _BitInt(1) (which is valid) to get signed _BitInt(1) (which is
> > invalid, the standard only allows signed _BitInt(2) and larger), so the
> > patch returns 1-bit signed INTEGER_TYPE for those cases.
>
> I think the last bit is a bit surprising - do the frontends use
> signed_or_unsigned_type_for and would they be confused if getting
> back an INTEGER_TYPE here?
I see a single c-family/c-pretty-print.cc use of signed_or_unsigned_type_for
and none of signed_type_for in the C/C++ FEs (unsigned_type_for is used in a
couple of spots, but that isn't affected), c_common_signed_type
or c_common_signed_or_unsigned_type is used more than that, but I still
think it is mostly used for warning stuff and similar or when called with
some specific types like sizetype. I don't think the FE uses (or should
use) those functions to decide e.g. on types of expressions etc., that is
what common_type etc. are for.
And, for the very small precisions the distinction between BITINT_TYPE and
INTEGER_TYPE should be limited to just loads/stores from memory (in case
there are different rules for what to do with padding bits in those cases if
any) and on function arguments/return values, I think none of this is really
affected by those signed_type_for/c_common_signed_type results.
And by ensuring we never create 1-bit signed BITINT_TYPE e.g. the backends
don't need to worry about them.
But I admit I don't feel strongly about that.
Joseph, what do you think about this?
Jakub
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] small _BitInt tweaks
2023-09-12 11:26 ` Jakub Jelinek
@ 2023-09-12 17:27 ` Joseph Myers
2023-09-19 7:23 ` [PATCH] v2: " Jakub Jelinek
0 siblings, 1 reply; 6+ messages in thread
From: Joseph Myers @ 2023-09-12 17:27 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: Richard Biener, gcc-patches
On Tue, 12 Sep 2023, Jakub Jelinek via Gcc-patches wrote:
> And by ensuring we never create 1-bit signed BITINT_TYPE e.g. the backends
> don't need to worry about them.
>
> But I admit I don't feel strongly about that.
>
> Joseph, what do you think about this?
I think it's appropriate to avoid 1-bit signed BITINT_TYPE consistently.
--
Joseph S. Myers
joseph@codesourcery.com
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH] v2: small _BitInt tweaks
2023-09-12 17:27 ` Joseph Myers
@ 2023-09-19 7:23 ` Jakub Jelinek
2023-09-19 7:33 ` Richard Biener
0 siblings, 1 reply; 6+ messages in thread
From: Jakub Jelinek @ 2023-09-19 7:23 UTC (permalink / raw)
To: Richard Biener, Joseph Myers; +Cc: gcc-patches
Hi!
On Tue, Sep 12, 2023 at 05:27:30PM +0000, Joseph Myers wrote:
> On Tue, 12 Sep 2023, Jakub Jelinek via Gcc-patches wrote:
>
> > And by ensuring we never create 1-bit signed BITINT_TYPE e.g. the backends
> > don't need to worry about them.
> >
> > But I admit I don't feel strongly about that.
> >
> > Joseph, what do you think about this?
>
> I think it's appropriate to avoid 1-bit signed BITINT_TYPE consistently.
Here is a patch which does that. In addition to the previously changed two
hunks it also adds a checking assertion that we don't create
signed _BitInt(0), unsigned _BitInt(0) or signed _BitInt(1) types.
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
2023-09-18 Jakub Jelinek <jakub@redhat.com>
gcc/
* tree.cc (build_bitint_type): Assert precision is not 0, or
for signed types 1.
(signed_or_unsigned_type_for): Return INTEGER_TYPE for signed variant
of unsigned _BitInt(1).
gcc/c-family/
* c-common.cc (c_common_signed_or_unsigned_type): Return INTEGER_TYPE
for signed variant of unsigned _BitInt(1).
--- gcc/tree.cc.jj 2023-09-11 17:01:17.612714178 +0200
+++ gcc/tree.cc 2023-09-18 12:36:37.598912717 +0200
@@ -7179,6 +7179,8 @@ build_bitint_type (unsigned HOST_WIDE_IN
{
tree itype, ret;
+ gcc_checking_assert (precision >= 1 + !unsignedp);
+
if (unsignedp)
unsignedp = MAX_INT_CACHED_PREC + 1;
@@ -11096,7 +11098,7 @@ signed_or_unsigned_type_for (int unsigne
else
return NULL_TREE;
- if (TREE_CODE (type) == BITINT_TYPE)
+ if (TREE_CODE (type) == BITINT_TYPE && (unsignedp || bits > 1))
return build_bitint_type (bits, unsignedp);
return build_nonstandard_integer_type (bits, unsignedp);
}
--- gcc/c-family/c-common.cc.jj 2023-09-11 17:01:17.517715431 +0200
+++ gcc/c-family/c-common.cc 2023-09-18 12:35:06.829126858 +0200
@@ -2739,7 +2739,9 @@ c_common_signed_or_unsigned_type (int un
|| TYPE_UNSIGNED (type) == unsignedp)
return type;
- if (TREE_CODE (type) == BITINT_TYPE)
+ if (TREE_CODE (type) == BITINT_TYPE
+ /* signed _BitInt(1) is invalid, avoid creating that. */
+ && (unsignedp || TYPE_PRECISION (type) > 1))
return build_bitint_type (TYPE_PRECISION (type), unsignedp);
#define TYPE_OK(node) \
Jakub
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] v2: small _BitInt tweaks
2023-09-19 7:23 ` [PATCH] v2: " Jakub Jelinek
@ 2023-09-19 7:33 ` Richard Biener
0 siblings, 0 replies; 6+ messages in thread
From: Richard Biener @ 2023-09-19 7:33 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: Joseph Myers, gcc-patches
On Tue, 19 Sep 2023, Jakub Jelinek wrote:
> Hi!
>
> On Tue, Sep 12, 2023 at 05:27:30PM +0000, Joseph Myers wrote:
> > On Tue, 12 Sep 2023, Jakub Jelinek via Gcc-patches wrote:
> >
> > > And by ensuring we never create 1-bit signed BITINT_TYPE e.g. the backends
> > > don't need to worry about them.
> > >
> > > But I admit I don't feel strongly about that.
> > >
> > > Joseph, what do you think about this?
> >
> > I think it's appropriate to avoid 1-bit signed BITINT_TYPE consistently.
>
> Here is a patch which does that. In addition to the previously changed two
> hunks it also adds a checking assertion that we don't create
> signed _BitInt(0), unsigned _BitInt(0) or signed _BitInt(1) types.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
OK.
Thanks,
Richard.
> 2023-09-18 Jakub Jelinek <jakub@redhat.com>
>
> gcc/
> * tree.cc (build_bitint_type): Assert precision is not 0, or
> for signed types 1.
> (signed_or_unsigned_type_for): Return INTEGER_TYPE for signed variant
> of unsigned _BitInt(1).
> gcc/c-family/
> * c-common.cc (c_common_signed_or_unsigned_type): Return INTEGER_TYPE
> for signed variant of unsigned _BitInt(1).
>
> --- gcc/tree.cc.jj 2023-09-11 17:01:17.612714178 +0200
> +++ gcc/tree.cc 2023-09-18 12:36:37.598912717 +0200
> @@ -7179,6 +7179,8 @@ build_bitint_type (unsigned HOST_WIDE_IN
> {
> tree itype, ret;
>
> + gcc_checking_assert (precision >= 1 + !unsignedp);
> +
> if (unsignedp)
> unsignedp = MAX_INT_CACHED_PREC + 1;
>
> @@ -11096,7 +11098,7 @@ signed_or_unsigned_type_for (int unsigne
> else
> return NULL_TREE;
>
> - if (TREE_CODE (type) == BITINT_TYPE)
> + if (TREE_CODE (type) == BITINT_TYPE && (unsignedp || bits > 1))
> return build_bitint_type (bits, unsignedp);
> return build_nonstandard_integer_type (bits, unsignedp);
> }
> --- gcc/c-family/c-common.cc.jj 2023-09-11 17:01:17.517715431 +0200
> +++ gcc/c-family/c-common.cc 2023-09-18 12:35:06.829126858 +0200
> @@ -2739,7 +2739,9 @@ c_common_signed_or_unsigned_type (int un
> || TYPE_UNSIGNED (type) == unsignedp)
> return type;
>
> - if (TREE_CODE (type) == BITINT_TYPE)
> + if (TREE_CODE (type) == BITINT_TYPE
> + /* signed _BitInt(1) is invalid, avoid creating that. */
> + && (unsignedp || TYPE_PRECISION (type) > 1))
> return build_bitint_type (TYPE_PRECISION (type), unsignedp);
>
> #define TYPE_OK(node) \
>
>
> Jakub
>
>
--
Richard Biener <rguenther@suse.de>
SUSE Software Solutions Germany GmbH,
Frankenstrasse 146, 90461 Nuernberg, Germany;
GF: Ivo Totev, Andrew McDonald, Werner Knoblich; (HRB 36809, AG Nuernberg)
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-09-19 7:33 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-09-11 17:42 [PATCH] small _BitInt tweaks Jakub Jelinek
2023-09-12 10:27 ` Richard Biener
2023-09-12 11:26 ` Jakub Jelinek
2023-09-12 17:27 ` Joseph Myers
2023-09-19 7:23 ` [PATCH] v2: " Jakub Jelinek
2023-09-19 7:33 ` Richard Biener
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).