[PATCH 1/3,ARM,libgcc]Code size optimization for the fmul/fdiv and dmul/ddiv function in libgcc
Hi there, In libgcc the file ieee754-sf.S and ieee754-df.S have some function pairs which will be bundled into one .o file and sharing the same .text section. For example, the fmul and fdiv, the libgcc makefile will build them into one .o file and archived into libgcc.a. So when user only call single float point multiply functions, the fdiv function will also be linked, and as fmul and fdiv share the same .text section, linker option --gc-sections or -flot can't remove the dead code. So this optimization just separates the function pair(fmul/fdiv and dmul/ddiv) into different sections, following the naming pattern of -ffunction-sections(.text.__functionname), through which the unused sections of fdiv/ddiv can be eliminated through option --gcc-sections when users only use fmul/dmul.The solution is to add a conditional statement in the macro FUNC_START, which will conditional change the section of a function from .text to .text.__\name. when compiling with the L_arm_muldivsf3 or L_arm_muldivdf3 macro. GCC regression test has been done on QEMU for Cortex-M3. No new regressions when turn on this patch. The code reduction for thumb2 on cortex-m3 is: 1. When user only use single float point multiply: fmul+fdiv = fmul will have a code size reduction of 318 bytes. 2. When user only use double float point multiply: dmul+ddiv = dmul will have a code size reduction of 474 bytes. Ok for trunk? BR, Tony Step 1: Provide another option: sp-scetion to control whether to split the section of a function pair into two part. gcc/libgcc/ChangeLog: 2014-08-21 Tony Wang tony.w...@arm.com * config/arm/lib1funcs.S (FUNC_START): Add conditional section redefine for macro L_arm_muldivsf3 and L_arm_muldivdf3 (SYM_END, ARM_SYM_START): Add macros used to expose function Symbols diff --git a/libgcc/config/arm/lib1funcs.S b/libgcc/config/arm/lib1funcs.S index b617137..0f87111 100644 --- a/libgcc/config/arm/lib1funcs.S +++ b/libgcc/config/arm/lib1funcs.S @@ -418,8 +418,12 @@ SYM (\name): #define THUMB_SYNTAX #endif -.macro FUNC_START name +.macro FUNC_START name sp_section= + .ifc \sp_section, function_section + .section.text.__\name,ax,%progbits + .else .text + .endif .globl SYM (__\name) TYPE (__\name) .align 0 @@ -429,14 +433,24 @@ SYM (\name): SYM (__\name): .endm +.macro ARM_SYM_START name + TYPE (\name) + .align 0 +SYM (\name): +.endm + +.macro SYM_END name + SIZE (\name) +.endm + /* Special function that will always be coded in ARM assembly, even if in Thumb-only compilation. */ #if defined(__thumb2__) /* For Thumb-2 we build everything in thumb mode. */ -.macro ARM_FUNC_START name - FUNC_START \name +.macro ARM_FUNC_START name sp_section= + FUNC_START \name \sp_section .syntax unified .endm #define EQUIV .thumb_set @@ -467,8 +481,12 @@ _L__\name: #ifdef __ARM_ARCH_6M__ #define EQUIV .thumb_set #else -.macro ARM_FUNC_START name +.macro ARM_FUNC_START name sp_section= + .ifc \sp_section, function_section + .section.text.__\name,ax,%progbits + .else .text + .endif .globl SYM (__\name) TYPE (__\name) .align 0 libgcc_mul_div_code_size_reduction_1.diff Description: Binary data
[PATCH 2/3,ARM,libgcc]Code size optimization for the fmul/fdiv and dmul/ddiv function in libgcc
Step 2: Mark all the symbols around the fragment boundaries as function symbols, so as to generate veneer when the two section is too far away from each other. Also, I have both manually and using some test cases to verify that IP and PSR are not alive at such point. gcc/libgcc/ChangeLog: 2014-8-21 Tony Wang tony.w...@arm.com * config/arm/ieee754-sf.S: Expose symbols around fragment boundaries as function symbols. * config/arm/ieee754-df.S: Same with above BR, Tony libgcc_mul_div_code_size_reduction_2.diff Description: Binary data
[PATCH 3/3,ARM,libgcc]Code size optimization for the fmul/fdiv and dmul/ddiv function in libgcc
Step 3: Test cases to verify the code size reduction. gcc/gcc/testsuite/ChangeLog: 2014-08-21 Tony Wang tony.w...@arm.com * gcc.target/arm/size-optimization-ieee-1.c: New test case * gcc.target/arm/size-optimization-ieee-2.c: New test case * lib/gcc-dg.exp: Add new function scan-symbol-common, scan-symbol-yes, scan-symbol-no to scan a user defined symbol in final elf file BR, Tony diff --git a/gcc/testsuite/gcc.target/arm/size-optimization-ieee-1.c b/gcc/testsuite/gcc.target/arm/size-optimization-ieee-1.c new file mode 100644 index 000..46e9cdf --- /dev/null +++ b/gcc/testsuite/gcc.target/arm/size-optimization-ieee-1.c @@ -0,0 +1,30 @@ +/* { dg-do link { target { arm_thumb2_ok } } } */ +/* { dg-options -Wl,--gc-sections } */ +int +foo () +{ + volatile float a; + volatile float b; + volatile float c = a * b; + return 0; +} + +int +bar () +{ + volatile double a; + volatile double b; + volatile double c = a * b; + return 0; +} + +int +main () +{ + foo (); + bar (); + return 0; +} +/* { dg-final { scan-symbol-no __aeabi_fdiv } } */ +/* { dg-final { scan-symbol-no __aeabi_ddiv } } */ + diff --git a/gcc/testsuite/gcc.target/arm/size-optimization-ieee-2.c b/gcc/testsuite/gcc.target/arm/size-optimization-ieee-2.c new file mode 100644 index 000..5007d62 --- /dev/null +++ b/gcc/testsuite/gcc.target/arm/size-optimization-ieee-2.c @@ -0,0 +1,30 @@ +/* { dg-do link { target { arm_thumb2_ok } } } */ +/* { dg-options -Wl,--gc-sections } */ +int +foo () +{ + volatile float a; + volatile float b; + volatile float c = a / b; + return 0; +} + +int +bar () +{ + volatile double a; + volatile double b; + volatile double c = a / b; + return 0; +} + +int +main () +{ + foo (); + bar (); + return 0; +} +/* { dg-final { scan-symbol-yes __aeabi_fmul } } */ +/* { dg-final { scan-symbol-yes __aeabi_dmul } } */ + diff --git a/gcc/testsuite/lib/gcc-dg.exp b/gcc/testsuite/lib/gcc-dg.exp index 3390caa..0d52e95 100644 --- a/gcc/testsuite/lib/gcc-dg.exp +++ b/gcc/testsuite/lib/gcc-dg.exp @@ -880,5 +880,57 @@ proc gdb-exists { args } { return 0; } +# Scan the OUTPUT_FILE for a symbol. Return 1 if it present, or +# return 0 if it doesn't present + +proc scan-symbol-common { args } { +global nm +global base_dir + +set testcase [testname-for-summary] +set output_file [file rootname [file tail $testcase]].exe + +# Find nm like we find g++ in g++.exp. +if ![info exists nm] { +set nm [findfile $base_dir/../../../binutils/nm \ +$base_dir/../../../binutils/nm \ +[findfile $base_dir/../../nm $base_dir/../../nm \ + [findfile $base_dir/nm $base_dir/nm \ + [transform nm +verbose -log nm is $nm +} + +if { $output_file == } { +fail scan-symbol-not $args: dump file does not exist +return +} + +set fd [open | $nm $output_file r] +set text [read $fd] +close $fd + +if [regexp -- [lindex $args 0] $text] { +return 1 +} else { +return 0 +} +} + +proc scan-symbol-yes { args } { +if { [scan-symbol-common $args] == 1 } { + pass scan-symbol-yes $args exists +} else { + fail scan-symbol-yes $args does not exist +} +} + +proc scan-symbol-no { args } { +if { [scan-symbol-common $args] != 1 } { +pass scan-symbol-no $args does not exist +} else { +fail scan-symbol-no $args exists +} +} + set additional_prunes set dg_runtest_extra_prunes libgcc_mul_div_code_size_reduction_3.diff Description: Binary data
Fix tree-optimization/62091
Hi, this should be hopefully last fix to tree-optimization/62091. The testcase triggers another unnecesary paranoia in get_dynamic_type: if location is of type A to start with and then it is re-initialized to type A, it should not count as dynamic type change. While looking into it, I added some debug info that I think is useful enough to be kept and I also noticed that we do not handle multiple inheritance very well. Fixed thus. Bootstrapped/regtested x86_64-linux, will commit it after bit of additional testing. With this change I can now build libreoffice, so I think I can drop the old ipa-prop intraprocedural code and remove the sanity check. Honza PR tree-optimization/62091 * g++.dg/ipa/devirt-37.C: Update template. * g++.dg/ipa/devirt-40.C: New testcase. * ipa-devirt.c (ipa_polymorphic_call_context::restrict_to_inner_type): handle correctly arrays. (extr_type_from_vtbl_ptr_store): Add debug output; handle multiple inheritance binfos. (record_known_type): Walk into inner type. (ipa_polymorphic_call_context::get_dynamic_type): Likewise; strenghten condition on no type changes. Index: testsuite/g++.dg/ipa/devirt-37.C === --- testsuite/g++.dg/ipa/devirt-37.C(revision 214255) +++ testsuite/g++.dg/ipa/devirt-37.C(working copy) @@ -30,7 +30,7 @@ t() /* After inlining the call within constructor needs to be checked to not go into a basetype. We should see the vtbl store and we should notice extcall as possibly clobbering the type but ignore it because b is in static storage. */ -/* { dg-final { scan-tree-dump Determined dynamic type. fre2 } } */ +/* { dg-final { scan-tree-dump No dynamic type change found. fre2 } } */ /* { dg-final { scan-tree-dump Checking vtbl store: fre2 } } */ /* { dg-final { scan-tree-dump Function call may change dynamic type:extcall fre2 } } */ /* { dg-final { scan-tree-dump converting indirect call to function virtual void fre2 } } */ Index: testsuite/g++.dg/ipa/devirt-40.C === --- testsuite/g++.dg/ipa/devirt-40.C(revision 0) +++ testsuite/g++.dg/ipa/devirt-40.C(revision 0) @@ -0,0 +1,23 @@ +/* { dg-options -O2 -fdump-tree-fre2-details } */ +typedef enum +{ +} UErrorCode; +class UnicodeString +{ +public: + UnicodeString (); + virtual ~UnicodeString (); +}; +class A +{ + UnicodeString m_fn1 (UnicodeString , int p2, UErrorCode ) const; +}; +UnicodeString::UnicodeString () {} +UnicodeString +A::m_fn1 (UnicodeString , int p2, UErrorCode ) const +{ + UnicodeString a[2]; +} + +/* { dg-final { scan-tree-dump converting indirect call to function virtual UnicodeString fre2 } } */ +/* { dg-final { cleanup-tree-dump fre2 } } */ Index: ipa-devirt.c === --- ipa-devirt.c(revision 214223) +++ ipa-devirt.c(working copy) @@ -2015,8 +2015,10 @@ ipa_polymorphic_call_context::restrict_t tree subtype = TYPE_MAIN_VARIANT (TREE_TYPE (type)); /* Give up if we don't know array size. */ - if (!tree_fits_shwi_p (TYPE_SIZE (subtype)) - || !tree_to_shwi (TYPE_SIZE (subtype)) = 0) + if (!TYPE_SIZE (subtype) + || !tree_fits_shwi_p (TYPE_SIZE (subtype)) + || tree_to_shwi (TYPE_SIZE (subtype)) = 0 + || !contains_polymorphic_type_p (subtype)) goto give_up; cur_offset = cur_offset % tree_to_shwi (TYPE_SIZE (subtype)); type = subtype; @@ -2630,7 +2632,7 @@ extr_type_from_vtbl_ptr_store (gimple st HOST_WIDE_INT *type_offset) { HOST_WIDE_INT offset, size, max_size; - tree lhs, rhs, base, binfo; + tree lhs, rhs, base; if (!gimple_assign_single_p (stmt)) return NULL_TREE; @@ -2639,7 +2641,11 @@ extr_type_from_vtbl_ptr_store (gimple st rhs = gimple_assign_rhs1 (stmt); if (TREE_CODE (lhs) != COMPONENT_REF || !DECL_VIRTUAL_P (TREE_OPERAND (lhs, 1))) -return NULL_TREE; + { + if (dump_file) + fprintf (dump_file, LHS is not virtual table.\n); + return NULL_TREE; + } if (tci-vtbl_ptr_ref operand_equal_p (lhs, tci-vtbl_ptr_ref, 0)) ; @@ -2649,33 +2655,80 @@ extr_type_from_vtbl_ptr_store (gimple st if (offset != tci-offset || size != POINTER_SIZE || max_size != POINTER_SIZE) - return NULL_TREE; + { + if (dump_file) + fprintf (dump_file, wrong offset %i!=%i or size %i\n, +(int)offset, (int)tci-offset, (int)size); + return NULL_TREE; + } if (DECL_P (tci-instance)) { if (base != tci-instance) - return NULL_TREE; + { + if (dump_file) + { + fprintf (dump_file, base:); +
Re: [PATCH v4] Add strict aliasing warning when inlining function.
Hi Richard, Do you think this version is good enough to commit to trunk? I have asked Andrew to commit for me. If it is not good enough, I may ask him not to do this. --- Lin Zuojian
Re: [PATCH 040/236] Use rtx_insn internally within generated functions
On Wed, 2014-08-13 at 12:03 -0600, Jeff Law wrote: On 08/06/14 11:20, David Malcolm wrote: With this patch, insn and curr_insn as used from C++ fragments in .md files are strengthened from rtx to rtx_insn *, allowing numerous target-specific functions to have their params be similiar strengthened. The top-level interfaces (recog, split, peephole2) continue to take a plain rtx for insn, to avoid introducing dependencies on other patches. gcc/ * recog.h (insn_output_fn): Update this function typedef to match the changes below to the generated output functions, strengthening the 2nd param from rtx to rtx_insn *. * final.c (get_insn_template): Add a checked cast to rtx_insn * on insn when invoking an output function, to match the new signature of insn_output_fn with a stronger second param. * genconditions.c (write_header): In the generated code for gencondmd.c, strengthen the global insn from rtx to rtx_insn * to match the other changes in this patch. * genemit.c (gen_split): Strengthen the 1st param curr_insn of the generated gen_ functions from rtx to rtx_insn * within their implementations. * genrecog.c (write_subroutine): Strengthen the 2nd param insn of the subfunctions within the generated recog_, split, peephole2 function trees from rtx to rtx_insn *. For now, the top-level generated functions (recog, split, peephole2) continue to take a plain rtx for insn, to avoid introducing dependencies on other patches. Rename this 2nd param from insn to uncast_insn, and reintroduce insn as a local variable of type rtx_insn *, initialized at the top of the generated function with a checked cast on uncast_insn. (make_insn_sequence): Strengthen the 1st param curr_insn of the generated gen_ functions from rtx to rtx_insn * within their prototypes. * genoutput.c (process_template): Strengthen the 2nd param within the generated output_ functions insn from rtx to rtx_insn *. OK. Thanks; bootstrapped and committed to trunk as r214257, with a trivial fixup of the as_a_nullable to safe_as_a.
[PATCH][match-and-simplify] Do not match loads
This avoids matching loads in the signle-rhs matching logic. Bootstrapped and tested on x86_64-unknown-linux-gnu, applied. Richard. 2014-08-21 Richard Biener rguent...@suse.de * genmatch.c (dt_operand::gen_gimple_expr): Only match non-reference-like GIMPLE single RHS. Index: gcc/genmatch.c === --- gcc/genmatch.c (revision 214225) +++ gcc/genmatch.c (working copy) @@ -1297,8 +1297,19 @@ dt_operand::gen_gimple_expr (FILE *f) { if (*id == REALPART_EXPR || *id == IMAGPART_EXPR || *id == BIT_FIELD_REF || *id == VIEW_CONVERT_EXPR) - fprintf (f, tree %s = TREE_OPERAND (gimple_assign_rhs1 (def_stmt), %i);\n, -child_opname, i); + { + /* ??? If this is a memory operation we can't (and should not) +match this. The only sensible operand types are +SSA names and invariants. */ + fprintf (f, tree %s = TREE_OPERAND (gimple_assign_rhs1 (def_stmt), %i);\n, + child_opname, i); + fprintf (f, if ((TREE_CODE (%s) == SSA_NAME\n + (%s = do_valueize (valueize, %s)))\n + || is_gimple_min_invariant (%s))\n + {\n, child_opname, child_opname, child_opname, + child_opname); + continue; + } else fprintf (f, tree %s = gimple_assign_rhs%u (def_stmt);\n, child_opname, i + 1);
Re: [PATCH 041/236] Debug hooks: use rtx_insn and rtx_code_label
On Wed, 2014-08-13 at 12:05 -0600, Jeff Law wrote: On 08/06/14 11:20, David Malcolm wrote: gcc/ * debug.h (struct gcc_debug_hooks): Strengthen param 1 of hook label from rtx to rtx_code_label *. Strengthen param 1 o var_location hook from rtx to rtx_insn *. (debug_nothing_rtx): Delete in favor of... (debug_nothing_rtx_code_label): New prototype. (debug_nothing_rtx_rtx): Delete unused prototype. (debug_nothing_rtx_insn): New prototype. * final.c (final_scan_insn): Add checked cast to rtx_insn * when invoking debug_hooks-var_location (in two places, one in a NOTE case of a switch statement, the other guarded by a CALL_P conditional. Add checked cast to rtx_code_label * when invoking debug_hooks-label (within CODE_LABEL case of switch statement). * dbxout.c (dbx_debug_hooks): Update label hook from debug_nothing_rtx to debug_nothing_rtx_code_label. Update var_location from debug_nothing_rtx to debug_nothing_rtx_insn. (xcoff_debug_hooks): Likewise. * debug.c (do_nothing_debug_hooks): Likewise. (debug_nothing_rtx): Delete in favor of... (debug_nothing_rtx_insn): New function. (debug_nothing_rtx_rtx): Delete unused function. (debug_nothing_rtx_code_label): New function. * dwarf2out.c (dwarf2_debug_hooks): Update label hook from debug_nothing_rtx to debug_nothing_rtx_code_label. (dwarf2out_var_location): Strengthen param loc_note from rtx to rtx_insn *. * sdbout.c (sdb_debug_hooks): Update var_location hook from debug_nothing_rtx to debug_nothing_rtx_insn. (sdbout_label): Strengthen param insn from rtx to rtx_code_label *. * vmsdbgout.c (vmsdbg_debug_hooks): Update label hook from debug_nothing_rtx to debug_nothing_rtx_code_label. Update var_location hook from debug_nothing_rtx to debug_nothing_rtx_insn. OK. Note minor typo in changelog line #2. o at EOL should probably be of Thanks. Bootstrapped; committed to trunk as r214259, with the ChangeLog typo fixed.
Re: DSE calls to builtins (memset, etc)
On Wed, Aug 20, 2014 at 5:05 PM, Marc Glisse marc.gli...@inria.fr wrote: On Wed, 20 Aug 2014, Richard Biener wrote: On Wed, Aug 20, 2014 at 4:18 PM, Marc Glisse marc.gli...@inria.fr wrote: On Wed, 20 Aug 2014, Richard Biener wrote: - if (stmt != use_stmt - ref_maybe_used_by_stmt_p (use_stmt, gimple_assign_lhs (stmt))) - return; - I don't see how you can remove this code? dse_possible_dead_store_p already tests ref_maybe_used_by_stmt_p and thus cannot return true with such a use_stmt, as far as I can see. As I said, bootstrap+testsuite with an assert instead of 'return' didn't turn up anything. I could keep it as a gcc_checking_assert (with a slight update to the comment) if you like. Or did I miss a path in dse_possible_dead_store_p? Yes, the one that early returns from the operand_equal_p check. You might want to do some svn blaming to see what testcases were added with the above code (and the code surrounding it). I'm not sure either... so if it passes bootstrap regtest it must be dead code... (well...) The early return operand_equal_p has use_stmt == stmt, so it doesn't even reach the call to ref_maybe_used_by_stmt_p I am removing. svn blame leads me to r132899 (gcc.c-torture/execute/pr35472.c) and before that to r131101 (gcc.c-torture/execute/20071219-1.c) Maybe add some noclone attributes as well? Ok, both testcases now have noinline,noclone instead of just noinline in my tree. * gcc.c-torture/execute/pr35472.c: Add noclone attribute. * gcc.c-torture/execute/20071219-1.c: Likewise. And try -fno-tree-fre/pre? No problem. (it makes it harder to find a killing stmt) But pr35472.c clearly shows what it was trying to fix: *p = a; *p = b; with p == b. The *p = b; store does _not_ make *p = a dead because *p = b; is a no-op. So a modified testcase would be *p = a; *p = *p; where without your patch the first DSE pass removes *p = *p (but not first *p = a). Maybe if that still works after your patch you can add this as a new testcase, also verifying that *p = *p store indeed is removed. DSE goes backwards, so it first removes *p=*p (already tested by gcc.dg/tree-ssa/pr57361.c) and when we look at *p=*a; there is no second statement left, so it wouldn't really test what you want. Ah, ok. Well, I suppose the patch is ok with the check removed then. Thanks, Richard. -- Marc Glisse
Re: [c++-concepts] variable concept fixes
Hi Andrew, On 08/20/2014 11:08 PM, Andrew Sutton wrote: On 08/20/2014 08:56 PM, Andrew Sutton wrote: + return VAR_P (decl) + DECL_LANG_SPECIFIC (decl) + DECL_DECLARED_CONCEPT_P (decl); this is brittle from the formatting point of view. Please double check in detail what I'm going to say, but I think that in such cases you simply want to wrap the whole thing in round parentheses. Sorry, did you just mean to wrap the entire conjunction in parens? I'm trying to find the formatting guidelines to check, but not succeeding at the moment. Yes, I meant the whole conjunction, sorry about my sloppy language. In terms of GNU coding standards: http://www.gnu.org/prep/standards/standards.html#Formatting toward the end of the section, for example. Paolo.
[PATCH GCC]Fix broken Canadian when checking isl library support
Hi, Canadian build of arm/aarch64 (and other targets) toolchains are broken because of isl library check. There is below code in top level gcc configuration. { $as_echo $as_me:${as_lineno-$LINENO}: checking for version 0.12 of ISL 5 $as_echo_n checking for version 0.12 of ISL... 6; } if test $cross_compiling = yes; then : gcc_cv_isl=yes else cat confdefs.h - _ACEOF conftest.$ac_ext For Canadian build, corss_compiling is set to `yes', then gcc_cv_isl is set to `yes' accordingly, no matter whether isl library is available or not. We also can't set it to `no' by default, because --with-isl= option would be nullified in this way. I think the best we can do here is add AC_LINK_IFELSE when checking. This patch fixes the issue. Is it OK? 2014-08-21 Bin Cheng bin.ch...@arm.com * config/isl.m4 (ISL_CHECK_VERSION): Check link of isl library for cross_compiling. * configure: Regenerated. Index: config/isl.m4 === --- config/isl.m4 (revision 214255) +++ config/isl.m4 (working copy) @@ -122,7 +122,11 @@ AC_DEFUN([ISL_CHECK_VERSION], AC_RUN_IFELSE([_ISL_CHECK_CT_PROG($1,$2)], [gcc_cv_isl=yes], [gcc_cv_isl=no], - [gcc_cv_isl=yes]) + [ + AC_LINK_IFELSE([_ISL_CHECK_CT_PROG($1,$2)], + [gcc_cv_isl=yes], + [gcc_cv_isl=no]) + ]) AC_MSG_RESULT([$gcc_cv_isl]) CFLAGS=$_isl_saved_CFLAGS Index: configure === --- configure (revision 214255) +++ configure (working copy) @@ -5907,8 +5907,30 @@ $as_echo $as_me: WARNING: using in-tree ISL, disa { $as_echo $as_me:${as_lineno-$LINENO}: checking for version 0.12 of ISL 5 $as_echo_n checking for version 0.12 of ISL... 6; } if test $cross_compiling = yes; then : + + cat confdefs.h - _ACEOF conftest.$ac_ext +/* end confdefs.h. */ +#include isl/version.h + #include string.h +int +main () +{ +if (strncmp (isl_version (), isl-0.12, strlen (isl-0.12)) != 0) + return 1; + + ; + return 0; +} +_ACEOF +if ac_fn_c_try_link $LINENO; then : gcc_cv_isl=yes else + gcc_cv_isl=no +fi +rm -f core conftest.err conftest.$ac_objext \ +conftest$ac_exeext conftest.$ac_ext + +else cat confdefs.h - _ACEOF conftest.$ac_ext /* end confdefs.h. */ #include isl/version.h
[PATCH, libstdc++, testsuite]Skip 62154.cc for target don't support the atomic builtins
It's a very simple patch. Some target don't support atomic builtins, so all related test cases should be disabled. In folder libstdc++-v3/testsuite/18_support/nested_exception, all other test cases have already been disabled for target don't have atomic builtins. gcc/libstdc++-v3/ChangeLog: 2014-08-21 Tony Wang tony.w...@arm.com * testsuite/18_support/nested_exception/62154.cc: Disable this test case when target don't have atomic buildins. diff --git a/libstdc++-v3/testsuite/18_support/nested_exception/62154.cc b/libstdc++-v3/testsuite/18_support/nested_exception/62154.cc index 9c6725f..da6ed4c 100644 --- a/libstdc++-v3/testsuite/18_support/nested_exception/62154.cc +++ b/libstdc++-v3/testsuite/18_support/nested_exception/62154.cc @@ -1,4 +1,5 @@ // { dg-options -std=gnu++11 } +// { dg-require-atomic-builtins } // Copyright (C) 2014 Free Software Foundation, Inc. //
Re: [PATCH, libstdc++, testsuite]Skip 62154.cc for target don't support the atomic builtins
Hi, On 08/21/2014 10:34 AM, Tony Wang wrote: It's a very simple patch. Some target don't support atomic builtins, so all related test cases should be disabled. In folder libstdc++-v3/testsuite/18_support/nested_exception, all other test cases have already been disabled for target don't have atomic builtins. gcc/libstdc++-v3/ChangeLog: 2014-08-21 Tony Wang tony.w...@arm.com * testsuite/18_support/nested_exception/62154.cc: Disable this test case when target don't have atomic buildins. Ok, with the typo buildins/builtins in the ChangeLog fixed. More generally, I would write it as: Disable when the target doesn't provide atomic builtins. Thanks, Paolo.
Re: [PATCH 042/236] try_split returns an rtx_insn
On Wed, 2014-08-13 at 12:06 -0600, Jeff Law wrote: On 08/06/14 11:20, David Malcolm wrote: gcc/ * rtl.h (try_split): Strengthen return type from rtx to rtx_insn *. * emit-rtl.c (try_split): Likewise, also for locals before and after. For now, don't strengthen param trial, which requires adding checked casts when returning it. OK. Thanks; committed to trunk as r214260.
Re: [Patch] Switch elimination pass for PR 54742
On Tue, Aug 19, 2014 at 10:39 PM, Steve Ellcey sell...@mips.com wrote: Here is an official submission for the switch optimization described in PR 54742. I have addressed the formatting/comment issues that were raised and also added a test case based on comment #27 from PR 54742 and I fixed a bug I found while doing benchmarking with SPEC2006 (the perl benchmark was generating an ICE in a routine with multiple switch statements). I ran the benchmarking to see if I could find any more tests that are helped like coremark is and while I found a number of benchmarks in SPEC 2006 and EEMBC where the optimization is triggered, this optimization generally didn't affect the performance of those benchmarks. The biggest impact I could find was on the perl benchmark in SPEC where I saw around a 0.4% improvement on a MIPS 74k. Not huge, but not nothing. So, OK to checkin? Without looking at the patch in detail what is the rationale for the pass placement (looks quite early)? I would have guessed that the pass could benefit from value-range analysis. Jeff, Steve is it possible to trigger the transform by simply manually forcing the right path jump-threads from inside VRP? That is, basically integrate the transform part with the existing jump threading framework but do an alternate discovery pass? Thanks, Richard. Steve Ellcey sell...@mips.com 2014-08-12 Steve Ellcey sell...@mips.com PR tree-opt/54742 * Makefile.in (OBJS): Add tree-switch-shortcut.o. * common.opt (ftree-switch-shortcut): New. * opts.c (default_options_table): Add OPT_ftree_switch_shortcut. * params.def (PARAM_MAX_SWITCH_INSNS): New. (PARAM_MAX_SWITCH_PATHS): New. * passes.def (pass_tree_switch_shortcut): New. * timevar.def (TV_TREE_SWITCH_SHORTCUT): New. * tree-pass.h (make_pass_tree_switch_shortcut): New. * tree-switch-shortcut.c: New. 2014-08-12 Steve Ellcey sell...@mips.com PR tree-opt/54742 * gcc.dg/pr54742.c: New test.
Re: [PATCH GCC]Fix broken Canadian when checking isl library support
On Thu, Aug 21, 2014 at 10:28 AM, Bin Cheng bin.ch...@arm.com wrote: Hi, Canadian build of arm/aarch64 (and other targets) toolchains are broken because of isl library check. There is below code in top level gcc configuration. { $as_echo $as_me:${as_lineno-$LINENO}: checking for version 0.12 of ISL 5 $as_echo_n checking for version 0.12 of ISL... 6; } if test $cross_compiling = yes; then : gcc_cv_isl=yes else cat confdefs.h - _ACEOF conftest.$ac_ext For Canadian build, corss_compiling is set to `yes', then gcc_cv_isl is set to `yes' accordingly, no matter whether isl library is available or not. We also can't set it to `no' by default, because --with-isl= option would be nullified in this way. I think the best we can do here is add AC_LINK_IFELSE when checking. This patch fixes the issue. Is it OK? I think it would be better to identify a set of features we rely on that are not present in earlier versions and make the test a link test unconditionally. Tobias, are there include files / types / functions we require that are not available in earlier versions? Unfortunately ISL upstream doesn't have ISL version defines in version.h (and no, don't go the cloog route of auto-generating the version.h file!). That way we still can't do any sanity check of an in-tree version (which we have to check from the source, before compiling it). If you don't want to work on this patch further the present patch is ok. Thanks, Richard. 2014-08-21 Bin Cheng bin.ch...@arm.com * config/isl.m4 (ISL_CHECK_VERSION): Check link of isl library for cross_compiling. * configure: Regenerated.
Re: [PATCH GCC]Fix broken Canadian when checking isl library support
On Thu, Aug 21, 2014 at 5:08 PM, Richard Biener richard.guent...@gmail.com wrote: On Thu, Aug 21, 2014 at 10:28 AM, Bin Cheng bin.ch...@arm.com wrote: Hi, Canadian build of arm/aarch64 (and other targets) toolchains are broken because of isl library check. There is below code in top level gcc configuration. { $as_echo $as_me:${as_lineno-$LINENO}: checking for version 0.12 of ISL 5 $as_echo_n checking for version 0.12 of ISL... 6; } if test $cross_compiling = yes; then : gcc_cv_isl=yes else cat confdefs.h - _ACEOF conftest.$ac_ext For Canadian build, corss_compiling is set to `yes', then gcc_cv_isl is set to `yes' accordingly, no matter whether isl library is available or not. We also can't set it to `no' by default, because --with-isl= option would be nullified in this way. I think the best we can do here is add AC_LINK_IFELSE when checking. This patch fixes the issue. Is it OK? I think it would be better to identify a set of features we rely on that are not present in earlier versions and make the test a link test unconditionally. Tobias, are there include files / types / functions we require that are not available in earlier versions? Unfortunately ISL upstream doesn't have ISL version defines in version.h (and no, don't go the cloog route of auto-generating the version.h file!). That way we still can't do any sanity check of an in-tree version (which we have to check from the source, before compiling it). If you don't want to work on this patch further the present patch is ok. Hi Richard, thanks for the suggestion. Meanwhile, I need to get this in firstly since it's a build breakage. I will put the re-factor on to-do list but can't guarantee that. Thanks, bin Thanks, Richard. 2014-08-21 Bin Cheng bin.ch...@arm.com * config/isl.m4 (ISL_CHECK_VERSION): Check link of isl library for cross_compiling. * configure: Regenerated.
Re: [PATCH 043/236] peephole returns an rtx_insn
On Wed, 2014-08-13 at 12:06 -0600, Jeff Law wrote: On 08/06/14 11:20, David Malcolm wrote: gcc/ * output.h (peephole): Strengthen return type from rtx to rtx_insn *. * rtl.h (delete_for_peephole): Likewise for both params. * genpeep.c (main): In generated peephole function, strengthen return type and local insn from rtx to rtx_insn *. For now, rename param ins1 to uncast_ins1, adding ins1 back as an rtx_insn *, with a checked cast. * jump.c (delete_for_peephole): Strengthen params from, to and locals insn, next, prev from rtx to rtx_insn *. OK Thanks; committed to trunk as r214264.
Re: [Patch] Switch elimination pass for PR 54742
On Thu, Aug 21, 2014 at 09:57:56AM +0100, Richard Biener wrote: On Tue, Aug 19, 2014 at 10:39 PM, Steve Ellcey sell...@mips.com wrote: Here is an official submission for the switch optimization described in PR 54742. I have addressed the formatting/comment issues that were raised and also added a test case based on comment #27 from PR 54742 and I fixed a bug I found while doing benchmarking with SPEC2006 (the perl benchmark was generating an ICE in a routine with multiple switch statements). I ran the benchmarking to see if I could find any more tests that are helped like coremark is and while I found a number of benchmarks in SPEC 2006 and EEMBC where the optimization is triggered, this optimization generally didn't affect the performance of those benchmarks. The biggest impact I could find was on the perl benchmark in SPEC where I saw around a 0.4% improvement on a MIPS 74k. Not huge, but not nothing. So, OK to checkin? Without looking at the patch in detail what is the rationale for the pass placement (looks quite early)? I would have guessed that the pass could benefit from value-range analysis. Jeff, Steve is it possible to trigger the transform by simply manually forcing the right path jump-threads from inside VRP? That is, basically integrate the transform part with the existing jump threading framework but do an alternate discovery pass? This seems like what I tried to do last year with: https://gcc.gnu.org/ml/gcc-patches/2013-06/msg01121.html It turns Jeff's jump-threading code in to a strange franken-pass of bits and pieces of detection and optimisation, and would need some substantial reworking to fit in with Jeff's changes last Autumn, but if it is more likely to be acceptable for trunk then perhaps we could look to revive it. It would be nice to reuse the path copy code Jeff added last year, but I don't have much intuition as to how feasible that is. Was this the sort of thing that you were imagining? Steve, Jeff? James Thanks, Richard. Steve Ellcey sell...@mips.com 2014-08-12 Steve Ellcey sell...@mips.com PR tree-opt/54742 * Makefile.in (OBJS): Add tree-switch-shortcut.o. * common.opt (ftree-switch-shortcut): New. * opts.c (default_options_table): Add OPT_ftree_switch_shortcut. * params.def (PARAM_MAX_SWITCH_INSNS): New. (PARAM_MAX_SWITCH_PATHS): New. * passes.def (pass_tree_switch_shortcut): New. * timevar.def (TV_TREE_SWITCH_SHORTCUT): New. * tree-pass.h (make_pass_tree_switch_shortcut): New. * tree-switch-shortcut.c: New. 2014-08-12 Steve Ellcey sell...@mips.com PR tree-opt/54742 * gcc.dg/pr54742.c: New test.
Re: [PATCH 044/236] Pass insn as an rtx_insn within generated get_attr_ fns in insn-attrtab.c
On Wed, 2014-08-13 at 12:07 -0600, Jeff Law wrote: On 08/06/14 11:20, David Malcolm wrote: Strengthen insn from rtx to rtx_insn * within the generated get_attr_ functions in insn-attrtab.c, without imposing a strengthening from rtx to rtx_insn * on the param itself and thus the callers. gcc/ * genattrtab.c (write_attr_get): Within the generated get_attr_ functions, rename param insn to uncast_insn and reintroduce insn as an local rtx_insn * using a checked cast, so that insn is an rtx_insn * within insn-attrtab.c OK. Thanks; committed to trunk as r214265.
Re: [Patch] Switch elimination pass for PR 54742
On Thu, Aug 21, 2014 at 11:41 AM, James Greenhalgh james.greenha...@arm.com wrote: On Thu, Aug 21, 2014 at 09:57:56AM +0100, Richard Biener wrote: On Tue, Aug 19, 2014 at 10:39 PM, Steve Ellcey sell...@mips.com wrote: Here is an official submission for the switch optimization described in PR 54742. I have addressed the formatting/comment issues that were raised and also added a test case based on comment #27 from PR 54742 and I fixed a bug I found while doing benchmarking with SPEC2006 (the perl benchmark was generating an ICE in a routine with multiple switch statements). I ran the benchmarking to see if I could find any more tests that are helped like coremark is and while I found a number of benchmarks in SPEC 2006 and EEMBC where the optimization is triggered, this optimization generally didn't affect the performance of those benchmarks. The biggest impact I could find was on the perl benchmark in SPEC where I saw around a 0.4% improvement on a MIPS 74k. Not huge, but not nothing. So, OK to checkin? Without looking at the patch in detail what is the rationale for the pass placement (looks quite early)? I would have guessed that the pass could benefit from value-range analysis. Jeff, Steve is it possible to trigger the transform by simply manually forcing the right path jump-threads from inside VRP? That is, basically integrate the transform part with the existing jump threading framework but do an alternate discovery pass? This seems like what I tried to do last year with: https://gcc.gnu.org/ml/gcc-patches/2013-06/msg01121.html It turns Jeff's jump-threading code in to a strange franken-pass of bits and pieces of detection and optimisation, and would need some substantial reworking to fit in with Jeff's changes last Autumn, but if it is more likely to be acceptable for trunk then perhaps we could look to revive it. It would be nice to reuse the path copy code Jeff added last year, but I don't have much intuition as to how feasible that is. Was this the sort of thing that you were imagining? Yeah, didn't look too closely though. Richard. Steve, Jeff? James Thanks, Richard. Steve Ellcey sell...@mips.com 2014-08-12 Steve Ellcey sell...@mips.com PR tree-opt/54742 * Makefile.in (OBJS): Add tree-switch-shortcut.o. * common.opt (ftree-switch-shortcut): New. * opts.c (default_options_table): Add OPT_ftree_switch_shortcut. * params.def (PARAM_MAX_SWITCH_INSNS): New. (PARAM_MAX_SWITCH_PATHS): New. * passes.def (pass_tree_switch_shortcut): New. * timevar.def (TV_TREE_SWITCH_SHORTCUT): New. * tree-pass.h (make_pass_tree_switch_shortcut): New. * tree-switch-shortcut.c: New. 2014-08-12 Steve Ellcey sell...@mips.com PR tree-opt/54742 * gcc.dg/pr54742.c: New test.
[Patch 1/2] Don't put out a call to memcpy for volatile struct operations
Hi, Andrew is quite right, we break the contract for volatile struct copies if we start double copying bytes. But, the generic code will call memcpy - at which point anything could happen. So, we must not put out a call to memcpy if either our source or destination operands are volatile. The same is true of memset, so also disable that call for volatile operands, and add a fallback loop implementation. Bootstrapped on x86, Arm and AArch64 with no issues. OK? Thanks, James --- gcc/ 2014-08-21 James Greenhalgh james.greenha...@arm.com * expr.c (set_storage_via_loop): New. (emit_block_move_hints): Do not call memcpy with volatile operands. (emit_block_move_via_movmem): Clarify that targets do have to care about volatile operands. (clear_storage_hints): Do not call memset for volatile operands, fall back to a loop implementation. gcc/testsuite/ 2014-08-21 James Greenhalgh james.greenha...@arm.com * gcc.dg/large-volatile-struct-copy-1.c: New. * gcc.dg/large-volatile-struct-set-1.c: Likewise. diff --git a/gcc/expr.c b/gcc/expr.c index 920d47b..764525f 100644 --- a/gcc/expr.c +++ b/gcc/expr.c @@ -134,6 +134,7 @@ static void store_by_pieces_1 (struct store_by_pieces_d *, unsigned int); static void store_by_pieces_2 (insn_gen_fn, machine_mode, struct store_by_pieces_d *); static tree clear_storage_libcall_fn (int); +static void set_storage_via_loop (rtx, rtx, rtx, unsigned int); static rtx_insn *compress_float_constant (rtx, rtx); static rtx get_subtarget (rtx); static void store_constructor_field (rtx, unsigned HOST_WIDE_INT, @@ -1139,6 +1140,10 @@ emit_block_move_hints (rtx x, rtx y, rtx size, enum block_op_methods method, x = adjust_address (x, BLKmode, 0); y = adjust_address (y, BLKmode, 0); + /* memcpy is not guaranteed to be safe for volatile operands. */ + may_use_call = (!MEM_VOLATILE_P (x) + !MEM_VOLATILE_P (y)); + /* Set MEM_SIZE as appropriate for this block copy. The main place this can be incorrect is coming from __builtin_memcpy. */ if (CONST_INT_P (size)) @@ -2788,15 +2793,62 @@ clear_storage_hints (rtx object, rtx size, enum block_op_methods method, expected_align, expected_size, min_size, max_size, probable_max_size)) ; - else if (ADDR_SPACE_GENERIC_P (MEM_ADDR_SPACE (object))) + else if (ADDR_SPACE_GENERIC_P (MEM_ADDR_SPACE (object)) + !MEM_VOLATILE_P (object)) return set_storage_via_libcall (object, size, const0_rtx, method == BLOCK_OP_TAILCALL); else -gcc_unreachable (); +set_storage_via_loop (object, size, const0_rtx, align); return NULL; } +/* A subroutine of clear_storage. Set the data via an explicit + loop. This is used only when libcalls are forbidden. */ +/* ??? It'd be nice to set in hunks larger than QImode. */ + +static void +set_storage_via_loop (rtx object, rtx size, rtx val, + unsigned int align ATTRIBUTE_UNUSED) +{ + rtx cmp_label, top_label, iter, object_addr, tmp; + enum machine_mode object_addr_mode = get_address_mode (object); + enum machine_mode iter_mode; + + iter_mode = GET_MODE (size); + if (iter_mode == VOIDmode) +iter_mode = word_mode; + + top_label = gen_label_rtx (); + cmp_label = gen_label_rtx (); + iter = gen_reg_rtx (iter_mode); + + emit_move_insn (iter, const0_rtx); + + object_addr = force_operand (XEXP (object, 0), NULL_RTX); + do_pending_stack_adjust (); + + emit_jump (cmp_label); + emit_label (top_label); + + tmp = convert_modes (object_addr_mode, iter_mode, iter, true); + object_addr = simplify_gen_binary (PLUS, object_addr_mode, object_addr, tmp); + + object = change_address (object, QImode, object_addr); + + emit_move_insn (object, val); + + tmp = expand_simple_binop (iter_mode, PLUS, iter, const1_rtx, iter, + true, OPTAB_LIB_WIDEN); + if (tmp != iter) +emit_move_insn (iter, tmp); + + emit_label (cmp_label); + + emit_cmp_and_jump_insns (iter, size, LT, NULL_RTX, iter_mode, + true, top_label, REG_BR_PROB_BASE * 90 / 100); +} + rtx clear_storage (rtx object, rtx size, enum block_op_methods method) { diff --git a/gcc/testsuite/gcc.dg/large-volatile-struct-copy-1.c b/gcc/testsuite/gcc.dg/large-volatile-struct-copy-1.c new file mode 100644 index 000..32e4bdf --- /dev/null +++ b/gcc/testsuite/gcc.dg/large-volatile-struct-copy-1.c @@ -0,0 +1,37 @@ +/* { dg-do run } */ +/* { dg-options -O3 --save-temps } */ + +#define SIZE 1000 + +extern void abort (void); + +struct foo +{ + char data[SIZE]; +}; + +void __attribute__ ((noinline)) +func (struct foo volatile *x, struct foo volatile *y) +{ + *x = *y; +} + +int +main (int argc, char** argv) +{ + /* We just need something to copy, it doesn't much matter what it is. */ + volatile struct foo y = { 1, 2, 3 }; + volatile struct foo x; + int i = 0; + + func (x, y); + + for (i = 0; i SIZE; ++i) +if (x.data[i] != y.data[i]) + abort (); + + return 0; +} + +/* {
[Patch AArch64 2/2] Do not double-copy bytes in volatile struct operations
Hi, We also need to be careful in AArch64's movmem implementation, we can't expand to our overlapping mode of operation. Bootstrapped with no issues. OK? Thanks, James --- 2014-08-21 James Greenhalgh james.greenha...@arm.com * config/aarch64/aarch64.c (aarch64_expand_movmem): Fail if we have volatile operands. diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c index 0f3c74b..56434bc 100644 --- a/gcc/config/aarch64/aarch64.c +++ b/gcc/config/aarch64/aarch64.c @@ -9741,6 +9741,10 @@ aarch64_expand_movmem (rtx *operands) if (!CONST_INT_P (operands[2])) return false; + /* We can't do anything smart if either of the operands are volatile. */ + if (MEM_VOLATILE_P (src) || MEM_VOLATILE_P (dst)) +return false; + n = UINTVAL (operands[2]); /* Try to keep the number of instructions low. For cases below 16 bytes we
Re: spar-rtems add leon3 multlib to 4.9
Hello Joel, this is the wrong patch. We need also the user mode CAS variants for hypervisor support, e.g. https://gcc.gnu.org/ml/gcc-patches/2014-05/msg00057.html We should also wait for Daniel's latest stuff. On 20/08/14 15:51, Joel Sherrill wrote: I would like to add this patch to the 4.9 branch. It is RTEMS specific and takes advantage of the leon3 multilib support from Eric Botcazou. OK to commit? --joel gcc/ChangeLog 2013-08-29 Sebastian Hubersebastian.hu...@embedded-brains.de * config/sparc/t-rtems: Add leon3 multilibs. --- gcc/config/sparc/t-rtems |4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/gcc/config/sparc/t-rtems b/gcc/config/sparc/t-rtems index 63d0217..f1a3d84 100644 --- a/gcc/config/sparc/t-rtems +++ b/gcc/config/sparc/t-rtems @@ -17,6 +17,6 @@ #http://www.gnu.org/licenses/. # -MULTILIB_OPTIONS = msoft-float mcpu=v8 -MULTILIB_DIRNAMES = soft v8 +MULTILIB_OPTIONS = msoft-float mcpu=v8/mcpu=leon3 +MULTILIB_DIRNAMES = soft v8 leon3 MULTILIB_MATCHES = msoft-float=mno-fpu -- Sebastian Huber, embedded brains GmbH Address : Dornierstr. 4, D-82178 Puchheim, Germany Phone : +49 89 189 47 41-16 Fax : +49 89 189 47 41-09 E-Mail : sebastian.hu...@embedded-brains.de PGP : Public key available on request. Diese Nachricht ist keine geschäftliche Mitteilung im Sinne des EHUG.
[PATCH] Fix PR62175, avoid trapping expressions in niters
This fixes PR62175 by avoiding trapping expressions in niter results. If those appear inside conditionals that are later re-gimplified the gimplifier will ICE (because it thinks it has to create control-flow). Now it seems we should use un-expanded expressions for code-generation (we use expanded form to eventually simplify the expression). But that's an optimization and for another day. Bootstrap and regtest running on x86_64-unknown-linux-gnu. Richard. 2014-08-21 Richard Biener rguent...@suse.de PR tree-optimization/62175 * tree-ssa-loop-niter.c (expand_simple_operations): Do not expand possibly trapping operations. * g++.dg/torture/pr62175.C: New testcase. Index: gcc/tree-ssa-loop-niter.c === --- gcc/tree-ssa-loop-niter.c (revision 214258) +++ gcc/tree-ssa-loop-niter.c (working copy) @@ -1633,6 +1633,9 @@ expand_simple_operations (tree expr) case PLUS_EXPR: case MINUS_EXPR: + if (TYPE_OVERFLOW_TRAPS (TREE_TYPE (expr))) + return expr; + /* Fallthru. */ case POINTER_PLUS_EXPR: /* And increments and decrements by a constant are simple. */ e1 = gimple_assign_rhs2 (stmt); Index: gcc/testsuite/g++.dg/torture/pr62175.C === --- gcc/testsuite/g++.dg/torture/pr62175.C (revision 0) +++ gcc/testsuite/g++.dg/torture/pr62175.C (working copy) @@ -0,0 +1,36 @@ +// { dg-do compile } +// { dg-additional-options -ftrapv } + +struct B { +B(int = 0); +}; +int c; +int *d; +struct G { +G(); +int numProcs_; +}; +int fn1(); +B fn2() { +if (c) + return 0; +return B(); +} + +long fn3(long p1, long p2) { +if (p2 p1) + return p2; +return p1; +} + +void fn4(long p1) { +long a = fn1(); +fn2(); +int b = fn3(p1, a); +for (int i; i b; ++i) + d[0] = 0; +for (; a p1; ++a) + d[a] = 0; +} + +G::G() { fn4(numProcs_ + 1); }
Re: [Patch 1/2] Don't put out a call to memcpy for volatile struct operations
On Thu, Aug 21, 2014 at 12:34 PM, James Greenhalgh james.greenha...@arm.com wrote: Hi, Andrew is quite right, we break the contract for volatile struct copies if we start double copying bytes. But, the generic code will call memcpy - at which point anything could happen. So, we must not put out a call to memcpy if either our source or destination operands are volatile. The same is true of memset, so also disable that call for volatile operands, and add a fallback loop implementation. Bootstrapped on x86, Arm and AArch64 with no issues. OK? Umm... using a byte-wise clearing loop is surely always the wrong thing for an access that _really_ cares about volatile. I see we do the same for the block-move case... ugh. I still say we need to solve the issue at language level - that is, try to figure out what the language standard says about volatile struct X x, y; x = y; or about struct X { volatile int x; } x, y; x = y; where we don't even _have_ MEM_VOLATILE set on x or y. I expect that most structs have volatile for a bogus reason anyway and we slow down and enlarge code for no good reason. So - why bother fixing this? ISTR reading in the C standard that structure assignments are expected to compile to memcpy. Richard. Thanks, James --- gcc/ 2014-08-21 James Greenhalgh james.greenha...@arm.com * expr.c (set_storage_via_loop): New. (emit_block_move_hints): Do not call memcpy with volatile operands. (emit_block_move_via_movmem): Clarify that targets do have to care about volatile operands. (clear_storage_hints): Do not call memset for volatile operands, fall back to a loop implementation. gcc/testsuite/ 2014-08-21 James Greenhalgh james.greenha...@arm.com * gcc.dg/large-volatile-struct-copy-1.c: New. * gcc.dg/large-volatile-struct-set-1.c: Likewise.
[PATCH][match-and-simplify] Auto-guess conversion types
This adds the capability to auto-guess the type of non-outermost conversions by looking at the parent expression type. This also adds fatal () to the cases we can't auto-detect. Bootstrapped on x86_64-unknown-linux-gnu, testing in progress. Richard. 2014-08-21 Richard Biener rguent...@suse.de * genmatch.c (*::gen_transform): Add type parameter. (expr::gen_transform): Handle conversion operators by converting to the parent expression type. Properly pass down the expression type to children. (dt_simplify::gen_gimple): Pass down 'type' to gen_transform. (dt_simplify::gen_generic): Likewise. * match.pd ((T1)(~(T2) X) - ~(T1) X): New pattern. Index: gcc/match.pd === --- gcc/match.pd(revision 214224) +++ gcc/match.pd(working copy) @@ -107,6 +107,19 @@ along with GCC; see the file COPYING3. (abs @0)) +/* From fold_unary. */ + +/* (T1)(~(T2) X) - ~(T1) X */ +(simplify + (convert (bit_not@0 (convert @1))) + (if (INTEGRAL_TYPE_P (type) + INTEGRAL_TYPE_P (TREE_TYPE (@0)) + TYPE_PRECISION (type) == TYPE_PRECISION (TREE_TYPE (@0)) + INTEGRAL_TYPE_P (TREE_TYPE (@1)) + TYPE_PRECISION (type) = TYPE_PRECISION (TREE_TYPE (@1))) + (bit_not (convert (bit_not @0) + + #include match-plusminus.pd #include match-bitwise.pd #include match-rotate.pd Index: gcc/genmatch.c === --- gcc/genmatch.c (revision 214258) +++ gcc/genmatch.c (working copy) @@ -238,14 +238,14 @@ struct operand { enum op_type { OP_PREDICATE, OP_EXPR, OP_CAPTURE, OP_C_EXPR }; operand (enum op_type type_) : type (type_) {} enum op_type type; - virtual void gen_transform (FILE *f, const char *, bool, int) = 0; + virtual void gen_transform (FILE *f, const char *, bool, int, const char *) = 0; }; struct predicate : public operand { predicate (const char *ident_) : operand (OP_PREDICATE), ident (ident_) {} const char *ident; - virtual void gen_transform (FILE *, const char *, bool, int) + virtual void gen_transform (FILE *, const char *, bool, int, const char *) { gcc_unreachable (); } }; @@ -263,7 +263,7 @@ struct expr : public operand void append_op (operand *op) { ops.safe_push (op); } e_operation *operation; vecoperand * ops; - virtual void gen_transform (FILE *f, const char *, bool, int); + virtual void gen_transform (FILE *f, const char *, bool, int, const char *); }; struct c_expr : public operand @@ -286,7 +286,7 @@ struct c_expr : public operand char *fname; vecid_tab ids; - virtual void gen_transform (FILE *f, const char *, bool, int); + virtual void gen_transform (FILE *f, const char *, bool, int, const char *); void output_code (FILE *f, bool); }; @@ -296,7 +296,7 @@ struct capture : public operand : operand (OP_CAPTURE), where (where_), what (what_) {} const char *where; operand *what; - virtual void gen_transform (FILE *f, const char *, bool, int); + virtual void gen_transform (FILE *f, const char *, bool, int, const char *); }; template @@ -860,32 +860,71 @@ check_no_user_id (simplify *s) /* Code gen off the AST. */ void -expr::gen_transform (FILE *f, const char *dest, bool gimple, int depth) +expr::gen_transform (FILE *f, const char *dest, bool gimple, int depth, +const char *in_type) { + bool is_conversion = false; + if (*operation-op == CONVERT_EXPR + || *operation-op == NOP_EXPR + || *operation-op == FLOAT_EXPR + || *operation-op == FIX_TRUNC_EXPR) +is_conversion = true; + + const char *type; + char optype[20]; + if (is_conversion) +/* For conversions we need to build the expression using the + outer type passed in. */ +type = in_type; + else +{ + /* Other operations are of the same type as their first operand. */ + sprintf (optype, TREE_TYPE (ops%d[0]), depth); + type = optype; +} + if (!type) +fatal (two conversions in a row); + fprintf (f, {\n); fprintf (f, tree ops%d[%u], res;\n, depth, ops.length ()); for (unsigned i = 0; i ops.length (); ++i) { char dest[32]; snprintf (dest, 32, ops%d[%u], depth, i); - ops[i]-gen_transform (f, dest, gimple, depth + 1); + ops[i]-gen_transform (f, dest, gimple, depth + 1, +is_conversion +/* If this op is a conversion its single + operand has to know its type itself. */ +? NULL +/* For other ops the type is the type + we got passed in, or if that is from + a conversion we can at most use the + first operand type for all further + operands. So (convert (plus @1 (convert @2)) +
[PATCH][match-and-simplify] Merge dt_simplify::gen_gimple and dt_simplify::gen_generic
This merges the two functions that have much in common and prettifies the code-gen. Committed. Richard. 2014-08-21 Richard Biener rguent...@suse.de * genmatch.c (dt_simplify::gen): New function merged from ... (dt_simplify::gen_gimple, dt_simplify::gen_generic): ... these, made to simple dispatchers. Index: gcc/genmatch.c === --- gcc/genmatch.c (revision 214266) +++ gcc/genmatch.c (working copy) @@ -457,8 +457,9 @@ struct dt_simplify: public dt_node indexes[i] = indexes_[i]; } - virtual void gen_gimple (FILE *f); - virtual void gen_generic (FILE *f); + void gen (FILE *f, bool); + virtual void gen_gimple (FILE *f) { gen (f, true); } + virtual void gen_generic (FILE *f) { gen (f, false); } }; template @@ -1690,10 +1691,9 @@ dt_operand::gen_generic_kids (FILE *f) void -dt_simplify::gen_gimple (FILE *f) +dt_simplify::gen (FILE *f, bool gimple) { - - fprintf (f, /* simplify %u */\n, pattern_no); + output_line_directive (f, s-result_location); fprintf (f, {\n); fprintf (f, tree captures[%u] ATTRIBUTE_UNUSED = {};\n, dt_simplify::capture_max); @@ -1708,20 +1708,24 @@ dt_simplify::gen_gimple (FILE *f) if (s-ifexpr_vec != vNULL) { fprintf (f, if (); - // we add in LIFO order, so traverse backwards - for (unsigned i = s-ifexpr_vec.length (); i; --i) - { - fprintf (f, (); - s-ifexpr_vec[i - 1]-gen_transform (f, NULL, true, 1, type); - fprintf (f, )); - if (i 1) - fprintf (f, ); - } + if (s-ifexpr_vec.length () == 1) + s-ifexpr_vec[0]-gen_transform (f, NULL, true, 1, type); + else + // we add in LIFO order, so traverse backwards + for (unsigned i = s-ifexpr_vec.length (); i; --i) + { + fprintf (f, (); + s-ifexpr_vec[i - 1]-gen_transform (f, NULL, true, 1, type); + fprintf (f, )); + if (i 1) + fprintf (f, \n ); + } fprintf (f, )\n); fprintf (f, {\n); } - output_line_directive (f, s-result_location); + if (gimple) +{ if (s-result-type == operand::OP_EXPR) { expr *e = static_cast expr * (s-result); @@ -1745,49 +1749,10 @@ dt_simplify::gen_gimple (FILE *f) } else gcc_unreachable (); - fprintf (f, return true;\n); - if (s-ifexpr_vec != vNULL) - fprintf (f, }\n); - - fprintf (f, }\n); -} - - -void -dt_simplify::gen_generic (FILE *f) -{ - - fprintf (f, /* simplify %u */\n, pattern_no); - - fprintf (f, {\n); - fprintf (f, tree captures[%u] ATTRIBUTE_UNUSED = {};\n, dt_simplify::capture_max); - - for (unsigned i = 0; i dt_simplify::capture_max; ++i) -if (indexes[i]) - { - char opname[20]; - fprintf (f, captures[%u] = %s;\n, i, indexes[i]-get_name (opname)); - } - - if (s-ifexpr_vec != vNULL) -{ - fprintf (f, if (\n); - // we add in LIFO order, so traverse backwards - for (unsigned i = s-ifexpr_vec.length (); i; --i) -{ - fprintf (f, (); - s-ifexpr_vec[i - 1]-gen_transform (f, NULL, false, 1, type); - fprintf (f, )); - if (i 1) -fprintf (f, ); -} - fprintf (f, )\n); - fprintf (f, {\n); } - - output_line_directive (f, s-result_location); - + else /* GENERIC */ +{ if (s-result-type == operand::OP_EXPR) { expr *e = static_cast expr * (s-result); @@ -1818,15 +1783,15 @@ dt_simplify::gen_generic (FILE *f) } else gcc_unreachable (); +} - if (s-ifexpr_vec != vNULL) - fprintf (f, }\n); + if (s-ifexpr_vec != vNULL) +fprintf (f, }\n); fprintf (f, }\n); } - void decision_tree::gen_gimple (FILE *f) {
Re: [PATCH, DOC]: New value 'default' for --enable-languages
Ping. There was no explicit agreement that I can commit the change to trunk? Thanks, Martin On 07/30/2014 08:19 PM, Martin Liška wrote: On 07/30/2014 06:38 PM, Mike Stump wrote: On Jul 30, 2014, at 6:20 AM, Richard Biener richard.guent...@gmail.com wrote: On Wed, Jul 30, 2014 at 3:19 PM, Richard Biener richard.guent...@gmail.com wrote: On Wed, Jul 30, 2014 at 1:37 PM, Martin Liška mli...@suse.cz wrote: Hello, as discussed in previous thread (https://gcc.gnu.org/ml/gcc-patches/2014-07/msg02010.html), I would like to add more intuitive behavior for --enable-languages configure option. It works for me, but as I'm currently always testing all,ada,obj-c++ how can I easily continue to do that - that is, not test go? ;) Of course with default,ada,obj-c++ ... stupid me. In time, we’ll have a all,!go…. :-) Does 'go' mean that the patch is ready for trunk :D ? Martin
Re: [c++-concepts] variable concept fixes
Ah... thanks for the clarification. Fixed (and committed). Andrew On Thu, Aug 21, 2014 at 4:26 AM, Paolo Carlini paolo.carl...@oracle.com wrote: Hi Andrew, On 08/20/2014 11:08 PM, Andrew Sutton wrote: On 08/20/2014 08:56 PM, Andrew Sutton wrote: + return VAR_P (decl) + DECL_LANG_SPECIFIC (decl) + DECL_DECLARED_CONCEPT_P (decl); this is brittle from the formatting point of view. Please double check in detail what I'm going to say, but I think that in such cases you simply want to wrap the whole thing in round parentheses. Sorry, did you just mean to wrap the entire conjunction in parens? I'm trying to find the formatting guidelines to check, but not succeeding at the moment. Yes, I meant the whole conjunction, sorry about my sloppy language. In terms of GNU coding standards: http://www.gnu.org/prep/standards/standards.html#Formatting toward the end of the section, for example. Paolo. Index: gcc/cp/decl.c === --- gcc/cp/decl.c (revision 214241) +++ gcc/cp/decl.c (working copy) @@ -6264,9 +6264,9 @@ value_dependent_init_p (tree init) static inline bool is_concept_var (tree decl) { - return VAR_P (decl) - DECL_LANG_SPECIFIC (decl) - DECL_DECLARED_CONCEPT_P (decl); + return (VAR_P (decl) + DECL_LANG_SPECIFIC (decl) + DECL_DECLARED_CONCEPT_P (decl)); } /* Finish processing of a declaration;
Re: [PATCH][RFC][match-and-simplify] Manually written patterns
On Fri, 15 Aug 2014, Richard Biener wrote: The following introduces manually written patterns. That is, part of the matching and the transform are fully manual. An example where this is necessary is when the result isn't really an expression but a series of statements. For example take simplifications of the memset builtin. With the proposal we coud write (simplify (BUILT_IN_MEMSET @1 @2 integer_zerop) @1) (simplify (BUILT_IN_MEMSET (addr@1 @4) INTEGER_CST_P@2 tree_fits_uhwi_p@3) (if (gimple_simplify_memset (@1, @2, @3, res_code, res_ops, seq, valueize))) /* Note result intentionally omitted. The predicate if applying is supposed to have populated *res_code and *res_ops and seq. */) covering the zero-length case with a regular pattern and the rest with a if-expr predicate that also does the transform. Note that parts of the argument constraining is done via regular matching predicates and the pattern is inserted into the decision tree as usual. How gimple_simplify_memset looks like is visible in the patch. Note that this exposes the implementation details of the _GIMPLE_ code-path (so the above doesn't even apply to GENERIC - luckily I've not implemented builtin function simplification for GENERIC so the above doesn't fall over ;)). The syntax for the trailing args could be made nicer, but we use 'type' freely as well. It clearly abuses (if ...) but it fits kind-of well. Makes simply omitting the result pattern in a regular simplify fail in interesting ways though... Caveat: runs into the issue that it's not possible to query the number of arguments to a function (thus no re-simplification yet). I can lookup the decl for the builtin and parse its DECL_ARGUMENTS, but well... Similar issue exists when parsing built-in calls, we can't error on not enough arguments. Status: it builds. Comments? No changes but updated patch. As the important part is being able to re-simplify calls so we can continue to stage sprintf_chk - sprintf - memcpy I still need to figure out the best way doing that and implement example patterns exercising this. I also need to merge from trunk so I can re-order the match-and-simplify transform to happen before the rest in fold_stmt. Richard. 2014-08-15 Richard Biener rguent...@suse.de * match.pd: Add example memset simplification with manual implemented part. * gimple-fold.c (gimple_simplify_memset): New function. * gimple-fold.h (gimple_simplify_memset): Declare. * gimple-match-head.c (gimple_resimplify): New function. * genmatch.c (check_no_user_id): Guard against NULL result. (write_header): Likewise. (dt_simplify::gen_gimple): Deal with NULL result. (parse_simplify): Allow missing result. Index: gcc/match.pd === *** gcc/match.pd.orig 2014-08-21 13:54:41.787051398 +0200 --- gcc/match.pd2014-08-21 14:11:56.196980181 +0200 *** along with GCC; see the file COPYING3. *** 127,132 --- 127,150 #include match-constant-folding.pd #include match-comparison.pd + + /* Manual simplifications but still in the decision tree. +Allows us to strip off easy parts and (parts of) the +pattern/predicate matching. */ + + (simplify + (BUILT_IN_MEMSET @1 @2 @3) + /* Size zero memset simplifies to the destination pointer. */ + (if (integer_zerop (@3)) +@1) + (if (TREE_CODE (@1) == ADDR_EXPR + TREE_CODE (@2) == INTEGER_CST + tree_fits_uhwi_p (@3) + gimple_simplify_memset (@1, @2, @3, res_code, res_ops, seq, valueize)) +/* Note result intentionally omitted. The predicate if applying is + supposed to have populated *res_code and *res_ops and seq. */)) + + /* s We cannot reasonably match vector CONSTRUCTORs or vector constants Index: gcc/gimple-fold.c === *** gcc/gimple-fold.c.orig 2014-08-21 13:54:41.801051398 +0200 --- gcc/gimple-fold.c 2014-08-21 14:16:46.148960218 +0200 *** gimple_fold_builtin_memset (gimple_stmt_ *** 1335,1340 --- 1335,1414 return true; } + /* Manual simplification example. +Fold function call to builtin memset or bzero setting the +memory of size LEN to VAL. Return whether a simplification was made. */ + + bool + gimple_simplify_memset (tree dest, tree c, tree len, + code_helper *res_code, tree *res_ops, + gimple_seq *seq, tree (*)(tree)) + { + tree etype; + unsigned HOST_WIDE_INT length, cval; + + if (!seq) + return false; + + /* If the LEN parameter is zero, this is handled by another pattern. + But as they are only differing in predicates we can still arrive + here (there isn't a integer_nonzerop). */ + if (integer_zerop (len)) + return false; + + gcc_assert
Re: [PATCH][match-and-simplify] Auto-guess conversion types
On Thu, 21 Aug 2014, Richard Biener wrote: +/* From fold_unary. */ + +/* (T1)(~(T2) X) - ~(T1) X */ +(simplify + (convert (bit_not@0 (convert @1))) + (if (INTEGRAL_TYPE_P (type) + INTEGRAL_TYPE_P (TREE_TYPE (@0)) + TYPE_PRECISION (type) == TYPE_PRECISION (TREE_TYPE (@0)) + INTEGRAL_TYPE_P (TREE_TYPE (@1)) + TYPE_PRECISION (type) = TYPE_PRECISION (TREE_TYPE (@1))) + (bit_not (convert (bit_not @0) There are a lot of bit_not in this line... I know the patterns themselves aren't your main preoccupation right now, and I agree that finishing the infrastructure is the priority, but it seems that the comments are becoming much terser during the move from fold-const to *.pd. I believe the == could be =, so I wanted to check the rationale, and fold-const at least tries to explain the condition. It would be nice to copy-paste those comments, if the version in fold-const.c is supposed to disappear. (if that's planned for later, please ignore my message) -- Marc Glisse
[PATCH][match-and-simplify] Merge from trunk
Committed. Richard. 2014-08-21 Richard Biener rguent...@suse.de Merge from trunk r213754 through r214265.
Re: [PATCH][match-and-simplify] Auto-guess conversion types
On Thu, 21 Aug 2014, Marc Glisse wrote: On Thu, 21 Aug 2014, Richard Biener wrote: +/* From fold_unary. */ + +/* (T1)(~(T2) X) - ~(T1) X */ +(simplify + (convert (bit_not@0 (convert @1))) + (if (INTEGRAL_TYPE_P (type) + INTEGRAL_TYPE_P (TREE_TYPE (@0)) + TYPE_PRECISION (type) == TYPE_PRECISION (TREE_TYPE (@0)) + INTEGRAL_TYPE_P (TREE_TYPE (@1)) + TYPE_PRECISION (type) = TYPE_PRECISION (TREE_TYPE (@1))) + (bit_not (convert (bit_not @0) There are a lot of bit_not in this line... Oops. I've meant to delete all excessive stuff I put there for parser testing... Fixed. I know the patterns themselves aren't your main preoccupation right now, and I agree that finishing the infrastructure is the priority, but it seems that the comments are becoming much terser during the move from fold-const to *.pd. I believe the == could be =, so I wanted to check the rationale, and fold-const at least tries to explain the condition. It would be nice to copy-paste those comments, if the version in fold-const.c is supposed to disappear. No, you are right - it's likely they'll get lost this way. But I _do_ have to do more systematic pattern writing at some point (started on fold_conversion this week just to notice the desirable changes to the language I've put in this week...). Thanks for keeping an eye on these patches, Richard. 2014-08-21 Richard Biener rguent...@suse.de * match.pd ((T1)(~(T2) X) - ~(T1) X): Paste all comment from fold-const.c, fix simplification result. Index: gcc/match.pd === --- gcc/match.pd(revision 214268) +++ gcc/match.pd(working copy) @@ -109,7 +109,9 @@ along with GCC; see the file COPYING3. /* From fold_unary. */ -/* (T1)(~(T2) X) - ~(T1) X */ +/* Convert (T1)(~(T2)X) into ~(T1)X if T1 and T2 are integral types + of the same precision, and X is an integer type not narrower than + types T1 or T2, i.e. the cast (T2)X isn't an extension. */ (simplify (convert (bit_not@0 (convert @1))) (if (INTEGRAL_TYPE_P (type) @@ -117,7 +119,7 @@ along with GCC; see the file COPYING3. TYPE_PRECISION (type) == TYPE_PRECISION (TREE_TYPE (@0)) INTEGRAL_TYPE_P (TREE_TYPE (@1)) TYPE_PRECISION (type) = TYPE_PRECISION (TREE_TYPE (@1))) - (bit_not (convert (bit_not @0) + (bit_not (convert @0 #include match-plusminus.pd
Re: [PATCH][match-and-simplify] Auto-guess conversion types
On Thu, 21 Aug 2014, Richard Biener wrote: 2014-08-21 Richard Biener rguent...@suse.de * match.pd ((T1)(~(T2) X) - ~(T1) X): Paste all comment from fold-const.c, fix simplification result. Index: gcc/match.pd === --- gcc/match.pd(revision 214268) +++ gcc/match.pd(working copy) @@ -109,7 +109,9 @@ along with GCC; see the file COPYING3. /* From fold_unary. */ -/* (T1)(~(T2) X) - ~(T1) X */ +/* Convert (T1)(~(T2)X) into ~(T1)X if T1 and T2 are integral types + of the same precision, and X is an integer type not narrower than + types T1 or T2, i.e. the cast (T2)X isn't an extension. */ (simplify (convert (bit_not@0 (convert @1))) (if (INTEGRAL_TYPE_P (type) @@ -117,7 +119,7 @@ along with GCC; see the file COPYING3. TYPE_PRECISION (type) == TYPE_PRECISION (TREE_TYPE (@0)) INTEGRAL_TYPE_P (TREE_TYPE (@1)) TYPE_PRECISION (type) = TYPE_PRECISION (TREE_TYPE (@1))) - (bit_not (convert (bit_not @0) + (bit_not (convert @0 Er, I think you mean: (bit_not (convert @1)) Your pattern (bit_not (convert (bit_not @0))) was valid, @0 is the bit_not, so it simplifies, but that was a pretty unreadable way to write it, and I was scared that you had done it on purpose to help the machinery guess the type. Glad to see that the simple version is ok. -- Marc Glisse
Re: [GOMP4, RFC] OpenMP4 offload support for Intel PHI targets.
Hello, The branch was rebased and updated: https://gcc.gnu.org/git/?p=gcc.git;a=shortlog;h=refs/heads/kyukhin/gomp4-offload Now it requires even less actions to build and run an executable with offloading. Instructions on wiki were updated: https://gcc.gnu.org/wiki/Offloading#How_to_try_offloading_enabled_GCC Bootstrap and all tests in 'make check-target-libgomp' passed. -- Ilya
[c++-concepts] constrained friends
Added tests for constrained friends. No code, we already to the right thing. 2014-08-15 Andrew Sutton andrew.n.sut...@gmail.com Add tests for constrained friends. * gcc/testsuite/g++.dg/concepts/friend1.C: New. * gcc/testsuite/g++.dg/concepts/friend2.C: New. Andrew Index: gcc/testsuite/g++.dg/concepts/friend2.C === --- gcc/testsuite/g++.dg/concepts/friend2.C (revision 0) +++ gcc/testsuite/g++.dg/concepts/friend2.C (revision 0) @@ -0,0 +1,20 @@ +// { dg-options -std=c++1z } + +templatetypename T + concept bool Eq() { return requires(T t) { t == t; }; } + +templateEq T struct Foo { }; + +templatetypename T + struct S { // { dg-error constraint failure } +templateEq U friend class Bar; + +friend class FooT; + }; + +struct X { }; + +int main() { + Sint si; // OK + SX sx; +} Index: gcc/testsuite/g++.dg/concepts/friend1.C === --- gcc/testsuite/g++.dg/concepts/friend1.C (revision 0) +++ gcc/testsuite/g++.dg/concepts/friend1.C (revision 0) @@ -0,0 +1,33 @@ +// { dg-options -std=c++1z } + +templatetypename T + concept bool Eq() { return requires(T t) { t == t; }; } + +struct Nt { + templateEq T friend void f(T) { } +} nt; + +templatetypename T struct S; + +templateEq T + void proc(ST*); + +templatetypename T + struct S { +friend bool operator==(S, S) requires EqT() { return true; } + +friend void proc(S*); // { dg-error does not match any template declaration } + }; + +struct X { } x; + +int main() { + f(0); // OK + f(x); // { dg-error cannot call } + + Sint si; + si == si; // OK + + SX sx; + sx == sx; // { dg-error no match } +}
Re: [PATCH 045/236] define_bypass guard functions take a pair of rtx_insn
On Wed, 2014-08-13 at 12:07 -0600, Jeff Law wrote: On 08/06/14 11:20, David Malcolm wrote: (define_bypass) clauses in .md files can specify the name of a guard function as their final operand. Currently these functions are called with a pair of rtx. This patch strengthens insn-automata.c so that such guard functions are passed a pair of rtx_insn *, allowing these guard functions to be similarly strengthened in the per-target phase of this patch kit. gcc/ * genautomata.c (output_internal_insn_latency_func): When writing the function internal_insn_latency to insn-automata.c, strengthen params insn and insn2 from rtx to rtx_insn *, thus allowing the optional guard function of (define_bypass) clauses to expect a pair of rtx_insn *, rather than a pair of rtx. (output_insn_latency_func): When writing the function insn_latency, add an uncast_ prefix to params insn and insn2, reintroducing insn and insn2 as rtx_insn * locals using checked casts from the params, thus enabling the above change to the generated internal_insn_latency function. OK. Thanks; committed to trunk as r214273, with the trivial fixup of as_a_nullable to safe_as_a.
Re: [RFC, PATCH 4/n] IPA C++ refactoring
Hello, following patch introduces a new symbol_table class, encapsulating functions related to symbol table management. Apart from that, cgraph_edge related functions become members of the class. Finally, a portion of clean-up has been applied to cgraph.h. Bootstrapped on x86_64-pc-linux-gnu and majority of BEs have been tested for C and C++, no regression observed. Thank you, Martin - if (!used_as_abstract_origin cgraph_state != CGRAPH_STATE_PARSING) + if (!used_as_abstract_origin symtab-state != CGRAPH_STATE_PARSING) I would probably rename this CGRAPH_ enum into SYMTAB. Given it is a symtab state now ;) struct GTY((chain_next (%h.next_caller), chain_prev (%h.prev_caller))) cgraph_edge { + /* Remove the edge in the cgraph. */ + void remove (void); + + /* Remove the edge from the list of the callers of the callee. */ + void remove_caller (void); + + /* Remove the edge from the list of the callees of the caller. */ + void remove_callee (void); Perhaps those two can become private? + + /* Change field call_stmt of edge to NEW_STMT. + If UPDATE_SPECULATIVE and E is any component of speculative + edge, then update all components. */ + void set_call_stmt (gimple new_stmt, bool update_speculative = true); + + /* Redirect callee of the edge to N. The function does not update underlying + call expression. */ + void redirect_callee (cgraph_node *n); + + /* Make an indirect edge with an unknown callee an ordinary edge leading to + CALLEE. DELTA is an integer constant that is to be added to the this + pointer (first parameter) to compensate for skipping + a thunk adjustment. */ + cgraph_edge *make_direct (cgraph_node *callee); + + /* Turn edge into speculative call calling N2. Update + the profile so the direct call is taken COUNT times + with FREQUENCY. */ + cgraph_edge *turn_to_speculative (cgraph_node *n2, gcov_type direct_count, + int direct_frequency); Porably make_speculative (like make_direct). + + /* If necessary, change the function declaration in the call statement + associated with the edge so that it corresponds to the edge callee. */ + gimple redirect_call_stmt_to_callee (void); + + /* Given speculative call edge, return all three components. */ + void speculative_call_info (cgraph_edge *direct, cgraph_edge *indirect, + ipa_ref *reference); + + /* Create clone of edge in the node N represented + by CALL_EXPR the callgraph. */ + cgraph_edge * clone (cgraph_node *n, gimple call_stmt, unsigned stmt_uid, +gcov_type count_scale, int freq_scale, bool update_original); + + /* Speculative call edge turned out to be direct call to CALLE_DECL. + Remove the speculative call sequence and return edge representing the call. + It is up to caller to redirect the call as appropriate. */ + cgraph_edge *resolve_speculation (tree callee_decl = NULL); Probably better to group speculation into one block. + + /* Return true when call of edge can not lead to return from caller + and thus it is safe to ignore its side effects for IPA analysis + when computing side effects of the caller. */ + bool cannot_lead_to_return_p (void); + + /* Return true when the edge represents a direct recursion. */ + bool recursive_p (void); + + /* Return true if the call can be hot. */ + bool maybe_hot_p (void); + + + /* Perform reachability analysis and reclaim all unreachable nodes. */ + bool remove_unreachable_nodes (bool before_inlining_p, FILE *file); + + /* Optimization of function bodies might've rendered some variables as + unnecessary so we want to avoid these from being compiled. Re-do + reachability starting from variables that are either externally visible + or was referred from the asm output routines. */ + void remove_unreferenced_decls (void); + + /* Process CGRAPH_NEW_FUNCTIONS and perform actions necessary to add these + functions into callgraph in a way so they look like ordinary reachable + functions inserted into callgraph already at construction time. */ + void process_new_functions (void); + + /* C++ frontend produce same body aliases all over the place, even before PCH + gets streamed out. It relies on us linking the aliases with their function + in order to do the fixups, but ipa-ref is not PCH safe. Consequentely we + first produce aliases without links, but once C++ FE is sure he won't sream + PCH we build the links via this function. */ + void process_same_body_aliases (void); + + /* Output all variables enqueued to be assembled. */ + bool output_variables (void); + + /* Output all asm statements we have stored up to be output. */ + void output_asm_statements (void); + + /* Analyze the whole compilation unit once it is parsed completely. */ +
[patch, nios2] testsuite cleanup
I've checked in this patch to tidy up some test cases observed to fail on nios2-elf; mostly tree-ssa tests that assume some non-default branch costs in the back end, plus a couple that pass -fPIC without requiring an effective target that supports that option. -Sandra 2014-08-21 Sandra Loosemore san...@codesourcery.com gcc/testsuite/ * lib/target-supports.exp (check_effective_target_logical_op_short_circuit): Add nios2. * gcc.dg/tree-ssa/reassoc-33.c: Skip for nios2. * gcc.dg/tree-ssa/reassoc-34.c: Likewise. * gcc.dg/tree-ssa/reassoc-35.c: Likewise. * gcc.dg/tree-ssa/reassoc-36.c: Likewise. * gcc.dg/tree-ssa/interposition.c: Require fpic effective target for test using -fPIC. * gcc.dg/lto/pr61526_0.c: Likewise. Index: gcc/testsuite/lib/target-supports.exp === --- gcc/testsuite/lib/target-supports.exp (revision 214241) +++ gcc/testsuite/lib/target-supports.exp (working copy) @@ -5970,6 +5970,7 @@ proc check_effective_target_logical_op_s || [istarget mmix-*-*] || [istarget s390*-*-*] || [istarget powerpc*-*-*] + || [istarget nios2*-*-*] || [check_effective_target_arm_cortex_m] } { return 1 } Index: gcc/testsuite/gcc.dg/tree-ssa/reassoc-33.c === --- gcc/testsuite/gcc.dg/tree-ssa/reassoc-33.c (revision 214241) +++ gcc/testsuite/gcc.dg/tree-ssa/reassoc-33.c (working copy) @@ -1,4 +1,4 @@ -/* { dg-do run { target { ! m68k*-*-* mmix*-*-* mep*-*-* bfin*-*-* v850*-*-* picochip*-*-* moxie*-*-* cris*-*-* m32c*-*-* fr30*-*-* mcore*-*-* powerpc*-*-* xtensa*-*-* hppa*-*-*} } } */ +/* { dg-do run { target { ! m68k*-*-* mmix*-*-* mep*-*-* bfin*-*-* v850*-*-* picochip*-*-* moxie*-*-* cris*-*-* m32c*-*-* fr30*-*-* mcore*-*-* powerpc*-*-* xtensa*-*-* hppa*-*-* nios2*-*-*} } } */ /* { dg-options -O2 -fno-inline -fdump-tree-reassoc1-details } */ /* { dg-additional-options -mbranch-cost=2 { target mips*-*-* avr-*-* s390*-*-* i?86-*-* x86_64-*-* } } */ Index: gcc/testsuite/gcc.dg/tree-ssa/reassoc-34.c === --- gcc/testsuite/gcc.dg/tree-ssa/reassoc-34.c (revision 214241) +++ gcc/testsuite/gcc.dg/tree-ssa/reassoc-34.c (working copy) @@ -1,4 +1,4 @@ -/* { dg-do run { target { ! m68k*-*-* mmix*-*-* mep*-*-* bfin*-*-* v850*-*-* picochip*-*-* moxie*-*-* cris*-*-* m32c*-*-* fr30*-*-* mcore*-*-* powerpc*-*-* xtensa*-*-* hppa*-*-*} } } */ +/* { dg-do run { target { ! m68k*-*-* mmix*-*-* mep*-*-* bfin*-*-* v850*-*-* picochip*-*-* moxie*-*-* cris*-*-* m32c*-*-* fr30*-*-* mcore*-*-* powerpc*-*-* xtensa*-*-* hppa*-*-* nios2*-*-*} } } */ /* { dg-options -O2 -fno-inline -fdump-tree-reassoc1-details } */ /* { dg-additional-options -mbranch-cost=2 { target mips*-*-* avr-*-* s390*-*-* i?86-*-* x86_64-*-* } } */ Index: gcc/testsuite/gcc.dg/tree-ssa/reassoc-35.c === --- gcc/testsuite/gcc.dg/tree-ssa/reassoc-35.c (revision 214241) +++ gcc/testsuite/gcc.dg/tree-ssa/reassoc-35.c (working copy) @@ -1,4 +1,4 @@ -/* { dg-do run { target { ! m68k*-*-* mmix*-*-* mep*-*-* bfin*-*-* v850*-*-* picochip*-*-* moxie*-*-* cris*-*-* m32c*-*-* fr30*-*-* mcore*-*-* powerpc*-*-* xtensa*-*-* hppa*-*-*} } } */ +/* { dg-do run { target { ! m68k*-*-* mmix*-*-* mep*-*-* bfin*-*-* v850*-*-* picochip*-*-* moxie*-*-* cris*-*-* m32c*-*-* fr30*-*-* mcore*-*-* powerpc*-*-* xtensa*-*-* hppa*-*-* nios2*-*-*} } } */ /* { dg-options -O2 -fno-inline -fdump-tree-reassoc1-details } */ /* { dg-additional-options -mbranch-cost=2 { target mips*-*-* avr-*-* s390*-*-* i?86-*-* x86_64-*-* } } */ Index: gcc/testsuite/gcc.dg/tree-ssa/reassoc-36.c === --- gcc/testsuite/gcc.dg/tree-ssa/reassoc-36.c (revision 214241) +++ gcc/testsuite/gcc.dg/tree-ssa/reassoc-36.c (working copy) @@ -1,4 +1,4 @@ -/* { dg-do run { target { ! m68k*-*-* mmix*-*-* mep*-*-* bfin*-*-* v850*-*-* picochip*-*-* moxie*-*-* cris*-*-* m32c*-*-* fr30*-*-* mcore*-*-* powerpc*-*-* xtensa*-*-* hppa*-*-*} } } */ +/* { dg-do run { target { ! m68k*-*-* mmix*-*-* mep*-*-* bfin*-*-* v850*-*-* picochip*-*-* moxie*-*-* cris*-*-* m32c*-*-* fr30*-*-* mcore*-*-* powerpc*-*-* xtensa*-*-* hppa*-*-* nios2*-*-*} } } */ /* { dg-options -O2 -fno-inline -fdump-tree-reassoc1-details } */ /* { dg-additional-options -mbranch-cost=2 { target mips*-*-* avr-*-* s390*-*-* i?86-*-* x86_64-*-* } } */ Index: gcc/testsuite/gcc.dg/tree-ssa/interposition.c === --- gcc/testsuite/gcc.dg/tree-ssa/interposition.c (revision 214241) +++ gcc/testsuite/gcc.dg/tree-ssa/interposition.c (working copy) @@ -1,4 +1,5 @@ /* { dg-do compile } */ +/* { dg-require-effective-target fpic } */ /* { dg-options -O1 -fno-semantic-interposition -fdump-tree-optimized -fPIC } */ int t(void) { Index: gcc/testsuite/gcc.dg/lto/pr61526_0.c
[PINGv4][PATCHv3] Fix vector tests on ARM platforms with disabled unaligned accesses
On 08/14/2014 05:40 PM, Marat Zakirov wrote: On 08/07/2014 12:52 PM, Marat Zakirov wrote: On 07/31/2014 04:08 PM, Marat Zakirov wrote: On 07/24/2014 07:40 PM, Marat Zakirov wrote: On 07/24/2014 04:27 PM, Marat Zakirov wrote: On 07/23/2014 06:23 PM, Marat Zakirov wrote: Hi there! I made a patch which fixes regressions on ARM platforms with disabled unaligned accesses. The problem is that 'arm_vect_no_misalign' predicate do not check 'unaligned_access' global variable to determine whether unaligned access to vector are allowed. This leads to spurious vect.exp test fails when GCC is configured --with-specs=%{!munaligned-access:-mno-unaligned-access}. Attached patch fixes ARM predicate and several tests to correctly handle the issue. The following targets were reg. tested for multiple targets (ARM, Thumb-1, Thumb-2, x86, x86_64) with and without -mno-unaligned-access. Analysis showed patch affects only vect.exp tests so only vect.exp was tested. For x86, x86_64, ARM without -mno-unaligned-access, Thumb-2 without -mno-unaligned-access and Thumb-1 no regressions occured. For ARM/Thumb2 with -mno-unaligned-access patch fixed most of failures but triggered some problems (see attached log) for current vect.exp tests: 1) https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61887 2) Some XPASS'es due to unexpected loop versioning (e.g. gcc.dg/vect/pr33804.c). 3) After predicate fix some passing tests which require unaligned vector support become NA (this was expected). Here is new version of patch and regression log. On the current trunk results are slightly different due to patches for Richard Biener (no UNRESOLVED fails) but some PASS-XPASS regressions still remain (see attachment): PASS-XPASS: gcc.dg/vect/if-cvt-stores-vect-ifcvt-18.c scan-tree-dump-times vect vectorized 1 loops 1 PASS-XPASS: gcc.dg/vect/pr33804.c -flto -ffat-lto-objects scan-tree-dump-times vect vectorized 1 loops 1 etc. These XPASS'es are due to code versioning: current GCC creates 2 versions of loop: aligned and misaligned. It's look like they are slightly out of date at lest for ARM. On 07/24/2014 06:50 PM, Ramana Radhakrishnan wrote: This is redundant. - || (defined(__ARMEL__) \ + || (defined(__ARM_FEATURE_UNALIGNED) \ +defined(__ARMEL__) \ (!defined(__thumb__) || defined(__thumb2__))) As is this line. I think you can restrict the check to defined(__ARM_FEATURE_UNALIGNED) defined(__ARMEL__) __ARM_FEATURE_UNALIGNED should tell you whether unaligned access is allowed or not, therefore you should no longer require any specific architectural checks. #error FOO #endif I'm not sure about the original intent of the tests right now. Ramana Thank you Ramana! --Marat gcc/testsuite/ChangeLog: 2014-07-23 Marat Zakirov m.zaki...@samsung.com * gcc.dg/vect/vect-109.c: Skip predicate added. * gcc.dg/vect/vect-93.c: Test check fixed. * gcc.dg/vect/bb-slp-10.c: Likewise. * lib/target-supports.exp (check_effective_target_arm_vect_no_misalign): Check unaligned feature. diff --git a/gcc/testsuite/gcc.dg/vect/bb-slp-10.c b/gcc/testsuite/gcc.dg/vect/bb-slp-10.c index a1850ed..0090a4b 100644 --- a/gcc/testsuite/gcc.dg/vect/bb-slp-10.c +++ b/gcc/testsuite/gcc.dg/vect/bb-slp-10.c @@ -49,7 +49,7 @@ int main (void) return 0; } -/* { dg-final { scan-tree-dump-times unsupported alignment in basic block. 1 slp2 { xfail vect_element_align } } } */ +/* { dg-final { scan-tree-dump unsupported alignment in basic block. 1 slp2 { xfail vect_element_align } } } */ /* { dg-final { scan-tree-dump-times basic block vectorized 1 slp2 { target vect_element_align } } } */ /* { dg-final { cleanup-tree-dump slp2 } } */ diff --git a/gcc/testsuite/gcc.dg/vect/vect-109.c b/gcc/testsuite/gcc.dg/vect/vect-109.c index 854c970..c671175 100644 --- a/gcc/testsuite/gcc.dg/vect/vect-109.c +++ b/gcc/testsuite/gcc.dg/vect/vect-109.c @@ -1,3 +1,4 @@ +/* { dg-skip-if { vect_no_align } } */ /* { dg-require-effective-target vect_int } */ #include stdarg.h diff --git a/gcc/testsuite/gcc.dg/vect/vect-93.c b/gcc/testsuite/gcc.dg/vect/vect-93.c index 65403eb..1065a6e 100644 --- a/gcc/testsuite/gcc.dg/vect/vect-93.c +++ b/gcc/testsuite/gcc.dg/vect/vect-93.c @@ -79,7 +79,7 @@ int main (void) /* { dg-final { scan-tree-dump-times vectorized 2 loops 1 vect { target vect_no_align } } } */ /* in main: */ -/* { dg-final { scan-tree-dump-times vectorized 0 loops 1 vect { target vect_no_align } } } */ +/* { dg-final { scan-tree-dump-times vectorized 1 loops 1 vect { target vect_no_align } } } */ /* { dg-final { scan-tree-dump-times Vectorizing an unaligned access 1 vect { xfail { vect_no_align } } } } */ /* { dg-final { cleanup-tree-dump vect } } */ diff --git a/gcc/testsuite/lib/target-supports.exp b/gcc/testsuite/lib/target-supports.exp index db65ebe..35076d2 100644 --- a/gcc/testsuite/lib/target-supports.exp +++
Re: [PATCH 046/236] delete_related_insns returns an rtx_insn
On Wed, 2014-08-13 at 12:10 -0600, Jeff Law wrote: On 08/06/14 11:20, David Malcolm wrote: gcc/ * rtl.h (delete_related_insns): Strengthen return type from rtx to rtx_insn *. * jump.c (delete_related_insns): Likewise, also for locals next and prev. OK. Thanks; committed to trunk as r214275.
Re: [PATCH 047/236] PHASE 2: Per-file commits in main source directory
On Wed, 2014-08-13 at 12:10 -0600, Jeff Law wrote: On 08/06/14 11:20, David Malcolm wrote: This commit is a placeholder for me when rebasing, to help organize the patch kit. / * rtx-classes-status.txt: Update OK. Thanks; committed to trunk r214276.
Re: [C++ PATCH] Fix -Wlogical-not-parentheses (PR c++/62199)
On Wed, Aug 20, 2014 at 05:02:24PM -0400, Jason Merrill wrote: On 08/20/2014 04:02 PM, Marek Polacek wrote: On Wed, Aug 20, 2014 at 02:36:21PM -0400, Jason Merrill wrote: Could we set current.lhs_type to TRUTH_NOT_EXPR when we see a ! rather than track nots in two separate local variables? Good point. So like the following? I was thinking to do away with parenthesized_not_lhs_warn as well, so instead of bool parenthesized_not_lhs_warn = cp_lexer_next_token_is (parser-lexer, CPP_NOT); /* Parse the first expression. */ current.lhs = cp_parser_cast_expression (parser, /*address_p=*/false, cast_p, decltype_p, pidk); current.lhs_type = ERROR_MARK; we would set current.lhs_type to TRUTH_NOT_EXPR here if the first token is CPP_NOT, and /* Extract another operand. It may be the RHS of this expression or the LHS of a new, higher priority expression. */ rhs = cp_parser_simple_cast_expression (parser); rhs_type = ERROR_MARK; here we would do the same thing for rhs_type. cp_lexer_consume_token (parser-lexer); + if (cp_lexer_next_token_is (parser-lexer, CPP_NOT)) +current.lhs_type = TRUTH_NOT_EXPR; ... current.lhs = rhs; + parenthesized_not_lhs_warn = current.lhs_type == TRUTH_NOT_EXPR; current.lhs_type = rhs_type; and then you don't need any changes in these places. if (warn_logical_not_paren parenthesized_not_lhs_warn) And here you check current.lhs_type. Oh, I see now. Thanks, this is much better. This change has an interesting effect on Wparentheses-25.C test; it made it XPASS - I believe we now generate the correct warnings, the same as C FE. So I got rid of all those xfails in that test and it now passes cleanly. Bootstrapped/regtested on x86_64-linux, ok for trunk? 2014-08-21 Marek Polacek pola...@redhat.com PR c++/62199 * parser.c (cp_parser_binary_expression): Check each LHS if it's preceded with logical not. Handle logical not of constants. * c-c++-common/pr62199.c: New test. * g++.dg/warn/Wparentheses-25.C: Remove XFAILs. diff --git gcc/cp/parser.c gcc/cp/parser.c index 9053bfa..ddcbab9 100644 --- gcc/cp/parser.c +++ gcc/cp/parser.c @@ -8020,13 +8020,12 @@ cp_parser_binary_expression (cp_parser* parser, bool cast_p, enum tree_code rhs_type; enum cp_parser_prec new_prec, lookahead_prec; tree overload; - bool parenthesized_not_lhs_warn -= cp_lexer_next_token_is (parser-lexer, CPP_NOT); /* Parse the first expression. */ + current.lhs_type = cp_lexer_next_token_is (parser-lexer, CPP_NOT) +? TRUTH_NOT_EXPR : ERROR_MARK; current.lhs = cp_parser_cast_expression (parser, /*address_p=*/false, cast_p, decltype_p, pidk); - current.lhs_type = ERROR_MARK; current.prec = prec; if (cp_parser_error_occurred (parser)) @@ -8081,8 +8080,9 @@ cp_parser_binary_expression (cp_parser* parser, bool cast_p, /* Extract another operand. It may be the RHS of this expression or the LHS of a new, higher priority expression. */ + rhs_type = cp_lexer_next_token_is (parser-lexer, CPP_NOT) +? TRUTH_NOT_EXPR : ERROR_MARK; rhs = cp_parser_simple_cast_expression (parser); - rhs_type = ERROR_MARK; /* Get another operator token. Look up its precedence to avoid building a useless (immediately popped) stack entry for common @@ -8125,9 +8125,18 @@ cp_parser_binary_expression (cp_parser* parser, bool cast_p, c_inhibit_evaluation_warnings -= current.lhs == truthvalue_true_node; if (warn_logical_not_paren - parenthesized_not_lhs_warn) - warn_logical_not_parentheses (current.loc, current.tree_type, - TREE_OPERAND (current.lhs, 0), rhs); + current.lhs_type == TRUTH_NOT_EXPR) + { + tree lhs; + /* If the LHS was !CST, we have true/false now. Convert it +to integer type, otherwise we wouldn't warn. */ + if (TREE_CODE (current.lhs) == INTEGER_CST) + lhs = convert (integer_type_node, current.lhs); + else + lhs = TREE_OPERAND (current.lhs, 0); + warn_logical_not_parentheses (current.loc, current.tree_type, + lhs, rhs); + } overload = NULL; /* ??? Currently we pass lhs_type == ERROR_MARK and rhs_type == diff --git gcc/testsuite/c-c++-common/pr62199.c gcc/testsuite/c-c++-common/pr62199.c index e69de29..51078c8 100644 --- gcc/testsuite/c-c++-common/pr62199.c +++ gcc/testsuite/c-c++-common/pr62199.c @@ -0,0 +1,22 @@ +/* PR c++/62199 */ +/* { dg-do compile } */ +/* { dg-options -Wlogical-not-parentheses } */ + +int r; +void +foo (int a) +{ + r = a 0 || !a = 2; /* { dg-warning 19:logical not is only applied to the left hand side of comparison }
[patch] propagate INSTALL Makefile variables down from gcc/
Hello, Experiments with custom install programs exposed that the INSTALL series of Makefile variables aren't propagated down from the gcc subdir. This patch fixes this. Checked that it addressed the unexpected behavior we were observing + bootstrapped regtested on x86_64-linux-gnu. OK to commit ? Thanks in advance for your feedback, With Kind Regards, Olivier 2014-08-21 Nicolas Roche ro...@adacore.com * Makefile.in (FLAGS_TO_PASS): Propagate INSTALL, INSTALL_DATA, INSTALL_SCRIPT and INSTALL_PROGRAM as well. [see attached file: propagate-install.diff] propagate-install.diff Description: Binary data
[PATCH] Fix condition in is_aligning_offset (PR c/61271)
This is one of the issues that -Wlogical-not-parentheses detected. Interestingly, this code has been added in 2002 (!). I believe the logical not there should be just removed; the comment above it says /* We must now have a BIT_AND_EXPR with a constant that is one less than power of 2 and which is larger than BIGGEST_ALIGNMENT. */ so if the constant is not one less than power of 2 (exact_log2 returns -1), we should bail out. Bootstrapped/regtested on x86_64-linux, ok for trunk? Should I backport this to 4.9/4.8 after a while? 2014-08-21 Marek Polacek pola...@redhat.com PR c/61271 * expr.c (is_aligning_offset): Remove logical not. diff --git gcc/expr.c gcc/expr.c index 920d47b..05d81dc 100644 --- gcc/expr.c +++ gcc/expr.c @@ -10721,7 +10721,7 @@ is_aligning_offset (const_tree offset, const_tree exp) || !tree_fits_uhwi_p (TREE_OPERAND (offset, 1)) || compare_tree_int (TREE_OPERAND (offset, 1), BIGGEST_ALIGNMENT / BITS_PER_UNIT) = 0 - || !exact_log2 (tree_to_uhwi (TREE_OPERAND (offset, 1)) + 1) 0) + || exact_log2 (tree_to_uhwi (TREE_OPERAND (offset, 1)) + 1) 0) return 0; /* Look at the first operand of BIT_AND_EXPR and strip any conversion. Marek
Re: [PATCH] gcc/gcc.c: XNEWVEC enough space for 'saved_suffix' using
On Aug 18, 2014, at 3:06 AM, Chen Gang gang.chen.5...@gmail.com wrote: - one for original latest gcc source code (master for 20140816). - one for my modification based on the original latest gcc source code. - they passed building, but for make check, they reported same issues: So, I see no evidence that you ran contrib/compare_tests yet. Did you? If so, what was the output? For only sending my patch, may I bypass them, So, the output of the make check is uninteresting, only the regressions identified by compare_tests are interesting. If none, then you’re set wrt the current patch. and then I shall try to analyze them one by one in another time, next? You can, if you want. For example, there is a PR on the darwin issue that has state in it. Roughly binds local is slightly wrong and need fixing.
Re: [PATCH] Remove CALLER_SAVE_PROFITABLE since nobody use it now
On 19/08/14 15:24, Kito Cheng wrote: Hi Richard: Hmm, I'm not sure about this. It might not be used at present, but on: AArch64, with more call-clobbered registers than call-saved registers, I would expect this ought to be a win. The fact that it isn't on today may say more about the way it works than the concept that it's the wrong thing to do in principle. In my view, calculate cost/profit should be done by register allocator instead of a target hook/marco is more reasonable since register allocator can have more globe view to it, and IRA do it now. And as Joseph say, no code calling it, so I think it's time to remove it. thanks for your comment :) Objection withdrawn. R. Hi Joseph: Nothing uses it - not just no targets defining it, but no code calling it. If you have a use in future for a target hook doing something suggested by the name of this macro, by all means add one - but there's no point at all in keeping the macro until then. (It should of course be poisoned in system.h when removing it, as standard when removing target macros.) Thanks, I add poison for CALLER_SAVE_PROFITABLE to system.h, updated patch attached. thanks for your review :) ChangLog 2014-08-19 Kito Cheng k...@0xlab.org * system.h (CALLER_SAVE_PROFITABLE): Poison. * doc/tm.texi.in (CALLER_SAVE_PROFITABLE): Remove. * gcc/doc/tm.texi: Regenerate. * gcc/regs.h (CALLER_SAVE_PROFITABLE): Remove. On Tue, Aug 19, 2014 at 4:26 AM, Joseph S. Myers jos...@codesourcery.com wrote: On Mon, 18 Aug 2014, Richard Earnshaw wrote: Hmm, I'm not sure about this. It might not be used at present, but on AArch64, with more call-clobbered registers than call-saved registers, I would expect this ought to be a win. The fact that it isn't on today may say more about the way it works than the concept that it's the wrong thing to do in principle. Nothing uses it - not just no targets defining it, but no code calling it. If you have a use in future for a target hook doing something suggested by the name of this macro, by all means add one - but there's no point at all in keeping the macro until then. (It should of course be poisoned in system.h when removing it, as standard when removing target macros.) -- Joseph S. Myers jos...@codesourcery.com 0001-Remove-CALLER_SAVE_PROFITABLE-since-nobody-use-it-no.patch From 6a99f77eb6785c8d471329bda4bc67885f35909a Mon Sep 17 00:00:00 2001 From: Kito Cheng k...@0xlab.org Date: Tue, 19 Aug 2014 22:10:47 +0800 Subject: [PATCH] Remove CALLER_SAVE_PROFITABLE since nobody use it now. --- gcc/doc/tm.texi| 10 -- gcc/doc/tm.texi.in | 10 -- gcc/regs.h | 8 gcc/system.h | 3 ++- 4 files changed, 2 insertions(+), 29 deletions(-) diff --git a/gcc/doc/tm.texi b/gcc/doc/tm.texi index 9dd8d68..4d6492b 100644 --- a/gcc/doc/tm.texi +++ b/gcc/doc/tm.texi @@ -4528,16 +4528,6 @@ If you enable it, GCC can save registers around function calls. This makes it possible to use call-clobbered registers to hold variables that must live across calls. -@defmac CALLER_SAVE_PROFITABLE (@var{refs}, @var{calls}) -A C expression to determine whether it is worthwhile to consider placing -a pseudo-register in a call-clobbered hard register and saving and -restoring it around each function call. The expression should be 1 when -this is worth doing, and 0 otherwise. - -If you don't define this macro, a default is used which is good on most -machines: @code{4 * @var{calls} @var{refs}}. -@end defmac - @defmac HARD_REGNO_CALLER_SAVE_MODE (@var{regno}, @var{nregs}) A C expression specifying which mode is required for saving @var{nregs} of a pseudo-register in call-clobbered hard register @var{regno}. If diff --git a/gcc/doc/tm.texi.in b/gcc/doc/tm.texi.in index dd72b98..7a63353 100644 --- a/gcc/doc/tm.texi.in +++ b/gcc/doc/tm.texi.in @@ -3683,16 +3683,6 @@ If you enable it, GCC can save registers around function calls. This makes it possible to use call-clobbered registers to hold variables that must live across calls. -@defmac CALLER_SAVE_PROFITABLE (@var{refs}, @var{calls}) -A C expression to determine whether it is worthwhile to consider placing -a pseudo-register in a call-clobbered hard register and saving and -restoring it around each function call. The expression should be 1 when -this is worth doing, and 0 otherwise. - -If you don't define this macro, a default is used which is good on most -machines: @code{4 * @var{calls} @var{refs}}. -@end defmac - @defmac HARD_REGNO_CALLER_SAVE_MODE (@var{regno}, @var{nregs}) A C expression specifying which mode is required for saving @var{nregs} of a pseudo-register in call-clobbered hard register @var{regno}. If diff --git a/gcc/regs.h b/gcc/regs.h index be81db4..36e803d 100644 --- a/gcc/regs.h +++ b/gcc/regs.h @@ -214,14 +214,6 @@ extern short *reg_renumber; extern int
Re: [PATCH 039/236] create_insn_rtx_from_pattern and create_copy_of_insn_rtx return rtx_insn
On Aug 20, 2014, at 6:03 PM, David Malcolm dmalc...@redhat.com wrote: OK. Thanks; committed to trunk as r214253. So, unless there is a consumer of this information, in general, you don’t need to let us know you checked things in. We assume that by default. Cases were it is useful, you check in someone else’s patch and you want them to know the patch is now in, or say, you want someone working with you to know the work went in, or someone depending upon a patch, you want them to know that they are now unblocked, say for testing or additional work.
Re: [patch, nios2] testsuite cleanup
On Aug 21, 2014, at 8:00 AM, Sandra Loosemore san...@codesourcery.com wrote: tests that assume some non-default branch costs in the back end Thanks. One comment, could you put in /* non default branch cost */ above the three where that is true. Even better would be to refactor those into something for target supports. Though, really the best course of action would be for someone to list the targets where the non default condition holds true, and make those pay the price, not the universal complement.
Re: [PATCH] Remove CALLER_SAVE_PROFITABLE since nobody use it now
On Thu, 21 Aug 2014, Richard Earnshaw wrote: On 19/08/14 15:24, Kito Cheng wrote: Hi Richard: Hmm, I'm not sure about this. It might not be used at present, but on: AArch64, with more call-clobbered registers than call-saved registers, I would expect this ought to be a win. The fact that it isn't on today may say more about the way it works than the concept that it's the wrong thing to do in principle. In my view, calculate cost/profit should be done by register allocator instead of a target hook/marco is more reasonable since register allocator can have more globe view to it, and IRA do it now. And as Joseph say, no code calling it, so I think it's time to remove it. thanks for your comment :) Objection withdrawn. Thanks. The patch is OK. -- Joseph S. Myers jos...@codesourcery.com
Re: TAGs for variables created through common.opt
Well, whadayaknow... Tom Tromey pointed me at --regex which we can use to add patterns for not only the .opt files, but for a bunch of other files/languages we define in GCC. The following patch adds support for common.opt, rtl.def, tree.def, and gimple.def. Now you can use your editor to tag things like GIMPLE_NOP, and PLUS_EXPR, which means I'll get lost less often. Kinda neat, IMO. Tested by inspecting TAGS.sub manually, as well as searching for random stuff with both vi and emacs. OK for mainline? * Makefile.in (TAGS): Handle constructs in common.opt, rtl.def, tree.def, and gimple.def diff --git a/gcc/Makefile.in b/gcc/Makefile.in index 1b3820b..f761153 100644 --- a/gcc/Makefile.in +++ b/gcc/Makefile.in @@ -3799,7 +3799,10 @@ TAGS: lang.tags incs=$$incs --include $$dir/TAGS.sub; \ fi; \ done; \ - etags -o TAGS.sub c-family/*.h c-family/*.c *.h *.c *.cc; \ + etags -o TAGS.sub c-family/*.h c-family/*.c *.h *.c *.cc \ + --language=none --regex=/\(char\|unsigned int\|int\|bool\|void\|HOST_WIDE_INT\|enum [A-Za-z_0-9]+\) [*]?\([A-Za-z_0-9]+\)/\2/ common.opt\ + --language=none --regex=/\(DEF_RTL_EXPR\|DEFTREECODE\|DEFGSCODE\).*(\([A-Za-z_0-9]+\)/\2/ rtl.def tree.def gimple.def \ + ; \ etags --include TAGS.sub $$incs) # -
Re: [patch, nios2] testsuite cleanup
On 08/21/2014 11:36 AM, Mike Stump wrote: On Aug 21, 2014, at 8:00 AM, Sandra Loosemore san...@codesourcery.com wrote: tests that assume some non-default branch costs in the back end Thanks. One comment, could you put in /* non default branch cost */ above the three where that is true. The three what? :-S Even better would be to refactor those into something for target supports. Though, really the best course of action would be for someone to list the targets where the non default condition holds true, and make those pay the price, not the universal complement. I definitely agree about this. Perhaps the tree-ssa maintainers can take care of this, or at least clarify what target(s) this is intended to work on rather than having to experimentally determine which ones don't implement these optimizations, or propose some way to test for the support rather than having to list targets explicitly. -Sandra
[PATCH] Fix condition in ira-color.c and lra-spills.c (PR c/61271)
Both ira-color.c and lra-spills.c contain this code, where -Wlogical-not-parentheses would warn: !FRAME_GROWS_DOWNWARD == STACK_GROWS_DOWNWARD I think the condition is semantically right, so I just tweaked it into a more common way with !=, so we don't warn on this anymore. Bootstrapped/regtested on x86_64-linux, ok for trunk? 2014-08-21 Marek Polacek pola...@redhat.com PR c/61271 * ira-color.c (coalesced_pseudo_reg_slot_compare): Fix condition. * lra-spills.c (pseudo_reg_slot_compare): Fix condition. diff --git gcc/ira-color.c gcc/ira-color.c index 36c3c87..14958ac 100644 --- gcc/ira-color.c +++ gcc/ira-color.c @@ -3850,7 +3850,7 @@ coalesced_pseudo_reg_slot_compare (const void *v1p, const void *v2p) slot_num2 = -ALLOCNO_HARD_REGNO (a2); if ((diff = slot_num1 - slot_num2) != 0) return (frame_pointer_needed - || !FRAME_GROWS_DOWNWARD == STACK_GROWS_DOWNWARD ? diff : -diff); + || (FRAME_GROWS_DOWNWARD != STACK_GROWS_DOWNWARD) ? diff : -diff); total_size1 = MAX (PSEUDO_REGNO_BYTES (regno1), regno_max_ref_width[regno1]); total_size2 = MAX (PSEUDO_REGNO_BYTES (regno2), diff --git gcc/lra-spills.c gcc/lra-spills.c index 50f63fc..0d14685 100644 --- gcc/lra-spills.c +++ gcc/lra-spills.c @@ -237,7 +237,7 @@ pseudo_reg_slot_compare (const void *v1p, const void *v2p) slot_num2 = pseudo_slots[regno2].slot_num; if ((diff = slot_num1 - slot_num2) != 0) return (frame_pointer_needed - || !FRAME_GROWS_DOWNWARD == STACK_GROWS_DOWNWARD ? diff : -diff); + || (FRAME_GROWS_DOWNWARD != STACK_GROWS_DOWNWARD) ? diff : -diff); total_size1 = GET_MODE_SIZE (lra_reg_info[regno1].biggest_mode); total_size2 = GET_MODE_SIZE (lra_reg_info[regno2].biggest_mode); if ((diff = total_size2 - total_size1) != 0) Marek
[PATCH], Fix error in powerpc bootstrap
There was some code in the rs6000.c that new warnings prevented the powerpc compiler from bootstraping. I fixed it under the 'obvious' rule, after doing a bootstrap with the change: 2014-08-21 Michael Meissner meiss...@linux.vnet.ibm.com * config/rs6000/rs6000.c (print_operand, 'y' case): Fix code that generated a warning and prevented bootstrapping the compiler. Index: gcc/config/rs6000/rs6000.c === --- gcc/config/rs6000/rs6000.c (revision 214277) +++ gcc/config/rs6000/rs6000.c (working copy) @@ -18601,7 +18601,7 @@ print_operand (FILE *file, rtx x, int co fprintf (file, 0,%s, reg_names[REGNO (tmp)]); else { - if (!GET_CODE (tmp) == PLUS + if (GET_CODE (tmp) != PLUS || !REG_P (XEXP (tmp, 0)) || !REG_P (XEXP (tmp, 1))) { -- Michael Meissner, IBM IBM, M/S 2506R, 550 King Street, Littleton, MA 01460-6245, USA email: meiss...@linux.vnet.ibm.com, phone: +1 (978) 899-4797
Re: [C++ PATCH] Fix -Wlogical-not-parentheses (PR c++/62199)
On 08/21/2014 11:41 AM, Marek Polacek wrote: + current.lhs_type = cp_lexer_next_token_is (parser-lexer, CPP_NOT) +? TRUTH_NOT_EXPR : ERROR_MARK; ... + rhs_type = cp_lexer_next_token_is (parser-lexer, CPP_NOT) +? TRUTH_NOT_EXPR : ERROR_MARK; Again, this indentation needs parens to survive Emacs. + /* If the LHS was !CST, we have true/false now. Convert it +to integer type, otherwise we wouldn't warn. */ + if (TREE_CODE (current.lhs) == INTEGER_CST) + lhs = convert (integer_type_node, current.lhs); Hmm, why are we checking for BOOLEAN_TYPE on the lhs, again? It seems to me that bool b; if (!b == 1) is also probably an error. Jason
[patch, fortran, committed] Fix PR 62214
Hello world, the attached patch fixes the PR. Committed to trunk as obvious after regression-testing. Will commit to 4.9 and 4.8 after regression testing there. Any idea why rather so many rather old bugs are coming up all of a sudden? Did a major distribution just upgrade its gcc version? Regards Thomas 2014-08-21 Thomas Koenig tkoe...@gcc.gnu.org PR fortran/62214 * frontend-passes.c (optimize_binop_array_assignment): Do not try to optimize the array assignment for string concatenation. 2014-08-21 Thomas Koenig tkoe...@gcc.gnu.org PR fortran/62214 * gfortran.dg/array_assignment_5.f90: New test. Index: frontend-passes.c === --- frontend-passes.c (Revision 214061) +++ frontend-passes.c (Arbeitskopie) @@ -903,6 +903,10 @@ optimize_binop_array_assignment (gfc_code *c, gfc_ return true; break; + case INTRINSIC_CONCAT: + /* Do not do string concatenations. */ + break; + default: /* Binary operators. */ if (optimize_binop_array_assignment (c, e-value.op.op1, true)) ! { dg-do run } ! { dg-options -ffrontend-optimize } ! PR 62214 - this used to give the wrong result. ! Original test case by Oliver Fuhrer PROGRAM test IMPLICIT NONE CHARACTER(LEN=20) :: fullNames(2) CHARACTER(LEN=255) :: pathName CHARACTER(LEN=5):: fileNames(2) pathName = /dir1/dir2/ fileNames = (/ file1, file2 /) fullNames = SPREAD(TRIM(pathName),1,2) // fileNames if (fullNames(1) /= '/dir1/dir2/file1' .or. fullnames(2) /= '/dir1/dir2/file2') call abort END PROGRAM test
[PATCH, rs6000] PR 62195, Fix wi constraint
(I'm not sure if an earlier patch got mailed out, I'm sorry if there are duplicate postings). I had a thinko in my patch on August 11th, in that I allowed the wi constraint to be FLOAT_REGS on a non-VSX system, but I had a pattern in movdi that generated a VSX instruction. I have tightened wi, so that it applies only when VSX is used. In addition, I had used wi for lfiwax/lfiwzx and in the case issued an ISA 2.07 (power8) instruction. I have changed these cases to use the wj constraint (constraint for register class under ISA 2.07 for doing move direct of DImode). I decided that it wasn't worth adding a new constraint for these cases, and if the user does -mcpu=power8 -mno-move-direct, it will still generate the lfiwax/lfiwzx instructions which target the traditional floating point registers. I have done bootstraps with this compiler and it had no regressions. Is the patch ok to check in? 2014-08-21 Michael Meissner meiss...@linux.vnet.ibm.com PR target/62195 * doc/md.texi (Machine Constraints): Update PowerPC wi constraint documentation to state it is only for VSX operations. * config/rs6000/rs6000.c (rs6000_init_hard_regno_mode_ok): Make wi constraint only active if VSX. * config/rs6000/rs6000.md (lfiwax): Use wj constraint instead of wi cosntraint for ISA 2.07 lxsiwax/lxsiwzx instructions. (lfiwzx): Likewise. -- Michael Meissner, IBM IBM, M/S 2506R, 550 King Street, Littleton, MA 01460-6245, USA email: meiss...@linux.vnet.ibm.com, phone: +1 (978) 899-4797 Index: gcc/doc/md.texi === --- gcc/doc/md.texi (revision 214280) +++ gcc/doc/md.texi (working copy) @@ -2136,7 +2136,7 @@ If @option{-mmfpgpr} was used, a floatin Floating point register if direct moves are available, or NO_REGS. @item wi -FP or VSX register to hold 64-bit integers or NO_REGS. +FP or VSX register to hold 64-bit integers for VSX insns or NO_REGS. @item wj FP or VSX register to hold 64-bit integers for direct moves or NO_REGS. Index: gcc/config/rs6000/constraints.md === --- gcc/config/rs6000/constraints.md(revision 214280) +++ gcc/config/rs6000/constraints.md(working copy) @@ -74,7 +74,7 @@ (define_register_constraint wh rs6000 ;; At present, DImode is not allowed in the Altivec registers. If in the ;; future it is allowed, wi/wj can be set to VSX_REGS instead of FLOAT_REGS. (define_register_constraint wi rs6000_constraints[RS6000_CONSTRAINT_wi] - FP or VSX register to hold 64-bit integers or NO_REGS.) + FP or VSX register to hold 64-bit integers for VSX insns or NO_REGS.) (define_register_constraint wj rs6000_constraints[RS6000_CONSTRAINT_wj] FP or VSX register to hold 64-bit integers for direct moves or NO_REGS.) Index: gcc/config/rs6000/rs6000.c === --- gcc/config/rs6000/rs6000.c (revision 214280) +++ gcc/config/rs6000/rs6000.c (working copy) @@ -2643,7 +2643,7 @@ rs6000_init_hard_regno_mode_ok (bool glo wf - Preferred register class for V4SFmode. wg - Float register for power6x move insns. wh - FP register for direct move instructions. - wi - FP or VSX register to hold 64-bit integers. + wi - FP or VSX register to hold 64-bit integers for VSX insns. wj - FP or VSX register to hold 64-bit integers for direct moves. wk - FP or VSX register to hold 64-bit doubles for direct moves. wl - Float register if we can do 32-bit signed int loads. @@ -2663,16 +2663,14 @@ rs6000_init_hard_regno_mode_ok (bool glo rs6000_constraints[RS6000_CONSTRAINT_f] = FLOAT_REGS; /* SFmode */ if (TARGET_HARD_FLOAT TARGET_FPRS TARGET_DOUBLE_FLOAT) -{ - rs6000_constraints[RS6000_CONSTRAINT_d] = FLOAT_REGS; /* DFmode */ - rs6000_constraints[RS6000_CONSTRAINT_wi] = FLOAT_REGS; /* DImode */ -} +rs6000_constraints[RS6000_CONSTRAINT_d] = FLOAT_REGS; /* DFmode */ if (TARGET_VSX) { rs6000_constraints[RS6000_CONSTRAINT_wa] = VSX_REGS; rs6000_constraints[RS6000_CONSTRAINT_wd] = VSX_REGS; /* V2DFmode */ rs6000_constraints[RS6000_CONSTRAINT_wf] = VSX_REGS; /* V4SFmode */ + rs6000_constraints[RS6000_CONSTRAINT_wi] = FLOAT_REGS; /* DImode */ if (TARGET_VSX_TIMODE) rs6000_constraints[RS6000_CONSTRAINT_wt] = VSX_REGS;/* TImode */ Index: gcc/config/rs6000/rs6000.md === --- gcc/config/rs6000/rs6000.md (revision 214280) +++ gcc/config/rs6000/rs6000.md (working copy) @@ -5614,7 +5614,7 @@ (define_insn *fselsfdf4 ; We don't define lfiwax/lfiwzx with the normal definition, because we ; don't want to support putting SImode in FPR registers. (define_insn lfiwax - [(set (match_operand:DI 0 gpc_reg_operand =d,wi,!wj) + [(set
Re: [PATCH] Fix condition in ira-color.c and lra-spills.c (PR c/61271)
On 08/21/2014 02:01 PM, Marek Polacek wrote: Both ira-color.c and lra-spills.c contain this code, where -Wlogical-not-parentheses would warn: !FRAME_GROWS_DOWNWARD == STACK_GROWS_DOWNWARD I think the condition is semantically right, so I just tweaked it into a more common way with !=, so we don't warn on this anymore. Bootstrapped/regtested on x86_64-linux, ok for trunk? 2014-08-21 Marek Polacek pola...@redhat.com PR c/61271 * ira-color.c (coalesced_pseudo_reg_slot_compare): Fix condition. * lra-spills.c (pseudo_reg_slot_compare): Fix condition. diff --git gcc/ira-color.c gcc/ira-color.c index 36c3c87..14958ac 100644 --- gcc/ira-color.c +++ gcc/ira-color.c @@ -3850,7 +3850,7 @@ coalesced_pseudo_reg_slot_compare (const void *v1p, const void *v2p) slot_num2 = -ALLOCNO_HARD_REGNO (a2); if ((diff = slot_num1 - slot_num2) != 0) return (frame_pointer_needed - || !FRAME_GROWS_DOWNWARD == STACK_GROWS_DOWNWARD ? diff : -diff); + || (FRAME_GROWS_DOWNWARD != STACK_GROWS_DOWNWARD) ? diff : -diff); total_size1 = MAX (PSEUDO_REGNO_BYTES (regno1), regno_max_ref_width[regno1]); total_size2 = MAX (PSEUDO_REGNO_BYTES (regno2), diff --git gcc/lra-spills.c gcc/lra-spills.c index 50f63fc..0d14685 100644 --- gcc/lra-spills.c +++ gcc/lra-spills.c @@ -237,7 +237,7 @@ pseudo_reg_slot_compare (const void *v1p, const void *v2p) slot_num2 = pseudo_slots[regno2].slot_num; if ((diff = slot_num1 - slot_num2) != 0) return (frame_pointer_needed - || !FRAME_GROWS_DOWNWARD == STACK_GROWS_DOWNWARD ? diff : -diff); + || (FRAME_GROWS_DOWNWARD != STACK_GROWS_DOWNWARD) ? diff : -diff); total_size1 = GET_MODE_SIZE (lra_reg_info[regno1].biggest_mode); total_size2 = GET_MODE_SIZE (lra_reg_info[regno2].biggest_mode); if ((diff = total_size2 - total_size1) != 0) Sorry, Marek. I guess it is wrong. STACK_GROWS_DOWNWARD is only 0 or 1 in these files (it is achieved by redefinition of STACK_GROWS_DOWNWARD in the files). FAME_GROWS_DOWNWARD can be 0 or anything non-zero (even non-constant) as tm.texi contains @defmac FRAME_GROWS_DOWNWARD Define this macro to nonzero value if the addresses of local variable slots are at negative offsets from the frame pointer. @end defmac It works now as all machine description files use 1 as non-zero value. So the documentation should be changed but it is not wise, error prune, and right now the macro value can be non-constant during compilation time theoretically. So you need another way to transform the expressions to get rid of the warning. As I remember the code was actually taken from the old RA and not simple as it looks.
[PATCH c++/57709] Wshadow is too strict / has false positives
In GCC 4.8 I implemented: The option -Wshadow no longer warns if a declaration shadows a function declaration, unless the former declares a function or pointer to function, because this is a common and valid case in real-world code. This patch applies the same heuristic to member functions. Bootstrapped and regression tested on x86_64-linux-gnu. OK? gcc/cp/ChangeLog: 2014-08-21 Manuel López-Ibáñez m...@gcc.gnu.org PR c++/57709 * name-lookup.c (pushdecl_maybe_friend_1): Do not warn if a declaration shadows a function declaration, unless the former declares a function or pointer to function, because this is a common and valid case in real-world code. gcc/testsuite/ChangeLog: 2014-08-21 Manuel López-Ibáñez m...@gcc.gnu.org PR c++/57709 * g++.dg/Wshadow.C: New test. Index: gcc/testsuite/g++.dg/Wshadow.C === --- gcc/testsuite/g++.dg/Wshadow.C (revision 0) +++ gcc/testsuite/g++.dg/Wshadow.C (revision 0) @@ -0,0 +1,12 @@ +// { dg-do compile } +// { dg-options -Wshadow } +// PR c++/57709 +class C { + int both_var; // { dg-message declaration } + void var_and_method() {} + void m() { +int + both_var, // { dg-warning shadows } + var_and_method; + } +}; Index: gcc/cp/name-lookup.c === --- gcc/cp/name-lookup.c(revision 214229) +++ gcc/cp/name-lookup.c(working copy) @@ -1237,13 +1237,28 @@ pushdecl_maybe_friend_1 (tree x, bool is else member = NULL_TREE; if (member !TREE_STATIC (member)) { - /* Location of previous decl is not useful in this case. */ - warning (OPT_Wshadow, declaration of %qD shadows a member of 'this', - x); + if (BASELINK_P (member)) + member = BASELINK_FUNCTIONS (member); + gcc_assert(!really_overloaded_fn (member)); + member = OVL_CURRENT (member); + + /* Do not warn if a variable shadows a function, unless +the variable is a function or a pointer-to-function. */ + if (!(TREE_CODE (member) == FUNCTION_DECL +TREE_CODE (x) != FUNCTION_DECL +!FUNCTION_POINTER_TYPE_P (TREE_TYPE (x + { + if (warning_at (input_location, OPT_Wshadow, + declaration of %qD shadows a member of %qT, + x, current_nonlambda_class_type ()) + DECL_P(member)) + inform (DECL_SOURCE_LOCATION (member), + shadowed declaration is here); + } } else if (oldglobal != NULL_TREE (VAR_P (oldglobal) /* If the old decl is a type decl, only warn if the old decl is an explicit typedef or if both the
Re: __intN patch 3/5: main __int128 - __intN conversion.
On Wed, 13 Aug 2014, DJ Delorie wrote: + for (i = 0; i NUM_INT_N_ENTS; i ++) +{ + int_n_trees[i].signed_type = make_signed_type (int_n_data[i].bitsize); + int_n_trees[i].unsigned_type = make_unsigned_type (int_n_data[i].bitsize); + TYPE_SIZE (int_n_trees[i].signed_type) = bitsize_int (int_n_data[i].bitsize); + TYPE_SIZE (int_n_trees[i].unsigned_type) = bitsize_int (int_n_data[i].bitsize); + + if (int_n_data[i].bitsize LONG_LONG_TYPE_SIZE) + { + integer_types[itk_intN_0 + i * 2] = int_n_trees[i].signed_type; + integer_types[itk_unsigned_intN_0 + i * 2] = int_n_trees[i].unsigned_type; + } +} Why are types only entered in integer_types if wider than long long? +static bool +standard_type_bitsize (int bitsize) +{ + /* As a special exception, we always want __int128 enabled if possible. */ + if (bitsize == 128) +return false; + if (bitsize == CHAR_TYPE_SIZE + || bitsize == SHORT_TYPE_SIZE + || bitsize == INT_TYPE_SIZE + || bitsize == LONG_TYPE_SIZE + || (bitsize == LONG_LONG_TYPE_SIZE LONG_LONG_TYPE_SIZE GET_MODE_BITSIZE (TImode))) I don't think there should be a special case depending on the size of long long here. + /* This must happen after the backend has a chance to process + command line options, but before the parsers are + initialized. */ + for (i = 0; i NUM_INT_N_ENTS; i ++) + if (targetm.scalar_mode_supported_p (int_n_data[i].m) + ! standard_type_bitsize (int_n_data[i].bitsize) + int_n_data[i].bitsize = HOST_BITS_PER_WIDE_INT * 2) + int_n_enabled_p[i] = true; This HOST_BITS_PER_WIDE_INT * 2 check seems wrong. wide-int should allow integer types bigger than HOST_BITS_PER_WIDE_INT * 2 to be supported; it's the job of targetm.scalar_mode_supported_p to return the correct result (not claiming to support such types if the target doesn't have the required wide-int support or is missing anything else required), not for target-independent code to override it. I don't see any definitions of that hook that obviously get this wrong. mode_for_size (unsigned int size, enum mode_class mclass, int limit) { - enum machine_mode mode; + enum machine_mode mode = BLKmode; + int i; I don't see any need for this initialization to be added. bprecision -= MIN (precision + BITS_PER_UNIT_LOG + 1, MAX_FIXED_MODE_SIZE); += MIN (precision, MAX_FIXED_MODE_SIZE); This change doesn't make sense to me - if it is needed, I think it should be separated out, not included in this patch which should simply about providing generic __intN support in the cases where __int128 is currently supported. @@ -1157,13 +1165,13 @@ enum omp_clause_map_kind OMP_CLAUSE_MAP_ALLOC, OMP_CLAUSE_MAP_TO, OMP_CLAUSE_MAP_FROM, OMP_CLAUSE_MAP_TOFROM, /* The following kind is an internal only map kind, used for pointer based array sections. OMP_CLAUSE_SIZE for these is not the pointer size, - which is implicitly POINTER_SIZE / BITS_PER_UNIT, but the bias. */ + which is implicitly POINTER_SIZE_UNITS, but the bias. */ POINTER_SIZE_UNITS changes belong in another patch, not this one. /* ptr_type_node can't be used here since ptr_mode is only set when toplev calls backend_init which is not done with -E or pch. */ - psize = POINTER_SIZE / BITS_PER_UNIT; + psize = POINTER_SIZE_UNITS; Likewise. @@ -844,12 +846,47 @@ c_cpp_builtins (cpp_reader *pfile) builtin_define_type_max (__LONG_LONG_MAX__, long_long_integer_type_node); builtin_define_type_minmax (__WCHAR_MIN__, __WCHAR_MAX__, underlying_wchar_type_node); builtin_define_type_minmax (__WINT_MIN__, __WINT_MAX__, wint_type_node); builtin_define_type_max (__PTRDIFF_MAX__, ptrdiff_type_node); builtin_define_type_max (__SIZE_MAX__, size_type_node); + for (i = 0; i NUM_INT_N_ENTS; i ++) +if (int_n_enabled_p[i]) + { + char buf[35+20+20]; + char buf_min[15+20]; + char buf_max[15+20]; + + sprintf(buf_min, __INT%d_MIN__, int_n_data[i].bitsize); + sprintf(buf_max, __INT%d_MAX__, int_n_data[i].bitsize); All this block of code appears to be new rather than replacing any existing code doing something similar with __int128. As such, I think it's best considered separately from the main __intN support. Also note missing spaces before '(' in this code. Some of this may be needed for libstdc++, but not all. As far as I can tell, the existing __glibcxx_min, __glibcxx_max, __glibcxx_digits, __glibcxx_digits10, __glibcxx_max_digits10 macros can be used in most cases and avoid any need to predefine macros for the min or max of __intN; you only need to communicate which types exist and their sizes in bits (that is, a single macro predefine for each N, with anything else being considered separately if otherwise thought desirable). + if
Re: __intN patch 3/5: main __int128 - __intN conversion.
Why are types only entered in integer_types if wider than long long? IIRC it was so that TImode (__int128) could get into the array (it was there before) without adding the other smaller types, which (I think) don't need to be in there. I don't recall why they're left out, but... ah, I remember. ITK has to be sorted by bitsize and it's used for type promotion, we don't want types promoted *to* the __intN types, just *from* them. +static bool +standard_type_bitsize (int bitsize) +{ + /* As a special exception, we always want __int128 enabled if possible. */ + if (bitsize == 128) +return false; + if (bitsize == CHAR_TYPE_SIZE + || bitsize == SHORT_TYPE_SIZE + || bitsize == INT_TYPE_SIZE + || bitsize == LONG_TYPE_SIZE + || (bitsize == LONG_LONG_TYPE_SIZE LONG_LONG_TYPE_SIZE GET_MODE_BITSIZE (TImode))) I don't think there should be a special case depending on the size of long long here. I think it was from before I had the special case for bitsize == 128. I'll remove it. + /* This must happen after the backend has a chance to process +command line options, but before the parsers are +initialized. */ + for (i = 0; i NUM_INT_N_ENTS; i ++) + if (targetm.scalar_mode_supported_p (int_n_data[i].m) +! standard_type_bitsize (int_n_data[i].bitsize) +int_n_data[i].bitsize = HOST_BITS_PER_WIDE_INT * 2) + int_n_enabled_p[i] = true; This HOST_BITS_PER_WIDE_INT * 2 check seems wrong. That check was there before, for __int128, I left it as-is. There is no __int128 (currently) if it's bigger then HBPWI*2. mode_for_size (unsigned int size, enum mode_class mclass, int limit) { - enum machine_mode mode; + enum machine_mode mode = BLKmode; + int i; I don't see any need for this initialization to be added. Removed. bprecision -= MIN (precision + BITS_PER_UNIT_LOG + 1, MAX_FIXED_MODE_SIZE); += MIN (precision, MAX_FIXED_MODE_SIZE); This change doesn't make sense to me - if it is needed, I think it should be separated out, not included in this patch which should simply about providing generic __intN support in the cases where __int128 is currently supported. Perhaps is belongs in the 1/5 patch, where I removed lots of assume all types are powers-of-two sizes logic? I split up the patch and logs by hand from a master patch, I'm not surprised I got a few mis-placed. @@ -1157,13 +1165,13 @@ enum omp_clause_map_kind OMP_CLAUSE_MAP_ALLOC, OMP_CLAUSE_MAP_TO, OMP_CLAUSE_MAP_FROM, OMP_CLAUSE_MAP_TOFROM, /* The following kind is an internal only map kind, used for pointer based array sections. OMP_CLAUSE_SIZE for these is not the pointer size, - which is implicitly POINTER_SIZE / BITS_PER_UNIT, but the bias. */ + which is implicitly POINTER_SIZE_UNITS, but the bias. */ POINTER_SIZE_UNITS changes belong in another patch, not this one. Again, probably the 1/5 patch. /* ptr_type_node can't be used here since ptr_mode is only set when toplev calls backend_init which is not done with -E or pch. */ - psize = POINTER_SIZE / BITS_PER_UNIT; + psize = POINTER_SIZE_UNITS; Likewise. Likewise :-) @@ -844,12 +846,47 @@ c_cpp_builtins (cpp_reader *pfile) builtin_define_type_max (__LONG_LONG_MAX__, long_long_integer_type_node); builtin_define_type_minmax (__WCHAR_MIN__, __WCHAR_MAX__, underlying_wchar_type_node); builtin_define_type_minmax (__WINT_MIN__, __WINT_MAX__, wint_type_node); builtin_define_type_max (__PTRDIFF_MAX__, ptrdiff_type_node); builtin_define_type_max (__SIZE_MAX__, size_type_node); + for (i = 0; i NUM_INT_N_ENTS; i ++) +if (int_n_enabled_p[i]) + { + char buf[35+20+20]; + char buf_min[15+20]; + char buf_max[15+20]; + + sprintf(buf_min, __INT%d_MIN__, int_n_data[i].bitsize); + sprintf(buf_max, __INT%d_MAX__, int_n_data[i].bitsize); All this block of code appears to be new rather than replacing any existing code doing something similar with __int128. As such, I think it's best considered separately from the main __intN support. For each __intN we need to provide an __INTN_MIN__ and __INTN_MAX__, just like for char we provide __CHAR_MIN__ and __CHAR_MAX__. Also note missing spaces before '(' in this code. Fixed. Some of this may be needed for libstdc++, but not all. As far as I can tell, the existing __glibcxx_min, __glibcxx_max, __glibcxx_digits, __glibcxx_digits10, __glibcxx_max_digits10 macros can be used in most cases and avoid any need to predefine macros for the min or max of __intN; you only need to communicate which types exist and their sizes in bits (that is, a single macro predefine for each N, with anything else being considered separately if otherwise thought desirable). I tried that, and wasn't able to get a simpler macro to do it inline
Re: [PATCH] gcc/gcc.c: XNEWVEC enough space for 'saved_suffix' using
On 08/22/2014 12:18 AM, Mike Stump wrote: On Aug 18, 2014, at 3:06 AM, Chen Gang gang.chen.5...@gmail.com wrote: - one for original latest gcc source code (master for 20140816). - one for my modification based on the original latest gcc source code. - they passed building, but for make check, they reported same issues: So, I see no evidence that you ran contrib/compare_tests yet. Did you? If so, what was the output? For only sending my patch, may I bypass them, So, the output of the make check is uninteresting, only the regressions identified by compare_tests are interesting. If none, then you’re set wrt the current patch. OK, thanks. At present, under Fedora 20 x86_64, it passed normal testsuite: - ../gcc/configure make make check succeed (echo $? == 0). - contrib/compare_test dir_orig dir_new succeed (echo $? == 0). - The related output of contrib/compare_test are: # Comparing directories ## Dir1=/upstream/build-gcc: 11 sum files ## Dir2=/upstream/build-gcc-new: 11 sum files # Comparing 11 common sum files ## /bin/sh ./compare_tests /tmp/gxx-sum1.7678 /tmp/gxx-sum2.7678 # No differences found in 11 common sum files and then I shall try to analyze them one by one in another time, next? You can, if you want. For example, there is a PR on the darwin issue that has state in it. Roughly binds local is slightly wrong and need fixing. OK, thank. I shall still try the Darwin, next, in another time, although it is not related with my new patches for sending. At last: Thank all of you very much for your much help, and I shall send related patch next. Thanks. -- Chen Gang Open share and attitude like air water and life which God blessed
Re: [PATCH c++/57709] Wshadow is too strict / has false positives
On 08/21/2014 04:22 PM, Manuel López-Ibáñez wrote: +TREE_CODE (x) != FUNCTION_DECL +!FUNCTION_POINTER_TYPE_P (TREE_TYPE (x How about pointer to member function? Jason
Re: __intN patch 3/5: main __int128 - __intN conversion.
On Thu, 21 Aug 2014, DJ Delorie wrote: + /* This must happen after the backend has a chance to process + command line options, but before the parsers are + initialized. */ + for (i = 0; i NUM_INT_N_ENTS; i ++) + if (targetm.scalar_mode_supported_p (int_n_data[i].m) + ! standard_type_bitsize (int_n_data[i].bitsize) + int_n_data[i].bitsize = HOST_BITS_PER_WIDE_INT * 2) + int_n_enabled_p[i] = true; This HOST_BITS_PER_WIDE_INT * 2 check seems wrong. That check was there before, for __int128, I left it as-is. There is no __int128 (currently) if it's bigger then HBPWI*2. I don't see any corresponding HOST_BITS_PER_WIDE_INT test for __int128 being removed (and anyway HOST_BITS_PER_WIDE_INT is now always 64, so such a test for __int128 would be dead code). All this block of code appears to be new rather than replacing any existing code doing something similar with __int128. As such, I think it's best considered separately from the main __intN support. For each __intN we need to provide an __INTN_MIN__ and __INTN_MAX__, just like for char we provide __CHAR_MIN__ and __CHAR_MAX__. No, those are provided for use by limits.h, which only covers standard C types (and in particular does not cover __int128). Some of this may be needed for libstdc++, but not all. As far as I can tell, the existing __glibcxx_min, __glibcxx_max, __glibcxx_digits, __glibcxx_digits10, __glibcxx_max_digits10 macros can be used in most cases and avoid any need to predefine macros for the min or max of __intN; you only need to communicate which types exist and their sizes in bits (that is, a single macro predefine for each N, with anything else being considered separately if otherwise thought desirable). I tried that, and wasn't able to get a simpler macro to do it inline than the full macro that let gcc figure out the values. Consider the two N of 20 and 128; one is not a multiple of bytes and the other will likely stress any runtime math. If __intN is supported, GCC needs to be able to handle folding arithmetic on it, such as the expansion of the existing __glibcxx_max macro. Maybe you need to refactor __glibcxx_digits so there is a version taking the bitsize as an argument rather than using sizeof(T) * __CHAR_BIT__, but that should be the only change needed to handle such types with the existing macros. The bitsize macros should be the only ones needing predefining to pass information to libstdc++. -- Joseph S. Myers jos...@codesourcery.com
[PATCH v2] gcc/c/c-aux-info.c: Resize 'buff' from 10 to 23 bytes
int_size_in_bytes() returns HOST_WIDE_INT (64-bit), theoretically, the maximized size is 23 -- it is sizeof([-9223372036854775808]) for 0x8000LL. It may not cause real world issue, but if another issues occur, it may lead things worse. It passes normal testsuite: ../gcc/configure make make check is OK. And 'contrib/compare_tests' is OK, too, the related output is: # Comparing directories ## Dir1=/upstream/build-gcc: 11 sum files ## Dir2=/upstream/build-gcc-new: 11 sum files # Comparing 11 common sum files ## /bin/sh ./compare_tests /tmp/gxx-sum1.7678 /tmp/gxx-sum2.7678 # No differences found in 11 common sum files 2014-08-17 Chen Gang gang.chen.5...@gmail.com * c/c-aux-info.c (gen_type): Resize 'buff' from 10 to 23 bytes, with using HOST_WIDE_INT without truncation to 'int' --- gcc/c/c-aux-info.c | 7 --- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/gcc/c/c-aux-info.c b/gcc/c/c-aux-info.c index 4b6b2d0..878807b 100644 --- a/gcc/c/c-aux-info.c +++ b/gcc/c/c-aux-info.c @@ -310,9 +310,10 @@ gen_type (const char *ret_val, tree t, formals_style style) TREE_TYPE (t), style); else { - int size = (int_size_in_bytes (t) / int_size_in_bytes (TREE_TYPE (t))); - char buff[10]; - sprintf (buff, [%d], size); + char buff[23]; + sprintf (buff, [HOST_WIDE_INT_PRINT_DEC], + int_size_in_bytes (t) + / int_size_in_bytes (TREE_TYPE (t))); ret_val = gen_type (concat (ret_val, buff, NULL), TREE_TYPE (t), style); }
Re: [patch, nios2] testsuite cleanup
On Aug 21, 2014, at 10:59 AM, Sandra Loosemore san...@codesourcery.com wrote: On 08/21/2014 11:36 AM, Mike Stump wrote: On Aug 21, 2014, at 8:00 AM, Sandra Loosemore san...@codesourcery.com wrote: tests that assume some non-default branch costs in the back end Thanks. One comment, could you put in /* non default branch cost */ above the three where that is true. The three what? :-S Sorry, I meant 4… Your patch has four instances of this change: -/* { dg-do run { target { ! m68k*-*-* mmix*-*-* mep*-*-* bfin*-*-* v850*-*-* picochip*-*-* moxie*-*-* cris*-*-* m32c*-*-* fr30*-*-* mcore*-*-* powerpc*-*-* xtensa*-*-* hppa*-*-*} } } */ +/* { dg-do run { target { ! m68k*-*-* mmix*-*-* mep*-*-* bfin*-*-* v850*-*-* picochip*-*-* moxie*-*-* cris*-*-* m32c*-*-* fr30*-*-* mcore*-*-* powerpc*-*-* xtensa*-*-* hppa*-*-* nios2*-*-*} } } */ see your patch for them. Can you change the patch effectively to: -/* { dg-do run { target { ! m68k*-*-* mmix*-*-* mep*-*-* bfin*-*-* v850*-*-* picochip*-*-* moxie*-*-* cris*-*-* m32c*-*-* fr30*-*-* mcore*-*-* powerpc*-*-* xtensa*-*-* hppa*-*-*} } } */ +/* non default branch cost */ +/* { dg-do run { target { ! m68k*-*-* mmix*-*-* mep*-*-* bfin*-*-* v850*-*-* picochip*-*-* moxie*-*-* cris*-*-* m32c*-*-* fr30*-*-* mcore*-*-* powerpc*-*-* xtensa*-*-* hppa*-*-* nios2*-*-*} } } */ instead? The comment serves as documentation as to what all the listed targets have in common. A person doing a new port, can then read the comment, and say I am non-default branch cost, so add me, or alternatively, I am the default, this failure is a bug I need to investigate and fix. I definitely agree about this. Perhaps the tree-ssa maintainers can take care of this, I added law as he added one of the test cases. :-) or at least clarify what target(s) this is intended to work on rather than having to experimentally determine which ones don't implement these optimizations, or propose some way to test for the support rather than having to list targets explicitly. I’d be fine with the entire list changing to the one target that the test case was put in for. That would be better than how it is now.
Re: [PATCH c++/57709] Wshadow is too strict / has false positives
On 21 August 2014 23:35, Jason Merrill ja...@redhat.com wrote: On 08/21/2014 04:22 PM, Manuel López-Ibáńez wrote: +TREE_CODE (x) != FUNCTION_DECL +!FUNCTION_POINTER_TYPE_P (TREE_TYPE (x How about pointer to member function? Maybe like this? BTW, I did not actually want to include the gcc_assert(), it was just a sanity check, thus I deleted it from this patch. Bootstrapping and regression testing in progress. But the testcase works as expected. OK if it passes? gcc/cp/ChangeLog: 2014-08-21 Manuel López-Ibáñez m...@gcc.gnu.org PR c++/57709 * name-lookup.c (pushdecl_maybe_friend_1): Do not warn if a declaration shadows a function declaration, unless the former declares a function, pointer to function or pointer to member function, because this is a common and valid case in real-world code. gcc/testsuite/ChangeLog: 2014-08-21 Manuel López-Ibáñez m...@gcc.gnu.org PR c++/57709 * g++.dg/Wshadow.C: New test. Index: gcc/testsuite/g++.dg/Wshadow.C === --- gcc/testsuite/g++.dg/Wshadow.C (revision 0) +++ gcc/testsuite/g++.dg/Wshadow.C (revision 0) @@ -0,0 +1,15 @@ +// { dg-do compile } +// { dg-options -Wshadow } +// PR c++/57709 +class C { + int both_var; // { dg-message declaration } + void var_and_method(void) {} // { dg-message declaration } + void m() { +int + both_var, // { dg-warning shadows } + var_and_method; + } + void m2() { +void (C::*var_and_method)(void); // { dg-warning shadows } + } +}; Index: gcc/cp/name-lookup.c === --- gcc/cp/name-lookup.c(revision 214229) +++ gcc/cp/name-lookup.c(working copy) @@ -1237,13 +1237,28 @@ pushdecl_maybe_friend_1 (tree x, bool is else member = NULL_TREE; if (member !TREE_STATIC (member)) { - /* Location of previous decl is not useful in this case. */ - warning (OPT_Wshadow, declaration of %qD shadows a member of 'this', - x); + if (BASELINK_P (member)) + member = BASELINK_FUNCTIONS (member); + member = OVL_CURRENT (member); + + /* Do not warn if a variable shadows a function, unless +the variable is a function or a pointer-to-function. */ + if (!(TREE_CODE (member) == FUNCTION_DECL +TREE_CODE (x) != FUNCTION_DECL +!(FUNCTION_POINTER_TYPE_P (TREE_TYPE (x)) +|| TYPE_PTRMEMFUNC_P (TREE_TYPE (x) + { + if (warning_at (input_location, OPT_Wshadow, + declaration of %qD shadows a member of %qT, + x, current_nonlambda_class_type ()) + DECL_P(member)) + inform (DECL_SOURCE_LOCATION (member), + shadowed declaration is here); + } } else if (oldglobal != NULL_TREE (VAR_P (oldglobal) /* If the old decl is a type decl, only warn if the old decl is an explicit typedef or if both the
[patch] PR fortran/62135
Hello, Low-hanging fruit, almost embarassing to fix. But then again I caused this bug -- in 1999 or so :-) Will commit after testing. Ciao! Steven fortran/ * resolve.c (resolve_select): Fix list traversal in case the last element of the CASE list was dropped as unreachable code. testsuite/ * gfortran.dg/pr62135.f90: New test. Index: fortran/resolve.c === --- fortran/resolve.c (revision 214292) +++ fortran/resolve.c (working copy) @@ -7761,7 +7761,7 @@ resolve_select (gfc_code *code, bool select_type) /* Strip all other unreachable cases. */ if (body-ext.block.case_list) { - for (cp = body-ext.block.case_list; cp-next; cp = cp-next) + for (cp = body-ext.block.case_list; cp cp-next; cp = cp-next) { if (cp-next-unreachable) { Index: testsuite/gfortran.dg/pr62135.f90 === --- testsuite/gfortran.dg/pr62135.f90 (revision 0) +++ testsuite/gfortran.dg/pr62135.f90 (working copy) @@ -0,0 +1,17 @@ +! { dg-do compile } +! { dg-options -Wsurprising } + + PROGRAM PR62135 + IMPLICIT NONE + CHARACTER*1 :: choice + choice = 'x' + SELECT CASE (choice) + ! This triggered an ICE: an unreachable case clause + ! as the last of a list. + CASE ('2':'7','9':'0') ! { dg-warning can never be matched } +WRITE(*,*) barf + CASE DEFAULT +CONTINUE + END SELECT + END PROGRAM PR62135 +
Re: [patch] PR fortran/62135
On Fri, Aug 22, 2014 at 12:07:21AM +0200, Steven Bosscher wrote: Hello, Low-hanging fruit, almost embarassing to fix. But then again I caused this bug -- in 1999 or so :-) Will commit after testing. I already approved the patch in the PR. In my comment on the PR I had assumed you were bored and found a Fortran bug to fix. Now, I see it is just guilt! -- Steve
Re: __intN patch 3/5: main __int128 - __intN conversion.
I don't see any corresponding HOST_BITS_PER_WIDE_INT test for __int128 being removed (and anyway HOST_BITS_PER_WIDE_INT is now always 64, so such a test for __int128 would be dead code). It was there when I started the patch, honest! :-) Removed ;-) For each __intN we need to provide an __INTN_MIN__ and __INTN_MAX__, just like for char we provide __CHAR_MIN__ and __CHAR_MAX__. No, those are provided for use by limits.h, which only covers standard C types (and in particular does not cover __int128). Ok, removed. Maybe you need to refactor __glibcxx_digits so there is a version taking the bitsize as an argument rather than using sizeof(T) * __CHAR_BIT__, but that should be the only change needed to handle such types with the existing macros. Since the other macros use this macro, we'd need a complete second set of macros just for the __intN types anyway, each of which takes a bitsize and passes it down. Since gcc already knows all the right answers for the __intN types and needs to emit other macros for them anyway, where's the benefit? Also, consider that the specialization for all the other types is duplicated in full, I'm already using less source code for my 1..4 types than the library would have used for its 1..4 types ;-)
Re: [PATCH c++/57709] Wshadow is too strict / has false positives
Hi, On 08/22/2014 12:01 AM, Manuel López-Ibáñez wrote: On 21 August 2014 23:35, Jason Merrill ja...@redhat.com wrote: On 08/21/2014 04:22 PM, Manuel López-Ibáńez wrote: +TREE_CODE (x) != FUNCTION_DECL +!FUNCTION_POINTER_TYPE_P (TREE_TYPE (x How about pointer to member function? Maybe like this? BTW, I did not actually want to include the gcc_assert(), it was just a sanity check, thus I deleted it from this patch. Note by the way, that this patch would introduce the *first* use of FUNCTION_POINTER_TYPE_P in the C++ front-end. Are we sure we want to use that vs TYPE_PTRFN_P?!? At the moment I'm a bit tired by I seem to remember subtleties in this area... Paolo.
Re: __intN patch 3/5: main __int128 - __intN conversion.
On Thu, 21 Aug 2014, DJ Delorie wrote: Maybe you need to refactor __glibcxx_digits so there is a version taking the bitsize as an argument rather than using sizeof(T) * __CHAR_BIT__, but that should be the only change needed to handle such types with the existing macros. Since the other macros use this macro, we'd need a complete second set of macros just for the __intN types anyway, each of which takes a Well, the existing macros would be defined in terms of the new ones. bitsize and passes it down. Since gcc already knows all the right answers for the __intN types and needs to emit other macros for them anyway, where's the benefit? The more predefined macros there are, the more impact on startup time (though I don't have any specific figures). -- Joseph S. Myers jos...@codesourcery.com
Re: [wwwdocs] Update GCC5 changes.html
On Wed, 13 Aug 2014, Marek Polacek wrote: I've put together a few lines describing what I (except -fsanitize=alignment) implemented for GCC 5. It's the file wwwdocs/htdocs/gcc-5/changes.html. Nice! + licode-fsanitize=float-cast-overflow/code: check that the result +of floating-point type to integer conversion does not overflow;/li conversions (plural)? +liA new code-Wswitch-bool/code option has been added for the C and C++ + compilers, which warns whenever a codeswitch/code statement has an + index of boolean type./li Here, and in the other cases, A new option code... might be less ambigous -- someone might read this as an option for this command-line option. This is just a suggestion, feel free to ignore. I would say command-line option instead of just option, though. +liA new code-Wlogical-not-parentheses/code option has been added for the + C and C++ compilers, which warns about logical not used on the left hand + side operand of a comparison./li logical not in quotes, perhaps? Otherwise this may be a bit hard to parse. +liA new code-Wsizeof-array-argument/code option has been added for the + C and C++ compilers, which warns when the codesizeof/code operator is + applied to a parameter that is declared as an array in a function definition. has been instead of is declard? +liIt is possible to disable warnings about conversions between pointers + that have incompatible types via a new warning option + code-Wno-incompatible-pointer-types/code; warnings about implicit + incompatible integer to pointer and pointer to integer conversions via + a new warning option code-Wno-int-conversion/code; and warnings about + qualifiers on pointers being discarded via a new warning option Should we write pointer-to-integer and the like, here and in other parts of the patch? Probably best a question for Joseph (and if he has approved code/document patches where that was not the case, than the answer pretty likely is now. ;-) Gerald
Re: [PATCH v2] gcc/c/c-aux-info.c: Resize 'buff' from 10 to 23 bytes
On Fri, 22 Aug 2014, Chen Gang wrote: 2014-08-17 Chen Gang gang.chen.5...@gmail.com * c/c-aux-info.c (gen_type): Resize 'buff' from 10 to 23 bytes, with using HOST_WIDE_INT without truncation to 'int' OK (without the c/ in the ChangeLog entry, as it should go in c/ChangeLog). -- Joseph S. Myers jos...@codesourcery.com
Re: [Patch 1/2] Don't put out a call to memcpy for volatile struct operations
On Aug 21, 2014, at 4:22 AM, Richard Biener richard.guent...@gmail.com wrote: I still say we need to solve the issue at language level - that is, try to figure out what the language standard says about volatile struct X x, y; x = y; The definition of x = y doesn’t change wrt volatile above. See below for the semantic of x = y; What this does is it makes the members of x and y volatile: [#7] EXAMPLE 2 In: | struct s { int i; const int ci; }; struct s s; const struct s cs; volatile struct s vs; the various members have the types: | s.i int s.ciconst int cs.iconst int cs.ci const int vs.ivolatile int vs.ci volatile const int or about struct X { volatile int x; } x, y; x = y; So, what the C99 standard[1] says is that memcpy copies n characters from one to the other, leaving unspecified the order of the copy. C++98 reuses by reference those semantics. Of course, there are quite a few memcpy implementations that don’t do that. For x = y, in C++98, it is defined like so: 8 The implicitly-defined copy constructor for class X performs a member- wise copy of its subobjects. The order of copying is the same as the order of initialization of bases and members in a user-defined con- structor (see _class.base.init_). Each subobject is copied in the manner appropriate to its type which means a volatile int member translates to volatile SI read/write as appropriate, or put another way, one can’t use memcpy for it. Now, that isn’t to say that we can’t change the language standard or improve it with different semantics. For C99: [#2] In simple assignment (=), the value of the right operand is converted to the type of the assignment expression and replaces the value stored in the object designated by the left operand. which I’d claim isn’t exactly clear and precise. Clearly what they were thinking was: 36)Thus, for example, structure assignment may be implemented element-at-a-time or via memcpy. left not exactly well defined is the case of volatile. Reasonable people would say that volatile semantics are likely the same as C++98 (also, C++ was mostly just noting what we thought the C standard said in the first place). I don’t keep up on DRs that might explicitly cover details, so I’d defer those those if any. I expect that most structs have volatile for a bogus reason anyway and we slow down and enlarge code for no good reason. Yes, I suspect if we put in code to handle volatile members better, that no code will care. Why, cause no one has asked for those semantics, no code depends upon those semantics. Though, in time, some code might care. So - why bother fixing this? ISTR reading in the C standard that structure assignments are expected to compile to memcpy. Your ISTR is quoted for you above. That wording isn’t a prescription of semantics. It is an observation that there are some situations where the implementation may use memcpy. In C99, sig_atomic_t defines when something is lock free, leaving unspecific what else may be. In later C++ standards (for example C++14), [atomics.lockfree] defines additional types that are atomic. 1 - I use n843 for C99, which is slightly different from the standard, but in this case I suspect it is the same.
Re: [patch, nios2] testsuite cleanup
On 08/21/2014 03:50 PM, Mike Stump wrote: Sorry, I meant 4… Your patch has four instances of this change: -/* { dg-do run { target { ! m68k*-*-* mmix*-*-* mep*-*-* bfin*-*-* v850*-*-* picochip*-*-* moxie*-*-* cris*-*-* m32c*-*-* fr30*-*-* mcore*-*-* powerpc*-*-* xtensa*-*-* hppa*-*-*} } } */ +/* { dg-do run { target { ! m68k*-*-* mmix*-*-* mep*-*-* bfin*-*-* v850*-*-* picochip*-*-* moxie*-*-* cris*-*-* m32c*-*-* fr30*-*-* mcore*-*-* powerpc*-*-* xtensa*-*-* hppa*-*-* nios2*-*-*} } } */ see your patch for them. Can you change the patch effectively to: -/* { dg-do run { target { ! m68k*-*-* mmix*-*-* mep*-*-* bfin*-*-* v850*-*-* picochip*-*-* moxie*-*-* cris*-*-* m32c*-*-* fr30*-*-* mcore*-*-* powerpc*-*-* xtensa*-*-* hppa*-*-*} } } */ +/* non default branch cost */ +/* { dg-do run { target { ! m68k*-*-* mmix*-*-* mep*-*-* bfin*-*-* v850*-*-* picochip*-*-* moxie*-*-* cris*-*-* m32c*-*-* fr30*-*-* mcore*-*-* powerpc*-*-* xtensa*-*-* hppa*-*-* nios2*-*-*} } } */ instead? The comment serves as documentation as to what all the listed targets have in common. A person doing a new port, can then read the comment, and say I am non-default branch cost, so add me, or alternatively, I am the default, this failure is a bug I need to investigate and fix. Hmmm, I think this is backwards. At least nios2 does not override BRANCH_COST. The default value (in defaults.h) is 1; when I was investigating these tree-ssa failures I saw that the test being used to enable the optimizations is = 2. It's also quite possible for back ends to override BRANCH_COST but in a way that still causes the tests to fail, at least for some multilibs or processor options. I'd really like the maintainers of these tree-ssa tests to figure out what target they're supposed to work for or come up with a suitable test for feature support, rather than me trying to guess the failure mode for all these other back ends I can't test. -Sandra
Re: [PATCH, g++, testsuite] Skip thread_local6.C on target using wrapper
On Aug 20, 2014, at 3:31 AM, Tony Wang tony.w...@arm.com wrote: It's a very simple test case modification to fix the test case failure on ARM bare metal target. thread_local6.C is a test case to test the behavior of the deconstruct of a thread local variable, and it just use _exit(0) to override the return 1(calling exit(1)). However, such a behavior can't be detected on embedded target with dejagnu wrapper mechanism, because the wrapper will only output the return value in exit() instead of _exit(). Hum, another solution might be to wrap _exit as well. The patch is so simple and short, I’ll approve the posted patch; it is a nice step forward. I’ll let you contemplate if you want to try a wrap on _exit. Could you please add /* wrapping doesn’t reflect _exit value */ if you add this version. This can go on line 3. If _exit isn’t used, this is the clue we can then run the test case more often, and if someone wants to copy the use of _exit or fix wrapping, this is a handy clue for this. Thanks.
Re: [patch, nios2] testsuite cleanup
On Aug 21, 2014, at 5:06 PM, Sandra Loosemore san...@codesourcery.com wrote: I'd really like the maintainers of these tree-ssa tests to figure out what target they're supposed to work for or come up with a suitable test for feature support, rather than me trying to guess the failure mode for all these other back ends I can't test. I know the feeling… :-)
Re: [PATCH v2] gcc/c/c-aux-info.c: Resize 'buff' from 10 to 23 bytes
On 8/22/14 7:11, Joseph S. Myers wrote: On Fri, 22 Aug 2014, Chen Gang wrote: 2014-08-17 Chen Gang gang.chen.5...@gmail.com * c/c-aux-info.c (gen_type): Resize 'buff' from 10 to 23 bytes, with using HOST_WIDE_INT without truncation to 'int' OK (without the c/ in the ChangeLog entry, as it should go in c/ChangeLog). OK, thanks. If necessary to send patch v3 for it, please let me know. Thanks -- Chen Gang Open, share, and attitude like air, water, and life which God blessed
RE: [PATCH, g++, testsuite] Skip thread_local6.C on target using wrapper
Hi Mike Hum, another solution might be to wrap _exit as well. The patch is so simple and short, I'll approve the posted patch; it is a nice step forward. I'll let you contemplate if you want to try a wrap on _exit. Thanks for your reply, and I also thought of your suggestion to wrap the _exit. The fact is that dejagnu does the wrapper to both exit() and _exit(), but it give a higher priority to the exit(), which means if you both call exit() and _exit(), it will only output the return value in exit(). Someone told me that you shouldn't explicitly call _exit() inside an exit(), and that's why the dejagnu skip the second _exit() output. However, I'm not sure about this, as there're quite a few c++ test cases are doing the same thing. Could you please add /* wrapping doesn't reflect _exit value */ if you add this version. This can go on line 3. If _exit isn't used, this is the clue we can then run the test case more often, and if someone wants to copy the use of _exit or fix wrapping, this is a handy clue for this. Ok, I will commit with this version. BR, Tony
Re: [PATCH v4] Add strict aliasing warning when inlining function.
Hi, Currently warning about strict aliasing is not strict enough. It has not considered tbaa. So a test case emit addition error. This patch is not good for commit. -- Lin Zuojian
Re: [PATCH v4] Add strict aliasing warning when inlining function.
Hi, My patch for warning about strict aliasing violation is not strict enough. Current implementation of strict_aliasing_warn has not consider tbaa. So a test case gcc/testsuite/g++.dg/opt/pmf1.C will emit additional warning whereas it shouldn't do. My patch is not good for committing. --- Lin Zuojian
Re: __intN patch 3/5: main __int128 - __intN conversion.
Maybe you need to refactor __glibcxx_digits so there is a version taking the bitsize as an argument rather than using sizeof(T) * __CHAR_BIT__, but that should be the only change needed to handle such types with the existing macros. The bitsize macros should be the only ones needing predefining to pass information to libstdc++. Like this? #define __glibcxx_signed_b(T,B) ((T)(-1) 0) #define __glibcxx_min_b(T,B)\ (__glibcxx_signed_b (T,B) ? -__glibcxx_max_b (T,B) - 1 : (T)0) #define __glibcxx_max_b(T,B)\ (__glibcxx_signed_b (T,B) ? \ (T)1 (__glibcxx_digits_b (T,B) - 1)) - 1) 1) + 1) : ~(T)0) #define __glibcxx_digits_b(T,B) \ (B - __glibcxx_signed_b (T,B)) // The fraction 643/2136 approximates log10(2) to 7 significant digits. #define __glibcxx_digits10_b(T,B) \ (__glibcxx_digits_b (T,B) * 643L / 2136) #define __glibcxx_signed(T) \ __glibcxx_signed_b (T, sizeof(T) * __CHAR_BIT__) #define __glibcxx_min(T) \ __glibcxx_min (T, sizeof(T) * __CHAR_BIT_) #define __glibcxx_max(T) \ __glibcxx_max (T, sizeof(T) * __CHAR_BIT_) #define __glibcxx_digits(T) \ __glibcxx_digits (T, sizeof(T) * __CHAR_BIT_) #define __glibcxx_digits10(T) \ __glibcxx_digits10 (T, sizeof(T) * __CHAR_BIT_)