Re: [PATCH 7/7] Move SYMBOL_REF_FLAGS to rtx header
On 05/29/14 06:07, Richard Sandiford wrote: Can't really approve it, but it looks obviously correct to me. Thanks for the fix. Is that something you'd like to see changed? Jeff
Re: [PATCH 7/7] Move SYMBOL_REF_FLAGS to rtx header
On 05/29/14 05:27, Tom de Vries wrote:
On 10-05-14 22:24, Richard Sandiford wrote:
/* A set of flags on a symbol_ref that are, in some respects,
redundant with
information derivable from the tree decl associated with this
symbol.
@@ -1791,7 +1794,9 @@ #define SYMBOL_REF_CONSTANT(RTX) \
this information to avoid recomputing it. Finally, this allows
space for
the target to store more than one bit of information, as with
SYMBOL_REF_FLAG. */
-#define SYMBOL_REF_FLAGS(RTX)X0INT ((RTX), 1)
+#define SYMBOL_REF_FLAGS(RTX) \
+ (RTL_FLAG_CHECK1 ("SYMBOL_REF_FLAGS", (RTX), SYMBOL_REF) \
+ ->u2.symbol_ref_flags)
Richard,
with an arm-linux-gnueabi non-bootstrap build with
--enable-checking=yes,rtl, I ran into the following error:
...
/home/vries/gcc_versions/devel/src/libgcc/libgcc2.c:819:1: internal
compiler error: RTL check: attempt to treat non-block symbol as a block
symbol in create_block_symbol, at varasm.c:394
};
^
0xc3c16b rtl_check_failed_block_symbol(char const*, int, char const*)
/home/vries/gcc_versions/devel/src/gcc/rtl.c:844
0x103c09d create_block_symbol
/home/vries/gcc_versions/devel/src/gcc/varasm.c:394
0x103f42d make_decl_rtl(tree_node*)
/home/vries/gcc_versions/devel/src/gcc/varasm.c:1379
0x103fc87 notice_global_symbol(tree_node*)
/home/vries/gcc_versions/devel/src/gcc/varasm.c:1552
0x7588bf varpool_finalize_decl(tree_node*)
/home/vries/gcc_versions/devel/src/gcc/cgraphunit.c:823
0xb4eaa0 rest_of_decl_compilation(tree_node*, int, int)
/home/vries/gcc_versions/devel/src/gcc/passes.c:241
0x5902c4 finish_decl(tree_node*, unsigned int, tree_node*, tree_node*,
tree_node*)
/home/vries/gcc_versions/devel/src/gcc/c/c-decl.c:4521
0x5e8586 c_parser_declaration_or_fndef
/home/vries/gcc_versions/devel/src/gcc/c/c-parser.c:1782
0x5e7644 c_parser_external_declaration
/home/vries/gcc_versions/devel/src/gcc/c/c-parser.c:1399
0x5e72c7 c_parser_translation_unit
/home/vries/gcc_versions/devel/src/gcc/c/c-parser.c:1286
0x606c6d c_parse_file()
/home/vries/gcc_versions/devel/src/gcc/c/c-parser.c:14077
0x66b7fa c_common_parse_file()
/home/vries/gcc_versions/devel/src/gcc/c-family/c-opts.c:1067
Please submit a full bug report,
with preprocessed source if appropriate.
Please include the complete backtrace with any bug report.
See for instructions.
...
It looks like BLOCK_SYMBOL_CHECK hasn't been updated.
Patch below fixes it for me. OK for trunk if bootstrap on x86_64 succeeds?
Yes. Ok.
jeff
Re: [PATCH 7/7] Move SYMBOL_REF_FLAGS to rtx header
Tom de Vries writes:
> On 10-05-14 22:24, Richard Sandiford wrote:
>> /* A set of flags on a symbol_ref that are, in some respects, redundant
>> with
>> information derivable from the tree decl associated with this symbol.
>> @@ -1791,7 +1794,9 @@ #define SYMBOL_REF_CONSTANT(RTX) \
>> this information to avoid recomputing it. Finally, this allows space
>> for
>> the target to store more than one bit of information, as with
>> SYMBOL_REF_FLAG. */
>> -#define SYMBOL_REF_FLAGS(RTX) X0INT ((RTX), 1)
>> +#define SYMBOL_REF_FLAGS(RTX) \
>> + (RTL_FLAG_CHECK1 ("SYMBOL_REF_FLAGS", (RTX), SYMBOL_REF) \
>> + ->u2.symbol_ref_flags)
>>
>
> Richard,
>
> with an arm-linux-gnueabi non-bootstrap build with --enable-checking=yes,rtl,
> I
> ran into the following error:
> ...
> /home/vries/gcc_versions/devel/src/libgcc/libgcc2.c:819:1: internal compiler
> error: RTL check: attempt to treat non-block symbol as a block symbol in
> create_block_symbol, at varasm.c:394
> };
> ^
> 0xc3c16b rtl_check_failed_block_symbol(char const*, int, char const*)
> /home/vries/gcc_versions/devel/src/gcc/rtl.c:844
> 0x103c09d create_block_symbol
> /home/vries/gcc_versions/devel/src/gcc/varasm.c:394
> 0x103f42d make_decl_rtl(tree_node*)
> /home/vries/gcc_versions/devel/src/gcc/varasm.c:1379
> 0x103fc87 notice_global_symbol(tree_node*)
> /home/vries/gcc_versions/devel/src/gcc/varasm.c:1552
> 0x7588bf varpool_finalize_decl(tree_node*)
> /home/vries/gcc_versions/devel/src/gcc/cgraphunit.c:823
> 0xb4eaa0 rest_of_decl_compilation(tree_node*, int, int)
> /home/vries/gcc_versions/devel/src/gcc/passes.c:241
> 0x5902c4 finish_decl(tree_node*, unsigned int, tree_node*, tree_node*,
> tree_node*)
> /home/vries/gcc_versions/devel/src/gcc/c/c-decl.c:4521
> 0x5e8586 c_parser_declaration_or_fndef
> /home/vries/gcc_versions/devel/src/gcc/c/c-parser.c:1782
> 0x5e7644 c_parser_external_declaration
> /home/vries/gcc_versions/devel/src/gcc/c/c-parser.c:1399
> 0x5e72c7 c_parser_translation_unit
> /home/vries/gcc_versions/devel/src/gcc/c/c-parser.c:1286
> 0x606c6d c_parse_file()
> /home/vries/gcc_versions/devel/src/gcc/c/c-parser.c:14077
> 0x66b7fa c_common_parse_file()
> /home/vries/gcc_versions/devel/src/gcc/c-family/c-opts.c:1067
> Please submit a full bug report,
> with preprocessed source if appropriate.
> Please include the complete backtrace with any bug report.
> See for instructions.
> ...
>
> It looks like BLOCK_SYMBOL_CHECK hasn't been updated.
>
> Patch below fixes it for me. OK for trunk if bootstrap on x86_64 succeeds?
Can't really approve it, but it looks obviously correct to me.
Thanks for the fix.
Richard
Re: [PATCH 7/7] Move SYMBOL_REF_FLAGS to rtx header
On 10-05-14 22:24, Richard Sandiford wrote:
/* A set of flags on a symbol_ref that are, in some respects, redundant with
information derivable from the tree decl associated with this symbol.
@@ -1791,7 +1794,9 @@ #define SYMBOL_REF_CONSTANT(RTX) \
this information to avoid recomputing it. Finally, this allows space for
the target to store more than one bit of information, as with
SYMBOL_REF_FLAG. */
-#define SYMBOL_REF_FLAGS(RTX) X0INT ((RTX), 1)
+#define SYMBOL_REF_FLAGS(RTX) \
+ (RTL_FLAG_CHECK1 ("SYMBOL_REF_FLAGS", (RTX), SYMBOL_REF) \
+ ->u2.symbol_ref_flags)
Richard,
with an arm-linux-gnueabi non-bootstrap build with --enable-checking=yes,rtl, I
ran into the following error:
...
/home/vries/gcc_versions/devel/src/libgcc/libgcc2.c:819:1: internal compiler
error: RTL check: attempt to treat non-block symbol as a block symbol in
create_block_symbol, at varasm.c:394
};
^
0xc3c16b rtl_check_failed_block_symbol(char const*, int, char const*)
/home/vries/gcc_versions/devel/src/gcc/rtl.c:844
0x103c09d create_block_symbol
/home/vries/gcc_versions/devel/src/gcc/varasm.c:394
0x103f42d make_decl_rtl(tree_node*)
/home/vries/gcc_versions/devel/src/gcc/varasm.c:1379
0x103fc87 notice_global_symbol(tree_node*)
/home/vries/gcc_versions/devel/src/gcc/varasm.c:1552
0x7588bf varpool_finalize_decl(tree_node*)
/home/vries/gcc_versions/devel/src/gcc/cgraphunit.c:823
0xb4eaa0 rest_of_decl_compilation(tree_node*, int, int)
/home/vries/gcc_versions/devel/src/gcc/passes.c:241
0x5902c4 finish_decl(tree_node*, unsigned int, tree_node*, tree_node*,
tree_node*)
/home/vries/gcc_versions/devel/src/gcc/c/c-decl.c:4521
0x5e8586 c_parser_declaration_or_fndef
/home/vries/gcc_versions/devel/src/gcc/c/c-parser.c:1782
0x5e7644 c_parser_external_declaration
/home/vries/gcc_versions/devel/src/gcc/c/c-parser.c:1399
0x5e72c7 c_parser_translation_unit
/home/vries/gcc_versions/devel/src/gcc/c/c-parser.c:1286
0x606c6d c_parse_file()
/home/vries/gcc_versions/devel/src/gcc/c/c-parser.c:14077
0x66b7fa c_common_parse_file()
/home/vries/gcc_versions/devel/src/gcc/c-family/c-opts.c:1067
Please submit a full bug report,
with preprocessed source if appropriate.
Please include the complete backtrace with any bug report.
See for instructions.
...
It looks like BLOCK_SYMBOL_CHECK hasn't been updated.
Patch below fixes it for me. OK for trunk if bootstrap on x86_64 succeeds?
Thanks,
- Tom
2014-05-29 Tom de Vries
* rtl.h (BLOCK_SYMBOL_CHECK): Use SYMBOL_REF_FLAGS.
---
gcc/rtl.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/gcc/rtl.h b/gcc/rtl.h
index 02ce424..51cfae5 100644
--- a/gcc/rtl.h
+++ b/gcc/rtl.h
@@ -708,7 +708,7 @@ struct GTY(()) rtvec_def {
#define BLOCK_SYMBOL_CHECK(RTX) __extension__\
({ __typeof (RTX) const _symbol = (RTX);\
- const unsigned int flags = RTL_CHECKC1 (_symbol, 1, SYMBOL_REF).rt_int; \
+ const unsigned int flags = SYMBOL_REF_FLAGS (_symbol); \
if ((flags & SYMBOL_FLAG_HAS_BLOCK_INFO) == 0) \
rtl_check_failed_block_symbol (__FILE__, __LINE__, \
__FUNCTION__); \
--
1.9.1
Re: [PATCH 7/7] Move SYMBOL_REF_FLAGS to rtx header
On 05/10/14 14:24, Richard Sandiford wrote: Very much like the code to move ORIGINAL_REGNO, but with a few more knock-on changes. I handled the printing by dumping the flags immediately before the SYMBOL_REF_DATA. Tested on x86_64-linux-gnu. OK to install? Thanks, Richard gcc/ * rtl.def (SYMBOL_REF): Remove middle "0" field. * rtl.h (block_symbol): Reduce number of fields to 2. (rtx_def): Add u2.symbol_ref_flags. (SYMBOL_REF_FLAGS): Use it. (SYMBOL_REF_DATA, SET_SYMBOL_REF_DECL, SYMBOL_REF_DECL) (SET_SYMBOL_REF_CONSTANT, SYMBOL_REF_CONSTANT): Lower index. * gengtype.c (adjust_field_rtx_def): Remove SYMBOL_REF_FLAGS handling. Lower index of SYMBOL_REF_DATA. * print-rtl.c (print_rtx): Lower index for SYMBOL_REF_DATA. Print SYMBOL_REF_FLAGS at the same time. * genattrtab.c (attr_rtx_1): Only initialize 1 "0" SYMBOL_REF field. OK for the trunk. jeff
Re: [PATCH 7/7] Move SYMBOL_REF_FLAGS to rtx header
On 05/12/14 05:00, Richard Sandiford wrote:
Michael Matz writes:
Hi,
On Sat, 10 May 2014, Richard Sandiford wrote:
@@ -362,6 +362,9 @@ struct GTY((chain_next ("RTX_NEXT (&%h)"
/* The INSN_UID of an RTX_INSN-class code. */
int insn_uid;
+/* The SYMBOL_REF_FLAGS of a SYMBOL_REF. */
+unsigned int symbol_ref_flags;
+
In [3/7] you used
+/* The ORIGINAL_REGNO of a REG. */
+unsigned original_regno;
+
Should be consistent.
Oops, indeed. Will fix them all to use unsigned int if approved.
Approved with the consistency fix.
jeff
Re: [PATCH 7/7] Move SYMBOL_REF_FLAGS to rtx header
Hi, On Mon, 12 May 2014, Richard Sandiford wrote: > > Also I'm idly wondering if the explicit sizing of > > the fields via a bit-field as originally would be better here or just > > confusing. I guess unsigned and enums are 32bit for all hosts we care > > about, but if we ever have one where it's larger the rtx will suddenly > > contain another hole. > > But that'll happen anyway, which is why I thought having bitfields > was confusing. Since this is a union, you'll always get the full > "unsigned int" regardless of the bitfield size; the bitfield can't > be packed with anything else. Hmm, true. Okay, it'd be premature optimization for hosts which don't exist anyway :) Ciao, Michael.
Re: [PATCH 7/7] Move SYMBOL_REF_FLAGS to rtx header
Michael Matz writes:
> Hi,
>
> On Sat, 10 May 2014, Richard Sandiford wrote:
>
>> @@ -362,6 +362,9 @@ struct GTY((chain_next ("RTX_NEXT (&%h)"
>> /* The INSN_UID of an RTX_INSN-class code. */
>> int insn_uid;
>>
>> +/* The SYMBOL_REF_FLAGS of a SYMBOL_REF. */
>> +unsigned int symbol_ref_flags;
>> +
>
> In [3/7] you used
>
> +/* The ORIGINAL_REGNO of a REG. */
> +unsigned original_regno;
> +
>
> Should be consistent.
Oops, indeed. Will fix them all to use unsigned int if approved.
> Also I'm idly wondering if the explicit sizing of
> the fields via a bit-field as originally would be better here or just
> confusing. I guess unsigned and enums are 32bit for all hosts we care
> about, but if we ever have one where it's larger the rtx will suddenly
> contain another hole.
But that'll happen anyway, which is why I thought having bitfields
was confusing. Since this is a union, you'll always get the full
"unsigned int" regardless of the bitfield size; the bitfield can't
be packed with anything else.
Thanks,
Richard
Re: [PATCH 7/7] Move SYMBOL_REF_FLAGS to rtx header
Hi,
On Sat, 10 May 2014, Richard Sandiford wrote:
> @@ -362,6 +362,9 @@ struct GTY((chain_next ("RTX_NEXT (&%h)"
> /* The INSN_UID of an RTX_INSN-class code. */
> int insn_uid;
>
> +/* The SYMBOL_REF_FLAGS of a SYMBOL_REF. */
> +unsigned int symbol_ref_flags;
> +
In [3/7] you used
+/* The ORIGINAL_REGNO of a REG. */
+unsigned original_regno;
+
Should be consistent. Also I'm idly wondering if the explicit sizing of
the fields via a bit-field as originally would be better here or just
confusing. I guess unsigned and enums are 32bit for all hosts we care
about, but if we ever have one where it's larger the rtx will suddenly
contain another hole.
Ciao,
Michael.
[PATCH 7/7] Move SYMBOL_REF_FLAGS to rtx header
Very much like the code to move ORIGINAL_REGNO, but with a few more
knock-on changes. I handled the printing by dumping the flags
immediately before the SYMBOL_REF_DATA.
Tested on x86_64-linux-gnu. OK to install?
Thanks,
Richard
gcc/
* rtl.def (SYMBOL_REF): Remove middle "0" field.
* rtl.h (block_symbol): Reduce number of fields to 2.
(rtx_def): Add u2.symbol_ref_flags.
(SYMBOL_REF_FLAGS): Use it.
(SYMBOL_REF_DATA, SET_SYMBOL_REF_DECL, SYMBOL_REF_DECL)
(SET_SYMBOL_REF_CONSTANT, SYMBOL_REF_CONSTANT): Lower index.
* gengtype.c (adjust_field_rtx_def): Remove SYMBOL_REF_FLAGS handling.
Lower index of SYMBOL_REF_DATA.
* print-rtl.c (print_rtx): Lower index for SYMBOL_REF_DATA.
Print SYMBOL_REF_FLAGS at the same time.
* genattrtab.c (attr_rtx_1): Only initialize 1 "0" SYMBOL_REF field.
Index: gcc/rtl.def
===
--- gcc/rtl.def 2014-05-10 21:17:11.883121021 +0100
+++ gcc/rtl.def 2014-05-10 21:22:23.685816826 +0100
@@ -429,10 +429,9 @@ DEF_RTL_EXPR(LABEL_REF, "label_ref", "u"
/* Reference to a named label:
Operand 0: label name
- Operand 1: flags (see SYMBOL_FLAG_* in rtl.h)
- Operand 2: tree from which this symbol is derived, or null.
+ Operand 1: tree from which this symbol is derived, or null.
This is either a DECL node, or some kind of constant. */
-DEF_RTL_EXPR(SYMBOL_REF, "symbol_ref", "s00", RTX_CONST_OBJ)
+DEF_RTL_EXPR(SYMBOL_REF, "symbol_ref", "s0", RTX_CONST_OBJ)
/* The condition code register is represented, in our imagination,
as a register holding a value that can be compared to zero.
Index: gcc/rtl.h
===
--- gcc/rtl.h 2014-05-10 21:17:11.883121021 +0100
+++ gcc/rtl.h 2014-05-10 21:22:23.686816835 +0100
@@ -211,7 +211,7 @@ struct GTY(()) reg_attrs {
if SYMBOL_REF_HAS_BLOCK_INFO_P is true. */
struct GTY(()) block_symbol {
/* The usual SYMBOL_REF fields. */
- rtunion GTY ((skip)) fld[3];
+ rtunion GTY ((skip)) fld[2];
/* The block that contains this object. */
struct object_block *block;
@@ -362,6 +362,9 @@ struct GTY((chain_next ("RTX_NEXT (&%h)"
/* The INSN_UID of an RTX_INSN-class code. */
int insn_uid;
+/* The SYMBOL_REF_FLAGS of a SYMBOL_REF. */
+unsigned int symbol_ref_flags;
+
/* The PAT_VAR_LOCATION_STATUS of a VAR_LOCATION. */
enum var_init_status var_location_status;
@@ -1765,24 +1768,24 @@ #define SYMBOL_REF_WEAK(RTX)
\
/* A pointer attached to the SYMBOL_REF; either SYMBOL_REF_DECL or
SYMBOL_REF_CONSTANT. */
-#define SYMBOL_REF_DATA(RTX) X0ANY ((RTX), 2)
+#define SYMBOL_REF_DATA(RTX) X0ANY ((RTX), 1)
/* Set RTX's SYMBOL_REF_DECL to DECL. RTX must not be a constant
pool symbol. */
#define SET_SYMBOL_REF_DECL(RTX, DECL) \
- (gcc_assert (!CONSTANT_POOL_ADDRESS_P (RTX)), X0TREE ((RTX), 2) = (DECL))
+ (gcc_assert (!CONSTANT_POOL_ADDRESS_P (RTX)), X0TREE ((RTX), 1) = (DECL))
/* The tree (decl or constant) associated with the symbol, or null. */
#define SYMBOL_REF_DECL(RTX) \
- (CONSTANT_POOL_ADDRESS_P (RTX) ? NULL : X0TREE ((RTX), 2))
+ (CONSTANT_POOL_ADDRESS_P (RTX) ? NULL : X0TREE ((RTX), 1))
/* Set RTX's SYMBOL_REF_CONSTANT to C. RTX must be a constant pool symbol. */
#define SET_SYMBOL_REF_CONSTANT(RTX, C) \
- (gcc_assert (CONSTANT_POOL_ADDRESS_P (RTX)), X0CONSTANT ((RTX), 2) = (C))
+ (gcc_assert (CONSTANT_POOL_ADDRESS_P (RTX)), X0CONSTANT ((RTX), 1) = (C))
/* The rtx constant pool entry for a symbol, or null. */
#define SYMBOL_REF_CONSTANT(RTX) \
- (CONSTANT_POOL_ADDRESS_P (RTX) ? X0CONSTANT ((RTX), 2) : NULL)
+ (CONSTANT_POOL_ADDRESS_P (RTX) ? X0CONSTANT ((RTX), 1) : NULL)
/* A set of flags on a symbol_ref that are, in some respects, redundant with
information derivable from the tree decl associated with this symbol.
@@ -1791,7 +1794,9 @@ #define SYMBOL_REF_CONSTANT(RTX) \
this information to avoid recomputing it. Finally, this allows space for
the target to store more than one bit of information, as with
SYMBOL_REF_FLAG. */
-#define SYMBOL_REF_FLAGS(RTX) X0INT ((RTX), 1)
+#define SYMBOL_REF_FLAGS(RTX) \
+ (RTL_FLAG_CHECK1 ("SYMBOL_REF_FLAGS", (RTX), SYMBOL_REF) \
+ ->u2.symbol_ref_flags)
/* These flags are common enough to be defined for all targets. They
are computed by the default version of targetm.encode_section_info. */
Index: gcc/gengtype.c
===
--- gcc/gengtype.c 2014-05-10 21:13:06.242993283 +0100
+++ gcc/gengtype.c 2014-05-10 21:22:23.685816826 +0100
@@ -1250,8 +1250,6 @@ adjust_field_rtx_def (type_p t, options_
else if (i == REG && aindex == 1)
t = reg_attrs_tp, subname = "rt_reg";
else if (i == SYMBOL_REF && aindex == 1)
- t = scalar_
