[Bug middle-end/108623] We need to grow the precision field in tree_type_common for PowerPC
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108623 Richard Biener changed: What|Removed |Added Resolution|--- |FIXED Status|NEW |RESOLVED Target Milestone|--- |14.0 --- Comment #12 from Richard Biener --- Fixed for GCC 14
[Bug middle-end/108623] We need to grow the precision field in tree_type_common for PowerPC
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108623 --- Comment #11 from CVS Commits --- The master branch has been updated by Richard Biener : https://gcc.gnu.org/g:e2b993db57f90fedd1bd7756f7ad4c5bfded4b8f commit r14-577-ge2b993db57f90fedd1bd7756f7ad4c5bfded4b8f Author: Michael Meissner Date: Wed Feb 1 12:30:19 2023 -0500 Bump up precision size to 16 bits. The new __dmr type that is being added as a possible future PowerPC instruction set bumps into a structure field size issue. The size of the __dmr type is 1024 bits. The precision field in tree_type_common is currently 10 bits, so if you store 1,024 into field, you get a 0 back. When you get 0 in the precision field, the ccp pass passes this 0 to sext_hwi in hwint.h. That function in turn generates a shift that is equal to the host wide int bit size, which is undefined as machine dependent for shifting in C/C++. int shift = HOST_BITS_PER_WIDE_INT - prec; return ((HOST_WIDE_INT) ((unsigned HOST_WIDE_INT) src << shift)) >> shift; It turns out the x86_64 where I first did my tests returns the original input before the two shifts, while the PowerPC always returns 0. In the ccp pass, the original input is -1, and so it worked. When I did the runs on the PowerPC, the result was 0, which ultimately led to the failure. 2023-02-01 Richard Biener Michael Meissner PR middle-end/108623 * tree-core.h (tree_type_common): Bump up precision field to 16 bits. Align bit fields > 1 bit to at least an 8-bit boundary.
[Bug middle-end/108623] We need to grow the precision field in tree_type_common for PowerPC
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108623 --- Comment #10 from rsandifo at gcc dot gnu.org --- (In reply to rguent...@suse.de from comment #9) > Please you do it, as far as I understand Richard S. no further adjustment > is necessary but we could simplify some code after the change(?) Yeah. AFAIK, nothing outside these two macros relies on the representation.
[Bug middle-end/108623] We need to grow the precision field in tree_type_common for PowerPC
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108623 --- Comment #9 from rguenther at suse dot de --- On Wed, 1 Feb 2023, meissner at gcc dot gnu.org wrote: > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108623 > > Michael Meissner changed: > >What|Removed |Added > >Last reconfirmed||2023-02-01 > Ever confirmed|0 |1 > Status|UNCONFIRMED |NEW > > --- Comment #6 from Michael Meissner --- > Yes I agree we want an assetion in sext_hwi as well. > > Richard, are you going to submit the patch, or did you want me to do it (along > with the assertion)? Please you do it, as far as I understand Richard S. no further adjustment is necessary but we could simplify some code after the change(?)
[Bug middle-end/108623] We need to grow the precision field in tree_type_common for PowerPC
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108623 --- Comment #8 from joseph at codesourcery dot com --- See also bug 102989 (C2x _BitInt) regarding another case for which growing TYPE_PRECISION would be useful.
[Bug middle-end/108623] We need to grow the precision field in tree_type_common for PowerPC
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108623 --- Comment #7 from Michael Meissner --- Created attachment 54387 --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=54387=edit Proposed patch combining Richard's patch and an assertion.
[Bug middle-end/108623] We need to grow the precision field in tree_type_common for PowerPC
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108623 Michael Meissner changed: What|Removed |Added Last reconfirmed||2023-02-01 Ever confirmed|0 |1 Status|UNCONFIRMED |NEW --- Comment #6 from Michael Meissner --- Yes I agree we want an assetion in sext_hwi as well. Richard, are you going to submit the patch, or did you want me to do it (along with the assertion)?
[Bug middle-end/108623] We need to grow the precision field in tree_type_common for PowerPC
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108623 --- Comment #5 from Segher Boessenkool --- The failure was not detected, only things down the road broke up, can we add something for that? Just a strategically placed assert should do fine. Less important if we grow the field all the way to 16 bits, but :-)
[Bug middle-end/108623] We need to grow the precision field in tree_type_common for PowerPC
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108623 --- Comment #4 from Michael Meissner --- I must have missed the spare bits. I think it is better to use the full 16 bits for precision. I also think your other changes to realign bit fields greater than 1 bit.
[Bug middle-end/108623] We need to grow the precision field in tree_type_common for PowerPC
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108623 --- Comment #3 from rsandifo at gcc dot gnu.org --- The explanation is in the SET_TYPE_VECTOR_SUBPARTS code: /* We have two coefficients that are each in the range 1 << [0, 63], so supporting all combinations would require 6 bits per coefficient and 12 bits in total. Since the precision field is only 10 bits in size, we need to be more restrictive than that. At present, coeff[1] is always either 0 (meaning that the number of units is constant) or equal to coeff[0] (meaning that the number of units is N + X * N for some target-dependent zero-based runtime parameter X). We can therefore encode coeff[1] in a single bit. The most compact encoding would be to use mask 0x3f for coeff[0] and 0x40 for coeff[1], leaving 0x380 unused. It's possible to get slightly more efficient code on some hosts if we instead treat the shift amount as an independent byte, so here we use 0xff for coeff[0] and 0x100 for coeff[1]. */ If we're happy to extend to 16 bits then things become simpler. We can just use one byte per coefficient.
[Bug middle-end/108623] We need to grow the precision field in tree_type_common for PowerPC
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108623
Richard Biener changed:
What|Removed |Added
CC||rguenth at gcc dot gnu.org,
||rsandifo at gcc dot gnu.org
--- Comment #2 from Richard Biener ---
Note there's code that might need adjustments. We for example have
/* Return the number of elements in the VECTOR_TYPE given by NODE. */
inline poly_uint64
TYPE_VECTOR_SUBPARTS (const_tree node)
{
STATIC_ASSERT (NUM_POLY_INT_COEFFS <= 2);
unsigned int precision = VECTOR_TYPE_CHECK (node)->type_common.precision;
if (NUM_POLY_INT_COEFFS == 2)
{
/* See the corresponding code in SET_TYPE_VECTOR_SUBPARTS for a
description of the encoding. */
poly_uint64 res = 0;
res.coeffs[0] = HOST_WIDE_INT_1U << (precision & 0xff);
if (precision & 0x100)
res.coeffs[1] = HOST_WIDE_INT_1U << (precision & 0xff);
return res;
}
which looks odd anyways. Richard might know where the 10 bits have been
baked in to (ISTR something about the INTEGER_CST encoding stuff here
with the three kinds of "precisions")
[Bug middle-end/108623] We need to grow the precision field in tree_type_common for PowerPC
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=108623
Richard Biener changed:
What|Removed |Added
Component|other |middle-end
Target||powerpc*
--- Comment #1 from Richard Biener ---
There are 15 spare bits at the end of the bits, a better solution is to
re-order things so precision remains aligned to 16 bits, for example by moving
the
6 bits adjacent to it to the "spare" and extending precision to a full 16 bits.
Like with the following which aligns all >1 bit fields to at least 8 bits
diff --git a/gcc/tree-core.h b/gcc/tree-core.h
index acd8deea34e..e5513208511 100644
--- a/gcc/tree-core.h
+++ b/gcc/tree-core.h
@@ -1686,18 +1686,8 @@ struct GTY(()) tree_type_common {
tree attributes;
unsigned int uid;
- unsigned int precision : 10;
- unsigned no_force_blk_flag : 1;
- unsigned needs_constructing_flag : 1;
- unsigned transparent_aggr_flag : 1;
- unsigned restrict_flag : 1;
- unsigned contains_placeholder_bits : 2;
-
+ unsigned int precision : 16;
ENUM_BITFIELD(machine_mode) mode : 8;
-
- /* TYPE_STRING_FLAG for INTEGER_TYPE and ARRAY_TYPE.
- TYPE_CXX_ODR_P for RECORD_TYPE and UNION_TYPE. */
- unsigned string_flag : 1;
unsigned lang_flag_0 : 1;
unsigned lang_flag_1 : 1;
unsigned lang_flag_2 : 1;
@@ -1713,12 +1703,22 @@ struct GTY(()) tree_type_common {
so we need to store the value 32 (not 31, as we need the zero
as well), hence six bits. */
unsigned align : 6;
+ /* TYPE_STRING_FLAG for INTEGER_TYPE and ARRAY_TYPE.
+ TYPE_CXX_ODR_P for RECORD_TYPE and UNION_TYPE. */
+ unsigned string_flag : 1;
+ unsigned no_force_blk_flag : 1;
+
unsigned warn_if_not_align : 6;
+ unsigned needs_constructing_flag : 1;
+ unsigned transparent_aggr_flag : 1;
+
+ unsigned contains_placeholder_bits : 2;
+ unsigned restrict_flag : 1;
unsigned typeless_storage : 1;
unsigned empty_flag : 1;
unsigned indivisible_p : 1;
unsigned no_named_args_stdarg_p : 1;
- unsigned spare : 15;
+ unsigned spare : 9;
alias_set_type alias_set;
tree pointer_to;
