| Message ID | 1863165.r8qPLI7fxq@polaris |
|---|---|
| State | New |
| Headers | show |
On Sat, Oct 8, 2016 at 8:56 PM, Eric Botcazou <ebotcazou@adacore.com> wrote: > Hi, > > adding patterns for unsigned arithmetic overflow checking in a back-end can > have unexpected fallout because of a latent GC issue: when they are present, > GIMPLE optimization passes can create complex (math. sense) types at will by > invoking build_complex_type. Now build_complex_type goes through the type > caonicalization hashtable, which is GC-ed, so its behavior depends on the > actual collection points. > > The other type-building functions present in tree.c do the same so no big deal > but build_complex_type is special because it also does: > > /* We need to create a name, since complex is a fundamental type. */ > if (! TYPE_NAME (t)) > { > const char *name; > if (component_type == char_type_node) > name = "complex char"; > else if (component_type == signed_char_type_node) > name = "complex signed char"; > else if (component_type == unsigned_char_type_node) > name = "complex unsigned char"; > else if (component_type == short_integer_type_node) > name = "complex short int"; > else if (component_type == short_unsigned_type_node) > name = "complex short unsigned int"; > else if (component_type == integer_type_node) > name = "complex int"; > else if (component_type == unsigned_type_node) > name = "complex unsigned int"; > else if (component_type == long_integer_type_node) > name = "complex long int"; > else if (component_type == long_unsigned_type_node) > name = "complex long unsigned int"; > else if (component_type == long_long_integer_type_node) > name = "complex long long int"; > else if (component_type == long_long_unsigned_type_node) > name = "complex long long unsigned int"; > else > name = 0; > > if (name != 0) > TYPE_NAME (t) = build_decl (UNKNOWN_LOCATION, TYPE_DECL, > get_identifier (name), t); > } > > so it creates a DECL node every time a new canonical complex type is created, > bumping the DECL_UID counter in the process. Which means that the DECL_UID > counter is sensitive to the collection points, which in turn means that the > result of algorithms depending on the DECL_UID counter also is. > > This for example resulted in a bootstrap comparison failure on a SPARC/Solaris > machine doing a strict stage2/stage3 comparison because the contents of the > .debug_loc section were different: location lists computed by var-tracking > were slightly different because of a different hashing. > > I'm not sure whether the hashing done by var-tracking should be sensitive to > the DECL_UID of nodes or not, but I think that having the DECL_UID counter > depend on the collection points is highly undesirable, so the attached patch > attempts to prevent it; it at least fixed the bootstrap comparison failure. I believe the rule is that you might only depend on the order of objects with respect to their DECL_UID, not the actual value of the DECL_UID. As var-tracking shouldn't look at TYPE_DECLs (?) it's probably a latent var-tracking bug as well. > Tested on x86_64-suse-linux, OK for the mainline? I'd prefer the named parameter to be defaulted to false and the few places in the FEs fixed (eventually that name business should be handled like names for nodes like integer_type_node -- I see no reason why build_complex_type should have this special-case at all! That is, why are the named vairants in the type hash in the first place?) Richard. > > 2016-10-08 Eric Botcazou <ebotcazou@adacore.com> > > * tree.h (build_complex_type): Add second parameter with default. > * builtins.c (expand_builtin_cexpi): Pass false in call to above. > (fold_builtin_sincos): Likewise. > (fold_builtin_arith_overflow): Likewise. > * gimple-fold.c (fold_builtin_atomic_compare_exchange): Likewise. > (gimple_fold_call): Likewise. > * stor-layout.c (bitwise_type_for_mode): Likewise. > * tree-ssa-dce.c (maybe_optimize_arith_overflow): Likewise. > * tree-ssa-math-opts.c (match_uaddsub_overflow): Likewise. > * tree.c (build_complex): Likewise. > (build_complex_type): Add NAMED second parameter and adjust recursive > call. Create a TYPE_DECL only if NAMED is true. > > -- > Eric Botcazou
> I believe the rule is that you might only depend on the order of objects > with respect to their DECL_UID, not the actual value of the DECL_UID. > As var-tracking shouldn't look at TYPE_DECLs (?) it's probably a latent > var-tracking bug as well. It presumably doesn't look at TYPE_DECLs, simply the DECL_UID of variables is also different so this changes some hashing. > I'd prefer the named parameter to be defaulted to false and the few > places in the FEs fixed (eventually that name business should be > handled like names for nodes like integer_type_node -- I see no > reason why build_complex_type should have this special-case at all! > That is, why are the named vairants in the type hash in the first place?) I think that the calls in build_common_tree_nodes need to be changed too then: complex_integer_type_node = build_complex_type (integer_type_node); complex_float_type_node = build_complex_type (float_type_node); complex_double_type_node = build_complex_type (double_type_node); complex_long_double_type_node = build_complex_type (long_double_type_node); in addition to: ./ada/gcc-interface/decl.c: = build_complex_type ./ada/gcc-interface/decl.c: return build_complex_type (nt); ./ada/gcc-interface/trans.c: tree gnu_ctype = build_complex_type (gnu_type); ./c/c-decl.c: specs->type = build_complex_type (specs->type); ./c/c-decl.c: specs->type = build_complex_type (specs->type); ./c/c-decl.c: specs->type = build_complex_type (specs->type); ./c/c-parser.c: build_complex_type ./c/c-typeck.c: return build_complex_type (subtype); ./c-family/c-common.c: return build_complex_type (inner_type); ./c-family/c-lex.c: type = build_complex_type (type); ./cp/decl.c: type = build_complex_type (type); ./cp/typeck.c: return build_type_attribute_variant (build_complex_type (subtype), ./fortran/trans-types.c:gfc_build_complex_type (tree scalar_type) ./fortran/trans-types.c: type = gfc_build_complex_type (type); ./go/go-gcc.cc: build_complex_type(TREE_TYPE(real_tree)), ./go/go-gcc.cc: type = build_complex_type(type); ./lto/lto-lang.c: return build_complex_type (inner_type); Or perhaps *only* the calls in build_common_tree_nodes need to be changed? It's certainly old code (r29604, September 1999).
On Mon, Oct 10, 2016 at 12:38 PM, Eric Botcazou <ebotcazou@adacore.com> wrote: >> I believe the rule is that you might only depend on the order of objects >> with respect to their DECL_UID, not the actual value of the DECL_UID. >> As var-tracking shouldn't look at TYPE_DECLs (?) it's probably a latent >> var-tracking bug as well. > > It presumably doesn't look at TYPE_DECLs, simply the DECL_UID of variables is > also different so this changes some hashing. > >> I'd prefer the named parameter to be defaulted to false and the few >> places in the FEs fixed (eventually that name business should be >> handled like names for nodes like integer_type_node -- I see no >> reason why build_complex_type should have this special-case at all! >> That is, why are the named vairants in the type hash in the first place?) > > I think that the calls in build_common_tree_nodes need to be changed too then: > > complex_integer_type_node = build_complex_type (integer_type_node); > complex_float_type_node = build_complex_type (float_type_node); > complex_double_type_node = build_complex_type (double_type_node); > complex_long_double_type_node = build_complex_type (long_double_type_node); > > in addition to: > > ./ada/gcc-interface/decl.c: = build_complex_type > ./ada/gcc-interface/decl.c: return build_complex_type (nt); > ./ada/gcc-interface/trans.c: tree gnu_ctype = build_complex_type > (gnu_type); > ./c/c-decl.c: specs->type = build_complex_type (specs->type); > ./c/c-decl.c: specs->type = build_complex_type (specs->type); > ./c/c-decl.c: specs->type = build_complex_type (specs->type); > ./c/c-parser.c: build_complex_type > ./c/c-typeck.c: return build_complex_type (subtype); > ./c-family/c-common.c: return build_complex_type (inner_type); > ./c-family/c-lex.c: type = build_complex_type (type); > ./cp/decl.c: type = build_complex_type (type); > ./cp/typeck.c: return build_type_attribute_variant (build_complex_type > (subtype), > ./fortran/trans-types.c:gfc_build_complex_type (tree scalar_type) > ./fortran/trans-types.c: type = gfc_build_complex_type (type); > ./go/go-gcc.cc: > build_complex_type(TREE_TYPE(real_tree)), > ./go/go-gcc.cc: type = build_complex_type(type); > ./lto/lto-lang.c: return build_complex_type (inner_type); > > Or perhaps *only* the calls in build_common_tree_nodes need to be changed? I think only the calls in build_common_tree_nodes -- those are the ones built early and that survive GC. The patch is ok if it passes testing with that. Richard. > It's certainly old code (r29604, September 1999). > -- > Eric Botcazou
On Mon, Oct 10, 2016 at 12:38 PM, Eric Botcazou <ebotcazou@adacore.com> wrote: >> I believe the rule is that you might only depend on the order of objects >> with respect to their DECL_UID, not the actual value of the DECL_UID. >> As var-tracking shouldn't look at TYPE_DECLs (?) it's probably a latent >> var-tracking bug as well. > > It presumably doesn't look at TYPE_DECLs, simply the DECL_UID of variables is > also different so this changes some hashing. Yes. But that's not the only source for DECL_UID differences. Btw, I see lots of FOR_EACH_HASH_TABLE_ELEMENT in var-tracking.c but they don't look like their outcome is supposed to be dependent on element ordering. Did you track down where exactly the code-gen difference appeared? >> I'd prefer the named parameter to be defaulted to false and the few >> places in the FEs fixed (eventually that name business should be >> handled like names for nodes like integer_type_node -- I see no >> reason why build_complex_type should have this special-case at all! >> That is, why are the named vairants in the type hash in the first place?) > > I think that the calls in build_common_tree_nodes need to be changed too then: > > complex_integer_type_node = build_complex_type (integer_type_node); > complex_float_type_node = build_complex_type (float_type_node); > complex_double_type_node = build_complex_type (double_type_node); > complex_long_double_type_node = build_complex_type (long_double_type_node); > > in addition to: > > ./ada/gcc-interface/decl.c: = build_complex_type > ./ada/gcc-interface/decl.c: return build_complex_type (nt); > ./ada/gcc-interface/trans.c: tree gnu_ctype = build_complex_type > (gnu_type); > ./c/c-decl.c: specs->type = build_complex_type (specs->type); > ./c/c-decl.c: specs->type = build_complex_type (specs->type); > ./c/c-decl.c: specs->type = build_complex_type (specs->type); > ./c/c-parser.c: build_complex_type > ./c/c-typeck.c: return build_complex_type (subtype); > ./c-family/c-common.c: return build_complex_type (inner_type); > ./c-family/c-lex.c: type = build_complex_type (type); > ./cp/decl.c: type = build_complex_type (type); > ./cp/typeck.c: return build_type_attribute_variant (build_complex_type > (subtype), > ./fortran/trans-types.c:gfc_build_complex_type (tree scalar_type) > ./fortran/trans-types.c: type = gfc_build_complex_type (type); > ./go/go-gcc.cc: > build_complex_type(TREE_TYPE(real_tree)), > ./go/go-gcc.cc: type = build_complex_type(type); > ./lto/lto-lang.c: return build_complex_type (inner_type); > > Or perhaps *only* the calls in build_common_tree_nodes need to be changed? > > It's certainly old code (r29604, September 1999). > > -- > Eric Botcazou
> Yes. But that's not the only source for DECL_UID differences. Btw, > I see lots of FOR_EACH_HASH_TABLE_ELEMENT in var-tracking.c > but they don't look like their outcome is supposed to be dependent on > element ordering. This leads to NOTE_INSN_VAR_LOCATION notes emitted in a different order, which are then interpreted by dwarf2out_var_location. In particular: (note 6350 6349 6351 (var_location temp (nil)) NOTE_INSN_VAR_LOCATION) (note 6351 6350 6352 (var_location temp$low (mem/c:DI (plus:SI (reg/f:SI 30 %fp) (const_int -112 [0xffffffffffffff90])) [10 MEM[(struct cpp_num *)&result + 8B]+0 S8 A64])) NOTE_INSN_VAR_LOCATION) (note 6352 6351 6353 (var_location temp$8 (nil)) NOTE_INSN_VAR_LOCATION) [...] (code_label 2091 6355 2092 79 912 "" [1 uses]) (note 2092 2091 5271 79 [bb 79] NOTE_INSN_BASIC_BLOCK) is interpreted differently from: (note 6350 6349 6351 (var_location temp (nil)) NOTE_INSN_VAR_LOCATION) (note 6351 6350 6352 (var_location temp$8 (nil)) NOTE_INSN_VAR_LOCATION) (note 6352 6351 6353 (var_location temp$low (mem/c:DI (plus:SI (reg/f:SI 30 %fp) (const_int -112 [0xffffffffffffff90])) [10 MEM[(struct cpp_num *)&result + 8B]+0 S8 A64])) NOTE_INSN_VAR_LOCATION) [...] (note 2092 2091 5271 79 [bb 79] NOTE_INSN_BASIC_BLOCK) @@ -32608,6 +32608,17 @@ .uleb128 0x8 .byte 0x93 ! DW_OP_piece .uleb128 0x8 + .uaword .LLVL592-.LLtext0 ! Location list begin address (*.LLLST153) + .uaword .LLVL597-.LLtext0 ! Location list end address (*.LLLST153) + .uahalf 0x9 ! Location expression size + .byte 0x93 ! DW_OP_piece + .uleb128 0x8 + .byte 0x8e ! DW_OP_breg30 + .sleb128 -112 + .byte 0x93 ! DW_OP_piece + .uleb128 0x8 + .byte 0x93 ! DW_OP_piece + .uleb128 0x8 .uaword .LLVL695-.LLtext0 ! Location list begin address (*.LLLST153) .uaword .LLVL696-.LLtext0 ! Location list end address (*.LLLST153) .uahalf 0xe ! Location expression size probably because the non-null location comes last in the second case.
On Thu, Oct 13, 2016 at 12:15 PM, Eric Botcazou <ebotcazou@adacore.com> wrote: >> Yes. But that's not the only source for DECL_UID differences. Btw, >> I see lots of FOR_EACH_HASH_TABLE_ELEMENT in var-tracking.c >> but they don't look like their outcome is supposed to be dependent on >> element ordering. > > This leads to NOTE_INSN_VAR_LOCATION notes emitted in a different order, which > are then interpreted by dwarf2out_var_location. In particular: > > (note 6350 6349 6351 (var_location temp (nil)) NOTE_INSN_VAR_LOCATION) > (note 6351 6350 6352 (var_location temp$low (mem/c:DI (plus:SI (reg/f:SI 30 > %fp) > (const_int -112 [0xffffffffffffff90])) [10 MEM[(struct cpp_num > *)&result + 8B]+0 S8 A64])) NOTE_INSN_VAR_LOCATION) > (note 6352 6351 6353 (var_location temp$8 (nil)) NOTE_INSN_VAR_LOCATION) > [...] > (code_label 2091 6355 2092 79 912 "" [1 uses]) > (note 2092 2091 5271 79 [bb 79] NOTE_INSN_BASIC_BLOCK) > > is interpreted differently from: > > (note 6350 6349 6351 (var_location temp (nil)) NOTE_INSN_VAR_LOCATION) > (note 6351 6350 6352 (var_location temp$8 (nil)) NOTE_INSN_VAR_LOCATION) > (note 6352 6351 6353 (var_location temp$low (mem/c:DI (plus:SI (reg/f:SI 30 > %fp) > (const_int -112 [0xffffffffffffff90])) [10 MEM[(struct cpp_num > *)&result + 8B]+0 S8 A64])) NOTE_INSN_VAR_LOCATION) > [...] > (note 2092 2091 5271 79 [bb 79] NOTE_INSN_BASIC_BLOCK) > > @@ -32608,6 +32608,17 @@ > .uleb128 0x8 > .byte 0x93 ! DW_OP_piece > .uleb128 0x8 > + .uaword .LLVL592-.LLtext0 ! Location list begin address > (*.LLLST153) > + .uaword .LLVL597-.LLtext0 ! Location list end address > (*.LLLST153) > + .uahalf 0x9 ! Location expression size > + .byte 0x93 ! DW_OP_piece > + .uleb128 0x8 > + .byte 0x8e ! DW_OP_breg30 > + .sleb128 -112 > + .byte 0x93 ! DW_OP_piece > + .uleb128 0x8 > + .byte 0x93 ! DW_OP_piece > + .uleb128 0x8 > .uaword .LLVL695-.LLtext0 ! Location list begin address > (*.LLLST153) > .uaword .LLVL696-.LLtext0 ! Location list end address > (*.LLLST153) > .uahalf 0xe ! Location expression size > > probably because the non-null location comes last in the second case. Definitely looks like a bug to me. Can you open a PR for this so it doesn't get lost? Thanks, Richard. > -- > Eric Botcazou
On Thu, Oct 13, 2016 at 12:19:53PM +0200, Richard Biener wrote: > > (note 6350 6349 6351 (var_location temp (nil)) NOTE_INSN_VAR_LOCATION) > > (note 6351 6350 6352 (var_location temp$low (mem/c:DI (plus:SI (reg/f:SI 30 > > %fp) > > (const_int -112 [0xffffffffffffff90])) [10 MEM[(struct cpp_num > > *)&result + 8B]+0 S8 A64])) NOTE_INSN_VAR_LOCATION) > > (note 6352 6351 6353 (var_location temp$8 (nil)) NOTE_INSN_VAR_LOCATION) > > [...] > > (code_label 2091 6355 2092 79 912 "" [1 uses]) > > (note 2092 2091 5271 79 [bb 79] NOTE_INSN_BASIC_BLOCK) > > > > is interpreted differently from: > > > > (note 6350 6349 6351 (var_location temp (nil)) NOTE_INSN_VAR_LOCATION) > > (note 6351 6350 6352 (var_location temp$8 (nil)) NOTE_INSN_VAR_LOCATION) > > (note 6352 6351 6353 (var_location temp$low (mem/c:DI (plus:SI (reg/f:SI 30 > > %fp) > > (const_int -112 [0xffffffffffffff90])) [10 MEM[(struct cpp_num > > *)&result + 8B]+0 S8 A64])) NOTE_INSN_VAR_LOCATION) > > [...] > > (note 2092 2091 5271 79 [bb 79] NOTE_INSN_BASIC_BLOCK) > > > > @@ -32608,6 +32608,17 @@ > > .uleb128 0x8 > > .byte 0x93 ! DW_OP_piece > > .uleb128 0x8 > > + .uaword .LLVL592-.LLtext0 ! Location list begin address > > (*.LLLST153) > > + .uaword .LLVL597-.LLtext0 ! Location list end address > > (*.LLLST153) > > + .uahalf 0x9 ! Location expression size > > + .byte 0x93 ! DW_OP_piece > > + .uleb128 0x8 > > + .byte 0x8e ! DW_OP_breg30 > > + .sleb128 -112 > > + .byte 0x93 ! DW_OP_piece > > + .uleb128 0x8 > > + .byte 0x93 ! DW_OP_piece > > + .uleb128 0x8 > > .uaword .LLVL695-.LLtext0 ! Location list begin address > > (*.LLLST153) > > .uaword .LLVL696-.LLtext0 ! Location list end address > > (*.LLLST153) > > .uahalf 0xe ! Location expression size > > > > probably because the non-null location comes last in the second case. > > Definitely looks like a bug to me. Can you open a PR for this so it doesn't get > lost? I guess it depends on whether temp$8 and temp$low overlap or not. If they overlap, then the different orders of course matter and should matter. Jakub
> I guess it depends on whether temp$8 and temp$low overlap or not.
I don't think so, the code is identical so that would be a more serious bug:
struct cpp_num
{
cpp_num_part high;
cpp_num_part low;
bool unsignedp;
bool overflow;
};
I think it's just dwarf2out_var_location being sensitive to the ordering.
Index: builtins.c =================================================================== --- builtins.c (revision 240888) +++ builtins.c (working copy) @@ -2356,7 +2356,7 @@ expand_builtin_cexpi (tree exp, rtx targ else { tree call, fn = NULL_TREE, narg; - tree ctype = build_complex_type (type); + tree ctype = build_complex_type (type, false); if (DECL_FUNCTION_CODE (fndecl) == BUILT_IN_CEXPIF) fn = builtin_decl_explicit (BUILT_IN_CEXPF); @@ -2396,7 +2396,7 @@ expand_builtin_cexpi (tree exp, rtx targ } /* Now build the proper return type. */ - return expand_expr (build2 (COMPLEX_EXPR, build_complex_type (type), + return expand_expr (build2 (COMPLEX_EXPR, build_complex_type (type, false), make_tree (TREE_TYPE (arg), op2), make_tree (TREE_TYPE (arg), op1)), target, VOIDmode, EXPAND_NORMAL); @@ -7226,7 +7226,7 @@ fold_builtin_sincos (location_t loc, /* Canonicalize sincos to cexpi. */ if (TREE_CODE (arg0) == REAL_CST) { - tree complex_type = build_complex_type (type); + tree complex_type = build_complex_type (type, false); call = fold_const_call (as_combined_fn (fn), complex_type, arg0); } if (!call) @@ -8137,7 +8137,7 @@ fold_builtin_arith_overflow (location_t ? boolean_true_node : boolean_false_node, arg2); - tree ctype = build_complex_type (type); + tree ctype = build_complex_type (type, false); tree call = build_call_expr_internal_loc (loc, ifn, ctype, 2, arg0, arg1); tree tgt = save_expr (call); Index: gimple-fold.c =================================================================== --- gimple-fold.c (revision 240888) +++ gimple-fold.c (working copy) @@ -3210,7 +3210,7 @@ fold_builtin_atomic_compare_exchange (gi tree fndecl = gimple_call_fndecl (stmt); tree parmt = TYPE_ARG_TYPES (TREE_TYPE (fndecl)); tree itype = TREE_VALUE (TREE_CHAIN (TREE_CHAIN (parmt))); - tree ctype = build_complex_type (itype); + tree ctype = build_complex_type (itype, false); tree expected = TREE_OPERAND (gimple_call_arg (stmt, 1), 0); gimple *g = gimple_build_assign (make_ssa_name (TREE_TYPE (expected)), expected); @@ -3582,7 +3582,7 @@ gimple_fold_call (gimple_stmt_iterator * { if (overflow == NULL_TREE) overflow = build_zero_cst (TREE_TYPE (result)); - tree ctype = build_complex_type (TREE_TYPE (result)); + tree ctype = build_complex_type (TREE_TYPE (result), false); if (TREE_CODE (result) == INTEGER_CST && TREE_CODE (overflow) == INTEGER_CST) result = build_complex (ctype, result, overflow); Index: stor-layout.c =================================================================== --- stor-layout.c (revision 240888) +++ stor-layout.c (working copy) @@ -469,7 +469,7 @@ bitwise_type_for_mode (machine_mode mode return build_vector_type_for_mode (inner_type, mode); if (COMPLEX_MODE_P (mode)) - return build_complex_type (inner_type); + return build_complex_type (inner_type, false); gcc_checking_assert (GET_MODE_INNER (mode) == mode); return inner_type; Index: tree-ssa-dce.c =================================================================== --- tree-ssa-dce.c (revision 240888) +++ tree-ssa-dce.c (working copy) @@ -1194,7 +1194,7 @@ maybe_optimize_arith_overflow (gimple_st if (TREE_CODE (result) == INTEGER_CST && TREE_OVERFLOW (result)) result = drop_tree_overflow (result); tree overflow = build_zero_cst (type); - tree ctype = build_complex_type (type); + tree ctype = build_complex_type (type, false); if (TREE_CODE (result) == INTEGER_CST) result = build_complex (ctype, result, overflow); else Index: tree-ssa-math-opts.c =================================================================== --- tree-ssa-math-opts.c (revision 240888) +++ tree-ssa-math-opts.c (working copy) @@ -3736,7 +3736,7 @@ match_uaddsub_overflow (gimple_stmt_iter if (!ovf_use_seen || !use_seen) return false; - tree ctype = build_complex_type (type); + tree ctype = build_complex_type (type, false); tree rhs1 = gimple_assign_rhs1 (stmt); tree rhs2 = gimple_assign_rhs2 (stmt); gcall *g = gimple_build_call_internal (code == PLUS_EXPR Index: tree.c =================================================================== --- tree.c (revision 240888) +++ tree.c (working copy) @@ -2021,7 +2021,7 @@ build_complex (tree type, tree real, tre TREE_REALPART (t) = real; TREE_IMAGPART (t) = imag; - TREE_TYPE (t) = type ? type : build_complex_type (TREE_TYPE (real)); + TREE_TYPE (t) = type ? type : build_complex_type (TREE_TYPE (real), false); TREE_OVERFLOW (t) = TREE_OVERFLOW (real) | TREE_OVERFLOW (imag); return t; } @@ -8758,10 +8758,15 @@ build_offset_type (tree basetype, tree t return t; } -/* Create a complex type whose components are COMPONENT_TYPE. */ +/* Create a complex type whose components are COMPONENT_TYPE. + + If NAMED is true, the type is given a TYPE_NAME. We do not always + do so because this creates a DECL node and thus make the DECL_UIDs + dependent on the type canonicalization hashtable, which is GC-ed, + so the DECL_UIDs would not be stable wrt garbage collection. */ tree -build_complex_type (tree component_type) +build_complex_type (tree component_type, bool named) { tree t; inchash::hash hstate; @@ -8788,11 +8793,11 @@ build_complex_type (tree component_type) SET_TYPE_STRUCTURAL_EQUALITY (t); else if (TYPE_CANONICAL (component_type) != component_type) TYPE_CANONICAL (t) - = build_complex_type (TYPE_CANONICAL (component_type)); + = build_complex_type (TYPE_CANONICAL (component_type), named); } /* We need to create a name, since complex is a fundamental type. */ - if (! TYPE_NAME (t)) + if (!TYPE_NAME (t) && named) { const char *name; if (component_type == char_type_node) Index: tree.h =================================================================== --- tree.h (revision 240888) +++ tree.h (working copy) @@ -4042,7 +4042,7 @@ extern tree build_varargs_function_type_ extern tree build_method_type_directly (tree, tree, tree); extern tree build_method_type (tree, tree); extern tree build_offset_type (tree, tree); -extern tree build_complex_type (tree); +extern tree build_complex_type (tree, bool named = true); extern tree array_type_nelts (const_tree); extern tree value_member (tree, tree);
Hi, adding patterns for unsigned arithmetic overflow checking in a back-end can have unexpected fallout because of a latent GC issue: when they are present, GIMPLE optimization passes can create complex (math. sense) types at will by invoking build_complex_type. Now build_complex_type goes through the type caonicalization hashtable, which is GC-ed, so its behavior depends on the actual collection points. The other type-building functions present in tree.c do the same so no big deal but build_complex_type is special because it also does: /* We need to create a name, since complex is a fundamental type. */ if (! TYPE_NAME (t)) { const char *name; if (component_type == char_type_node) name = "complex char"; else if (component_type == signed_char_type_node) name = "complex signed char"; else if (component_type == unsigned_char_type_node) name = "complex unsigned char"; else if (component_type == short_integer_type_node) name = "complex short int"; else if (component_type == short_unsigned_type_node) name = "complex short unsigned int"; else if (component_type == integer_type_node) name = "complex int"; else if (component_type == unsigned_type_node) name = "complex unsigned int"; else if (component_type == long_integer_type_node) name = "complex long int"; else if (component_type == long_unsigned_type_node) name = "complex long unsigned int"; else if (component_type == long_long_integer_type_node) name = "complex long long int"; else if (component_type == long_long_unsigned_type_node) name = "complex long long unsigned int"; else name = 0; if (name != 0) TYPE_NAME (t) = build_decl (UNKNOWN_LOCATION, TYPE_DECL, get_identifier (name), t); } so it creates a DECL node every time a new canonical complex type is created, bumping the DECL_UID counter in the process. Which means that the DECL_UID counter is sensitive to the collection points, which in turn means that the result of algorithms depending on the DECL_UID counter also is. This for example resulted in a bootstrap comparison failure on a SPARC/Solaris machine doing a strict stage2/stage3 comparison because the contents of the .debug_loc section were different: location lists computed by var-tracking were slightly different because of a different hashing. I'm not sure whether the hashing done by var-tracking should be sensitive to the DECL_UID of nodes or not, but I think that having the DECL_UID counter depend on the collection points is highly undesirable, so the attached patch attempts to prevent it; it at least fixed the bootstrap comparison failure. Tested on x86_64-suse-linux, OK for the mainline? 2016-10-08 Eric Botcazou <ebotcazou@adacore.com> * tree.h (build_complex_type): Add second parameter with default. * builtins.c (expand_builtin_cexpi): Pass false in call to above. (fold_builtin_sincos): Likewise. (fold_builtin_arith_overflow): Likewise. * gimple-fold.c (fold_builtin_atomic_compare_exchange): Likewise. (gimple_fold_call): Likewise. * stor-layout.c (bitwise_type_for_mode): Likewise. * tree-ssa-dce.c (maybe_optimize_arith_overflow): Likewise. * tree-ssa-math-opts.c (match_uaddsub_overflow): Likewise. * tree.c (build_complex): Likewise. (build_complex_type): Add NAMED second parameter and adjust recursive call. Create a TYPE_DECL only if NAMED is true.