[Ada] Attribute Loop_Entry and index check generation
This patch delays the generation of index checks for the following cases of Loop_Entry attribute references: Prefix'Loop_Entry (Expr) Prefix'Loop_Entry (Expr1, Expr2, ... ExprN) Even though these constructs appear initially as attribute references, analysis converts them into indexed components to reflect their true semantics. Without this patch, expansion of the indexed components would generate index checks of the following form [constraint_error when not (blah in a'loop_entry'first .. a'loop_entry'last) index check failed] and the back end would subsequently fail because it cannot process Loop_Entry. Tested on x86_64-pc-linux-gnu, committed on trunk 2013-01-04 Hristian Kirtchev kirtc...@adacore.com * checks.adb (Generate_Index_Checks): Delay the generation of the check for an indexed component where the prefix mentions Loop_Entry until the attribute has been properly expanded. * exp_ch5.adb (Expand_Loop_Entry_Attributes): Perform minor decoration of the constant that captures the value of Loop_Entry's prefix at the entry point into a loop. Generate index checks for an attribute reference that has been transformed into an indexed component. Index: exp_ch5.adb === --- exp_ch5.adb (revision 194841) +++ exp_ch5.adb (working copy) @@ -1828,11 +1828,29 @@ Object_Definition = New_Reference_To (Typ, Loc), Expression = Relocate_Node (Prefix (LE; + -- Perform minor decoration as this information will be needed for + -- the creation of index checks (if applicable). + + Set_Ekind (Temp, E_Constant); + Set_Etype (Temp, Typ); + -- Replace the original attribute with a reference to the constant Rewrite (LE, New_Reference_To (Temp, Loc)); Set_Etype (LE, Typ); + -- Analysis converts attribute references of the following form + + -- Prefix'Loop_Entry (Expr) + -- Prefix'Loop_Entry (Expr1, Expr2, ... ExprN) + + -- into indexed components for error detection purposes. Generate + -- index checks now that 'Loop_Entry has been properly expanded. + + if Nkind (Parent (LE)) = N_Indexed_Component then +Generate_Index_Checks (Parent (LE)); + end if; + Next_Elmt (LE_Elmt); end loop; Index: checks.adb === --- checks.adb (revision 194848) +++ checks.adb (working copy) @@ -5522,6 +5522,23 @@ or else Index_Checks_Suppressed (Etype (A)) then return; + + -- The indexed component we are dealing with contains 'Loop_Entry in its + -- prefix. This case arises when analysis has determined that constructs + -- such as + + -- Prefix'Loop_Entry (Expr) + -- Prefix'Loop_Entry (Expr1, Expr2, ... ExprN) + + -- require rewriting for error detection purposes. A side effect of this + -- action is the generation of index checks that mention 'Loop_Entry. + -- Delay the generation of the check until 'Loop_Entry has been properly + -- expanded. This is done in Expand_Loop_Entry_Attributes. + + elsif Nkind (Prefix (N)) = N_Attribute_Reference +and then Attribute_Name (Prefix (N)) = Name_Loop_Entry + then + return; end if; -- Generate a raise of constraint error with the appropriate reason and
[Ada] Aspect Global
This patch provides the initial implementation of aspect Global. This construct is intended for formal verification proofs. The syntax of the aspect is as follows: global_specification ::= (moded_global_list {, moded_global_list}) | global_list | null moded_global_list::= mode_selector = global_list global_list ::= global_item | (global_item {, global_item}) mode_selector::= Input | Output | In_Out | Contract_In global_item ::= name The semantics of the aspect are as follows: A global_item of a subprogram shall be a stand alone variable object [that is, it is not part of a larger object] or it shall be a state abstraction. Each mode_selector shall occur at most once in a single Global aspect. A function subprogram may not have a mode_selector of Output or In_Out in its Global aspect. Global_items in the same Global aspect shall denote distinct entities. A global item occurring in a Global aspect of a subprogram aspect specification shall not have the same defining_identifier as a formal parameter of the subprogram. A global item of mode in out or out shall not be a Volatile Input state abstraction (see Abstract State Aspect). A global item of mode in or in out shall not be a Volatile Output state abstraction. This patch also corrects the timing of pragma Abstract_State analysis. -- Source -- -- semantics.ads package Semantics with Abstract_State = ((Input_State with Volatile, Input), (Output_State with Volatile, Output)) is type Composite is record Comp : Integer; end record; Var : Composite; type Composite_Array is array (1 .. 5) of Composite; Arr : Composite_Array; procedure Dummy_Proc; procedure OK_1 with Global = (Var, Input_State); procedure Error_1 with Global = (Var.Comp, Arr (2), Dummy_Proc); procedure OK_2 with Global = (Input = (Var, Input_State), Output = Arr); procedure Error_2 with Global = (Input = Var, Contract_In = Input_State, Input = Arr); function OK_3 return Boolean with Global = (Input = Var, Contract_In = Input_State); function Error_3 return Boolean with Global = (In_Out = Arr, Output = Var); procedure Error_4 with Global = (Input = Semantics.Var, Output = Var); function Error_5 (Formal : Boolean) return Boolean with Global = Formal; procedure Error_6 with Global = (In_Out = Input_State); procedure Error_7 with Global = (Output = Input_State); procedure Error_8 with Global = (In_Out = Output_State); procedure Error_9 with Global = (Input = Output_State); procedure Error_10 with Global = Output_State; procedure OK_4 with Global = null; procedure Error_11 with Global = (null, Var); procedure Error_12 with Global = (Var, null, Arr); procedure Error_13 with Global = (Var, null); end Semantics; -- Compilation and output -- $ gcc -c -gnat12 -gnatd.V semantics.ads semantics.ads:19:12: global item must denote variable or state semantics.ads:20:09: global item must denote variable or state semantics.ads:21:09: global item must denote variable or state semantics.ads:30:09: duplicate global mode semantics.ads:37:09: global mode In_Out not applicable to functions semantics.ads:38:09: global mode Output not applicable to functions semantics.ads:42:19: duplicate global item semantics.ads:44:21: global item cannot reference formal parameter semantics.ads:46:32: global item of mode In_Out or Output cannot reference Volatile Input state semantics.ads:48:32: global item of mode In_Out or Output cannot reference Volatile Input state semantics.ads:50:32: global item of mode In_Out or Input cannot reference Volatile Output state semantics.ads:52:31: global item of mode In_Out or Input cannot reference Volatile Output state semantics.ads:54:21: global item of mode In_Out or Input cannot reference Volatile Output state semantics.ads:58:22: cannot mix null and non-null global items semantics.ads:60:27: cannot mix null and non-null global items semantics.ads:62:27: cannot mix null and non-null global items Tested on x86_64-pc-linux-gnu, committed on trunk 2013-01-04 Hristian Kirtchev kirtc...@adacore.com * aspects.adb, aspects.ads: Add Aspect_Global to all relevant tables. * par-prag.adb: Add pragma Global to the list of pragmas that do not need special processing by the parser. * sem_ch13.adb (Analyze_Aspect_Specifications): Convert aspect Global into a pragma without any form of legality checks. The work is done by Analyze_Pragma. The aspect and pragma are both marked as needing delayed processing. Insert the
[Ada] Better error message for aspect specification without Ada 2012 mode
This patch improves the error message when compiling a unit with an aspect specification in an older Ada mode, and the first aspect specification is for a boolean aspect whose value is defaulted. Compiling the following: gcc -c r1.ads must yield: r1.ads:2:16: aspect specification is an Ada 2012 feature r1.ads:2:16: unit must be compiled with -gnat2012 switch package R1 is procedure Q with Inline, Pre = False; end R1; Tested on x86_64-pc-linux-gnu, committed on trunk 2013-01-04 Ed Schonberg schonb...@adacore.com * par-ch13.adb (Aspect_Specifications_Present): In Strict mode, accept an aspect name followed by a comma, indicating a defaulted boolean aspect. Index: par-ch13.adb === --- par-ch13.adb(revision 194841) +++ par-ch13.adb(working copy) @@ -105,6 +105,13 @@ if Token = Tok_Arrow then Result := True; +-- The identifier may be the name of a boolean aspect with a +-- defaulted True value. Further checks when analyzing aspect +-- specification. + +elsif Token = Tok_Comma then + Result := True; + elsif Token = Tok_Apostrophe then Scan; -- past apostrophe
[Ada] Follow on work for tagging of warning switches
Correct some missing cases of setting this switch, document it in usage. Primarily documentation, so no new test needed. Also makes clear that this switch is not available in VMS. No test needed. Tested on x86_64-pc-linux-gnu, committed on trunk 2013-01-04 Robert Dewar de...@adacore.com * gnat_ugn.texi: Document -gnatw.d/w.D (does no apply in VMS mode). * usage.adb: Add lines for -gnatw.d/w.D switches. * warnsw.adb: Minor fixes (some missing cases of setting Warning_Doc_Switch). Reject -gnatw.d and -gnatw.D in VMS mode. Index: usage.adb === --- usage.adb (revision 194894) +++ usage.adb (working copy) @@ -474,6 +474,16 @@ Write_Line (.C* turn off warnings for unrepped components); Write_Line (dturn on warnings for implicit dereference); Write_Line (D* turn off warnings for implicit dereference); + + -- Switches -gnatw.d/w.D not available on VMS + + if not OpenVMS_On_Target then + Write_Line +(.d turn on tagging of warnings with -gnatw switch); + Write_Line +(.D* turn off tagging of warnings with -gnatw switch); + end if; + Write_Line (etreat all warnings (but not info) as errors); Write_Line (.e turn on every optional info/warning (no exceptions)); Index: gnat_ugn.texi === --- gnat_ugn.texi (revision 194889) +++ gnat_ugn.texi (working copy) @@ -5214,6 +5214,9 @@ switch are @option{-gnatwd} (implicit dereferencing), @option{-gnatwh} (hiding), +@ifclear VMS +@option{-gnatw.d} (tag warnings with -gnatw switch) +@end ifclear @option{-gnatw.h} (holes (gaps) in record layouts) @option{-gnatw.i} (overlapping actuals), @option{-gnatw.k} (redefinition of names in standard), @@ -5362,6 +5365,24 @@ This switch suppresses warnings for implicit dereferences in indexed components, slices, and selected components. +@ifclear vms +@item -gnatw.d +@emph{Activate tagging of warning messages.} +@cindex @option{-gnatw.d} (@command{gcc}) +If this switch is set, then warning messages are tagged, either with +the string ``@option{-gnatw?}'' showing which switch controls the warning, +or with ``[enabled by default]'' if the warning is not under control of a +specific @option{-gnatw?} switch. This mode is off by default, and is not +affected by the use of @code{-gnatwa}. + +@item -gnatw.D +@emph{Deactivate tagging of warning messages.} +@cindex @option{-gnatw.d} (@command{gcc}) +If this switch is set, then warning messages return to the default +mode in which warnings are not tagged as described above for +@code{-gnatw.d}. +@end ifclear + @item -gnatwe @emph{Treat warnings and style checks as errors.} @cindex @option{-gnatwe} (@command{gcc}) Index: warnsw.adb === --- warnsw.adb (revision 194841) +++ warnsw.adb (working copy) @@ -53,11 +53,19 @@ Warn_On_Unrepped_Components := False; when 'd' = -Warning_Doc_Switch := True; +if Open_VMS_On_Target then + return False; +end if; +Warning_Doc_Switch := True; + when 'D' = -Warning_Doc_Switch := False; +if Open_VMS_On_Target then + return False; +end if; +Warning_Doc_Switch := False; + when 'e' = Address_Clause_Overlay_Warnings := True; Check_Unreferenced := True; @@ -68,6 +76,7 @@ Implementation_Unit_Warnings:= True; Ineffective_Inline_Warnings := True; List_Inherited_Aspects := True; +Warning_Doc_Switch := True; Warn_On_Ada_2005_Compatibility := True; Warn_On_Ada_2012_Compatibility := True; Warn_On_All_Unread_Out_Parameters := True; @@ -217,6 +226,7 @@ Implementation_Unit_Warnings:= False; Ineffective_Inline_Warnings := True; List_Inherited_Aspects := False; + Warning_Doc_Switch := False; Warn_On_Ada_2005_Compatibility := True; Warn_On_Ada_2012_Compatibility := True; Warn_On_All_Unread_Out_Parameters := False; @@ -296,6 +306,7 @@ Implementation_Unit_Warnings:= False; Ineffective_Inline_Warnings := False; List_Inherited_Aspects := False; +Warning_Doc_Switch := False; Warn_On_Ada_2005_Compatibility := False; Warn_On_Ada_2012_Compatibility := False;
[Ada] Internal fix to Insert_Actions routine in compiler
This patch corrects an obvious latent bug in Insert_Actions, namely that in the case of an expression with actions node, the actions were inserted in reverse order from the calls. As far as we know, this bug is only latent (found from code reading), so no test is required for this fix. Tested on x86_64-pc-linux-gnu, committed on trunk 2013-01-04 Robert Dewar de...@adacore.com * exp_util.ads, exp_util.adb (Insert_Actions): In expression with actions case, new actions are appended to the sequence rather than prepended. Index: exp_util.adb === --- exp_util.adb(revision 194894) +++ exp_util.adb(working copy) @@ -3138,7 +3138,7 @@ and then not Is_Frozen (Current_Scope) then if No (Scope_Stack.Table - (Scope_Stack.Last).Pending_Freeze_Actions) + (Scope_Stack.Last).Pending_Freeze_Actions) then Scope_Stack.Table (Scope_Stack.Last).Pending_Freeze_Actions := Ins_Actions; @@ -3306,13 +3306,13 @@ return; -- Case of appearing within an Expressions_With_Actions node. We --- prepend the actions to the list of actions already there, if +-- append the actions to the list of actions already there, if -- the node has not been analyzed yet. Otherwise find insertion -- location further up the tree. when N_Expression_With_Actions = if not Analyzed (P) then - Prepend_List (Ins_Actions, Actions (P)); + Append_List (Ins_Actions, Actions (P)); return; end if; Index: exp_util.ads === --- exp_util.ads(revision 194887) +++ exp_util.ads(working copy) @@ -75,6 +75,9 @@ --expansion of the N_If_Expression node rewrites the node so that the --actions can be positioned normally. + --For actions coming from expansion of the expression in an expression + --with actions node, the action is appended to the list of actions. + -- Basically what we do is to climb up to the tree looking for the -- proper insertion point, as described by one of the above cases, -- and then insert the appropriate action or actions.
Re: [AARCH64] Optimize cmp in some cases
On 03/01/13 22:39, Andrew Pinski wrote: Hi, For aarch64, we don't CSE some cmp away. This patch fixes the case where we are CSE across some basic-blocks like: int f(int a, int b) { if(ab) return 1; if(ab) return -1; return 0; } --- CUT --- To fix this, I implemented the target hook TARGET_FIXED_CONDITION_CODE_REGS as there was already code in CSE which uses this target hook to find the extra setting of the CC registers. OK? Build and tested on aarch64-thunder-elf (using Cavium's internal simulator). Build a cross to aarch64-thunder-linux-gnu and a Canadian cross with that one for the native toolchain. Thanks, Andrew Pinski * config/aarch64/aarch64.c (aarch64_fixed_condition_code_regs): New function. (TARGET_FIXED_CONDITION_CODE_REGS): Define. * gcc.target/aarch64/cmp-1.c: New testcase. fixed_condition_code.diff.txt * config/aarch64/aarch64.c (aarch64_fixed_condition_code_regs): New function. (TARGET_FIXED_CONDITION_CODE_REGS): Define. * gcc.target/aarch64/cmp-1.c: New testcase. Index: config/aarch64/aarch64.c === --- config/aarch64/aarch64.c(revision 194872) +++ config/aarch64/aarch64.c(working copy) @@ -3041,6 +3041,16 @@ aarch64_const_double_zero_rtx_p (rtx x) return REAL_VALUES_EQUAL (r, dconst0); } +/* Return the fixed registers used for condition codes. */ + +static bool +aarch64_fixed_condition_code_regs (unsigned int *p1, unsigned int *p2) +{ + *p1 = CC_REGNUM; + *p2 = -1; Please use INVALID_REGNUM here (as the documentation states). Otherwise, OK. A backport to the AArch64-4.7 branch would be appreciated. R.
Re: [C++ Patch] PR 54526 (again)
Hi, On 01/03/2013 10:56 PM, Jason Merrill wrote: On 01/03/2013 05:44 AM, Paolo Carlini wrote: + /* C++11 - 2.5 p3, bullet 2. */ Please flesh out this comment some more. Ok, I extended it like this. Thanks, Paolo. / Index: gcc/cp/parser.c === --- gcc/cp/parser.c (revision 194884) +++ gcc/cp/parser.c (working copy) @@ -1,6 +1,6 @@ /* C++ Parser. Copyright (C) 2000, 2001, 2002, 2003, 2004, - 2005, 2007, 2008, 2009, 2010, 2011, 2012 Free Software Foundation, Inc. + 2005, 2007-2013 Free Software Foundation, Inc. Written by Mark Mitchell m...@codesourcery.com. This file is part of GCC. @@ -12655,11 +12655,9 @@ cp_parser_template_id (cp_parser *parser, return error_mark_node; } /* Otherwise, emit an error about the invalid digraph, but continue -parsing because we got our argument list. In C++11 do not emit -any error, per 2.5/3. */ - if (cxx_dialect cxx0x - permerror (next_token-location, - %::% cannot begin a template-argument list)) +parsing because we got our argument list. */ + if (permerror (next_token-location, +%::% cannot begin a template-argument list)) { static bool hint = false; inform (next_token-location, Index: gcc/testsuite/g++.dg/cpp0x/parse2.C === --- gcc/testsuite/g++.dg/cpp0x/parse2.C (revision 194884) +++ gcc/testsuite/g++.dg/cpp0x/parse2.C (working copy) @@ -10,3 +10,6 @@ int main() { X::A x; } + +int a; +bool b = 0::a; Index: gcc/testsuite/g++.old-deja/g++.other/crash28.C === --- gcc/testsuite/g++.old-deja/g++.other/crash28.C (revision 194884) +++ gcc/testsuite/g++.old-deja/g++.other/crash28.C (working copy) @@ -31,5 +31,5 @@ class foo }; void foo::x() throw(bar) { - if (!b) throw bar (static_cast::N::X*(this)); // { dg-error lambda expressions|expected } parse error + if (!b) throw bar (static_cast::N::X*(this)); // { dg-error lambda expressions|expected|invalid } parse error } Index: libcpp/lex.c === --- libcpp/lex.c(revision 194884) +++ libcpp/lex.c(working copy) @@ -1,6 +1,6 @@ /* CPP Library - lexical analysis. - Copyright (C) 2000, 2001, 2002, 2003, 2004, 2005, 2007, 2008, 2009, 2010, - 2011, 2012 Free Software Foundation, Inc. + Copyright (C) 2000, 2001, 2002, 2003, 2004, 2005, 2007-2013 + Free Software Foundation, Inc. Contributed by Per Bothner, 1994-95. Based on CCCP program by Paul Rubin, June 1986 Adapted to ANSI C, Richard Stallman, Jan 1987 @@ -2290,6 +2290,17 @@ _cpp_lex_direct (cpp_reader *pfile) { if (*buffer-cur == ':') { + /* C++11 [2.5/3 lex.pptoken], Otherwise, if the next +three characters are :: and the subsequent character +is neither : nor , the is treated as a preprocessor +token by itself. */ + if (CPP_OPTION (pfile, cplusplus) + (CPP_OPTION (pfile, lang) == CLK_CXX11 + || CPP_OPTION (pfile, lang) == CLK_GNUCXX11) + buffer-cur[1] == ':' + buffer-cur[2] != ':' buffer-cur[2] != '') + break; + buffer-cur++; result-flags |= DIGRAPH; result-type = CPP_OPEN_SQUARE;
Re: Use libstdc++-raw-cxx.m4 in libjava
H.J. Lu hjl.to...@gmail.com writes: On Thu, Jan 3, 2013 at 10:09 AM, Andreas Schwab sch...@linux-m68k.org wrote: H.J. Lu hjl.to...@gmail.com writes: diff --git a/libjava/Makefile.am b/libjava/Makefile.am index c6c84e4..dd08a4f 100644 --- a/libjava/Makefile.am +++ b/libjava/Makefile.am @@ -594,7 +594,7 @@ lib_gnu_awt_xlib_la_CPPFLAGS = \ $(AM_CPPFLAGS) \ $(LIBSTDCXX_RAW_CXX_CXXFLAGS) ## The mysterious backslash in the grep pattern is consumed by make. -lib_gnu_awt_xlib_la_LDFLAGS = $(LIBSTDCXX_RAW_CXX_LDLAGS) \ +lib_gnu_awt_xlib_la_LDFLAGS = $(LIBSTDCXX_RAW_CXX_LIBADD) \ @X_PRE_LIBS@ @X_LIBS@ -lX11 @X_EXTRA_LIBS@ \ -rpath $(toolexeclibdir) $(LIBJAVA_LDFLAGS_NOUNDEF) \ -version-info `grep -v '^\#' $(srcdir)/libtool-version` $(LIBGCJ_LD_SYMBOLIC) It is still wrong to use LDFLAGS for libraries to be linked in. All of $(LIBSTDCXX_RAW_CXX_LIBADD) @X_PRE_LIBS@ @X_LIBS@ -lX11 @X_EXTRA_LIBS@ should be on lib_gnu_awt_xlib_la_LDADD. This was how it was done before my change. If we want to make a change, I can submit a separate patch. Libraries should never occur before the objects which reference them. Andreas. -- Andreas Schwab, sch...@linux-m68k.org GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 And now for something completely different.
[RFA/ARM/4.7] Fix PR54974: Thumb literal pools don't handle PC rounding
On 29/11/12 14:42, Matthew Gretton-Dann wrote: On 24 November 2012 00:27, Ramana Radhakrishnan ramana@googlemail.com wrote: On Wed, Nov 21, 2012 at 7:59 PM, Matthew Gretton-Dann matthew.gretton-d...@linaro.org wrote: [snip] The fix is to decrease the pool_range of all insns by 2 when generating Thumb code. There is no need to change neg_pool_range values as rounding down here will reduce the distance of the literal pool. A comment about this fact around thumb2_pool_range would be appropriate. [snip] Tested arm-none-linux-gnueabi cross, and with the testcase attached to the PR. No added testcase in the patch as this code is sensitive to other code generation and so it is not easy to generate a testcase which will reliably test this condition. OK for trunk, 4.7, and 4.6? Ok for trunk today - please wait a few days before backporting into 4.6 and 4.7 to see what the fallout is like . Watch out for any fallout with the auto-testers. No fallout has been seen with the auto-testers. The attached is what was actually committed as revision 193930 (original patch + requested comment). The attached patch is a backport of the trunk patch to 4.7. Cross tested arm-none-linux-gnueabi with QEMU OK for 4.7? Thanks, Matt 2013-01-04 Matthew Gretton-Dann matthew.gretton-d...@linaro.org Backport from mainline. 2012-11-29 Matthew Gretton-Dann matthew.gretton-d...@linaro.org PR target/54974 * config/arm/arm.md (thumb2_pool_range, pool_range): Add comment on Thumb pool ranges. (thumb1_extendhisi2): Reduce Thumb pool range. (arm_movdi): Likewise. (thumb1_movdi_insn): Likewise. (thumb1_movsi_insn): Likewise. (pic_load_addr_unified): Likewise. (pic_load_addr_32bit): Likewise. (pic_load_addr_thumb1): Likewise. (thumb1_movhf): Likewise. (arm_movsf_soft_insn): Likewise. (thumb1_movsf_soft_insn): Likewise. (movdf_soft_insn): Likewise. (thumb1_movdf_soft_insn): Likewise. * config/arm/neon.md (*neon_movmode): Likewise. (*neon_movmode): Likwise. * config/arm/thumb2.md: (*thumb2_movsi_insn): Likewise. (*thumb2_movhi_insn): Likewise. (*thumb2_extendqisi_v6): Likewise. (*thumb2_zero_extendqisi_v6): Likewise. (*thumb2_zero_extendqisi2_v6): Likewise. * config/arm/vfp.md: (*thumb2_movsi_vfp): Likewise. (*movdi_vfp): Likewise. (*movdi_vfp_cortexa8): Likewise. (*thumb2_movsf_vfp): Likewise. (*thumb2_movdf_vfp): Likewise. -- Matthew Gretton-Dann Toolchain Working Group, Linaro Index: gcc/config/arm/arm.md === --- gcc/config/arm/arm.md (revision 194852) +++ gcc/config/arm/arm.md (working copy) @@ -256,6 +256,9 @@ (define_attr insn_enabled no,yes ; POOL_RANGE is how far away from a constant pool entry that this insn ; can be placed. If the distance is zero, then this insn will never ; reference the pool. +; Note that for Thumb constant pools the PC value is rounded down to the +; nearest multiple of four. Therefore, THUMB2_POOL_RANGE (and POOL_RANGE for +; Thumb insns) should be set to max_range - 2. ; NEG_POOL_RANGE is nonzero for insns that can reference a constant pool entry ; before its address. It is set to max_range - (8 + data_size). (define_attr arm_pool_range (const_int 0)) @@ -4833,7 +4836,7 @@ (define_insn thumb1_extendhisi2 (const_int 2) (const_int 4)) (const_int 4)]) (set_attr type alu_shift,load_byte) - (set_attr pool_range *,1020)] + (set_attr pool_range *,1018)] ) ;; This pattern will only be used when ldsh is not available @@ -5239,7 +5242,7 @@ (define_insn *arm_movdi (set_attr type *,*,*,load2,store2) (set_attr arm_pool_range *,*,*,1020,*) (set_attr arm_neg_pool_range *,*,*,1004,*) - (set_attr thumb2_pool_range *,*,*,4096,*) + (set_attr thumb2_pool_range *,*,*,4094,*) (set_attr thumb2_neg_pool_range *,*,*,0,*)] ) @@ -5379,7 +5382,7 @@ (define_insn *thumb1_movdi_insn [(set_attr length 4,4,6,2,2,6,4,4) (set_attr type *,*,*,load2,store2,load2,store2,*) (set_attr insn *,mov,*,*,*,*,*,mov) - (set_attr pool_range *,*,*,*,*,1020,*,*)] + (set_attr pool_range *,*,*,*,*,1018,*,*)] ) (define_expand movsi @@ -5539,7 +5542,7 @@ (define_insn *thumb1_movsi_insn mov\\t%0, %1 [(set_attr length 2,2,4,4,2,2,2,2,2) (set_attr type *,*,*,*,load1,store1,load1,store1,*) - (set_attr pool_range *,*,*,*,*,*,1020,*,*) + (set_attr pool_range *,*,*,*,*,*,1018,*,*) (set_attr conds set,clob,*,*,nocond,nocond,nocond,nocond,nocond)]) (define_split @@ -5632,7 +5635,7 @@ (define_insn_and_split pic_load_addr_un (match_dup 2)] UNSPEC_PIC_BASE))] operands[3] = TARGET_THUMB ? GEN_INT (4) : GEN_INT (8); [(set_attr type load1,load1,load1)
[Ada] Cleanup order of Nkind declarations in Sinfo
This is an internal cleanup, no functional effect, no test Tested on x86_64-pc-linux-gnu, committed on trunk 2013-01-04 Robert Dewar de...@adacore.com * sinfo.ads: Clean up order of N_xxx subtypes Index: sinfo.ads === --- sinfo.ads (revision 194887) +++ sinfo.ads (working copy) @@ -7638,6 +7638,12 @@ N_Function_Call, N_Procedure_Call_Statement, + -- N_Subexpr, N_Has_Etype, N_Raise_xxx_Error + + N_Raise_Constraint_Error, + N_Raise_Program_Error, + N_Raise_Storage_Error, + -- N_Subexpr, N_Has_Etype N_Explicit_Dereference, @@ -7648,15 +7654,6 @@ N_Null, N_Qualified_Expression, N_Quantified_Expression, - - -- N_Raise_xxx_Error, N_Subexpr, N_Has_Etype - - N_Raise_Constraint_Error, - N_Raise_Program_Error, - N_Raise_Storage_Error, - - -- N_Subexpr, N_Has_Etype - N_Aggregate, N_Allocator, N_Case_Expression,
Commit: V850: Only compile callt support functions when CALLT is enabled
Hi Guys, I am checking in the patch below to fix a small problem with libgcc for the V850. The CALLT support functions were being compiled even if the particular multilib concerned did not support the CALLT insn. Normally this would not matter, but the linker will now report an unknown relocation error for these functions due to the way that they are implemented. Tested with no regressions on a v850-elf toolchain. Cheers Nick libgcc/ChangeLog 2013-01-04 Nick Clifton ni...@redhat.com * config/v850/lib1funcs.S: Only provide CALLT support functions if the CALLT instruction is supported. Index: libgcc/config/v850/lib1funcs.S === --- libgcc/config/v850/lib1funcs.S (revision 194883) +++ libgcc/config/v850/lib1funcs.S (working copy) @@ -1764,6 +1764,7 @@ .size __restore_all_interrupt,.-__restore_all_interrupt #endif /* L_save_all_interrupt */ +#if defined __V850_CALLT__ #if defined(__v850e__) || defined(__v850e1__) || defined(__v850e2__) || defined(__v850e2v3__) #ifdef L_callt_save_r2_r29 /* Put these functions into the call table area. */ @@ -2146,6 +2147,7 @@ #endif #endif /* __v850e__ */ +#endif /* __V850_CALLT__ */ /* libgcc2 routines for NEC V850. */ /* Double Integer Arithmetical Operation. */
[PATCH] Fix PR55863
This fixes PR55863, a folding missed optimization caused by folding -X - 1 to ~X. This makes data dependence analysis fail for offsetted-by-1 accesses (in some cases). Bootstrapped and tested on x86_64-unknown-linux-gnu, applied to trunk. Richard. 2013-01-04 Richard Biener rguent...@suse.de PR middle-end/55863 * fold-const.c (split_tree): Undo -X - 1 to ~X folding for reassociation. * gcc.dg/fold-reassoc-2.c: New testcase. Index: gcc/fold-const.c === *** gcc/fold-const.c(revision 194855) --- gcc/fold-const.c(working copy) *** split_tree (tree in, enum tree_code code *** 821,826 --- 821,833 if (neg_var_p) var = negate_expr (var); } + else if (TREE_CODE (in) == BIT_NOT_EXPR + code == PLUS_EXPR) + { + /* -X - 1 is folded to ~X, undo that here. */ + *minus_litp = build_one_cst (TREE_TYPE (in)); + var = negate_expr (TREE_OPERAND (in, 0)); + } else if (TREE_CONSTANT (in)) *conp = in; else Index: gcc/testsuite/gcc.dg/fold-reassoc-2.c === *** gcc/testsuite/gcc.dg/fold-reassoc-2.c (revision 0) --- gcc/testsuite/gcc.dg/fold-reassoc-2.c (working copy) *** *** 0 --- 1,14 + /* { dg-do compile } */ + /* { dg-options -O -fdump-tree-original } */ + + int foo (int i) + { + return (i + 2) - (i + 1); + } + int bar (int i) + { + return (i + 2) + ~i; + } + + /* { dg-final { scan-tree-dump return 1; original } } */ + /* { dg-final { cleanup-tree-dump original } } */
Re: [PATCH] Clarify error message (PR middle-end/55859)
On Fri, Jan 04, 2013 at 02:59:46AM +0100, Marek Polacek wrote: This patch clarifies the error message when using e.g. -Obar option, except non-negative numbers we accept some other levels too. Bootstrapped on x86_64-linux. Ok for trunk? 2013-01-04 Marek Polacek pola...@redhat.com PR middle-end/55859 * opts.c (default_options_optimization): Clarify error message. --- gcc/opts.c.mp 2013-01-04 02:48:33.116785922 +0100 +++ gcc/opts.c2013-01-04 02:48:51.729870314 +0100 @@ -1,7 +1,6 @@ /* Command line option handling. Copyright (C) 2002, 2003, 2004, 2005, 2006, 2007, 2008, 2009, 2010, 2011, - 2012 - + 2012, 2013 Free Software Foundation, Inc. Contributed by Neil Booth. @@ -543,7 +542,8 @@ default_options_optimization (struct gcc const int optimize_val = integral_argument (opt-arg); if (optimize_val == -1) error_at (loc, - argument to %qs should be a non-negative integer, + argument to %qs should be a non-negative integer, + 'g', 's', or 'fast', -O); I wonder what is the point of using %qs with a fixed string, and ' should be probably replaced by % and %, so perhaps argument to %-O% should be a non-negative integer, %s%, %g% or %fast%); Jakub
Re: [PATCH, PR 55755] Make SRA create less VIEW_CONVERT_EXPRs
On Thu, 3 Jan 2013, Martin Jambor wrote: Hi, the patch below fixes PR 55755 which was in the compiler for years. The problem is that a replacement of a bit-field can have a larger TYPE_SIZE than the type of the field and so creating a V_C_E from it to the field type may result in invalid gimple. We do that when we scalarize only one side of an assignment and get incompatible types on both sides and the other (non-scalar) side has a child access in the access tree (regardless if it is to be scalarize or not). When looking at the issue I realized that the last condition is completely unnecessary (at least now, the first concepts of the new SRA were a bit different) because the subsequent handling of sub-replacements will do the right thing (load/store them to the original aggregate) and removing it is indeed the correct thing to deal with this bug - if both sides are scalarized, size of both will grow to mode size, if only one, we can avoid the V_C_E. I am a little worried about the contains_bitfld_comp_ref_p and contains_vce_or_bfcref_p gurads which are there because of Ada PR 46349 (which involves aggregate bit-fields) and which might in theory lead to the same problem but I'm weary of touching it, at least not in one commit (I'm testing what happens if I remove them right now), and this patch does not make the current situation any worse. In order to make sure we do not mess up when the non-scalar side has sub-replacements in it, I have added a new testcase. The patch has passed bootstrap and testing on x86_64-linux on trunk and the 4.7 and 4.6 branches. I'd like to commit it to all of them, perhaps after having it on trunk only for a while. Ok for trunk and branches after a while. Thanks, Richard. Thanks, Martin 2013-01-02 Martin Jambor mjam...@suse.cz PR tree-optimization/55755 * tree-sra.c (sra_modify_assign): Do not check that an access has no children when trying to avoid producing a VIEW_CONVERT_EXPR. testsuite/ * gcc.dg/torture/pr55755.c: New test. * gcc.dg/tree-ssa/sra-13.c: Likewise. * gcc.dg/tree-ssa/pr45144.c: Update. Index: src/gcc/testsuite/gcc.dg/torture/pr55755.c === --- /dev/null +++ src/gcc/testsuite/gcc.dg/torture/pr55755.c @@ -0,0 +1,43 @@ +/* { dg-do run } */ +/* { dg-require-effective-target int32plus } */ + +struct S4 +{ + unsigned f0:24; +} __attribute__((__packed__)); + +struct S4 g_10 = { + 6210831 +}; + +struct S5 +{ + int i; + struct S4 l_8[2]; +} __attribute__((__packed__)); + +int a, b; + +struct S4 func_2 (int x) +{ + struct S5 l = { +0, +{{0}, {0}} + }; + l.i = a; + g_10 = l.l_8[1]; + for (; x2; x++) { +struct S4 tmp = { + 11936567 +}; +l.l_8[x] = tmp; + } + b = l.i; + return g_10; +} + +int main (void) +{ + func_2 (0); + return 0; +} Index: src/gcc/testsuite/gcc.dg/tree-ssa/sra-13.c === --- /dev/null +++ src/gcc/testsuite/gcc.dg/tree-ssa/sra-13.c @@ -0,0 +1,114 @@ +/* Test that SRA replacement can deal with assignments that have + sub-replacements on one side and a single scalar replacement on another. */ +/* { dg-do run } */ +/* { dg-options -O1 } */ + +struct A +{ + int i1, i2; +}; + +struct B +{ + long long int l; +}; + +union U +{ + struct A a; + struct B b; +}; + +int b, gi; +long gl; +union U gu1, gu2; + +int __attribute__ ((noinline, noclone)) +foo (void) +{ + union U x, y; + int r; + + y = gu1; + if (b) +y.b.l = gl; + + x = y; + + if (!b) +r = x.a.i1; + else +r = 0; + + gu2 = x; + return r; +} + +long long int __attribute__ ((noinline, noclone)) +bar (void) +{ + union U x, y; + int r; + + y = gu1; + if (b) +y.a.i1 = gi; + + x = y; + + if (!b) +r = x.b.l; + else +r = 0; + + gu2 = x; + return r; +} + + +int +main (void) +{ + int r; + long long int s; + + b = 0; + gu1.a.i1 = 123; + gu1.a.i2 = 234; + r = foo (); + if (r != 123) +__builtin_abort (); + if (gu2.a.i1 != 123) +__builtin_abort (); + if (gu2.a.i2 != 234) +__builtin_abort (); + + b = 1; + gl = 1001; + gu1.b.l = 1000; + r = foo (); + if (r != 0) +__builtin_abort (); + if (gu2.b.l != 1001) +__builtin_abort (); + + b = 0; + gu1.b.l = 2000; + s = bar (); + if (s != 2000) +__builtin_abort (); + if (gu2.b.l != 2000) +__builtin_abort (); + + b = 1; + gi = 456; + gu1.a.i1 = 123; + gu1.a.i2 = 234; + s = bar (); + if (s != 0) +__builtin_abort (); + if (gu2.a.i1 != 456) +__builtin_abort (); + + return 0; +} Index: src/gcc/tree-sra.c === --- src.orig/gcc/tree-sra.c +++ src/gcc/tree-sra.c @@
Re: RFA: Fix ICE on PARALLEL returns when expand builtins
On Thu, Jan 3, 2013 at 7:42 PM, Richard Sandiford rdsandif...@googlemail.com wrote: Richard Biener richard.guent...@gmail.com writes: On Wed, Jan 2, 2013 at 5:36 PM, Richard Sandiford rdsandif...@googlemail.com wrote: Richard Biener richard.guent...@gmail.com writes: On Sun, Dec 23, 2012 at 10:43 AM, Richard Sandiford rdsandif...@googlemail.com wrote: Some of the maths builtins can expand to a call followed by a bit of postprocessing. With 4.8's PARALLEL return optimisations, these embedded calls might return a PARALLEL of pseudos, but the postprocessing isn't prepared to deal with that. This leads to an ICE in builtins-53.c on n32 and n64 mips64-linux-gnu. One fix might have been to pass an explicit register target to the expand routines, but that target's only a hint. This patch instead adds an avoid_group_rtx function (named after gen_group_rtx) to convert PARALLELs to pseudos where necessary. I wondered whether it was really safe for expand_builtin_int_roundingfn_2 to pass target == const0_rtx as the ignore parameter to expand_call, given that we don't actually ignore the return value ourselves (even if the caller does). I suppose it is safe though, since expand_call will always return const0_rtx in that case, and this code is dealing with integer return values. Tested on mips64-linux-gnu. OK to install? Or is there a better way? You didn't add a testcase so I can't check myself It's gcc.dg/builtins-53.c. - but why isn't using force_reg enough here? I can imagine other cases than PARALLELs that are not well handled, no? Not sure either way TBH. Fortunately expanding your own calls seems to be pretty rare. But yeah, having force_reg (or I suppose force_operand) do it sounds good in principle. The problem is that the operation needs the type tree, which the force_* routines don't have. Hm? force_reg/operand only need a mode. Index: builtins.c === *** builtins.c (revision 194787) --- builtins.c (working copy) *** expand_builtin_int_roundingfn (tree exp, *** 2760,2765 --- 2760,2766 /* Truncate the result of floating point optab to integer via expand_fix (). */ + tmp = force_reg (TYPE_MODE (TREE_TYPE (TREE_TYPE (fallback_fndecl))), tmp); target = gen_reg_rtx (mode); expand_fix (target, tmp, 0); What I mean is: force_operand doesn't convert PARALLELs of pseudos to single pseudos, so this won't work as-is. And we can't make force_operand do the conversion using the current emit_group_store machinery because emit_group_store needs access to the type (in order to work out the padding), which force_operand doesn't have. Hmm, how would anyone else get at the padding info dealing with the parallel? This looks broken if we need access to the type :/ Now, why's that rounding function case relevant at all? The rounding fns in question return FP mode values - I seriously doubt there is any padding involved (and I can't see how any target would use a multi-register passing here?!) Eh. Eric, any comments? Thanks, Richard. Richard
Re: [PATCH] Clarify error message (PR middle-end/55859)
On Fri, Jan 04, 2013 at 11:42:24AM +0100, Jakub Jelinek wrote: I wonder what is the point of using %qs with a fixed string, and ' should be probably replaced by % and %, so perhaps argument to %-O% should be a non-negative integer, %s%, %g% or %fast%); So, like this? 2013-01-04 Marek Polacek pola...@redhat.com * opts.c (default_options_optimization): Clarify error message. --- gcc/opts.c.mp 2013-01-04 02:48:33.116785922 +0100 +++ gcc/opts.c 2013-01-04 12:00:15.983233160 +0100 @@ -1,7 +1,6 @@ /* Command line option handling. Copyright (C) 2002, 2003, 2004, 2005, 2006, 2007, 2008, 2009, 2010, 2011, - 2012 - + 2012, 2013 Free Software Foundation, Inc. Contributed by Neil Booth. @@ -542,9 +541,8 @@ default_options_optimization (struct gcc { const int optimize_val = integral_argument (opt-arg); if (optimize_val == -1) - error_at (loc, - argument to %qs should be a non-negative integer, - -O); + error_at (loc, argument to %-O% should be a non-negative + integer, %g%, %s% or %fast%); else { opts-x_optimize = optimize_val; Marek
Re: [PATCH] Clarify error message (PR middle-end/55859)
On Fri, Jan 04, 2013 at 12:05:49PM +0100, Marek Polacek wrote: On Fri, Jan 04, 2013 at 11:42:24AM +0100, Jakub Jelinek wrote: I wonder what is the point of using %qs with a fixed string, and ' should be probably replaced by % and %, so perhaps argument to %-O% should be a non-negative integer, %s%, %g% or %fast%); So, like this? Yes, thanks. Jakub
Re: [Patch, Fortran] PR55763 - reject MOLD with NULL() in init-data expressions
Hello, Le 04/01/2013 00:23, Tobias Burnus a écrit : NULL with MOLD should be rejected as (default) initialization expression. From F2008: R506 null-init is function-reference C512 (R506) The function-reference shall be a reference to the intrinsic function NULL with no arguments. null-init occurs twice, as R505 initialization in R505 initialization and in R442 component-initialization (default initialization). Before, integer, pointer :: p = null(x) gave an type error (LHS: integer, RHS: unknown). While class(*), pointer :: p = null(x) was accepted without error diagnostic. Build and regtested on x86-64-gnu-linux. OK for the trunk? Tobias diff --git a/gcc/fortran/decl.c b/gcc/fortran/decl.c index 5ed8388..7d49578 100644 --- a/gcc/fortran/decl.c +++ b/gcc/fortran/decl.c @@ -1671,11 +1671,31 @@ match gfc_match_null (gfc_expr **result) { gfc_symbol *sym; - match m; + match m, m2 = MATCH_NO; - m = gfc_match ( null ( )); - if (m != MATCH_YES) -return m; + if ((m = gfc_match ( null ( ))) == MATCH_ERROR) +return MATCH_ERROR; + + if (m == MATCH_NO) +{ + locus old_loc; + char name[GFC_MAX_SYMBOL_LEN + 1]; + + if ((m2 = gfc_match ( null (, name)) != MATCH_YES) It seems the `name' argument to `gfc_match' is superfluous here. Thanks for the patch. Mikael
[Patch, Fortran] PR55763 - improve init-data checks for pointers
Fortran 2008 allows: integer :: pointer = init_data and type t integer :: pointer = init_data end type t The current check in gfc_check_assign_symbol was only called for former and for constructors, but not for the type definition. Additionally, BT_CLASS wasn't handled. I also improved the error location. The patch has a downside: One gets some messages twice or trice: Once for resolving the type declaration (type t) and then for resolving the default initialization via gfc_traverse_ns (ns, resolve_values); Currently, that's unavoidable as one cannot trivially distinguish between a user-supplied sym-value and the default constructor. If you think that this is a problem, one can change it, e.g. by setting a sym-attr.value_is_default_init. Build and regtested on x86-64-gnu-linux. OK for the trunk? Tobias PS: For CLASS pointers, there will be an ICE if one tries to associate a variable to them; that's unchanged by this patch. 2013-01-04 Tobias Burnus bur...@net-b.de PR fortran/55763 * gfortran.h (gfc_check_assign_symbol): Update prototype. * decl.c (add_init_expr_to_sym, do_parm): Update call. * expr.c (gfc_check_assign_symbol): Handle BT_CLASS and improve error location; support components. (gfc_check_pointer_assign): Handle component assignments. * resolve.c (resolve_fl_derived0): Call gfc_check_assign_symbol. (resolve_values): Update call. (resolve_structure_cons): Avoid double diagnostic. 2013-01-04 Tobias Burnus bur...@net-b.de PR fortran/55763 * gfortran.dg/pointer_init_2.f90: Update dg-error. * gfortran.dg/pointer_init_7.f90: New. diff --git a/gcc/fortran/decl.c b/gcc/fortran/decl.c index fc86efb..5952b70 100644 --- a/gcc/fortran/decl.c +++ b/gcc/fortran/decl.c @@ -1353,14 +1353,14 @@ add_init_expr_to_sym (const char *name, gfc_expr **initp, locus *var_locus) if (sym-ts.type != BT_DERIVED init-ts.type != BT_DERIVED sym-ts.type != BT_CLASS init-ts.type != BT_CLASS !sym-attr.proc_pointer - gfc_check_assign_symbol (sym, init) == FAILURE) + gfc_check_assign_symbol (sym, NULL, init) == FAILURE) return FAILURE; if (sym-ts.type == BT_CHARACTER sym-ts.u.cl init-ts.type == BT_CHARACTER) { /* Update symbol character length according initializer. */ - if (gfc_check_assign_symbol (sym, init) == FAILURE) + if (gfc_check_assign_symbol (sym, NULL, init) == FAILURE) return FAILURE; if (sym-ts.u.cl-length == NULL) @@ -6955,7 +6955,7 @@ do_parm (void) goto cleanup; } - if (gfc_check_assign_symbol (sym, init) == FAILURE + if (gfc_check_assign_symbol (sym, NULL, init) == FAILURE || gfc_add_flavor (sym-attr, FL_PARAMETER, sym-name, NULL) == FAILURE) { m = MATCH_ERROR; diff --git a/gcc/fortran/expr.c b/gcc/fortran/expr.c index 2610784..146154e 100644 --- a/gcc/fortran/expr.c +++ b/gcc/fortran/expr.c @@ -3291,22 +3291,21 @@ gfc_check_assign (gfc_expr *lvalue, gfc_expr *rvalue, int conform) gfc_try gfc_check_pointer_assign (gfc_expr *lvalue, gfc_expr *rvalue) { - symbol_attribute attr; + symbol_attribute attr, lhs_attr; gfc_ref *ref; bool is_pure, is_implicit_pure, rank_remap; int proc_pointer; - if (lvalue-symtree-n.sym-ts.type == BT_UNKNOWN - !lvalue-symtree-n.sym-attr.proc_pointer) + lhs_attr = gfc_expr_attr (lvalue); + if (lvalue-ts.type == BT_UNKNOWN !lhs_attr.proc_pointer) { gfc_error (Pointer assignment target is not a POINTER at %L, lvalue-where); return FAILURE; } - if (lvalue-symtree-n.sym-attr.flavor == FL_PROCEDURE - lvalue-symtree-n.sym-attr.use_assoc - !lvalue-symtree-n.sym-attr.proc_pointer) + if (lhs_attr.flavor == FL_PROCEDURE lvalue-symtree-n.sym-attr.use_assoc + !lhs_attr.proc_pointer) { gfc_error ('%s' in the pointer assignment at %L cannot be an l-value since it is a procedure, @@ -3735,10 +3734,11 @@ gfc_check_pointer_assign (gfc_expr *lvalue, gfc_expr *rvalue) symbol. Used for initialization assignments. */ gfc_try -gfc_check_assign_symbol (gfc_symbol *sym, gfc_expr *rvalue) +gfc_check_assign_symbol (gfc_symbol *sym, gfc_component *comp, gfc_expr *rvalue) { gfc_expr lvalue; gfc_try r; + bool pointer, proc_pointer; memset (lvalue, '\0', sizeof (gfc_expr)); @@ -3750,9 +3750,27 @@ gfc_check_assign_symbol (gfc_symbol *sym, gfc_expr *rvalue) lvalue.symtree-n.sym = sym; lvalue.where = sym-declared_at; - if (sym-attr.pointer || sym-attr.proc_pointer - || (sym-ts.type == BT_CLASS CLASS_DATA (sym)-attr.class_pointer - rvalue-expr_type == EXPR_NULL)) + if (comp) +{ + lvalue.ref = gfc_get_ref (); + lvalue.ref-type = REF_COMPONENT; + lvalue.ref-u.c.component = comp; + lvalue.ref-u.c.sym = sym; + lvalue.ts = comp-ts; + lvalue.rank = comp-as ? comp-as-rank : 0; + lvalue.where = comp-loc; + pointer = comp-ts.type == BT_CLASS CLASS_DATA
[committed] 2011 and 2012 Copyright year updates
Hi! I've run a script to notice gcc maintained files with FSF copyright that have been modified in 2011 and/or 2012 (according to svn log, ignoring r168438 and r184997 commits), but didn't have years 2011 and/or 2012 included in Copyright lines. I've kept the preexisting style, so where year ranges were used, updated those if needed, if not, kept the year lists. Jakub Copyright.updates.bz2 Description: BZip2 compressed data
[PATCH, PR 55579] Make SRA keep candidated with only debug replacements
Hi, the patch below fixes PR 55579 by not disqualifying a candidate variable when it does not have any real replacements but has ones for debug statements only. analyze_access_subtree has to return the same value regardless of MAY_HAVE_DEBUG_STMTS or we get debug comparison errors. I also have a patch that simply avoids disqualifying candidates with no replacements which means we keep some information about it so that SRA can remove a few more un-needed reads but I suppose that is stage1 material. OK for trunk? Thanks, Martin 2013-01-03 Martin Jambor mjam...@suse.cz PR debug/55579 * tree-sra.c (analyze_access_subtree): Return true also after potentially creating a debug-only replacement. testsuite/ * gcc.dg/tree-ssa/pr55579.c: New test. Index: src/gcc/testsuite/gcc.dg/tree-ssa/pr55579.c === --- /dev/null +++ src/gcc/testsuite/gcc.dg/tree-ssa/pr55579.c @@ -0,0 +1,14 @@ +/* { dg-do compile } */ +/* { dg-options -O2 -g -fdump-tree-esra } */ + +struct S { int a; char b; char c; short d; }; + +int +foo (int x) +{ + struct S s = { x + 1, x + 2, x + 3, x + 4 }; + char *p = s.c; + return x; +} + +/* { dg-final { scan-tree-dump Created a debug-only replacement for s esra } } */ Index: src/gcc/tree-sra.c === --- src.orig/gcc/tree-sra.c +++ src/gcc/tree-sra.c @@ -2197,20 +2197,25 @@ analyze_access_subtree (struct access *r } else { - if (MAY_HAVE_DEBUG_STMTS allow_replacements + if (allow_replacements scalar !root-first_child (root-grp_scalar_write || root-grp_assignment_write)) { gcc_checking_assert (!root-grp_scalar_read !root-grp_assignment_read); - root-grp_to_be_debug_replaced = 1; - if (dump_file (dump_flags TDF_DETAILS)) + sth_created = true; + if (MAY_HAVE_DEBUG_STMTS) { - fprintf (dump_file, Marking ); - print_generic_expr (dump_file, root-base, 0); - fprintf (dump_file, offset: %u, size: %u , - (unsigned) root-offset, (unsigned) root-size); - fprintf (dump_file, to be replaced with debug statements.\n); + root-grp_to_be_debug_replaced = 1; + if (dump_file (dump_flags TDF_DETAILS)) + { + fprintf (dump_file, Marking ); + print_generic_expr (dump_file, root-base, 0); + fprintf (dump_file, offset: %u, size: %u , + (unsigned) root-offset, (unsigned) root-size); + fprintf (dump_file, to be replaced with debug + statements.\n); + } } } @@ -2220,17 +2225,11 @@ analyze_access_subtree (struct access *r root-grp_total_scalarization = 0; } - if (sth_created - (!hole || root-grp_total_scalarization)) -{ - root-grp_covered = 1; - return true; -} - if (root-grp_write || TREE_CODE (root-base) == PARM_DECL) + if (!hole || root-grp_total_scalarization) +root-grp_covered = 1; + else if (root-grp_write || TREE_CODE (root-base) == PARM_DECL) root-grp_unscalarized_data = 1; /* not covered and written to */ - if (sth_created) -return true; - return false; + return sth_created; } /* Analyze all access trees linked by next_grp by the means of
[PATCH] Ensure gimplify_one_sizepos doesn't change something with INTEGER_TYPE into something with e.g. ENUMERAL_TYPE (PR middle-end/55851)
Hi! As discussed in the PR, as all INTEGRAL_TYPE_P types with the same sign/precision are usually considered compatible (useless_type_conversion_p), during gimplify_one_sizepos the gimplifier can change e.g. an integer expression into e.g. VAR_DECL with ENUMERAL_TYPE, and when the gimplifier later on passes that to size_binop etc., it can trigger asserts because ENUMERAL_TYPE isn't considered sizetype-ish enough. The following patch (which I don't like too much admittedly) makes sure for constants we fold it back to INTEGER_TYPE constant, and for VAR_DECLs add an extra VAR_DECL in such a case. Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? Or do you prefer some other way? 2013-01-04 Jakub Jelinek ja...@redhat.com PR middle-end/55851 * gimplify.c (gimplify_one_sizepos): Ensure gimplify_expr doesn't turn *expr_p from INTEGER_TYPE expression into e.g. ENUMERAL_TYPE expression. * gcc.c-torture/compile/pr55851.c: New test. --- gcc/gimplify.c.jj 2012-12-20 19:13:00.0 +0100 +++ gcc/gimplify.c 2013-01-03 16:16:07.288707387 +0100 @@ -8180,6 +8180,26 @@ gimplify_one_sizepos (tree *expr_p, gimp *expr_p = unshare_expr (expr); gimplify_expr (expr_p, stmt_p, NULL, is_gimple_val, fb_rvalue); + + /* Ensure we don't change an INTEGER_TYPE expression into e.g. ENUMERAL_TYPE + expression. */ + if (TREE_CODE (TREE_TYPE (*expr_p)) != TREE_CODE (TREE_TYPE (expr))) +{ + gcc_checking_assert (useless_type_conversion_p (TREE_TYPE (expr), + TREE_TYPE (*expr_p))); + if (TREE_CODE (*expr_p) == INTEGER_CST) + *expr_p = fold_convert (TREE_TYPE (expr), *expr_p); + else + { + tree var = create_tmp_var (TREE_TYPE (expr), NULL); + tree mod = build2 (INIT_EXPR, TREE_TYPE (var), +var, unshare_expr (*expr_p)); + SET_EXPR_LOCATION (mod, EXPR_LOC_OR_HERE (*expr_p)); + gimplify_and_add (mod, stmt_p); + ggc_free (mod); + *expr_p = var; + } +} } /* Gimplify the body of statements of FNDECL and return a GIMPLE_BIND node --- gcc/testsuite/gcc.c-torture/compile/pr55851.c.jj2013-01-03 16:20:19.085284806 +0100 +++ gcc/testsuite/gcc.c-torture/compile/pr55851.c 2013-01-03 16:19:27.698571718 +0100 @@ -0,0 +1,12 @@ +/* PR middle-end/55851 */ + +enum { A = 1UL, B = -1UL } var = A; +void foo (char *); + +void +test (void) +{ + char vla[1][var]; + vla[0][0] = 1; + foo (vla[0][0]); +} Jakub
Re: [Patch, Darwin, ppc] constrain processor usage for m32.
Assembler could be fixed easily (I'll contribute the patch to MacPorts cctools port) and the library is loading and working now. So I suggest disabling the use of 64 bit instructions and registers when targetting PowerPC Darwin 32 bit ABI can be considered working and a patch to gcc shouldn't be necessary. Am 04.01.2013 um 01:02 schrieb Tobias Netzel: Found the problem: The assembler generates a relocation of type LO14, meaning 14 bits that represent bits 2 - 15 of the effective address (lowest two bits are forced to zero), which is used for some types of 64 bit loads and stores. Unfortunately the OS X dynamic linker doesn't support that relocation type any more beginning with 10.4. After looking at the dyld sources It seems LO16 should be used instead. I'll see if I can fix the assembler. Am 01.01.2013 um 22:06 schrieb Mike Stump: On Jan 1, 2013, at 10:03 AM, Tobias Netzel tobias.net...@googlemail.com wrote: Or do you have any other ideas? I don't. I'd grab the .s files (compile with -save-temps) and start stripping things out til it loads, then then last thing stripped was the thing that broke it.
Re: [PATCH] Ensure gimplify_one_sizepos doesn't change something with INTEGER_TYPE into something with e.g. ENUMERAL_TYPE (PR middle-end/55851)
On Fri, 4 Jan 2013, Jakub Jelinek wrote: Hi! As discussed in the PR, as all INTEGRAL_TYPE_P types with the same sign/precision are usually considered compatible (useless_type_conversion_p), during gimplify_one_sizepos the gimplifier can change e.g. an integer expression into e.g. VAR_DECL with ENUMERAL_TYPE, and when the gimplifier later on passes that to size_binop etc., it can trigger asserts because ENUMERAL_TYPE isn't considered sizetype-ish enough. The following patch (which I don't like too much admittedly) makes sure for constants we fold it back to INTEGER_TYPE constant, and for VAR_DECLs add an extra VAR_DECL in such a case. Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? Or do you prefer some other way? The other way would be Index: gcc/fold-const.c === --- gcc/fold-const.c(revision 194900) +++ gcc/fold-const.c(working copy) @@ -900,9 +900,9 @@ associate_trees (location_t loc, tree t1 static bool int_binop_types_match_p (enum tree_code code, const_tree type1, const_tree type2) { - if (TREE_CODE (type1) != INTEGER_TYPE !POINTER_TYPE_P (type1)) + if (!INTEGRAL_TYPE_P (type1) !POINTER_TYPE_P (type1)) return false; - if (TREE_CODE (type2) != INTEGER_TYPE !POINTER_TYPE_P (type2)) + if (!INTEGRAL_TYPE_P (type2) !POINTER_TYPE_P (type2)) return false; switch (code) at least the present check doesn't really verify we use sizetypes for size_*op anymore. I'd agree more with your patch if we'd verify that we have proper [s]sizetype, [s]bitsizetype type arguments to size_*op functions (which of course would be again questionable to some degree). That said - we seem to have moved into a direction that makes the above patch not so questionable. Richard. 2013-01-04 Jakub Jelinek ja...@redhat.com PR middle-end/55851 * gimplify.c (gimplify_one_sizepos): Ensure gimplify_expr doesn't turn *expr_p from INTEGER_TYPE expression into e.g. ENUMERAL_TYPE expression. * gcc.c-torture/compile/pr55851.c: New test. --- gcc/gimplify.c.jj 2012-12-20 19:13:00.0 +0100 +++ gcc/gimplify.c2013-01-03 16:16:07.288707387 +0100 @@ -8180,6 +8180,26 @@ gimplify_one_sizepos (tree *expr_p, gimp *expr_p = unshare_expr (expr); gimplify_expr (expr_p, stmt_p, NULL, is_gimple_val, fb_rvalue); + + /* Ensure we don't change an INTEGER_TYPE expression into e.g. ENUMERAL_TYPE + expression. */ + if (TREE_CODE (TREE_TYPE (*expr_p)) != TREE_CODE (TREE_TYPE (expr))) +{ + gcc_checking_assert (useless_type_conversion_p (TREE_TYPE (expr), + TREE_TYPE (*expr_p))); + if (TREE_CODE (*expr_p) == INTEGER_CST) + *expr_p = fold_convert (TREE_TYPE (expr), *expr_p); + else + { + tree var = create_tmp_var (TREE_TYPE (expr), NULL); + tree mod = build2 (INIT_EXPR, TREE_TYPE (var), + var, unshare_expr (*expr_p)); + SET_EXPR_LOCATION (mod, EXPR_LOC_OR_HERE (*expr_p)); + gimplify_and_add (mod, stmt_p); + ggc_free (mod); + *expr_p = var; + } +} } /* Gimplify the body of statements of FNDECL and return a GIMPLE_BIND node --- gcc/testsuite/gcc.c-torture/compile/pr55851.c.jj 2013-01-03 16:20:19.085284806 +0100 +++ gcc/testsuite/gcc.c-torture/compile/pr55851.c 2013-01-03 16:19:27.698571718 +0100 @@ -0,0 +1,12 @@ +/* PR middle-end/55851 */ + +enum { A = 1UL, B = -1UL } var = A; +void foo (char *); + +void +test (void) +{ + char vla[1][var]; + vla[0][0] = 1; + foo (vla[0][0]); +} Jakub -- Richard Biener rguent...@suse.de SUSE / SUSE Labs SUSE LINUX Products GmbH - Nuernberg - AG Nuernberg - HRB 16746 GF: Jeff Hawn, Jennifer Guild, Felix Imend
[PATCH] Fix PR55862
The following reverts a change I did during the various PRE cleanups. Bootstrapped and tested on x86_64-unknown-linux-gnu, applied. Richard. 2013-01-04 Richard Biener rguent...@suse.de PR tree-optimization/55862 * tree-ssa-pre.c (phi_translate_1): Valueize SSA names after translating them through PHI nodes. * gcc.dg/torture/pr55862.c: New testcase. Index: gcc/tree-ssa-pre.c === *** gcc/tree-ssa-pre.c (revision 194900) --- gcc/tree-ssa-pre.c (working copy) *** phi_translate_1 (pre_expr expr, bitmap_s *** 1729,1734 --- 1729,1738 edge e = find_edge (pred, gimple_bb (def_stmt)); tree def = PHI_ARG_DEF (def_stmt, e-dest_idx); + /* Valueize it. */ + if (TREE_CODE (def) == SSA_NAME) + def = VN_INFO (def)-valnum; + /* Handle constant. */ if (is_gimple_min_invariant (def)) return get_or_alloc_expr_for_constant (def); Index: gcc/testsuite/gcc.dg/torture/pr55862.c === *** gcc/testsuite/gcc.dg/torture/pr55862.c (revision 0) --- gcc/testsuite/gcc.dg/torture/pr55862.c (working copy) *** *** 0 --- 1,31 + /* { dg-do compile } */ + + int g, a, *b; + + void f(void) + { + int *p; + + if(g) + { + int **k = p; + + for(; g; p++) + for(a = 0; a 1; a++) + { + int *c = p; + label2: + if(a 1) + *c = 0; + } + + goto label1; + + while(g++) + for(*b = 0; *b; b++) + label1: + ; + } + + goto label2; + }
Re: Use libstdc++-raw-cxx.m4 in libjava
On Fri, Jan 4, 2013 at 2:06 AM, Andreas Schwab sch...@linux-m68k.org wrote: H.J. Lu hjl.to...@gmail.com writes: On Thu, Jan 3, 2013 at 10:09 AM, Andreas Schwab sch...@linux-m68k.org wrote: H.J. Lu hjl.to...@gmail.com writes: diff --git a/libjava/Makefile.am b/libjava/Makefile.am index c6c84e4..dd08a4f 100644 --- a/libjava/Makefile.am +++ b/libjava/Makefile.am @@ -594,7 +594,7 @@ lib_gnu_awt_xlib_la_CPPFLAGS = \ $(AM_CPPFLAGS) \ $(LIBSTDCXX_RAW_CXX_CXXFLAGS) ## The mysterious backslash in the grep pattern is consumed by make. -lib_gnu_awt_xlib_la_LDFLAGS = $(LIBSTDCXX_RAW_CXX_LDLAGS) \ +lib_gnu_awt_xlib_la_LDFLAGS = $(LIBSTDCXX_RAW_CXX_LIBADD) \ @X_PRE_LIBS@ @X_LIBS@ -lX11 @X_EXTRA_LIBS@ \ -rpath $(toolexeclibdir) $(LIBJAVA_LDFLAGS_NOUNDEF) \ -version-info `grep -v '^\#' $(srcdir)/libtool-version` $(LIBGCJ_LD_SYMBOLIC) It is still wrong to use LDFLAGS for libraries to be linked in. All of $(LIBSTDCXX_RAW_CXX_LIBADD) @X_PRE_LIBS@ @X_LIBS@ -lX11 @X_EXTRA_LIBS@ should be on lib_gnu_awt_xlib_la_LDADD. This was how it was done before my change. If we want to make a change, I can submit a separate patch. Libraries should never occur before the objects which reference them. This is in GCC 4.7: lib_gnu_awt_xlib_la_LDFLAGS = ../libstdc++-v3/src/libstdc++.la \ @X_PRE_LIBS@ @X_LIBS@ -lX11 @X_EXTRA_LIBS@ \ -rpath $(toolexeclibdir) $(LIBJAVA_LDFLAGS_NOUNDEF) \ -version-info `grep -v '^\#' $(srcdir)/libtool-version` $(LIBGCJ_LD_SYMBOLIC) lib_gnu_awt_xlib_la_LINK = $(LIBLINK) $(lib_gnu_awt_xlib_la_LDFLAGS) \ $(lib_gnu_awt_xlib_la_version_arg) It does put libraries first. If it is a real bug, it should be fixed separately. -- H.J.
Re: [C++ Patch] PR 54526 (again)
OK. Jason
Re: [committed] 2011 and 2012 Copyright year updates
On Fri, Jan 4, 2013 at 4:54 AM, Jakub Jelinek ja...@redhat.com wrote: Hi! I've run a script to notice gcc maintained files with FSF copyright that have been modified in 2011 and/or 2012 (according to svn log, ignoring r168438 and r184997 commits), but didn't have years 2011 and/or 2012 included in Copyright lines. I've kept the preexisting style, so where year ranges were used, updated those if needed, if not, kept the year lists. Can't we just move to ranges of years now that the FSF approves of them. They even approve of ranges where a file was not touched during that year. This seems better than listing all the years out. Thanks, Andrew
C++ PATCH for c++/55877 (wrong linkage for nested classes)
When an anonymous class gets a linkage name from a typedef, we recalculate its visibility; anonymous types have internal linkage, but types with linkage names have external linkage and therefore can have visibility. But we were forgetting to also recalculate the visibility of any nested types, which were also affected by the former anonymity of the enclosing class. Tested x86_64-pc-linux-gnu, applying to trunk, 4.7, 4.6. commit 28beb0d493a46f126f23d15216d4ae9279223514 Author: Jason Merrill ja...@redhat.com Date: Fri Jan 4 10:50:46 2013 -0500 PR c++/55877 * decl.c (reset_type_linkage, bt_reset_linkage): New. (grokdeclarator): Use reset_type_linkage. * name-lookup.c (binding_table_foreach): Handle null table. * tree.c (decl_anon_ns_mem_p): Check TYPE_MAIN_DECL, not TYPE_NAME. diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c index 5c268b1..9640824 100644 --- a/gcc/cp/decl.c +++ b/gcc/cp/decl.c @@ -8513,6 +8513,23 @@ check_var_type (tree identifier, tree type) return type; } +/* Functions for adjusting the visibility of a tagged type and its nested + types when it gets a name for linkage purposes from a typedef. */ + +static void bt_reset_linkage (binding_entry, void *); +static void +reset_type_linkage (tree type) +{ + set_linkage_according_to_type (type, TYPE_MAIN_DECL (type)); + if (CLASS_TYPE_P (type)) +binding_table_foreach (CLASSTYPE_NESTED_UTDS (type), bt_reset_linkage, NULL); +} +static void +bt_reset_linkage (binding_entry b, void */*data*/) +{ + reset_type_linkage (b-type); +} + /* Given declspecs and a declarator (abstract or otherwise), determine the name and type of the object declared and construct a DECL node for it. @@ -10053,8 +10070,7 @@ grokdeclarator (const cp_declarator *declarator, = TYPE_IDENTIFIER (type); /* Adjust linkage now that we aren't anonymous anymore. */ - set_linkage_according_to_type (type, TYPE_MAIN_DECL (type)); - determine_visibility (TYPE_MAIN_DECL (type)); + reset_type_linkage (type); /* FIXME remangle member functions; member functions of a type with external linkage have external linkage. */ diff --git a/gcc/cp/name-lookup.c b/gcc/cp/name-lookup.c index 87b1f51..754e830 100644 --- a/gcc/cp/name-lookup.c +++ b/gcc/cp/name-lookup.c @@ -251,9 +251,13 @@ binding_table_find (binding_table table, tree name) void binding_table_foreach (binding_table table, bt_foreach_proc proc, void *data) { - const size_t chain_count = table-chain_count; + size_t chain_count; size_t i; + if (!table) +return; + + chain_count = table-chain_count; for (i = 0; i chain_count; ++i) { binding_entry entry = table-chain[i]; diff --git a/gcc/cp/tree.c b/gcc/cp/tree.c index c658582..fcab1a4 100644 --- a/gcc/cp/tree.c +++ b/gcc/cp/tree.c @@ -2404,7 +2404,7 @@ decl_anon_ns_mem_p (const_tree decl) /* Classes and namespaces inside anonymous namespaces have TREE_PUBLIC == 0, so we can shortcut the search. */ else if (TYPE_P (decl)) - return (TREE_PUBLIC (TYPE_NAME (decl)) == 0); + return (TREE_PUBLIC (TYPE_MAIN_DECL (decl)) == 0); else if (TREE_CODE (decl) == NAMESPACE_DECL) return (TREE_PUBLIC (decl) == 0); else diff --git a/gcc/testsuite/g++.dg/ext/visibility/anon11.C b/gcc/testsuite/g++.dg/ext/visibility/anon11.C new file mode 100644 index 000..dfb4f12 --- /dev/null +++ b/gcc/testsuite/g++.dg/ext/visibility/anon11.C @@ -0,0 +1,13 @@ +// PR c++/55877 +// { dg-final { scan-assembler-not \\.local } } + +typedef struct { + typedef enum { X, Y } A; + typedef struct { } B; + struct C { }; +} D; + +D d; +D::A a; +D::B b; +D::C c;
Re: [committed] 2011 and 2012 Copyright year updates
On Fri, Jan 04, 2013 at 08:44:13AM -0800, Andrew Pinski wrote: On Fri, Jan 4, 2013 at 4:54 AM, Jakub Jelinek ja...@redhat.com wrote: I've run a script to notice gcc maintained files with FSF copyright that have been modified in 2011 and/or 2012 (according to svn log, ignoring r168438 and r184997 commits), but didn't have years 2011 and/or 2012 included in Copyright lines. I've kept the preexisting style, so where year ranges were used, updated those if needed, if not, kept the year lists. Can't we just move to ranges of years now that the FSF approves of them. They even approve of ranges where a file was not touched during that year. This seems better than listing all the years out. If somebody is willing to do the conversion, sure, but even with some scripting that is going to be lots of work. Even this patch took more than 6 hours of svn log, some scripting and a few hours of manual work, while the conversion would take IMHO more than that. Jakub
Re: [google][4.7] Allow function reordering linker plugin to separate hot and cold code into different ELF segments
The code looks fine to me. Please consider David's comments about the option name. -Rong On Thu, Jan 3, 2013 at 9:14 PM, Xinliang David Li davi...@google.com wrote: Is it better to change the option to something like: split_segment|nosplit-segment or split_segment=yes|no David On Thu, Jan 3, 2013 at 5:41 PM, Sriraman Tallam tmsri...@google.com wrote: Hi Rong, The following patch modifies the behaviour of the linker plugin to not create a separate segment for cold sections by default. Separate segments can be created with the plugin option segment=cold. Is this alright to commit? Thanks, -Sri. On Mon, Dec 17, 2012 at 11:14 AM, Sriraman Tallam tmsri...@google.com wrote: I have committed this patch. Thanks, -Sri. On Fri, Dec 14, 2012 at 4:16 PM, Rong Xu x...@google.com wrote: Looks good to me for google/gcc-4_7 branch. Thanks, -Rong On Fri, Dec 14, 2012 at 3:42 PM, Sriraman Tallam tmsri...@google.com wrote: Hi Rong, Please review this code. This code allows the function reordering plugin to separate hot and cold code into different ELF segments. This would allow optimizations like mapping the hot code alone to huge pages. With this patch, by default, the plugin maps .text.unlikely sections into a separate ELF segment. This can be turned off with plugin option --segment=none. The include/plugin-api.h changes are a backport from trunk. Thanks, -Sri.
[google 4_7] Backport r194909 (:: is incorrectly treated as digraph ...) to google/gcc-4_7 branch (issue7028052)
Back-port revision 194909 to google/gcc-4_7 branch: http://gcc.gnu.org/viewcvs?root=gccview=revrev=194909 http://gcc.gnu.org/bugzilla/show_bug.cgi?id=54526 Google ref: b/7427993 Index: gcc/testsuite/g++.old-deja/g++.other/crash28.C === --- gcc/testsuite/g++.old-deja/g++.other/crash28.C (revision 194909) +++ gcc/testsuite/g++.old-deja/g++.other/crash28.C (working copy) @@ -31,5 +31,5 @@ }; void foo::x() throw(bar) { - if (!b) throw bar (static_cast::N::X*(this)); // { dg-error lambda expressions|expected } parse error + if (!b) throw bar (static_cast::N::X*(this)); // { dg-error lambda expressions|expected|invalid } parse error } Index: libcpp/lex.c === --- libcpp/lex.c(revision 194909) +++ libcpp/lex.c(working copy) @@ -2224,6 +2224,17 @@ { if (*buffer-cur == ':') { + /* C++11 [2.5/3 lex.pptoken], Otherwise, if the next +three characters are :: and the subsequent character +is neither : nor , the is treated as a preprocessor +token by itself. */ + if (CPP_OPTION (pfile, cplusplus) + (CPP_OPTION (pfile, lang) == CLK_CXX11 + || CPP_OPTION (pfile, lang) == CLK_GNUCXX11) + buffer-cur[1] == ':' + buffer-cur[2] != ':' buffer-cur[2] != '') + break; + buffer-cur++; result-flags |= DIGRAPH; result-type = CPP_OPEN_SQUARE; -- This patch is available for review at http://codereview.appspot.com/7028052
Re: [google 4_7] Backport r194909 (:: is incorrectly treated as digraph ...) to google/gcc-4_7 branch (issue7028052)
ok. thanks, David On Fri, Jan 4, 2013 at 9:32 AM, Paul Pluzhnikov ppluzhni...@google.com wrote: Back-port revision 194909 to google/gcc-4_7 branch: http://gcc.gnu.org/viewcvs?root=gccview=revrev=194909 http://gcc.gnu.org/bugzilla/show_bug.cgi?id=54526 Google ref: b/7427993 Index: gcc/testsuite/g++.old-deja/g++.other/crash28.C === --- gcc/testsuite/g++.old-deja/g++.other/crash28.C (revision 194909) +++ gcc/testsuite/g++.old-deja/g++.other/crash28.C (working copy) @@ -31,5 +31,5 @@ }; void foo::x() throw(bar) { - if (!b) throw bar (static_cast::N::X*(this)); // { dg-error lambda expressions|expected } parse error + if (!b) throw bar (static_cast::N::X*(this)); // { dg-error lambda expressions|expected|invalid } parse error } Index: libcpp/lex.c === --- libcpp/lex.c(revision 194909) +++ libcpp/lex.c(working copy) @@ -2224,6 +2224,17 @@ { if (*buffer-cur == ':') { + /* C++11 [2.5/3 lex.pptoken], Otherwise, if the next +three characters are :: and the subsequent character +is neither : nor , the is treated as a preprocessor +token by itself. */ + if (CPP_OPTION (pfile, cplusplus) + (CPP_OPTION (pfile, lang) == CLK_CXX11 + || CPP_OPTION (pfile, lang) == CLK_GNUCXX11) + buffer-cur[1] == ':' + buffer-cur[2] != ':' buffer-cur[2] != '') + break; + buffer-cur++; result-flags |= DIGRAPH; result-type = CPP_OPEN_SQUARE; -- This patch is available for review at http://codereview.appspot.com/7028052
Re: [Patch, fortran] [4.7/4.8 Regression] [OOP] ICE on invalid: gfc_variable_attr(): Bad array reference
Dear Tobias, Thanks for the review. The patched trunk is just now bootstrapping and regtesting prior to commitment. I have added a check for co-indexed selectors in resolve.c and have added tests in select_type_31.f03 for co-indexing and vector indices. I am now turning my attention to FINAL :-) Cheers Paul PS BTW co-arrays fail similarly as selectors in SELECT_TYPE and in ASSOCIATE. Do I assume correctly that the associate variable should be a co-array? On 2 January 2013 23:48, Tobias Burnus bur...@net-b.de wrote: Dear Paul, First, the new patch is fine from my side. (Although, I think the test case should also include the vector-section example.) Thanks for working on that regression. Paul Richard Thomas wrote: First of all, thanks for the review! I still owe you my comments on FINAL; I got lost in trying to fix these various regressions :-) I promise that I'll come back to you first thing tomorrow. I am looking forward to them - and in particular to a patch review of the FINAL patches. I am also interested in your comment to my LOGICAL in BIND(C) procedures patch, namely http://gcc.gnu.org/ml/fortran/2012-12/msg00200.html It looks mostly okay; however, you do not handle vector sections correctly, which leads to an ICE. Without your patch, one gets: Error: CLASS selector at (1) needs a temporary which is not yet implemented With your patch, it fails as one has: This was fairly easily fixed - see attached. Thanks. By the way, I have filled a PR to track this not yet implemented issue: http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55849 I am not quite sure whether the following ICE has the same cause or a different one, but it also ICEs with your patch applied: select type (component = self%cb[4]) This co-array example was never OK, as far as I can tell. The error is similar to that of the PR. However, co-arrays were just never handled at all again, as far as I can tell. I'll have a go at it tomorrow night. I recall that we did add some coarray support for polymorphic variables. At least with coarray arrays (contrary to coarray scalars) it seems to compile. But it is very likely that select type never worked with coarrays or coarray scalars. Note that the coindexd select type (component = self%cb[4]) is invalid (C803; PR55850 (a)). However, the same failure occurs for noncoindexed valid selector: select type (component = self%cb) (See also PR 55850 for some other SELECT TYPE issues I found.) Tobias -- The knack of flying is learning how to throw yourself at the ground and miss. --Hitchhikers Guide to the Galaxy
Merge from trunk to gccgo branch
I've merged trunk revision 194911 to the gccgo branch. Ian
[PATCH, i386]: Call convert_to_mode unconditionally.
Hello! As suggested by rth at [1]. 2013-01-04 Uros Bizjak ubiz...@gmail.com * config/i386/i386.c (ix86_legitimize_address): Call convert_to_mode unconditionally. (ix86_expand_move): Ditto. (ix86_zero_extend_to_Pmode): Ditto. (ix86_expand_call): Ditto. (ix86_expand_special_args_builtin): Ditto. (ix86_expand_builtin): Ditto. Bootstrapped, regression tested on x86_64-pc-linux-gnu {,-m32} and committed to mainline SVN. [1] http://gcc.gnu.org/ml/gcc-patches/2013-01/msg00145.html Uros. Index: config/i386/i386.c === --- config/i386/i386.c (revision 194883) +++ config/i386/i386.c (working copy) @@ -13247,8 +13247,7 @@ ix86_legitimize_address (rtx x, rtx oldx ATTRIBUTE rtx val = force_operand (XEXP (x, 1), temp); if (val != temp) { - if (GET_MODE (val) != Pmode) - val = convert_to_mode (Pmode, val, 1); + val = convert_to_mode (Pmode, val, 1); emit_move_insn (temp, val); } @@ -13262,8 +13261,7 @@ ix86_legitimize_address (rtx x, rtx oldx ATTRIBUTE rtx val = force_operand (XEXP (x, 0), temp); if (val != temp) { - if (GET_MODE (val) != Pmode) - val = convert_to_mode (Pmode, val, 1); + val = convert_to_mode (Pmode, val, 1); emit_move_insn (temp, val); } @@ -15931,8 +15929,7 @@ ix86_expand_move (enum machine_mode mode, rtx oper op1 = force_operand (op1, op0); if (op1 == op0) return; - if (GET_MODE (op1) != mode) - op1 = convert_to_mode (mode, op1, 1); + op1 = convert_to_mode (mode, op1, 1); } else if (TARGET_DLLIMPORT_DECL_ATTRIBUTES SYMBOL_REF_DLLIMPORT_P (op1)) @@ -16013,8 +16010,7 @@ ix86_expand_move (enum machine_mode mode, rtx oper op1 = legitimize_pic_address (op1, reg); if (op0 == op1) return; - if (GET_MODE (op1) != mode) - op1 = convert_to_mode (mode, op1, 1); + op1 = convert_to_mode (mode, op1, 1); } } } @@ -21650,9 +21646,7 @@ ix86_adjust_counter (rtx countreg, HOST_WIDE_INT v rtx ix86_zero_extend_to_Pmode (rtx exp) { - if (GET_MODE (exp) != Pmode) -exp = convert_to_mode (Pmode, exp, 1); - return force_reg (Pmode, exp); + return force_reg (Pmode, convert_to_mode (Pmode, exp, 1)); } /* Divide COUNTREG by SCALE. */ @@ -23624,9 +23618,7 @@ ix86_expand_call (rtx retval, rtx fnaddr, rtx call ? !sibcall_insn_operand (XEXP (fnaddr, 0), word_mode) : !call_insn_operand (XEXP (fnaddr, 0), word_mode)) { - fnaddr = XEXP (fnaddr, 0); - if (GET_MODE (fnaddr) != word_mode) - fnaddr = convert_to_mode (word_mode, fnaddr, 1); + fnaddr = convert_to_mode (word_mode, XEXP (fnaddr, 0), 1); fnaddr = gen_rtx_MEM (QImode, copy_to_mode_reg (word_mode, fnaddr)); } @@ -31276,9 +31268,8 @@ ix86_expand_special_args_builtin (const struct bui gcc_assert (target == 0); if (memory) { - if (GET_MODE (op) != Pmode) - op = convert_to_mode (Pmode, op, 1); - target = gen_rtx_MEM (tmode, force_reg (Pmode, op)); + op = force_reg (Pmode, convert_to_mode (Pmode, op, 1)); + target = gen_rtx_MEM (tmode, op); } else target = force_reg (tmode, op); @@ -31322,9 +31313,8 @@ ix86_expand_special_args_builtin (const struct bui if (i == memory) { /* This must be the memory operand. */ - if (GET_MODE (op) != Pmode) - op = convert_to_mode (Pmode, op, 1); - op = gen_rtx_MEM (mode, force_reg (Pmode, op)); + op = force_reg (Pmode, convert_to_mode (Pmode, op, 1)); + op = gen_rtx_MEM (mode, op); gcc_assert (GET_MODE (op) == mode || GET_MODE (op) == VOIDmode); } @@ -31572,9 +31562,8 @@ ix86_expand_builtin (tree exp, rtx target, rtx sub mode1 = insn_data[icode].operand[1].mode; mode2 = insn_data[icode].operand[2].mode; - if (GET_MODE (op0) != Pmode) - op0 = convert_to_mode (Pmode, op0, 1); - op0 = gen_rtx_MEM (mode1, force_reg (Pmode, op0)); + op0 = force_reg (Pmode, convert_to_mode (Pmode, op0, 1)); + op0 = gen_rtx_MEM (mode1, op0); if (!insn_data[icode].operand[0].predicate (op0, mode0)) op0 = copy_to_mode_reg (mode0, op0); @@ -31605,11 +31594,7 @@ ix86_expand_builtin (tree exp, rtx target, rtx sub op0 = expand_normal (arg0); icode = CODE_FOR_sse2_clflush; if (!insn_data[icode].operand[0].predicate (op0, Pmode)) - { - if (GET_MODE (op0) != Pmode) - op0 = convert_to_mode (Pmode, op0, 1); - op0 = force_reg (Pmode, op0); -
Re: RFA: Fix ICE on PARALLEL returns when expand builtins
Hmm, how would anyone else get at the padding info dealing with the parallel? This looks broken if we need access to the type :/ Yes, you need to access the type for return values, but that's not really new. Eric, any comments? Richard's patch seems fine to me, modulo the name of the routine; I'd rather use maybe_emit_group_store or something along these lines. -- Eric Botcazou
libiberty patch committed: Minor optimization (PR 54800)
PR 54800 points out a minor optimization uncovered by cppcheck. This optimization is not important, but we might as well fix it in case cppcheck comes up with something useful. Tested by Iain Sandoe (thanks!). Committed to mainline. Ian 2013-01-04 Ian Lance Taylor i...@google.com PR other/54800 * simple-object-mach-o.c (simple_object_mach_o_segment): Don't bother to zero out a buffer we are about to set anyhow. Index: simple-object-mach-o.c === --- simple-object-mach-o.c (revision 194911) +++ simple-object-mach-o.c (working copy) @@ -1,5 +1,5 @@ /* simple-object-mach-o.c -- routines to manipulate Mach-O object files. - Copyright 2010, 2011 Free Software Foundation, Inc. + Copyright 2010, 2011, 2013 Free Software Foundation, Inc. Written by Ian Lance Taylor, Google. This program is free software; you can redistribute it and/or modify it @@ -701,12 +701,13 @@ simple_object_mach_o_segment (simple_obj /* Otherwise, make a name like __segment,__section as per the convention in mach-o asm. */ name = namebuf[0]; - memset (namebuf, 0, MACH_O_NAME_LEN * 2 + 2); memcpy (namebuf, (char *) sechdr + segname_offset, MACH_O_NAME_LEN); + namebuf[MACH_O_NAME_LEN] = '\0'; l = strlen (namebuf); namebuf[l] = ','; memcpy (namebuf + l + 1, (char *) sechdr + sectname_offset, MACH_O_NAME_LEN); + namebuf[l + 1 + MACH_O_NAME_LEN] = '\0'; } simple_object_mach_o_section_info (omr-is_big_endian, is_32, sechdr,
[PATCH] Remove unnecessaily included limits.h in libgcc2.c
I filed http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55743 (my first upstream gcc bug so be gentle :-) Details are there but the short version is that the limits.h inclusion in libgcc2.c is now a relic because the constants that it brings in are no longer used (since http://repo.or.cz/w/official-gcc.git/blobdiff/49f0f270673c4512c11f72a038b84c321ae5534a..7429c938827aa98bf3b02c4ac89510f4d28ef0b1:/gcc/libgcc2.c ) And this inclusion can break --without-headers bootstrapping (which is how I noticed it). Doko poked me to send the patch to this list for consideration for inclusion in trunk. --- gcc-4.7-4.7.2/src/libgcc/libgcc2.c.orig 2011-11-02 15:26:35.0 + +++ gcc-4.7-4.7.2/src/libgcc/libgcc2.c 2012-12-18 19:33:40.0 + @@ -1676,18 +1676,6 @@ #endif #if defined(L_fixunsxfsi) LIBGCC2_HAS_XF_MODE -/* Reenable the normal types, in case limits.h needs them. */ -#undef char -#undef short -#undef int -#undef long -#undef unsigned -#undef float -#undef double -#undef MIN -#undef MAX -#include limits.h - UWtype __fixunsxfSI (XFtype a) { @@ -1698,18 +1686,6 @@ #endif #if defined(L_fixunsdfsi) LIBGCC2_HAS_DF_MODE -/* Reenable the normal types, in case limits.h needs them. */ -#undef char -#undef short -#undef int -#undef long -#undef unsigned -#undef float -#undef double -#undef MIN -#undef MAX -#include limits.h - UWtype __fixunsdfSI (DFtype a) { @@ -1720,18 +1696,6 @@ #endif #if defined(L_fixunssfsi) LIBGCC2_HAS_SF_MODE -/* Reenable the normal types, in case limits.h needs them. */ -#undef char -#undef short -#undef int -#undef long -#undef unsigned -#undef float -#undef double -#undef MIN -#undef MAX -#include limits.h - UWtype __fixunssfSI (SFtype a) { Wookey -- Principal hats: Linaro, Emdebian, Wookware, Balloonboard, ARM http://wookware.org/
Re: [Patch, fortran] [4.7/4.8 Regression] [OOP] ICE on invalid: gfc_variable_attr(): Bad array reference
Dear Tobias, Committed as revision 194916. I am leaving the PR open to deal with 4.7 just as soon as I have downloaded it! Thanks again for the careful review. Cheers Paul On 2 January 2013 23:48, Tobias Burnus bur...@net-b.de wrote: Dear Paul, First, the new patch is fine from my side. (Although, I think the test case should also include the vector-section example.) Thanks for working on that regression. Paul Richard Thomas wrote: First of all, thanks for the review! I still owe you my comments on FINAL; I got lost in trying to fix these various regressions :-) I promise that I'll come back to you first thing tomorrow. I am looking forward to them - and in particular to a patch review of the FINAL patches. I am also interested in your comment to my LOGICAL in BIND(C) procedures patch, namely http://gcc.gnu.org/ml/fortran/2012-12/msg00200.html It looks mostly okay; however, you do not handle vector sections correctly, which leads to an ICE. Without your patch, one gets: Error: CLASS selector at (1) needs a temporary which is not yet implemented With your patch, it fails as one has: This was fairly easily fixed - see attached. Thanks. By the way, I have filled a PR to track this not yet implemented issue: http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55849 I am not quite sure whether the following ICE has the same cause or a different one, but it also ICEs with your patch applied: select type (component = self%cb[4]) This co-array example was never OK, as far as I can tell. The error is similar to that of the PR. However, co-arrays were just never handled at all again, as far as I can tell. I'll have a go at it tomorrow night. I recall that we did add some coarray support for polymorphic variables. At least with coarray arrays (contrary to coarray scalars) it seems to compile. But it is very likely that select type never worked with coarrays or coarray scalars. Note that the coindexd select type (component = self%cb[4]) is invalid (C803; PR55850 (a)). However, the same failure occurs for noncoindexed valid selector: select type (component = self%cb) (See also PR 55850 for some other SELECT TYPE issues I found.) Tobias -- The knack of flying is learning how to throw yourself at the ground and miss. --Hitchhikers Guide to the Galaxy
Re: [Patch, Fortran] FINAL (prep patches 1/5): Add _final to intrinsic vtables for CLASS(*)
Dear Tobias, This one is 'obvious' - OK for trunk. Thanks Paul On 31 December 2012 13:16, Tobias Burnus bur...@net-b.de wrote: This simple patch fixes a wrong indent and adds a _final component to the virtual tables generated for intrinsic types. This patch not only prepares the trunk for finalization support, it also avoids ABI issues which a later addition would cause. (For intrinsic types, changing the .mod version number will not reliable force a recompilation.) Build on x86-64-gnu-linux. OK for the trunk? Tobias -- The knack of flying is learning how to throw yourself at the ground and miss. --Hitchhikers Guide to the Galaxy
Re: [Patch, Fortran] FINAL (prep patches 2/5): Add internal STRIDE intrinsic
Dear Tobias, This one is also OK for trunk. It does cross my mind, however, that we should offer STRIDE as a gcc extension, in anticipation of F201x. Cheers Paul On 31 December 2012 13:17, Tobias Burnus bur...@net-b.de wrote: The attached patch adds a new - internal only - intrinsic, STRIDE, which returns the stride of an array with array descriptor; for an integer :: array(5,5), the stride of dim=2 is 5. This new intrinsic is only internally available and will be used by the finalization wrapper to handle noncontiguous arrays. Build on x86-64-gnu-linux. OK for the trunk? Tobias -- The knack of flying is learning how to throw yourself at the ground and miss. --Hitchhikers Guide to the Galaxy
Re: [Patch, Fortran] FINAL (prep patches 3/5): Auxiliary functions for calling the FINAL wrapper
Dear Tobias, s/Argument is neither a pointer/allocatble/Argument is neither a pointer/allocatable/ Why have you all the asserts for se.pre and se.post? Did you have trouble with this or is there some hidden nasty that I am unaware of? Should you not either return rapidly for an intrinsic type or add a gcc_assert to pick up accidental calls? Modulo these points this patch is OK for trunk. Cheers Paul On 31 December 2012 13:21, Tobias Burnus bur...@net-b.de wrote: This patch adds one auxiliary functions, which will be used when invoking the finalization wrapper. It is currently unused. Build on x86-64-gnu-linux. OK for the trunk? Tobias -- The knack of flying is learning how to throw yourself at the ground and miss. --Hitchhikers Guide to the Galaxy
Re: [RFC PATCH, i386]: Use %r15 for REAL_PIC_OFFSET_TABLE_REGNUM on x86_64
On Fri, Jan 04, 2013 at 10:39:05PM +0100, Leif Ekblad wrote: I just tried the patch, but it seems to produce some problems for me. The other patch which used a 64-bit specific register (r15) instead of rbx was easier to adapt to. The problem for me is that syscalls might clobber higher-half of all 32-bit registers, and that I cannot use the stack to preserve rbx in the call because of the red-zone. Of course you can use the stack, just subtract the red zone size (plus whatever you need) from %rsp first, then save to stack, do syscall, then restore from stack and finally increment %rsp back. Jakub
[Patch, libfortran] Improve performance of byte swapped IO
Hi, currently byte swapped unformatted IO can be quite slow compared to the same code with no byte swapping. There are two major reasons for this: 1) The byte swapping code path resorts to transferring data element by element, leading to a lot of overhead in the IO library. 2) The function used for the actual byte swapping, reverse_memcpy , while able to handle general element sizes, is not particularly fast, especially considering that many CPU's have fast byte swapping instructions (e.g. BSWAP on x86). In order to access these fast byte swapping instructions, gcc provides the __builtin_bswap{16,32,64} builtins, falling back to libgcc code for targets that lack support. The attached patch fixes these issues. For issue (1), the read path uses in-place byte swapping of the data that has been read into the user buffer, while the write path uses a larger temporary buffer (since we are not allowed to modify the user supplied data in this case). For issue(2), the patch uses __builtin_bswap{16,32,64} where appropriate, only falling back to reverse_memcpy for other sizes. With the attached test program run on a tmpfs filesystem to avoid doing actual disk IO, I get the following: - With no byte swapping: Unformatted sequential write/read performance test Record size Write MB/s Read MB/s == 4 52.72384281742220272.721158943820441 8 77.50829689085638697.237815640377221 16 110.26209495334321143.80831184546381 32 173.94872143231535221.89704881197937 64 282.19818562682684373.77854583735541 128 442.22084579742244628.80041029142183 256 636.69620860705299966.37723642576316 512 826.059688407380801380.8835166612221 1024 987.186864651975611763.5990036057208 2048 1047.67215441917102058.0875622043550 4096 1115.58171471348012251.8731832850176 8192 1191.50211509965902283.8893409728184 16384 1417.61109095193912441.0530373866482 32768 1570.44134790460182543.0836384048471 65536 1673.03787065029662651.2182395008308 131072 1697.49442461884452688.2398923155783 262144 1669.63298621458722735.668973292 524288 1594.46699352315522697.7208298823243 - Before patch, with byte swapping: Unformatted sequential write/read performance test Record size Write MB/s Read MB/s == 4 50.57281289368979368.858701306591627 8 58.68851330069031781.591733130441327 16 73.55118848060782096.638995590227665 32 91.593767813989018116.65817140076214 64 107.41379323761915128.32512066346368 128 121.33499652432221147.80777892360237 256 128.99627771476628155.91619889220266 512 135.02742063670030161.30042382365372 1024 137.02276709585524164.11267056940963 2048 138.62774254302394165.22456826188971 4096 139.27695763341924166.34707691429571 8192 147.64584950575932166.59526981475742 16384 147.91235479266419166.77890398940283 32768 150.77029430529927166.90834867503827 65536 151.59474472614465166.84075600288520 131072 155.75202672623249166.96550283835097 262144 155.36506626794849166.78075976148853 524288 155.64305086921487167.44468828946083 - After patch, with byte swapping: Unformatted sequential write/read performance test Record size Write MB/s Read MB/s == 4 49.41477177682136170.808060042286343 8 72.91815640245977293.234093684373946 16 102.72461544178078136.21700026949074 32 160.57240200649090205.97612602315186 64 249.32082957447636331.85515010907363 128 385.71299236810387522.06354804855266 256 535.40608912076459766.59668706247294 512 669.478641203685241006.4275938227961 1024 742.905388955002651187.9846039167674 2048 789.713405573405231333.8411634622269 4096 826.442532047316831395.5536995933605 8192 832.935403161166621361.4621716558986 16384 897.950819770101131469.0940087507722 32768 961.187363080333171533.7736812111871 65536 989.413849084968321564.7013916917260 131072 1003.61137620680401597.4063253370084 262144 980.030676643243961602.3188995993287
Re: [google][4.7] Allow function reordering linker plugin to separate hot and cold code into different ELF segments
Attached new patch. Thanks, -Sri. On Fri, Jan 4, 2013 at 9:12 AM, Rong Xu x...@google.com wrote: The code looks fine to me. Please consider David's comments about the option name. -Rong On Thu, Jan 3, 2013 at 9:14 PM, Xinliang David Li davi...@google.com wrote: Is it better to change the option to something like: split_segment|nosplit-segment or split_segment=yes|no David On Thu, Jan 3, 2013 at 5:41 PM, Sriraman Tallam tmsri...@google.com wrote: Hi Rong, The following patch modifies the behaviour of the linker plugin to not create a separate segment for cold sections by default. Separate segments can be created with the plugin option segment=cold. Is this alright to commit? Thanks, -Sri. On Mon, Dec 17, 2012 at 11:14 AM, Sriraman Tallam tmsri...@google.com wrote: I have committed this patch. Thanks, -Sri. On Fri, Dec 14, 2012 at 4:16 PM, Rong Xu x...@google.com wrote: Looks good to me for google/gcc-4_7 branch. Thanks, -Rong On Fri, Dec 14, 2012 at 3:42 PM, Sriraman Tallam tmsri...@google.com wrote: Hi Rong, Please review this code. This code allows the function reordering plugin to separate hot and cold code into different ELF segments. This would allow optimizations like mapping the hot code alone to huge pages. With this patch, by default, the plugin maps .text.unlikely sections into a separate ELF segment. This can be turned off with plugin option --segment=none. The include/plugin-api.h changes are a backport from trunk. Thanks, -Sri. Index: function_reordering_plugin/function_reordering_plugin.c === --- function_reordering_plugin/function_reordering_plugin.c (revision 194878) +++ function_reordering_plugin/function_reordering_plugin.c (working copy) @@ -33,9 +33,13 @@ along with this program; see the file COPYING3. I This plugin dumps the final layout order of the functions in a file called final_layout.txt. To change the output file, pass the new - file name with --plugin-opt. To dump to stderr instead, just pass - stderr to --plugin-opt. */ + file name with --plugin-opt,file=name. To dump to stderr instead, + just pass stderr as the file name. + This plugin also allows placing all functions found cold in a separate + segment. This can be enabled with the linker option: + --plugin-opt,split_segment=yes. */ + #if HAVE_STDINT_H #include stdint.h #endif @@ -89,9 +93,10 @@ static int is_api_exist = 0; /* The plugin does nothing when no-op is 1. */ static int no_op = 0; -/* The plugin does not create a new segment for unlikely code if - no_segment is set. */ -static int no_segment = 0; +/* The plugin creates a new segment for unlikely code if split_segment + is set. This can be set with the linker option: + --plugin-opt,split_segment=yes. */ +static int split_segment = 0; /* Copies new output file name out_file */ void get_filename (const char *name) @@ -110,7 +115,7 @@ process_option (const char *name) { const char *option_group = group=; const char *option_file = file=; - const char *option_segment = segment=; + const char *option_segment = split_segment=; /* Check if option is group= */ if (strncmp (name, option_group, strlen (option_group)) == 0) @@ -129,13 +134,14 @@ process_option (const char *name) return; } - /* Check if options is segment=none */ + /* Check if options is split_segment=[yes|no] */ if (strncmp (name, option_segment, strlen (option_segment)) == 0) { - if (strcmp (name + strlen (option_segment), none) == 0) - no_segment = 1; - else - no_segment = 0; + const char *option_val = name + strlen (option_segment); + if (strcmp (option_val, no) == 0) + split_segment = 0; + else if (strcmp (option_val, yes) == 0) + split_segment = 1; return; } @@ -244,7 +250,10 @@ claim_file_hook (const struct ld_plugin_input_file { /* Inform the linker to prepare for section reordering. */ (*allow_section_ordering) (); - (*allow_unique_segment_for_sections) (); + /* Inform the linker to allow certain sections to be placed in +a separate segment. */ + if (split_segment == 1) +(*allow_unique_segment_for_sections) (); is_ordering_specified = 1; } @@ -335,15 +344,29 @@ all_symbols_read_hook (void) if (out_file != NULL strcmp (out_file, stderr) != 0) fclose (fp); - /* Pass the new order of functions to the linker. */ - update_section_order (section_list, unlikely_segment_start); - assert (num_entries = unlikely_segment_end); - update_section_order (section_list, num_entries - unlikely_segment_end); - /* Map all unlikely code into a new segment. */ - if (no_segment == 0) -unique_segment_for_sections (.text.unlikely_executed, 0, 0x1000, -section_list + unlikely_segment_start, -
[PATCH] Adding target rdos to GCC
Change log: * config/gthr.m4: Added rdos thread header. * gcc/config/i386/i386.c: Provided a way to define a default setting for medium memory model and PIC. Provided a way to define a default value for large-data-threshold. * gcc/config/i386/i386.h: Defined default value for medium memory model PIC. * gcc/config/i386/rdos.h: Added new file for rdos target definition. * gcc/config.gcc: Added rdos target The change to gthr.m4 requires rebuilding the configure scripts. Tested with target rdos and rdos32. Is this ok for trunk? Regards, Leif Ekblad gcc.diff Description: Binary data
Re: [AARCH64] Optimize cmp in some cases
On Fri, Jan 4, 2013 at 1:34 AM, Richard Earnshaw rearn...@arm.com wrote: On 03/01/13 22:39, Andrew Pinski wrote: Hi, For aarch64, we don't CSE some cmp away. This patch fixes the case where we are CSE across some basic-blocks like: int f(int a, int b) { if(ab) return 1; if(ab) return -1; return 0; } --- CUT --- To fix this, I implemented the target hook TARGET_FIXED_CONDITION_CODE_REGS as there was already code in CSE which uses this target hook to find the extra setting of the CC registers. OK? Build and tested on aarch64-thunder-elf (using Cavium's internal simulator). Build a cross to aarch64-thunder-linux-gnu and a Canadian cross with that one for the native toolchain. Thanks, Andrew Pinski * config/aarch64/aarch64.c (aarch64_fixed_condition_code_regs): New function. (TARGET_FIXED_CONDITION_CODE_REGS): Define. * gcc.target/aarch64/cmp-1.c: New testcase. fixed_condition_code.diff.txt * config/aarch64/aarch64.c (aarch64_fixed_condition_code_regs): New function. (TARGET_FIXED_CONDITION_CODE_REGS): Define. * gcc.target/aarch64/cmp-1.c: New testcase. Index: config/aarch64/aarch64.c === --- config/aarch64/aarch64.c(revision 194872) +++ config/aarch64/aarch64.c(working copy) @@ -3041,6 +3041,16 @@ aarch64_const_double_zero_rtx_p (rtx x) return REAL_VALUES_EQUAL (r, dconst0); } +/* Return the fixed registers used for condition codes. */ + +static bool +aarch64_fixed_condition_code_regs (unsigned int *p1, unsigned int *p2) +{ + *p1 = CC_REGNUM; + *p2 = -1; Please use INVALID_REGNUM here (as the documentation states). The comment in target.def says: Up to two condition code registers are supported. If there is only one for this target, the int pointed at by the second argument should be set to -1. */ While tm.texi says: arguments point to the hard register numbers used for condition codes. When there is only one such register, as is true on most systems, the integer pointed to by @var{p2} should be set to @code{INVALID_REGNUM}. I had just read the comment in target.def when I was writing this patch which is why I had used -1. I agree INVALID_REGNUM is better. I will send out a patch to fix the comment in target.def later. Otherwise, OK. A backport to the AArch64-4.7 branch would be appreciated. I don't have time to do a backport and to test it. Thanks, Andrew Pinski
Re: [RFC PATCH, i386]: Use %r15 for REAL_PIC_OFFSET_TABLE_REGNUM on x86_64
OK. I know I can do that, but I would need to do it for every syscall since every syscall can potentially clobber rbx. Also, it is very strange that it is only one of the inlines the compiler complains about. I have another inline (which uses rbx as input), but that doesn't generate any error: #define RdosUserGateEbxEdiEcxParRetEax(nr, rbx, rdi, rcx, res) do { \ register int _id asm(r14) = nr; \ register typeof(rbx) _rbx asm(rbx) = (rbx); \ register typeof(rdi) _rdi asm(rdi) = (rdi); \ register typeof(rcx) _rcx asm(r8) = (rcx); \ register int _size asm(r12) = (rcx); \ asm volatile ( \ syscall\n\t \ jnc 1f\n\t \ xorq %%rax,%%rax\n\t \ 1: \n\t \ : =a (res) : r (_id), r (_rbx), r (_rdi), r (_rcx), r (_size) : rdx, rsi \ ); \ } while(0); inline int RdosWriteFile(int Handle, void *Buf, int Size) { int res; RdosUserGateEbxEdiEcxParRetEax(usergate_read_file, Handle, Buf, Size, res); return res; } Why is not this inline causing the problem? Might that be because it will not use rbx, but instead another register? OTOH, the code seems to work and rbx is not assigned to PIC at the point of the call, but to the defined parameter. Regards, Leif Ekblad - Original Message - From: Jakub Jelinek ja...@redhat.com To: Leif Ekblad l...@rdos.net Cc: Uros Bizjak ubiz...@gmail.com; Richard Henderson r...@redhat.com; Andi Kleen a...@firstfloor.org; Mike Frysinger vap...@gentoo.org; gcc-patches@gcc.gnu.org; j...@suse.cz; H.J. Lu hjl.to...@gmail.com Sent: Friday, January 04, 2013 10:42 PM Subject: Re: [RFC PATCH, i386]: Use %r15 for REAL_PIC_OFFSET_TABLE_REGNUM on x86_64 On Fri, Jan 04, 2013 at 10:39:05PM +0100, Leif Ekblad wrote: I just tried the patch, but it seems to produce some problems for me. The other patch which used a 64-bit specific register (r15) instead of rbx was easier to adapt to. The problem for me is that syscalls might clobber higher-half of all 32-bit registers, and that I cannot use the stack to preserve rbx in the call because of the red-zone. Of course you can use the stack, just subtract the red zone size (plus whatever you need) from %rsp first, then save to stack, do syscall, then restore from stack and finally increment %rsp back. Jakub
Re: [google][4.7] Allow function reordering linker plugin to separate hot and cold code into different ELF segments
Looks good -- but better with followup : 1) give a warning when the parameter to the option is not allowed; 2) add test cases if possible. David On Fri, Jan 4, 2013 at 2:19 PM, Sriraman Tallam tmsri...@google.com wrote: Attached new patch. Thanks, -Sri. On Fri, Jan 4, 2013 at 9:12 AM, Rong Xu x...@google.com wrote: The code looks fine to me. Please consider David's comments about the option name. -Rong On Thu, Jan 3, 2013 at 9:14 PM, Xinliang David Li davi...@google.com wrote: Is it better to change the option to something like: split_segment|nosplit-segment or split_segment=yes|no David On Thu, Jan 3, 2013 at 5:41 PM, Sriraman Tallam tmsri...@google.com wrote: Hi Rong, The following patch modifies the behaviour of the linker plugin to not create a separate segment for cold sections by default. Separate segments can be created with the plugin option segment=cold. Is this alright to commit? Thanks, -Sri. On Mon, Dec 17, 2012 at 11:14 AM, Sriraman Tallam tmsri...@google.com wrote: I have committed this patch. Thanks, -Sri. On Fri, Dec 14, 2012 at 4:16 PM, Rong Xu x...@google.com wrote: Looks good to me for google/gcc-4_7 branch. Thanks, -Rong On Fri, Dec 14, 2012 at 3:42 PM, Sriraman Tallam tmsri...@google.com wrote: Hi Rong, Please review this code. This code allows the function reordering plugin to separate hot and cold code into different ELF segments. This would allow optimizations like mapping the hot code alone to huge pages. With this patch, by default, the plugin maps .text.unlikely sections into a separate ELF segment. This can be turned off with plugin option --segment=none. The include/plugin-api.h changes are a backport from trunk. Thanks, -Sri.
Re: [PATCH] Adding target rdos to GCC
On Fri, Jan 4, 2013 at 2:22 PM, Leif Ekblad l...@rdos.net wrote: Change log: * config/gthr.m4: Added rdos thread header. * gcc/config/i386/i386.c: Provided a way to define a default setting for medium memory model and PIC. Provided a way to define a default value for large-data-threshold. * gcc/config/i386/i386.h: Defined default value for medium memory model PIC. * gcc/config/i386/rdos.h: Added new file for rdos target definition. * gcc/config.gcc: Added rdos target The change to gthr.m4 requires rebuilding the configure scripts. Tested with target rdos and rdos32. Is this ok for trunk? Regards, Leif Ekblad + #ifdef TARGET_SECTION_THRESHOLD + ix86_section_threshold = TARGET_SECTION_THRESHOLD; + #endif You should 1. Add DEFAULT_SECTION_THRESHOLD and set it to 65536. 2. Change the init value of ix86_section_threshold to -1. 3. Set ix86_section_threshold to DEFAULT_SECTION_THRESHOLD if it is -1. -- H.J.
Re: [Patch, libfortran] Improve performance of byte swapped IO
Janne Blomqvist blomqvist.ja...@gmail.com writes: diff --git a/libgfortran/io/file_pos.c b/libgfortran/io/file_pos.c index c8ecc3a..bf2250a 100644 --- a/libgfortran/io/file_pos.c +++ b/libgfortran/io/file_pos.c @@ -140,15 +140,21 @@ unformatted_backspace (st_parameter_filepos *fpp, gfc_unit *u) } else { + uint32_t u32; + uint64_t u64; switch (length) { case sizeof(GFC_INTEGER_4): - reverse_memcpy (m4, p, sizeof (m4)); + memcpy (u32, p, sizeof (u32)); + u32 = __builtin_bswap32 (u32); + m4 = *(GFC_INTEGER_4*)u32; Isn't that an aliasing violation? Andreas. -- Andreas Schwab, sch...@linux-m68k.org GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 And now for something completely different.
Re: [google 4_7] Backport r194909 (:: is incorrectly treated as digraph ...) to google/gcc-4_7 branch (issue7028052)
On Fri, Jan 4, 2013 at 9:41 AM, Xinliang David Li davi...@google.com wrote: ok. The patch as sent caused some breakage, I had to adjust test cases a bit. Submitting attached patch. Thanks, -- Paul Pluzhnikov Index: gcc/testsuite/g++.old-deja/g++.other/crash28.C === --- gcc/testsuite/g++.old-deja/g++.other/crash28.C (revision 194909) +++ gcc/testsuite/g++.old-deja/g++.other/crash28.C (working copy) @@ -31,5 +31,5 @@ }; void foo::x() throw(bar) { - if (!b) throw bar (static_cast::N::X*(this)); // { dg-error lambda expressions|expected } parse error + if (!b) throw bar (static_cast::N::X*(this)); // { dg-error lambda expressions|expected|invalid } parse error } Index: gcc/testsuite/g++.dg/parse/error12.C === --- gcc/testsuite/g++.dg/parse/error12.C(revision 194909) +++ gcc/testsuite/g++.dg/parse/error12.C(working copy) @@ -8,6 +8,6 @@ template class A struct Foo {}; -Foo::B foo; // { dg-bogus error error in place of warning } -// { dg-warning 4: '::' cannot begin a template-argument list warning :: { target *-*-* } 11 } -// { dg-message 4:':' is an alternate spelling for '.'. Insert whitespace between '' and '::' note : { target *-*-* } 11 } +Foo::B foo; // { dg-bogus error error in place of warning { target c++98 } } +// { dg-warning 4: '::' cannot begin a template-argument list warning :: { target c++98 } 11 } +// { dg-message 4:':' is an alternate spelling for '.'. Insert whitespace between '' and '::' note : { target c++98 } 11 } Index: gcc/testsuite/g++.dg/parse/error11.C === --- gcc/testsuite/g++.dg/parse/error11.C(revision 194909) +++ gcc/testsuite/g++.dg/parse/error11.C(working copy) @@ -16,22 +16,22 @@ }; void method(void) { -typename Foo::B::template Nested::B n; // { dg-error 17:'::' cannot begin 17-begin } -// { dg-message 17:':' is an alternate spelling 17-alt { target *-*-* } 19 } -// { dg-error 39:'::' cannot begin 39-begin { target *-*-* } 19 } -// { dg-message 39:':' is an alternate spelling 39-alt { target *-*-* } 19 } +typename Foo::B::template Nested::B n; // { dg-error 17:'::' cannot begin 17-begin { target c++98 } } +// { dg-message 17:':' is an alternate spelling 17-alt { target c++98 } 19 } +// { dg-error 39:'::' cannot begin 39-begin { target c++98 } 19 } +// { dg-message 39:':' is an alternate spelling 39-alt { target c++98 } 19 } n.template NestedB::method(); -n.template Nested::B::method(); // { dg-error 22:'::' cannot begin error } -// { dg-message 22:':' is an alternate note { target *-*-* } 24 } +n.template Nested::B::method(); // { dg-error 22:'::' cannot begin error { target c++98 } } +// { dg-message 22:':' is an alternate note { target c++98 } 24 } NestedB::method(); -Nested::B::method(); // { dg-error 11:'::' cannot begin error } -// { dg-message 11:':' is an alternate note { target *-*-* } 27 } +Nested::B::method(); // { dg-error 11:'::' cannot begin error { target c++98 } } +// { dg-message 11:':' is an alternate note { target c++98 } 27 } } }; template int N struct Foo2 {}; -template struct Foo2::B; // { dg-error 21:'::' cannot begin begin } -// { dg-message 21:':' is an alternate alt { target *-*-* } 33 } +template struct Foo2::B; // { dg-error 21:'::' cannot begin begin { target c++98 } } +// { dg-message 21:':' is an alternate alt { target c++98 } 33 } // { dg-message 25:type/value mismatch mismatch { target *-*-* } 33 } // { dg-error 25:expected a constant const { target *-*-* } 33 } @@ -39,11 +39,11 @@ void func(void) { - Foo::B f; // { dg-error cannot begin begin } -// { dg-message alternate spelling alt { target *-*-* } 42 } + Foo::B f; // { dg-error cannot begin begin { target c++98 } } +// { dg-message alternate spelling alt { target c++98 } 42 } f.FooB::method(); - f.Foo::B::method(); // { dg-error 8:cannot begin begin } -// { dg-message 8:alternate spelling alt { target *-*-* } 45 } + f.Foo::B::method(); // { dg-error 8:cannot begin begin { target c++98 } } +// { dg-message 8:alternate spelling alt { target c++98 } 45 } // Check cases where we the token sequence is the correct one, but there // was no digraph or whitespaces in the middle, so we should not emit @@ -63,9 +63,9 @@ Foo[::value] = 0; } -template struct Foo::B; // { dg-error 20:'::' cannot begin begin } -// { dg-message 20:is an alternate alt { target *-*-* } 66 } +template struct Foo::B; // { dg-error 20:'::' cannot begin begin { target c++98 } } +// { dg-message 20:is an alternate alt { target c++98 } 66 } // On the first error message, an additional note about the use of // -fpermissive should be present -// { dg-message 17:\\(if you use '-fpermissive' G\\+\\+ will accept your code\\) -fpermissive { target *-*-* } 19 } +// {
Re: [google 4_7] Backport r194909 (:: is incorrectly treated as digraph ...) to google/gcc-4_7 branch (issue7028052)
I saw only one test case fix patch from Paolo in trunk. Does this patch include more local fixes? David On Fri, Jan 4, 2013 at 3:03 PM, Paul Pluzhnikov ppluzhni...@google.com wrote: On Fri, Jan 4, 2013 at 9:41 AM, Xinliang David Li davi...@google.com wrote: ok. The patch as sent caused some breakage, I had to adjust test cases a bit. Submitting attached patch. Thanks, -- Paul Pluzhnikov
[Patch, Fortran] Remove the Fortran-only flag -fno-whole-file
This patch removes -fno-whole-file. (Actually, it turns it into Ignore.) Reasoning: * -fwhole-file/-fno-whole-file was added in 4.5 to make the transition easier; -fwhole-file is the default since 4.6. * There are many wrong-code issues and probably also some ICEs with -fno-whole file. * The generated code of -fwhole-file is faster as it allows for inlining. * -fno-whole-file has been deprecated since 4.6 and announced for removal. * Code cleanup is always nice (diff -w): 17 insertions(+), 80 deletions(-) Build and regtested on x86-64-gnu-linux. OK for the trunk? Tobias PS: Dominique pointed out that PR 45128 is a -fwhole-file regression. However, it mainly shows that gfortran needs the new array descriptor to fix such subpointer issues (and other PRs). 2013-01-05 Tobias Burnus bur...@net-b.de * gfortran.h (gfc_option_t): Remove flag_whole_file. * invoke.texi (-fno-whole-file): Remove. * lang.opt (fwhole-file): Change to Ignore. * options.c (gfc_init_options, gfc_post_options, gfc_handle_option): Remove !flag_whole_file handling * parse.c (resolve_all_program_units, translate_all_program_units, gfc_parse_file): Ditto. * resolve.c (resolve_global_procedure): Ditto. * trans-decl.c (gfc_get_symbol_decl, gfc_get_extern_function_decl, gfc_create_module_variable): Ditto. * trans-types.c (gfc_get_derived_type): Ditto. diff --git a/gcc/fortran/gfortran.h b/gcc/fortran/gfortran.h index 027cab6..b87bf64 100644 --- a/gcc/fortran/gfortran.h +++ b/gcc/fortran/gfortran.h @@ -2287,7 +2287,6 @@ typedef struct int flag_init_character; char flag_init_character_value; int flag_align_commons; - int flag_whole_file; int flag_protect_parens; int flag_realloc_lhs; int flag_aggressive_function_elimination; diff --git a/gcc/fortran/invoke.texi b/gcc/fortran/invoke.texi index d7c3219..4ba94e5 100644 --- a/gcc/fortran/invoke.texi +++ b/gcc/fortran/invoke.texi @@ -182,7 +182,7 @@ and warnings}. -finit-real=@var{zero|inf|-inf|nan|snan} @gol -fmax-array-constructor=@var{n} -fmax-stack-var-size=@var{n} -fno-align-commons @gol --fno-automatic -fno-protect-parens -fno-underscoring -fno-whole-file @gol +-fno-automatic -fno-protect-parens -fno-underscoring @gol -fsecond-underscore -fpack-derived -frealloc-lhs -frecursive @gol -frepack-arrays -fshort-enums -fstack-arrays } @@ -1293,22 +1293,6 @@ in the source, even if the names as seen by the linker are mangled to prevent accidental linking between procedures with incompatible interfaces. -@item -fno-whole-file -@opindex @code{fno-whole-file} -This flag causes the compiler to resolve and translate each procedure in -a file separately. - -By default, the whole file is parsed and placed in a single front-end tree. -During resolution, in addition to all the usual checks and fixups, references -to external procedures that are in the same file effect resolution of -that procedure, if not already done, and a check of the interfaces. The -dependences are resolved by changing the order in which the file is -translated into the backend tree. Thus, a procedure that is referenced -is translated before the reference and the duplication of backend tree -declarations eliminated. - -The @option{-fno-whole-file} option is deprecated and may lead to wrong code. - @item -fsecond-underscore @opindex @code{fsecond-underscore} @cindex underscore diff --git a/gcc/fortran/lang.opt b/gcc/fortran/lang.opt index 1535187..c885ff3 100644 --- a/gcc/fortran/lang.opt +++ b/gcc/fortran/lang.opt @@ -591,8 +591,8 @@ Fortran Append underscores to externally visible names fwhole-file -Fortran -Compile all program units at once and check all interfaces +Fortran Ignore +Does nothing. Preserved for backward compatibility. fworking-directory Fortran diff --git a/gcc/fortran/options.c b/gcc/fortran/options.c index e05b935..1b2373b 100644 --- a/gcc/fortran/options.c +++ b/gcc/fortran/options.c @@ -126,7 +126,6 @@ gfc_init_options (unsigned int decoded_options_count, gfc_option.flag_real8_kind = 0; gfc_option.flag_dollar_ok = 0; gfc_option.flag_underscoring = 1; - gfc_option.flag_whole_file = 1; gfc_option.flag_f2c = 0; gfc_option.flag_second_underscore = -1; gfc_option.flag_implicit_none = 0; @@ -266,14 +265,6 @@ gfc_post_options (const char **pfilename) sorry (-fexcess-precision=standard for Fortran); flag_excess_precision_cmdline = EXCESS_PRECISION_FAST; - /* Whole program needs whole file mode. */ - if (flag_whole_program) -gfc_option.flag_whole_file = 1; - - /* Enable whole-file mode if LTO is in effect. */ - if (flag_lto) -gfc_option.flag_whole_file = 1; - /* Fortran allows associative math - but we cannot reassociate if we want traps or signed zeros. Cf. also flag_protect_parens. */ if (flag_associative_math == -1) @@ -432,9 +423,6 @@ gfc_post_options (const char **pfilename) gfc_option.warn_tabs = 0; } - if (pedantic gfc_option.flag_whole_file) -gfc_option.flag_whole_file =
Re: [google 4_7] Backport r194909 (:: is incorrectly treated as digraph ...) to google/gcc-4_7 branch (issue7028052)
On Fri, Jan 4, 2013 at 3:27 PM, Xinliang David Li davi...@google.com wrote: I saw only one test case fix patch from Paolo in trunk. Does this patch include more local fixes? There was an earlier patch which touched g++.dg/parse/error1{1,2}.C: r191712 | paolo | 2012-09-25 07:44:52 -0700 (Tue, 25 Sep 2012) | 15 lines /cp 2012-09-25 Paolo Carlini paolo.carl...@oracle.com PR c++/54526 * parser.c (cp_parser_template_id): In C++11 mode simply accept X::A. /testsuite 2012-09-25 Paolo Carlini paolo.carl...@oracle.com PR c++/54526 * g++.dg/cpp0x/parse2.C: New. * g++.dg/parse/error11.C: Adjust. * g++.dg/parse/error12.C: Likewise. The r194909 reverted parser.c change, but didn't revert error1{1,2}.C changes. Current state of trunk vs. google/gcc-4_7 branch: diff -u ${trunk}/gcc/testsuite/g++.dg/parse/error11.C ${gcc-4_7}/gcc/testsuite/g++.dg/parse/error11.C @@ -68,4 +68,4 @@ // On the first error message, an additional note about the use of // -fpermissive should be present -// { dg-message 17:\\(if you use '-fpermissive' or '-std=c\\+\\+11', or '-std=gnu\\+\\+11' G\\+\\+ will accept your code\\) -fpermissive { target c++98 } 19 } +// { dg-message 17:\\(if you use '-fpermissive' G\\+\\+ will accept your code\\) -fpermissive { target c++98 } 19 } (Minor adjustment to the expected error message). error12.C is identical on trunk and google/gcc-4_7. Thanks, -- Paul Pluzhnikov
Re: [google 4_7] Backport r194909 (:: is incorrectly treated as digraph ...) to google/gcc-4_7 branch (issue7028052)
ok. thanks, David On Fri, Jan 4, 2013 at 3:38 PM, Paul Pluzhnikov ppluzhni...@google.com wrote: On Fri, Jan 4, 2013 at 3:27 PM, Xinliang David Li davi...@google.com wrote: I saw only one test case fix patch from Paolo in trunk. Does this patch include more local fixes? There was an earlier patch which touched g++.dg/parse/error1{1,2}.C: r191712 | paolo | 2012-09-25 07:44:52 -0700 (Tue, 25 Sep 2012) | 15 lines /cp 2012-09-25 Paolo Carlini paolo.carl...@oracle.com PR c++/54526 * parser.c (cp_parser_template_id): In C++11 mode simply accept X::A. /testsuite 2012-09-25 Paolo Carlini paolo.carl...@oracle.com PR c++/54526 * g++.dg/cpp0x/parse2.C: New. * g++.dg/parse/error11.C: Adjust. * g++.dg/parse/error12.C: Likewise. The r194909 reverted parser.c change, but didn't revert error1{1,2}.C changes. Current state of trunk vs. google/gcc-4_7 branch: diff -u ${trunk}/gcc/testsuite/g++.dg/parse/error11.C ${gcc-4_7}/gcc/testsuite/g++.dg/parse/error11.C @@ -68,4 +68,4 @@ // On the first error message, an additional note about the use of // -fpermissive should be present -// { dg-message 17:\\(if you use '-fpermissive' or '-std=c\\+\\+11', or '-std=gnu\\+\\+11' G\\+\\+ will accept your code\\) -fpermissive { target c++98 } 19 } +// { dg-message 17:\\(if you use '-fpermissive' G\\+\\+ will accept your code\\) -fpermissive { target c++98 } 19 } (Minor adjustment to the expected error message). error12.C is identical on trunk and google/gcc-4_7. Thanks, -- Paul Pluzhnikov
[google 4-7] adjust single target i-call heursitic (issue7059044)
Hi, This patch adjusts the single target indirect call promotion heuristics. (1) it uses bb counts (bb_all), instead of count all which only counts the targets with the same modules as the caller. (becaue __gcov_indirect_call_callee is file scope statics). Using overall bb counts makes more sense here. (2) use a prameter option to control the threshold. (3) change the default threshold from 75% to 67%. Tested with google internal benchmarks and gcc regression test. Thanks, -Rong 2013-01-04 Rong Xu x...@google.com * gcc/value-prof.c (gimple_ic_transform_single_targ): adjust the single_target i_call promotion heuristics. * gcc/params.def.h: Add new parameter options. Index: gcc/value-prof.c === --- gcc/value-prof.c(revision 194916) +++ gcc/value-prof.c(working copy) @@ -1491,16 +1491,20 @@ gimple_ic_transform_single_targ (gimple stmt, hist gcov_type prob; gimple modify; struct cgraph_node *direct_call; + int perc_threshold, count_threshold; val = histogram-hvalue.counters [0]; count = histogram-hvalue.counters [1]; all = histogram-hvalue.counters [2]; gimple_remove_histogram_value (cfun, stmt, histogram); + bb_all = gimple_bb (stmt)-count; - if (4 * count = 3 * all) + perc_threshold = PARAM_VALUE (PARAM_SINGLE_ICALL_PROMOTE_PERCENT_THRESHOLD); + count_threshold = PARAM_VALUE (PARAM_SINGLE_ICALL_PROMOTE_COUNT_THRESHOLD); + + if (100 * count bb_all * perc_threshold || count count_threshold) return false; - bb_all = gimple_bb (stmt)-count; /* The order of CHECK_COUNTER calls is important - since check_counter can correct the third parameter and we want to make count = all = bb_all. */ @@ -1508,6 +1512,7 @@ gimple_ic_transform_single_targ (gimple stmt, hist || check_counter (stmt, ic, count, all, all)) return false; + all = bb_all; if (all 0) prob = (count * REG_BR_PROB_BASE + all / 2) / all; else Index: gcc/params.def === --- gcc/params.def (revision 194916) +++ gcc/params.def (working copy) @@ -907,6 +907,7 @@ DEFPARAM (PARAM_LOOP_INVARIANT_MAX_BBS_IN_LOOP, /* Promote indirect call to conditional direct call only when the percentage of the target count over the total indirect call count is no smaller than the threshold. */ +/* For multi-targ indirect_call. */ DEFPARAM (PARAM_ICALL_PROMOTE_PERCENT_THRESHOLD, icall-promote-target-percent-threshold, percentage threshold for direct call promotion @@ -919,6 +920,19 @@ DEFPARAM (PARAM_ICALL_PROMOTE_COUNT_THRESHOLD, of a callee target, 1, 0, 0) +/* For single-targ indirect_call. */ +DEFPARAM (PARAM_SINGLE_ICALL_PROMOTE_PERCENT_THRESHOLD, + single_icall-promote-target-percent-threshold, + percentage threshold for direct call promotion + of a callee target, + 67, 0, 100) + +DEFPARAM (PARAM_SINGLE_ICALL_PROMOTE_COUNT_THRESHOLD, + single_icall-promote-target_count-threshold, + call count threshold for direct call promotion + of a callee target, + 1, 0, 0) + /* 0: do not always inline icall target: other value: always inline icall target when call count exceeds this value. -- This patch is available for review at http://codereview.appspot.com/7059044
Re: [google][4.7] Allow function reordering linker plugin to separate hot and cold code into different ELF segments
On Fri, Jan 4, 2013 at 2:32 PM, Xinliang David Li davi...@google.com wrote: Looks good -- but better with followup : 1) give a warning when the parameter to the option is not allowed; 2) add test cases if possible. Made all the changes. Modified the test case to check if the segment splitting API is invoked. The gold linker has a test case to check if the segment API actually splits segments. Thanks, -Sri. David On Fri, Jan 4, 2013 at 2:19 PM, Sriraman Tallam tmsri...@google.com wrote: Attached new patch. Thanks, -Sri. On Fri, Jan 4, 2013 at 9:12 AM, Rong Xu x...@google.com wrote: The code looks fine to me. Please consider David's comments about the option name. -Rong On Thu, Jan 3, 2013 at 9:14 PM, Xinliang David Li davi...@google.com wrote: Is it better to change the option to something like: split_segment|nosplit-segment or split_segment=yes|no David On Thu, Jan 3, 2013 at 5:41 PM, Sriraman Tallam tmsri...@google.com wrote: Hi Rong, The following patch modifies the behaviour of the linker plugin to not create a separate segment for cold sections by default. Separate segments can be created with the plugin option segment=cold. Is this alright to commit? Thanks, -Sri. On Mon, Dec 17, 2012 at 11:14 AM, Sriraman Tallam tmsri...@google.com wrote: I have committed this patch. Thanks, -Sri. On Fri, Dec 14, 2012 at 4:16 PM, Rong Xu x...@google.com wrote: Looks good to me for google/gcc-4_7 branch. Thanks, -Rong On Fri, Dec 14, 2012 at 3:42 PM, Sriraman Tallam tmsri...@google.com wrote: Hi Rong, Please review this code. This code allows the function reordering plugin to separate hot and cold code into different ELF segments. This would allow optimizations like mapping the hot code alone to huge pages. With this patch, by default, the plugin maps .text.unlikely sections into a separate ELF segment. This can be turned off with plugin option --segment=none. The include/plugin-api.h changes are a backport from trunk. Thanks, -Sri. Index: function_reordering_plugin/function_reordering_plugin.c === --- function_reordering_plugin/function_reordering_plugin.c (revision 194878) +++ function_reordering_plugin/function_reordering_plugin.c (working copy) @@ -33,9 +33,13 @@ along with this program; see the file COPYING3. I This plugin dumps the final layout order of the functions in a file called final_layout.txt. To change the output file, pass the new - file name with --plugin-opt. To dump to stderr instead, just pass - stderr to --plugin-opt. */ + file name with --plugin-opt,file=name. To dump to stderr instead, + just pass stderr as the file name. + This plugin also allows placing all functions found cold in a separate + segment. This can be enabled with the linker option: + --plugin-opt,split_segment=yes. */ + #if HAVE_STDINT_H #include stdint.h #endif @@ -89,9 +93,10 @@ static int is_api_exist = 0; /* The plugin does nothing when no-op is 1. */ static int no_op = 0; -/* The plugin does not create a new segment for unlikely code if - no_segment is set. */ -static int no_segment = 0; +/* The plugin creates a new segment for unlikely code if split_segment + is set. This can be set with the linker option: + --plugin-opt,split_segment=yes. */ +static int split_segment = 0; /* Copies new output file name out_file */ void get_filename (const char *name) @@ -110,7 +115,7 @@ process_option (const char *name) { const char *option_group = group=; const char *option_file = file=; - const char *option_segment = segment=; + const char *option_segment = split_segment=; /* Check if option is group= */ if (strncmp (name, option_group, strlen (option_group)) == 0) @@ -121,22 +126,26 @@ process_option (const char *name) no_op = 0; return; } - /* Check if option is file= */ - if (strncmp (name, option_file, strlen (option_file)) == 0) + else if (strncmp (name, option_file, strlen (option_file)) == 0) { get_filename (name + strlen (option_file)); return; } - - /* Check if options is segment=none */ - if (strncmp (name, option_segment, strlen (option_segment)) == 0) + /* Check if options is split_segment=[yes|no] */ + else if (strncmp (name, option_segment, strlen (option_segment)) == 0) { - if (strcmp (name + strlen (option_segment), none) == 0) - no_segment = 1; - else - no_segment = 0; - return; + const char *option_val = name + strlen (option_segment); + if (strcmp (option_val, no) == 0) + { + split_segment = 0; + return; + } + else if (strcmp (option_val, yes) == 0) + { + split_segment = 1; + return; + } } /* Unknown option, set no_op to 1. */ @@ -244,7 +253,10 @@ claim_file_hook (const struct ld_plugin_input_file { /*
Re: [google 4-7] adjust single target i-call heursitic (issue7059044)
Need to document the parameter in doc/invoke.texi. Ok with that change. David On Fri, Jan 4, 2013 at 5:28 PM, Rong Xu x...@google.com wrote: Hi, This patch adjusts the single target indirect call promotion heuristics. (1) it uses bb counts (bb_all), instead of count all which only counts the targets with the same modules as the caller. (becaue __gcov_indirect_call_callee is file scope statics). Using overall bb counts makes more sense here. (2) use a prameter option to control the threshold. (3) change the default threshold from 75% to 67%. Tested with google internal benchmarks and gcc regression test. Thanks, -Rong 2013-01-04 Rong Xu x...@google.com * gcc/value-prof.c (gimple_ic_transform_single_targ): adjust the single_target i_call promotion heuristics. * gcc/params.def.h: Add new parameter options. Index: gcc/value-prof.c === --- gcc/value-prof.c(revision 194916) +++ gcc/value-prof.c(working copy) @@ -1491,16 +1491,20 @@ gimple_ic_transform_single_targ (gimple stmt, hist gcov_type prob; gimple modify; struct cgraph_node *direct_call; + int perc_threshold, count_threshold; val = histogram-hvalue.counters [0]; count = histogram-hvalue.counters [1]; all = histogram-hvalue.counters [2]; gimple_remove_histogram_value (cfun, stmt, histogram); + bb_all = gimple_bb (stmt)-count; - if (4 * count = 3 * all) + perc_threshold = PARAM_VALUE (PARAM_SINGLE_ICALL_PROMOTE_PERCENT_THRESHOLD); + count_threshold = PARAM_VALUE (PARAM_SINGLE_ICALL_PROMOTE_COUNT_THRESHOLD); + + if (100 * count bb_all * perc_threshold || count count_threshold) return false; - bb_all = gimple_bb (stmt)-count; /* The order of CHECK_COUNTER calls is important - since check_counter can correct the third parameter and we want to make count = all = bb_all. */ @@ -1508,6 +1512,7 @@ gimple_ic_transform_single_targ (gimple stmt, hist || check_counter (stmt, ic, count, all, all)) return false; + all = bb_all; if (all 0) prob = (count * REG_BR_PROB_BASE + all / 2) / all; else Index: gcc/params.def === --- gcc/params.def (revision 194916) +++ gcc/params.def (working copy) @@ -907,6 +907,7 @@ DEFPARAM (PARAM_LOOP_INVARIANT_MAX_BBS_IN_LOOP, /* Promote indirect call to conditional direct call only when the percentage of the target count over the total indirect call count is no smaller than the threshold. */ +/* For multi-targ indirect_call. */ DEFPARAM (PARAM_ICALL_PROMOTE_PERCENT_THRESHOLD, icall-promote-target-percent-threshold, percentage threshold for direct call promotion @@ -919,6 +920,19 @@ DEFPARAM (PARAM_ICALL_PROMOTE_COUNT_THRESHOLD, of a callee target, 1, 0, 0) +/* For single-targ indirect_call. */ +DEFPARAM (PARAM_SINGLE_ICALL_PROMOTE_PERCENT_THRESHOLD, + single_icall-promote-target-percent-threshold, + percentage threshold for direct call promotion + of a callee target, + 67, 0, 100) + +DEFPARAM (PARAM_SINGLE_ICALL_PROMOTE_COUNT_THRESHOLD, + single_icall-promote-target_count-threshold, + call count threshold for direct call promotion + of a callee target, + 1, 0, 0) + /* 0: do not always inline icall target: other value: always inline icall target when call count exceeds this value. -- This patch is available for review at http://codereview.appspot.com/7059044
RE: [PATCH][ARM][thumb1] Reduce lr save for leaf function with non-far jump
Ping -Original Message- From: Joey Ye Sent: Thursday, December 20, 2012 17:53 To: gcc-patches@gcc.gnu.org; Ramana Radhakrishnan Cc: Joey Ye Subject: [PATCH][ARM][thumb1] Reduce lr save for leaf function with non- far jump Current GCC thumb1 has an annoying problem that always assuming far branch. So it forces to save lr, even when unnecessarily. The most extreme case complained by partner is: // compiled with -mthumb -mcpu=cortex-m0 -Os. void foo() { for (;;); } = foo: push{lr} // Crazy!!! .L2: b .L2 The reason is that thumb1 far jump is only resolved in the very late pass shorten_branch. Prologue/epilogue pass doesn't actually know a branch is far or not from its attribute. It has to conservatively save/restore lr whenever there is a branch. This patch tries to fix it with a simple heuristic, i.e., using function size to decide if a far jump will likely be used. Function size information is meaningful in prologue/epilogue pass. The heuristic uses following check to decide if lr should be saved for far jump: function_size * 3 = 2048 // yes: save lr for possible far jump. No: don't save lr for far jump The scheme has an issue: if some corner case does break above condition, there is no chance to fix-up but to ICE. But the heuristic condition is very conservative. It is base on the worse normal condition that each instruction is associated with a 4 byte literal ( (2+4)/2=3, blooming size by 3 times ). I can't think of a real case to trigger the ICE. So I think it should work. Other approaches than the heuristic scheme are too expensive to implement for this small size/performance issue. I did explored some but none of them persuaded myself. Tests passed: * build libgcc, libstdc++, newlib, libm * make check-gcc with cpu=cortex-m0 * Small and extreme test cases ChangeLog: 2012-12-20 Joey Ye joey...@arm.com * config/arm/arm.c(thumb1_final_prescan_insn): Assert lr save for real far jump. (thumb_far_jump_used_p): Count instruction size and set far_jump_used. diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c index 327ef22..ad79451 100644 --- a/gcc/config/arm/arm.c +++ b/gcc/config/arm/arm.c @@ -21790,6 +21857,11 @@ thumb1_final_prescan_insn (rtx insn) else if (conds != CONDS_NOCOND) cfun-machine-thumb1_cc_insn = NULL_RTX; } + +/* Check if unexpected far jump is used. */ +if (cfun-machine-lr_save_eliminated + get_attr_far_jump (insn) == FAR_JUMP_YES) + internal_error(Unexpected thumb1 far jump); } int @@ -21815,6 +21887,8 @@ static int thumb_far_jump_used_p (void) { rtx insn; + bool far_jump = false; + unsigned int func_size = 0; /* This test is only important for leaf functions. */ /* assert (!leaf_function_p ()); */ @@ -21870,6 +21944,26 @@ thumb_far_jump_used_p (void) get_attr_far_jump (insn) == FAR_JUMP_YES ) { + far_jump = true; + } + func_size += get_attr_length (insn); +} + + /* Attribute far_jump will always be true for thumb1 before shorten_branch + pass. So checking far_jump attribute before shorten_branch isn't much + useful. + + Following heuristic tries to estimate more accruately if a far jump may + finally be used. The heuristic is very conservative as there is no chance + to roll-back the decision of not to use far jump. + + Thumb1 long branch offset is -2048 to 2046. The worst case is each 2-byte + insn is assiociated with a 4 byte constant pool. Using function size + 2048/3 as the threshold is conservative enough. */ + if (far_jump) +{ + if ((func_size * 3) = 2048) +{ /* Record the fact that we have decided that the function does use far jumps. */ cfun-machine-far_jump_used = 1;