Re: [RFC PATCH, i386]: Remove peephole2s for (subreg (operator (...)(...))) RTXes
On Sun, Oct 28, 2012 at 2:37 AM, H.J. Lu wrote: >>> As suggested by Richard S. [1], after the patch that converts subreg:M >>> (op:N (...)(...)) to op:M (subreg:M (...) subreg:M (...)), we can >>> remove several peephole2 patterns that handle subregs of PLUS, MINUS >>> and MULT operators. I have attached RFC prototype patch that will >>> trigger an ICE when to-be-removed pattern triggers, with the intention >>> that these patterns wil be removed entirely (An "invalid" pattern was >>> indeed generated elsewhere, see patch). I have committed following version that avoids all failures, reported by H.J.: 2012-10-29 Uros Bizjak * config/i386/i386.c (ix86_decompose_address): Use simplify_gen_subreg to generate SImode equivalent of address, zero-extended with AND RTX. * config/i386/i386.md (ashift to lea splitter): Split to SImode mult. (simple lea to add/shift peephole2s): Remove peephole2s that operate on subregs of DImode operations. Re-tested on x86_64-pc-linux-gnu {,-m32} and committed to mainline SVN. Uros. Index: config/i386/i386.c === --- config/i386/i386.c (revision 192906) +++ config/i386/i386.c (working copy) @@ -11821,7 +11821,11 @@ ix86_decompose_address (rtx addr, struct ix86_addr return 0; } else if (GET_MODE (addr) == DImode) - addr = gen_rtx_SUBREG (SImode, addr, 0); + { + addr = simplify_gen_subreg (SImode, addr, DImode, 0); + if (addr == NULL_RTX) + return 0; + } else if (GET_MODE (addr) != VOIDmode) return 0; } Index: config/i386/i386.md === --- config/i386/i386.md (revision 192906) +++ config/i386/i386.md (working copy) @@ -9600,10 +9600,10 @@ "TARGET_64BIT && reload_completed && true_regnum (operands[0]) != true_regnum (operands[1])" [(set (match_dup 0) - (zero_extend:DI (subreg:SI (mult:DI (match_dup 1) (match_dup 2)) 0)))] + (zero_extend:DI (mult:SI (match_dup 1) (match_dup 2] { - operands[1] = gen_lowpart (DImode, operands[1]); - operands[2] = gen_int_mode (1 << INTVAL (operands[2]), DImode); + operands[1] = gen_lowpart (SImode, operands[1]); + operands[2] = gen_int_mode (1 << INTVAL (operands[2]), SImode); }) ;; This pattern can't accept a variable shift count, since shifts by @@ -17358,28 +17358,6 @@ (clobber (reg:CC FLAGS_REG))])]) (define_peephole2 - [(set (match_operand:SI 0 "register_operand") - (subreg:SI (plus:DI (match_operand:DI 1 "register_operand") - (match_operand:DI 2 "nonmemory_operand")) 0))] - "TARGET_64BIT && !TARGET_OPT_AGU - && REGNO (operands[0]) == REGNO (operands[1]) - && peep2_regno_dead_p (0, FLAGS_REG)" - [(parallel [(set (match_dup 0) (plus:SI (match_dup 0) (match_dup 2))) - (clobber (reg:CC FLAGS_REG))])] - "operands[2] = gen_lowpart (SImode, operands[2]);") - -(define_peephole2 - [(set (match_operand:SI 0 "register_operand") - (subreg:SI (plus:DI (match_operand:DI 1 "nonmemory_operand") - (match_operand:DI 2 "register_operand")) 0))] - "TARGET_64BIT && !TARGET_OPT_AGU - && REGNO (operands[0]) == REGNO (operands[2]) - && peep2_regno_dead_p (0, FLAGS_REG)" - [(parallel [(set (match_dup 0) (plus:SI (match_dup 0) (match_dup 1))) - (clobber (reg:CC FLAGS_REG))])] - "operands[1] = gen_lowpart (SImode, operands[1]);") - -(define_peephole2 [(set (match_operand:DI 0 "register_operand") (zero_extend:DI (plus:SI (match_operand:SI 1 "register_operand") @@ -17404,36 +17382,6 @@ (clobber (reg:CC FLAGS_REG))])]) (define_peephole2 - [(set (match_operand:DI 0 "register_operand") - (zero_extend:DI - (subreg:SI (plus:DI (match_dup 0) - (match_operand:DI 1 "nonmemory_operand")) 0)))] - "TARGET_64BIT && !TARGET_OPT_AGU - && peep2_regno_dead_p (0, FLAGS_REG)" - [(parallel [(set (match_dup 0) - (zero_extend:DI (plus:SI (match_dup 2) (match_dup 1 - (clobber (reg:CC FLAGS_REG))])] -{ - operands[1] = gen_lowpart (SImode, operands[1]); - operands[2] = gen_lowpart (SImode, operands[0]); -}) - -(define_peephole2 - [(set (match_operand:DI 0 "register_operand") - (zero_extend:DI - (subreg:SI (plus:DI (match_operand:DI 1 "nonmemory_operand") - (match_dup 0)) 0)))] - "TARGET_64BIT && !TARGET_OPT_AGU - && peep2_regno_dead_p (0, FLAGS_REG)" - [(parallel [(set (match_dup 0) - (zero_extend:DI (plus:SI (match_dup 2) (match_dup 1 - (clobber (reg:CC FLAGS_REG))])] -{ - operands[1] = gen_lowpart (SImode, operands[1]); - operands[2] = gen_lowpart (SImode, operands[0]); -}) - -(define_peephole2 [(set (match_operand:SWI48 0 "register_opera
[PATCH/AARCH64] Fix build after __builtin_thread_pointer changes
Hi, Right now aarch64-*-* is broken because the C front-end also defines a __builtin_thread_pointer which causes an internal error. We don't need to do the __builtin_thread_pointer support in the back-end. This patch moves aarch64 to using the common code instead. OK? Built for aarch64-thunder-elf. Thanks, Andrew Pinski ChangeLog: * config/aarch64/aarch64-protos.h (aarch64_load_tp): New proto. * config/aarch64/aarch64.c (aarch64_load_tp): Export. (aarch64_init_builtins): Don't add __builtin_thread_pointer builtin. * config/aarch64/aarch64.h (aarch64_builtins): Delete AARCH64_BUILTIN_THREAD_POINTER. * config/aarch64/aarch64.md (get_thread_pointerdi): New pattern. diff --git a/gcc/config/aarch64/aarch64-protos.h b/gcc/config/aarch64/aarch64-protos.h index 765d192..ca4e306 100644 --- a/gcc/config/aarch64/aarch64-protos.h +++ b/gcc/config/aarch64/aarch64-protos.h @@ -254,6 +254,7 @@ bool aarch64_split_128bit_move_p (rtx, rtx); bool aarch64_legitimate_address_p (enum machine_mode, rtx, RTX_CODE, bool); enum machine_mode aarch64_select_cc_mode (RTX_CODE, rtx, rtx); rtx aarch64_gen_compare_reg (RTX_CODE, rtx, rtx); +rtx aarch64_load_tp (rtx); #endif /* RTX_CODE */ diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c index 1bc0c8a..aaebdf6 100644 --- a/gcc/config/aarch64/aarch64.c +++ b/gcc/config/aarch64/aarch64.c @@ -102,7 +102,6 @@ static bool aarch64_vfp_is_call_or_return_candidate (enum machine_mode, bool *); static void aarch64_elf_asm_constructor (rtx, int) ATTRIBUTE_UNUSED; static void aarch64_elf_asm_destructor (rtx, int) ATTRIBUTE_UNUSED; -static rtx aarch64_load_tp (rtx); static void aarch64_override_options_after_change (void); static int aarch64_simd_valid_immediate (rtx, enum machine_mode, int, rtx *, int *, unsigned char *, int *, int *); @@ -4988,20 +4987,11 @@ aarch64_legitimate_constant_p (enum machine_mode mode, rtx x) static void aarch64_init_builtins (void) { - tree ftype, decl = NULL; - - ftype = build_function_type (ptr_type_node, void_list_node); - decl = add_builtin_function ("__builtin_thread_pointer", ftype, - AARCH64_BUILTIN_THREAD_POINTER, BUILT_IN_MD, - NULL, NULL_TREE); - TREE_NOTHROW (decl) = 1; - TREE_READONLY (decl) = 1; - if (TARGET_SIMD) init_aarch64_simd_builtins (); } -static rtx +rtx aarch64_load_tp (rtx target) { if (!target @@ -5026,9 +5016,6 @@ aarch64_expand_builtin (tree exp, tree fndecl = TREE_OPERAND (CALL_EXPR_FN (exp), 0); int fcode = DECL_FUNCTION_CODE (fndecl); - if (fcode == AARCH64_BUILTIN_THREAD_POINTER) -return aarch64_load_tp (target); - if (fcode >= AARCH64_SIMD_BUILTIN_BASE) return aarch64_simd_expand_builtin (fcode, exp, target); diff --git a/gcc/config/aarch64/aarch64.h b/gcc/config/aarch64/aarch64.h index 3b8b033..122a7a5 100644 --- a/gcc/config/aarch64/aarch64.h +++ b/gcc/config/aarch64/aarch64.h @@ -794,7 +794,6 @@ do { \ enum aarch64_builtins { AARCH64_BUILTIN_MIN, - AARCH64_BUILTIN_THREAD_POINTER, AARCH64_SIMD_BUILTIN_BASE }; diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md index b803922..804d7e7 100644 --- a/gcc/config/aarch64/aarch64.md +++ b/gcc/config/aarch64/aarch64.md @@ -2923,6 +2923,17 @@ [(set_attr "length" "0")] ) +;; Named pattern for expanding thread pointer reference. +(define_expand "get_thread_pointerdi" + [(match_operand:DI 0 "register_operand" "=r")] + "" +{ + rtx tmp = aarch64_load_tp (operands[0]); + if (tmp != operands[0]) +emit_move_insn (operands[0], tmp); + DONE; +}) + ;; AdvSIMD Stuff (include "aarch64-simd.md")
Re: Ping: [PATCH] Install error handler for out-of-memory when using STL containers
Hi Tobi, It looks straightforward to me but I am not convinced that a fortran maintainer should be giving the green light on this :-) Cheers Paul On 28 October 2012 16:28, Tobias Schlüter wrote: > > Ping. This issue stands in the way of a very simple solution of PR > fortran/51727. I've re-attached the patch for your convenience. > > On 15 Oct 2012 at 22:51:05 +0200 Tobias Schlüter wrote: >> >> The attached patch adds out-of-memory diagnostics for code using STL >> containers by using set_new_handler. Since the intended allocation >> size is not available to a new_handler, I had to forego a more >> detailed error message such as the one from xmalloc_failed(). >> fatal_error() and abort() don't give a meaningful location when the >> new_handler is called, so I chose to put together the error message >> manually as is done in xmalloc_failed(). I would have found it more >> appealing to have operator new call xmalloc() unless a custom >> allocator is given, but I don't think there's a standard way of doing >> this. >> >> Built and tested on the C and Fortran testsuites. Ok for trunk? > > > Best regards, > - Tobi > > 2012-10-15 Tobias Schlüter > > * toplev.c: Add '#include '. > (cxx_out_of_memory): New function. > (general_init): Install cxx_out_of_memory as handler for > out-of-memory condition. -- The knack of flying is learning how to throw yourself at the ground and miss. --Hitchhikers Guide to the Galaxy
Re: [RFC PATCH, i386]: Remove peephole2s for (subreg (operator (...)(...))) RTXes
On Mon, Oct 29, 2012 at 9:07 AM, Uros Bizjak wrote: As suggested by Richard S. [1], after the patch that converts subreg:M (op:N (...)(...)) to op:M (subreg:M (...) subreg:M (...)), we can remove several peephole2 patterns that handle subregs of PLUS, MINUS and MULT operators. I have attached RFC prototype patch that will trigger an ICE when to-be-removed pattern triggers, with the intention that these patterns wil be removed entirely (An "invalid" pattern was indeed generated elsewhere, see patch). > > I have committed following version that avoids all failures, reported by H.J.: > > 2012-10-29 Uros Bizjak > > * config/i386/i386.c (ix86_decompose_address): Use simplify_gen_subreg > to generate SImode equivalent of address, zero-extended with AND RTX. > * config/i386/i386.md (ashift to lea splitter): Split to SImode mult. > (simple lea to add/shift peephole2s): Remove peephole2s that operate > on subregs of DImode operations. > > Re-tested on x86_64-pc-linux-gnu {,-m32} and committed to mainline SVN. Oh, we don't have to use simplify_gen_subreg, gen_subreg is enough. Patch committed as obvious to mainline SVN. Uros. Index: config/i386/i386.c === --- config/i386/i386.c (revision 192908) +++ config/i386/i386.c (working copy) @@ -11822,7 +11822,7 @@ ix86_decompose_address (rtx addr, struct ix86_addr } else if (GET_MODE (addr) == DImode) { - addr = simplify_gen_subreg (SImode, addr, DImode, 0); + addr = simplify_subreg (SImode, addr, DImode, 0); if (addr == NULL_RTX) return 0; } > Uros.
Re: [PATCH/AARCH64] Fix build after __builtin_thread_pointer changes
On 29 Oct 2012, at 09:26, "Andrew Pinski" wrote: > Hi, > Right now aarch64-*-* is broken because the C front-end also defines > a __builtin_thread_pointer which causes an internal error. We don't > need to do the __builtin_thread_pointer support in the back-end. This > patch moves aarch64 to using the common code instead. > > OK? Built for aarch64-thunder-elf. > > Thanks, > Andrew Pinski > > ChangeLog: > >* config/aarch64/aarch64-protos.h (aarch64_load_tp): New proto. >* config/aarch64/aarch64.c (aarch64_load_tp): Export. >(aarch64_init_builtins): Don't add __builtin_thread_pointer builtin. >* config/aarch64/aarch64.h (aarch64_builtins): Delete > AARCH64_BUILTIN_THREAD_POINTER. >* config/aarch64/aarch64.md (get_thread_pointerdi): New pattern. > Ok. R.
[Ada] Special case for case expression alternative moved in caller
Is_Signed_Integer_Arithmetic_Op is called to recognize an arithmetic operation on signed integers, or on if expressions and case expressions whose dependent expressions are arithmetic operation on signed integers. A special case was included for N_Case_Expression_Alternative which should have been included in the caller Apply_Arithmetic_Overflow_Minimized_Eliminated. Now done. Tested on x86_64-pc-linux-gnu, committed on trunk 2012-10-29 Yannick Moy * checks.adb (Apply_Arithmetic_Overflow_Minimized_Eliminated): Add special case for case expression alternative. (Is_Signed_Integer_Arithmetic_Op): Remove special case for case expression alternative. * exp_ch4.adb Minor reformatting. Index: checks.adb === --- checks.adb (revision 192908) +++ checks.adb (working copy) @@ -1108,8 +1108,12 @@ or else Nkind (P) in N_Membership_Test or else Nkind (P) in N_Op_Compare --- We may also be a range operand in a membership test +-- This is also true for an alternative in a case expression +or else Nkind (P) = N_Case_Expression_Alternative + +-- This is also true for a range operand in a membership test + or else (Nkind (P) = N_Range and then Nkind (Parent (P)) in N_Membership_Test) then @@ -6268,9 +6272,6 @@ when N_If_Expression | N_Case_Expression => return Is_Signed_Integer_Type (Etype (N)); - when N_Case_Expression_Alternative => -return Is_Signed_Integer_Type (Etype (Parent (N))); - when others => return False; end case; Index: exp_ch4.adb === --- exp_ch4.adb (revision 192908) +++ exp_ch4.adb (working copy) @@ -3877,8 +3877,8 @@ end if; -- Right operand is a subtype name and the subtype has a predicate. We - -- have to make sure predicate is checked, and for that we need to use - -- the standard N_In circuitry with appropriate types. + -- have to make sure the predicate is checked, and for that we need to + -- use the standard N_In circuitry with appropriate types. else pragma Assert (Present (Predicate_Function (Etype (Rop; @@ -3921,7 +3921,7 @@ -- Bnn -- end --- A bit gruesome, but here goes. +-- A bit gruesome, but there doesn't seem to be a simpler way declare Blk : constant Node_Id := Make_Bignum_Block (Loc); @@ -3937,10 +3937,8 @@ Nin := Make_In (Loc, - Left_Opnd => - Convert_To (Base_Type (Etype (Rop)), - New_Occurrence_Of (Lnn, Loc)), - Right_Opnd => New_Occurrence_Of (Etype (Rop), Loc)); + Left_Opnd => Convert_To (TB, New_Occurrence_Of (Lnn, Loc)), + Right_Opnd => New_Occurrence_Of (T, Loc)); Set_No_Minimize_Eliminate (Nin); -- Now decorate the block
[Ada] Warn for failure to parenthesize unary minus
This patch gives a warning when unary minus is used in a context that requires parentheses, and no parentheses are present, as shown by the following test: 1. function Test_Mod_B (N: in Integer) return Integer is 2. begin 3.return N mod -10; | >>> parentheses required for unary minus 4. end Test_Mod_B; Tested on x86_64-pc-linux-gnu, committed on trunk 2012-10-29 Robert Dewar * par-ch4.adb (P_Primary): Warn on bad use of unary minus. Index: par-ch4.adb === --- par-ch4.adb (revision 192908) +++ par-ch4.adb (working copy) @@ -2364,6 +2364,7 @@ begin -- The loop runs more than once only if misplaced pragmas are found + -- or if a misplaced unary minus is skipped. loop case Token is @@ -2537,8 +2538,15 @@ return P_Identifier; end if; +-- Minus may well be an improper attempt at a unary minus. Give +-- a message, skip the minus and keep going! + +when Tok_Minus => + Error_Msg_SC ("parentheses required for unary minus"); + Scan; -- past minus + -- Anything else is illegal as the first token of a primary, but --- we test for a reserved identifier so that it is treated nicely +-- we test for some common errors, to improve error messages. when others => if Is_Reserved_Identifier then
[Ada] Keep checks in Alfa mode for formal verification
Checks were suppressed to avoid undesired expansion in Alfa mode. It turns out these checks are needed during semantic analysis for the frontend to properly mark nodes as needing or not a run-time check, and the special expansion done in Alfa mode prevents the undesired expansion of checks. So reenable checks in Alfa mode. Tested on x86_64-pc-linux-gnu, committed on trunk 2012-10-29 Yannick Moy * gnat1drv.adb (Adjust_Global_Switches): Do not suppress checks in Alfa mode. Index: gnat1drv.adb === --- gnat1drv.adb(revision 192908) +++ gnat1drv.adb(working copy) @@ -419,7 +419,6 @@ -- Set switches for formal verification mode if Debug_Flag_Dot_FF then - Alfa_Mode := True; -- Set strict standard interpretation of compiler permissions @@ -448,16 +447,14 @@ Restrict.Restrictions.Set (No_Initialize_Scalars) := True; - -- Suppress all language checks since they are handled implicitly by - --the formal verification backend. - -- Turn off dynamic elaboration checks. - -- Turn off alignment checks. - -- Turn off validity checking. + -- Note: at this point we used to suppress various checks, but that + -- is not what we want. We need the semantic processing for these + -- checks (which will set flags like Do_Overflow_Check, showing the + -- points at which potential checks are required semantically). We + -- don't want the expansion associated with these checks, but that + -- happens anyway because this expansion is simply not done in the + -- Alfa version of the expander. - Suppress_Options := Suppress_All; - Dynamic_Elaboration_Checks := False; - Reset_Validity_Check_Options; - -- Kill debug of generated code, since it messes up sloc values Debug_Generated_Code := False;
[Ada] Warn for suspicious same range nested loops
If nested loops appear where the iteration ranges are over the same range of a multi-dimensional range (where the range number has been defaulted to 1), then a warning is issued as shown in the following example: 1. procedure Warn2D is 2.type E is array (Integer range <>, Integer range <>) 3. of Integer; 4.S : Integer := 0; 5.EE : E (1 .. 3, 1 .. 3) := (Others => (Others => 0)); 6. begin 7.for J in EE'Range loop 8. for K in EE'Range loop | >>> warning: inner range same as outer range at line 7 9. S := S + EE (J, K); 10. end loop; 11.end loop; 12. end Warn2D; Tested on x86_64-pc-linux-gnu, committed on trunk 2012-10-29 Robert Dewar * sem_ch5.adb (Analyze_Loop_Statement): Add warning for identical inner/outer ranges. Index: sem_ch5.adb === --- sem_ch5.adb (revision 192908) +++ sem_ch5.adb (working copy) @@ -2626,6 +2626,56 @@ Push_Scope (Ent); Analyze_Iteration_Scheme (Iter); + -- Check for following case which merits a warning if the type E of is + -- a multi-dimensional array (and no explicit subscript ranges present). + + -- for J in E'Range + -- for K in E'Range + + if Present (Iter) +and then Present (Loop_Parameter_Specification (Iter)) + then + declare +LPS : constant Node_Id := Loop_Parameter_Specification (Iter); +DSD : constant Node_Id := +Original_Node (Discrete_Subtype_Definition (LPS)); + begin +if Nkind (DSD) = N_Attribute_Reference + and then Attribute_Name (DSD) = Name_Range + and then No (Expressions (DSD)) +then + declare + Typ : constant Entity_Id := Etype (Prefix (DSD)); + begin + if Is_Array_Type (Typ) +and then Number_Dimensions (Typ) > 1 +and then Nkind (Parent (N)) = N_Loop_Statement +and then Present (Iteration_Scheme (Parent (N))) + then + declare +OIter : constant Node_Id := + Iteration_Scheme (Parent (N)); +OLPS : constant Node_Id := + Loop_Parameter_Specification (OIter); +ODSD : constant Node_Id := + Original_Node (Discrete_Subtype_Definition (OLPS)); + begin +if Nkind (ODSD) = N_Attribute_Reference + and then Attribute_Name (ODSD) = Name_Range + and then No (Expressions (ODSD)) + and then Etype (Prefix (ODSD)) = Typ +then + Error_Msg_Sloc := Sloc (ODSD); + Error_Msg_N + ("inner range same as outer range#?", DSD); +end if; + end; + end if; + end; +end if; + end; + end if; + -- Analyze the statements of the body except in the case of an Ada 2012 -- iterator with the expander active. In this case the expander will do -- a rewrite of the loop into a while loop. We will then analyze the
[Ada] Source location of generated nodes for To_Any calls
This change modifies the source location assigned to expander generated nodes produced in the context of the distributed systems annex. Previously, we always assigned code generated for the conversion of an expression to the intermediate Any representation the location of the expression. However when such a call is generated as part of the generation of calling stubs for an RACW, this may lead to spurious ABE warnings if the RACW is declared earlier than the point where the expression occurs (case e.g. of a default value of a discriminant for a discriminated type used as formal parameter type in an RACW primitive operation, when the discriminated type declaration occurs after the RACW declaration). Tested on x86_64-pc-linux-gnu, committed on trunk 2012-10-29 Thomas Quinot * exp_attr.adb, exp_dist.adb, exp_dist.ads (Build_To_Any_Call): Pass an explicit Loc parameter to set the source location of generated nodes. Index: exp_attr.adb === --- exp_attr.adb(revision 192908) +++ exp_attr.adb(working copy) @@ -5141,7 +5141,8 @@ begin Rewrite (N, Build_To_Any_Call - (Convert_To (P_Type, + (Loc, + Convert_To (P_Type, Relocate_Node (First (Exprs))), Decls)); Insert_Actions (N, Decls); Analyze_And_Resolve (N, RTE (RE_Any)); Index: exp_dist.adb === --- exp_dist.adb(revision 192908) +++ exp_dist.adb(working copy) @@ -6,7 +6,7 @@ -- -- -- B o d y -- -- -- --- Copyright (C) 1992-2011, Free Software Foundation, Inc. -- +-- Copyright (C) 1992-2012, Free Software Foundation, Inc. -- -- -- -- GNAT is free software; you can redistribute it and/or modify it under -- -- terms of the GNU General Public License as published by the Free Soft- -- @@ -803,12 +803,14 @@ -- the declaration and entity for the newly-created function. function Build_To_Any_Call - (N : Node_Id; + (Loc : Source_Ptr; +N : Node_Id; Decls : List_Id) return Node_Id; -- Build call to To_Any attribute function with expression as actual - -- parameter. Decls is the declarations list for an appropriate - -- enclosing scope of the point where the call will be inserted; if - -- the To_Any attribute for Typ needs to be generated at this point, + -- parameter. Loc is the reference location ofr generated nodes, + -- Decls is the declarations list for an appropriate enclosing scope + -- of the point where the call will be inserted; if the To_Any + -- attribute for the type of N needs to be generated at this point, -- its declaration is appended to Decls. procedure Build_To_Any_Function @@ -879,7 +881,8 @@ renames PolyORB_Support.Helpers.Build_From_Any_Call; function Build_To_Any_Call - (N : Node_Id; + (Loc : Source_Ptr; + N : Node_Id; Decls : List_Id) return Node_Id renames PolyORB_Support.Helpers.Build_To_Any_Call; @@ -6562,7 +6565,7 @@ Object_Definition => New_Occurrence_Of (RTE (RE_Any), Loc), Expression => PolyORB_Support.Helpers.Build_To_Any_Call - (RACW_Parameter, No_List))); + (Loc, RACW_Parameter, No_List))); Statements := New_List ( Make_Procedure_Call_Statement (Loc, @@ -7362,7 +7365,7 @@ -- the first one. Expr := PolyORB_Support.Helpers.Build_To_Any_Call - (Actual_Parameter, Decls); + (Loc, Actual_Parameter, Decls); else Expr := Make_Function_Call (Loc, @@ -7448,7 +7451,7 @@ New_Occurrence_Of (RTE (RE_Any), Loc), Expression => PolyORB_Support.Helpers.Build_To_Any_Call - (Parameter_Exp, Decls))); + (Loc, Parameter_Exp, Decls))); Append_To (Extra_Formal_Statements, Add_Parameter_To_NVList (Loc, @@ -7934,7 +7937,7 @@ Parameter_Associations => New_List ( New_Occurrence_Of (Any, Loc), PolyORB_Support.Helpers.Build_To_Any_Call - (New_Occurrence_Of (Object, Loc), Decls; + (Loc, New_Occu
[Ada] Only one of Priority or Interrupt_Priority can be specified
ARM D.1.7 states that only one pragma priority or interrupt_priority can be given for a given entity. This patch recognizes some illegal instances of these two pragmas or aspects, that were previously undiagnosed. Compiling tasking.ads in Ada 2012 mode must yield: tasking.ads:17:07: pragma "Interrupt_Priority" for "Monitor" duplicates pragma at line 16 tasking.ads:30:07: aspect "Priority" for "Lock" previously given at line 29 --- with System; with Ada.Real_Time; use Ada.Real_Time; package Tasking is procedure Last_Chance_Handler (Msg : System.Address; Line : Integer); pragma Export (C, Last_Chance_Handler, "__gnat_last_chance_handler"); procedure Wait_Forever; task Test_Driver is pragma Interrupt_Priority (System.Interrupt_Priority'Last); end Test_Driver; protected Monitor is pragma Priority (System.Priority'Last); pragma Interrupt_Priority (System.Interrupt_Priority'Last); procedure Handler; pragma Attach_Handler (Handler, 12); entry Wait; private Signaled : Boolean := False; end Monitor; protected type Lock with interrupt_priority => System.priority'last, priority => System.priority'last is procedure Seize; procedure Release; private In_Use : Boolean := False; end Lock; end Tasking; Tested on x86_64-pc-linux-gnu, committed on trunk 2012-10-29 Ed Schonberg * sem_aux.adb (Get_Rep_Item): Treat Priority and Interrupt_Priority as equivalent, because only one of them can be specified for a task, protected definition, or subprogram body. * aspects.adb ((Same_Aspect): The canonical aspect of Interrupt_Priority is Priority. Index: sem_aux.adb === --- sem_aux.adb (revision 192908) +++ sem_aux.adb (working copy) @@ -431,11 +431,17 @@ begin N := First_Rep_Item (E); while Present (N) loop + + -- Only one of Priority / Interrupt_Priority can be specified, so + -- return whichever one is present to catch illegal duplication. + if Nkind (N) = N_Pragma and then (Pragma_Name (N) = Nam or else (Nam = Name_Priority - and then Pragma_Name (N) = Name_Interrupt_Priority)) + and then Pragma_Name (N) = Name_Interrupt_Priority) + or else (Nam = Name_Interrupt_Priority + and then Pragma_Name (N) = Name_Priority)) then if Check_Parents then return N; Index: aspects.adb === --- aspects.adb (revision 192908) +++ aspects.adb (working copy) @@ -275,7 +275,7 @@ Aspect_Inline_Always=> Aspect_Inline, Aspect_Input=> Aspect_Input, Aspect_Interrupt_Handler=> Aspect_Interrupt_Handler, -Aspect_Interrupt_Priority => Aspect_Interrupt_Priority, +Aspect_Interrupt_Priority => Aspect_Priority, Aspect_Invariant=> Aspect_Invariant, Aspect_Iterator_Element => Aspect_Iterator_Element, Aspect_Link_Name=> Aspect_Link_Name,
[Ada] Warn on redefinition of standard entities
A new warning flag -gnatw.k causes the compiler to emit a warning if a declaration redefines an entity of package Standard. Such redefinitions are usually not a good idea, since these entities are directly visible, and this can lead to confusion. This warning is off by default. The following, compiled with -gnatw.k shows the warning in action: 1. package StandNames is 2.type Integer is new Natural; | >>> warning: redefinition of entity "Integer" in Standard 3.type Exceptions is (Tasking_Error, Storage_Error); | >>> warning: redefinition of entity "Tasking_Error" in Standard 4. end StandNames; Tested on x86_64-pc-linux-gnu, committed on trunk 2012-10-29 Robert Dewar * i-cstrea.ads: Avoid redefinition of standard symbol string. * prj-makr.adb: Add comment for OK redefinition of Stadard. * prj.ads: Add comment for OK redefinition of Stadard. * s-crtl.ads: Avoid redefinition of standard symbol string. * sinfo-cn.adb (Change_Identifier_To_Defining_Identifier): Generate warning for standard redefinition if Warn_On_Standard_Definition set. * usage.adb: Add lines for -gnatw.k and -gnatw.K * warnsw.adb: Set/reset Warn_On_Standard_Redefinition appropriately. * warnsw.ads (Warn_On_Standard_Redefinition): New flag. * s-stratt-xdr.adb: Avoid new warning. Index: i-cstrea.ads === --- i-cstrea.ads(revision 192908) +++ i-cstrea.ads(working copy) @@ -175,7 +175,7 @@ mode : int; size : size_t) return int; - procedure tmpnam (string : chars) renames System.CRTL.tmpnam; + procedure tmpnam (str : chars) renames System.CRTL.tmpnam; -- The parameter must be a pointer to a string buffer of at least L_tmpnam -- bytes (the call with a null parameter is not supported). The returned -- value, which is just a copy of the input argument, is discarded. Index: prj.ads === --- prj.ads (revision 192908) +++ prj.ads (working copy) @@ -68,14 +68,21 @@ type Yes_No_Unknown is (Yes, No, Unknown); -- Tri-state to decide if -lgnarl is needed when linking + pragma Warnings (Off); type Project_Qualifier is (Unspecified, + + -- The following clash with Standard is OK, and justified by the context + -- which really wants to use the same set of qualifiers. + Standard, + Library, Configuration, Dry, Aggregate, Aggregate_Library); + pragma Warnings (On); -- Qualifiers that can prefix the reserved word "project" in a project -- file: --Standard: standard project ... @@ -1188,8 +1195,18 @@ -- The following record describes a project file representation - type Standalone is (No, Standard, Encapsulated); + pragma Warnings (Off); + type Standalone is + (No, + -- The following clash with Standard is OK, and justified by the context + -- which really wants to use the same set of qualifiers. + + Standard, + + Encapsulated); + pragma Warnings (On); + type Project_Data (Qualifier : Project_Qualifier := Unspecified) is record - Index: prj-makr.adb === --- prj-makr.adb(revision 192908) +++ prj-makr.adb(working copy) @@ -6,7 +6,7 @@ -- -- -- B o d y -- -- -- --- Copyright (C) 2001-2011, Free Software Foundation, Inc. -- +-- Copyright (C) 2001-2012, Free Software Foundation, Inc. -- -- -- -- GNAT is free software; you can redistribute it and/or modify it under -- -- terms of the GNU General Public License as published by the Free Soft- -- @@ -120,7 +120,12 @@ Non_Empty_Node : constant Project_Node_Id := 1; -- Used for the With_Clause of the naming project + -- Turn off warnings for now around this redefinition of True and False, + -- but it really seems a bit horrible to do this redefinition ??? + + pragma Warnings (Off); type Matched_Type is (True, False, Excluded); + pragma Warnings (On); Naming_File_Suffix : constant String := "_naming"; Source_List_File_Suffix : constant String := "_source_list.txt"; Index: s-crtl.ads === --- s-crtl.ads (revision 192908) +++ s-crtl.ads (working copy) @@ -177,7 +177,7 @@ size : size_t) return int; pragma Import (C, setvbuf, "setvbuf"); - procedure tmpna
[Ada] Internal bindgen.adb cleanup
This patch remove a duplicated logic to check wether a package is in the closure. No functional change. Tested on x86_64-pc-linux-gnu, committed on trunk 2012-10-29 Tristan Gingold * bindgen.adb (Check_File_In_Partition, Check_System_Restrictions_Used): Removed. (Check_Dispatching_Domains_Used): Removed. (Gen_Adafinal): Remove call to above procedures. (Resolve_Binder_Options): Handle system restrictions and dispatching domains. Index: bindgen.adb === --- bindgen.adb (revision 192918) +++ bindgen.adb (working copy) @@ -63,20 +63,20 @@ Num_Elab_Calls : Nat := 0; -- Number of generated calls to elaboration routines - System_Restrictions_Used : Boolean; + System_Restrictions_Used : Boolean := False; -- Flag indicating whether the unit System.Restrictions is in the closure - -- of the partition. This is set by Check_System_Restrictions_Used, and + -- of the partition. This is set by Resolve_Binder_Options, and -- is used to determine whether or not to initialize the restrictions -- information in the body of the binder generated file (we do not want -- to do this unconditionally, since it drags in the System.Restrictions -- unit unconditionally, which is unpleasand, especially for ZFP etc.) - Dispatching_Domains_Used : Boolean; + Dispatching_Domains_Used : Boolean := False; -- Flag indicating whether multiprocessor dispatching domains are used in - -- the closure of the partition. This is set by - -- Check_Dispatching_Domains_Used, and is used to call the routine to - -- disallow the creation of new dispatching domains just before calling - -- the main procedure from the environment task. + -- the closure of the partition. This is set by Resolve_Binder_Options, + -- and is used to call the routine to disallow the creation of new + -- dispatching domains just before calling the main procedure from the + -- environment task. System_Tasking_Restricted_Stages_Used : Boolean := False; -- Flag indicating whether the unit System.Tasking.Restricted.Stages is in @@ -242,21 +242,6 @@ -- Local Subprograms -- --- - procedure Check_File_In_Partition - (File_Name : String; - Flag : out Boolean); - -- If the file indicated by File_Name is in the partition the Flag is set - -- to True, False otherwise. - - procedure Check_System_Restrictions_Used; - -- Sets flag System_Restrictions_Used (Set to True if and only if the unit - -- System.Restrictions is present in the partition, otherwise False). - - procedure Check_Dispatching_Domains_Used; - -- Sets flag Dispatching_Domains_Used to True when using the unit - -- System.Multiprocessors.Dispatching_Domains is present in the partition, - -- otherwise set to False. - procedure Gen_Adainit; -- Generates the Adainit procedure @@ -391,43 +376,6 @@ -- First writes its argument (using Set_String (S)), then writes out the -- contents of statement buffer up to Last, and reset Last to 0 - - -- Check_Dispatching_Domains_Used -- - - - procedure Check_Dispatching_Domains_Used is - begin - Check_File_In_Partition ("s-mudido.ads", Dispatching_Domains_Used); - end Check_Dispatching_Domains_Used; - - - - -- Check_File_In_Partition -- - - - - procedure Check_File_In_Partition - (File_Name : String; - Flag : out Boolean) - is - begin - for J in Units.First .. Units.Last loop - if Get_Name_String (Units.Table (J).Sfile) = File_Name then -Flag := True; -return; - end if; - end loop; - - Flag := False; - end Check_File_In_Partition; - - - -- Check_System_Restrictions_Used -- - - - procedure Check_System_Restrictions_Used is - begin - Check_File_In_Partition ("s-restri.ads", System_Restrictions_Used); - end Check_System_Restrictions_Used; - -- -- Gen_Adafinal -- -- @@ -2124,9 +2072,6 @@ -- Generate output file in appropriate language - Check_System_Restrictions_Used; - Check_Dispatching_Domains_Used; - Gen_Output_File_Ada (Filename); end Gen_Output_File; @@ -2869,6 +2814,23 @@ procedure Resolve_Binder_Options is + procedure Check_Package (Var : in out Boolean; Name : String); + -- Set Var to true iff the current identifier in Namet is Name. + -- Do nothing if it doesn't match. This procedure is just an helper + -- to avoid to explicitely deal with length. + + --- + -- Check_Package -- + --- + + proc
[Ada] Add support for returning binary message digest
If a binary digest is needed it is a waste of time to recompute it from the string encoded digest. Tested on x86_64-pc-linux-gnu, committed on trunk 2012-10-29 Pascal Obry * g-sechas.adb, g-sechas.ads: (Binary_Message_Digest): New subtype. (Digest): New versions returning a Binary_Message_Digest. (Wide_Digest): Likewise. Index: g-sechas.adb === --- g-sechas.adb(revision 192920) +++ g-sechas.adb(working copy) @@ -184,6 +184,30 @@ return Digest (C); end Digest; + function Digest (C : Context) return Binary_Message_Digest is + Hash_Bits : Stream_Element_Array + (1 .. Stream_Element_Offset (Hash_Length)); + begin + Final (C, Hash_Bits); + return Hash_Bits; + end Digest; + + function Digest (S : String) return Binary_Message_Digest is + C : Context; + begin + Update (C, S); + return Digest (C); + end Digest; + + function Digest +(A : Stream_Element_Array) return Binary_Message_Digest + is + C : Context; + begin + Update (C, A); + return Digest (C); + end Digest; + --- -- Final -- --- @@ -325,6 +349,13 @@ return Digest (C); end Wide_Digest; + function Wide_Digest (W : Wide_String) return Binary_Message_Digest is + C : Context; + begin + Wide_Update (C, W); + return Digest (C); + end Wide_Digest; + end H; - Index: g-sechas.ads === --- g-sechas.ads(revision 192920) +++ g-sechas.ads(working copy) @@ -156,6 +156,22 @@ Word_Length : constant Natural := Hash_State.Word'Size / 8; Hash_Length : constant Natural := Hash_Words * Word_Length; + subtype Binary_Message_Digest is +Stream_Element_Array (1 .. Stream_Element_Offset (Hash_Length)); + -- The fixed-length byte array returned by Digest, providing + -- the hash in binary representation. + + function Digest (C : Context) return Binary_Message_Digest; + -- Return hash for the data accumulated with C + + function Digest (S : String) return Binary_Message_Digest; + function Wide_Digest (W : Wide_String) return Binary_Message_Digest; + function Digest +(A : Stream_Element_Array) return Binary_Message_Digest; + -- These functions are equivalent to the corresponding Update (or + -- Wide_Update) on a default initialized Context, followed by Digest + -- on the resulting Context. + subtype Message_Digest is String (1 .. 2 * Hash_Length); -- The fixed-length string returned by Digest, providing the hash in -- hexadecimal representation.
Re: [patch][rfc] Clean up CFG dumping
On Mon, Jul 16, 2012 at 8:57 AM, Steven Bosscher wrote: > Hello, > > There are comments in basic-block.h that advise to update certain > parts of the compiler if a new edge flag or basic block flag is added: > > -/* Always update the table in cfg.c dump_edge_info. */ > > and > > - Always update the table in cfg.c dump_bb_info. */ > > Apparently this is not enough, because there are more places where the > BB flags are dumped. For instance, cfg.c:dump_cfg_bb_info does not > handle BB_MODIFIED, BB_VISITED, and BB_IN_TRANSACTION, and > dump_bb_info doesn't even exist in cfg.c (it's in cfgrtl.c). The flags > also aren't documented very well in the code. > > Furthermore, there are multiple places where "common" (i.e. IR > agnostic) basic block information is dumped, with some functions > taking TDF_* flags and others not, some functions taking a FILE as the > first argument and others as the second argument, etc. In short: > Unnecessarily messy. > > The attached patch cleans up the worst of it: > > * A new file cfg-flags.def is used to define the BB_* and EDGE_* > flags. The names are the full string of the name of the flag in > uppercase, that's a change from before. I can add a separate name > field to DEF_EDGE_FLAG and DEF_BB_FLAG if necessary. > > * Now that there is dumpfile.h for the TDF_* masks, it's easier to use > them everywhere. I have added one new flag to dump with the ";;" > prefix (I think it should be used in more places, but that's something > for later, perhaps). > > * All basic block header/footer and edge dumping is consolidated in > dump_edge_info and dump_bb_info. This affects GIMPLE dump the most, > because it uses a different form of dumping for basic block > predecessors and successors. I expect some fall-out in the test suite > (patterns that no longer match) that I'll have to address before the > patch is ready for mainline. > > * Slim RTL printing is better integrated: print_rtl_with_bb takes > flags and uses dump_rtl_slim if TDF_SLIM is passed. The same happens > in rtl_dump_bb, which always dumps non-slim RTL without this patch. > > Bootstrapped on powerpc64-unknown-linux-gnu. Testing will probably > reveal some test suite pattern mismatches, and I also still want to > bootstrap&test this on x86_64. > I'd like to hear what people think of the cfg-flags.def change. > > Ciao! > Steven > > * dumpfile.h (TDF_COMMENT): New define. > * basic-block.h (EDGE_FALLTHRU, EDGE_ABNORMAL, EDGE_ABNORMAL_CALL, > EDGE_EH, EDGE_FAKE, EDGE_DFS_BACK, EDGE_CAN_FALLTHRU, > EDGE_IRREDUCIBLE_LOOP, EDGE_SIBCALL, EDGE_LOOP_EXIT, EDGE_TRUE_VALUE, > EDGE_FALSE_VALUE, EDGE_EXECUTABLE, EDGE_CROSSING, EDGE_PRESERVE): > Move to new file cfg-flags.h. > (enum cfg_edge_flags): New enum, using cfg-flags.h. > (EDGE_ALL_FLAGS): Compute value automatically. > (BB_NEW, BB_REACHABLE, BB_IRREDUCIBLE_LOOP, BB_SUPERBLOCK, > BB_DISABLE_SCHEDULE, BB_HOT_PARTITION, BB_COLD_PARTITION, > BB_DUPLICATED, BB_NON_LOCAL_GOTO_TARGET, BB_RTL, > BB_FORWARDER_BLOCK, BB_NONTHREADABLE_BLOCK, BB_MODIFIED, BB_VISITED, > BB_IN_TRANSACTION): Move to new file cfg-flags.h. > (enum bb_flags): Rename to cfg_bb_flags. Use cfg-flags.h. > (BB_ALL_FLAGS): New, compute value automatically. > (dump_bb_info): Update prototype. > (dump_edge_info): Update prototype. > * cfg-flags.h: New file. > * cfg.c (dump_edge_info): Take flags argument. Be verbose only if > TDF_DETAILS and not TDF_SLIM. Include cfg-flags.h for bitnames. > Check that the edge flags are within the range of EDGE_ALL_FLAGS. > (debug_bb): Update dump_bb call. > (dump_cfg_bb_info): Remove. > (dump_bb_info): New function. Use cfg-flags.h for bitnames. > Adjust verbosity using TDF_* flags. Check that the basic block flags > are within the range of BB_ALL_FLAGS. > (brief_dump_cfg): Use dump_bb_info instead of dump_cfg_bb_info. > * cfghooks.h (struct cfghooks): Update dump_bb hook, take a FILE > first for consistency with other dump functions. > (dump_bb): Update prototype accordingly. > * cfghooks.c: Include dumpfile.h. > (verify_flow_info): Update dump_edge_info calls. > (dump_bb): Take a flags argument and pass it around. > Use dump_bb_info to dump common information about a basic block. > (dump_flow_info): Moved here from cfgrtl.c. Make IL agnostic. > (debug_flow_info): Moved here from cfgrtl.c. > * profile.c (is_edge_inconsistent): Update dump_bb calls. > * loop-invariant.c (find_defs): Update print_rtl_with_bb call. > * rtl.h (debug_bb_n_slim, debug_bb_slim, print_rtl_slim, > print_rtl_slim_with_bb): Remove prototypes. > (dump_insn_slim): Adjust prototype to take a const_rtx. > (print_rtl_with_bb): Adjust prototype. > * sched-rgn.c (debug_
[Ada] Add a new pragma Assert_And_Cut for formal verification tools
The new pragma Assert_And_Cut behaves exactly like pragma Assert for compilation, but it informs formal verification tools of program points where the tool may simpllify it treatment by using the given assertion. Tested on x86_64-pc-linux-gnu, committed on trunk 2012-10-29 Yannick Moy * gnat_rm.texi: Describe new pragma Assert_And_Cut. * par-prag.adb, sem_prag.adb, snames.ads-tmpl: Add new pragma and treat it like pragma Assert. Index: gnat_rm.texi === --- gnat_rm.texi(revision 192918) +++ gnat_rm.texi(working copy) @@ -105,6 +105,7 @@ * Pragma Ada_2012:: * Pragma Annotate:: * Pragma Assert:: +* Pragma Assert_And_Cut:: * Pragma Assertion_Policy:: * Pragma Assume_No_Invalid_Values:: * Pragma Ast_Entry:: @@ -843,6 +844,7 @@ * Pragma Ada_2012:: * Pragma Annotate:: * Pragma Assert:: +* Pragma Assert_And_Cut:: * Pragma Assertion_Policy:: * Pragma Assume_No_Invalid_Values:: * Pragma Ast_Entry:: @@ -1195,7 +1197,23 @@ of Ada, and the DISABLE policy is an implementation-defined addition. +@node Pragma Assert_And_Cut +@unnumberedsec Pragma Assert_And_Cut +@findex Assert_And_Cut +@noindent +Syntax: +@smallexample @c ada +pragma Assert_And_Cut ( + boolean_EXPRESSION + [, string_EXPRESSION]); +@end smallexample +@noindent +The effect of this pragma for compilation is exactly the same as the one +of pragma @code{Assert}. This pragma is used to help formal verification +tools by marking program points where the tool can simplify precise +knowledge about execution based on the assertion given. + @node Pragma Assertion_Policy @unnumberedsec Pragma Assertion_Policy @findex Debug_Policy Index: sem_prag.adb === --- sem_prag.adb(revision 192918) +++ sem_prag.adb(working copy) @@ -6759,14 +6759,17 @@ end if; end Annotate; - - -- Assert -- - + - + -- Assert & Assert_And_Cut -- + - -- pragma Assert ([Check =>] Boolean_EXPRESSION -- [, [Message =>] Static_String_EXPRESSION]); - when Pragma_Assert => Assert : declare + -- pragma Assert_And_Cut ([Check =>] Boolean_EXPRESSION + -- [, [Message =>] Static_String_EXPRESSION]); + + when Pragma_Assert | Pragma_Assert_And_Cut => Assert : declare Expr : Node_Id; Newa : List_Id; @@ -6784,6 +6787,13 @@ -- So rewrite pragma in this manner, transfer the message -- argument if present, and analyze the result +-- Pragma Assert_And_Cut is treated exactly like pragma Assert by +-- the frontend. Formal verification tools may use it to "cut" the +-- paths through the code, to make verification tractable. When +-- dealing with a semantically analyzed tree, the information that +-- a Check node N corresponds to a source Assert_And_Cut pragma +-- can be retrieved from the pragma kind of Original_Node(N). + Expr := Get_Pragma_Arg (Arg1); Newa := New_List ( Make_Pragma_Argument_Association (Loc, @@ -15185,6 +15195,7 @@ Pragma_All_Calls_Remote => -1, Pragma_Annotate => -1, Pragma_Assert => -1, + Pragma_Assert_And_Cut => -1, Pragma_Assertion_Policy => 0, Pragma_Assume_No_Invalid_Values => 0, Pragma_Asynchronous => -1, Index: par-prag.adb === --- par-prag.adb(revision 192918) +++ par-prag.adb(working copy) @@ -1098,6 +1098,7 @@ Pragma_All_Calls_Remote | Pragma_Annotate | Pragma_Assert | + Pragma_Assert_And_Cut | Pragma_Asynchronous | Pragma_Atomic | Pragma_Atomic_Components | Index: snames.ads-tmpl === --- snames.ads-tmpl (revision 192918) +++ snames.ads-tmpl (working copy) @@ -448,6 +448,7 @@ -- correctly recognize and process Name_AST_Entry. Name_Assert : constant Name_Id := N + $; -- Ada 05 + Name_Assert_And_Cut : constant Name_Id := N + $; -- GNAT Name_Asynchronous : constant Name_Id := N + $; Name_Atomic : constant Name_Id := N + $; Name_Atomic_Components : constant Name_Id := N + $; @@ -1697,6 +1698,7 @@ Pragma_Abort_Defer,
[Ada] Clean up of gnatbind output (for non-standard run times)
A declaration for Is_Elaborated was emitted but never referenced. No functional change. Tested on x86_64-pc-linux-gnu, committed on trunk 2012-10-29 Tristan Gingold * bindgen.adb (Gen_Output_File_Ada): Do not emit declaration for Is_Elaborated if not referenced. Index: bindgen.adb === --- bindgen.adb (revision 192922) +++ bindgen.adb (working copy) @@ -2394,8 +2394,13 @@ -- The B.1 (39) implementation advice says that the adainit/adafinal -- routines should be idempotent. Generate a flag to ensure that. + -- This is not needed if we are suppressing the standard library + -- since it would never be referenced. - WBI (" Is_Elaborated : Boolean := False;"); + if not Suppress_Standard_Library_On_Target then +WBI (" Is_Elaborated : Boolean := False;"); + end if; + WBI (""); end if;
[Ada] Duplicate entries for parameters in gnatxref output
When a call to a subprogram in another package uses named parameters, gnatxref will sometimes output duplicate entries for the parameter, depending on the order in which the ALI files are read. Tested on x86_64-pc-linux-gnu, committed on trunk 2012-10-29 Emmanuel Briot * xr_tabls.adb, xr_tabls.ads (Add_Declaration, Add_Reference): No longer assume that a parameter declaration is seen after the subprogram that uses it. Index: xr_tabls.adb === --- xr_tabls.adb(revision 192918) +++ xr_tabls.adb(working copy) @@ -6,7 +6,7 @@ -- -- -- B o d y -- -- -- --- Copyright (C) 1998-2010, Free Software Foundation, Inc. -- +-- Copyright (C) 1998-2012, Free Software Foundation, Inc. -- -- -- -- GNAT is free software; you can redistribute it and/or modify it under -- -- terms of the GNU General Public License as published by the Free Soft- -- @@ -223,6 +223,7 @@ Line : Natural; Column : Natural; Decl_Type: Character; + Is_Parameter : Boolean := False; Remove_Only : Boolean := False; Symbol_Match : Boolean := True) return Declaration_Reference @@ -235,7 +236,7 @@ New_Decl : Declaration_Reference := Entities_HTable.Get (Key'Unchecked_Access); - Is_Parameter : Boolean := False; + Is_Param : Boolean := Is_Parameter; begin -- Insert the Declaration in the table. There might already be a @@ -243,7 +244,7 @@ -- need to check that first. if New_Decl /= null and then New_Decl.Symbol_Length = 0 then - Is_Parameter := New_Decl.Is_Parameter; + Is_Param := Is_Parameter or else New_Decl.Is_Parameter; Entities_HTable.Remove (Key'Unrestricted_Access); Entities_Count := Entities_Count - 1; Free (New_Decl.Key); @@ -269,7 +270,7 @@ Column=> Column, Source_Line => null, Next => null), - Is_Parameter => Is_Parameter, + Is_Parameter => Is_Param, Decl_Type => Decl_Type, Body_Ref => null, Ref_Ref => null, @@ -294,6 +295,10 @@ then New_Decl.Match := Default_Match or else Match (File_Ref, Line, Column); + New_Decl.Is_Parameter := New_Decl.Is_Parameter or else Is_Param; + + elsif New_Decl /= null then + New_Decl.Is_Parameter := New_Decl.Is_Parameter or else Is_Param; end if; return New_Decl; @@ -392,6 +397,8 @@ Labels_As_Ref : Boolean) is New_Ref : Reference; + New_Decl : Declaration_Reference; + pragma Unreferenced (New_Decl); begin case Ref_Type is @@ -407,37 +414,22 @@ when '=' | '<' | '>' | '^' => -- Create a dummy declaration in the table to report it as a --- parameter. Note that the current declaration for the subprogram --- comes before the declaration of the parameter. +-- parameter. +-- In a given ALI file, the declaration of the subprogram comes +-- before the declaration of the parameter. However, it is +-- possible that another ALI file has been parsed that also +-- references the parameter (for instance a named parameter in a +-- call), so we need to check whether there already exists a +-- declaration for the parameter. -declare - Key : constant String := -Key_From_Ref (File_Ref, Line, Column); - New_Decl : Declaration_Reference; +New_Decl := Add_Declaration + (File_Ref => File_Ref, + Symbol=> "", + Line => Line, + Column=> Column, + Decl_Type => ' ', + Is_Parameter => True); -begin - New_Decl := new Declaration_Record' - (Symbol_Length => 0, - Symbol=> "", - Key => new String'(Key), - Decl => new Reference_Record' - (File => File_Ref, - Line => Line, - Column=> Column, - Source_Line => null, - Next => null), - Is_Para
[Ada] Implement Pragma Partition_Elaboration_Policy
Initial work to implement pragma Partition_Elaboration_Policy. Currently, only consistency is checked, the runtime only implements one policy. A following patch will add a pragma Partition_Elaboration_Policy in the runtime to enforce the policy (when tasking is used). Tested on x86_64-pc-linux-gnu, committed on trunk 2012-10-29 Tristan Gingold * lib-writ.adb (Write_ALI): Emit partition elaboration policy in P line. * lib-writ.ads: Document partition elaboration policy indication. * sem_prag.adb (Check_Arg_Is_Partition_Elaboration_Policy): New procedure. (Analyze_Pragma): Handle Partition_Elaboration_Policy. (Sig_Flags): Add flag for Pragma_Partition_Elaboration_Policy * ali.adb (Initialize_ALI): Init Partition_Elaboration_Policy_Specified. (Scan_ALI): Read Ex indications. * ali.ads: ALIs_Record: Add Partition_Elaboration_Policy. * par-prag.adb (Prag): Add Partition_Elaboration_Policy. * snames.adb-tmpl (Is_Partition_Elaboration_Policy_Name): New function. * opt.ads (Partition_Elaboration_Policy): Declare. (Partition_Elaboration_Policy_Sloc): Declare. * bcheck.adb (Check_Consistent_Partition_Elaboration_Policy): New procedure. (Check_Configuration_Consistency): Check partition elaboration policy consistency. * snames.ads-tmpl (Name_Partition_Elaboration_Policy): New name. (First_Partition_Elaboration_Policy_Name, Name_Concurrent, Name_Sequential, Last_Partition_Elaboration_Policy_Name): Likewise. (Pragma_Partition_Elaboration_Policy): New literal. (Is_Partition_Elaboration_Policy_Name): New function. Index: lib-writ.adb === --- lib-writ.adb(revision 192918) +++ lib-writ.adb(working copy) @@ -1099,6 +1099,11 @@ end if; end if; + if Partition_Elaboration_Policy /= ' ' then + Write_Info_Str (" E"); + Write_Info_Char (Partition_Elaboration_Policy); + end if; + if not Object then Write_Info_Str (" NO"); end if; Index: lib-writ.ads === --- lib-writ.ads(revision 192918) +++ lib-writ.ads(working copy) @@ -196,6 +196,10 @@ -- DB Detect_Blocking pragma is in effect for all units in this -- file. -- + -- Ex A valid Partition_Elaboration_Policy pragma applies to all + -- the units in this file, where x is the first character + -- (upper case) of the policy name (e.g. 'C' for Concurrent). + -- -- FD Configuration pragmas apply to all the units in this file -- specifying a possibly non-standard floating point format -- (VAX float with Long_Float using D_Float). Index: sem_prag.adb === --- sem_prag.adb(revision 192926) +++ sem_prag.adb(working copy) @@ -505,6 +505,10 @@ -- Check the specified argument Arg to make sure that it is a valid -- locking policy name. If not give error and raise Pragma_Exit. + procedure Check_Arg_Is_Partition_Elaboration_Policy (Arg : Node_Id); + -- Check the specified argument Arg to make sure that it is a valid + -- elaboration policy name. If not give error and raise Pragma_Exit. + procedure Check_Arg_Is_One_Of (Arg: Node_Id; N1, N2 : Name_Id); @@ -1190,6 +1194,22 @@ end if; end Check_Arg_Is_Locking_Policy; + --- + -- Check_Arg_Is_Partition_Elaboration_Policy -- + --- + + procedure Check_Arg_Is_Partition_Elaboration_Policy (Arg : Node_Id) is + Argx : constant Node_Id := Get_Pragma_Arg (Arg); + + begin + Check_Arg_Is_Identifier (Argx); + + if not Is_Partition_Elaboration_Policy_Name (Chars (Argx)) then +Error_Pragma_Arg + ("& is not a valid partition elaboration policy name", Argx); + end if; + end Check_Arg_Is_Partition_Elaboration_Policy; + - -- Check_Arg_Is_One_Of -- - @@ -12039,6 +12059,53 @@ when Pragma_Page => null; + -- + -- Partition_Elaboration_Policy -- + -- + + -- pragma Partition_Elaboration_Policy (policy_IDENTIFIER); + + when Pragma_Partition_Elaboration_Policy => declare +subtype PEP_Range is Name_Id + range First_Partition_Elaboration_Policy_Name + .. Last_Partition_Elaboration_Policy_Name; +PEP_Val : PEP_Range; +PEP : Character;
Re: [RFC PATCH, i386]: Remove peephole2s for (subreg (operator (...)(...))) RTXes
On Mon, Oct 29, 2012 at 10:06 AM, Uros Bizjak wrote: > As suggested by Richard S. [1], after the patch that converts subreg:M > (op:N (...)(...)) to op:M (subreg:M (...) subreg:M (...)), we can > remove several peephole2 patterns that handle subregs of PLUS, MINUS > and MULT operators. I have attached RFC prototype patch that will > trigger an ICE when to-be-removed pattern triggers, with the intention > that these patterns wil be removed entirely (An "invalid" pattern was > indeed generated elsewhere, see patch). >> >> I have committed following version that avoids all failures, reported by >> H.J.: >> >> 2012-10-29 Uros Bizjak >> >> * config/i386/i386.c (ix86_decompose_address): Use >> simplify_gen_subreg >> to generate SImode equivalent of address, zero-extended with AND RTX. >> * config/i386/i386.md (ashift to lea splitter): Split to SImode mult. >> (simple lea to add/shift peephole2s): Remove peephole2s that operate >> on subregs of DImode operations. >> >> Re-tested on x86_64-pc-linux-gnu {,-m32} and committed to mainline SVN. > > Oh, we don't have to use simplify_gen_subreg, gen_subreg is enough. Not really, then we won't generate SUBREGs of pseudos... Reverting the commit. Uros.
[Ada] Static entry [family] names for VMS Debug
This patch reimplements the mechanism which creates string names for entries and families. The names are now created statically during object initialization and associated with the concurrent object's Protection_Entries or ATCB. This approach eliminates the need for runtime support (allocation, deallocation) of the names. No simple test as this requires VMS and gdb to demonstrate Tested on x86_64-pc-linux-gnu, committed on trunk 2012-10-29 Hristian Kirtchev * exp_ch3.adb (Build_Initialization_Call): Create static strings which denote entry [family] names and associate them with the object's Protection_Entries or ATCB. (Build_Init_Statements): Remove local variable Names. Do not generate the entry [family] names inside the init proc because they are now static. * exp_ch9.adb (Build_Entry_Names): Reimplemented. The strings which denote entry [family] names are now generated statically and associated with the concurrent object's Protection_Entries or ATCB during initialization. * exp_ch9.ads (Build_Entry_Names): Change subprogram profile and associated comment on usage. * rtsfind.ads: Add the following entries to tables RE_Id and RE_Unit_Table: RE_Protected_Entry_Names_Array RE_Task_Entry_Names_Array RO_PE_Number_Of_Entries RO_PE_Set_Entry_Names RO_ST_Number_Of_Entries RO_ST_Set_Entry_Names Remove the following entries from tables RE_Id and RE_Unit_Table: RO_PE_Set_Entry_Name RO_TS_Set_Entry_Name * s-taskin.adb: Remove with clause for Ada.Unchecked_Deallocation. (Free_Entry_Names_Array): Removed. (Number_Of_Entries): New routine. (Set_Entry_Names): New routine. * s-taskin.ads: Rename type Entry_Names_Array to Task_Entry_Names_Array. Rename type Entry_Names_Array_Access to Task_Entry_Names_Access. Update the type of ACTB field Entry_Names and add a comment on its protection status. (Free_Entry_Names_Array): Removed. (Number_Of_Entries): New routine. (Set_Entry_Names): New routine. * s-tassta.adb (Create_Task): Remove formal parameter Build_Entry_Names. Do not allocate an array to hold the string names of entries and families. (Free_Entry_Names): Removed. (Free_Task): Remove the call to Free_Entry_Names. (Set_Entry_Name): Removed. (Vulnerable_Free_Task): Remove the call to Free_Entry_Names. * s-tassta.ads (Create_Task): Remove formal parameter Build_Entry_Names along with associated comment. (Set_Entry_Name): Removed. * s-tpoben.adb: Remove with clause for Ada.Unchecked_Deallocation. (Finalize): Remove the call to Free_Entry_Names. (Free_Entry_Names): Removed. (Initialize_Protection_Entries): Remove formal parameter Build_Entry_Names. Do not allocate an array to hold the string names of entries and families. (Number_Of_Entries): New routine. (Set_Entry_Name): Removed. (Set_Entry_Names): New routine. * s-tpoben.ads: Add types Protected_Entry_Names_Array and Protected_Entry_Names_Access. Update the type of Protection_Enties field Entry_Names. (Initialize_Protection_Entries): Remove formal parameter Build_Entry_Names along with associated comment. (Number_Of_Entries): New routine. (Set_Entry_Name): Removed. (Set_Entry_Names): New routine. Index: exp_ch9.adb === --- exp_ch9.adb (revision 192929) +++ exp_ch9.adb (working copy) @@ -1363,59 +1363,54 @@ -- Build_Entry_Names -- --- - function Build_Entry_Names (Conc_Typ : Entity_Id) return Node_Id is - Loc : constant Source_Ptr := Sloc (Conc_Typ); - B_Decls : List_Id; - B_Stmts : List_Id; - Comp : Node_Id; - Index : Entity_Id; - Index_Typ : RE_Id; - Typ : Entity_Id := Conc_Typ; + procedure Build_Entry_Names + (Obj_Ref : Node_Id; + Obj_Typ : Entity_Id; + Stmts : List_Id) + is + Loc : constant Source_Ptr := Sloc (Obj_Ref); + Data : Entity_Id := Empty; + Index : Entity_Id := Empty; + Typ : Entity_Id := Obj_Typ; - procedure Build_Entry_Family_Name (Id : Entity_Id); - -- Generate: - --for Lnn in Family_Low .. Family_High loop - -- Inn := Inn + 1; - -- Set_Entry_Name - -- (_init._object _init._task_id, - -- Inn, - -- new String ("(" & Lnn'Img & ")")); - --end loop; - -- Note that the bounds of the range may reference discriminants. The - -- above construct is added directly to the statements of the block. + procedure Build_Entry_Name (Comp_Id : Entity_Id); + -- Given an entry [family], create a static st
[Ada] New implementation-defined pragma: Attribute_Definition
This change introduces a new implementation defined pragma "Attribute_Definition", which allows an attribute definition clause to be expressed in a backward-compatible manner: compilers not supporting the pragma, or the specified attribute, will just ignore it. Tested on x86_64-pc-linux-gnu, committed on trunk 2012-10-29 Thomas Quinot * gnat_rm.texi, sem_prag.adb, sem_util.adb, sem_util.ads, par-prag.adb, par-util.adb, snames.ads-tmpl (Sem_Prag.Analyze_Pragma): Handle new pragma Attribute_Definition. (Sem_Util.Bad_Attribute): New routine, moved here from par-util, so that it can be used by the above. (Par_Util.Signal_Bad_Attribute): Processing moved to Sem_Util.Bad_Attribute. Index: gnat_rm.texi === --- gnat_rm.texi(revision 192933) +++ gnat_rm.texi(working copy) @@ -107,6 +107,7 @@ * Pragma Assert:: * Pragma Assertion_Policy:: * Pragma Assume_No_Invalid_Values:: +* Pragma Attribute_Definition:: * Pragma Ast_Entry:: * Pragma C_Pass_By_Copy:: * Pragma Check:: @@ -845,6 +846,7 @@ * Pragma Assert:: * Pragma Assertion_Policy:: * Pragma Assume_No_Invalid_Values:: +* Pragma Attribute_Definition:: * Pragma Ast_Entry:: * Pragma C_Pass_By_Copy:: * Pragma Check:: @@ -1308,6 +1310,28 @@ normal use of the entry. For further details on this pragma, see the DEC Ada Language Reference Manual, section 9.12a. +@node Pragma Attribute_Definition +@unnumberedsec Pragma Attribute_Definition +@findex Attribute_Definition +@noindent +Syntax: +@smallexample @c ada +pragma Attribute_Definition + ([Attribute =>] ATTRIBUTE_DESIGNATOR, + [Entity =>] LOCAL_NAME, + [Expression =>] EXPRESSION | NAME); +@end smallexample + +@noindent +If Attribute is a known attribute name, this pragma is equivalent to +the attribute definition clause: +@smallexample @c ada + for Entity'Attribute use Expression; +@end smallexample +else the pragma is ignored, and a warning is emitted. This allows source +code to be written that takes advantage of some new attribute, while remaining +compilable with earlier compilers. + @node Pragma C_Pass_By_Copy @unnumberedsec Pragma C_Pass_By_Copy @cindex Passing by copy Index: sem_prag.adb === --- sem_prag.adb(revision 192934) +++ sem_prag.adb(working copy) @@ -6919,6 +6919,47 @@ Assume_No_Invalid_Values := False; end if; + -- + -- Attribute_Definition -- + -- + + -- pragma Attribute_Definition + --([Attribute =>] ATTRIBUTE_DESIGNATOR, + -- [Entity =>] LOCAL_NAME, + -- [Expression =>] EXPRESSION | NAME); + + when Pragma_Attribute_Definition => Attribute_Definition : declare +Attribute_Designator : constant Node_Id := Get_Pragma_Arg (Arg1); +Aname : Name_Id; + + begin +GNAT_Pragma; +Check_Arg_Count (3); +Check_Optional_Identifier (Arg1, "attribute"); +Check_Optional_Identifier (Arg2, "entity"); +Check_Optional_Identifier (Arg3, "expression"); + +if Nkind (Attribute_Designator) /= N_Identifier then + Error_Msg_N ("attribute name expected", Attribute_Designator); + return; +end if; + +Check_Arg_Is_Local_Name (Arg2); + +Aname := Chars (Attribute_Designator); +if not Is_Attribute_Name (Aname) then + Bad_Attribute (Attribute_Designator, Aname, Warn => True); + return; +end if; + +Rewrite (N, + Make_Attribute_Definition_Clause (Loc, +Name => Get_Pragma_Arg (Arg2), +Chars => Aname, +Expression => Get_Pragma_Arg (Arg3))); +Analyze (N); + end Attribute_Definition; + --- -- AST_Entry -- --- @@ -15289,6 +15330,7 @@ Pragma_Assert_And_Cut => -1, Pragma_Assertion_Policy => 0, Pragma_Assume_No_Invalid_Values => 0, + Pragma_Attribute_Definition => +3, Pragma_Asynchronous => -1, Pragma_Atomic => 0, Pragma_Atomic_Components => 0, Index: sem_util.adb === --- sem_util.adb(revision 192918) +++ sem_util.adb(working copy) @@ -36,6 +36,7 @@ with Freeze; use Freeze; with Lib; use Lib; with Lib.Xref; use Lib.Xref; +with Namet.Sp; use Namet.Sp; with Nlists; use Nlists; with Nmake;use Nmake; with Output; use Output; @@ -404,6 +405,33 @@ and then Scope_Depth (ST) >= Scope_Depth (SCT); end Available_Full_View_
[Ada] Fix thinko in bignum allocation routine
This makes the new support for extended overflow checking work on big-endian platforms as well by fixing a thinko in the allocation routine of bignums. No functional changes. Tested on x86_64-pc-linux-gnu, committed on trunk 2012-10-29 Eric Botcazou * s-bignum.adb (Allocate_Bignum): Use the exact layout of Bignum_Data for the overlay. Index: s-bignum.adb === --- s-bignum.adb(revision 192933) +++ s-bignum.adb(working copy) @@ -233,14 +233,27 @@ pragma Import (Ada, BD); -- Expose a writable view of discriminant BD.Len so that we can --- initialize it. +-- initialize it. We need to use the exact layout of the record +-- for the overlay to shield ourselves from endianness issues. -BL : Length; -for BL'Address use BD.Len'Address; -pragma Import (Ada, BL); +type Bignum_Data_Header is record + Len : Length; + Neg : Boolean; +end record; +for Bignum_Data_Header use record + Len at 0 range 0 .. 23; + Neg at 3 range 0 .. 7; +end record; + +BDH : Bignum_Data_Header; +for BDH'Address use BD'Address; +pragma Import (Ada, BDH); + +pragma Assert (BDH.Len'Size = BD.Len'Size); + begin -BL := Len; +BDH.Len := Len; return B; end; end if;
[Ada] Incorrect sign extensions on IOCTL system constants
This change fixes incorrect values in s-oscons.ads on x86_64-freebsd, which were caused by those values being processed as signed, instead of unsigned, integers. Tested on x86_64-pc-linux-gnu, committed on trunk 2012-10-29 Thomas Quinot * s-oscons-tmplt.c: Fix signedness of ioctl request identifiers for x86_64-freebsd. Index: s-oscons-tmplt.c === --- s-oscons-tmplt.c(revision 192918) +++ s-oscons-tmplt.c(working copy) @@ -218,6 +218,14 @@ #define CST(name,comment) C(#name,String,name,comment) +/* ioctl(2) requests are "int" in UNIX, but "unsigned long" on FreeBSD */ + +#ifdef __FreeBSD__ +# define CNI CNU +#else +# define CNI CND +#endif + #define STR(x) STR1(x) #define STR1(x) #x @@ -373,12 +381,12 @@ #ifndef FIONBIO # define FIONBIO -1 #endif -CND(FIONBIO, "Set/clear non-blocking io") +CNI(FIONBIO, "Set/clear non-blocking io") #ifndef FIONREAD # define FIONREAD -1 #endif -CND(FIONREAD, "How many bytes to read") +CNI(FIONREAD, "How many bytes to read") /* @@ -1318,6 +1326,7 @@ #define SIZEOF_sigset (sizeof (sigset_t)) CND(SIZEOF_sigset, "sigset"); #endif + /* -- Fields of struct msghdr @@ -1399,7 +1408,7 @@ /* There's no clock_gettime or clock_id's on Darwin, generate a dummy value */ # define CLOCK_RT_Ada "-1" -#elif defined(FreeBSD) || defined(_AIX) +#elif defined(__FreeBSD__) || defined(_AIX) /** On these platforms use system provided monotonic clock instead of ** the default CLOCK_REALTIME. We then need to set up cond var attributes ** appropriately (see thread.c).
[Ada] Ignore Optimize_Alignment (Space) for packed variable length record
Normally pragma Optimize_Alignment (Space) forces an alignment of 1 for any packed record, but we can't do that for a variable length record, since we can't implement that case, so ignore the pragma in this case with a warning. The following test program shows the patch in action: 1. pragma optimize_alignment (space); 2. procedure testalign is 3.subtype Continuous_Values is float; 4.type Continuous_Values_Map is 5. array ( integer range <> ) of Continuous_Values; 6. 7.type my_record (s : integer) is record | >>> Optimize_Alignment has no effect for "my_record" >>> pragma is ignored for variable length record 8. x : continuous_values_map (1 .. s); 9.end record; 10.pragma pack (my_record); 11. 12.type my_record2 (s : integer) is record 13. x : continuous_values_map (1 .. 1000); 14.end record; 15.pragma pack (my_record2); 16. 17. begin 18. null; 19. end; Tested on x86_64-pc-linux-gnu, committed on trunk 2012-10-29 Robert Dewar * layout.adb (Set_Composite_Alignment): Ignore pragma Optimize_Alignment (Space) for packed variable length records. Index: layout.adb === --- layout.adb (revision 192918) +++ layout.adb (working copy) @@ -2882,7 +2882,12 @@ and then Is_Packed (E) and then not Is_Atomic (E) then - Align := 1; + if not Size_Known_At_Compile_Time (E) then +Error_Msg_N ("Optimize_Alignment has no effect for &", E); +Error_Msg_N ("\pragma is ignored for variable length record?", E); + else +Align := 1; + end if; -- Not a record, or not packed
[Patch] Add myself to MAINTAINERS as Write After Approval
Hi, This patch adds me to the Write After Approval section of MAINTAINERS. Regards, James Greenhalgh -- 2012-10-26 James Greenhalgh * MAINTAINERS (Write After Approval): Add myself. wap.patch Description: wap.patch
PATCH: Correct alloca length in dump_gimple_bb_header
Hi, When indent == 0, we call alloca with -1 bytes. This patch changes it to indent + 1. This is a trunk only regression. OK to install? Thanks. H.J. --- 2012-10-29 H.J. Lu * gimple-pretty-print.c (dump_gimple_bb_header): Correct alloca length. diff --git a/gcc/gimple-pretty-print.c b/gcc/gimple-pretty-print.c index 4b3235e..62c315e 100644 --- a/gcc/gimple-pretty-print.c +++ b/gcc/gimple-pretty-print.c @@ -2093,7 +2093,7 @@ dump_gimple_bb_header (FILE *outf, basic_block bb, int indent, int flags) gimple stmt = first_stmt (bb); if (!stmt || gimple_code (stmt) != GIMPLE_LABEL) { - char *s_indent = (char *) alloca ((size_t) indent - 2 + 1); + char *s_indent = (char *) alloca ((size_t) indent + 1); memset (s_indent, ' ', (size_t) indent); s_indent[indent] = '\0'; fprintf (outf, "%s:\n", s_indent, bb->index);
Re: PATCH: Correct alloca length in dump_gimple_bb_header
On Mon, Oct 29, 2012 at 05:06:01AM -0700, H.J. Lu wrote: > When indent == 0, we call alloca with -1 bytes. This patch changes > it to indent + 1. This is a trunk only regression. OK to install? > 2012-10-29 H.J. Lu > > * gimple-pretty-print.c (dump_gimple_bb_header): Correct alloca > length. > > diff --git a/gcc/gimple-pretty-print.c b/gcc/gimple-pretty-print.c > index 4b3235e..62c315e 100644 > --- a/gcc/gimple-pretty-print.c > +++ b/gcc/gimple-pretty-print.c > @@ -2093,7 +2093,7 @@ dump_gimple_bb_header (FILE *outf, basic_block bb, int > indent, int flags) >gimple stmt = first_stmt (bb); >if (!stmt || gimple_code (stmt) != GIMPLE_LABEL) > { > - char *s_indent = (char *) alloca ((size_t) indent - 2 + 1); > + char *s_indent = (char *) alloca ((size_t) indent + 1); > memset (s_indent, ' ', (size_t) indent); > s_indent[indent] = '\0'; > fprintf (outf, "%s:\n", s_indent, bb->index); Can't you instead of all this just do fprintf (outf, "%*s:\n", indent, "", bb->index); ? Jakub
Re: User directed Function Multiversioning via Function Overloading (issue5752064)
> Index: gcc/cgraph.c > === > --- gcc/cgraph.c (revision 192623) > +++ gcc/cgraph.c (working copy) > @@ -132,6 +132,74 @@ static GTY(()) struct cgraph_edge *free_edges; > /* Did procss_same_body_aliases run? */ > bool same_body_aliases_done; > > +/* Map a cgraph_node to cgraph_function_version_info using this htab. > + The cgraph_function_version_info has a THIS_NODE field that is the > + corresponding cgraph_node.. */ > +htab_t GTY((param_is (struct cgraph_function_version_info *))) > + cgraph_fnver_htab = NULL; I think you want declare the htab static and arrange it to be freed after cgraph construction, so you don't need to take care of nodes being removed via the hooks. OK with this change. I have few other comments: > + /* IFUNC resolvers have to be externally visible. */ > + TREE_PUBLIC (decl) = 1; > + DECL_UNINLINABLE (decl) = 1; Why the resolvers can not be inlined? > + > + DECL_EXTERNAL (decl) = 0; > + DECL_EXTERNAL (dispatch_decl) = 0; > + > + DECL_CONTEXT (decl) = NULL_TREE; > + DECL_INITIAL (decl) = make_node (BLOCK); > + DECL_STATIC_CONSTRUCTOR (decl) = 0; > + TREE_READONLY (decl) = 0; > + DECL_PURE_P (decl) = 0; I think those can be copied from the functions you are resolving. (well as well as many attributes and properties) > + > + if (DECL_COMDAT_GROUP (default_decl)) > +{ > + DECL_COMDAT (decl) = DECL_COMDAT (default_decl); > + make_decl_one_only (decl, DECL_COMDAT_GROUP (default_decl)); > +} > + else if (TREE_PUBLIC (default_decl)) > +{ > + /* In this case, each translation unit with a call to this > + versioned function will put out a resolver. Ensure it > + is comdat to keep just one copy. */ > + DECL_COMDAT (decl) = 1; > + make_decl_one_only (decl, DECL_ASSEMBLER_NAME (decl)); > +} > + /* Build result decl and add to function_decl. */ > + t = build_decl (UNKNOWN_LOCATION, RESULT_DECL, NULL_TREE, ptr_type_node); > + DECL_ARTIFICIAL (t) = 1; > + DECL_IGNORED_P (t) = 1; > + DECL_RESULT (decl) = t; > + > + gimplify_function_tree (decl); > + push_cfun (DECL_STRUCT_FUNCTION (decl)); > + gimple_register_cfg_hooks (); > + init_empty_tree_cfg_for_function (DECL_STRUCT_FUNCTION (decl)); > + cfun->curr_properties |= > +(PROP_gimple_lcf | PROP_gimple_leh | PROP_cfg | PROP_ssa > + | PROP_gimple_any); > + cfun->curr_properties = 15; > + new_bb = create_empty_bb (ENTRY_BLOCK_PTR); > + make_edge (ENTRY_BLOCK_PTR, new_bb, EDGE_FALLTHRU); > + make_edge (new_bb, EXIT_BLOCK_PTR, 0); > + *empty_bb = new_bb; You can simplify this by init_lowered_empty_function. Honza
Re: PATCH: Correct alloca length in dump_gimple_bb_header
On Mon, Oct 29, 2012 at 5:18 AM, Jakub Jelinek wrote: > On Mon, Oct 29, 2012 at 05:06:01AM -0700, H.J. Lu wrote: >> When indent == 0, we call alloca with -1 bytes. This patch changes >> it to indent + 1. This is a trunk only regression. OK to install? > >> 2012-10-29 H.J. Lu >> >> * gimple-pretty-print.c (dump_gimple_bb_header): Correct alloca >> length. >> >> diff --git a/gcc/gimple-pretty-print.c b/gcc/gimple-pretty-print.c >> index 4b3235e..62c315e 100644 >> --- a/gcc/gimple-pretty-print.c >> +++ b/gcc/gimple-pretty-print.c >> @@ -2093,7 +2093,7 @@ dump_gimple_bb_header (FILE *outf, basic_block bb, int >> indent, int flags) >>gimple stmt = first_stmt (bb); >>if (!stmt || gimple_code (stmt) != GIMPLE_LABEL) >> { >> - char *s_indent = (char *) alloca ((size_t) indent - 2 + 1); >> + char *s_indent = (char *) alloca ((size_t) indent + 1); >> memset (s_indent, ' ', (size_t) indent); >> s_indent[indent] = '\0'; >> fprintf (outf, "%s:\n", s_indent, bb->index); > > Can't you instead of all this just do > fprintf (outf, "%*s:\n", indent, "", bb->index); > ? > The whole function has another: char *s_indent = (char *) alloca ((size_t) indent + 1); memset (s_indent, ' ', (size_t) indent); s_indent[indent] = '\0'; We should make the same switch for both. -- H.J.
Re: [Patch] Remove _GLIBCXX_HAVE_BROKEN_VSWPRINTF from mingw32-w64/os_defines.h
Fix the mail subject line. On 10/29/2012 06:56, JonY wrote: > Hi, > > Workaround now exists for trunk mingw-w64 headers. > Kai approved over IRC, so anybody with commit rights please push. > > ChangeLog > 2012-10-29 Jonathan Yong > > * config/os/mingw32-w64/os_defines.h: Remove > _GLIBCXX_HAVE_BROKEN_VSWPRINTF > as no longer required. > > > > Index: libstdc++-v3/config/os/mingw32-w64/os_defines.h > === > --- libstdc++-v3/config/os/mingw32-w64/os_defines.h (revision 192802) > +++ libstdc++-v3/config/os/mingw32-w64/os_defines.h (working copy) > @@ -63,8 +63,9 @@ > // See libstdc++/20806. > #define _GLIBCXX_HAVE_DOS_BASED_FILESYSTEM 1 > > -// See libstdc++/37522. > -#define _GLIBCXX_HAVE_BROKEN_VSWPRINTF 1 > +// See libstdc++/37522. mingw-w64 stdio redirect for C++ > +// #define _GLIBCXX_HAVE_BROKEN_VSWPRINTF 1 > +// Workaround added for mingw-w64 trunk headers r5437 > > // See libstdc++/43738 > // On native windows targets there is no ioctl function. And the existing > signature.asc Description: OpenPGP digital signature
Re: [patch] Unify bitmap interface.
On Thu, Oct 25, 2012 at 6:39 PM, Lawrence Crowl wrote: > The sbitmap non-bool returning bitwise operations have been merged with > the bool versions. Sometimes this merge involved modifying the non-bool > version to compute the bool value, and sometimes modifying bool version to > add additional work from the non-bool version. The redundant routines have > been ifdef'd out. I will remove the ifdef'd out code later. No #if 0 code, please. Let's just remove them. > The allocation functions have not been renamed, because we often do not > have an argument on which to overload. Would it work if we made the first argument a reference to the bitmap being allocated? I suppose this shouldn't be a big deal. > The cardinality functions have not > been renamed, because they have different parameters, and are thus not > interchangable. Why not change the parameters then? > The iteration functions have not been renamed, because > they are functionally different. Functionally different? How? It seems like the patch only goes skin deep. I would rather make a true unification. If the only thing that remains the different are the allocation functions, that's not a big deal. But the point was to make everything else the same. I also notice that the patch does not rename any of the SET_BIT, RESET_BIT functions in sbitmap.c. > Tested on x86_64. Config testing in progress. You will also want to test on a couple other architecture on the farm. By config testing, do you mean contrib/config-list.mk? Diego.
PATCH: PR rtl-optimization/55093: [4.8 Regression] [x32] -maddress-mode=long failed
Hi, This patch changes get_elimination to check register number instead of RTX. Tested on Linux/x32 with -maddress-mode=long. OK to install? Thanks. H.J. --- gcc/ 2012-10-29 H.J. Lu PR rtl-optimization/55093 * lra-eliminations.c (get_elimination): Check register number instead of RTX. gcc/testsuite/ 2012-10-29 H.J. Lu PR rtl-optimization/55093 * gcc.target/i386/pr55093.c: New file. diff --git a/gcc/lra-eliminations.c b/gcc/lra-eliminations.c index d80..cbfbe7a 100644 --- a/gcc/lra-eliminations.c +++ b/gcc/lra-eliminations.c @@ -272,7 +272,7 @@ get_elimination (rtx reg) if ((hard_regno = REGNO (reg)) < 0 || hard_regno >= FIRST_PSEUDO_REGISTER) return NULL; if ((ep = elimination_map[hard_regno]) != NULL) -return ep->from_rtx != reg ? NULL : ep; +return ep->from != hard_regno ? NULL : ep; if ((offset = self_elim_offsets[hard_regno]) == 0) return NULL; /* This is an iteration to restore offsets just after HARD_REGNO diff --git a/gcc/testsuite/gcc.target/i386/pr55093.c b/gcc/testsuite/gcc.target/i386/pr55093.c new file mode 100644 index 000..76b4042 --- /dev/null +++ b/gcc/testsuite/gcc.target/i386/pr55093.c @@ -0,0 +1,80 @@ +/* { dg-do compile { target { ! { ia32 } } } } */ +/* { dg-options "-O2 -mx32 -maddress-mode=long" } */ + +typedef union tree_node *tree; +typedef const union tree_node *const_tree; +typedef struct { + unsigned long long low; + long long high; +} double_int; +struct real_value { +}; +struct real_format { + int has_signed_zero; +}; +extern const struct real_format * real_format_for_mode[]; +extern int real_isnegzero (const struct real_value *); +enum tree_code { REAL_CST, SSA_NAME }; +struct tree_base { + enum tree_code code : 16; + union { +unsigned int version; + } + u; +}; +extern void tree_check_failed (const_tree, const char *, int, const char *, ...) __attribute__ ((__noreturn__)); +union tree_node { + struct tree_base base; +}; +inline tree tree_check (tree __t, const char *__f, int __l, const char *__g, enum tree_code __c) { + if (((enum tree_code) (__t)->base.code) != __c) +tree_check_failed (__t, __f, __l, __g, __c, 0); + return __t; +} +struct prop_value_d { + int lattice_val; + tree value; + double_int mask; +}; +typedef struct prop_value_d prop_value_t; +static prop_value_t *const_val; +static void canonicalize_float_value (prop_value_t *); +typedef void (*ssa_prop_visit_stmt_fn) (prop_value_t); +typedef void (*ssa_prop_visit_phi_fn) (void); +typedef void (*ssa_prop_fold_stmt_fn) (void *gsi); +typedef void (*ssa_prop_get_value_fn) ( prop_value_t *val); +void ssa_propagate (ssa_prop_visit_stmt_fn, ssa_prop_visit_phi_fn); +int substitute_and_fold (ssa_prop_get_value_fn, ssa_prop_fold_stmt_fn); +void ccp_fold_stmt (void *); +static void get_constant_value (prop_value_t *val) { + canonicalize_float_value (val); +} +static void canonicalize_float_value (prop_value_t *val) { + int mode; + struct real_value d; + if (val->lattice_val != 1 + || ((enum tree_code) (val->value)->base.code) != REAL_CST) +return; + mode = val->lattice_val; + if (real_format_for_mode[mode]->has_signed_zero && real_isnegzero (&d)) +ccp_fold_stmt (0); +} +static void set_lattice_value (tree var, prop_value_t new_val) { + prop_value_t *old_val = &const_val[(tree_check ((var), "", + 0, "", + (SSA_NAME)))->base.u.version]; + canonicalize_float_value (&new_val); + canonicalize_float_value (old_val); +} +static void ccp_visit_phi_node (void) { + prop_value_t new_val; + set_lattice_value (0, new_val); +} +static void ccp_visit_stmt (prop_value_t v) { + set_lattice_value (0, v); +} +unsigned int do_ssa_ccp (void) { + ssa_propagate (ccp_visit_stmt, ccp_visit_phi_node); + substitute_and_fold (get_constant_value, ccp_fold_stmt); + return 0; +}
Re: [PATCH] Fix debug info for expr and jump stmt
Hi, On Fri, 26 Oct 2012, Dehao Chen wrote: > 1. abandon the changes in cfgexpand.c Well, you merely moved the bogus code to gimple-low.c. It is bogus because you unconditionally overwrite TREE_BLOCK of all operands (and all operands operands) with the statements block. I assume the unit-test you added shows the problem you were trying to fix? >From the scan-assembler-no directive I'm not really sure what exactly the problem is you're seeing. Can you try to describe it with words for the example code? Which operands has no tree-block where it should have one, or which one has the wrong tree-block? Ciao, Michael.
Re: Make inliner to predict &this->field to be optimized out
On Mon, Oct 29, 2012 at 1:03 AM, Jan Hubicka wrote: > Hi, > this patch makes optimizer to predict &this->field to be optimized out. > The main motivation for this is to make destructors that only calls destructor > of inner type to be inlined, even in the cold regions. > > Bootstrapped/regtested x86_64-linux. Comitted. > > 2012-10-28 Jan Hubicka > > * ipa-inline-analysis.c (eliminated_by_inlining_prob): Handle > &this->field expressions. > > Index: ipa-inline-analysis.c > === > --- ipa-inline-analysis.c (revision 192898) > +++ ipa-inline-analysis.c (working copy) > @@ -1567,6 +1567,7 @@ eliminated_by_inlining_prob (gimple stmt > if (gimple_assign_rhs_code (stmt) == CONVERT_EXPR > || gimple_assign_rhs_code (stmt) == NOP_EXPR > || gimple_assign_rhs_code (stmt) == VIEW_CONVERT_EXPR > + || gimple_assign_rhs_code (stmt) == ADDR_EXPR > || gimple_assign_rhs_class (stmt) == GIMPLE_SINGLE_RHS) Please cache gimple_assign_rhs_code somewhere ... > { > tree rhs = gimple_assign_rhs1 (stmt); > @@ -1584,6 +1585,20 @@ eliminated_by_inlining_prob (gimple stmt > /* Reads of parameter are expected to be free. */ > if (unmodified_parm (stmt, inner_rhs)) > rhs_free = true; > + /* Match expressions of form &this->field. Those will most likely > + combine with something upstream after inlining. */ > + else if (TREE_CODE (inner_rhs) == ADDR_EXPR) > + { > + tree op = get_base_address (TREE_OPERAND (inner_rhs, 0)); > + if (TREE_CODE (op) == PARM_DECL) > + rhs_free = true; > + else if (TREE_CODE (op) == MEM_REF) > + { > + op = get_base_address (TREE_OPERAND (op, 0)); Not necessary, just check TREE_CODE (op) == SSA_NAME && unmodif ... > + if (unmodified_parm (stmt, op)) > + rhs_free = true; > + } > + } > > /* When parameter is not SSA register because its address is taken >and it is just copied into one, the statement will be > completely
Re: [patch] Unify bitmap interface.
On Mon, Oct 29, 2012 at 2:06 PM, Diego Novillo wrote: > On Thu, Oct 25, 2012 at 6:39 PM, Lawrence Crowl wrote: > >> The sbitmap non-bool returning bitwise operations have been merged with >> the bool versions. Sometimes this merge involved modifying the non-bool >> version to compute the bool value, and sometimes modifying bool version to >> add additional work from the non-bool version. The redundant routines have >> been ifdef'd out. I will remove the ifdef'd out code later. > > No #if 0 code, please. Let's just remove them. > >> The allocation functions have not been renamed, because we often do not >> have an argument on which to overload. > > Would it work if we made the first argument a reference to the bitmap > being allocated? > I suppose this shouldn't be a big deal. It's definitely good to at least in one place see what kind of bitmap is actually being used ... ;) >> The cardinality functions have not >> been renamed, because they have different parameters, and are thus not >> interchangable. > > Why not change the parameters then? Agree, as a followup maybe. >> The iteration functions have not been renamed, because >> they are functionally different. > > Functionally different? How? It seems like the patch only goes skin > deep. I would rather make a true unification. If the only thing that > remains the different are the allocation functions, that's not a big > deal. But the point was to make everything else the same. Indeed. > I also notice that the patch does not rename any of the SET_BIT, > RESET_BIT functions in sbitmap.c. They should go - a followup is fine though. >> Tested on x86_64. Config testing in progress. > > You will also want to test on a couple other architecture on the farm. > By config testing, do you mean contrib/config-list.mk? Richard. > > Diego.
Re: real_zerop for vectors
On Sun, Oct 28, 2012 at 4:14 PM, Marc Glisse wrote: > Hello, > > this patch lets some predicates on floating point constants answer true for > vectors, so optimizations are applied. > > Tested bootstrap + testsuite (default languages). Ok. Bonus points if you quickly eyed users of whether they may be surprised in vectors coming through now. Thanks, Richard. > 2012-10-29 Marc Glisse > > PR middle-end/55027 > > gcc/ > * tree.c (real_zerop, real_onep, real_twop, real_minus_onep): > Handle VECTOR_CST. > > testsuite/ > * gcc.dg/pr55027.c: New testcase. > > -- > Marc Glisse > Index: gcc/testsuite/gcc.dg/pr55027.c > === > --- gcc/testsuite/gcc.dg/pr55027.c (revision 0) > +++ gcc/testsuite/gcc.dg/pr55027.c (revision 0) > @@ -0,0 +1,12 @@ > +/* { dg-do compile } */ > +/* { dg-options "-Ofast -fdump-tree-optimized-raw" } */ > + > +typedef double v2df __attribute__ ((__vector_size__ (2 * sizeof > (double; > + > +void f (v2df *x) > +{ > + *x = 0 + 1 * *x; > +} > + > +/* { dg-final { scan-tree-dump-not "gimple_assign" "optimized" } } */ > +/* { dg-final { cleanup-tree-dump "optimized" } } */ > > Property changes on: gcc/testsuite/gcc.dg/pr55027.c > ___ > Added: svn:keywords >+ Author Date Id Revision URL > Added: svn:eol-style >+ native > > Index: gcc/tree.c > === > --- gcc/tree.c (revision 192894) > +++ gcc/tree.c (working copy) > @@ -1985,75 +1985,127 @@ tree_floor_log2 (const_tree expr) > } > > /* Return 1 if EXPR is the real constant zero. Trailing zeroes matter for > decimal float constants, so don't return 1 for them. */ > > int > real_zerop (const_tree expr) > { >STRIP_NOPS (expr); > > - return ((TREE_CODE (expr) == REAL_CST > - && REAL_VALUES_EQUAL (TREE_REAL_CST (expr), dconst0) > - && !(DECIMAL_FLOAT_MODE_P (TYPE_MODE (TREE_TYPE (expr) > - || (TREE_CODE (expr) == COMPLEX_CST > - && real_zerop (TREE_REALPART (expr)) > - && real_zerop (TREE_IMAGPART (expr; > + switch (TREE_CODE (expr)) > +{ > +case REAL_CST: > + return REAL_VALUES_EQUAL (TREE_REAL_CST (expr), dconst0) > +&& !(DECIMAL_FLOAT_MODE_P (TYPE_MODE (TREE_TYPE (expr; > +case COMPLEX_CST: > + return real_zerop (TREE_REALPART (expr)) > +&& real_zerop (TREE_IMAGPART (expr)); > +case VECTOR_CST: > + { > + unsigned i; > + for (i = 0; i < VECTOR_CST_NELTS (expr); ++i) > + if (!real_zerop (VECTOR_CST_ELT (expr, i))) > + return false; > + return true; > + } > +default: > + return false; > +} > } > > /* Return 1 if EXPR is the real constant one in real or complex form. > Trailing zeroes matter for decimal float constants, so don't return > 1 for them. */ > > int > real_onep (const_tree expr) > { >STRIP_NOPS (expr); > > - return ((TREE_CODE (expr) == REAL_CST > - && REAL_VALUES_EQUAL (TREE_REAL_CST (expr), dconst1) > - && !(DECIMAL_FLOAT_MODE_P (TYPE_MODE (TREE_TYPE (expr) > - || (TREE_CODE (expr) == COMPLEX_CST > - && real_onep (TREE_REALPART (expr)) > - && real_zerop (TREE_IMAGPART (expr; > + switch (TREE_CODE (expr)) > +{ > +case REAL_CST: > + return REAL_VALUES_EQUAL (TREE_REAL_CST (expr), dconst1) > +&& !(DECIMAL_FLOAT_MODE_P (TYPE_MODE (TREE_TYPE (expr; > +case COMPLEX_CST: > + return real_onep (TREE_REALPART (expr)) > +&& real_zerop (TREE_IMAGPART (expr)); > +case VECTOR_CST: > + { > + unsigned i; > + for (i = 0; i < VECTOR_CST_NELTS (expr); ++i) > + if (!real_onep (VECTOR_CST_ELT (expr, i))) > + return false; > + return true; > + } > +default: > + return false; > +} > } > > /* Return 1 if EXPR is the real constant two. Trailing zeroes matter > for decimal float constants, so don't return 1 for them. */ > > int > real_twop (const_tree expr) > { >STRIP_NOPS (expr); > > - return ((TREE_CODE (expr) == REAL_CST > - && REAL_VALUES_EQUAL (TREE_REAL_CST (expr), dconst2) > - && !(DECIMAL_FLOAT_MODE_P (TYPE_MODE (TREE_TYPE (expr) > - || (TREE_CODE (expr) == COMPLEX_CST > - && real_twop (TREE_REALPART (expr)) > - && real_zerop (TREE_IMAGPART (expr; > + switch (TREE_CODE (expr)) > +{ > +case REAL_CST: > + return REAL_VALUES_EQUAL (TREE_REAL_CST (expr), dconst2) > +&& !(DECIMAL_FLOAT_MODE_P (TYPE_MODE (TREE_TYPE (expr; > +case COMPLEX_CST: > + return real_twop (TREE_REALPART (expr)) > +&& real_zerop (TREE_IMAGPART (expr)); > +case VECTOR_CST: > + { > + unsigned i; > + for (i = 0; i < VECTOR_CST_NELTS
Re: [PATCH] Fix CDDCE miscompilation (PR tree-optimization/55018)
On Sun, Oct 28, 2012 at 7:33 PM, Steven Bosscher wrote: > On Mon, Oct 22, 2012 at 11:09 PM, Jakub Jelinek wrote: >> On Mon, Oct 22, 2012 at 10:51:43PM +0200, Steven Bosscher wrote: >> Wouldn't it be way cheaper to just export dfs_find_deadend from cfganal.c >> and call it in calc_dfs_tree on each unconnected bb? >> I.e. (untested with the exception of the testcase): >> >> 2012-10-22 Jakub Jelinek >> >> PR tree-optimization/55018 >> * cfganal.c (dfs_find_deadend): No longer static. >> * basic-block.h (dfs_find_deadend): New prototype. >> * dominance.c (calc_dfs_tree): If saw_unconnected, >> traverse from dfs_find_deadend of unconnected b >> instead of b directly. >> >> * gcc.dg/torture/pr55018.c: New test. > > I have no better solution than this for the moment. I thought there > was a common DFS machinery in cfganal.c but there are actually many of > them, but unfortunately all doing things slightly different. Something > for the cleanup list for GCC 4.9... > > We should use dfs_find_deadend in flow_dfs_compute_reverse_execute > also. This results in fewer fake edges created in > connect_infinite_loops_to_exit, especially for loops with multiple > dead ends. > > (BTW, connect_infinite_loops_to_exit also connects other > reverse-unreachable points in the CFG to EXIT, so that calling > add_noreturn_fake_exit_edges and connect_infinite_loops_to_exit is > doing a bit of duplicate work -- another thing for the cleanup > list...) > > Attached patch was bootstrapped&tested on > {powerpc64,x86_64}-unknown-linux-gnu. OK? Ok. Thanks, Richard. > Ciao! > Steven
Re: [PATCH] Fix debug info for expr and jump stmt
On Mon, Oct 29, 2012 at 2:51 PM, Michael Matz wrote: > Hi, > > On Fri, 26 Oct 2012, Dehao Chen wrote: > >> 1. abandon the changes in cfgexpand.c > > Well, you merely moved the bogus code to gimple-low.c. It is bogus > because you unconditionally overwrite TREE_BLOCK of all operands (and all > operands operands) with the statements block. I assume the unit-test you > added shows the problem you were trying to fix? > > From the scan-assembler-no directive I'm not really sure what exactly the > problem is you're seeing. Can you try to describe it with words for the > example code? Which operands has no tree-block where it should have one, > or which one has the wrong tree-block? trees without block could be an issue when we extract them to some other statement (then without block), and move that to some random place. But the only issue should be that the stmt/expressions effective block becomes a different one (the currently "active" one during expansion). I don't see how we could end up generating too many block location DIEs because of this. Does the issue vanish when using -fno-tree-ter? And yes, I agree, the patch looks bogus. Richard. > > Ciao, > Michael.
Re: [PATCH] Fix debug info for expr and jump stmt
Hi, On Mon, 29 Oct 2012, Richard Biener wrote: > > Well, you merely moved the bogus code to gimple-low.c. It is bogus > > because you unconditionally overwrite TREE_BLOCK of all operands (and all > > operands operands) with the statements block. I assume the unit-test you > > added shows the problem you were trying to fix? > > > > From the scan-assembler-no directive I'm not really sure what exactly the > > problem is you're seeing. Can you try to describe it with words for the > > example code? Which operands has no tree-block where it should have one, > > or which one has the wrong tree-block? > > trees without block could be an issue when we extract them to some other > statement (then without block), and move that to some random place. Even then it would be either the frontends job to set the block, or the the job of the pass that introduced the new expression, when there is a clearly sane candidate for the block. > But the only issue should be that the stmt/expressions effective block > becomes a different one (the currently "active" one during expansion). Yep. > I don't see how we could end up generating too many block location > DIEs because of this. And even if, I don't see what's the _problem_ with too many block locations, except bloat. Ciao, Michael.
Re: [Ada] Ignore Optimize_Alignment (Space) for packed variable length record
Arnaud, After updating my tree to rev 192942 this morning, I'm getting a bootstrap failure with ada enabled: raised CONSTRAINT_ERROR : a-comlin.adb:65 explicit raise make[6]: *** [rts/s-oscons.ads] Error 1 make[6]: Leaving directory `bld-gcc/gcc/ada' make[5]: *** [gnatlib-shared-default] Error 2 I've configured with $ configure --enable-languages=all,ada,go,obj-c++ --enable-checking=release Does this look familiar? Thanks. Diego.
Re: [Ada] Ignore Optimize_Alignment (Space) for packed variable length record
> After updating my tree to rev 192942 this morning, I'm getting a > bootstrap failure with ada enabled: Looks like make did not rebuild (or too late?) xoscons. I'd suggest doing a rm -rf gcc/ada/bldtools from your obj directory and see if this fixes the error. If not, you might need to do a make -s -k clean Arno
Re: real_zerop for vectors
On Mon, 29 Oct 2012, Richard Biener wrote: On Sun, Oct 28, 2012 at 4:14 PM, Marc Glisse wrote: Hello, this patch lets some predicates on floating point constants answer true for vectors, so optimizations are applied. Tested bootstrap + testsuite (default languages). Ok. Bonus points if you quickly eyed users of whether they may be surprised in vectors coming through now. I did (that's why I didn't also change integer_twop), but I won't claim the bonus points, because in fold-const.c: * there is probably an issue with signed zero in fold_real_zero_addition_p * the RDIV_EXPR case would like real_zerop to mean "at least one is zero" * abs(x)>=0 or <0 doesn't seem ready but that's not much compared to the broken integer vector optimizations ;-) -- Marc Glisse
Re: [Ada] Ignore Optimize_Alignment (Space) for packed variable length record
On Mon, Oct 29, 2012 at 10:36 AM, Arnaud Charlet wrote: >> After updating my tree to rev 192942 this morning, I'm getting a >> bootstrap failure with ada enabled: > > Looks like make did not rebuild (or too late?) xoscons. > > I'd suggest doing a rm -rf gcc/ada/bldtools from your obj directory and > see if this fixes the error. > > If not, you might need to do a make -s -k clean Well, this was a build from scratch. Maybe there is a parallel make bug in the new ada changes? Some undeclared dependency? Diego.
Re: [Ada] Ignore Optimize_Alignment (Space) for packed variable length record
> Well, this was a build from scratch. Maybe there is a parallel make > bug in the new ada changes? Some undeclared dependency? There is not in the recent changes. Maybe there's a latent bug, although I have never experienced it. At which revision are you? Arno
Re: [Ada] Ignore Optimize_Alignment (Space) for packed variable length record
On Mon, Oct 29, 2012 at 10:39 AM, Arnaud Charlet wrote: >> Well, this was a build from scratch. Maybe there is a parallel make >> bug in the new ada changes? Some undeclared dependency? > > There is not in the recent changes. Maybe there's a latent bug, although I > have never experienced it. > > At which revision are you? rev 192942. Removing bld/gcc/ada/bldtools did not help. I'm trying a -j1 build after doing 'make clean'. Diego.
Re: [Ada] Ignore Optimize_Alignment (Space) for packed variable length record
> >> Well, this was a build from scratch. Maybe there is a parallel make > >> bug in the new ada changes? Some undeclared dependency? > > > > There is not in the recent changes. Maybe there's a latent bug, although I > > have never experienced it. > > > > At which revision are you? > > rev 192942. Can you confirm that gcc/ada/Make-generated.in contains the following line: ./xoscons s-oscons ) ; \ and NOT: ./xoscons ) ; \ Arno
Re: patch to fix PR55106
On 12-10-28 8:49 PM, H.J. Lu wrote: On Sun, Oct 28, 2012 at 5:43 PM, Vladimir Makarov wrote: The following patch fixes PR55106. A value in GENERAL_REGS is inherited into a move with destination pseudo of SSE_REGS. It results into secondary move for which inheritance is tried again an again. It means cycling LRA passes. The patch was successfully bootstrapped on x86/x86-64. Committed as rev. 192904. 2012-10-28 Vladimir Makarov PR rtl-optimization/55106 * lra-constraints.c (skip_usage_debug_insns): New function. (check_secondary_memory_needed_p): Ditto. (inherit_reload_reg): Use the new functions. Improve debug output. Please add the testcase at http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55106#c1 I've committed the following patch. 2012-10-29 Vladimir Makarov PR rtl-optimization/55106 * g++.dg/pr55106.C: New. Index: testsuite/g++.dg/pr55106.C === --- testsuite/g++.dg/pr55106.C (revision 0) +++ testsuite/g++.dg/pr55106.C (working copy) @@ -0,0 +1,67 @@ +/* { dg-do compile } */ +/* { dg-options "-c -O3" } */ +template struct A { + typedef _Tp *pointer; + typedef _Tp& reference; + typedef _Tp& const_reference; + templatestruct rebind + { +typedef A other; + }; +}; + +templatestruct __alloc_traits +{ + typedef typename _Alloc::pointer pointer; + typedef typename _Alloc::reference reference; + typedef typename _Alloc::const_reference const_reference; + templatestruct rebind + { +typedef typename _Alloc::template rebind<_Tp>::other other; + }; +}; +templatestruct B +{ + typedef typename __alloc_traits<_Alloc>::template rebind< + _Tp>::other _Tp_alloc_type; + typedef typename __alloc_traits<_Tp_alloc_type>::pointer pointer; + struct F + { +pointer _M_start; + }; + F _M_impl; +}; +template >class vec : B<_Tp, _Alloc>{ + typedef B<_Tp, _Alloc> _Base; + typedef typename _Base::_Tp_alloc_type _Tp_alloc_type; + typedef __alloc_traits<_Tp_alloc_type> _Alloc_traits; + +public: + typedef _Tp value_type; + typedef typename _Alloc_traits::reference reference; + typedef typename _Alloc_traits::const_reference const_reference; + reference operator[](int p1) + { +return *(this->_M_impl._M_start + p1); + } + + const_reference operator[](long) const; +}; + +int a[17]; +class C { + vec m_value; + void opModDivGuts(const C&); + int mostSetBitP1() const; +}; +void C::opModDivGuts(const C& p1) +{ + int b = p1.mostSetBitP1(), c = b + 1; + int d[16]; + + for (int i = c; i; i--) +a[i] = p1.m_value[i] << b; + + for (int i = 0; i < c; i++) +m_value[i] = d[i] >> b << -b; +}
Re: PATCH: PR rtl-optimization/55093: [4.8 Regression] [x32] -maddress-mode=long failed
On 12-10-29 9:06 AM, H.J. Lu wrote: Hi, This patch changes get_elimination to check register number instead of RTX. Tested on Linux/x32 with -maddress-mode=long. OK to install? Yes. Thanks, H.J.
Re: Trailing white spaces in LRA codes
On 12-10-28 11:03 PM, H.J. Lu wrote: Hi Vladimir, There are many trialling white spaces in LRA codes. This patch removes them. Thank you for pointing this out. I should be more careful with emacs macros using TAB. It would be nice if you install the patch.
Re: [Ada] Ignore Optimize_Alignment (Space) for packed variable length record
On Mon, Oct 29, 2012 at 10:44 AM, Arnaud Charlet wrote: >> >> Well, this was a build from scratch. Maybe there is a parallel make >> >> bug in the new ada changes? Some undeclared dependency? >> > >> > There is not in the recent changes. Maybe there's a latent bug, although I >> > have never experienced it. >> > >> > At which revision are you? >> >> rev 192942. > > Can you confirm that gcc/ada/Make-generated.in contains the following > line: > > ./xoscons s-oscons ) ; \ > > and NOT: > > ./xoscons ) ; \ > Yup, it does: 94 $(OSCONS_CPP) ; \ 95 $(OSCONS_EXTRACT) ; \ 96 ./xoscons s-oscons ) ; \ 97 $(MOVE_IF_CHANGE) $(ADA_GEN_SUBDIR)/bldtools/oscons/s-oscons.ads $(ADA_GEN_SUBDIR)/s-oscons.ads ; \ 98 $(MOVE_IF_CHANGE) $(ADA_GEN_SUBDIR)/bldtools/oscons/s-oscons.h $(ADA_GEN_SUBDIR)/s-oscons.h
Re: [Ada] Ignore Optimize_Alignment (Space) for packed variable length record
On Mon, Oct 29, 2012 at 10:48 AM, Diego Novillo wrote: > On Mon, Oct 29, 2012 at 10:44 AM, Arnaud Charlet wrote: >>> >> Well, this was a build from scratch. Maybe there is a parallel make >>> >> bug in the new ada changes? Some undeclared dependency? >>> > >>> > There is not in the recent changes. Maybe there's a latent bug, although I >>> > have never experienced it. >>> > >>> > At which revision are you? >>> >>> rev 192942. >> >> Can you confirm that gcc/ada/Make-generated.in contains the following >> line: >> >> ./xoscons s-oscons ) ; \ >> >> and NOT: >> >> ./xoscons ) ; \ >> > > Yup, it does: > > 94 $(OSCONS_CPP) ; \ > 95 $(OSCONS_EXTRACT) ; \ > 96 ./xoscons s-oscons ) ; \ > 97 $(MOVE_IF_CHANGE) > $(ADA_GEN_SUBDIR)/bldtools/oscons/s-oscons.ads > $(ADA_GEN_SUBDIR)/s-oscons.ads ; \ > 98 $(MOVE_IF_CHANGE) > $(ADA_GEN_SUBDIR)/bldtools/oscons/s-oscons.h > $(ADA_GEN_SUBDIR)/s-oscons.h
Re: [Ada] Ignore Optimize_Alignment (Space) for packed variable length record
> Yup, it does: OK then can you check the timestamp of ./gcc/ada/bldtools/oscons/xoscons vs e.g. ./gcc/ada/bldtools/oscons/xoscons/xoscons.adb and $(srcdir)/gcc/ada/xoscons.adb? Arno
Re: [PATCH 3/3] [asan] Instrument built-in memory access function calls
On Wed, 24 Oct 2012, Dodji Seketeli wrote: > Jakub Jelinek writes: > > On Tue, Oct 23, 2012 at 03:11:29PM +0200, Dodji Seketeli wrote: > > > * asan.c (insert_if_then_before_iter) > > > (instrument_mem_region_access) > > > (maybe_instrument_builtin_call, maybe_instrument_call): New static > > > > Why not just write it: > > * asan.c (insert_if_then_before_iter, instrument_mem_region_access, > > maybe_instrument_builtin_call, maybe_instrument_call): New static > > ? > > It's emacs that formats it like that automatically. I am not sure how > to teach him otherwise. I have fixed this as you want by doing it "by > hand". JFTR, the emacs-formatted-version *is* the correct one, at least when going by the GNU coding standard. Maybe something about being eager to balance parentheses as early as possible. :) But, I'll keep formatting it like that myself until someone updates the GCC coding standard. Hint, like. brgds, H-P
Re: Make inliner to predict &this->field to be optimized out
> > { > > tree rhs = gimple_assign_rhs1 (stmt); > > @@ -1584,6 +1585,20 @@ eliminated_by_inlining_prob (gimple stmt > > /* Reads of parameter are expected to be free. */ > > if (unmodified_parm (stmt, inner_rhs)) > > rhs_free = true; > > + /* Match expressions of form &this->field. Those will most > > likely > > + combine with something upstream after inlining. */ > > + else if (TREE_CODE (inner_rhs) == ADDR_EXPR) > > + { > > + tree op = get_base_address (TREE_OPERAND (inner_rhs, 0)); > > + if (TREE_CODE (op) == PARM_DECL) > > + rhs_free = true; > > + else if (TREE_CODE (op) == MEM_REF) > > + { > > + op = get_base_address (TREE_OPERAND (op, 0)); > > Not necessary, just check > > TREE_CODE (op) == SSA_NAME > && unmodif ... Hmm, right, I can't have non-SSA there. Will fix that, thanks! Honza
Re: PATCH: PR rtl-optimization/55093: [4.8 Regression] [x32] -maddress-mode=long failed
"H.J. Lu" writes: > Hi, > > This patch changes get_elimination to check register number instead of > RTX. Tested on Linux/x32 with -maddress-mode=long. OK to install? FWIW, this doesn't sound right to me, at least not without more justification. The idea is that things like frame_pointer_rtx are supposed to be unique, so the original code: >if ((ep = elimination_map[hard_regno]) != NULL) > -return ep->from_rtx != reg ? NULL : ep; > from != hard_regno ? NULL : ep; ought to be correct in itself. reload did the same thing: for (ep = reg_eliminate; ep < ®_eliminate[NUM_ELIMINABLE_REGS]; ep++) if (ep->from_rtx == x && ep->can_eliminate) return plus_constant (Pmode, ep->to_rtx, ep->previous_offset); It sounds on the face of it like the bug is elsewhere. Richard
Re: [RFC PATCH, i386]: Remove peephole2s for (subreg (operator (...)(...))) RTXes
Uros Bizjak writes: > On Sun, Oct 28, 2012 at 2:37 AM, H.J. Lu wrote: > As suggested by Richard S. [1], after the patch that converts subreg:M (op:N (...)(...)) to op:M (subreg:M (...) subreg:M (...)), we can remove several peephole2 patterns that handle subregs of PLUS, MINUS and MULT operators. I have attached RFC prototype patch that will trigger an ICE when to-be-removed pattern triggers, with the intention that these patterns wil be removed entirely (An "invalid" pattern was indeed generated elsewhere, see patch). > > I have committed following version that avoids all failures, reported by H.J.: > > 2012-10-29 Uros Bizjak > > * config/i386/i386.c (ix86_decompose_address): Use simplify_gen_subreg > to generate SImode equivalent of address, zero-extended with AND RTX. > * config/i386/i386.md (ashift to lea splitter): Split to SImode mult. > (simple lea to add/shift peephole2s): Remove peephole2s that operate > on subregs of DImode operations. > > Re-tested on x86_64-pc-linux-gnu {,-m32} and committed to mainline SVN. Thanks!
Re: [Ada] Ignore Optimize_Alignment (Space) for packed variable length record
On Mon, Oct 29, 2012 at 10:51 AM, Arnaud Charlet wrote: >> Yup, it does: > > OK then can you check the timestamp of ./gcc/ada/bldtools/oscons/xoscons > vs e.g. ./gcc/ada/bldtools/oscons/xoscons/xoscons.adb and > $(srcdir)/gcc/ada/xoscons.adb? I have no directory called gcc/ada/bldtools/oscons/xoscons. That's an executable. I suppose you meant gcc/ada/bldtools/oscons/xoscons.adb? $ ls -l gcc/ada/bldtools/oscons/xoscons gcc/ada/bldtools/oscons/xoscons.adb ~/gcc/clean/trunk/gcc/ada/xoscons.adb -rw-r- 1 dnovillo eng 17767 Oct 29 08:32 /home/dnovillo/gcc/clean/trunk/gcc/ada/xoscons.adb -rwxr-x--- 1 dnovillo eng 59793 Oct 29 10:38 gcc/ada/bldtools/oscons/xoscons* -rw-r- 1 dnovillo eng 17767 Oct 29 08:32 gcc/ada/bldtools/oscons/xoscons.adb Still happens with -j1 (after starting from scratch), so it's probably not a parallel make issue. We chatted about this on IRC, and Richi is seeing the same problem on his builds. Thanks. Diego.
Re: [PATCH] Fix debug info for expr and jump stmt
On Mon, Oct 29, 2012 at 7:17 AM, Michael Matz wrote: > Hi, > > On Mon, 29 Oct 2012, Richard Biener wrote: > >> > Well, you merely moved the bogus code to gimple-low.c. It is bogus >> > because you unconditionally overwrite TREE_BLOCK of all operands (and all Emm, then in gimple-low.c, we should probably not unconditionally overwrite gimple_block for stmts too? >> > operands operands) with the statements block. I assume the unit-test you >> > added shows the problem you were trying to fix? >> > >> > From the scan-assembler-no directive I'm not really sure what exactly the >> > problem is you're seeing. Can you try to describe it with words for the >> > example code? Which operands has no tree-block where it should have one, >> > or which one has the wrong tree-block? >> >> trees without block could be an issue when we extract them to some other >> statement (then without block), and move that to some random place. > > Even then it would be either the frontends job to set the block, or the > the job of the pass that introduced the new expression, when there is a > clearly sane candidate for the block. Please correct me if I'm wrong: front-end does not set blocks for either stmt/expr. lower_stmt is the first place that block is set for stmt, thus I think it should also be the first place to set block for expr. > >> But the only issue should be that the stmt/expressions effective block >> becomes a different one (the currently "active" one during expansion). I agree. Initially, both stmt and expressions is set to NULL block. Whenever we update the stmt's block, we should also update the expression's block, otherwise they will be inconsistent. > > Yep. > >> I don't see how we could end up generating too many block location >> DIEs because of this. For the unittest I added, all the assembly code guarded by "if" are should be within one lexical block, which is lock: LBB1 { asm1 asm2 asm3 asm4 } However, because some expression are with NULL lexical block, these assembly codes are separated by several discontinual lexical block which is like: LBB1 { asm1 } asm2 LBB2 { asm3 } asm4 > > And even if, I don't see what's the _problem_ with too many block > locations, except bloat. Yes, bloat is the major problem. Thanks, Dehao > > > Ciao, > Michael.
Re: [Ada] Ignore Optimize_Alignment (Space) for packed variable length record
> I have no directory called gcc/ada/bldtools/oscons/xoscons. That's an > executable. Yes, that's as expected, I asked you to check the timestamp of this file. > Still happens with -j1 (after starting from scratch), so it's probably > not a parallel make issue. Right, no surprise here. > We chatted about this on IRC, and Richi is seeing the same problem on > his builds. Very strange. I'm looking at it. Arno
Re: [PR54693] loss of debug info in jump threading and loop ivopts
On Oct 26, 2012, Jakub Jelinek wrote: > On Fri, Oct 26, 2012 at 04:30:41AM -0200, Alexandre Oliva wrote: >> From: Alexandre Oliva Fixed .git/config and the patches to use my @redhat.com address for these patches. >> +++ b/gcc/testsuite/gcc.dg/guality/pr54693.c > The #include looks useless to me, please remove it > and adjust gdb-test line accordingly. Thanks for catching this, it was a left-over from a failed experiment. > Don't we always ignore gimple_location on debug_stmts anyway? Right now, we mostly do, but they may become relevant once stmt frontier notes are implemented. >> + gimple def_temp = gimple_build_debug_bind (vexpr, comp, NULL); > Just wonder whether the above is sufficient It surely works. It may prevent common subexpression equivalences from being detected by VTA, but it also saves some temps. The exprs are of linear scaling, so they should be relatively small, but I can't guess whether there'd be any measurable benefit from splitting them up. Thanks for the reviews. -- Alexandre Oliva, freedom fighterhttp://FSFLA.org/~lxoliva/ You must be the change you wish to see in the world. -- Gandhi Be Free! -- http://FSFLA.org/ FSF Latin America board member Free Software Evangelist Red Hat Brazil Compiler Engineer
Re: [PATCH] Fix libbacktrace on 32-bit sparc
On Sat, Oct 27, 2012 at 10:12 PM, David Miller wrote: > From: David Miller > Date: Sun, 28 Oct 2012 00:31:27 -0400 (EDT) > >> The size is 24, and my patch definitely makes the crashes go away. >> >> It seems like a vector is being used for a mixed set of objects. >> I'll try to figure out how that is happening. > > Ok, the problem seems to have to do with releases. > > The releases place vector memory chunks into a global pool. > > So a memory chunk from a vector used for one type of object, > can be sucked into and used by another vector. > > But the alignment requirements are different, so we can > obtain a chunk from the freelist that was being used for > a vector of 4-byte aligned objects. > > The crash sequences are always of the form: > > vec_release(0xffb37ac8) base+size(0xf0199008) amount(312) > ... > vec_grow(0xffb37ac8:24) from 0x975168, ret=0xf01754cc [size(24):alc(360)] > > That size alignment done by backtrace_alloc() has no influence upon > this issue. Since chunks are released from wherever the vector's > allocation point was at the time of the release. > > In fact I bet that alignment in backtrace_alloc() never triggers when > it is invoked from backtrace_vector_grow(). Thanks for tracking it down. This patch should fix it. Bootstrapped and ran libbacktrace testsuite on x86_64-unknown-linux-gnu. Committed to mainline. Ian 2012-10-29 Ian Lance Taylor * mmap.c (backtrace_vector_release): Make sure freed block is aligned on 8-byte boundary. foo.patch Description: Binary data
Minor update for inlining for code size
Hi, this patch updates inlining functions called once into inlining functions called multiple times when the overall unit size for inlining everywhere shriks. This is now quite possible in the cases where inline predicates catch some DCE as seen in the silly testcase attached. We already caught most of instances of those as part of inlining small functions but in rarer cases where the function is too large we mis the oppurtunity. Bootstrapped/regtested x86_64-linux, will commit it shortly. Honza * gcc.dg/ipa/inline-6.c: New testcase. * ipa-inline.c (want_inline_function_called_once_p): Rename to ... (want_inline_function_to_all_callers_p): check also functions with multiple callers. (ipa_inline): Handle inlining for size into multiple callers. Index: ipa-inline.c === *** ipa-inline.c(revision 192937) --- ipa-inline.c(working copy) *** check_caller_edge (struct cgraph_node *n *** 681,714 } ! /* Decide if NODE is called once inlining it would eliminate need !for the offline copy of function. */ static bool ! want_inline_function_called_once_p (struct cgraph_node *node) { struct cgraph_node *function = cgraph_function_or_thunk_node (node, NULL); /* Already inlined? */ if (function->global.inlined_to) return false; !/* Zero or more then one callers? */ !if (!node->callers !|| node->callers->next_caller) return false; /* Maybe other aliases has more direct calls. */ if (cgraph_for_node_and_aliases (node, check_caller_edge, node->callers, true)) return false; !/* Recursive call makes no sense to inline. */ !if (cgraph_edge_recursive_p (node->callers)) ! return false; !/* External functions are not really in the unit, so inlining ! them when called once would just increase the program size. */ !if (DECL_EXTERNAL (function->symbol.decl)) ! return false; !/* Offline body must be optimized out. */ !if (!cgraph_will_be_removed_from_program_if_no_direct_calls (function)) ! return false; !if (!can_inline_edge_p (node->callers, true)) return false; return true; } --- 681,721 } ! /* Decide if inlining NODE would reduce unit size by eliminating !the offline copy of function. !When COLD is true the cold calls are considered, too. */ static bool ! want_inline_function_to_all_callers_p (struct cgraph_node *node, bool cold) { struct cgraph_node *function = cgraph_function_or_thunk_node (node, NULL); +struct cgraph_edge *e; +bool has_hot_call = false; + +/* Does it have callers? */ +if (!node->callers) + return false; /* Already inlined? */ if (function->global.inlined_to) return false; !if (cgraph_function_or_thunk_node (node, NULL) != node) ! return false; !/* Inlining into all callers would increase size? */ !if (estimate_growth (node) > 0) return false; /* Maybe other aliases has more direct calls. */ if (cgraph_for_node_and_aliases (node, check_caller_edge, node->callers, true)) return false; !/* All inlines must be possible. */ !for (e = node->callers; e; e = e->next_caller) ! { !if (!can_inline_edge_p (e, true)) ! return false; !if (!has_hot_call && cgraph_maybe_hot_edge_p (e)) !has_hot_call = 1; ! } ! !if (!cold && !has_hot_call) return false; return true; } *** ipa_inline (void) *** 1729,1742 symtab_remove_unreachable_nodes (true, dump_file); free (order); ! /* We already perform some inlining of functions called once during ! inlining small functions above. After unreachable nodes are removed, ! we still might do a quick check that nothing new is found. */ if (flag_inline_functions_called_once) { int cold; if (dump_file) ! fprintf (dump_file, "\nDeciding on functions called once:\n"); /* Inlining one function called once has good chance of preventing inlining other function into the same callee. Ideally we should --- 1736,1751 symtab_remove_unreachable_nodes (true, dump_file); free (order); ! /* Inline functions with a property that after inlining into all callers the ! code size will shrink because the out-of-line copy is eliminated. ! We do this regardless on the callee size as long as function growth limits ! are met. */ if (flag_inline_functions_called_once) { int cold; if (dump_file) ! fprintf (dump_file, !"\nDeciding on functions to be inlined into all callers:\n"); /* Inlining one function called once has good chance of preventing inlining other function into the same callee. Ideally we should *** ipa_inline (void) *** 1757,17
Re: [PATCH] gcc-{ar,nm,ranlib}: Find binutils binaries relative to self
Ping ^ 3. On 10/18/2012 10:30 AM, Meador Inge wrote: > Ping ^ 2. > > On 10/09/2012 09:44 PM, Meador Inge wrote: >> Ping. >> >> On 10/04/2012 03:45 PM, Meador Inge wrote: >>> Hi All, >>> >>> Currently the gcc-{ar,nm,ranlib} utilities assume that binutils is in >>> path when invoking the wrapped binutils program. This goes against the >>> accepted practice in GCC to find sub-programs relative to where the >>> GCC binaries are stored and to not make assumptions about the PATH. >>> >>> This patch changes the gcc-{ar,nm,ranlib} utilities to do the same >>> by factoring out some utility code for finding files from collect2.c. >>> These functions are then leveraged to find the binutils programs. >>> Note that similar code exist in gcc.c. Perhaps one day everything >>> can be merged to the file-find files. >>> >>> Tested for Windows and GNU/Linux hosts and i686-pc-linux-gnu and >>> arm-none-eabi targets. OK? >>> >>> P.S. I am not quite sure what is best for the copyrights and contributed >>> by comments in the file-find* files I added since that code was just moved. >>> This patch drops the contributed by and keeps all the copyright dates from >>> collect2.c. >>> >>> 2012-10-04 Meador Inge >>> >>> * collect2.c (main): Call find_file_set_debug. >>> (find_a_find, add_prefix, prefix_from_env, prefix_from_string): >>> Factor out into ... >>> * file-find.c (New file): ... here and ... >>> * file-find.h (New file): ... here. >>> * gcc-ar.c (standard_exec_prefix): New variable. >>> (standard_libexec_prefix): Ditto. >>> (tooldir_base_prefix) Ditto. >>> (self_exec_prefix): Ditto. >>> (self_libexec_prefix): Ditto. >>> (self_tooldir_prefix): Ditto. >>> (target_version): Ditto. >>> (path): Ditto. >>> (target_path): Ditto. >>> (setup_prefixes): New function. >>> (main): Rework how wrapped programs are found. >>> * Makefile.in (OBJS-libcommon-target): Add file-find.o. >>> (AR_OBJS): New variable. >>> (gcc-ar$(exeext)): Add dependency on $(AR_OBJS). >>> (gcc-nm$(exeext)): Ditto. >>> (gcc-ranlib(exeext)): Ditto. >>> (COLLECT2_OBJS): Add file-find.o. >>> (collect2.o): Add file-find.h prerequisite. >>> (file-find.o): New rule. >>> >>> Index: gcc/gcc-ar.c >>> === >>> --- gcc/gcc-ar.c(revision 192099) >>> +++ gcc/gcc-ar.c(working copy) >>> @@ -21,21 +21,110 @@ >>> #include "config.h" >>> #include "system.h" >>> #include "libiberty.h" >>> +#include "file-find.h" >>> >>> #ifndef PERSONALITY >>> #error "Please set personality" >>> #endif >>> >>> +/* The exec prefix as derived at compile-time from --prefix. */ >>> + >>> +static const char standard_exec_prefix[] = STANDARD_EXEC_PREFIX; >>> + >>> +/* The libexec prefix as derived at compile-time from --prefix. */ >>> + >>> static const char standard_libexec_prefix[] = STANDARD_LIBEXEC_PREFIX; >>> + >>> +/* The bindir prefix as derived at compile-time from --prefix. */ >>> + >>> static const char standard_bin_prefix[] = STANDARD_BINDIR_PREFIX; >>> -static const char *const target_machine = TARGET_MACHINE; >>> >>> +/* A relative path to be used in finding the location of tools >>> + relative to this program. */ >>> + >>> +static const char *const tooldir_base_prefix = TOOLDIR_BASE_PREFIX; >>> + >>> +/* The exec prefix as relocated from the location of this program. */ >>> + >>> +static const char *self_exec_prefix; >>> + >>> +/* The libexec prefix as relocated from the location of this program. */ >>> + >>> +static const char *self_libexec_prefix; >>> + >>> +/* The tools prefix as relocated from the location of this program. */ >>> + >>> +static const char *self_tooldir_prefix; >>> + >>> +/* The name of the machine that is being targeted. */ >>> + >>> +static const char *const target_machine = DEFAULT_TARGET_MACHINE; >>> + >>> +/* The target version. */ >>> + >>> +static const char *const target_version = DEFAULT_TARGET_VERSION; >>> + >>> +/* The collection of target specific path prefixes. */ >>> + >>> +static struct path_prefix target_path; >>> + >>> +/* The collection path prefixes. */ >>> + >>> +static struct path_prefix path; >>> + >>> +/* The directory separator. */ >>> + >>> static const char dir_separator[] = { DIR_SEPARATOR, 0 }; >>> >>> +static void >>> +setup_prefixes (const char *exec_path) >>> +{ >>> + const char *self; >>> + >>> + self = getenv ("GCC_EXEC_PREFIX"); >>> + if (!self) >>> +self = exec_path; >>> + else >>> +self = concat (self, "gcc-" PERSONALITY, NULL); >>> + >>> + /* Relocate the exec prefix. */ >>> + self_exec_prefix = make_relative_prefix (self, >>> + standard_bin_prefix, >>> + standard_exec_prefix); >>> + if (self_exec_prefix == NULL) >>> +self_exec_prefix = standard_exec_prefix; >>> + >>> + /* Relocate libexec prefix. */ >>> + self_libexec_prefix = make_relativ
RFA: patch to fix PR55116
H.J. in http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55116 reported an interesting address (and:DI (subreg:DI (plus:SI (ashift:SI (reg:SI 96 [ glob_vol_int.22 ]) (const_int 2 [0x2])) (symbol_ref:SI ("glob_vol_int_arr") 0x703c2720 glob_vol_int_arr>)) 0) (const_int 4294967295 [0x])) which can not be correctly extracted. Here `and' with `subreg' behaves as an address mutation. The following patch fixes the problem. Ok to commit, Richard? 2012-10-29 Vladimir Makarov PR middle-end/55116 * rtlanal.c (strip_address_mutation): Add SUBREG case. Index: rtlanal.c === --- rtlanal.c (revision 192942) +++ rtlanal.c (working copy) @@ -5459,6 +5459,11 @@ strip_address_mutations (rtx *loc, enum else if (code == AND && CONST_INT_P (XEXP (*loc, 1))) /* (and ... (const_int -X)) is used to align to X bytes. */ loc = &XEXP (*loc, 0); + else if (code == SUBREG + && ! REG_P (XEXP (*loc, 0)) && ! MEM_P (XEXP (*loc, 0))) + /* (subreg (operator ...) ...) usually inside and is used for + mode conversion too. */ + loc = &XEXP (*loc, 0); else return loc; if (outer_code)
Re: [PATCH] Fix debug info for expr and jump stmt
On Mon, Oct 29, 2012 at 4:25 PM, Dehao Chen wrote: > On Mon, Oct 29, 2012 at 7:17 AM, Michael Matz wrote: >> Hi, >> >> On Mon, 29 Oct 2012, Richard Biener wrote: >> >>> > Well, you merely moved the bogus code to gimple-low.c. It is bogus >>> > because you unconditionally overwrite TREE_BLOCK of all operands (and all > > Emm, then in gimple-low.c, we should probably not unconditionally > overwrite gimple_block for stmts too? gimple stmts have no block before gimple-low. >>> > operands operands) with the statements block. I assume the unit-test you >>> > added shows the problem you were trying to fix? >>> > >>> > From the scan-assembler-no directive I'm not really sure what exactly the >>> > problem is you're seeing. Can you try to describe it with words for the >>> > example code? Which operands has no tree-block where it should have one, >>> > or which one has the wrong tree-block? >>> >>> trees without block could be an issue when we extract them to some other >>> statement (then without block), and move that to some random place. >> >> Even then it would be either the frontends job to set the block, or the >> the job of the pass that introduced the new expression, when there is a >> clearly sane candidate for the block. > > Please correct me if I'm wrong: front-end does not set blocks for > either stmt/expr. lower_stmt is the first place that block is set for > stmt, thus I think it should also be the first place to set block for > expr. No, only the block on the gimple stmt matters for gimple. You should not need to touch the operands block. >> >>> But the only issue should be that the stmt/expressions effective block >>> becomes a different one (the currently "active" one during expansion). > > I agree. Initially, both stmt and expressions is set to NULL block. > Whenever we update the stmt's block, we should also update the > expression's block, otherwise they will be inconsistent. I don't think so. >> >> Yep. >> >>> I don't see how we could end up generating too many block location >>> DIEs because of this. > > For the unittest I added, all the assembly code guarded by "if" are > should be within one lexical block, which is lock: > > LBB1 > { > asm1 > asm2 > asm3 > asm4 > > } > > However, because some expression are with NULL lexical block, these > assembly codes are separated by several discontinual lexical block > which is like: > >LBB1 > { >asm1 > } > asm2 asm2 should have gone into the currently open scope (similiar for what we do for line number information). > LBB2 > { >asm3 > } > asm4 > > >> >> And even if, I don't see what's the _problem_ with too many block >> locations, except bloat. > > Yes, bloat is the major problem. > > Thanks, > Dehao > >> >> >> Ciao, >> Michael.
Re: [Ada] Ignore Optimize_Alignment (Space) for packed variable length record
> > We chatted about this on IRC, and Richi is seeing the same problem on > > his builds. > > Very strange. I'm looking at it. OK, this comes from code duplicated between gcc-interface/Makefile.in and Make-generated.in, pretty messy. Testing the patch below, will commit if success. 2012-10-29 Arnaud Charlet * gcc-interface/Makefile.in (s-oscons.ads): Adjust call to xoscons. Arno -- --- gcc-interface/Makefile.in (revision 192944) +++ gcc-interface/Makefile.in (working copy) @@ -2610,7 +2610,7 @@ $(RTSDIR)/s-oscons.ads: ../stamp-gnatlib (cd $(RTSDIR) ; \ $(OSCONS_CPP) ; \ $(OSCONS_EXTRACT) ; \ - ../bldtools/oscons/xoscons) + ../bldtools/oscons/xoscons s-oscons) # Don't use semicolon separated shell commands that involve list expansions. # The semicolon triggers a call to DCL on VMS and DCL can't handle command
Re: RFA: patch to fix PR55116
Vladimir Makarov writes: >H.J. in > > http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55116 > >reported an interesting address > > (and:DI (subreg:DI (plus:SI (ashift:SI (reg:SI 96 [ glob_vol_int.22 ]) > (const_int 2 [0x2])) > (symbol_ref:SI ("glob_vol_int_arr") 0x703c2720 glob_vol_int_arr>)) 0) > (const_int 4294967295 [0x])) > >which can not be correctly extracted. Here `and' with `subreg' > behaves as an address mutation. > >The following patch fixes the problem. > > Ok to commit, Richard? Heh, I wondered if subregs might still be used like that, and was almost tempted to add them just in case. I think this particular case is really a failed canonicalisation and that: (and:DI (subreg:DI (foo:SI ...) 0) (const_int 0x)) ought to be: (zero_extend:DI (foo:SI ...)) But I know I've approved MIPS patches to accept (and:DI ... (const_int 0x)) as an alternative. > Index: rtlanal.c > === > --- rtlanal.c (revision 192942) > +++ rtlanal.c (working copy) > @@ -5459,6 +5459,11 @@ strip_address_mutations (rtx *loc, enum > else if (code == AND && CONST_INT_P (XEXP (*loc, 1))) > /* (and ... (const_int -X)) is used to align to X bytes. */ > loc = &XEXP (*loc, 0); > + else if (code == SUBREG > + && ! REG_P (XEXP (*loc, 0)) && ! MEM_P (XEXP (*loc, 0))) > + /* (subreg (operator ...) ...) usually inside and is used for > + mode conversion too. */ > + loc = &XEXP (*loc, 0); I think the condition should be: else if (code == SUBREG && !OBJECT_P (SUBREG_REG (*loc)) && subreg_lowpart (*loc)) OK with that change, if it works. Thanks for fixing this. Richard
Re: [Ada] Ignore Optimize_Alignment (Space) for packed variable length record
> Testing the patch below, will commit if success. > > 2012-10-29 Arnaud Charlet > >* gcc-interface/Makefile.in (s-oscons.ads): Adjust call to >xoscons. Committed on trunk now. Sorry for the breakage, my build was incremental and missed this missing part. Arno
Re: RFA: patch to fix PR55116
Sorry, forgot a line: Richard Sandiford writes: > Vladimir Makarov writes: >> Index: rtlanal.c >> === >> --- rtlanal.c (revision 192942) >> +++ rtlanal.c (working copy) >> @@ -5459,6 +5459,11 @@ strip_address_mutations (rtx *loc, enum >> else if (code == AND && CONST_INT_P (XEXP (*loc, 1))) >> /* (and ... (const_int -X)) is used to align to X bytes. */ >> loc = &XEXP (*loc, 0); >> + else if (code == SUBREG >> + && ! REG_P (XEXP (*loc, 0)) && ! MEM_P (XEXP (*loc, 0))) >> + /* (subreg (operator ...) ...) usually inside and is used for >> + mode conversion too. */ >> + loc = &XEXP (*loc, 0); > > I think the condition should be: > > else if (code == SUBREG >&& !OBJECT_P (SUBREG_REG (*loc)) >&& subreg_lowpart (*loc)) loc = &SUBREG_REG (*loc); i.e. please use SUBREG_REG rather than XEXP.
Re: Fix estimation of array accesses
> ICK ... > > Why not sth as simple as > > return num_ssa_operands (stmt, SSA_OP_USE); > > ? a[1][2] and b[2] really have the same cost, variable length > objects have extra SSA operands in ARRAY_REF/COMPONENT_REF for > the size. Thus, stmt cost somehow should reflect the number > of dependent stmts. > > So in estimate_num_insns I'd try > > int > estimate_num_insns (gimple stmt, eni_weights *weights) > { > unsigned cost, i; > enum gimple_code code = gimple_code (stmt); > tree lhs; > tree rhs; > > switch (code) > { >case GIMPLE_ASSIGN: > /* Initialize with the number of SSA uses, one is free. */ > cost = num_ssa_operands (stmt, SSA_OP_USE); > if (cost > 1) > --cost; Hi, this is the udpated patch I am testing after today discussion. I decided to drop the ipa-inline-analysis changes and do that incrementally. So the patch now trashes tramp3d performance by increasing need for early-inlining-insns, but it is not inexpected. The patch also fixes accounting of addr expr that got previously confused with a load. Does this seem sane? * tree-inline.c (estimate_operator_cost): Return 1 for non-trivial ADDR_EXPR operations. (estimate_num_insns): Do not confuse general single rhs with loads; account cost of non-trivial addr_expr for ASSIGN_EXPR, GIMPLE_RETURN and GIMPLE_ASM Index: tree-inline.c === --- tree-inline.c (revision 192945) +++ tree-inline.c (working copy) @@ -3447,6 +3447,19 @@ estimate_operator_cost (enum tree_code c if (TREE_CODE (op2) != INTEGER_CST) return weights->div_mod_cost; return 1; +case ADDR_EXPR: + { +tree addr_base; +HOST_WIDE_INT addr_offset; + + addr_base = get_addr_base_and_unit_offset (TREE_OPERAND (op1, 0), + &addr_offset); + /* If the offset is variable or with non-zero offset, return 2. */ + if (!addr_base || addr_offset != 0 || TREE_CODE (addr_base) != MEM_REF + || !integer_zerop (TREE_OPERAND (addr_base, 1))) + return 1; + } + return 0; default: /* We expect a copy assignment with no operator. */ @@ -3512,14 +3525,24 @@ estimate_num_insns (gimple stmt, eni_wei lhs = gimple_assign_lhs (stmt); rhs = gimple_assign_rhs1 (stmt); - if (is_gimple_reg (lhs)) - cost = 0; - else + /* Store. */ + if (gimple_vdef (stmt)) cost = estimate_move_cost (TREE_TYPE (lhs)); + else + cost = 0; - if (!is_gimple_reg (rhs) && !is_gimple_min_invariant (rhs)) + /* Load. */ + if (gimple_vuse (stmt)) cost += estimate_move_cost (TREE_TYPE (rhs)); + /* Stores, loads and address expressions may have variable array +references in them. Account these. */ + if (gimple_vuse (stmt)) + cost += num_ssa_operands (stmt, SSA_OP_USE); + else if (gimple_vdef (stmt) + || gimple_assign_rhs_code (stmt) == ADDR_EXPR) + cost += MAX (0, (int)num_ssa_operands (stmt, SSA_OP_USE) - 1); + cost += estimate_operator_cost (gimple_assign_rhs_code (stmt), weights, gimple_assign_rhs1 (stmt), get_gimple_rhs_class (gimple_assign_rhs_code (stmt)) @@ -3596,6 +3619,7 @@ estimate_num_insns (gimple stmt, eni_wei case GIMPLE_RETURN: return weights->return_cost; ++ MAX (0, (int)num_ssa_operands (stmt, SSA_OP_USE) - 1); case GIMPLE_GOTO: case GIMPLE_LABEL: @@ -3606,7 +3630,10 @@ estimate_num_insns (gimple stmt, eni_wei return 0; case GIMPLE_ASM: - return asm_str_count (gimple_asm_string (stmt)); + return (asm_str_count (gimple_asm_string (stmt)) + /* Account also possible non-trivial addr_exprs +in the arguments. */ + + num_ssa_operands (stmt, SSA_OP_USE)); case GIMPLE_RESX: /* This is either going to be an external function call with one
Re: [PATCH] Fix debug info for expr and jump stmt
Hi, On Mon, 29 Oct 2012, Dehao Chen wrote: > On Mon, Oct 29, 2012 at 7:17 AM, Michael Matz wrote: > > Hi, > > > > On Mon, 29 Oct 2012, Richard Biener wrote: > > > >> > Well, you merely moved the bogus code to gimple-low.c. It is bogus > >> > because you unconditionally overwrite TREE_BLOCK of all operands (and all > > Emm, then in gimple-low.c, we should probably not unconditionally > overwrite gimple_block for stmts too? That's okay, because the gimple statements were just generated during gimplification. > >> trees without block could be an issue when we extract them to some > >> other statement (then without block), and move that to some random > >> place. > > > > Even then it would be either the frontends job to set the block, or > > the the job of the pass that introduced the new expression, when there > > is a clearly sane candidate for the block. > > Please correct me if I'm wrong: front-end does not set blocks for > either stmt/expr. Yes, sorry, you're right. It's the lowering of bind expressions that tacks blocks to statements. > lower_stmt is the first place that block is set for stmt, thus I think > it should also be the first place to set block for expr. The point is, there shouldn't be any expressions left after gimplification that would receive locations/blocks. Every side-effect of computing an expression should be a separate gimple statement. The only exception should be loads for arguments of call and asm statements statements and there setting the location of those expressions to the one of the call statement itself is superfluous. > >> But the only issue should be that the stmt/expressions effective block > >> becomes a different one (the currently "active" one during expansion). > > I agree. Initially, both stmt and expressions is set to NULL block. > Whenever we update the stmt's block, we should also update the > expression's block, otherwise they will be inconsistent. No, we should not. Basically the thing to keep in mind is that the location/block of expressions after gimplification is supposed to be meaningless. The gimple statements are the anchors for locations. Everything should be made to work in this model, not by tacking locations to expression. But see below... > >> I don't see how we could end up generating too many block location > >> DIEs because of this. > > For the unittest I added, all the assembly code guarded by "if" are > should be within one lexical block, which is lock: > > LBB1 > { > asm1 > asm2 > asm3 > asm4 > > } > > However, because some expression are with NULL lexical block, these > assembly codes are separated by several discontinual lexical block > which is like: > >LBB1 > { >asm1 > } > asm2 > LBB2 > { >asm3 > } > asm4 > Hmm, I asked about specifics for this testcase. Let me find out myself... You're talking about this situation: .LBB4: # code for line 20 A .LBE4: .loc 1 22 0 movq-16(%rbp), %rax # key, tmp82 .LBB5: # code for line 22/23 .LBE5: movq-16(%rbp), %rax # key, tmp83 .LBB6: .LBE6 where the reload of 'key' is splitting the lexical block? This comes from _10 = key_3->rulesToParseHdl; Where the MEM_REF of *key_3 does have a location (hence locus), but no block associated: (gdb) p expand_location(((tree)0x762252d0)->exp.locus) $12 = {file = 0x7fffe38e "blabla.C", line = 22, column = 29, data = 0x0, sysp = false} whereas the statement has the same location, but is associated with a block: (gdb) p expand_location(curr_location ) $14 = {file = 0x7fffe38e "blabla.C", line = 22, column = 29, data = 0x760cf410, sysp = false} This inconsistency is indeed generated by lower_stmt, which inserts the block into this locator for the statements, but not for operands. The issue is that tree->RTL expand listens to what's stored in the EXPR_LOCATION, instead of taking the location info from curr_location (which itself comes from the currently expanded gimple statement). The patch/hack below solves this, and probably also your problem. It might introduce others, but I think we should work towards making this patch possible. I.e. follow the above layed out model, that after gimplification locators are _only_ in gimple statements, nowhere else (though I'm not sure what to do about constructs that don't generate statements, like simple global data constructors). I'm not sure how realistic reaching this goal for 4.8 is. If it isn't then I fear something along the lines of your hack is necessary. Ciao, Michael. - Index: expr.c === --- expr.c (revision 192809) +++ expr.c (working copy) @@ -5030,7 +5030,7 @@ store_expr (tree exp, rtx target, int ca { rtx temp; rtx alt_rtl = NULL_RTX; - location_t loc = EXPR_LOCATION (exp); + loca
Re: RFA: patch to fix PR55116
On 12-10-29 12:21 PM, Richard Sandiford wrote: Vladimir Makarov writes: H.J. in http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55116 reported an interesting address (and:DI (subreg:DI (plus:SI (ashift:SI (reg:SI 96 [ glob_vol_int.22 ]) (const_int 2 [0x2])) (symbol_ref:SI ("glob_vol_int_arr") )) 0) (const_int 4294967295 [0x])) which can not be correctly extracted. Here `and' with `subreg' behaves as an address mutation. The following patch fixes the problem. Ok to commit, Richard? Heh, I wondered if subregs might still be used like that, and was almost tempted to add them just in case. I think this particular case is really a failed canonicalisation and that: (and:DI (subreg:DI (foo:SI ...) 0) (const_int 0x)) ought to be: (zero_extend:DI (foo:SI ...)) Yes, that was my thought too. But I know I've approved MIPS patches to accept (and:DI ... (const_int 0x)) as an alternative. Index: rtlanal.c === --- rtlanal.c (revision 192942) +++ rtlanal.c (working copy) @@ -5459,6 +5459,11 @@ strip_address_mutations (rtx *loc, enum else if (code == AND && CONST_INT_P (XEXP (*loc, 1))) /* (and ... (const_int -X)) is used to align to X bytes. */ loc = &XEXP (*loc, 0); + else if (code == SUBREG + && ! REG_P (XEXP (*loc, 0)) && ! MEM_P (XEXP (*loc, 0))) + /* (subreg (operator ...) ...) usually inside and is used for + mode conversion too. */ + loc = &XEXP (*loc, 0); I think the condition should be: else if (code == SUBREG && !OBJECT_P (SUBREG_REG (*loc)) && subreg_lowpart (*loc)) OK with that change, if it works. Yes, it works. I've submitted the following patch. Index: rtlanal.c === --- rtlanal.c (revision 192942) +++ rtlanal.c (working copy) @@ -5459,6 +5459,12 @@ strip_address_mutations (rtx *loc, enum else if (code == AND && CONST_INT_P (XEXP (*loc, 1))) /* (and ... (const_int -X)) is used to align to X bytes. */ loc = &XEXP (*loc, 0); + else if (code == SUBREG + && !OBJECT_P (SUBREG_REG (*loc)) + && subreg_lowpart_p (*loc)) + /* (subreg (operator ...) ...) inside and is used for mode + conversion too. */ + loc = &XEXP (*loc, 0); else return loc; if (outer_code)
Re: RFA: patch to fix PR55116
On 12-10-29 12:30 PM, Richard Sandiford wrote: Sorry, forgot a line: i.e. please use SUBREG_REG rather than XEXP. I've just fixed it.
[PATCH, i386]: Simplify AND-extended address decomposition
Hello! Since we use simplify_gen_subreg on address part, we can use it for DImode subreg of SImode address as well. simplify_gen_subreg knows how to strip subreg. 2012-10-29 Uros Bizjak * config/i386/i386.c (ix86_decompose_address): Use simplify_gen_subreg for all addresses, zero-extended with AND. Tested on x86_64-pc-linux-gnu {,-m32}, committed to mainline SVN. Uros. Index: config/i386/i386.c === --- config/i386/i386.c (revision 192933) +++ config/i386/i386.c (working copy) @@ -11810,23 +11810,11 @@ ix86_decompose_address (rtx addr, struct ix86_addr else if (GET_CODE (addr) == AND && const_32bit_mask (XEXP (addr, 1), DImode)) { - addr = XEXP (addr, 0); + addr = simplify_gen_subreg (SImode, XEXP (addr, 0), DImode, 0); + if (addr == NULL_RTX) + return 0; - /* Adjust SUBREGs. */ - if (GET_CODE (addr) == SUBREG - && GET_MODE (SUBREG_REG (addr)) == SImode) - { - addr = SUBREG_REG (addr); - if (CONST_INT_P (addr)) - return 0; - } - else if (GET_MODE (addr) == DImode) - { - addr = simplify_gen_subreg (SImode, addr, DImode, 0); - if (addr == NULL_RTX) - return 0; - } - else if (GET_MODE (addr) != VOIDmode) + if (CONST_INT_P (addr)) return 0; } }
Re: [PATCH] Fix debug info for expr and jump stmt
On Mon, Oct 29, 2012 at 9:10 AM, Richard Biener wrote: > On Mon, Oct 29, 2012 at 4:25 PM, Dehao Chen wrote: >> On Mon, Oct 29, 2012 at 7:17 AM, Michael Matz wrote: >>> Hi, >>> >>> On Mon, 29 Oct 2012, Richard Biener wrote: >>> > Well, you merely moved the bogus code to gimple-low.c. It is bogus > because you unconditionally overwrite TREE_BLOCK of all operands (and all >> >> Emm, then in gimple-low.c, we should probably not unconditionally >> overwrite gimple_block for stmts too? > > gimple stmts have no block before gimple-low. > > operands operands) with the statements block. I assume the unit-test you > added shows the problem you were trying to fix? > > From the scan-assembler-no directive I'm not really sure what exactly the > problem is you're seeing. Can you try to describe it with words for the > example code? Which operands has no tree-block where it should have one, > or which one has the wrong tree-block? trees without block could be an issue when we extract them to some other statement (then without block), and move that to some random place. >>> >>> Even then it would be either the frontends job to set the block, or the >>> the job of the pass that introduced the new expression, when there is a >>> clearly sane candidate for the block. >> >> Please correct me if I'm wrong: front-end does not set blocks for >> either stmt/expr. lower_stmt is the first place that block is set for >> stmt, thus I think it should also be the first place to set block for >> expr. > > No, only the block on the gimple stmt matters for gimple. You should not > need to touch the operands block. > >>> But the only issue should be that the stmt/expressions effective block becomes a different one (the currently "active" one during expansion). >> >> I agree. Initially, both stmt and expressions is set to NULL block. >> Whenever we update the stmt's block, we should also update the >> expression's block, otherwise they will be inconsistent. > > I don't think so. Hi, Richard, Can you share some insights why you don't think we want to make stmt and expr's block consistent? > >>> >>> Yep. >>> I don't see how we could end up generating too many block location DIEs because of this. >> >> For the unittest I added, all the assembly code guarded by "if" are >> should be within one lexical block, which is lock: >> >> LBB1 >> { >> asm1 >> asm2 >> asm3 >> asm4 >> >> } >> >> However, because some expression are with NULL lexical block, these >> assembly codes are separated by several discontinual lexical block >> which is like: >> >>LBB1 >> { >>asm1 >> } >> asm2 > > asm2 should have gone into the currently open scope (similiar for what > we do for line number information). In location_block patch, we changed this not to fall into the currently open scope, because we want every insn to have correct location/block with it. This is one of the reasons why we introduced the location_block patch: it makes it easy to update block for stmt/expr/phi_arg thus keeping a consistent location/block pair for all instructions becomes possible. Yes, let location/block fall into the currently open scope/location was a workaround before, but if we are able to make the location/block pair consistent all through compilation, we should do that, right? Thanks, Dehao > >> LBB2 >> { >>asm3 >> } >> asm4 >> >> >>> >>> And even if, I don't see what's the _problem_ with too many block >>> locations, except bloat. >> >> Yes, bloat is the major problem. >> >> Thanks, >> Dehao >> >>> >>> >>> Ciao, >>> Michael.
Parts 3 and 4 to the vxworks/fixincludes patches
The first two patches I've applied. The remaining two are needed to fully enable building the VxWorks flavor of GCC, but those bits affect parts outside of fixincludes and there is some breakage somewhere. All evidence seems to me to show fixincludes still doing its thing correctly, but somewhere along the line the build becomes confused and unable to find stdarg.h: > echo timestamp > stmp-int-hdrs > g++ -c -g -DIN_GCC -fno-exceptions -fno-rtti -fasynchronous-unwind-tables > \ > -W -Wall -Wno-narrowing -Wwrite-strings -Wcast-qual > -Wmissing-format-attribute \ > -pedantic -Wno-long-long -Wno-variadic-macros -Wno-overlength-strings > -fno-common \ > -DHAVE_CONFIG_H -I. -Ilto -I../.././gcc -I../.././gcc/lto > -I../.././gcc/../include \ > -I../.././gcc/../libcpp/include -I../.././gcc/../libdecnumber \ > -I../.././gcc/../libdecnumber/bid -I../libdecnumber > -I../.././gcc/../libbacktrace \ >../.././gcc/lto/lto-lang.c -o lto/lto-lang.o > In file included from ../.././gcc/lto/lto-lang.c:22:0: > ../.././gcc/system.h:28:20: fatal error: stdarg.h: No such file or directory > #include > ^ > compilation terminated. I have not been able to run down the cause. Until I've found it, I'm holding back on the bits that change stuff outside of fixincludes: > gcc/gcov-io.c > libstdc++-v3/config/os/vxworks/os_defines.h > configure.ac I hope someone knows what this is.
Re: [Ada] Ignore Optimize_Alignment (Space) for packed variable length record
On Mon, Oct 29, 2012 at 12:29 PM, Arnaud Charlet wrote: > Committed on trunk now. Sorry for the breakage, my build was incremental > and missed this missing part. Great. Thanks for the quick fix! Diego.
Re: [PATCH] Fix debug info for expr and jump stmt
Hi, Micheal, Thanks for explaining the design principle of debug info with gimple, now I can understand your concerns. And thanks for providing the patch. However, in some places after gimplification (e.g. tree-inline.c), we still updates the block/location info. And EXPR_LOCATION is still used widely after gimplification. Do you mean that in the long run, we'd want to remove all these? Thanks, Dehao On Mon, Oct 29, 2012 at 9:49 AM, Dehao Chen wrote: > On Mon, Oct 29, 2012 at 9:10 AM, Richard Biener > wrote: >> On Mon, Oct 29, 2012 at 4:25 PM, Dehao Chen wrote: >>> On Mon, Oct 29, 2012 at 7:17 AM, Michael Matz wrote: Hi, On Mon, 29 Oct 2012, Richard Biener wrote: > > Well, you merely moved the bogus code to gimple-low.c. It is bogus > > because you unconditionally overwrite TREE_BLOCK of all operands (and > > all >>> >>> Emm, then in gimple-low.c, we should probably not unconditionally >>> overwrite gimple_block for stmts too? >> >> gimple stmts have no block before gimple-low. >> > > operands operands) with the statements block. I assume the unit-test > > you > > added shows the problem you were trying to fix? > > > > From the scan-assembler-no directive I'm not really sure what exactly > > the > > problem is you're seeing. Can you try to describe it with words for the > > example code? Which operands has no tree-block where it should have > > one, > > or which one has the wrong tree-block? > > trees without block could be an issue when we extract them to some other > statement (then without block), and move that to some random place. Even then it would be either the frontends job to set the block, or the the job of the pass that introduced the new expression, when there is a clearly sane candidate for the block. >>> >>> Please correct me if I'm wrong: front-end does not set blocks for >>> either stmt/expr. lower_stmt is the first place that block is set for >>> stmt, thus I think it should also be the first place to set block for >>> expr. >> >> No, only the block on the gimple stmt matters for gimple. You should not >> need to touch the operands block. >> > But the only issue should be that the stmt/expressions effective block > becomes a different one (the currently "active" one during expansion). >>> >>> I agree. Initially, both stmt and expressions is set to NULL block. >>> Whenever we update the stmt's block, we should also update the >>> expression's block, otherwise they will be inconsistent. >> >> I don't think so. > > Hi, Richard, > > Can you share some insights why you don't think we want to make stmt > and expr's block consistent? > >> Yep. > I don't see how we could end up generating too many block location > DIEs because of this. >>> >>> For the unittest I added, all the assembly code guarded by "if" are >>> should be within one lexical block, which is lock: >>> >>> LBB1 >>> { >>> asm1 >>> asm2 >>> asm3 >>> asm4 >>> >>> } >>> >>> However, because some expression are with NULL lexical block, these >>> assembly codes are separated by several discontinual lexical block >>> which is like: >>> >>>LBB1 >>> { >>>asm1 >>> } >>> asm2 >> >> asm2 should have gone into the currently open scope (similiar for what >> we do for line number information). > > In location_block patch, we changed this not to fall into the > currently open scope, because we want every insn to have correct > location/block with it. This is one of the reasons why we introduced > the location_block patch: it makes it easy to update block for > stmt/expr/phi_arg thus keeping a consistent location/block pair for > all instructions becomes possible. > > Yes, let location/block fall into the currently open scope/location > was a workaround before, but if we are able to make the location/block > pair consistent all through compilation, we should do that, right? > > Thanks, > Dehao > >> >>> LBB2 >>> { >>>asm3 >>> } >>> asm4 >>> >>> And even if, I don't see what's the _problem_ with too many block locations, except bloat. >>> >>> Yes, bloat is the major problem. >>> >>> Thanks, >>> Dehao >>> Ciao, Michael.
Re: Ping: [PATCH] Install error handler for out-of-memory when using STL containers
On Oct 29, 2012, at 2:02 AM, Paul Richard Thomas wrote: > It looks straightforward to me but I am not convinced that a fortran > maintainer should be giving the green light on this :-) Well, seems reasonable to me as well… I'd let Diego or Jason approve it. :-)
Re: RFA: patch to fix PR55116
On 12-10-29 12:21 PM, Richard Sandiford wrote: > Vladimir Makarov writes: >>H.J. in >> >> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55116 >> >>reported an interesting address >> >>> (and:DI (subreg:DI (plus:SI (ashift:SI (reg:SI 96 [ glob_vol_int.22 ]) >> (const_int 2 [0x2])) >> (symbol_ref:SI ("glob_vol_int_arr") > 0x703c2720 glob_vol_int_arr>)) 0) >> (const_int 4294967295 [0x])) >> >>which can not be correctly extracted. Here `and' with `subreg' >> behaves as an address mutation. >> >>The following patch fixes the problem. >> >> Ok to commit, Richard? > > Heh, I wondered if subregs might still be used like that, and was almost > tempted to add them just in case. > > I think this particular case is really a failed canonicalisation and that: > >(and:DI (subreg:DI (foo:SI ...) 0) (const_int 0x)) > > ought to be: > >(zero_extend:DI (foo:SI ...)) This canonicalisation would make my life much easier. Please note that (and:DI ((foo:DI ...) (const_int 0x))) is also valid address. We have tripped on it a couple of times with x32. Uros.
Re: User directed Function Multiversioning via Function Overloading (issue5752064)
On Mon, Oct 29, 2012 at 5:55 AM, Jan Hubicka wrote: >> Index: gcc/cgraph.c >> === >> --- gcc/cgraph.c (revision 192623) >> +++ gcc/cgraph.c (working copy) >> @@ -132,6 +132,74 @@ static GTY(()) struct cgraph_edge *free_edges; >> /* Did procss_same_body_aliases run? */ >> bool same_body_aliases_done; >> >> +/* Map a cgraph_node to cgraph_function_version_info using this htab. >> + The cgraph_function_version_info has a THIS_NODE field that is the >> + corresponding cgraph_node.. */ >> +htab_t GTY((param_is (struct cgraph_function_version_info *))) >> + cgraph_fnver_htab = NULL; > > I think you want declare the htab static and arrange it to be freed after > cgraph construction, so you don't need to take care of nodes being removed > via the hooks. I will declare the htab static but I want this htab for later optimizations, like dispatch hoisting. Please see: http://gcc.gnu.org/ml/gcc-patches/2011-04/msg02285.html for a description of the optimization. IFUNC based dispatch blocks inlining of multi-versioned functions and dispatch hoisting will help with this. I will make the other changes asap. Thanks, -Sri. > > OK with this change. > > I have few other comments: >> + /* IFUNC resolvers have to be externally visible. */ >> + TREE_PUBLIC (decl) = 1; >> + DECL_UNINLINABLE (decl) = 1; > > Why the resolvers can not be inlined? >> + >> + DECL_EXTERNAL (decl) = 0; >> + DECL_EXTERNAL (dispatch_decl) = 0; >> + >> + DECL_CONTEXT (decl) = NULL_TREE; >> + DECL_INITIAL (decl) = make_node (BLOCK); >> + DECL_STATIC_CONSTRUCTOR (decl) = 0; >> + TREE_READONLY (decl) = 0; >> + DECL_PURE_P (decl) = 0; > > I think those can be copied from the functions you are resolving. (well as > well > as many attributes and properties) >> + >> + if (DECL_COMDAT_GROUP (default_decl)) >> +{ >> + DECL_COMDAT (decl) = DECL_COMDAT (default_decl); >> + make_decl_one_only (decl, DECL_COMDAT_GROUP (default_decl)); >> +} >> + else if (TREE_PUBLIC (default_decl)) >> +{ >> + /* In this case, each translation unit with a call to this >> + versioned function will put out a resolver. Ensure it >> + is comdat to keep just one copy. */ >> + DECL_COMDAT (decl) = 1; >> + make_decl_one_only (decl, DECL_ASSEMBLER_NAME (decl)); >> +} >> + /* Build result decl and add to function_decl. */ >> + t = build_decl (UNKNOWN_LOCATION, RESULT_DECL, NULL_TREE, ptr_type_node); >> + DECL_ARTIFICIAL (t) = 1; >> + DECL_IGNORED_P (t) = 1; >> + DECL_RESULT (decl) = t; >> + >> + gimplify_function_tree (decl); >> + push_cfun (DECL_STRUCT_FUNCTION (decl)); >> + gimple_register_cfg_hooks (); >> + init_empty_tree_cfg_for_function (DECL_STRUCT_FUNCTION (decl)); >> + cfun->curr_properties |= >> +(PROP_gimple_lcf | PROP_gimple_leh | PROP_cfg | PROP_ssa >> + | PROP_gimple_any); >> + cfun->curr_properties = 15; >> + new_bb = create_empty_bb (ENTRY_BLOCK_PTR); >> + make_edge (ENTRY_BLOCK_PTR, new_bb, EDGE_FALLTHRU); >> + make_edge (new_bb, EXIT_BLOCK_PTR, 0); >> + *empty_bb = new_bb; > > You can simplify this by init_lowered_empty_function. > > Honza
Re: Fix estimation of array accesses
> > ICK ... > > > > Why not sth as simple as > > > > return num_ssa_operands (stmt, SSA_OP_USE); > > > > ? a[1][2] and b[2] really have the same cost, variable length > > objects have extra SSA operands in ARRAY_REF/COMPONENT_REF for > > the size. Thus, stmt cost somehow should reflect the number > > of dependent stmts. > > > > So in estimate_num_insns I'd try > > > > int > > estimate_num_insns (gimple stmt, eni_weights *weights) > > { > > unsigned cost, i; > > enum gimple_code code = gimple_code (stmt); > > tree lhs; > > tree rhs; > > > > switch (code) > > { > >case GIMPLE_ASSIGN: > > /* Initialize with the number of SSA uses, one is free. */ > > cost = num_ssa_operands (stmt, SSA_OP_USE); > > if (cost > 1) > > --cost; > > Hi, > this is the udpated patch I am testing after today discussion. I decided to > drop the ipa-inline-analysis changes and do that incrementally. So the patch > now trashes tramp3d performance by increasing need for early-inlining-insns, > but it is not inexpected. > > The patch also fixes accounting of addr expr that got previously confused with > a load. > > Does this seem sane? > > * tree-inline.c (estimate_operator_cost): Return 1 for non-trivial > ADDR_EXPR operations. > (estimate_num_insns): Do not confuse general single rhs with > loads; account cost of non-trivial addr_expr for ASSIGN_EXPR, > GIMPLE_RETURN and GIMPLE_ASM Hi, this patch actually do not cause that much of tramp3d fuzz and no differences in testsuite due to unroling changes The change are the constants when accounting loads and stores. Typical store has two SSA uses (one for address and one for value to store). Of course we lose difference in between array offset and pointer dereference. Typical load/address expression has one SSA use (for the address) Bootstrapped/regtested x86_64-linux, OK? Honza Index: tree-inline.c === --- tree-inline.c (revision 192945) +++ tree-inline.c (working copy) @@ -3447,6 +3447,19 @@ estimate_operator_cost (enum tree_code c if (TREE_CODE (op2) != INTEGER_CST) return weights->div_mod_cost; return 1; +case ADDR_EXPR: + { +tree addr_base; +HOST_WIDE_INT addr_offset; + + addr_base = get_addr_base_and_unit_offset (TREE_OPERAND (op1, 0), + &addr_offset); + /* If the offset is variable or with non-zero offset, return 2. */ + if (!addr_base || addr_offset != 0 || TREE_CODE (addr_base) != MEM_REF + || !integer_zerop (TREE_OPERAND (addr_base, 1))) + return 1; + } + return 0; default: /* We expect a copy assignment with no operator. */ @@ -3512,14 +3525,24 @@ estimate_num_insns (gimple stmt, eni_wei lhs = gimple_assign_lhs (stmt); rhs = gimple_assign_rhs1 (stmt); - if (is_gimple_reg (lhs)) - cost = 0; - else + /* Store. */ + if (gimple_vdef (stmt)) cost = estimate_move_cost (TREE_TYPE (lhs)); + else + cost = 0; - if (!is_gimple_reg (rhs) && !is_gimple_min_invariant (rhs)) + /* Load. */ + if (gimple_vuse (stmt)) cost += estimate_move_cost (TREE_TYPE (rhs)); + /* Stores, loads and address expressions may have variable array +references in them. Account these. */ + if (gimple_vdef (stmt)) + cost += MAX (0, (int)num_ssa_operands (stmt, SSA_OP_USE) - 2); + else if (gimple_vuse (stmt) + || gimple_assign_rhs_code (stmt) == ADDR_EXPR) + cost += MAX (0, (int)num_ssa_operands (stmt, SSA_OP_USE) - 1); + cost += estimate_operator_cost (gimple_assign_rhs_code (stmt), weights, gimple_assign_rhs1 (stmt), get_gimple_rhs_class (gimple_assign_rhs_code (stmt)) @@ -3596,6 +3619,7 @@ estimate_num_insns (gimple stmt, eni_wei case GIMPLE_RETURN: return weights->return_cost; ++ MAX (0, (int)num_ssa_operands (stmt, SSA_OP_USE) - 1); case GIMPLE_GOTO: case GIMPLE_LABEL: @@ -3606,7 +3630,10 @@ estimate_num_insns (gimple stmt, eni_wei return 0; case GIMPLE_ASM: - return asm_str_count (gimple_asm_string (stmt)); + return (asm_str_count (gimple_asm_string (stmt)) + /* Account also possible non-trivial addr_exprs +in the arguments. */ + + num_ssa_operands (stmt, SSA_OP_USE)); case GIMPLE_RESX: /* This is either going to be an external function call with one
GCC 4.8.0 Status Report (2012-10-29), Stage 1 to end soon
Status == I'd like to close the stage 1 phase of GCC 4.8 development on Monday, November 5th. If you have still patches for new features you'd like to see in GCC 4.8, please post them for review soon. Patches posted before the freeze, but reviewed shortly after the freeze, may still go in, further changes should be just bugfixes and documentation fixes. Quality Data Priority # Change from Last Report --- --- P1 23 + 23 P2 77 + 8 P3 85 + 84 --- --- Total 185 +115 Previous Report === http://gcc.gnu.org/ml/gcc/2012-03/msg00011.html The next report will be sent by me again, announcing end of stage 1.
Re: [PATCH] Fix libbacktrace on 32-bit sparc
From: Ian Lance Taylor Date: Mon, 29 Oct 2012 08:43:42 -0700 > This patch should fix it. Bootstrapped and ran libbacktrace testsuite > on x86_64-unknown-linux-gnu. Committed to mainline. I can tell just by looking at your patch that it doesn't fix the problem. It's "vec->base + vec->size" that's not aligned, rather than the length. All your change does it align the length term. You have to align the vec->size term in the pointer computation. Actually you need to align both. First you have to align vec->size to an 8 byte boundary. Then you have to align the remaining length that results to an 8 byte boundary as well. Can you post a test patch for me this time, instead of just committing a change which hasn't been tested to fix the problem? I'd very much appreciate that. Thanks.
Re: GCC 4.8.0 Status Report (2012-10-29), Stage 1 to end soon
From: Jakub Jelinek Date: Mon, 29 Oct 2012 18:56:42 +0100 > I'd like to close the stage 1 phase of GCC 4.8 development > on Monday, November 5th. If you have still patches for new features you'd > like to see in GCC 4.8, please post them for review soon. Patches > posted before the freeze, but reviewed shortly after the freeze, may > still go in, further changes should be just bugfixes and documentation > fixes. I'd like to get the Sparc cbcond stuff in (3 revisions posted) which is waiting for Eric B. to do some Solaris specific work. I'd also like to enable LRA for at least 32-bit sparc, even if I can't find the time to work on auditing 64-bit completely.
Re: Minor update for inlining for code size
> Bootstrapped/regtested x86_64-linux, will commit it shortly. > > Honza > > * gcc.dg/ipa/inline-6.c: New testcase. > * ipa-inline.c (want_inline_function_called_once_p): Rename to ... > (want_inline_function_to_all_callers_p): check also functions with > multiple callers. > (ipa_inline): Handle inlining for size into multiple callers. The entry for the testcase must go into testsuite/ChangeLog. -- Eric Botcazou
Re: [PATCH] Fix debug info for expr and jump stmt
On Mon, Oct 29, 2012 at 6:07 PM, Dehao Chen wrote: > Hi, Micheal, > > Thanks for explaining the design principle of debug info with gimple, > now I can understand your concerns. And thanks for providing the > patch. > > However, in some places after gimplification (e.g. tree-inline.c), we > still updates the block/location info. And EXPR_LOCATION is still used > widely after gimplification. Do you mean that in the long run, we'd > want to remove all these? Yes. Unfortunately many of the core inlining routines are also used by the C++ frontend for template instantiation... Richard. > Thanks > Dehao > > On Mon, Oct 29, 2012 at 9:49 AM, Dehao Chen wrote: >> On Mon, Oct 29, 2012 at 9:10 AM, Richard Biener >> wrote: >>> On Mon, Oct 29, 2012 at 4:25 PM, Dehao Chen wrote: On Mon, Oct 29, 2012 at 7:17 AM, Michael Matz wrote: > Hi, > > On Mon, 29 Oct 2012, Richard Biener wrote: > >> > Well, you merely moved the bogus code to gimple-low.c. It is bogus >> > because you unconditionally overwrite TREE_BLOCK of all operands (and >> > all Emm, then in gimple-low.c, we should probably not unconditionally overwrite gimple_block for stmts too? >>> >>> gimple stmts have no block before gimple-low. >>> >> > operands operands) with the statements block. I assume the unit-test >> > you >> > added shows the problem you were trying to fix? >> > >> > From the scan-assembler-no directive I'm not really sure what exactly >> > the >> > problem is you're seeing. Can you try to describe it with words for >> > the >> > example code? Which operands has no tree-block where it should have >> > one, >> > or which one has the wrong tree-block? >> >> trees without block could be an issue when we extract them to some other >> statement (then without block), and move that to some random place. > > Even then it would be either the frontends job to set the block, or the > the job of the pass that introduced the new expression, when there is a > clearly sane candidate for the block. Please correct me if I'm wrong: front-end does not set blocks for either stmt/expr. lower_stmt is the first place that block is set for stmt, thus I think it should also be the first place to set block for expr. >>> >>> No, only the block on the gimple stmt matters for gimple. You should not >>> need to touch the operands block. >>> > >> But the only issue should be that the stmt/expressions effective block >> becomes a different one (the currently "active" one during expansion). I agree. Initially, both stmt and expressions is set to NULL block. Whenever we update the stmt's block, we should also update the expression's block, otherwise they will be inconsistent. >>> >>> I don't think so. >> >> Hi, Richard, >> >> Can you share some insights why you don't think we want to make stmt >> and expr's block consistent? >> >>> > > Yep. > >> I don't see how we could end up generating too many block location >> DIEs because of this. For the unittest I added, all the assembly code guarded by "if" are should be within one lexical block, which is lock: LBB1 { asm1 asm2 asm3 asm4 } However, because some expression are with NULL lexical block, these assembly codes are separated by several discontinual lexical block which is like: LBB1 { asm1 } asm2 >>> >>> asm2 should have gone into the currently open scope (similiar for what >>> we do for line number information). >> >> In location_block patch, we changed this not to fall into the >> currently open scope, because we want every insn to have correct >> location/block with it. This is one of the reasons why we introduced >> the location_block patch: it makes it easy to update block for >> stmt/expr/phi_arg thus keeping a consistent location/block pair for >> all instructions becomes possible. >> >> Yes, let location/block fall into the currently open scope/location >> was a workaround before, but if we are able to make the location/block >> pair consistent all through compilation, we should do that, right? >> >> Thanks, >> Dehao >> >>> LBB2 { asm3 } asm4 > > And even if, I don't see what's the _problem_ with too many block > locations, except bloat. Yes, bloat is the major problem. Thanks, Dehao > > > Ciao, > Michael.
Re: [PATCH] Fix libbacktrace on 32-bit sparc
On Mon, Oct 29, 2012 at 11:03 AM, David Miller wrote: > From: Ian Lance Taylor > Date: Mon, 29 Oct 2012 08:43:42 -0700 > >> This patch should fix it. Bootstrapped and ran libbacktrace testsuite >> on x86_64-unknown-linux-gnu. Committed to mainline. > > I can tell just by looking at your patch that it doesn't fix the > problem. > > It's "vec->base + vec->size" that's not aligned, rather than the > length. All your change does it align the length term. > > You have to align the vec->size term in the pointer computation. I did. I changed it from vec->base + vec->size. Now it is vec->base + size, where size is the aligned version of vec->size. I know that vec->base is aligned. Now that size is aligned, I know that vec->base + size is aligned. > First you have to align vec->size to an 8 byte boundary. Then > you have to align the remaining length that results to an 8 > byte boundary as well. Yes, the patch does both of those things. > Can you post a test patch for me this time, instead of just committing > a change which hasn't been tested to fix the problem? I'd very much > appreciate that. Sure, if you can confirm that this patch fails, I will take another look. Ian
Re: [PATCH] Fix libbacktrace on 32-bit sparc
From: Ian Lance Taylor Date: Mon, 29 Oct 2012 11:13:19 -0700 > Sure, if you can confirm that this patch fails, I will take another look. It fails. Same problem, SIGBUS.
Re: [PATCH] Fix libbacktrace on 32-bit sparc
From: Ian Lance Taylor Date: Mon, 29 Oct 2012 11:13:19 -0700 > I changed it from vec->base + vec->size. Now it is vec->base > + size, where size is the aligned version of vec->size. It is not. You don't modify 'size' at all.
Re: [PATCH GCC]Fix test case failure reported in PR54989
On 10/26/2012 10:14 PM, Bin Cheng wrote: Actually I am going to submit a patch monitoring the change of register pressure accurately as we discussed before. With that patch, expressions will be hoisted anyway if it can decrease register pressure thus this test case won't fail anymore. So I am wondering whether I should check in this patch or just waiting for that patch to fix this failure? What's your opinion? I'd go ahead and get in the fix for this failure as we would prefer as few unexpected failures as possible. jeff
Re: GCC 4.8.0 Status Report (2012-10-29), Stage 1 to end soon
> I'd like to get the Sparc cbcond stuff in (3 revisions posted) which > is waiting for Eric B. to do some Solaris specific work. > > I'd also like to enable LRA for at least 32-bit sparc, even if I can't > find the time to work on auditing 64-bit completely. End of stage #1 isn't a hard limit for architecture-specific patches, so we need not make a decision about LRA immediately. I don't think we want to half enable it though, so it's all or nothing. -- Eric Botcazou
Re: [PATCH] Fix libbacktrace on 32-bit sparc
On Mon, Oct 29, 2012 at 11:15 AM, David Miller wrote: > From: Ian Lance Taylor > Date: Mon, 29 Oct 2012 11:13:19 -0700 > >> I changed it from vec->base + vec->size. Now it is vec->base >> + size, where size is the aligned version of vec->size. > > It is not. > > You don't modify 'size' at all. Ah, yes, sorry. Could you test this patch? Ian Index: mmap.c === --- mmap.c (revision 192945) +++ mmap.c (working copy) @@ -240,7 +240,8 @@ backtrace_vector_release (struct backtra aligned = (size + 7) & ~ (size_t) 7; alc -= aligned - size; - backtrace_free (state, (char *) vec->base + size, alc, error_callback, data); + backtrace_free (state, (char *) vec->base + aligned, alc, + error_callback, data); vec->alc = 0; return 1; }
[patch] move GIMPLE_TRANSACTION expansion to tmmark pass
What was supposed to be a simple fix, turned out to be a nightmare... I am working on adding an uninstrumented code path for TM, in expectation of hardware TM. This means that we eventually want to generate something like this: prop = _ITM_beginTransaction(PR_multiwayCode); if (prop & PR_UNINSTRUMENTEDCODE) global = 55; else _ITM_WU4 (&global, 55); _ITM_commitTransaction (); Since reading "prop" and duplicating BB's must happen at the TMMARK pass (instrumentation pass), we must move the expansion of GIMPLE_TRANSACTION -> _ITM_beginTransaction() into the TMMARK pass, instead of a later pass like is currently being done. Moving this, caused all sorts of grief, because I found that the abnormal edges for each TM built-in, have been incorrectly generated since day one. I have no idea, how things worked before... Every TM built-in must have an abnormal edge back to the outer-most transaction. As it currently stands, TM built-ins have an edge to the current transaction, which means that for nested transactions, we end up with incorrect edges to every transaction. Transaction cancel statements, OTOH, must _also_ have an edge to the inner transaction-- the transaction to which the cancel appears in. And finally, __transaction_cancel[[outer]] statements must only have one abnormal edge to the transaction marked as outer by the front-end (__transaction_atomic[[outer]]). I had to fix all this, as part of the work moving the transaction expansion into the TMMARK pass. There are also various cleanups that seemed sensible while I was re-arranging things, so my apologies for the plethora of fixes. I have not added further tests, since the present testsuite triggers the problems I found. Finally, I adapted various tests, because things now happen in other passes. OK, pending one more round of testing? p.s. You'll hate tm_region_for_stmt*(), but (a) it's only called once for every __transaction_cancel, which is rare (b) I was really tired :)). If you hate it more than I do, I can optimize it later. + * trans-mem.c (tm_log_emit_save_or_restores): Remove FALLTHRU_EDGE + parameter and calculate locally. + (struct tm_region): Add new fields: + original_transaction_was_outer, transaction_label, tm_state. + (tm_region_init_0): Initialize new tm_region fields. + (expand_assign_tm): Add comments. + (expand_transaction): Reorganize to be called in tmmark pass + instead of the tmedge pass. + (generate_tm_state): New. + (execute_tm_mark): Generate TM_STATE temporaries. Expand gimple + transactions into calls into the run-time. + (tm_region_for_stmt_1): New. + (tm_region_for_stmt): New. + (make_tm_edge): Point abnormal TM built-in edges to the outermost + transaction. Point abnormal TM_ABORT edges appropriately. + (tmedge_wire_transaction_restarts): Abstract code previously in + expand_transaction to wire abnormal edges. + (expand_regions_1): Rewrite to use callback. + (expand_regions): Same. + (execute_tm_edges): Call expand_regions with new callback. + * trans-mem.h (PR_MULTIWAYCODE): New. + * tree-cfg.c (gimple_debug_cfg): Update comment. + + testsuite/ + * gcc.dg/tm/debug-1.c: Scan for _ITM_beginTransaction. + * gcc.dg/tm/irrevocable-3.c: Scan for doesGoIrrevocable. + * gcc.dg/tm/irrevocable-4.c: Scan for hasNoIrrevocable. + * gcc.dg/tm/props-4.c: Do not scan for LABEL. + * gcc.dg/tm/memopt-11.c: Adjust scan text. + * gcc.dg/tm/memopt-10.c: Adjust scan text. diff --git a/gcc/testsuite/gcc.dg/tm/debug-1.c b/gcc/testsuite/gcc.dg/tm/debug-1.c index fae5d6b..01acfae 100644 --- a/gcc/testsuite/gcc.dg/tm/debug-1.c +++ b/gcc/testsuite/gcc.dg/tm/debug-1.c @@ -20,7 +20,7 @@ main() { } /* { dg-final { scan-tree-dump-times ": 13:.*b = 9898" 1 "tmmark" } } */ -/* { dg-final { scan-tree-dump-times ": 14:.*__transaction" 1 "tmmark" } } */ +/* { dg-final { scan-tree-dump-times ": 14:.*_ITM_beginTransaction" 1 "tmmark" } } */ /* { dg-final { scan-tree-dump-times ": 15:.*ITM_WU. \\(&z" 1 "tmmark" } } */ /* { dg-final { scan-tree-dump-times ": 16:.*ITM_WU. \\(&a" 1 "tmmark" } } */ /* { dg-final { cleanup-tree-dump "tmmark" } } */ diff --git a/gcc/testsuite/gcc.dg/tm/irrevocable-3.c b/gcc/testsuite/gcc.dg/tm/irrevocable-3.c index c085479..fdf3e52 100644 --- a/gcc/testsuite/gcc.dg/tm/irrevocable-3.c +++ b/gcc/testsuite/gcc.dg/tm/irrevocable-3.c @@ -10,5 +10,5 @@ foo() } } -/* { dg-final { scan-tree-dump-times "GTMA_MAY_ENTER_IRREVOCABLE" 1 "tmmark" } } */ +/* { dg-final { scan-tree-dump-times "doesGoIrrevocable" 1 "tmmark" } } */ /* { dg-final { cleanup-tree-dump "tmmark" } } */ diff --git a/gcc/tes
Re: [PATCH] Fix libbacktrace on 32-bit sparc
From: Ian Lance Taylor Date: Mon, 29 Oct 2012 11:31:33 -0700 > On Mon, Oct 29, 2012 at 11:15 AM, David Miller wrote: >> From: Ian Lance Taylor >> Date: Mon, 29 Oct 2012 11:13:19 -0700 >> >>> I changed it from vec->base + vec->size. Now it is vec->base >>> + size, where size is the aligned version of vec->size. >> >> It is not. >> >> You don't modify 'size' at all. > > Ah, yes, sorry. > > Could you test this patch? Will do, hopefully before ConEd turns my power off.
Re: GCC 4.8.0 Status Report (2012-10-29), Stage 1 to end soon
From: Eric Botcazou Date: Mon, 29 Oct 2012 20:25:15 +0100 >> I'd like to get the Sparc cbcond stuff in (3 revisions posted) which >> is waiting for Eric B. to do some Solaris specific work. >> >> I'd also like to enable LRA for at least 32-bit sparc, even if I can't >> find the time to work on auditing 64-bit completely. > > End of stage #1 isn't a hard limit for architecture-specific patches, so we > need not make a decision about LRA immediately. I don't think we want to > half > enable it though, so it's all or nothing. Upon further consideration, agreed. I'll only turn this on if I can get the whole backend working. FWIW, I think we should consider delaying stage1 for another reason. A large number of North American developers are about to be hit by a major natural disaster, and may be without power for weeks.
Re: [PATCH] Fix libbacktrace on 32-bit sparc
From: David Miller Date: Mon, 29 Oct 2012 14:33:33 -0400 (EDT) > From: Ian Lance Taylor > Date: Mon, 29 Oct 2012 11:31:33 -0700 > >> On Mon, Oct 29, 2012 at 11:15 AM, David Miller wrote: >>> From: Ian Lance Taylor >>> Date: Mon, 29 Oct 2012 11:13:19 -0700 >>> I changed it from vec->base + vec->size. Now it is vec->base + size, where size is the aligned version of vec->size. >>> >>> It is not. >>> >>> You don't modify 'size' at all. >> >> Ah, yes, sorry. >> >> Could you test this patch? > > Will do, hopefully before ConEd turns my power off. Yep, this works, thanks!
Re: [PATCH] Fix libbacktrace on 32-bit sparc
On Mon, Oct 29, 2012 at 11:40 AM, David Miller wrote: > From: David Miller > Date: Mon, 29 Oct 2012 14:33:33 -0400 (EDT) > >> From: Ian Lance Taylor >> Date: Mon, 29 Oct 2012 11:31:33 -0700 >> >>> On Mon, Oct 29, 2012 at 11:15 AM, David Miller wrote: From: Ian Lance Taylor Date: Mon, 29 Oct 2012 11:13:19 -0700 > I changed it from vec->base + vec->size. Now it is vec->base > + size, where size is the aligned version of vec->size. It is not. You don't modify 'size' at all. >>> >>> Ah, yes, sorry. >>> >>> Could you test this patch? >> >> Will do, hopefully before ConEd turns my power off. > > Yep, this works, thanks! Great, thanks for testing it! Committed to mainline. Ian 2012-10-29 Ian Lance Taylor * mmap.c (backtrace_vector_release): Correct last patch: add aligned, not size.
Re: [patch] Unify bitmap interface.
On 10/29/12, Richard Biener wrote: > On Oct 29, 2012 Diego Novillo wrote: > > On Oct 25, 2012 Lawrence Crowl wrote: > > > The sbitmap non-bool returning bitwise operations have been > > > merged with the bool versions. Sometimes this merge involved > > > modifying the non-bool version to compute the bool value, > > > and sometimes modifying bool version to add additional work > > > from the non-bool version. The redundant routines have been > > > ifdef'd out. I will remove the ifdef'd out code later. > > > > No #if 0 code, please. Let's just remove them. Okay, but I point out that there is an awful lot of #if 0 code out there. I would rather have done such removal in a followup patch. > > > The allocation functions have not been renamed, because we > > > often do not have an argument on which to overload. > > > > Would it work if we made the first argument a reference to the > > bitmap being allocated? I suppose this shouldn't be a big deal. > > It's definitely good to at least in one place see what kind of > bitmap is actually being used ... ;) The parameters to the allocation functions are different. So even if they had the same name, they are not interchangable. > > > The cardinality functions have not been renamed, because they > > > have different parameters, and are thus not interchangable. > > > > Why not change the parameters then? > > Agree, as a followup maybe. The sbitmap popcount function is only used in ebitmap, which is itself not used. If we do anything, removing them might be the thing to do. > > > The iteration functions have not been renamed, because they > > > are functionally different. > > > > Functionally different? How? The bitmap.h iterators set and the sbitmap.h iterator set intersect in one element. I would rather treat the iterator conversion as a separate patch. > > It seems like the patch only goes skin deep. I would rather > > make a true unification. If the only thing that remains the > > different are the allocation functions, that's not a big deal. > > But the point was to make everything else the same. > > Indeed. It's not quite skin deep as it removes the distinction between the value returning and non-value returning functions. The intent here is to change the skin. Richard said in an earlier mail, "I'd rather not mix this with any kind of further C++-ification (that is introduction of member functions or operator overloads)." So, exactly what are you all after? > > I also notice that the patch does not rename any of the SET_BIT, > > RESET_BIT functions in sbitmap.c. > > They should go - a followup is fine though. That was the intent. > > > Tested on x86_64. Config testing in progress. > > > > You will also want to test on a couple other architecture on > > the farm. By config testing, do you mean contrib/config-list.mk? Yes, I meant that. There are no functional changes here. Why is contrib/config-list.mk not sufficient? -- Lawrence Crowl