Re: [PATCH][GCC][AArch64] Restrict lrint inlining on ILP32.
Hi Andrew, Yes I noticed the changelog after I sent it. But I have a new version of the patch on internal review Which is why I didn't ping this one. I'll send it out after the Cauldron. Thanks, Tamar From: Andrew Pinski Sent: Saturday, September 9, 2017 7:57:00 AM To: Tamar Christina Cc: GCC Patches; nd; James Greenhalgh; Richard Earnshaw; Marcus Shawcroft Subject: Re: [PATCH][GCC][AArch64] Restrict lrint inlining on ILP32. On Fri, Aug 11, 2017 at 2:58 AM, Tamar Christina wrote: > Hi All, > > The inlining of lrint isn't valid in all cases on ILP32 when > -fno-math-errno is used because an inexact exception is raised in > certain circumstances. > > For ILP32 I now restrict the inlining only when -fno-trapping-math > is also specified. > > This fixed PR/81800. > > Regtested on aarch64-none-linux-gnu and no regressions. > > Ok for trunk? I can't approve this patch but it looks good except for your changelog does not match what the code is doing. Maybe: Add check on !ilp32 or !flag_trapping_math. Thanks, Andrew > > Thanks, > Tamar > > gcc/ > 2017-08-11 Tamar Christina > > PR target/81800 > * config/aarch64/aarch64.md (lrint2): Add > flag_trapping_math. > > gcc/testsuite/ > 2017-08-11 Tamar Christina > > * gcc.target/aarch64/inline-lrint_2.c (dg-options): Add > -fno-trapping-math. > > --
Re: [C++] Fix PR bootstrap/81926
On Mon, Sep 4, 2017 at 10:08 AM, Eric Botcazou wrote: >> A solution would be to put them into a global GCed pointer-map or vector, >> freeing that at free-lang-data time. > > Here's a version that implements a bona-fide type->offset_type map. > > PR bootstrap/81926 > * cp-objcp-common.c (struct offset_type_hasher): New class. > (offset_type_hash): New variable. > (cp_get_debug_type): Associate the OFFSET_TYPEs with the types. This looks good. But let's call it debug_type_hash instead. OK with that change. Jason
Re: [PATCH] PR c++/81852 define feature-test macro for -fthreadsafe-statics
On Fri, Sep 8, 2017 at 10:03 PM, Jonathan Wakely wrote: > Define the __cpp_lib_threadsafe_static_init feature-test macro as per > recent SD-6 drafts. > > Tested powerpc64le-linux and x86_64-linux. > > OK for trunk? > > The branches too? Yes. Jason
Re: [PATCH, rs6000] Fix PR target/80210: ICE in extract_insn
On Sep 08 2017, Peter Bergner wrote: > The following patch fixes the problem I saw on Linux and bootstraps and > regtests > with no regressions on LE and BE (running testsuite in both 32-bit and 64-bit > modes). I was waiting to submit this until David had a chance to verify this > fixes the problem on AIX. I've verified that this fixes the ICE. Andreas. -- Andreas Schwab, sch...@linux-m68k.org GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 "And now for something completely different."
Re: [PATCH] Improve alloca alignment
> No, the stack never gets misaligned - my patch doesn't change that at all. Yes, it does. Dynamic allocation works like this: the amount to be allocated is added to VIRTUAL_STACK_DYNAMIC_REGNUM and the result is then dynamically aligned. Your patch assumes that VIRTUAL_STACK_DYNAMIC_REGNUM is as aligned as the stack pointer, i.e. STACK_BOUNDARY, but that's wrong for 32-bit SPARC at least (that's why I put the ??? note at line 5746 in emit-rtl.c). The previous attempt by Dominic Vogt was more correct in this respect: 2016-08-23 Dominik Vogt * explow.c (get_dynamic_stack_size): Take known alignment of stack pointer + STACK_DYNAMIC_OFFSET into account when calculating the size needed. since it was using the declared alignment of VIRTUAL_STACK_DYNAMIC_REGNUM and not STACK_BOUNDARY directly. But the outcome was the same, since the declared alignment is still wrong for 32-bit SPARC. > The issue is that Sparc backend doesn't correctly set STACK_BOUNDSARY and > neither aligns the outgoing args. Run my test which proves alloca was > broken before my patch. How could this have been broken for so long, realistically? The SPARC back- end is parameterized according to the ABI and the documented interface between middle-end and back-end. -- Eric Botcazou
Re: [PATCH] Add macro DISABLE_COPY_AND_ASSIGN
On Fri, Aug 11, 2017 at 3:14 PM, Pedro Alves > Yeah, this is a macro that lots of projects out there reinvent, > can't imagine it being very controversial. > > I could have used this today in another spot in gdb. > > The patch as is touches areas with different maintainers, it > may have fallen victim of diffusion of responsibility. > > Could we get at least the ansidecl.h change in, so we can > start using it in gdb? CCing Ian as a libiberty maintainer. Hi Ian, I just talked with you about this patch. You are cc'ed. Could you take a look at the change in include/ansidecl.h? Then, we can use it in different projects, gcc, gdb and gold. -- Yao (齐尧)
[Ada] Small tweak to conversion to padding types
While investigating another issue, I came across a case where a simple assignment gives rise to the creation of two CONSTRUCTORS in a row when a padding type is involved. I think we can get away with only one. Tested on x86_64-suse-linux, applied on the mainline. 2017-09-09 Eric Botcazou * gcc-interface/utils.c (convert): When converting to a padding type, reuse an existing CONSTRUCTOR if it has got the right size. -- Eric BotcazouIndex: gcc-interface/utils.c === --- gcc-interface/utils.c (revision 251906) +++ gcc-interface/utils.c (working copy) @@ -4219,8 +4219,6 @@ convert (tree type, tree expr) constructor to build the record, unless a variable size is involved. */ else if (code == RECORD_TYPE && TYPE_PADDING_P (type)) { - vec *v; - /* If we previously converted from another type and our type is of variable size, remove the conversion to avoid the need for variable-sized temporaries. Likewise for a conversion between @@ -4272,9 +4270,21 @@ convert (tree type, tree expr) expr), false); + tree t = convert (TREE_TYPE (TYPE_FIELDS (type)), expr); + + /* If converting to the inner type has already created a CONSTRUCTOR with + the right size, then reuse it instead of creating another one. This + can happen for the padding type built to overalign local variables. */ + if (TREE_CODE (t) == VIEW_CONVERT_EXPR + && TREE_CODE (TREE_OPERAND (t, 0)) == CONSTRUCTOR + && TREE_CONSTANT (TYPE_SIZE (TREE_TYPE (TREE_OPERAND (t, 0 + && tree_int_cst_equal (TYPE_SIZE (type), + TYPE_SIZE (TREE_TYPE (TREE_OPERAND (t, 0) + return build1 (VIEW_CONVERT_EXPR, type, TREE_OPERAND (t, 0)); + + vec *v; vec_alloc (v, 1); - CONSTRUCTOR_APPEND_ELT (v, TYPE_FIELDS (type), - convert (TREE_TYPE (TYPE_FIELDS (type)), expr)); + CONSTRUCTOR_APPEND_ELT (v, TYPE_FIELDS (type), t); return gnat_build_constructor (type, v); }
Re: [Patch, fortran] Parameterized Derived Types
Dear All, The patch has been committed as revision 251925. I look forward to "feedback" (aka PRs) in the coming weeks. I will return to Parameterized Derived Types at the end of this month to clear up some of the known deficiencies (see the notes attached to the patch submission) and any PRs that arise. Regards Paul
[Ada] Internal error on volatile array with VFA component
It's caused by a mode mismatch between variants of the same type. Tested on x86_64-suse-linux, applied on the mainline and 7 branch. 2017-09-09 Eric Botcazou * gcc-interface/decl.c (gnat_to_gnu_entity): Only set theTYPE_ALIGN_OK and TYPE_BY_REFERENCE_P flags on types after various promotions. * gcc-interface/trans.c (node_has_volatile_full_access): Consider all kinds of entities. 2017-09-09 Eric Botcazou * gnat.dg/specs/vfa.ads: Rename into... * gnat.dg/specs/vfa1.ads: ...this. * gnat.dg/specs/vfa2.ads: New test. -- Eric BotcazouIndex: gcc-interface/decl.c === --- gcc-interface/decl.c (revision 251906) +++ gcc-interface/decl.c (working copy) @@ -4277,18 +4277,6 @@ gnat_to_gnu_entity (Entity_Id gnat_entit already defined so we cannot pass true for IN_PLACE here. */ process_attributes (&gnu_type, &attr_list, false, gnat_entity); - /* Tell the middle-end that objects of tagged types are guaranteed to - be properly aligned. This is necessary because conversions to the - class-wide type are translated into conversions to the root type, - which can be less aligned than some of its derived types. */ - if (Is_Tagged_Type (gnat_entity) - || Is_Class_Wide_Equivalent_Type (gnat_entity)) - TYPE_ALIGN_OK (gnu_type) = 1; - - /* Record whether the type is passed by reference. */ - if (!VOID_TYPE_P (gnu_type) && Is_By_Reference_Type (gnat_entity)) - TYPE_BY_REFERENCE_P (gnu_type) = 1; - /* ??? Don't set the size for a String_Literal since it is either confirming or we don't handle it properly (if the low bound is non-constant). */ @@ -4498,17 +4486,29 @@ gnat_to_gnu_entity (Entity_Id gnat_entit /* If this is not an unconstrained array type, set some flags. */ if (TREE_CODE (gnu_type) != UNCONSTRAINED_ARRAY_TYPE) { + /* Tell the middle-end that objects of tagged types are guaranteed to + be properly aligned. This is necessary because conversions to the + class-wide type are translated into conversions to the root type, + which can be less aligned than some of its derived types. */ + if (Is_Tagged_Type (gnat_entity) + || Is_Class_Wide_Equivalent_Type (gnat_entity)) + TYPE_ALIGN_OK (gnu_type) = 1; + + /* Record whether the type is passed by reference. */ + if (Is_By_Reference_Type (gnat_entity) && !VOID_TYPE_P (gnu_type)) + TYPE_BY_REFERENCE_P (gnu_type) = 1; + + /* Record whether an alignment clause was specified. */ if (Present (Alignment_Clause (gnat_entity))) TYPE_USER_ALIGN (gnu_type) = 1; + /* Record whether a pragma Universal_Aliasing was specified. */ if (Universal_Aliasing (gnat_entity) && !TYPE_IS_DUMMY_P (gnu_type)) TYPE_UNIVERSAL_ALIASING_P (gnu_type) = 1; /* If it is passed by reference, force BLKmode to ensure that objects of this type will always be put in memory. */ - if (TYPE_MODE (gnu_type) != BLKmode - && AGGREGATE_TYPE_P (gnu_type) - && TYPE_BY_REFERENCE_P (gnu_type)) + if (AGGREGATE_TYPE_P (gnu_type) && TYPE_BY_REFERENCE_P (gnu_type)) SET_TYPE_MODE (gnu_type, BLKmode); } Index: gcc-interface/trans.c === --- gcc-interface/trans.c (revision 251906) +++ gcc-interface/trans.c (working copy) @@ -4075,8 +4075,6 @@ node_has_volatile_full_access (Node_Id g case N_Identifier: case N_Expanded_Name: gnat_entity = Entity (gnat_node); - if (Ekind (gnat_entity) != E_Variable) - break; return Is_Volatile_Full_Access (gnat_entity) || Is_Volatile_Full_Access (Etype (gnat_entity)); -- { dg-do compile } -- { dg-options "-O" } package VFA2 is type Bit is mod 2**1 with Size => 1; type UInt2 is mod 2**2 with Size => 2; type UInt22 is mod 2**22 with Size => 22; type MODE_ENUM is ( Function_0_Default, Function_1, Function_2, Function_3, Function_4, Function_5, Function_6, Function_7) with Size => 3; type EPD_ENUM is ( Disable_Pull_Down, Enable_Pull_Down) with Size => 1; type EPUN_ENUM is ( Enable_Pull_Up, Disable_Pull_Up) with Size => 1; type EHS_ENUM is ( Slow_Low_Noise_With, Fast_Medium_Noise_W) with Size => 1; type EZI_ENUM is ( Disable_Input_Buffer, Enable_Input_Buffer) with Size => 1; type ZIF_ENUM is ( Enable_Input_Glitch, Disable_Input_Glitch) with Size => 1; type EHD_ENUM is ( Normal_Drive_4_Ma_D, Medium_Drive_8_Ma_D, High_Drive_14_Ma_Dr, Ultra_High_Drive_20) with Size => 2; type Pin_Type is (Normal_Drive, High_Drive, High_Speed); type SFS_Register(Pin : Pin_Type := Normal_Drive) is record MODE : MODE_ENUM; EPD : EPD_ENUM; EP
[Ada] Improve handling of aliased constant of UNC array type
Tested on x86_64-suse-linux, applied on the mainline and 7 branch. 2017-09-09 Eric Botcazou * gcc-interface/decl.c (gnat_to_gnu_entity) : Apply the promotion to static memory earlier in the processing. -- Eric BotcazouIndex: gcc-interface/decl.c === --- gcc-interface/decl.c (revision 251927) +++ gcc-interface/decl.c (working copy) @@ -1423,6 +1423,19 @@ gnat_to_gnu_entity (Entity_Id gnat_entit gnu_size = NULL_TREE; } + /* If this is an aggregate constant initialized to a constant, force it + to be statically allocated. This saves an initialization copy. */ + if (!static_flag + && const_flag + && gnu_expr + && TREE_CONSTANT (gnu_expr) + && AGGREGATE_TYPE_P (gnu_type) + && tree_fits_uhwi_p (TYPE_SIZE_UNIT (gnu_type)) + && !(TYPE_IS_PADDING_P (gnu_type) + && !tree_fits_uhwi_p (TYPE_SIZE_UNIT + (TREE_TYPE (TYPE_FIELDS (gnu_type)) + static_flag = true; + /* If this is an aliased object with an unconstrained array nominal subtype, we make its type a thin reference, i.e. the reference counterpart of a thin pointer, so it points to the array part. @@ -1474,18 +1487,6 @@ gnat_to_gnu_entity (Entity_Id gnat_entit && No (Address_Clause (gnat_entity gnu_ext_name = create_concat_name (gnat_entity, NULL); - /* If this is an aggregate constant initialized to a constant, force it - to be statically allocated. This saves an initialization copy. */ - if (!static_flag - && const_flag - && gnu_expr && TREE_CONSTANT (gnu_expr) - && AGGREGATE_TYPE_P (gnu_type) - && tree_fits_uhwi_p (TYPE_SIZE_UNIT (gnu_type)) - && !(TYPE_IS_PADDING_P (gnu_type) - && !tree_fits_uhwi_p (TYPE_SIZE_UNIT - (TREE_TYPE (TYPE_FIELDS (gnu_type)) - static_flag = true; - /* Deal with a pragma Linker_Section on a constant or variable. */ if ((kind == E_Constant || kind == E_Variable) && Present (Linker_Section_Pragma (gnat_entity)))
Re: [PATCH] Add macro DISABLE_COPY_AND_ASSIGN
On Sat, Sep 9, 2017 at 2:45 AM, Yao Qi wrote: > On Fri, Aug 11, 2017 at 3:14 PM, Pedro Alves >> Yeah, this is a macro that lots of projects out there reinvent, >> can't imagine it being very controversial. >> >> I could have used this today in another spot in gdb. >> >> The patch as is touches areas with different maintainers, it >> may have fallen victim of diffusion of responsibility. >> >> Could we get at least the ansidecl.h change in, so we can >> start using it in gdb? CCing Ian as a libiberty maintainer. > > Hi Ian, > I just talked with you about this patch. You are cc'ed. Could you > take a look at the change in include/ansidecl.h? Then, we can use it > in different > projects, gcc, gdb and gold. The patch to include/ansidecl.h is basically OK. Please add an example in the comment showing how to use it: after `private:`, and with a trailing semicolon. Thanks. The patches to the other files will have to be approved by the relevant maintainers. Ian
[Ada] Pragma Atomic wrongly rejected on composite component
The compiler wrongly rejects a pragma Atomic on a component of a record whose type is composite, but it accepts the pragma on a variable of the same type. Tested on x86_64-suse-linux, applied on the mainline and 7 branch. 2017-09-09 Eric Botcazou * gcc-interface/decl.c (promote_object_alignment): New function taken from... (gnat_to_gnu_entity) : ...here. Invoke it. (gnat_to_gnu_field): If the field is Atomic or VFA, invoke it and create a padding type on success before doing the atomic check. 2017-09-09 Eric Botcazou * gnat.dg/specs/atomic3.ads: New test. -- Eric Botcazou-- { dg-do compile } package Atomic3 is type Four_Bits is mod 2**4; type Fourteen_Bits is mod 2**14; type Twenty_Eight_Bits is mod 2**28; type Rec1 (Mode : Boolean := True) is record Reserved : Four_Bits; case Mode is when True => High_Part : Fourteen_Bits; Low_Part : Fourteen_Bits; when False => Data : Twenty_Eight_Bits; end case; end record; for Rec1 use record Reserved at 0 range 28 .. 31; High_Part at 0 range 14 .. 27; Low_Part at 0 range 0 .. 13; Data at 0 range 0 .. 27; end record; for Rec1'Size use 32; pragma Unchecked_Union (Rec1); type Rec2 is record A : Rec1; pragma Atomic (A); end record; end Atomic3; Index: gcc-interface/decl.c === --- gcc-interface/decl.c (revision 251929) +++ gcc-interface/decl.c (working copy) @@ -230,6 +230,7 @@ static vec build_variant_l static tree validate_size (Uint, tree, Entity_Id, enum tree_code, bool, bool); static void set_rm_size (Uint, tree, Entity_Id); static unsigned int validate_alignment (Uint, Entity_Id, unsigned int); +static unsigned int promote_object_alignment (tree, Entity_Id); static void check_ok_for_atomic_type (tree, Entity_Id, bool); static tree create_field_decl_from (tree, tree, tree, tree, tree, vec); @@ -856,45 +857,7 @@ gnat_to_gnu_entity (Entity_Id gnat_entit && No (Renamed_Object (gnat_entity)) && No (Address_Clause (gnat_entity && TREE_CODE (TYPE_SIZE (gnu_type)) == INTEGER_CST) - { - unsigned int size_cap, align_cap; - - /* No point in promoting the alignment if this doesn't prevent - BLKmode access to the object, in particular block copy, as - this will for example disable the NRV optimization for it. - No point in jumping through all the hoops needed in order - to support BIGGEST_ALIGNMENT if we don't really have to. - So we cap to the smallest alignment that corresponds to - a known efficient memory access pattern of the target. */ - if (Is_Atomic_Or_VFA (gnat_entity)) - { - size_cap = UINT_MAX; - align_cap = BIGGEST_ALIGNMENT; - } - else - { - size_cap = MAX_FIXED_MODE_SIZE; - align_cap = get_mode_alignment (ptr_mode); - } - - if (!tree_fits_uhwi_p (TYPE_SIZE (gnu_type)) - || compare_tree_int (TYPE_SIZE (gnu_type), size_cap) > 0) - align = 0; - else if (compare_tree_int (TYPE_SIZE (gnu_type), align_cap) > 0) - align = align_cap; - else - align = ceil_pow2 (tree_to_uhwi (TYPE_SIZE (gnu_type))); - - /* But make sure not to under-align the object. */ - if (align <= TYPE_ALIGN (gnu_type)) - align = 0; - - /* And honor the minimum valid atomic alignment, if any. */ -#ifdef MINIMUM_ATOMIC_ALIGNMENT - else if (align < MINIMUM_ATOMIC_ALIGNMENT) - align = MINIMUM_ATOMIC_ALIGNMENT; -#endif - } + align = promote_object_alignment (gnu_type, gnat_entity); /* If the object is set to have atomic components, find the component type and validate it. @@ -6891,7 +6854,15 @@ gnat_to_gnu_field (Entity_Id gnat_field, } if (Is_Atomic_Or_VFA (gnat_field)) -check_ok_for_atomic_type (gnu_field_type, gnat_field, false); +{ + const unsigned int align + = promote_object_alignment (gnu_field_type, gnat_field); + if (align > 0) + gnu_field_type + = maybe_pad_type (gnu_field_type, NULL_TREE, align, gnat_field, + false, false, definition, true); + check_ok_for_atomic_type (gnu_field_type, gnat_field, false); +} if (Present (Component_Clause (gnat_field))) { @@ -8807,6 +8778,53 @@ validate_alignment (Uint alignment, Enti return align; } + +/* Promote the alignment of GNU_TYPE corresponding to GNAT_ENTITY. Return + a positive value on success or zero on failure. */ + +static unsigned int +promote_object_alignment (tree gnu_type, Entity_Id gnat_entity) +{ + unsigned int align, size_cap, align_cap; + + /* No point in promoting the alignment if this doesn't prevent BLKmode access + to the object, in particular block copy, as this will for example disable + the NRV optimization for it. No point in jumping through all the hoops + needed in order to suppor
[Ada] Fix wrong derivation of record type into unchecked union
It's a fallout of the new implementation of layout for derived record types. Tested on x86_64-suse-linux, applied on the mainline. 2017-09-09 Eric Botcazou * gcc-interface/decl.c (gnat_to_gnu_entity) : Copy the layout of the record from the parent type only if both are or are not unchecked unions. (is_stored_discriminant): Return false for an unchecked union. -- Eric BotcazouIndex: gcc-interface/decl.c === --- gcc-interface/decl.c (revision 251931) +++ gcc-interface/decl.c (working copy) @@ -3287,15 +3287,15 @@ gnat_to_gnu_entity (Entity_Id gnat_entit } /* If this is a derived type with discriminants and these discriminants - affect the initial shape it has inherited, factor them in. But for - an Unchecked_Union (it must be an Itype), just process the type. */ + affect the initial shape it has inherited, factor them in. */ if (has_discr && !is_extension && !Has_Record_Rep_Clause (gnat_entity) && Stored_Constraint (gnat_entity) != No_Elist && (gnat_parent_type = Underlying_Type (Etype (gnat_entity))) && Is_Record_Type (gnat_parent_type) - && !Is_Unchecked_Union (gnat_parent_type) + && Is_Unchecked_Union (gnat_entity) + == Is_Unchecked_Union (gnat_parent_type) && No_Reordering (gnat_entity) == No_Reordering (gnat_parent_type)) { tree gnu_parent_type @@ -9328,7 +9328,9 @@ copy_and_substitute_in_size (tree new_ty static inline bool is_stored_discriminant (Entity_Id discr, Entity_Id record_type) { - if (Is_Tagged_Type (record_type)) + if (Is_Unchecked_Union (record_type)) +return false; + else if (Is_Tagged_Type (record_type)) return No (Corresponding_Discriminant (discr)); else if (Ekind (record_type) == E_Record_Type) return Original_Record_Component (discr) == discr;
[Ada] Override inlining heuristics for expression functions
Expression functions are supposed to be very small so it makes sense to inline them in almost all cases. Tested on x86_64-suse-linux, applied on the mainline. 2017-09-09 Eric Botcazou * gcc-interface/trans.c (Subprogram_Body_to_gnu): Disregard inlining limits for expression functions. (gnat_to_gnu) : Fix formatting. -- Eric BotcazouIndex: gcc-interface/trans.c === --- gcc-interface/trans.c (revision 251927) +++ gcc-interface/trans.c (working copy) @@ -3777,6 +3777,11 @@ Subprogram_Body_to_gnu (Node_Id gnat_nod Sloc_to_locus (Sloc (gnat_node), &locus); DECL_SOURCE_LOCATION (gnu_subprog_decl) = locus; + /* If the body comes from an expression function, arrange it to be inlined + in almost all cases. */ + if (Was_Expression_Function (gnat_node)) +DECL_DISREGARD_INLINE_LIMITS (gnu_subprog_decl) = 1; + /* Initialize the information structure for the function. */ allocate_struct_function (gnu_subprog_decl, false); gnu_subprog_language = ggc_cleared_alloc (); @@ -6140,7 +6145,7 @@ gnat_to_gnu (Node_Id gnat_node) && (((Is_Array_Type (Etype (gnat_temp)) || Is_Record_Type (Etype (gnat_temp))) && !Is_Constrained (Etype (gnat_temp))) - || Is_Concurrent_Type (Etype (gnat_temp + || Is_Concurrent_Type (Etype (gnat_temp break; if (Present (Expression (gnat_node))
[Ada] Take into account alignment gap for component reordering
This disables the recently implemented component reordering in one more case. Tested on x86_64-suse-linux, applied on the mainline. 2017-09-09 Eric Botcazou * gcc-interface/decl.c (components_to_record): Do not reorder in non- packed record types if pragma Optimize_Alignment (Space) is enabled. -- Eric BotcazouIndex: gcc-interface/decl.c === --- gcc-interface/decl.c (revision 251934) +++ gcc-interface/decl.c (working copy) @@ -7683,7 +7683,9 @@ components_to_record (Node_Id gnat_compo of a byte, so that they don't cause the regular fields to be either at self-referential/variable offset or misaligned. Note, in the latter case, that this can only happen in packed record types so the alignment - is effectively capped to the byte for the whole record. + is effectively capped to the byte for the whole record. But we don't + do it for non-packed record types if pragma Optimize_Alignment (Space) + is specified because this can prevent alignment gaps from being filled. Optionally, if the layout warning is enabled, keep track of the above 4 different kinds of fields and issue a warning if some of them would be @@ -7694,6 +7696,8 @@ components_to_record (Node_Id gnat_compo const bool do_reorder = (Convention (gnat_record_type) == Convention_Ada && !No_Reordering (gnat_record_type) + && (!Optimize_Alignment_Space (gnat_record_type) + || Is_Packed (gnat_record_type)) && !debug__debug_flag_dot_r); const bool w_reorder = (Convention (gnat_record_type) == Convention_Ada
[Ada] Fix fallout of component reordering with -fgnat-encodings=minimal
Tested on x86_64-suse-linux, applied on the mainline. 2017-09-09 Pierre-Marie de Rodat * gcc-interface/decl.c (gnat_to_gnu_entity) : Don't generate debug info for inner record types if -fgnat-encodings=minimal (gnat_to_gnu_entity) : Use the ultimate base record type as the debug type. 2017-09-09 Pierre-Marie de Rodat * gnat.dg/debug14.adb: New test. -- Eric BotcazouIndex: gcc-interface/decl.c === --- gcc-interface/decl.c (revision 251936) +++ gcc-interface/decl.c (working copy) @@ -3308,10 +3308,14 @@ gnat_to_gnu_entity (Entity_Id gnat_entit = build_subst_list (gnat_entity, gnat_parent_type, definition); /* Set the layout of the type to match that of the parent type, - doing required substitutions. */ - copy_and_substitute_in_layout (gnat_entity, gnat_parent_type, - gnu_type, gnu_parent_type, - gnu_subst_list, debug_info_p); + doing required substitutions. If we are in minimal GNAT + encodings mode, we don't need debug info for the inner record + types, as they will be part of the embedding variant record's + debug info. */ + copy_and_substitute_in_layout + (gnat_entity, gnat_parent_type, gnu_type, gnu_parent_type, + gnu_subst_list, + debug_info_p && gnat_encodings != DWARF_GNAT_ENCODINGS_MINIMAL); } else { @@ -3439,7 +3443,17 @@ gnat_to_gnu_entity (Entity_Id gnat_entit gnu_type = make_node (RECORD_TYPE); TYPE_NAME (gnu_type) = gnu_entity_name; if (gnat_encodings == DWARF_GNAT_ENCODINGS_MINIMAL) - SET_TYPE_DEBUG_TYPE (gnu_type, gnu_base_type); + { + /* Use the ultimate base record type as the debug type. + Subtypes and derived types bring no useful + information. */ + Entity_Id gnat_debug_type = gnat_entity; + while (Etype (gnat_debug_type) != gnat_debug_type) + gnat_debug_type = Etype (gnat_debug_type); + tree gnu_debug_type + = TYPE_MAIN_VARIANT (gnat_to_gnu_type (gnat_debug_type)); + SET_TYPE_DEBUG_TYPE (gnu_type, gnu_debug_type); + } TYPE_PACKED (gnu_type) = TYPE_PACKED (gnu_base_type); TYPE_REVERSE_STORAGE_ORDER (gnu_type) = Reverse_Storage_Order (gnat_entity); -- { dg-do compile } -- { dg-options "-g -fgnat-encodings=minimal" } procedure Debug14 is type Db_Kind_T is (Raw, Relational, Object); type Db_Model_T (Kind : Db_Kind_T) is record case Kind is when Raw => Fs_Type : Integer; when Relational | Object => Vendor_Id : Integer; case Kind is when Relational => N_Tables : Integer; when others => null; end case; end case; end record; type Raw_Db_T is new Db_Model_T (Kind => Raw); type Raw_Db_P is access Raw_Db_T; Db : Raw_Db_P := new Raw_Db_T; begin null; end;
Re: [PATCH] Improve -Ofast vectorization of std::sin etc. (PR libstdc++/81706)
On Fri, Sep 1, 2017 at 1:12 PM, Jakub Jelinek wrote: > + tree s = lookup_attribute ("omp declare simd", > +DECL_ATTRIBUTES (newdecl)); > + if (s) > + { > + tree b > + = builtin_decl_explicit (DECL_FUNCTION_CODE > (newdecl)); > + if (b) > + duplicate_one_attribute (&DECL_ATTRIBUTES (b), s, > +"omp declare simd"); > + } Is there a reason not to set b first and move the lookup of s into the function as well? Jason
Re: [wwwdocs] Update reference to MMIX
On Sat, Sep 9, 2017 at 2:34 AM, Gerald Pfeifer wrote: > + href="http://www-cs-faculty.stanford.edu/~knuth/mmix.htmll";>MMIX This one has one too many l's.
Re: C/C++ PATCH to add __remove_qualifiers (PR c/65455, c/39985)
On Fri, Sep 1, 2017 at 2:46 PM, Marek Polacek wrote: > @@ -16960,6 +16961,24 @@ cp_parser_simple_type_specifier (cp_parser* parser, > >return type; > > +case RID_REMOVE_QUALS: > + /* Consume the `__remove_qualifiers' token. */ > + cp_lexer_consume_token (parser->lexer); > + /* Parse the operand to __remove_qualifiers`'. */ > + type = cp_parser_sizeof_operand (parser, RID_REMOVE_QUALS); Wouldn't it be simpler to use cp_parser_type_id? > + if (!TYPE_P (type)) > + { > + error_at (token->location, > + "%<__remove_qualifiers%> can only be applied to a type"); > + type = error_mark_node; > + } ...rather than check this. It doesn't look like you handle use of __remove_qualifiers in a template, and you don't have any testcases for it. > +@code{__remove_qualifiers} takes a typename as an argument: I think it would be better to use the term "type-id" here, to avoid confusion with "type-name", which is only a single identifier. Jason
Re: [wwwdocs] Update reference to MMIX
On Sat, 9 Sep 2017, Tim Song wrote: >> + href="http://www-cs-faculty.stanford.edu/~knuth/mmix.htmll";>MMIX > This one has one too many l's. You've got sharp eyes, Tim! Thanks for catching this; fixed thusly. Gerald Index: readings.html === RCS file: /cvs/gcc/wwwdocs/htdocs/readings.html,v retrieving revision 1.280 diff -u -r1.280 readings.html --- readings.html 9 Sep 2017 06:33:54 - 1.280 +++ readings.html 9 Sep 2017 14:00:45 - @@ -209,7 +209,7 @@ http://www-cs-faculty.stanford.edu/~uno/taocp.html";>The Art of Computer Programming (ISBN 0-201-89683-4). The http://www-cs-faculty.stanford.edu/~knuth/mmix.htmll";>MMIX + href="http://www-cs-faculty.stanford.edu/~knuth/mmix.html";>MMIX page has more information about MMIX. Knuth also wrote a http://www-cs-faculty.stanford.edu/~knuth/mmixware.html";>book specifically about MMIX (MMIXware, ISBN 3-540-66938-8).
Re: [PATCH] Early LTO debug TODO
On Thu, Aug 31, 2017 at 4:03 PM, Richard Biener wrote: > On Thu, 31 Aug 2017, Richard Biener wrote: > >> >> As suspected during review the DECL_ABSTRACT_P handling in >> gen_formal_parameter_die is no longer necessary so the following >> patch removes it. >> >> [LTO] bootstrapped on x86_64-unknown-linux-gnu, testing in progress. >> The gdb testsuite shows no regression. >> >> Will apply after testing finished. > > Ok, so it doesn't work - it regresses for example > gcc.dg/debug/dwarf2/inline1.c because we end up annotating the > abstract DIE with a location. This is because in gen_subprogram_die > (with decl set as abstract-self and thus generating a concrete > instance subroutine die) we call > > else if (parm && !POINTER_BOUNDS_P (parm)) > { > dw_die_ref parm_die = gen_decl_die (parm, NULL, NULL, > subr_die); > > thus unconditionally pass NULL as origin where gen_formal_parameter_die > just has > > /* If we have a previously generated DIE, use it, unless this is an > concrete instance (origin != NULL), in which case we need a new > DIE with a corresponding DW_AT_abstract_origin. */ > bool reusing_die; > if (parm_die && origin == NULL) > reusing_die = true; > else > { > parm_die = new_die (DW_TAG_formal_parameter, context_die, node); > reusing_die = false; > } > > but obviously that logic is flawed with us passing origin as NULL... > > What saved us here is the check whether the existing param_die has > the correct parent, if not we didn't re-use it. OTOH for die_parent > == NULL we have the extra check that would have been dead code. > > Any hint as to whether we should pass in anything as origin or > whether we should keep the existing die_parent logic somehow? > (do we ever get a NULL context_die passed?) The problem is that we aren't checking decl_ultimate_origin soon enough, and thereby setting origin. Tested x86_64-pc-linux-gnu, applying to trunk: commit b7702a2cbca9c50e94053cc125ff93438d377126 Author: Jason Merrill Date: Sat Sep 9 11:33:14 2017 +0200 * dwarf2out.c (gen_formal_parameter_die): Remove obsolete hunk. Check ultimate_origin before setting reusing_die. diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c index 651dd0c..cc93db3 100644 --- a/gcc/dwarf2out.c +++ b/gcc/dwarf2out.c @@ -21285,30 +21285,15 @@ gen_formal_parameter_die (tree node, tree origin, bool emit_name_p, dw_die_ref context_die) { tree node_or_origin = node ? node : origin; - tree ultimate_origin; dw_die_ref parm_die = NULL; - if (TREE_CODE_CLASS (TREE_CODE (node_or_origin)) == tcc_declaration) + if (DECL_P (node_or_origin)) { parm_die = lookup_decl_die (node); - /* If the contexts differ, we may not be talking about the same -thing. -??? When in LTO the DIE parent is the "abstract" copy and the -context_die is the specification "copy". But this whole block -should eventually be no longer needed. */ - if (parm_die && parm_die->die_parent != context_die && !in_lto_p) - { - if (!DECL_ABSTRACT_P (node)) - { - /* This can happen when creating an inlined instance, in -which case we need to create a new DIE that will get -annotated with DW_AT_abstract_origin. */ - parm_die = NULL; - } - else - gcc_unreachable (); - } + tree ultimate_origin = decl_ultimate_origin (node_or_origin); + if (node || ultimate_origin) + origin = ultimate_origin; if (parm_die && parm_die->die_parent == NULL) { @@ -21343,10 +21328,6 @@ gen_formal_parameter_die (tree node, tree origin, bool emit_name_p, switch (TREE_CODE_CLASS (TREE_CODE (node_or_origin))) { case tcc_declaration: - ultimate_origin = decl_ultimate_origin (node_or_origin); - if (node || ultimate_origin) - origin = ultimate_origin; - if (reusing_die) goto add_location;
Re: Announcing ARM and AArch64 port maintainers.
On Sat, Sep 09, 2017 at 12:44:14PM +0100, Ramana Radhakrishnan wrote: > I'm pleased to announce that the steering committee has appointed > > - James Greenhalgh as a full maintainer for the AArch64 port > > and > > - Kyrylo Tkachov as a full maintainer for the ARM port. > > James & Kyrylo, if you could update your entries in the MAINTAINERS > file to reflect these roles, it would be appreciated. Thanks! I'm looking forward to continuing my contributions to the AArch64 port under my new role as port maintainer. This is what I committed as revision 251940, it moves my name to the AArch64 maintainers and puts the other names there back in alphabetical order, as elsewhere in the file. Thanks, James --- 2017-09-09 James Greenhalgh * MAINTAINERS (James Greenhalgh): Move to AArch64 maintainers. diff --git a/MAINTAINERS b/MAINTAINERS index 7effec1..2ed1ef9 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -39,8 +39,9 @@ their own patches from other maintainers or reviewers. CPU Port Maintainers (CPU alphabetical order) -aarch64 port Marcus Shawcroft aarch64 port Richard Earnshaw +aarch64 port James Greenhalgh +aarch64 port Marcus Shawcroft alpha port Richard Henderson arc port Joern Rennecke arm port Nick Clifton @@ -249,7 +250,6 @@ check in changes outside of the parts of the compiler they maintain. Reviewers -aarch64 port James Greenhalgh arc port Andrew Burgess arc port Claudiu Zissulescu arm port Kyrylo Tkachov
Re: [PATCH, DWARF] Add DW_CFA_AARCH64_negate_ra_state to dwarf2.def/h and dwarfnames.c
On Thu, Aug 10, 2017 at 6:39 PM, Jiong Wang wrote: > Hi, > > A new vendor CFA DW_CFA_AARCH64_negate_ra_state was introduced for > ARMv8.3-A > return address signing, it is multiplexing DW_CFA_GNU_window_save in CFA > vendor > extension space. > > This patch adds necessary code to make it available to external, the GDB > patch (https://sourceware.org/ml/gdb-patches/2017-08/msg00215.html) is > intended > to use it. > > A new DW_CFA_DUP for it is added in dwarf2.def. The use of DW_CFA_DUP is > to > avoid duplicated case value issue when included in libiberty/dwarfnames. > > Native x86 builds OK to make sure no macro expanding errors. > > OK for trunk? OK. Jason
Re: [PATCH] streambuf_iterator: avoid debug-dependent behaviour
Hi Completing the execution of tests revealed a lot about the current implementation. The main point of current implementation is to delay as much as possible the capture of the current streambuf position. So my original proposal capturing state on instantiation was wrong. This new proposal concentrate on the debug-dependent code. Debug assertions now avoids calling _M_at_eof() which also capture iterator state. It also simplifies _M_get() method a little bit like Petr proposed but keeping the _M_sbuf reset when reaching eof. Thanks to this work I realized that std::find specialization could also be simplified by returning a streambuf_iterator which will capture current streambuf state on evaluation. Note that I haven't been able to create a test case revealing the problem. This is more a code quality issue as current code violates the principal that debug asserts shouldn't impact object state. AFAIK this is noticeable only under gdb. Regarding avoiding the reset of _M_sbuf it might be possible,your test case could be a good reason to do so. But this is going to be a big change for current implementation so don't forget to run all testsuite and to consider the std::copy and std::find specializations. Tested under Linux x86_64, normal and debug modes. Ok to commit ? François On 08/09/2017 07:47, Petr Ovtchenkov wrote: -gcc-patches On Thu, 7 Sep 2017 23:02:15 +0200 François Dumont wrote: + _M_c = _M_sbuf->sgetc(); + if (_S_at_eof(_M_c)) + _M_sbuf = 0; _M_sbuf = 0; <--- Is not what I axpect here. Suggestions will be later, after we finish copyright assignment for changes procedure (in progress). WBR, -- - ptr diff --git a/libstdc++-v3/include/bits/streambuf_iterator.h b/libstdc++-v3/include/bits/streambuf_iterator.h index f0451b1..ad9c89f 100644 --- a/libstdc++-v3/include/bits/streambuf_iterator.h +++ b/libstdc++-v3/include/bits/streambuf_iterator.h @@ -103,7 +103,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION : _M_sbuf(0), _M_c(traits_type::eof()) { } #if __cplusplus >= 201103L - istreambuf_iterator(const istreambuf_iterator&) noexcept = default; + istreambuf_iterator(const istreambuf_iterator&) = default; ~istreambuf_iterator() = default; #endif @@ -122,28 +122,29 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION char_type operator*() const { + int_type __c = _M_get(); + #ifdef _GLIBCXX_DEBUG_PEDANTIC // Dereferencing a past-the-end istreambuf_iterator is a // libstdc++ extension - __glibcxx_requires_cond(!_M_at_eof(), + __glibcxx_requires_cond(!_S_at_eof(__c), _M_message(__gnu_debug::__msg_deref_istreambuf) ._M_iterator(*this)); #endif - return traits_type::to_char_type(_M_get()); + return traits_type::to_char_type(__c); } /// Advance the iterator. Calls streambuf.sbumpc(). istreambuf_iterator& operator++() { - __glibcxx_requires_cond(!_M_at_eof(), + __glibcxx_requires_cond(_M_sbuf + && (!_S_at_eof(_M_c) || !_S_at_eof(_M_sbuf->sgetc())), _M_message(__gnu_debug::__msg_inc_istreambuf) ._M_iterator(*this)); - if (_M_sbuf) - { + _M_sbuf->sbumpc(); _M_c = traits_type::eof(); - } return *this; } @@ -151,16 +152,14 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION istreambuf_iterator operator++(int) { - __glibcxx_requires_cond(!_M_at_eof(), + __glibcxx_requires_cond(_M_sbuf + && (!_S_at_eof(_M_c) || !_S_at_eof(_M_sbuf->sgetc())), _M_message(__gnu_debug::__msg_inc_istreambuf) ._M_iterator(*this)); istreambuf_iterator __old = *this; - if (_M_sbuf) - { __old._M_c = _M_sbuf->sbumpc(); _M_c = traits_type::eof(); - } return __old; } @@ -176,26 +175,20 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION int_type _M_get() const { - const int_type __eof = traits_type::eof(); - int_type __ret = __eof; - if (_M_sbuf) - { - if (!traits_type::eq_int_type(_M_c, __eof)) - __ret = _M_c; - else if (!traits_type::eq_int_type((__ret = _M_sbuf->sgetc()), - __eof)) - _M_c = __ret; - else + if (_M_sbuf && _S_at_eof(_M_c) && _S_at_eof(_M_c = _M_sbuf->sgetc())) _M_sbuf = 0; - } - return __ret; + return _M_c; } bool _M_at_eof() const + { return _S_at_eof(_M_get()); } + + static bool + _S_at_eof(int_type __c) { const int_type __eof = traits_type::eof(); - return traits_type::eq_int_type(_M_get(), __eof); + return traits_type::eq_int_type(__c, __eof); } }; @@ -373,13 +366,14 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION typedef typename __is_iterator_type::traits_type traits_type; typedef typename __is_iterator_type::streambuf_type streambuf_type; typedef typename traits_type::int_type int_type; + const int_type __eof = traits_type::eof(); if (__first._M_sbuf && !__last._M_sbuf) { const int_type __ival = traits_type::to_int_type(__val); str
Re: [PATCH] Early LTO debug TODO
On Sat, Sep 9, 2017 at 4:00 PM, Jason Merrill wrote: > On Thu, Aug 31, 2017 at 4:03 PM, Richard Biener wrote: >> On Thu, 31 Aug 2017, Richard Biener wrote: >> >>> >>> As suspected during review the DECL_ABSTRACT_P handling in >>> gen_formal_parameter_die is no longer necessary so the following >>> patch removes it. >>> >>> [LTO] bootstrapped on x86_64-unknown-linux-gnu, testing in progress. >>> The gdb testsuite shows no regression. >>> >>> Will apply after testing finished. >> >> Ok, so it doesn't work - it regresses for example >> gcc.dg/debug/dwarf2/inline1.c because we end up annotating the >> abstract DIE with a location. This is because in gen_subprogram_die >> (with decl set as abstract-self and thus generating a concrete >> instance subroutine die) we call >> >> else if (parm && !POINTER_BOUNDS_P (parm)) >> { >> dw_die_ref parm_die = gen_decl_die (parm, NULL, NULL, >> subr_die); >> >> thus unconditionally pass NULL as origin where gen_formal_parameter_die >> just has >> >> /* If we have a previously generated DIE, use it, unless this is an >> concrete instance (origin != NULL), in which case we need a new >> DIE with a corresponding DW_AT_abstract_origin. */ >> bool reusing_die; >> if (parm_die && origin == NULL) >> reusing_die = true; >> else >> { >> parm_die = new_die (DW_TAG_formal_parameter, context_die, node); >> reusing_die = false; >> } >> >> but obviously that logic is flawed with us passing origin as NULL... >> >> What saved us here is the check whether the existing param_die has >> the correct parent, if not we didn't re-use it. OTOH for die_parent >> == NULL we have the extra check that would have been dead code. >> >> Any hint as to whether we should pass in anything as origin or >> whether we should keep the existing die_parent logic somehow? >> (do we ever get a NULL context_die passed?) > > The problem is that we aren't checking decl_ultimate_origin soon > enough, and thereby setting origin. > > Tested x86_64-pc-linux-gnu, applying to trunk: ...and reverting, as it caused a bunch of GDB regressions. > FAIL: gdb.cp/anon-struct.exp: print type of t3::~t3 > FAIL: gdb.cp/anon-struct.exp: print type of t::t > FAIL: gdb.cp/anon-struct.exp: print type of X::t2::t2 > FAIL: gdb.cp/cpexprs.exp: print base1::base1(int) > FAIL: gdb.cp/cpexprs.exp: print base1::base1(void) > FAIL: gdb.cp/cpexprs.exp: print base2::base2 > FAIL: gdb.cp/cpexprs.exp: print base::~base > FAIL: gdb.cp/cpexprs.exp: print base::base(int) > FAIL: gdb.cp/cpexprs.exp: print base::base(void) > FAIL: gdb.cp/cpexprs.exp: print derived::derived > FAIL: gdb.cp/cpexprs.exp: print policy1::policy > FAIL: gdb.cp/cpexprs.exp: print policy2::policy > FAIL: gdb.cp/cpexprs.exp: print policy3::policy > FAIL: gdb.cp/cpexprs.exp: print policy4::policy > FAIL: gdb.cp/cpexprs.exp: print policyd1::policyd > FAIL: gdb.cp/cpexprs.exp: print policyd1::~policyd > FAIL: gdb.cp/cpexprs.exp: print policyd2::policyd > FAIL: gdb.cp/cpexprs.exp: print policyd2::~policyd > FAIL: gdb.cp/cpexprs.exp: print policyd3::policyd > FAIL: gdb.cp/cpexprs.exp: print policyd3::~policyd > FAIL: gdb.cp/cpexprs.exp: print policyd4::policyd > FAIL: gdb.cp/cpexprs.exp: print policyd4::~policyd > FAIL: gdb.cp/cpexprs.exp: print policyd5::policyd > FAIL: gdb.cp/cpexprs.exp: print policyd5::~policyd > FAIL: gdb.cp/cpexprs.exp: print policyd >::policyd > FAIL: gdb.cp/cpexprs.exp: print policyd >::~policyd > FAIL: gdb.cp/cpexprs.exp: print policyd >::policyd > FAIL: gdb.cp/cpexprs.exp: print policyd >::~policyd > FAIL: gdb.cp/cpexprs.exp: print policyd >::policyd > FAIL: gdb.cp/cpexprs.exp: print policyd >::~policyd > FAIL: gdb.cp/cpexprs.exp: print policyd >::policyd > FAIL: gdb.cp/cpexprs.exp: print policyd >::~policyd > FAIL: gdb.cp/cpexprs.exp: print policyd, operation_1 > > >::policyd > FAIL: gdb.cp/cpexprs.exp: print policyd, operation_1 > > >::~policyd Jason