[PATCH] relax constraint on integral<->offset conversions (PR53336)
OFFSET_TYPE is treated as an integral type for the purpose of conversion in fold-const.c. However, the GIMPLE verifier disagrees, leading to verification errors when a cast from boolean to offset type is gimplified. Bootstrapped/regtested x86_64-pc-linux-gnu, ok for mainline? Paolo 2012-05-16 Paolo Bonzini * tree-cfg.c (verify_gimple_assign_unary): Allow conversion from non-integer integral types to offset type and vice versa. 2012-05-16 Paolo Bonzini * g++.dg/torture/pr53336.C: New testcase. Index: tree-cfg.c === --- tree-cfg.c (revisione 186903) +++ tree-cfg.c (copia locale) @@ -3374,11 +3374,11 @@ verify_gimple_assign_unary (gimple stmt) || ptrofftype_p (sizetype return false; - /* Allow conversion from integer to offset type and vice versa. */ + /* Allow conversion from integral to offset type and vice versa. */ if ((TREE_CODE (lhs_type) == OFFSET_TYPE -&& TREE_CODE (rhs1_type) == INTEGER_TYPE) +&& INTEGRAL_TYPE_P (rhs1_type)) || (TREE_CODE (lhs_type) == INTEGER_TYPE - && TREE_CODE (rhs1_type) == OFFSET_TYPE)) + && INTEGRAL_TYPE_P (rhs1_type))) return false; /* Otherwise assert we are converting between types of the Index: testsuite/g++.dg/torture/pr53336.C === --- testsuite/g++.dg/torture/pr53336.C (revisione 0) +++ testsuite/g++.dg/torture/pr53336.C (revisione 0) @@ -0,0 +1,45 @@ +// { dg-do compile } + +bool foo(); + +struct C +{ +C() +{ +if (foo()) +foo(); +} +}; + +struct S +{ +struct dummy +{ +int i_; +}; +typedef int dummy::*bool_type; + +operator bool_type() const +{ +return foo() ? &dummy::i_ : 0; +} +}; + +int x; + +struct adaptor +{ +C c; + +virtual void bar() +{ +if (S()) +x = 0; +} +}; + +int main() +{ +adaptor a; +} +
Re: [PATCH, Android] Stack protector enabling for Android target
Hi all, First hunk is not needed indeed. Testing is ok. Thanks!!! Here is the final patch: diff --git a/gcc/configure.ac b/gcc/configure.ac index 2c17736..43e760b 100644 --- a/gcc/configure.ac +++ b/gcc/configure.ac @@ -4566,6 +4566,11 @@ AC_CACHE_CHECK(__stack_chk_fail in target C library, $target_header_dir/bits/uClibc_config.h > /dev/null; then gcc_cv_libc_provides_ssp=yes fi + # all versions of Bionic support stack protector + elif test -f $target_header_dir/sys/cdefs.h \ +&& $EGREP '^[ ]*#[]*define[ ]+__BIONIC__[ ]+1' \ + $target_header_dir/sys/cdefs.h > /dev/null; then + gcc_cv_libc_provides_ssp=yes fi] ;; *-*-gnu*) 2012/5/15 Maxim Kuvyrkov : > On 12/05/2012, at 9:03 AM, Igor Zamyatin wrote: > >> Hi! >> >> Please look at the modified patch in the attachment. ChangeLog remains the >> same. >> >> Tested in android environment(x86_64-*-linux-android), also >> bootstrapped on x86_64-unknown-linux-gnu. >> I also started regtesting on linux. Is it ok after successfull regtesting? > > diff --git a/gcc/configure.ac b/gcc/configure.ac > index 2c17736..43e760b 100644 > --- a/gcc/configure.ac > +++ b/gcc/configure.ac > @@ -4545,7 +4545,7 @@ AC_CACHE_CHECK(__stack_chk_fail in target C library, > gcc_cv_libc_provides_ssp, > [gcc_cv_libc_provides_ssp=no > case "$target" in > - *-*-linux* | *-*-kfreebsd*-gnu | *-*-knetbsd*-gnu) > + *-*-linux* | *-android* | *-*-kfreebsd*-gnu | *-*-knetbsd*-gnu) > > You should not need this change. Android target triplets are of form > *-*-linux-android*, which matches *-*-linux*. > > [# glibc 2.4 and later provides __stack_chk_fail and > # either __stack_chk_guard, or TLS access to stack guard canary. > if test -f $target_header_dir/features.h \ > @@ -4566,6 +4566,11 @@ AC_CACHE_CHECK(__stack_chk_fail in target C library, > $target_header_dir/bits/uClibc_config.h > /dev/null; then > gcc_cv_libc_provides_ssp=yes > fi > + # all versions of Bionic support stack protector > + elif test -f $target_header_dir/sys/cdefs.h \ > + && $EGREP '^[ ]*#[ ]*define[ ]+__BIONIC__[ ]+1' \ > + $target_header_dir/sys/cdefs.h > /dev/null; then > + gcc_cv_libc_provides_ssp=yes > fi] > ;; > *-*-gnu*) > > The patch is OK provided successful regtest and either dropping the first > hunk or explaining why it is necessary. > > Thanks! > > -- > Maxim Kuvyrkov > CodeSourcery / Mentor Graphics > -- Pavel Chupin Software Engineer Intel Corporation
Re: Fix glitch in GNAT detection test
> Fixed thusly, tested on i586-suse-linux, applied as obvious on mainline, > 4.6 and 4.5 branches. > > > 2011-12-18 Eric Botcazou > > * configure: Regenerate. > config/ > * acx.m4 (Test for GNAT): Update comment and add quotes in final test. It turns out that gcc/configure needs to be regenerated as well. Now done. -- Eric Botcazou
Re: [PATCH] Fix var tracking ICE due to reorg.
David Miller writes: > Ok for mainline and 4.7 branch? OK for both, thanks. > * jump.c (delete_related_insns): If we remove a CALL, make sure > we delete it's NOTE_INSN_CALL_ARG_LOCATION note too.
Re: remove TARGET_E500 and factorize SPE defaults computation in powerpc ports
On May 16, 2012, at 03:10 , David Edelsohn wrote: > Okay. revision 187581. Thanks!
Re: ping: Use "sed -n …" instead of "sed s/…/p -e d" in s-header-vars
On May 16, 2012, at 08:29 , Paolo Bonzini wrote: > Ok. Applied, Thanks Paolo :) Opinion on http://gcc.gnu.org/ml/gcc-patches/2012-04/msg00274.html ? Thanks in advance, Olivier
Re: Turn check macros into functions. (issue6188088)
On Tue, May 15, 2012 at 7:19 PM, Lawrence Crowl wrote: > The gcc source uses several constructs that GDB does not understand. > This patch corrects some of them. It affects only compilers built > with ENABLE_TREE_CHECKING, and hence release compilers are unaffected. > > In particular, I change the implementation of CHECK macros using > __extension__ into macros calling template inline functions. I do not > change the accessor macros themselves. > > The effect is that it now possible to get useful responses to gdb > command like > > (gdb) print DECL_FUNCTION_CODE (decl) > > To make the feature work, you must define two macros within gdb. > > (gdb) macro define __FILE__ "gdb" > (gdb) macro define __LINE__ 1 > > The reason for these definitions is that gdb does not expand the magic > preprocessor macros, and then cannot find what it thinks is a regular > symbol. (There is now a gdb issue for this problem.) I added these > definitions to gdbinit.in. > > The inline functions would normally introduce additional lines in a > sequence of gdb 'step' commands. To prevent that behavior, I have > added the new 'skip' command for the "tree.h" file to gdbinit.in. The > 'skip' command works with gdb 7.4 and later. > > (The better solution to the stepping behavior is a new gdb command. > See http://sourceware.org/bugzilla/show_bug.cgi?id=12940.) > > Tested on x86-64. What's the reason for templating these functions? They all take trees as parameter!? Richard. > Index: gcc/ChangeLog.cxx-conversion > > 2012-05-15 Lawrence Crowl > > * tree.h (tree_check): New. > (TREE_CHECK): Use inline function above instead of __extension__. > (tree_not_check): New. > (TREE_NOT_CHECK): Use inline function above instead of __extension__. > (tree_check2): New. > (TREE_CHECK2): Use inline function above instead of __extension__. > (tree_not_check2): New. > (TREE_NOT_CHECK2): Use inline function above instead of __extension__. > (tree_check3): New. > (TREE_CHECK3): Use inline function above instead of __extension__. > (tree_not_check3): New. > (TREE_NOT_CHECK3): Use inline function above instead of __extension__. > (tree_check4): New. > (TREE_CHECK4): Use inline function above instead of __extension__. > (tree_not_check4): New. > (TREE_NOT_CHECK4): Use inline function above instead of __extension__. > (tree_check5): New. > (TREE_CHECK5): Use inline function above instead of __extension__. > (tree_not_check5): New. > (TREE_NOT_CHECK5): Use inline function above instead of __extension__. > (contains_struct_check): New. > (CONTAINS_STRUCT_CHECK): Use inline function above instead of > __extension__. > (tree_class_check): New. > (TREE_CLASS_CHECK): Use inline function above instead of __extension__. > (tree_range_check): New. > (TREE_RANGE_CHECK): Use inline function above instead of __extension__. > (omp_clause_subcode_check): New. > (OMP_CLAUSE_SUBCODE_CHECK): Use inline function above instead of > __extension__. > (omp_clause_range_check): New. > (OMP_CLAUSE_RANGE_CHECK): Use inline function above instead of > __extension__. > (expr_check): New. > (EXPR_CHECK): Use inline function above instead of __extension__. > (non_type_check): New. > (NON_TYPE_CHECK): Use inline function above instead of __extension__. > (tree_vec_elt_check): New. > (TREE_VEC_ELT_CHECK): Use inline function above instead of > __extension__. > (omp_clause_elt_check): New. > (OMP_CLAUSE_ELT_CHECK): Use inline function above instead of > __extension__. > (tree_operand_check): New. > (TREE_OPERAND_CHECK): Use inline function above instead of > __extension__. > (tree_operand_check_code): New. > (TREE_OPERAND_CHECK_CODE): Use inline function above instead of > __extension__. > (TREE_CHAIN): Simplify implementation. > (TREE_TYPE): Simplify implementation. > (tree_operand_length): Move for compilation dependences. > * gdbinit.in: (macro define __FILE__): New. > (macro define __LINE__): New. > (skip "tree.h"): New. > > > Index: gcc/tree.h > === > --- gcc/tree.h (revision 187477) > +++ gcc/tree.h (working copy) > @@ -727,195 +727,80 @@ enum tree_node_structure_enum { > is accessed incorrectly. The macros die with a fatal error. */ > #if defined ENABLE_TREE_CHECKING && (GCC_VERSION >= 2007) > > -#define TREE_CHECK(T, CODE) __extension__ \ > -({ __typeof (T) const __t = (T); \ > - if (TREE_CODE (__t) != (CODE)) \ > - tree_check_failed (__t, __FILE__, __LINE__, __FUNCTION__, \ > -
Re: [PATCH] relax constraint on integral<->offset conversions (PR53336)
On Wed, May 16, 2012 at 9:19 AM, Paolo Bonzini wrote: > OFFSET_TYPE is treated as an integral type for the purpose of conversion > in fold-const.c. However, the GIMPLE verifier disagrees, leading to > verification errors when a cast from boolean to offset type is gimplified. > > Bootstrapped/regtested x86_64-pc-linux-gnu, ok for mainline? Ok. Thanks, Richard. > Paolo > > 2012-05-16 Paolo Bonzini > > * tree-cfg.c (verify_gimple_assign_unary): Allow conversion from > non-integer integral types to offset type and vice versa. > > 2012-05-16 Paolo Bonzini > > * g++.dg/torture/pr53336.C: New testcase. > > Index: tree-cfg.c > === > --- tree-cfg.c (revisione 186903) > +++ tree-cfg.c (copia locale) > @@ -3374,11 +3374,11 @@ verify_gimple_assign_unary (gimple stmt) > || ptrofftype_p (sizetype > return false; > > - /* Allow conversion from integer to offset type and vice versa. */ > + /* Allow conversion from integral to offset type and vice versa. */ > if ((TREE_CODE (lhs_type) == OFFSET_TYPE > - && TREE_CODE (rhs1_type) == INTEGER_TYPE) > + && INTEGRAL_TYPE_P (rhs1_type)) > || (TREE_CODE (lhs_type) == INTEGER_TYPE > - && TREE_CODE (rhs1_type) == OFFSET_TYPE)) > + && INTEGRAL_TYPE_P (rhs1_type))) > return false; > > /* Otherwise assert we are converting between types of the > Index: testsuite/g++.dg/torture/pr53336.C > === > --- testsuite/g++.dg/torture/pr53336.C (revisione 0) > +++ testsuite/g++.dg/torture/pr53336.C (revisione 0) > @@ -0,0 +1,45 @@ > +// { dg-do compile } > + > +bool foo(); > + > +struct C > +{ > + C() > + { > + if (foo()) > + foo(); > + } > +}; > + > +struct S > +{ > + struct dummy > + { > + int i_; > + }; > + typedef int dummy::*bool_type; > + > + operator bool_type() const > + { > + return foo() ? &dummy::i_ : 0; > + } > +}; > + > +int x; > + > +struct adaptor > +{ > + C c; > + > + virtual void bar() > + { > + if (S()) > + x = 0; > + } > +}; > + > +int main() > +{ > + adaptor a; > +} > +
Re: PING: [PATCH] Fix PR53217
On Tue, 15 May 2012, William J. Schmidt wrote: > Ping. I don't like it too much - but pondering a bit over it I can't find a nicer solution. So, ok. Thanks, Richard. > Thanks, > Bill > > On Tue, 2012-05-08 at 22:04 -0500, William J. Schmidt wrote: > > This fixes another statement-placement issue when reassociating > > expressions with repeated factors. Multiplies feeding into > > __builtin_powi calls were not getting placed properly ahead of them in > > some cases. > > > > Bootstrapped and tested on powerpc64-unknown-linux-gnu with no new > > regressions. I've also run SPEC cpu2006 with no build or correctness > > issues. OK for trunk? > > > > Thanks, > > Bill > > > > > > gcc: > > > > 2012-05-08 Bill Schmidt > > > > PR tree-optimization/53217 > > * tree-ssa-reassoc.c (bip_map): New static variable. > > (possibly_move_powi): Move feeding multiplies with __builtin_powi call. > > (attempt_builtin_powi): Save feeding multiplies on a stack. > > (reassociate_bb): Create and destroy bip_map. > > > > gcc/testsuite: > > > > 2012-05-08 Bill Schmidt > > > > PR tree-optimization/53217 > > * gfortran.dg/pr53217.f90: New test. > > > > > > Index: gcc/testsuite/gfortran.dg/pr53217.f90 > > === > > --- gcc/testsuite/gfortran.dg/pr53217.f90 (revision 0) > > +++ gcc/testsuite/gfortran.dg/pr53217.f90 (revision 0) > > @@ -0,0 +1,28 @@ > > +! { dg-do compile } > > +! { dg-options "-O1 -ffast-math" } > > + > > +! This tests only for compile-time failure, which formerly occurred > > +! when statements were emitted out of order, failing verify_ssa. > > + > > +MODULE xc_cs1 > > + INTEGER, PARAMETER :: dp=KIND(0.0D0) > > + REAL(KIND=dp), PARAMETER :: a = 0.04918_dp, & > > + c = 0.2533_dp, & > > + d = 0.349_dp > > +CONTAINS > > + SUBROUTINE cs1_u_2 ( rho, grho, r13, e_rho_rho, e_rho_ndrho, > > e_ndrho_ndrho,& > > + npoints, error) > > +REAL(KIND=dp), DIMENSION(*), & > > + INTENT(INOUT) :: e_rho_rho, e_rho_ndrho, & > > +e_ndrho_ndrho > > +DO ip = 1, npoints > > + IF ( rho(ip) > eps_rho ) THEN > > + oc = 1.0_dp/(r*r*r3*r3 + c*g*g) > > + d2rF4 = c4p*f13*f23*g**4*r3/r * (193*d*r**5*r3*r3+90*d*d*r**5*r3 & > > + -88*g*g*c*r**3*r3-100*d*d*c*g*g*r*r*r3*r3 & > > + +104*r**6)*od**3*oc**4 > > + e_rho_rho(ip) = e_rho_rho(ip) + d2F1 + d2rF2 + d2F3 + d2rF4 > > + END IF > > +END DO > > + END SUBROUTINE cs1_u_2 > > +END MODULE xc_cs1 > > Index: gcc/tree-ssa-reassoc.c > > === > > --- gcc/tree-ssa-reassoc.c (revision 187117) > > +++ gcc/tree-ssa-reassoc.c (working copy) > > @@ -200,6 +200,10 @@ static long *bb_rank; > > /* Operand->rank hashtable. */ > > static struct pointer_map_t *operand_rank; > > > > +/* Map from inserted __builtin_powi calls to multiply chains that > > + feed them. */ > > +static struct pointer_map_t *bip_map; > > + > > /* Forward decls. */ > > static long get_rank (tree); > > > > @@ -2249,7 +2253,7 @@ remove_visited_stmt_chain (tree var) > > static void > > possibly_move_powi (gimple stmt, tree op) > > { > > - gimple stmt2; > > + gimple stmt2, *mpy; > >tree fndecl; > >gimple_stmt_iterator gsi1, gsi2; > > > > @@ -2278,9 +2282,39 @@ possibly_move_powi (gimple stmt, tree op) > >return; > > } > > > > + /* Move the __builtin_powi. */ > >gsi1 = gsi_for_stmt (stmt); > >gsi2 = gsi_for_stmt (stmt2); > >gsi_move_before (&gsi2, &gsi1); > > + > > + /* See if there are multiplies feeding the __builtin_powi base > > + argument that must also be moved. */ > > + while ((mpy = (gimple *) pointer_map_contains (bip_map, stmt2)) != NULL) > > +{ > > + /* If we've already moved this statement, we're done. This is > > + identified by a NULL entry for the statement in bip_map. */ > > + gimple *next = (gimple *) pointer_map_contains (bip_map, *mpy); > > + if (next && !*next) > > + return; > > + > > + stmt = stmt2; > > + stmt2 = *mpy; > > + gsi1 = gsi_for_stmt (stmt); > > + gsi2 = gsi_for_stmt (stmt2); > > + gsi_move_before (&gsi2, &gsi1); > > + > > + /* The moved multiply may be DAG'd from multiple calls if it > > +was the result of a cached multiply. Only move it once. > > +Rank order ensures we move it to the right place the first > > +time. */ > > + if (next) > > + *next = NULL; > > + else > > + { > > + next = (gimple *) pointer_map_insert (bip_map, *mpy); > > + *next = NULL; > > + } > > +} > > } > > > > /* This function checks three consequtive operands in > > @@ -3281,6 +3315,7 @@ attempt_builtin_powi (gimple stmt, VEC(operand_ent > >while (true) > > { > >HOST_WIDE
Re: ping: Use "sed -n …" instead of "sed s/…/p -e d" in s-header-vars
> *** /tmp/dL6Ouq_Makefile.tpl 2012-04-05 14:37:34.144103910 +0200 > --- Makefile.tpl 2012-04-04 22:03:53.822060326 +0200 > *** install.all: install-no-fixedincludes > *** 906,916 > true ; \ > fi > > ! # install-no-fixedincludes is used because Cygnus can not distribute > ! # the fixed header files. > .PHONY: install-no-fixedincludes > install-no-fixedincludes: installdirs install-host-nogcc \ > ! install-target gcc-no-fixedincludes > > .PHONY: install-strip > install-strip: > --- 906,917 > true ; \ > fi > > ! # install-no-fixedincludes is used to allow the elaboration of binary > packages > ! # suitable for distribution, where we cannot include the fixed system header > ! # files. > .PHONY: install-no-fixedincludes > install-no-fixedincludes: installdirs install-host-nogcc \ > ! install-target gcc-install-no-fixedincludes > > .PHONY: install-strip > install-strip: This is missing in the ChangeLog. > *** check-gcc-[+language+]: > *** 1443,1467 > check-[+language+]: check-gcc-[+language+][+ FOR lib-check-target +] [+ > lib-check-target +][+ ENDFOR lib-check-target +] > [+ ENDFOR languages +] > > ! # Install the gcc headers files, but not the fixed include files, > ! # which Cygnus is not allowed to distribute. This rule is very > ! # dependent on the workings of the gcc Makefile.in. > ! .PHONY: gcc-no-fixedincludes > ! gcc-no-fixedincludes: > @if [ -f ./gcc/Makefile ]; then \ > - rm -rf gcc/tmp-include; \ > - mv gcc/include gcc/tmp-include 2>/dev/null; \ > - mkdir gcc/include; \ > - cp $(srcdir)/gcc/gsyslimits.h gcc/include/syslimits.h; \ > - touch gcc/stmp-fixinc gcc/include/fixed; \ > - rm -f gcc/stmp-headers gcc/stmp-int-hdrs; \ > r=`${PWD_COMMAND}`; export r; \ > ! s=`cd $(srcdir); ${PWD_COMMAND}` ; export s; \ > $(HOST_EXPORTS) \ > ! (cd ./gcc && \ > !$(MAKE) $(GCC_FLAGS_TO_PASS) install); \ > ! rm -rf gcc/include; \ > ! mv gcc/tmp-include gcc/include 2>/dev/null; \ > else true; fi > @endif gcc > > --- 1444,1459 > check-[+language+]: check-gcc-[+language+][+ FOR lib-check-target +] [+ > lib-check-target +][+ ENDFOR lib-check-target +] > [+ ENDFOR languages +] > > ! # The gcc part of install-no-fixedincludes, which relies on an intimate > ! # knowledge of how a number of gcc internal targets (inter)operate. > Delegate. > ! .PHONY: gcc-install-no-fixedincludes > ! gcc-install-no-fixedincludes: > @if [ -f ./gcc/Makefile ]; then \ > r=`${PWD_COMMAND}`; export r; \ > ! s=`cd $(srcdir); ${PWD_COMMAND}`; export s; \ > $(HOST_EXPORTS) \ > ! (cd ./gcc \ > !&& $(MAKE) $(GCC_FLAGS_TO_PASS) install-no-fixedincludes); \ > else true; fi > @endif gcc > > *** /tmp/jqbGTw_Makefile.in 2012-04-05 14:37:34.157109608 +0200 > --- gcc/Makefile.in 2012-04-04 22:03:53.823228924 +0200 > *** gcov-dump$(exeext): $(GCOV_DUMP_OBJS) $( > *** 4013,4022 > --- 4013,4028 > # Build the include directories. > stmp-int-hdrs: $(STMP_FIXINC) $(USER_H) fixinc_list > # Copy in the headers provided with gcc. > + # > # The sed command gets just the last file name component; > # this is necessary because VPATH could add a dirname. > # Using basename would be simpler, but some systems don't have it. > + # > # The touch command is here to workaround an AIX/Linux NFS bug. > + # > + # The move-if-change + cp -p twists for limits.h are intended to preserve > + # the time stamp when we regenerate, to prevent pointless rebuilds during > + # e.g. install-no-fixedincludes. > -if [ -d include ] ; then true; else mkdir include; chmod a+rx include; > fi > -if [ -d include-fixed ] ; then true; else mkdir include-fixed; chmod > a+rx include-fixed; fi > for file in .. $(USER_H); do \ > *** stmp-int-hdrs: $(STMP_FIXINC) $(USER_H) > *** 4065,4072 > fi; \ > $(mkinstalldirs) $${fix_dir}; \ > chmod a+rx $${fix_dir} || true; \ > rm -f $${fix_dir}/limits.h; \ > ! mv tmp-xlimits.h $${fix_dir}/limits.h; \ > chmod a+r $${fix_dir}/limits.h; \ > done > # Install the README > --- 4071,4080 > fi; \ > $(mkinstalldirs) $${fix_dir}; \ > chmod a+rx $${fix_dir} || true; \ > + $(SHELL) $(srcdir)/../move-if-change \ > + tmp-xlimits.h tmp-limits.h; \ > rm -f $${fix_dir}/limits.h; \ > ! cp -p tmp-limits.h $${fix_dir}/limits.h; \ > chmod a+r $${fix_dir}/limits.h; \ > done > # Install the README > *** stmp-fixinc: gsyslimits.h macro_list fix > *** 4168,4173 > --- 4176,4225 > fi > $(STAMP) stmp-fixinc > # > + > + # Install with the gcc headers files, not the fixed include files, which we > + # are typically not allowed to distribute. The general idea
Re: [Patch, libgfortran] Pass mode in "open" for O_CREAT and on VxWorks
Dear Janne, On 05/16/2012 08:45 AM, Janne Blomqvist wrote: IMHO it would be cleaner if you instead somewhere in the beginning of unix.c did #ifdef __VXWORKS__ /* open is not a variadic function on vxworks (or something...) */ #define open(path, flags) open(path, flags, 0666) #endif Untested, but AFAICS it should work (unless I'm missing something wrt macro expansion, which of course is quite possible). Testing shows that CPP does not like it: $ cpp #define a(x,y) a(x,y,0666) a(1,2) a(1,2,3) # 1 "" # 1 "" # 1 "" a(1,2,0666) :3:8: error: macro "a" passed 3 arguments, but takes just 2 a The patch was build on x86-64-linux, but I only expect an effect on VxWorks and on systems without mkstemp. OK for the trunk? (Is there a need for backporting?) Ok with the above change. I suspect there isn't that much demand for gfortran on vxworks, but the patch isn't intrusive either so if you want to backport it go ahead. There might be not much demand, but it *is* being used on VxWorks, thus, it potentially affects real users. However, I was told that VxWorks 6.6 *does* have access() [1] and it is defined in unistd.h of 6.3. (Recall that fallback_access does not get complied if HAS_ACCESS is true.) He didn't know whether it is available in all versions of VxWorks, however. Additionally, he has heard about gfortran issues (but has no experience with them); and he promised to compile everything to see whether everything works. [1] http://www-ad.fnal.gov/controls/micro_p/manuals/vxworks_application_api_reference_6.6.pdf In light of the CPP issue and the HAS_ACCESAS: Shall I only commit the second part or also the VxWorks part? Tobias
Re: [PATCH][ARM] Improve 64-bit shifts (non-NEON)
> > extern const struct tune_params *current_tune; > extern int vfp3_const_double_for_fract_bits (rtx); > + > +extern void arm_emit_coreregs_64bit_shift (enum rtx_code, rtx, rtx, rtx, rtx, > +rtx); > #endif /* RTX_CODE */ > #endif /* ! GCC_ARM_PROTOS_H */ > diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c > index c3a19e4..02dc6ca 100644 > --- a/gcc/config/arm/arm.c > +++ b/gcc/config/arm/arm.c > @@ -25213,5 +25213,206 @@ vfp3_const_double_for_fract_bits (rtx operand) >return 0; > } > +/* The default expansion of general 64-bit shifts in core-regs is suboptimal > + on ARM, since we know that shifts by negative amounts are no-ops. > + > + It's safe for the input and output to be the same register, but > + early-clobber rules apply for the shift amount and scratch registers. > + > + Shift by register requires both scratch registers. Shift by a constant > + less than 32 in Thumb2 mode requires SCRATCH1 only. In all other cases > + the scratch registers may be NULL. > + > + Additionally, ashiftrt by a register also clobbers the CC register. */ > +void > +arm_emit_coreregs_64bit_shift (enum rtx_code code, rtx out, rtx in, > +rtx amount, rtx scratch1, rtx scratch2) > +{ > + rtx out_high = gen_highpart (SImode, out); > + rtx out_low = gen_lowpart (SImode, out); > + rtx in_high = gen_highpart (SImode, in); > + rtx in_low = gen_lowpart (SImode, in); > + > + /* Bits flow from up-stream to down-stream. */ Some thing more about "upstream" and "downstream" here would be nice :) > + rtx out_up = code == ASHIFT ? out_low : out_high; > + rtx out_down = code == ASHIFT ? out_high : out_low; > + rtx in_up = code == ASHIFT ? in_low : in_high; > + rtx in_down = code == ASHIFT ? in_high : in_low; > + > + gcc_assert (code == ASHIFT || code == ASHIFTRT || code == LSHIFTRT); > + gcc_assert (out > + && (REG_P (out) || GET_CODE (out) == SUBREG) > + && GET_MODE (out) == DImode); > + gcc_assert (in > + && (REG_P (in) || GET_CODE (in) == SUBREG) > + && GET_MODE (in) == DImode); > + gcc_assert (amount > + && (((REG_P (amount) || GET_CODE (amount) == SUBREG) > +&& GET_MODE (amount) == SImode) > + || CONST_INT_P (amount))); > + gcc_assert (scratch1 == NULL > + || (GET_CODE (scratch1) == SCRATCH) > + || (GET_MODE (scratch1) == SImode > + && REG_P (scratch1))); > + gcc_assert (scratch2 == NULL > + || (GET_CODE (scratch2) == SCRATCH) > + || (GET_MODE (scratch2) == SImode > + && REG_P (scratch2))); > + gcc_assert (!REG_P (out) || !REG_P (amount) > + || !HARD_REGISTER_P (out) > + || (REGNO (out) != REGNO (amount) > + && REGNO (out) + 1 != REGNO (amount))); > + > + /* Macros to make following code more readable. */ > + #define SUB_32(DEST,SRC) \ > + gen_addsi3 ((DEST), (SRC), gen_rtx_CONST_INT (VOIDmode, -32)) > + #define RSB_32(DEST,SRC) \ > + gen_subsi3 ((DEST), gen_rtx_CONST_INT (VOIDmode, 32), (SRC)) > + #define SUB_S_32(DEST,SRC) \ > + gen_addsi3_compare0 ((DEST), (SRC), \ > + gen_rtx_CONST_INT (VOIDmode, -32)) > + #define SET(DEST,SRC) \ > + gen_rtx_SET (SImode, (DEST), (SRC)) > + #define SHIFT(CODE,SRC,AMOUNT) \ > + gen_rtx_fmt_ee ((CODE), SImode, (SRC), (AMOUNT)) > + #define LSHIFT(CODE,SRC,AMOUNT) \ > + gen_rtx_fmt_ee ((CODE) == ASHIFT ? ASHIFT : LSHIFTRT, \ > + SImode, (SRC), (AMOUNT)) > + #define REV_LSHIFT(CODE,SRC,AMOUNT) \ > + gen_rtx_fmt_ee ((CODE) == ASHIFT ? LSHIFTRT : ASHIFT, \ > + SImode, (SRC), (AMOUNT)) > + #define ORR(A,B) \ > + gen_rtx_IOR (SImode, (A), (B)) > + #define BRANCH(COND,LABEL) \ > + gen_arm_cond_branch ((LABEL), \ > + gen_rtx_ ## COND (CCmode, cc_reg, \ > +const0_rtx), \ > + cc_reg) > + > + if (CONST_INT_P (amount)) > +{ > + /* Shifts by a constant amount. */ > + if (INTVAL (amount) <= 0) > + /* Match what shift-by-register would do. */ > + emit_insn (gen_movdi (out, in)); > + else if (INTVAL (amount) >= 64) > + { > + /* Match what shift-by-register would do. */ > + if (code == ASHIFTRT) > + { > + rtx const31_rtx = gen_rtx_CONST_INT (VOIDmode, 31); > + emit_insn (SET (out_down, SHIFT (code, in_up, const31_rtx))); > + emit_insn (SET (out_up, SHIFT (code, in_up, const31_rtx))); > + } > + else > + emit_insn (gen_movdi (out, const0_rtx)); > + } > + else if (INTVAL (amount) < 32) > + { > + /* Shifts by a constant less than 32. */ > + rtx reverse_amount = gen_rtx_CONST_INT (VOIDmode, > +
Re: ping: Use "sed -n …" instead of "sed s/…/p -e d" in s-header-vars
On May 16, 2012, at 11:47 , Paolo Bonzini wrote: >> install-no-fixedincludes: installdirs install-host-nogcc \ >> !install-target gcc-install-no-fixedincludes ... > This is missing in the ChangeLog. Indeed. >> + install-no-fixedincludes: \ >> +stash-maybefixed-headers \ >> +stmp-int-hdrs \ >> +restore-header-stamps \ >> +install \ >> +restore-maybefixed-headers > > This is not safe for parallel build. Ah, right. > What about > install-no-fixedincludes: > ... contents of stash-maybefixed-headers ... > $(MAKE) $(FLAGS_TO_PASS) stmp-int-hdrs > ... contents of restore-header-stamps ... > $(MAKE) $(FLAGS_TO_PASS) install > ... contents of restore-maybefixed-headers ... > > ? Hmm, fine with me. I liked the use of separate targets to help provide a high level view of what the sequence is doing but each step is short enough for the inline inclusion not to damage readability. I'll post a followup patch+ChangeLog on the original thread. Thanks much for your prompt feedback :) Olivier
Re: [C++ Patch] PR 44516
Hi, On 05/16/2012 06:02 AM, Jason Merrill wrote: On 05/15/2012 07:56 PM, Paolo Carlini wrote: But, speaking of incremental work: what if, post the build_min_nt_loc chunk, we handle build_min_non_dep and build_min in a case by case way? Thus we keep around the non-_loc variant and gradually replace each call? With testcases (small!) checking columns too. That sounds fine. For sure *many* build_min_non_dep_loc and build_min_loc can be introduced without negative and with many positive effects on the testsuite. Finally, I'm attaching my latest build_min_nt_loc patch. Turns out, in the light of the better understanding I now have, I can't justify most of the EXPR_LOC_OR_HERE, outside those for build_x_modify_expr which you explicitly indicated. I think I was wrong when I indicated that, and that EXPR_LOCATION is better there, too. EXPR_LOC_OR_HERE is good for error messages, but not for setting the location of a tree. Though it occurs to me that we're likely to use the passed in location for errors as well, so we had better make sure we're passing in a useful location. Ok. Let's see if now I understand. This is what I did: 1- All the build_x_* callers pass EXPR_LOCATIONs (which can be UNKNOWN_LOCATION) 2- Inside the build_x_*, the passed in location is used as-is in template context to call build_min_nt_loc, and through a new LOC_OR_HERE to call build_new_op and the like (which need something meaningful for the error messages). Oh my.. at the risk of having to send another iteration, which would be identical to this one but without the LOC_OR_HERE thingy, isn't the diagnostic machinery able to cope with UNKNOWN_LOCATION? By default should be interpreted as input_location, no? Tested x86_64-linux (I added column info to an existing testcase) Thanks, Paolo. 2012-05-16 Paolo Carlini PR c++/44516 * tree.h (LOC_OR_HERE): Add. gcc/cp 2012-05-16 Paolo Carlini PR c++/44516 * typeck.c (build_x_array_ref, build_x_conditional_expr, build_x_compound_expr, build_x_modify_expr): Add location_t parameter, use build_min_nt_loc and LOC_OR_HERE. (finish_class_member_access_expr, build_x_indirect_ref, build_x_unary_op, build_x_binary_op): Use build_min_nt_loc and LOC_OR_HERE. (build_x_compound_expr_from_list, build_x_compound_expr_from_vec): Adjust callers. * tree.c (build_min_nt_loc): New. (build_min_nt): Remove. * typeck2.c (build_x_arrow): Use build_min_nt_loc and LOC_OR_HERE. * pt.c (tsubst_qualified_id): Use build_min_nt_loc. (tsubst_omp_for_iterator, tsubst_copy_and_build): Adjust calls. * semantics.c (finish_mem_initializers, handle_omp_for_class_iterator, finish_omp_atomic): Likewise. * decl2.c (grok_array_decl): Use build_min_nt_loc and LOC_OR_HERE. (build_anon_union_vars): Use build_min_nt_loc. * parser.c (cp_parser_question_colon_clause, cp_parser_assignment_expression, cp_parser_expression, cp_parser_template_id, cp_parser_omp_for_loop): Use build_min_nt_loc, adjust calls. * cp-tree.h: Update. gcc/testsuite 2012-05-16 Paolo Carlini PR c++/44516 * g++.dg/parse/error48.C: New. * g++.dg/template/crash89.C: Adjust dg-error line numbers. * g++.old-deja/g++.robertl/eb109.C: Add column info to dg-error string. libstdc++ 2012-05-16 Paolo Carlini PR c++/44516 * testsuite/20_util/ratio/cons/cons_overflow_neg.cc: Adjust dg-error line number. Index: libstdc++-v3/testsuite/20_util/ratio/cons/cons_overflow_neg.cc === --- libstdc++-v3/testsuite/20_util/ratio/cons/cons_overflow_neg.cc (revision 187559) +++ libstdc++-v3/testsuite/20_util/ratio/cons/cons_overflow_neg.cc (working copy) @@ -2,7 +2,7 @@ // { dg-options "-std=gnu++0x" } // { dg-require-cstdint "" } -// Copyright (C) 2008, 2009, 2010, 2011 Free Software Foundation +// Copyright (C) 2008, 2009, 2010, 2011, 2012 Free Software Foundation // // This file is part of the GNU ISO C++ Library. This library is free // software; you can redistribute it and/or modify it under the @@ -51,5 +51,5 @@ test04() // { dg-error "required from here" "" { target *-*-* } 46 } // { dg-error "denominator cannot be zero" "" { target *-*-* } 265 } // { dg-error "out of range" "" { target *-*-* } 266 } -// { dg-error "overflow in constant expression" "" { target *-*-* } 61 } +// { dg-error "overflow in constant expression" "" { target *-*-* } 62 } // { dg-prune-output "not a member" } Index: gcc/tree.h === --- gcc/tree.h (revision 187552) +++ gcc/tree.h (working copy) @@ -1695,6 +1695,9 @@ struct GTY(()) tree_constructor { location. */ #define CAN_HAVE_LOCATION_P(NODE) ((NODE) && EXPR_P (NODE)) +#define LOC_OR_HERE
Re: [Patch, libgfortran] Pass mode in "open" for O_CREAT and on VxWorks
On Wed, May 16, 2012 at 1:03 PM, Tobias Burnus wrote: > Dear Janne, > > > On 05/16/2012 08:45 AM, Janne Blomqvist wrote: >> >> IMHO it would be cleaner if you instead somewhere in the beginning of >> unix.c did >> >> #ifdef __VXWORKS__ >> /* open is not a variadic function on vxworks (or something...) */ >> #define open(path, flags) open(path, flags, 0666) >> #endif >> >> Untested, but AFAICS it should work (unless I'm missing something wrt >> macro expansion, which of course is quite possible). > > > Testing shows that CPP does not like it: > > $ cpp > #define a(x,y) a(x,y,0666) > a(1,2) > a(1,2,3) > # 1 "" > # 1 "" > # 1 "" > > a(1,2,0666) > :3:8: error: macro "a" passed 3 arguments, but takes just 2 Ah, bummer. We have something roughly similiar for snprintf (see libgfortran.h), but it seems that it works slightly differently due to using a variadic macro etc. So it seems this idea will not work, sorry. > However, I was told that VxWorks 6.6 *does* have access() [1] and it is > defined in unistd.h of 6.3. (Recall that fallback_access does not get > complied if HAS_ACCESS is true.) He didn't know whether it is available in > all versions of VxWorks, however. Additionally, he has heard about gfortran > issues (but has no experience with them); and he promised to compile > everything to see whether everything works. > > [1] > http://www-ad.fnal.gov/controls/micro_p/manuals/vxworks_application_api_reference_6.6.pdf Interestingly, according to that document open() also has the POSIXly correct varargs prototype. So apparently the old 3-argument prototype was used only in some older vxworks version? Since the document above is from 2007, one wonders how long ago did the prototype change, and how much new development is done on these older vxworks versions? > In light of the CPP issue and the HAS_ACCESAS: Shall I only commit the > second part or also the VxWorks part? IMHO lets do only the second part now, and wait for someone with vxworks experience to pop in and explain what changes are necessary, and which vxworks versions are worth considering to support. -- Janne Blomqvist
Re: [C++ Patch] PR 44516
On 05/16/2012 12:54 PM, Paolo Carlini wrote: I think I was wrong when I indicated that, and that EXPR_LOCATION is better there, too. EXPR_LOC_OR_HERE is good for error messages, but not for setting the location of a tree. Though it occurs to me that we're likely to use the passed in location for errors as well, so we had better make sure we're passing in a useful location. Ok. Let's see if now I understand. This is what I did: 1- All the build_x_* callers pass EXPR_LOCATIONs (which can be UNKNOWN_LOCATION) 2- Inside the build_x_*, the passed in location is used as-is in template context to call build_min_nt_loc, and through a new LOC_OR_HERE to call build_new_op and the like (which need something meaningful for the error messages). Oh my.. at the risk of having to send another iteration, which would be identical to this one but without the LOC_OR_HERE thingy, isn't the diagnostic machinery able to cope with UNKNOWN_LOCATION? By default should be interpreted as input_location, no? On the other hand - in case this is what you are thinking - I don't believe we should try to surrogate an UNKNOWN location coming from upstream with input_location also for the purpose of calling build_min_nt_loc in template context. I can easily imagine cases where it would be wrong, like we are parsing a binary operation, thus we can only be past the second operand with the input_location when we call build_x_binary_op, but the location we would like to pass to build_min_nt_loc is that of the *operator*! Thus at this point in our location work I don't believe we can do much (in terms of simple obvious changes) for UNKNOWN_LOCATIONs coming from upstream, besides maybe using the LOC_OR_HERE trick for the immediate error messages (but maybe isn't really necessary). Paolo.
Re: PING: [PATCH] Fix PR53217
On Wed, 2012-05-16 at 11:45 +0200, Richard Guenther wrote: > On Tue, 15 May 2012, William J. Schmidt wrote: > > > Ping. > > I don't like it too much - but pondering a bit over it I can't find > a nicer solution. > > So, ok. > > Thanks, > Richard. > Agreed. I'm not fond of it either, and I feel it's a bit fragile. An alternative would be to go back to handling the exponentiation expressions outside of the ops list (generating an explicit multiply to hook them up with the results of normal linear/parallel expansion). In hindsight, placing the exponentiation results in the ops list and letting the rank order handle things introduces some complexity as well as saving some. The DAG'd nature of the exponentiation expressions isn't a perfect fit for the pure tree form of the reassociated multiplies. Let me know if you'd like me to pursue that instead. Thanks, Bill > > Thanks, > > Bill > > > > On Tue, 2012-05-08 at 22:04 -0500, William J. Schmidt wrote: > > > This fixes another statement-placement issue when reassociating > > > expressions with repeated factors. Multiplies feeding into > > > __builtin_powi calls were not getting placed properly ahead of them in > > > some cases. > > > > > > Bootstrapped and tested on powerpc64-unknown-linux-gnu with no new > > > regressions. I've also run SPEC cpu2006 with no build or correctness > > > issues. OK for trunk? > > > > > > Thanks, > > > Bill > > > > > > > > > gcc: > > > > > > 2012-05-08 Bill Schmidt > > > > > > PR tree-optimization/53217 > > > * tree-ssa-reassoc.c (bip_map): New static variable. > > > (possibly_move_powi): Move feeding multiplies with __builtin_powi call. > > > (attempt_builtin_powi): Save feeding multiplies on a stack. > > > (reassociate_bb): Create and destroy bip_map. > > > > > > gcc/testsuite: > > > > > > 2012-05-08 Bill Schmidt > > > > > > PR tree-optimization/53217 > > > * gfortran.dg/pr53217.f90: New test. > > > > > > > > > Index: gcc/testsuite/gfortran.dg/pr53217.f90 > > > === > > > --- gcc/testsuite/gfortran.dg/pr53217.f90 (revision 0) > > > +++ gcc/testsuite/gfortran.dg/pr53217.f90 (revision 0) > > > @@ -0,0 +1,28 @@ > > > +! { dg-do compile } > > > +! { dg-options "-O1 -ffast-math" } > > > + > > > +! This tests only for compile-time failure, which formerly occurred > > > +! when statements were emitted out of order, failing verify_ssa. > > > + > > > +MODULE xc_cs1 > > > + INTEGER, PARAMETER :: dp=KIND(0.0D0) > > > + REAL(KIND=dp), PARAMETER :: a = 0.04918_dp, & > > > + c = 0.2533_dp, & > > > + d = 0.349_dp > > > +CONTAINS > > > + SUBROUTINE cs1_u_2 ( rho, grho, r13, e_rho_rho, e_rho_ndrho, > > > e_ndrho_ndrho,& > > > + npoints, error) > > > +REAL(KIND=dp), DIMENSION(*), & > > > + INTENT(INOUT) :: e_rho_rho, e_rho_ndrho, & > > > +e_ndrho_ndrho > > > +DO ip = 1, npoints > > > + IF ( rho(ip) > eps_rho ) THEN > > > + oc = 1.0_dp/(r*r*r3*r3 + c*g*g) > > > + d2rF4 = c4p*f13*f23*g**4*r3/r * > > > (193*d*r**5*r3*r3+90*d*d*r**5*r3 & > > > + -88*g*g*c*r**3*r3-100*d*d*c*g*g*r*r*r3*r3 & > > > + +104*r**6)*od**3*oc**4 > > > + e_rho_rho(ip) = e_rho_rho(ip) + d2F1 + d2rF2 + d2F3 + d2rF4 > > > + END IF > > > +END DO > > > + END SUBROUTINE cs1_u_2 > > > +END MODULE xc_cs1 > > > Index: gcc/tree-ssa-reassoc.c > > > === > > > --- gcc/tree-ssa-reassoc.c(revision 187117) > > > +++ gcc/tree-ssa-reassoc.c(working copy) > > > @@ -200,6 +200,10 @@ static long *bb_rank; > > > /* Operand->rank hashtable. */ > > > static struct pointer_map_t *operand_rank; > > > > > > +/* Map from inserted __builtin_powi calls to multiply chains that > > > + feed them. */ > > > +static struct pointer_map_t *bip_map; > > > + > > > /* Forward decls. */ > > > static long get_rank (tree); > > > > > > @@ -2249,7 +2253,7 @@ remove_visited_stmt_chain (tree var) > > > static void > > > possibly_move_powi (gimple stmt, tree op) > > > { > > > - gimple stmt2; > > > + gimple stmt2, *mpy; > > >tree fndecl; > > >gimple_stmt_iterator gsi1, gsi2; > > > > > > @@ -2278,9 +2282,39 @@ possibly_move_powi (gimple stmt, tree op) > > >return; > > > } > > > > > > + /* Move the __builtin_powi. */ > > >gsi1 = gsi_for_stmt (stmt); > > >gsi2 = gsi_for_stmt (stmt2); > > >gsi_move_before (&gsi2, &gsi1); > > > + > > > + /* See if there are multiplies feeding the __builtin_powi base > > > + argument that must also be moved. */ > > > + while ((mpy = (gimple *) pointer_map_contains (bip_map, stmt2)) != > > > NULL) > > > +{ > > > + /* If we've already moved this statement, we're done. This is > > > + identified by a NULL entry for the
Re: PING: [PATCH] Fix PR53217
On Wed, 16 May 2012, William J. Schmidt wrote: > On Wed, 2012-05-16 at 11:45 +0200, Richard Guenther wrote: > > On Tue, 15 May 2012, William J. Schmidt wrote: > > > > > Ping. > > > > I don't like it too much - but pondering a bit over it I can't find > > a nicer solution. > > > > So, ok. > > > > Thanks, > > Richard. > > > Agreed. I'm not fond of it either, and I feel it's a bit fragile. > > An alternative would be to go back to handling the exponentiation > expressions outside of the ops list (generating an explicit multiply to > hook them up with the results of normal linear/parallel expansion). In > hindsight, placing the exponentiation results in the ops list and > letting the rank order handle things introduces some complexity as well > as saving some. The DAG'd nature of the exponentiation expressions > isn't a perfect fit for the pure tree form of the reassociated > multiplies. True. > Let me know if you'd like me to pursue that instead. You can try - if the result looks better I'm all for it ;) Thanks, Richard. > Thanks, > Bill > > > > Thanks, > > > Bill > > > > > > On Tue, 2012-05-08 at 22:04 -0500, William J. Schmidt wrote: > > > > This fixes another statement-placement issue when reassociating > > > > expressions with repeated factors. Multiplies feeding into > > > > __builtin_powi calls were not getting placed properly ahead of them in > > > > some cases. > > > > > > > > Bootstrapped and tested on powerpc64-unknown-linux-gnu with no new > > > > regressions. I've also run SPEC cpu2006 with no build or correctness > > > > issues. OK for trunk? > > > > > > > > Thanks, > > > > Bill > > > > > > > > > > > > gcc: > > > > > > > > 2012-05-08 Bill Schmidt > > > > > > > > PR tree-optimization/53217 > > > > * tree-ssa-reassoc.c (bip_map): New static variable. > > > > (possibly_move_powi): Move feeding multiplies with > > > > __builtin_powi call. > > > > (attempt_builtin_powi): Save feeding multiplies on a stack. > > > > (reassociate_bb): Create and destroy bip_map. > > > > > > > > gcc/testsuite: > > > > > > > > 2012-05-08 Bill Schmidt > > > > > > > > PR tree-optimization/53217 > > > > * gfortran.dg/pr53217.f90: New test. > > > > > > > > > > > > Index: gcc/testsuite/gfortran.dg/pr53217.f90 > > > > === > > > > --- gcc/testsuite/gfortran.dg/pr53217.f90 (revision 0) > > > > +++ gcc/testsuite/gfortran.dg/pr53217.f90 (revision 0) > > > > @@ -0,0 +1,28 @@ > > > > +! { dg-do compile } > > > > +! { dg-options "-O1 -ffast-math" } > > > > + > > > > +! This tests only for compile-time failure, which formerly occurred > > > > +! when statements were emitted out of order, failing verify_ssa. > > > > + > > > > +MODULE xc_cs1 > > > > + INTEGER, PARAMETER :: dp=KIND(0.0D0) > > > > + REAL(KIND=dp), PARAMETER :: a = 0.04918_dp, & > > > > + c = 0.2533_dp, & > > > > + d = 0.349_dp > > > > +CONTAINS > > > > + SUBROUTINE cs1_u_2 ( rho, grho, r13, e_rho_rho, e_rho_ndrho, > > > > e_ndrho_ndrho,& > > > > + npoints, error) > > > > +REAL(KIND=dp), DIMENSION(*), & > > > > + INTENT(INOUT) :: e_rho_rho, > > > > e_rho_ndrho, & > > > > +e_ndrho_ndrho > > > > +DO ip = 1, npoints > > > > + IF ( rho(ip) > eps_rho ) THEN > > > > + oc = 1.0_dp/(r*r*r3*r3 + c*g*g) > > > > + d2rF4 = c4p*f13*f23*g**4*r3/r * > > > > (193*d*r**5*r3*r3+90*d*d*r**5*r3 & > > > > + -88*g*g*c*r**3*r3-100*d*d*c*g*g*r*r*r3*r3 & > > > > + +104*r**6)*od**3*oc**4 > > > > + e_rho_rho(ip) = e_rho_rho(ip) + d2F1 + d2rF2 + d2F3 + d2rF4 > > > > + END IF > > > > +END DO > > > > + END SUBROUTINE cs1_u_2 > > > > +END MODULE xc_cs1 > > > > Index: gcc/tree-ssa-reassoc.c > > > > === > > > > --- gcc/tree-ssa-reassoc.c (revision 187117) > > > > +++ gcc/tree-ssa-reassoc.c (working copy) > > > > @@ -200,6 +200,10 @@ static long *bb_rank; > > > > /* Operand->rank hashtable. */ > > > > static struct pointer_map_t *operand_rank; > > > > > > > > +/* Map from inserted __builtin_powi calls to multiply chains that > > > > + feed them. */ > > > > +static struct pointer_map_t *bip_map; > > > > + > > > > /* Forward decls. */ > > > > static long get_rank (tree); > > > > > > > > @@ -2249,7 +2253,7 @@ remove_visited_stmt_chain (tree var) > > > > static void > > > > possibly_move_powi (gimple stmt, tree op) > > > > { > > > > - gimple stmt2; > > > > + gimple stmt2, *mpy; > > > >tree fndecl; > > > >gimple_stmt_iterator gsi1, gsi2; > > > > > > > > @@ -2278,9 +2282,39 @@ possibly_move_powi (gimple stmt, tree op) > > > >return; > > > > } > > > > > > > > + /* Move the __builtin_powi. */ > > > >gsi1 = gsi
Re: [Patch, libgfortran] Pass mode in "open" for O_CREAT and on VxWorks
On 05/16/2012 07:26 AM, Janne Blomqvist wrote: On Wed, May 16, 2012 at 1:03 PM, Tobias Burnus wrote: On 05/16/2012 08:45 AM, Janne Blomqvist wrote: IMHO it would be cleaner if you instead somewhere in the beginning of unix.c did #ifdef __VXWORKS__ /* open is not a variadic function on vxworks (or something...) */ #define open(path, flags) open(path, flags, 0666) #endif Untested, but AFAICS it should work (unless I'm missing something wrt macro expansion, which of course is quite possible). Testing shows that CPP does not like it: $ cpp #define a(x,y) a(x,y,0666) a(1,2) a(1,2,3) # 1 "" # 1 "" # 1 "" a(1,2,0666) :3:8: error: macro "a" passed 3 arguments, but takes just 2 Ah, bummer. We have something roughly similiar for snprintf (see libgfortran.h), but it seems that it works slightly differently due to using a variadic macro etc. So it seems this idea will not work, sorry. Actually, this works for me: $ cat test.c #include void a(int b, int c, int d) { printf("%i\t%i\t%i", b, c, d); } #define a(b, c) (a)(b, c, 3) int main() { a(1, 2); return 0; } $ gcc test.c -o a.out $ ./a.out 1 2 3 The significance is in the parentheses in the macro definition. However, I was told that VxWorks 6.6 *does* have access() [1] and it is defined in unistd.h of 6.3. (Recall that fallback_access does not get complied if HAS_ACCESS is true.) He didn't know whether it is available in all versions of VxWorks, however. Additionally, he has heard about gfortran issues (but has no experience with them); and he promised to compile everything to see whether everything works. [1] http://www-ad.fnal.gov/controls/micro_p/manuals/vxworks_application_api_reference_6.6.pdf Interestingly, according to that document open() also has the POSIXly correct varargs prototype. So apparently the old 3-argument prototype was used only in some older vxworks version? Since the document above is from 2007, one wonders how long ago did the prototype change, and how much new development is done on these older vxworks versions? In light of the CPP issue and the HAS_ACCESAS: Shall I only commit the second part or also the VxWorks part? IMHO lets do only the second part now, and wait for someone with vxworks experience to pop in and explain what changes are necessary, and which vxworks versions are worth considering to support. I'm using the VxWorks 6.3 headers. The reason for this is that these headers are available at [1], and that this is the version of VxWorks used for FRC. I can also say that these headers do NOT contain the POSIX-ly correct version of open(). Additionally, they do NOT have the POSIX-ly correct version of ioctl(), so something like the following is needed: #define ioctl(a, b, c) (ioctl)(a, b, (int)c) To explain FRC- I'm a high school student on a FIRST Robotics Competition team working on getting a full gcc4 toolchain, as the provided toolchain is gcc 3.4 and windows binaries . Please don't hold my age/experience (or lack thereof) against me. So if you're looking for users, there's probably around 100,000 of us high school students in FRC around the world [1]: ftp://ftp.ni.com/pub/devzone/tut/updated_vxworks63gccdist.zip Robert Mason
PATCH: Regenerate configure files
Hi, I am checking in this patch to regenerate configure files with the updated libtoo.m4. libjava/libltdl and libgo aren't updated since they have their own version of libtoo.m4. They should be updated separately. H.J. boehm-gc/ 2012-05-16 H.J. Lu * configure: Regenerated. gcc/ 2012-05-16 H.J. Lu * configure: Regenerated. libatomic/ 2012-05-16 H.J. Lu * configure: Regenerated. libffi/ 2012-05-16 H.J. Lu * configure: Regenerated. libgfortran/ 2012-05-16 H.J. Lu * configure: Regenerated. libgomp/ 2012-05-16 H.J. Lu * configure: Regenerated. libitm/ 2012-05-16 H.J. Lu * configure: Regenerated. libjava/classpath/ 2012-05-16 H.J. Lu * configure: Regenerated. libjava/ 2012-05-16 H.J. Lu * configure: Regenerated. libmudflap/ 2012-05-16 H.J. Lu * configure: Regenerated. libobjc/ 2012-05-16 H.J. Lu * configure: Regenerated. libquadmath/ 2012-05-16 H.J. Lu * configure: Regenerated. libssp/ 2012-05-16 H.J. Lu * configure: Regenerated. libstdc++-v3/ 2012-05-16 H.J. Lu * configure: Regenerated. lto-plugin/ 2012-05-16 H.J. Lu * configure: Regenerated. zlib/ 2012-05-16 H.J. Lu * configure: Regenerated. diff --git a/boehm-gc/configure b/boehm-gc/configure index c76ea44..aa61053 100755 --- a/boehm-gc/configure +++ b/boehm-gc/configure @@ -6786,7 +6786,14 @@ s390*-*linux*|s390*-*tpf*|sparc*-*linux*) LD="${LD-ld} -m elf_i386_fbsd" ;; x86_64-*linux*) - LD="${LD-ld} -m elf_i386" + case `/usr/bin/file conftest.o` in + *x86-64*) + LD="${LD-ld} -m elf32_x86_64" + ;; + *) + LD="${LD-ld} -m elf_i386" + ;; + esac ;; ppc64-*linux*|powerpc64-*linux*) LD="${LD-ld} -m elf32ppclinux" @@ -11304,7 +11311,7 @@ else lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2 lt_status=$lt_dlunknown cat > conftest.$ac_ext <<_LT_EOF -#line 11307 "configure" +#line 11314 "configure" #include "confdefs.h" #if HAVE_DLFCN_H @@ -11410,7 +11417,7 @@ else lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2 lt_status=$lt_dlunknown cat > conftest.$ac_ext <<_LT_EOF -#line 11413 "configure" +#line 11420 "configure" #include "confdefs.h" #if HAVE_DLFCN_H diff --git a/gcc/configure b/gcc/configure index 557a4cc7..af248e0 100755 --- a/gcc/configure +++ b/gcc/configure @@ -13746,7 +13746,14 @@ s390*-*linux*|s390*-*tpf*|sparc*-*linux*) LD="${LD-ld} -m elf_i386_fbsd" ;; x86_64-*linux*) - LD="${LD-ld} -m elf_i386" + case `/usr/bin/file conftest.o` in + *x86-64*) + LD="${LD-ld} -m elf32_x86_64" + ;; + *) + LD="${LD-ld} -m elf_i386" + ;; + esac ;; ppc64-*linux*|powerpc64-*linux*) LD="${LD-ld} -m elf32ppclinux" @@ -17960,7 +17967,7 @@ else lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2 lt_status=$lt_dlunknown cat > conftest.$ac_ext <<_LT_EOF -#line 17963 "configure" +#line 17970 "configure" #include "confdefs.h" #if HAVE_DLFCN_H @@ -18066,7 +18073,7 @@ else lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2 lt_status=$lt_dlunknown cat > conftest.$ac_ext <<_LT_EOF -#line 18069 "configure" +#line 18076 "configure" #include "confdefs.h" #if HAVE_DLFCN_H diff --git a/libatomic/configure b/libatomic/configure index a3b819d..1d63af4 100755 --- a/libatomic/configure +++ b/libatomic/configure @@ -6524,7 +6520,14 @@ s390*-*linux*|s390*-*tpf*|sparc*-*linux*) LD="${LD-ld} -m elf_i386_fbsd" ;; x86_64-*linux*) - LD="${LD-ld} -m elf_i386" + case `/usr/bin/file conftest.o` in + *x86-64*) + LD="${LD-ld} -m elf32_x86_64" + ;; + *) + LD="${LD-ld} -m elf_i386" + ;; + esac ;; ppc64-*linux*|powerpc64-*linux*) LD="${LD-ld} -m elf32ppclinux" @@ -11008,7 +11011,7 @@ else lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2 lt_status=$lt_dlunknown cat > conftest.$ac_ext <<_LT_EOF -#line 11011 "configure" +#line 11018 "configure" #include "confdefs.h" #if HAVE_DLFCN_H @@ -4,7 +7,7 @@ else lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2 lt_status=$lt_dlunknown cat > conftest.$ac_ext <<_LT_EOF -#line 7 "configure" +#line 11124 "configure" #include "confdefs.h" #if HAVE_DLFCN_H diff --git a/libffi/configure b/libffi/configure index 1591495..28ed513 100755 --- a/libffi/configure +++ b/libffi/configure @@ -6282,7 +6282,14 @@ s390*-*linux*|s390*-*tpf*|sparc*-*linux*) LD="${LD-ld} -m elf_i386_fbsd" ;; x86_64-*linux
Re: PING: [PATCH] Fix PR53217
On Wed, 2012-05-16 at 14:05 +0200, Richard Guenther wrote: > On Wed, 16 May 2012, William J. Schmidt wrote: > > > On Wed, 2012-05-16 at 11:45 +0200, Richard Guenther wrote: > > > On Tue, 15 May 2012, William J. Schmidt wrote: > > > > > > > Ping. > > > > > > I don't like it too much - but pondering a bit over it I can't find > > > a nicer solution. > > > > > > So, ok. > > > > > > Thanks, > > > Richard. > > > > > Agreed. I'm not fond of it either, and I feel it's a bit fragile. > > > > An alternative would be to go back to handling the exponentiation > > expressions outside of the ops list (generating an explicit multiply to > > hook them up with the results of normal linear/parallel expansion). In > > hindsight, placing the exponentiation results in the ops list and > > letting the rank order handle things introduces some complexity as well > > as saving some. The DAG'd nature of the exponentiation expressions > > isn't a perfect fit for the pure tree form of the reassociated > > multiplies. > > True. > > > Let me know if you'd like me to pursue that instead. > > You can try - if the result looks better I'm all for it ;) > OK. :) I'll commit this for now to deal with the fallout, and work on the alternative version in my spare time. Thanks, Bill > Thanks, > Richard. > > > Thanks, > > Bill > > > > > > Thanks, > > > > Bill > > > > > > > > On Tue, 2012-05-08 at 22:04 -0500, William J. Schmidt wrote: > > > > > This fixes another statement-placement issue when reassociating > > > > > expressions with repeated factors. Multiplies feeding into > > > > > __builtin_powi calls were not getting placed properly ahead of them in > > > > > some cases. > > > > > > > > > > Bootstrapped and tested on powerpc64-unknown-linux-gnu with no new > > > > > regressions. I've also run SPEC cpu2006 with no build or correctness > > > > > issues. OK for trunk? > > > > > > > > > > Thanks, > > > > > Bill > > > > > > > > > > > > > > > gcc: > > > > > > > > > > 2012-05-08 Bill Schmidt > > > > > > > > > > PR tree-optimization/53217 > > > > > * tree-ssa-reassoc.c (bip_map): New static variable. > > > > > (possibly_move_powi): Move feeding multiplies with > > > > > __builtin_powi call. > > > > > (attempt_builtin_powi): Save feeding multiplies on a stack. > > > > > (reassociate_bb): Create and destroy bip_map. > > > > > > > > > > gcc/testsuite: > > > > > > > > > > 2012-05-08 Bill Schmidt > > > > > > > > > > PR tree-optimization/53217 > > > > > * gfortran.dg/pr53217.f90: New test. > > > > > > > > > > > > > > > Index: gcc/testsuite/gfortran.dg/pr53217.f90 > > > > > === > > > > > --- gcc/testsuite/gfortran.dg/pr53217.f90 (revision 0) > > > > > +++ gcc/testsuite/gfortran.dg/pr53217.f90 (revision 0) > > > > > @@ -0,0 +1,28 @@ > > > > > +! { dg-do compile } > > > > > +! { dg-options "-O1 -ffast-math" } > > > > > + > > > > > +! This tests only for compile-time failure, which formerly occurred > > > > > +! when statements were emitted out of order, failing verify_ssa. > > > > > + > > > > > +MODULE xc_cs1 > > > > > + INTEGER, PARAMETER :: dp=KIND(0.0D0) > > > > > + REAL(KIND=dp), PARAMETER :: a = 0.04918_dp, & > > > > > + c = 0.2533_dp, & > > > > > + d = 0.349_dp > > > > > +CONTAINS > > > > > + SUBROUTINE cs1_u_2 ( rho, grho, r13, e_rho_rho, e_rho_ndrho, > > > > > e_ndrho_ndrho,& > > > > > + npoints, error) > > > > > +REAL(KIND=dp), DIMENSION(*), & > > > > > + INTENT(INOUT) :: e_rho_rho, > > > > > e_rho_ndrho, & > > > > > +e_ndrho_ndrho > > > > > +DO ip = 1, npoints > > > > > + IF ( rho(ip) > eps_rho ) THEN > > > > > + oc = 1.0_dp/(r*r*r3*r3 + c*g*g) > > > > > + d2rF4 = c4p*f13*f23*g**4*r3/r * > > > > > (193*d*r**5*r3*r3+90*d*d*r**5*r3 & > > > > > + -88*g*g*c*r**3*r3-100*d*d*c*g*g*r*r*r3*r3 & > > > > > + +104*r**6)*od**3*oc**4 > > > > > + e_rho_rho(ip) = e_rho_rho(ip) + d2F1 + d2rF2 + d2F3 + d2rF4 > > > > > + END IF > > > > > +END DO > > > > > + END SUBROUTINE cs1_u_2 > > > > > +END MODULE xc_cs1 > > > > > Index: gcc/tree-ssa-reassoc.c > > > > > === > > > > > --- gcc/tree-ssa-reassoc.c(revision 187117) > > > > > +++ gcc/tree-ssa-reassoc.c(working copy) > > > > > @@ -200,6 +200,10 @@ static long *bb_rank; > > > > > /* Operand->rank hashtable. */ > > > > > static struct pointer_map_t *operand_rank; > > > > > > > > > > +/* Map from inserted __builtin_powi calls to multiply chains that > > > > > + feed them. */ > > > > > +static struct pointer_map_t *bip_map; > > > > > + > > > > > /* Forward decls. */ > > > > > static long get_rank (tree); > > > > > > > > > > @@ -2249,7 +2253,7 @@ remove_visited_stmt_
Re: [C++ Patch] PR 44516
On 05/16/2012 01:25 PM, Paolo Carlini wrote: Thus at this point in our location work I don't believe we can do much (in terms of simple obvious changes) for UNKNOWN_LOCATIONs coming from upstream, besides maybe using the LOC_OR_HERE trick for the immediate error messages (but maybe isn't really necessary). I'm attaching the version of the patch not using it, which also passes testing. Paolo. /// gcc/cp 2012-05-16 Paolo Carlini PR c++/44516 * typeck.c (build_x_array_ref, build_x_conditional_expr, build_x_compound_expr, build_x_modify_expr): Add location_t parameter. (finish_class_member_access_expr, build_x_indirect_ref, build_x_binary_op, build_x_compound_expr_from_list, build_x_compound_expr_from_vec): Adjust callers. * tree.c (build_min_nt_loc): New. (build_min_nt): Remove. * typeck2.c (build_x_arrow): Adjust callers. * pt.c (tsubst_qualified_id, tsubst_omp_for_iterator, tsubst_copy_and_build): Likewise. * semantics.c (finish_mem_initializers, handle_omp_for_class_iterator, finish_omp_atomic): Likewise. * decl2.c (grok_array_decl, build_anon_union_vars): Adjust. * parser.c (cp_parser_question_colon_clause, cp_parser_assignment_expression, cp_parser_expression, cp_parser_template_id, cp_parser_omp_for_loop): Likewise. * cp-tree.h: Update. gcc/testsuite 2012-05-16 Paolo Carlini PR c++/44516 * g++.dg/parse/error48.C: New. * g++.dg/template/crash89.C: Adjust dg-error line numbers. * g++.old-deja/g++.robertl/eb109.C: Add column info to dg-error string. libstdc++ 2012-05-16 Paolo Carlini PR c++/44516 * testsuite/20_util/ratio/cons/cons_overflow_neg.cc: Adjust dg-error line number. Index: libstdc++-v3/testsuite/20_util/ratio/cons/cons_overflow_neg.cc === --- libstdc++-v3/testsuite/20_util/ratio/cons/cons_overflow_neg.cc (revision 187559) +++ libstdc++-v3/testsuite/20_util/ratio/cons/cons_overflow_neg.cc (working copy) @@ -2,7 +2,7 @@ // { dg-options "-std=gnu++0x" } // { dg-require-cstdint "" } -// Copyright (C) 2008, 2009, 2010, 2011 Free Software Foundation +// Copyright (C) 2008, 2009, 2010, 2011, 2012 Free Software Foundation // // This file is part of the GNU ISO C++ Library. This library is free // software; you can redistribute it and/or modify it under the @@ -51,5 +51,5 @@ test04() // { dg-error "required from here" "" { target *-*-* } 46 } // { dg-error "denominator cannot be zero" "" { target *-*-* } 265 } // { dg-error "out of range" "" { target *-*-* } 266 } -// { dg-error "overflow in constant expression" "" { target *-*-* } 61 } +// { dg-error "overflow in constant expression" "" { target *-*-* } 62 } // { dg-prune-output "not a member" } Index: gcc/testsuite/g++.old-deja/g++.robertl/eb109.C === --- gcc/testsuite/g++.old-deja/g++.robertl/eb109.C (revision 187551) +++ gcc/testsuite/g++.old-deja/g++.robertl/eb109.C (working copy) @@ -44,16 +44,16 @@ ostream& operator<<(ostream& os, Graph::Successor::iterator - startN = G[i].second.begin(), // { dg-error "no match" } no index operator - endN = G[i].second.end(); // { dg-error "no match" } no index operator + startN = G[i].second.begin(), // { dg-error "14:no match" } no index operator + endN = G[i].second.end(); // { dg-error "14:no match" } no index operator while(startN != endN) { -os << G[(*startN).first].first << ' ' // { dg-error "no match" } no index operator +os << G[(*startN).first].first << ' ' // { dg-error "20:no match" } no index operator << (*startN).second << ' '; ++startN; } Index: gcc/testsuite/g++.dg/parse/error48.C === --- gcc/testsuite/g++.dg/parse/error48.C(revision 0) +++ gcc/testsuite/g++.dg/parse/error48.C(revision 0) @@ -0,0 +1,10 @@ +// PR c++/44516 + +struct WebService { }; +struct Server { }; + +void addHTTPService(Server const &server, + WebService const *http) +{ + server += http; // { dg-error "10:no match for 'operator\\+='" } +} Index: gcc/testsuite/g++.dg/template/crash89.C === --- gcc/testsuite/g++.dg/template/crash89.C (revision 187559) +++ gcc/testsuite/g++.dg/template/crash89.C (working copy) @@ -1,8 +1,8 @@ // PR c++/34397 -template struct A +template struct A // { dg-error "subscripted|template" } { typedef A B; }; -A a; // { dg-error "subscripted|template|declaration" } +A a; // { dg-error "declaration" } Index: gcc/cp/typeck.c === --- gcc/cp/typ
Re: remove TARGET_E500 and factorize SPE defaults computation in powerpc ports
Hello, since you touch the SPE area would you mind looking at this PR: http://gcc.gnu.org/bugzilla/show_bug.cgi?id=47856 -- Sebastian Huber, embedded brains GmbH Address : Obere Lagerstr. 30, D-82178 Puchheim, Germany Phone : +49 89 18 90 80 79-6 Fax : +49 89 18 90 80 79-9 E-Mail : sebastian.hu...@embedded-brains.de PGP : Public key available on request. Diese Nachricht ist keine geschäftliche Mitteilung im Sinne des EHUG.
Re: Turn check macros into functions. (issue6188088)
On 12-05-16 05:41 , Richard Guenther wrote: What's the reason for templating these functions? They all take trees as parameter!? True. I don't recall what Lawrence had in mind, but I remember that by using templates here, you don't need to deal with the mess of distinguishing tree from const_tree, and having to force constness out with ugly casts. Additionally, templates are producing slightly smaller code than the non-template variant (about 0.2% smaller). I'm not actually sure why this happens, but it's consistent across all binaries. Diego.
PING PATCH: break lines in announce_function
Hello All, I am pinging the patch http://gcc.gnu.org/ml/gcc-patches/2012-02/msg00474.html below for trunk svn 187587 # patch Index: gcc/toplev.c === --- gcc/toplev.c(revision 187587) +++ gcc/toplev.c(working copy) @@ -229,6 +229,12 @@ { if (!quiet_flag) { + + static long count; + count++; + if (count % 8 == 0) +putc('\n', stderr); + if (rtl_dump_and_exit) fprintf (stderr, "%s ", identifier_to_locale (IDENTIFIER_POINTER (DECL_NAME (decl; ### gcc/ChangeLog entry 2011-05-16 Basile Starynkevitch * toplev.c (announce_function): Output newline periodically. ### Ok for trunk? Cheers -- Basile STARYNKEVITCH http://starynkevitch.net/Basile/ email: basilestarynkevitchnet mobile: +33 6 8501 2359 8, rue de la Faiencerie, 92340 Bourg La Reine, France *** opinions {are only mines, sont seulement les miennes} ***
[Testsuite,committed] Fix some tests for 16-bit integers
Committed to fix some fallout from testsuite. http://gcc.gnu.org/viewcvs?view=revision&revision=187588 PR testsuite/52641 * gcc.dg/pr52549.c: Fix test for long != void* * gcc.c-torture/execute/pr52979-1.x: New file. * gcc.c-torture/execute/pr52979-2.x: New file.
Re: [PR tree-optimization/52558]: RFC: questions on store data race
On Mon, 7 May 2012, Aldy Hernandez wrote: > Hi. Sorry for the delay. There were various tricky hiccups along the way to > bootstrappability and regression cleanliness... > > On 04/26/12 04:51, Richard Guenther wrote: > > On Wed, 25 Apr 2012, Aldy Hernandez wrote: > > > > > On 04/25/12 06:45, Richard Guenther wrote: > > > > On Tue, Apr 24, 2012 at 7:43 PM, Aldy Hernandez > > > > wrote: > > > > > On 04/13/12 03:46, Richard Guenther wrote: > > > > > > > > > > > > On Fri, Apr 13, 2012 at 12:11 AM, Aldy Hernandez > > > > > > wrote: > > > > Speak of loads, I am keeping the information as an additional bitmap in > > > `memory_accesses', as ->refs_in_loop was set for stores as well, so I > > > couldn't > > > depend on it. Let me know if you have another idea. > > > > Hmm, refs_in_loop& ~all_refs_stored_in_loop, so instead of > > > > + bitmap reads = VEC_index (bitmap, memory_accesses.reads_in_loop, > > + loop->num); > > + ref_read_in_loop_p = bitmap_bit_p (reads, ref->id); > > > > ref_read_in_loop_p = bitmap_bit_p (refs, ref->id)&& !bitmap_bit_p > > (stores, ref->id); > > > > ? But maybe that doesn't work if a ref is both read and stored to. > > Btw, rather than adding a bitmap to memory_accesses I'd rather add > > a mark_ref_loaded corresponding to mark_ref_stored (or rather merge > > both into one) and a bitmap to struct mem_ref. > > I have done so as mark_ref_loaded(). I first tried to merge both into a > generic mark_ref(), to avoid iterating twice, but I was concerned with the > early loop exit of !bitmap_bit_p(ref->stored, loop->num), not knowing if I > should exit the loop altogether in the merged case or what. In either case, > the code was somewhat convoluted, so I opted for a separate clean function. > > In scanning the gimple stream for the loads I noticed that instances of two > component refs (say foo.bar) were not pointer comparable, so I am asking the > alias oracle with refs_may_alias_p. It also seemed to make more sense wrt > memory chunks that may alias. What do you think? No, it asks for equal must aliases (it will replace them after all!). You should use operand_equal_p to compare reference trees. But why do all the following dance + + if (gimple_assign_single_p (stmt)) +{ + tree t = gimple_assign_rhs1 (stmt); + /* ?? For some reason two exact COMPONENT_REFs cannot be +compared with pointer equality, so ask the alias oracle. */ + if (ref->mem == t + || ((TREE_CODE (t) == SSA_NAME + || DECL_P (t) + || handled_component_p (t) + || TREE_CODE (t) == MEM_REF + || TREE_CODE (t) == TARGET_MEM_REF) + && refs_may_alias_p (t, ref->mem))) + mark_ref_loaded (ref, loop); +} at all and not just if (is_stored) mark_ref_stored (ref, loop); else mark_ref_loaded (ref, loop); and similar in the !mem case mark the ref loaded unconditionally: mark_ref_loaded (ref, loop); if (gimple_vdef (stmt)) mark_ref_stored (ref, loop); > This patch is far more robust and fully tested. Two wrenches to complicate > things though... > > First, while inserting the code writing the _LSM_ temporary writes into the > original variables, I found a handful of tests where the order of the writes > mattered (aliased locations, inlined code, etc, IIRC). [This is in loops > where multiple stores are moved.] So iterating and inserting on the exit > edges caused the stores to be saved in reverse. I am now saving the edges and > threading the code, so they happen in the correct order: > > if (foo_flag_lsm) > foo = foo_lsm; > if (foo2_flag_lsm) > foo2 = foo2_lsm; > actual_exit: > > This required some edge redirection so we didn't jump to the original actual > exit prematurely when handling multiple moved stores. The dominator tree > needed to be updated, and there is one instance where I couldn't find a clever > way of saving the order without jumping through an inordinate amount of hoops. > It seemed easier to call recompute_dominator. > > Second, avoiding the load, unless it happens in the loop like you have > suggested, brought about some bogus unused warnings with the LSM temporaries. > What happens is that compute_ssa() creates uses of the LSM temporaries that > are not yet defined, so we end up with PHI nodes with a definition entry (D). > Since we are sure the eventual uses will be gated by their corresponding flag, > I created phi nodes with an initial value of 0 in the preheader of the loops > in which they are used. This solved the problem. I also played with > initializing to 0 at the entry block, but that seemed a bit too invasive. Hm. Btw, see also PR39612 which you could solve with that? > Andrew suggested the correct fix was to add a new pass that was able to do > some ?? flow sensitive data flow analysis ?? that could discover these > unreac
Re: fix install-no-fixedincludes mishaps
Hello Paolo, This is a followup on the exchange we just had about this patch, as part of another thread: http://gcc.gnu.org/ml/gcc-patches/2012-05/msg01093.html Attached is a second version, adjusted to account for the suggestions you made. Looks good ? Thanks for your feedback, With Kind Regards, Olivier 2012-05-16 Olivier Hainque toplevel/ * Makefile.tpl (gcc-no-fixedincludes): Rename into ... (gcc-install-no-fixedincludes): Now forwarder to local target in gcc/ (install-no-fixedincludes): Adjust accordingly. * Makefile.in: Regenerate. gcc/ * Makefile.in (install-no-fixedincludes): New target, former toplevel gcc-no-fixedincludes. Stash "include-fixed" in addition to "include". Add comments and improve stamp preservation across the whole sequence. (stmp-int-hdrs): Use move-if-change + cp -p to setup fix_dir/limits.h. nofixed-2.diff Description: Binary data
Re: Turn check macros into functions. (issue6188088)
On Wed, May 16, 2012 at 2:43 PM, Diego Novillo wrote: > On 12-05-16 05:41 , Richard Guenther wrote: > >> What's the reason for templating these functions? They all take trees as >> parameter!? > > > True. I don't recall what Lawrence had in mind, but I remember that by > using templates here, you don't need to deal with the mess of distinguishing > tree from const_tree, and having to force constness out with ugly casts. Well, but you get no typesafety for it in return. You can simply provide a const_tree overload. With using templates you are also forced to retain these functions in the header file - another thing that I suppose you guys were about to "fix"? It's after all debugging code. > Additionally, templates are producing slightly smaller code than the > non-template variant (about 0.2% smaller). I'm not actually sure why this > happens, but it's consistent across all binaries. Well, what did you compare? Make sure to not have -fkeep-inline-functions, otherwise you get all bodies as compared to only the instantiated bodies. Richard. > > Diego.
Re: PING PATCH: break lines in announce_function
On Wed, May 16, 2012 at 2:46 PM, Basile Starynkevitch wrote: > Hello All, > > I am pinging the patch http://gcc.gnu.org/ml/gcc-patches/2012-02/msg00474.html > below for trunk svn 187587 > # patch > Index: gcc/toplev.c > === > --- gcc/toplev.c (revision 187587) > +++ gcc/toplev.c (working copy) > @@ -229,6 +229,12 @@ > { > if (!quiet_flag) > { > + Seems pretty arbitrary and with bogus whitespace here ^^^ So - why? I like it the way it is. Richard. > + static long count; > + count++; > + if (count % 8 == 0) > + putc('\n', stderr); > + > if (rtl_dump_and_exit) > fprintf (stderr, "%s ", > identifier_to_locale (IDENTIFIER_POINTER (DECL_NAME (decl; > ### gcc/ChangeLog > entry > 2011-05-16 Basile Starynkevitch > > * toplev.c (announce_function): Output newline periodically. > ### > > Ok for trunk? > > Cheers > -- > Basile STARYNKEVITCH http://starynkevitch.net/Basile/ > email: basilestarynkevitchnet mobile: +33 6 8501 2359 > 8, rue de la Faiencerie, 92340 Bourg La Reine, France > *** opinions {are only mines, sont seulement les miennes} ***
[PATCH] Fix PR53364
This fixes PR53364 - an oversight in the alias oracle when detecting view-conversions on decls. Bootstrapped and tested on x86_64-unknown-linux-gnu, applied to trunk and branch (possibly latent on the 4.6 branch). Richard. 2012-05-16 Richard Guenther PR tree-optimization/53364 * tree-ssa-alias.c (indirect_ref_may_alias_decl_p): Properly detect a view-conversion of the decl. * g++.dg/torture/pr53364.C: New testcase. Index: gcc/tree-ssa-alias.c === *** gcc/tree-ssa-alias.c(revision 187586) --- gcc/tree-ssa-alias.c(working copy) *** indirect_ref_may_alias_decl_p (tree ref1 *** 804,811 /* If either reference is view-converted, give up now. */ if (same_type_for_tbaa (TREE_TYPE (base1), TREE_TYPE (ptrtype1)) != 1 ! || same_type_for_tbaa (TREE_TYPE (dbase2), !TREE_TYPE (reference_alias_ptr_type (dbase2))) != 1) return true; /* If both references are through the same type, they do not alias --- 804,810 /* If either reference is view-converted, give up now. */ if (same_type_for_tbaa (TREE_TYPE (base1), TREE_TYPE (ptrtype1)) != 1 ! || same_type_for_tbaa (TREE_TYPE (dbase2), TREE_TYPE (base2)) != 1) return true; /* If both references are through the same type, they do not alias Index: gcc/testsuite/g++.dg/torture/pr53364.C === *** gcc/testsuite/g++.dg/torture/pr53364.C (revision 0) --- gcc/testsuite/g++.dg/torture/pr53364.C (revision 0) *** *** 0 --- 1,37 + // { dg-do run } + + extern "C" void abort (void); + + template + inline const _Tp& + min(const _Tp& __a, const _Tp& __b) + { + if (__b < __a) + return __b; + return __a; + } + + struct A + { + int m_x; + + explicit A(int x) : m_x(x) {} + operator int() const { return m_x; } + }; + + struct B : public A + { + public: + explicit B(int x) : A(x) {} + }; + + int data = 1; + + int main() + { + B b = B(10); + b = min(b, B(data)); + if (b != 1) + abort (); + return 0; + }
Re: Turn check macros into functions. (issue6188088)
On 12-05-16 09:00 , Richard Guenther wrote: On Wed, May 16, 2012 at 2:43 PM, Diego Novillo wrote: On 12-05-16 05:41 , Richard Guenther wrote: What's the reason for templating these functions? They all take trees as parameter!? True. I don't recall what Lawrence had in mind, but I remember that by using templates here, you don't need to deal with the mess of distinguishing tree from const_tree, and having to force constness out with ugly casts. Well, but you get no typesafety for it in return. You can simply provide a const_tree overload. There's less typing if you use the template variant. Not sure why you say there is less type safety with templates. With using templates you are also forced to retain these functions in the header file - another thing that I suppose you guys were about to "fix"? It's after all debugging code. No, templated functions must always stay in the header file. There is no changing that. Additionally, templates are producing slightly smaller code than the non-template variant (about 0.2% smaller). I'm not actually sure why this happens, but it's consistent across all binaries. Well, what did you compare? Make sure to not have -fkeep-inline-functions, otherwise you get all bodies as compared to only the instantiated bodies. Two bootstrapped compilers built exactly the same, except one was using the template version, the other using the straight inline functions with const_tree parameters and CONST_CAST_TREE in return values. Diego.
Re: fix install-no-fixedincludes mishaps
Il 16/05/2012 14:54, Olivier Hainque ha scritto: > + > + install-no-fixedincludes: > + # Stash the current set of headers away, save stamps we're going to alter > + # explicitly, and arrange for fixincludes not to run next time we trigger > + # a headers rebuild. > + > + -rm -rf tmp-include > + -mv include tmp-include 2>/dev/null > + -mv include-fixed tmp-include-fixed 2>/dev/null > + -mv stmp-int-hdrs tmp-stmp-int-hdrs 2>/dev/null > + -mv stmp-fixinc tmp-stmp-fixinc 2>/dev/null > + -mkdir include > + -cp -p $(srcdir)/gsyslimits.h include/syslimits.h > + -touch stmp-fixinc > + > + # Rebuild our internal headers, restore the original stamps so that > "install" > + # doesn't trigger pointless rebuilds because of that update, then do install > + > + $(MAKE) $(FLAGS_TO_PASS) stmp-int-hdrs > + > + -mv tmp-stmp-int-hdrs stmp-int-hdrs 2>/dev/null > + -mv tmp-stmp-fixinc stmp-fixinc 2>/dev/null > + > + $(MAKE) $(FLAGS_TO_PASS) install > + > + # Restore the original set of maybe-fixed headers > + > + -rm -rf include; mv tmp-include include 2>/dev/null > + -rm -rf include-fixed; mv tmp-include-fixed include-fixed 2>/dev/null > + Please indent the comments with a tab and remove the empty lines. Ok with that change. Paolo
Re: PING PATCH: break lines in announce_function
On Wed, May 16, 2012 at 03:02:39PM +0200, Richard Guenther wrote: > On Wed, May 16, 2012 at 2:46 PM, Basile Starynkevitch > wrote: > > Hello All, > > > > I am pinging the patch > > http://gcc.gnu.org/ml/gcc-patches/2012-02/msg00474.html > > below for trunk svn 187587 --- gcc/toplev.c(revision 187587) +++ gcc/toplev.c(working copy) @@ -229,6 +229,11 @@ announce_function (tree decl) { if (!quiet_flag) { + static long count; + count++; + if (count % 8 == 0) +putc('\n', stderr); + if (rtl_dump_and_exit) fprintf (stderr, "%s ", identifier_to_locale (IDENTIFIER_POINTER (DECL_NAME (decl; > So - why? I like it the way it is. Because, as I explained in http://gcc.gnu.org/ml/gcc-patches/2012-02/msg00474.html without that patch you have arbitrarily long output lines, and that is unpleasant, in particular when running under gdb or under emacs (also, there may be buffering issues: if GCC misbehave, stderr might not be flushed often enough...) The announce_function is quite rarely really used (because quiet_flag is almost always true), and it is used mostly to debug GCC (or plugins), and in that case having not too large output is quite useful in practice. The patch above is quick & dirty but seems enough. Do you want me to add a comment like /* Hack to avoid very large output lines. */ before? Regards. -- Basile STARYNKEVITCH http://starynkevitch.net/Basile/ email: basilestarynkevitchnet mobile: +33 6 8501 2359 8, rue de la Faiencerie, 92340 Bourg La Reine, France *** opinions {are only mines, sont seulement les miennes} ***
[PATCH] Vectorizer TLC
I noticed a write-only bitmap and some odd CFG hooks initializing code. Removed as follows. Bootstrapped and tested on x86_64-unknown-linux-gnu, applied. Richard. 2012-05-16 Richard Guenther * tree-vect-loop-manip.c (slpeel_update_phi_nodes_for_guard1): Remove set-only bitmap of new names. (slpeel_tree_peel_loop_to_edge): Likewise. Do not set CFG hooks. * tree-flow.h (ssa_names_to_replace): Remove. * tree-into-ssa.c (ssa_names_to_replace): Likewise. Index: gcc/tree-vect-loop-manip.c === --- gcc/tree-vect-loop-manip.c (revision 187534) +++ gcc/tree-vect-loop-manip.c (working copy) @@ -489,8 +489,7 @@ LOOP-> loop1 static void slpeel_update_phi_nodes_for_guard1 (edge guard_edge, struct loop *loop, -bool is_new_loop, basic_block *new_exit_bb, -bitmap *defs) +bool is_new_loop, basic_block *new_exit_bb) { gimple orig_phi, new_phi; gimple update_phi, update_phi2; @@ -586,7 +585,6 @@ slpeel_update_phi_nodes_for_guard1 (edge gcc_assert (get_current_def (current_new_name) == NULL_TREE); set_current_def (current_new_name, PHI_RESULT (new_phi)); - bitmap_set_bit (*defs, SSA_NAME_VERSION (current_new_name)); } } @@ -1159,7 +1157,6 @@ slpeel_tree_peel_loop_to_edge (struct lo struct loop *new_loop = NULL, *first_loop, *second_loop; edge skip_e; tree pre_condition = NULL_TREE; - bitmap definitions; basic_block bb_before_second_loop, bb_after_second_loop; basic_block bb_before_first_loop; basic_block bb_between_loops; @@ -1172,12 +1169,6 @@ slpeel_tree_peel_loop_to_edge (struct lo if (!slpeel_can_duplicate_loop_p (loop, e)) return NULL; - /* We have to initialize cfg_hooks. Then, when calling - cfg_hooks->split_edge, the function tree_split_edge - is actually called and, when calling cfg_hooks->duplicate_block, - the function tree_duplicate_bb is called. */ - gimple_register_cfg_hooks (); - /* If the loop has a virtual PHI, but exit bb doesn't, create a virtual PHI in the exit bb and rename all the uses after the loop. This simplifies the *guard[12] routines, which assume loop closed SSA form for all PHIs @@ -1259,7 +1250,6 @@ slpeel_tree_peel_loop_to_edge (struct lo second_loop = loop; } - definitions = ssa_names_to_replace (); slpeel_update_phis_for_duplicate_loop (loop, new_loop, e == exit_e); rename_variables_in_loop (new_loop); @@ -1397,7 +1387,7 @@ slpeel_tree_peel_loop_to_edge (struct lo bb_before_second_loop, bb_before_first_loop); slpeel_update_phi_nodes_for_guard1 (skip_e, first_loop, first_loop == new_loop, - &new_exit_bb, &definitions); + &new_exit_bb); /* 3. Add the guard that controls whether the second loop is executed. @@ -1441,7 +1431,6 @@ slpeel_tree_peel_loop_to_edge (struct lo if (update_first_loop_count) slpeel_make_loop_iterate_ntimes (first_loop, *first_niters); - BITMAP_FREE (definitions); delete_update_ssa (); adjust_vec_debug_stmts (); Index: gcc/tree-flow.h === *** gcc/tree-flow.h (revision 187589) --- gcc/tree-flow.h (working copy) *** tree create_new_def_for (tree, gimple, d *** 576,582 bool need_ssa_update_p (struct function *); bool name_mappings_registered_p (void); bool name_registered_for_update_p (tree); - bitmap ssa_names_to_replace (void); void release_ssa_name_after_update_ssa (tree); void compute_global_livein (bitmap, bitmap); void mark_sym_for_renaming (tree); --- 574,579 Index: gcc/tree-into-ssa.c === *** gcc/tree-into-ssa.c (revision 187589) --- gcc/tree-into-ssa.c (working copy) *** name_registered_for_update_p (tree n ATT *** 3053,3078 } - /* Return the set of all the SSA names marked to be replaced. */ - - bitmap - ssa_names_to_replace (void) - { - unsigned i = 0; - bitmap ret; - sbitmap_iterator sbi; - - gcc_assert (update_ssa_initialized_fn == NULL - || update_ssa_initialized_fn == cfun); - - ret = BITMAP_ALLOC (NULL); - EXECUTE_IF_SET_IN_SBITMAP (old_ssa_names, 0, i, sbi) - bitmap_set_bit (ret, i); - - return ret; - } - - /* Mark NAME to be released after update_ssa has finished. */ void --- 3053,3058
Re: Turn check macros into functions. (issue6188088)
On Wed, May 16, 2012 at 3:16 PM, Diego Novillo wrote: > On 12-05-16 09:00 , Richard Guenther wrote: >> >> On Wed, May 16, 2012 at 2:43 PM, Diego Novillo >> wrote: >>> >>> On 12-05-16 05:41 , Richard Guenther wrote: >>> What's the reason for templating these functions? They all take trees as parameter!? >>> >>> >>> >>> True. I don't recall what Lawrence had in mind, but I remember that by >>> using templates here, you don't need to deal with the mess of >>> distinguishing >>> tree from const_tree, and having to force constness out with ugly casts. >> >> >> Well, but you get no typesafety for it in return. You can simply provide >> a const_tree overload. > > > There's less typing if you use the template variant. Not sure why you say > there is less type safety with templates. Because it accepts any type as tree argument? It's of course not less type safety than using macros, but less type safety compared to not using templates. >> With using templates you are also forced to retain >> these functions in the header file - another thing that I suppose you guys >> were about to "fix"? It's after all debugging code. > > > No, templated functions must always stay in the header file. There is no > changing that. If they ain't templates they are not templates. And thus do not need to stay in the header. Not sure what you are after here ;) >>> Additionally, templates are producing slightly smaller code than the >>> non-template variant (about 0.2% smaller). I'm not actually sure why >>> this >>> happens, but it's consistent across all binaries. >> >> >> Well, what did you compare? Make sure to not have >> -fkeep-inline-functions, >> otherwise you get all bodies as compared to only the instantiated bodies. > > > Two bootstrapped compilers built exactly the same, except one was using the > template version, the other using the straight inline functions with > const_tree parameters and CONST_CAST_TREE in return values. That's of course not exactly the same. The checking fns should be able to unconditionally use const_tree anyway. Richard. > > Diego.
Re: PING PATCH: break lines in announce_function
On Wed, May 16, 2012 at 3:18 PM, Basile Starynkevitch wrote: > On Wed, May 16, 2012 at 03:02:39PM +0200, Richard Guenther wrote: >> On Wed, May 16, 2012 at 2:46 PM, Basile Starynkevitch >> wrote: >> > Hello All, >> > >> > I am pinging the patch >> > http://gcc.gnu.org/ml/gcc-patches/2012-02/msg00474.html >> > below for trunk svn 187587 > > --- gcc/toplev.c (revision 187587) > +++ gcc/toplev.c (working copy) > @@ -229,6 +229,11 @@ announce_function (tree decl) > { > if (!quiet_flag) > { > + static long count; > + count++; > + if (count % 8 == 0) > + putc('\n', stderr); > + > if (rtl_dump_and_exit) > fprintf (stderr, "%s ", > identifier_to_locale (IDENTIFIER_POINTER (DECL_NAME (decl; > > >> So - why? I like it the way it is. > > Because, as I explained in > http://gcc.gnu.org/ml/gcc-patches/2012-02/msg00474.html without that patch > you have arbitrarily long output lines, and that is unpleasant, in particular > when running under gdb > or under emacs (also, there may be buffering issues: if GCC misbehave, stderr > might not be flushed > often enough...) stderr is unbuffered > The announce_function is quite rarely really used (because quiet_flag is > almost always true), > and it is used mostly to debug GCC (or plugins), and in that case having not > too large output > is quite useful in practice. The patch above is quick & dirty but seems > enough. > Do you want me to add a comment like /* Hack to avoid very large output > lines. */ before? No, I want the large line to stay as-is. It's pleasant for me. Richard. > Regards. > > > -- > Basile STARYNKEVITCH http://starynkevitch.net/Basile/ > email: basilestarynkevitchnet mobile: +33 6 8501 2359 > 8, rue de la Faiencerie, 92340 Bourg La Reine, France > *** opinions {are only mines, sont seulement les miennes} ***
[PATCH] Remove unused weird get_virtual_var
Committed. Richard. 2012-05-16 Richard Guenther * tree-flow.h (get_virtual_var): Remove. * tree-dfa.c (get_virtual_var): Likewise. Index: gcc/tree-flow.h === --- gcc/tree-flow.h (revision 187591) +++ gcc/tree-flow.h (working copy) @@ -491,7 +491,6 @@ extern void debug_referenced_vars (void) extern void dump_referenced_vars (FILE *); extern void dump_variable (FILE *, tree); extern void debug_variable (tree); -extern tree get_virtual_var (tree); extern bool add_referenced_var (tree); extern void remove_referenced_var (tree); extern void mark_symbols_for_renaming (gimple); Index: gcc/tree-dfa.c === --- gcc/tree-dfa.c (revision 187591) +++ gcc/tree-dfa.c (working copy) @@ -624,29 +624,6 @@ remove_referenced_var (tree var) } -/* Return the virtual variable associated to the non-scalar variable VAR. */ - -tree -get_virtual_var (tree var) -{ - STRIP_NOPS (var); - - if (TREE_CODE (var) == SSA_NAME) -var = SSA_NAME_VAR (var); - - while (TREE_CODE (var) == REALPART_EXPR || TREE_CODE (var) == IMAGPART_EXPR -|| handled_component_p (var)) -var = TREE_OPERAND (var, 0); - - /* Treating GIMPLE registers as virtual variables makes no sense. - Also complain if we couldn't extract a _DECL out of the original - expression. */ - gcc_assert (SSA_VAR_P (var)); - gcc_assert (!is_gimple_reg (var)); - - return var; -} - /* Mark all the naked symbols in STMT for SSA renaming. */ void
[PATCH][6/n] Remove mark_symbols_for_renaming
This tackles the remaining caller of mark_symbols_for_renaming, the inliner. Incrementally as it's a mess ;) Bootstrapped and tested on x86_64-unknown-linux-gnu, applied to trunk. Richard. 2012-05-16 Richard Guenther * tree-inline.c (insert_init_stmt): Do not call mark_symbols_for_renaming. (setup_one_parameter): Avoid initializing unused parameters. (declare_return_variable): Properly handle DECL_BY_REFERENCE return vars in SSA form. Index: gcc/tree-inline.c === *** gcc/tree-inline.c (revision 187585) --- gcc/tree-inline.c (working copy) *** insert_init_stmt (copy_body_data *id, ba *** 2542,2548 } gsi_insert_after (&si, init_stmt, GSI_NEW_STMT); gimple_regimplify_operands (init_stmt, &si); - mark_symbols_for_renaming (init_stmt); if (!is_gimple_debug (init_stmt) && MAY_HAVE_DEBUG_STMTS) { --- 2551,2556 *** setup_one_parameter (copy_body_data *id, *** 2707,2720 STRIP_USELESS_TYPE_CONVERSION (rhs); ! /* We want to use MODIFY_EXPR, not INIT_EXPR here so that we !keep our trees in gimple form. */ ! if (def && gimple_in_ssa_p (cfun) && is_gimple_reg (p)) ! { ! def = remap_ssa_name (def, id); ! init_stmt = gimple_build_assign (def, rhs); ! SSA_NAME_IS_DEFAULT_DEF (def) = 0; ! set_default_def (var, NULL); } else init_stmt = gimple_build_assign (var, rhs); --- 2715,2731 STRIP_USELESS_TYPE_CONVERSION (rhs); ! /* If we are in SSA form properly remap the default definition ! or omit the initialization if the parameter is unused. */ ! if (gimple_in_ssa_p (cfun) && is_gimple_reg (p)) ! { ! if (def) ! { ! def = remap_ssa_name (def, id); ! init_stmt = gimple_build_assign (def, rhs); ! SSA_NAME_IS_DEFAULT_DEF (def) = 0; ! set_default_def (var, NULL); ! } } else init_stmt = gimple_build_assign (var, rhs); *** declare_return_variable (copy_body_data *** 2974,2983 if (gimple_in_ssa_p (id->src_cfun)) add_referenced_var (temp); insert_decl_map (id, result, temp); ! /* When RESULT_DECL is in SSA form, we need to use it's default_def !SSA_NAME. */ ! if (gimple_in_ssa_p (id->src_cfun) && gimple_default_def (id->src_cfun, result)) ! temp = remap_ssa_name (gimple_default_def (id->src_cfun, result), id); insert_init_stmt (id, entry_bb, gimple_build_assign (temp, var)); } else --- 2985,2999 if (gimple_in_ssa_p (id->src_cfun)) add_referenced_var (temp); insert_decl_map (id, result, temp); ! /* When RESULT_DECL is in SSA form, we need to remap and initialize !it's default_def SSA_NAME. */ ! if (gimple_in_ssa_p (id->src_cfun) ! && is_gimple_reg (result)) ! { ! temp = make_ssa_name (temp, NULL); ! insert_decl_map (id, gimple_default_def (id->src_cfun, result), ! temp); ! } insert_init_stmt (id, entry_bb, gimple_build_assign (temp, var)); } else
Re: PING PATCH: break lines in announce_function
On Wed, May 16, 2012 at 03:29:12PM +0200, Richard Guenther wrote: > On Wed, May 16, 2012 at 3:18 PM, Basile Starynkevitch > wrote: > > On Wed, May 16, 2012 at 03:02:39PM +0200, Richard Guenther wrote: > >> On Wed, May 16, 2012 at 2:46 PM, Basile Starynkevitch > >> wrote: > >> > Hello All, > >> > > >> > I am pinging the patch > >> > http://gcc.gnu.org/ml/gcc-patches/2012-02/msg00474.html > >> > below for trunk svn 187587 > > > > --- gcc/toplev.c (revision 187587) > > +++ gcc/toplev.c (working copy) > > @@ -229,6 +229,11 @@ announce_function (tree decl) > > { > > if (!quiet_flag) > > { > > + static long count; > > + count++; > > + if (count % 8 == 0) > > + putc('\n', stderr); > > + > > if (rtl_dump_and_exit) > > fprintf (stderr, "%s ", > > identifier_to_locale (IDENTIFIER_POINTER (DECL_NAME > > (decl; > > > > > >> So - why? I like it the way it is. > > > > Because, as I explained in > > http://gcc.gnu.org/ml/gcc-patches/2012-02/msg00474.html without that patch > > you have arbitrarily long output lines, and that is unpleasant, in > > particular when running under gdb > > or under emacs (also, there may be buffering issues: if GCC misbehave, > > stderr might not be flushed > > often enough...) > > stderr is unbuffered > > > The announce_function is quite rarely really used (because quiet_flag is > > almost always true), > > and it is used mostly to debug GCC (or plugins), and in that case having > > not too large output > > is quite useful in practice. The patch above is quick & dirty but seems > > enough. > > Do you want me to add a comment like /* Hack to avoid very large output > > lines. */ before? > > No, I want the large line to stay as-is. It's pleasant for me. Do you have any hints or tricks to have this work well when running cc1 under emacs (eg in a *shell* or *gud-cc1* Emacs buffer)? Without that patch displaying happen too late (and eats a lot of Emacs CPU)!! Regards -- Basile STARYNKEVITCH http://starynkevitch.net/Basile/ email: basilestarynkevitchnet mobile: +33 6 8501 2359 8, rue de la Faiencerie, 92340 Bourg La Reine, France *** opinions {are only mines, sont seulement les miennes} ***
Re: PING PATCH: break lines in announce_function
On Wed, May 16, 2012 at 3:38 PM, Basile Starynkevitch wrote: > On Wed, May 16, 2012 at 03:29:12PM +0200, Richard Guenther wrote: >> On Wed, May 16, 2012 at 3:18 PM, Basile Starynkevitch >> wrote: >> > On Wed, May 16, 2012 at 03:02:39PM +0200, Richard Guenther wrote: >> >> On Wed, May 16, 2012 at 2:46 PM, Basile Starynkevitch >> >> wrote: >> >> > Hello All, >> >> > >> >> > I am pinging the patch >> >> > http://gcc.gnu.org/ml/gcc-patches/2012-02/msg00474.html >> >> > below for trunk svn 187587 >> > >> > --- gcc/toplev.c (revision 187587) >> > +++ gcc/toplev.c (working copy) >> > @@ -229,6 +229,11 @@ announce_function (tree decl) >> > { >> > if (!quiet_flag) >> > { >> > + static long count; >> > + count++; >> > + if (count % 8 == 0) >> > + putc('\n', stderr); >> > + >> > if (rtl_dump_and_exit) >> > fprintf (stderr, "%s ", >> > identifier_to_locale (IDENTIFIER_POINTER (DECL_NAME >> > (decl; >> > >> > >> >> So - why? I like it the way it is. >> > >> > Because, as I explained in >> > http://gcc.gnu.org/ml/gcc-patches/2012-02/msg00474.html without that patch >> > you have arbitrarily long output lines, and that is unpleasant, in >> > particular when running under gdb >> > or under emacs (also, there may be buffering issues: if GCC misbehave, >> > stderr might not be flushed >> > often enough...) >> >> stderr is unbuffered >> >> > The announce_function is quite rarely really used (because quiet_flag is >> > almost always true), >> > and it is used mostly to debug GCC (or plugins), and in that case having >> > not too large output >> > is quite useful in practice. The patch above is quick & dirty but seems >> > enough. >> > Do you want me to add a comment like /* Hack to avoid very large output >> > lines. */ before? >> >> No, I want the large line to stay as-is. It's pleasant for me. > > > Do you have any hints or tricks to have this work well when running cc1 under > emacs > (eg in a *shell* or *gud-cc1* Emacs buffer)? > > Without that patch displaying happen too late (and eats a lot of Emacs CPU)!! 1) Fix emacs (do not buffer stderr) 2) do not omit -quiet when running from inside emacs Richard.
Re: Turn check macros into functions. (issue6188088)
On 12-05-16 09:27 , Richard Guenther wrote: There's less typing if you use the template variant. Not sure why you say there is less type safety with templates. Because it accepts any type as tree argument? Yes and no. It accepts any type that responds to tree operations and has the same fields. No, templated functions must always stay in the header file. There is no changing that. If they ain't templates they are not templates. And thus do not need to stay in the header. Not sure what you are after here ;) I think we are talking past each other. I simply stated that function templates must stay in headers. There's no export template, after all. I'm not defending the choice of using templates for these particular functions. That's something for Lawrence to argue. I am simply curious as to how using the template variant produces slightly smaller code. Two bootstrapped compilers built exactly the same, except one was using the template version, the other using the straight inline functions with const_tree parameters and CONST_CAST_TREE in return values. That's of course not exactly the same. The checking fns should be able to unconditionally use const_tree anyway. Sorry, I don't know what you are after. I was comparing two compilers that only differ in how they implement the tree checking functions. One uses templates, the other uses inline functions taking const_tree (not even static inline, just inline). They are bootstrapped compilers, so -fkeep-inline-functions is not used. What do you think I should have compared? Diego.
Re: Turn check macros into functions. (issue6188088)
On Wed, May 16, 2012 at 3:40 PM, Diego Novillo wrote: > On 12-05-16 09:27 , Richard Guenther wrote: > >>> There's less typing if you use the template variant. Not sure why >>> you say there is less type safety with templates. >> >> >> Because it accepts any type as tree argument? > > > Yes and no. It accepts any type that responds to tree > operations and has the same fields. > > >>> No, templated functions must always stay in the header file. >>> There is no changing that. >> >> >> If they ain't templates they are not templates. And thus do not >> need to stay in the header. Not sure what you are after here ;) > > > I think we are talking past each other. I simply stated that function > templates must stay in headers. There's no export template, after all. > > I'm not defending the choice of using templates for these particular > functions. That's something for Lawrence to argue. > > I am simply curious as to how using the template variant produces > slightly smaller code. > > >>> Two bootstrapped compilers built exactly the same, except one was >>> using the template version, the other using the straight inline >>> functions with const_tree parameters and CONST_CAST_TREE in return >>> values. >> >> >> That's of course not exactly the same. The checking fns should be >> able to unconditionally use const_tree anyway. > > > Sorry, I don't know what you are after. I was comparing two compilers > that only differ in how they implement the tree checking functions. One > uses templates, the other uses inline functions taking const_tree (not even > static inline, just inline). > > They are bootstrapped compilers, so -fkeep-inline-functions is not used. > What do you think I should have compared? Sounds like it should come out equally sized then. Possibly an instantiated template leads to different code from the frontend? Possibly we do not inline all calls in either case? Possibly because only instantiated templates count towards initial unit size (but all, even unused(?), inline functions count towards it)? Would be interesting to see preprocessed source for both cases for a single object files where the size difference appears. It might be artificial, too, caused by UID perturbations and random codegen difference due to that. Non-templates should use less memory (with templates you at least have a single instantiation and thus copy of the functions, with functions you just have one). That said - I see no technical reasons to use templates here and we should not use C++ just because we can ... (and not scare people off here if the technical reason is minor, such as an unknown size advantage). Richard. > > Diego.
Re: [PATCH] ARM/NEON: vld1q_dup_s64 builtin
On 11.05.2012 16:48, Ramana Radhakrishnan wrote: I would change the iterator from VQX to VQ in the pattern above (you can also simplify the setting of neon_type in that case as well as change that to be a vec_duplicate as below and get rid of any lingering definitions of UNSPEC_VLD1_DUP if they exist), define a separate pattern that expressed this as a define_insn_and_split as below. (define_insn_and_split "neon_vld1_dupv2di" [(set (match_operand:V2DI 0 "s_register_operand" "=w") (vec_duplicate:V2DI (match_operand:DI 1 "neon_struct_operand" "Um")))] "TARGET_NEON" "#" "&& reload_completed" [(const_int 0)] { rtx tmprtx = gen_lowpart (DImode, operands[0]); emit_insn (gen_neon_vld1_dupdi (tmprtx, operands[1])); emit_move_insn (gen_highpart (DImode, operands[0]), tmprtx ); DONE; } (set_attr "length" "8") (set_attr "neon_type" ") ) Do you want to try this and see what you get ? Thanks for this example and suggestion, it does work. I'd rather have an extra regression test in gcc.target/arm that was a run time test. for e.g. take a look at gcc.target/arm/neon-vadds64.c . Here is an updated patch: 2012-05-16 Christophe Lyon * gcc/config/arm/neon.md (neon_vld1_dup): Restrict to VQ operands. (neon_vld1_dupv2di): New, fixes vld1q_dup_s64. * gcc/testsuite/gcc.target/arm/neon-vld1_dupQ.c: New test. Index: gcc/testsuite/gcc.target/arm/neon-vld1_dupQ.c === --- gcc.orig/gcc/testsuite/gcc.target/arm/neon-vld1_dupQ.c(revision 0) +++ gcc.new/gcc/testsuite/gcc.target/arm/neon-vld1_dupQ.c(revision 0) @@ -0,0 +1,24 @@ +/* Test the `vld1q_s64' ARM Neon intrinsic. */ + +/* { dg-do run } */ +/* { dg-require-effective-target arm_neon_hw } */ +/* { dg-options "-O0" } */ +/* { dg-add-options arm_neon } */ + +#include "arm_neon.h" +#include + +int main (void) +{ + int64x1_t input[2] = {(int64x1_t)0x0123456776543210LL, +(int64x1_t)0x89abcdeffedcba90LL}; + int64x1_t output[2] = {0, 0}; + int64x2_t var = vld1q_dup_s64(input); + + vst1q_s64(output, var); + if (output[0] != (int64x1_t)0x0123456776543210LL) +abort(); + if (output[1] != (int64x1_t)0x0123456776543210LL) +abort(); + return 0; +} Index: gcc/config/arm/neon.md === --- gcc.orig/gcc/config/arm/neon.md(revision 2659) +++ gcc.new/gcc/config/arm/neon.md(working copy) @@ -4195,20 +4195,32 @@ ) (define_insn "neon_vld1_dup" - [(set (match_operand:VQX 0 "s_register_operand" "=w") -(unspec:VQX [(match_operand: 1 "neon_struct_operand" "Um")] + [(set (match_operand:VQ 0 "s_register_operand" "=w") +(unspec:VQ [(match_operand: 1 "neon_struct_operand" "Um")] UNSPEC_VLD1_DUP))] "TARGET_NEON" { - if (GET_MODE_NUNITS (mode) > 2) return "vld1.\t{%e0[], %f0[]}, %A1"; - else -return "vld1.\t%h0, %A1"; } [(set (attr "neon_type") - (if_then_else (gt (const_string "") (const_string "1")) -(const_string "neon_vld2_2_regs_vld1_vld2_all_lanes") -(const_string "neon_vld1_1_2_regs")))] + (const_string "neon_vld2_2_regs_vld1_vld2_all_lanes"))] +) + +(define_insn_and_split "neon_vld1_dupv2di" + [(set (match_operand:V2DI 0 "s_register_operand" "=w") +(vec_duplicate:V2DI (match_operand:DI 1 "neon_struct_operand" "Um")))] + "TARGET_NEON" + "#" + "&& reload_completed" + [(const_int 0)] + { +rtx tmprtx = gen_lowpart (DImode, operands[0]); +emit_insn (gen_neon_vld1_dupdi (tmprtx, operands[1])); +emit_move_insn (gen_highpart (DImode, operands[0]), tmprtx ); +DONE; +} + [(set_attr "length" "8") + (set (attr "neon_type") (const_string "neon_vld2_2_regs_vld1_vld2_all_lanes"))] ) (define_expand "vec_store_lanes"
Re: PING PATCH: break lines in announce_function
On 16 May 2012 15:40, Richard Guenther wrote: >> >> Without that patch displaying happen too late (and eats a lot of Emacs CPU)!! > > 1) Fix emacs (do not buffer stderr) > 2) do not omit -quiet when running from inside emacs Actually, I wonder how you (Richard) and other GCC hackers work with and debug GCC, because it is a real pain in the ass. * All the TREE_ macros don't work. * __extension__ prevents GDB from evaluating many things. * announce_function dumps slow down Emacs (and the shell when working via SSH) when debugging anything related to libstdc++ (or any large testcase). * Hitting auto-completion in GDB means staring at the window for 5-10 minutes until it decides to stop listing stuff. There are quite a few other annoyances, but these are the most common (perhaps I should make a list in the wiki). Cheers, Manuel.
Re: PING PATCH: break lines in announce_function
On Wed, May 16, 2012 at 3:56 PM, Manuel López-Ibáñez wrote: > On 16 May 2012 15:40, Richard Guenther wrote: >>> >>> Without that patch displaying happen too late (and eats a lot of Emacs >>> CPU)!! >> >> 1) Fix emacs (do not buffer stderr) >> 2) do not omit -quiet when running from inside emacs > > Actually, I wonder how you (Richard) and other GCC hackers work with > and debug GCC, because it is a real pain in the ass. > > * All the TREE_ macros don't work. I have memoized all sub-fields of tree, so I don't use those macros (still other people use them with some gdbinit macdefs and -g3). > * __extension__ prevents GDB from evaluating many things. I did not run into this one yet - maybe open a GDB bug? > * announce_function dumps slow down Emacs (and the shell when working > via SSH) when debugging anything related to libstdc++ (or any large > testcase). I always use -quiet, even when debugging. I do not use -quite when tracking down compile-time hog issues. > * Hitting auto-completion in GDB means staring at the window for 5-10 > minutes until it decides to stop listing stuff. That works for me. Though my debug-cc1 is built with a C compiler (yes, with C++ this will get a pain in the ass ...) > There are quite a few other annoyances, but these are the most common > (perhaps I should make a list in the wiki). The most common annoyance for me is stepping into trivial inline functions when trying to step into an "important function", thus foo (tree_code_length (x), a, b); something people want to make even more prominent :( Richard. > Cheers, > > Manuel.
[ping][PATCH] configure.ac: Also quote '$' in tbaseargs
Hi, Would anyone please have a look at: http://gcc.gnu.org/ml/gcc-patches/2012-05/msg00044.html I know this is trivial boring stuff, but perhaps especially because of this let's get rid of it ASAP? Thanks. Maciej
[PATCH][7/7] Remove mark_symbols_for_renaming
This is it. Yay. Apart from return variable handling the inliner is clean (even that we could clean up, but we're currently re-writing virtual SSA form from the scratch anyway, so the benefit would be minor). This patch saves us one update_stmt call per statement (and update_stmt is not cheap!). Bootstrapped on x86_64-unknown-linux-gnu, testing nearly finished (and fingers crossing ...). Richard. 2012-05-16 Richard Guenther * tree-flow.h (mark_symbols_for_renaming): Remove. * tree-dfa.c (mark_symbols_for_renaming): Likewise. * tree-inline.c (copy_edges_for_bb): Do not mark symbols for renaming. (copy_debug_stmt): Likewise. (expand_call_inline): Likewise. (declare_return_variable): Mark the return variable for renaming if necessary. Index: gcc/tree-flow.h === *** gcc/tree-flow.h (revision 187593) --- gcc/tree-flow.h (working copy) *** extern void dump_variable (FILE *, tree) *** 493,499 extern void debug_variable (tree); extern bool add_referenced_var (tree); extern void remove_referenced_var (tree); - extern void mark_symbols_for_renaming (gimple); extern tree make_rename_temp (tree, const char *); extern void set_default_def (tree, tree); extern tree gimple_default_def (struct function *, tree); --- 493,498 Index: gcc/tree-dfa.c === *** gcc/tree-dfa.c (revision 187593) --- gcc/tree-dfa.c (working copy) *** remove_referenced_var (tree var) *** 624,646 } - /* Mark all the naked symbols in STMT for SSA renaming. */ - - void - mark_symbols_for_renaming (gimple stmt) - { - tree op; - ssa_op_iter iter; - - update_stmt (stmt); - - /* Mark all the operands for renaming. */ - FOR_EACH_SSA_TREE_OPERAND (op, stmt, iter, SSA_OP_ALL_OPERANDS) - if (DECL_P (op)) - mark_sym_for_renaming (op); - } - - /* If EXP is a handled component reference for a structure, return the base variable. The access range is delimited by bit positions *POFFSET and *POFFSET + *PMAX_SIZE. The access size is *PSIZE bits. If either --- 616,621 Index: gcc/tree-inline.c === *** gcc/tree-inline.c (revision 187593) --- gcc/tree-inline.c (working copy) *** copy_edges_for_bb (basic_block bb, gcov_ *** 1921,1931 copy_stmt = gsi_stmt (si); if (!is_gimple_debug (copy_stmt)) ! { ! update_stmt (copy_stmt); ! if (gimple_in_ssa_p (cfun)) ! mark_symbols_for_renaming (copy_stmt); ! } /* Do this before the possible split_block. */ gsi_next (&si); --- 1921,1927 copy_stmt = gsi_stmt (si); if (!is_gimple_debug (copy_stmt)) ! update_stmt (copy_stmt); /* Do this before the possible split_block. */ gsi_next (&si); *** copy_debug_stmt (gimple stmt, copy_body_ *** 2397,2404 processing_debug_stmt = 0; update_stmt (stmt); - if (gimple_in_ssa_p (cfun)) - mark_symbols_for_renaming (stmt); } /* Process deferred debug stmts. In order to give values better odds --- 2393,2398 *** declare_return_variable (copy_body_data *** 2961,2966 --- 2955,2965 TREE_ADDRESSABLE (var) = 1; var = build_fold_addr_expr (var); } + else if (gimple_in_ssa_p (cfun) + && is_gimple_reg (var)) + /* ??? Re-org id->retval and its special handling so that we can +record an SSA name directly and not need to invoke the SSA renamer. */ + mark_sym_for_renaming (var); done: /* Register the VAR_DECL as the equivalent for the RESULT_DECL; that *** expand_call_inline (basic_block bb, gimp *** 4032,4039 gimple old_stmt = stmt; stmt = gimple_build_assign (gimple_call_lhs (stmt), use_retvar); gsi_replace (&stmt_gsi, stmt, false); - if (gimple_in_ssa_p (cfun)) - mark_symbols_for_renaming (stmt); maybe_clean_or_replace_eh_stmt (old_stmt, stmt); } else --- 4031,4036
[libgo] Undefined references to log_syslog.syslog_c on Solaris
As of today, all Go link tests were failing like this: Undefined first referenced symbol in file log_syslog.syslog_c /var/gcc/regression/trunk/11-gcc/build/i386-pc-solaris2.11/. FAIL: go.go-torture/execute/array-1.go compilation, -O0 The symbol is referenced in log/syslog.o, but not defined anywhere. It turned out that this happens because an asm() in go/log/syslog/syslog_c.c hasn't been updated. The following patch fixes this and allowed the go and libgo tests to pass as expected on i386-pc-solaris2.11. Rainer diff --git a/libgo/go/log/syslog/syslog_c.c b/libgo/go/log/syslog/syslog_c.c --- a/libgo/go/log/syslog/syslog_c.c +++ b/libgo/go/log/syslog/syslog_c.c @@ -10,7 +10,7 @@ can't represent a C varargs function in Go. */ void syslog_c(int, const char*) - asm ("libgo_log.syslog.syslog_c"); + asm ("log_syslog.syslog_c"); void syslog_c (int priority, const char *msg) -- - Rainer Orth, Center for Biotechnology, Bielefeld University
Re: fix install-no-fixedincludes mishaps
On May 16, 2012, at 15:17 , Paolo Bonzini wrote: > Please indent the comments with a tab and remove the empty lines. Sure. > Ok with that change. Installed, thanks :) If I may, we actually hit another install-no-fixedincludes issue in more recent versions of gcc, after http://gcc.gnu.org/ml/gcc-patches/2011-08/msg00319.html (Move unwinder to toplevel libgcc) gcc: * Makefile.in (UNWIND_H): Remove. ... (stmp-int-hdrs): Remove $(UNWIND_H) dependency. Don't copy $(UNWIND_H). ... install-no-fixedincludes was not updated. unwind.h is thus not installed any more and this is causing troubles for at least canadian builds. We fixed it with the attached patch in our tree. How does that look to you ? Many thanks in advance, Olivier 2012-05-16 Olivier Hainque libgcc/ * Makefile.in (install-unwind_h): Rename into ... (install-unwind_h-forbuild): New target. (all): Use it instead of the former install-unwind_h. (install-unwind_h): Reinstate, copy to user install destination for include files, not to the internal gcc object directory one. (install-leaf): Depend on it. unwindh.diff Description: Binary data
Re: [Patch, libgfortran] Pass mode in "open" for O_CREAT and on VxWorks
On 05/16/2012 08:06 AM, rbmj wrote: On 05/16/2012 07:26 AM, Janne Blomqvist wrote: On Wed, May 16, 2012 at 1:03 PM, Tobias Burnus wrote: On 05/16/2012 08:45 AM, Janne Blomqvist wrote: IMHO it would be cleaner if you instead somewhere in the beginning of unix.c did #ifdef __VXWORKS__ /* open is not a variadic function on vxworks (or something...) */ #define open(path, flags) open(path, flags, 0666) #endif Untested, but AFAICS it should work (unless I'm missing something wrt macro expansion, which of course is quite possible). Testing shows that CPP does not like it: $ cpp #define a(x,y) a(x,y,0666) a(1,2) a(1,2,3) # 1 "" # 1 "" # 1 "" a(1,2,0666) :3:8: error: macro "a" passed 3 arguments, but takes just 2 Ah, bummer. We have something roughly similiar for snprintf (see libgfortran.h), but it seems that it works slightly differently due to using a variadic macro etc. So it seems this idea will not work, sorry. Actually, this works for me: $ cat test.c #include void a(int b, int c, int d) { printf("%i\t%i\t%i", b, c, d); } #define a(b, c) (a)(b, c, 3) int main() { a(1, 2); return 0; } $ gcc test.c -o a.out $ ./a.out 1 2 3 Just realized that mine doesn't work either. Sorry, wasn't looking closely enough :-( The closest one can get is an inline overload, but that requires c++. On a side note, isn't this what autoconf is designed for? Robert Mason
Re: [libgo] Undefined references to log_syslog.syslog_c on Solaris
Rainer Orth writes: > As of today, all Go link tests were failing like this: > > Undefined first referenced > symbol in file > log_syslog.syslog_c > /var/gcc/regression/trunk/11-gcc/build/i386-pc-solaris2.11/. > FAIL: go.go-torture/execute/array-1.go compilation, -O0 > > The symbol is referenced in log/syslog.o, but not defined anywhere. It > turned out that this happens because an asm() in > go/log/syslog/syslog_c.c hasn't been updated. > > The following patch fixes this and allowed the go and libgo tests to > pass as expected on i386-pc-solaris2.11. Thanks. Committed to mainline and 4.7 branch. Ian
Re: fix install-no-fixedincludes mishaps
Il 16/05/2012 16:46, Olivier Hainque ha scritto: > > On May 16, 2012, at 15:17 , Paolo Bonzini wrote: >> Please indent the comments with a tab and remove the empty lines. > > Sure. > >> Ok with that change. > > Installed, thanks :) > > If I may, we actually hit another install-no-fixedincludes issue > in more recent versions of gcc, after > > http://gcc.gnu.org/ml/gcc-patches/2011-08/msg00319.html > (Move unwinder to toplevel libgcc) > > gcc: > * Makefile.in (UNWIND_H): Remove. > ... > (stmp-int-hdrs): Remove $(UNWIND_H) dependency. > Don't copy $(UNWIND_H). > ... > > install-no-fixedincludes was not updated. unwind.h is > thus not installed any more and this is causing troubles > for at least canadian builds. > > We fixed it with the attached patch in our tree. > > How does that look to you ? Ok, thanks! All these are regressions, right? Please commit them to 4.7 branch too. Paolo > Many thanks in advance, > > Olivier > > 2012-05-16 Olivier Hainque > > libgcc/ > * Makefile.in (install-unwind_h): Rename into ... > (install-unwind_h-forbuild): New target. > (all): Use it instead of the former install-unwind_h. > (install-unwind_h): Reinstate, copy to user install destination > for include files, not to the internal gcc object directory one. > (install-leaf): Depend on it. >
Re: PING [RFA] PowerPC e5500 and e6500 cores support
On Fri, 16 Mar 2012, Edmar wrote: > I am pinging this post: > > http://gcc.gnu.org/ml/gcc-patches/2012-03/msg00430.html I don't see any sign of anything further having happened with this patch ... you should ping approximately weekly until a patch is reviewed, and probably also CC relevant maintainers on pings. -- Joseph S. Myers jos...@codesourcery.com
Fix folding through externally keyed vtables
Hi, currently build of Mozilla with -flto-partition=none fails at: /tmp/ccQ0smdA.ltrans3.ltrans.o:ccQ0smdA.ltrans3.o:function scriptableInvokeDefault(NPObject*, _NPVariant const*, unsigned int, _NPVariant*) [clone .part.84.4761]: error: undefi ned reference to 'construction vtable for std::ostream-in-std::basic_ostringstream, std::allocator >' /tmp/ccQ0smdA.ltrans3.ltrans.o:ccQ0smdA.ltrans3.o:function scriptableInvokeDefault(NPObject*, _NPVariant const*, unsigned int, _NPVariant*) [clone .part.84.4761]: error: undefi ned reference to 'construction vtable for std::ostream-in-std::basic_ostringstream, std::allocator >' /tmp/ccQ0smdA.ltrans4.ltrans.o:ccQ0smdA.ltrans4.o:function NPP_New: error: undefined reference to 'construction vtable for std::ostream-in-std::basic_ostringstream, std::allocator >' /tmp/ccQ0smdA.ltrans6.ltrans.o:ccQ0smdA.ltrans6.o:function mozilla::NoteIntentionalCrash(char const*) [clone .local.0] [clone .constprop.67]: error: undefined reference to 'con struction vtable for std::ostream-in-std::basic_ostringstream, std::allocator >' and my patch actually makes this failure to happen always. What happen is the following: C++ represents vtables as initialized variables. When VTABLE is keyed, they are DECL_EXTENRAL but their constructor is known. We do look into them when folding and derive useful data, primarily for devirtualization. The initializers of DECL_EXTERNAL vars however do not exists in current compilatoin unit. They are build as if they were in the compilation unit they are comming from. We previously run into probelm with fact that they can reffer static sumbol from other unit and added can_refer_decl_in_current_unit_p for this reason. This case is even more slipperly - the symbols in question are hidden and thus they pass the check. Still they can not be referred since they live in libstdc++ DSO rather than in Mozilla. Unforutnately there is no way to safely detect these cases - we really don't know where the unit of vtable will live and if it will be compiled with -fdefault-visibility=hidden or not, so in order to keep units interoperable no matter of default visibility setting we can not even look for the hiddden visibility. I think we are still safe when visibilities was not defined by user, since if it is hidden in the other unit, everything will be hidden there and we won't be able to get to the vtable. We must however exclude all references where visibility was defined by user. This means that we can no longer detect this safely. For this reason I added parameter from_decl to can_refer_decl_in_current_unit_p and execute all the strange logic only when the value comes from constructor that can't be output in current unit. Bootstrapped/regtested x86_64-linux, seems sane? More conservative approach would be in this case fold only to values really defined locally. The patch would not be much different, only can_refer_decl_in_current_unit_p would be more strict. There are couple thousdant references in Mozilla that however belongs to this cagegory. Honza * cgraphbuild.c (record_reference): Update. * lto-cgraph.c (lto_output_varpool_node): External vars are not in other partition even if they are not output in current partition. * gimple-fold.c (can_refer_decl_in_current_unit_p): Take FROM_DECL argument; fix. (canonicalize_constructor_val): Take FROM_DECL argument. (fold_ctor_reference, fold_string_cst_ctor_reference, fold_array_ctor_reference, fold_nonarray_ctor_reference, fold_ctor_reference): Likewise. (fold_const_aggregate_ref_1, gimple_get_virt_method_for_binfo): Update. * gimple.h (gimple_fold_builtin): Likewise. Index: cgraphbuild.c === *** cgraphbuild.c (revision 187412) --- cgraphbuild.c (working copy) *** record_reference (tree *tp, int *walk_su *** 54,60 tree decl; struct record_reference_ctx *ctx = (struct record_reference_ctx *)data; ! t = canonicalize_constructor_val (t); if (!t) t = *tp; else if (t != *tp) --- 54,60 tree decl; struct record_reference_ctx *ctx = (struct record_reference_ctx *)data; ! t = canonicalize_constructor_val (t, NULL); if (!t) t = *tp; else if (t != *tp) Index: lto-cgraph.c === *** lto-cgraph.c(revision 187412) --- lto-cgraph.c(working copy) *** lto_output_varpool_node (struct lto_simp *** 585,591 bp_pack_value (&bp, node->analyzed && referenced_from_other_partition_p (&node->symbol.ref_list,
Re: PING [RFA] PowerPC e5500 and e6500 cores support
Joseph, Thanks for the tip. It is likely I need to re-generate the patch (and re-test) as there had been a lot of activity in the trunk since the original post. I will do this soon. Regards, Edmar On 05/16/2012 11:10 AM, Joseph S. Myers wrote: On Fri, 16 Mar 2012, Edmar wrote: I am pinging this post: http://gcc.gnu.org/ml/gcc-patches/2012-03/msg00430.html I don't see any sign of anything further having happened with this patch ... you should ping approximately weekly until a patch is reviewed, and probably also CC relevant maintainers on pings.
Re: fix install-no-fixedincludes mishaps
On May 16, 2012, at 17:03 , Paolo Bonzini wrote: > Ok, thanks! Great :-) > All these are regressions, right? Right. > Please commit them to 4.7 branch too. Will do. Thanks for your prompt and constructive feedback, With Kind Regards, Olivier
Re: gnu-tm: Dont allow assigning transaction_unsafe functions to transaction_safe function pointers (issue6198054)
On 05/15/2012 02:16 PM, Patrick Marlier wrote: Tested on i686. Is the patch ok? Thanks. BTW, Should we generate a warning or an error? -- 2012-05-15 Patrick Marlier * trans-mem.c (diagnose_tm_1_op): Warn about assignment of transaction unsafe function to safe function pointer. Hum. I wonder if there's some other way to do this in the front end. The transition safe->unsafe is of course ok, but unsafe->safe is not. It sure seems like we ought to be able to leverage the type check elsewhere in the language. Unfortunately, the only existing switch we have for attributes is a hard compatible/incompatible flag. So it would take some effort to extend that to somethine more complicated. I don't think this check here really does any good at all, since it only applies to tm regions, and the assignment can happen anywhere. I suppose for the moment we could flip the switch and force incompatibility, then relax that as we improve the front end? r~
Re: Turn check macros into functions. (issue6188088)
Il 16/05/2012 15:27, Richard Guenther ha scritto: >> > >> > Two bootstrapped compilers built exactly the same, except one was using the >> > template version, the other using the straight inline functions with >> > const_tree parameters and CONST_CAST_TREE in return values. > That's of course not exactly the same. The checking fns should be able > to unconditionally use const_tree anyway. Take const_tree, yes. Return const_tree, no. So you need a tree->tree version and a const_tree->const_tree version, like the strchr overloads in the standard C++ library. Paolo
Re: PING PATCH: break lines in announce_function
On 16 May 2012 16:02, Richard Guenther wrote: > On Wed, May 16, 2012 at 3:56 PM, Manuel López-Ibáñez > wrote: >> On 16 May 2012 15:40, Richard Guenther wrote: Without that patch displaying happen too late (and eats a lot of Emacs CPU)!! >>> >>> 1) Fix emacs (do not buffer stderr) >>> 2) do not omit -quiet when running from inside emacs >> >> Actually, I wonder how you (Richard) and other GCC hackers work with >> and debug GCC, because it is a real pain in the ass. >> >> * All the TREE_ macros don't work. > > I have memoized all sub-fields of tree, so I don't use those macros > (still other people use them with some gdbinit macdefs and -g3). Could these macdefs be loaded by default? I see now that any change in the underlying implementation becomes a serious annoyance for you. > >> * __extension__ prevents GDB from evaluating many things. > > I did not run into this one yet - maybe open a GDB bug? It seems it will never work for statement expressions: http://article.gmane.org/gmane.comp.gcc.devel/107339 >> * announce_function dumps slow down Emacs (and the shell when working >> via SSH) when debugging anything related to libstdc++ (or any large >> testcase). > > I always use -quiet, even when debugging. I do not use -quite when tracking > down compile-time hog issues. So why not make that the default? >> * Hitting auto-completion in GDB means staring at the window for 5-10 >> minutes until it decides to stop listing stuff. > > That works for me. Though my debug-cc1 is built with a C compiler > (yes, with C++ this will get a pain in the ass ...) > >> There are quite a few other annoyances, but these are the most common >> (perhaps I should make a list in the wiki). > > The most common annoyance for me is stepping into trivial inline functions > when trying to step into an "important function", thus > > foo (tree_code_length (x), a, b); > > something people want to make even more prominent :( I guess 'n' doesn't work because they are really inlined. Open a GDB bug? I guess this answer does not work both ways... :-( I see how these small functions can quickly become annoying. Is it really that hard to make GDB ignore some functions? On a more general note, there is really a conflict between the old-timers who know every little detail and have grown their custom recipes for going around annoyances, so they resist any change that breaks those recipes or their hard-won knowledge of GCC internals. And any new potential contributor, who feels that trivial things are extremely hard and time-consuming and wants to make things simpler and easier. Of course, it is not really a conflict. New contributors have little saying, which is normal because obviously they contribute the least. However, the mere fact that it is hard to contribute prevents them to contribute more. So we have 2 or 3 people that do 90% of the work, and not a long tail but a very small number of people that contribute the other 10%. Not a good prospect for the future... On this topic, I found this talk particularly interesting: http://percival-music.ca/blog/2010-08-01-sustainable-development.html However, I don't have any solutions to offer, but the situation has been a source of frustration for me since a long time, and I think also for others. Cheers, Manuel.
Re: [C++ Patch] PR 44516
On 05/16/2012 06:54 AM, Paolo Carlini wrote: isn't the diagnostic machinery able to cope with UNKNOWN_LOCATION? By default should be interpreted as input_location, no? That would make sense to me; I don't know if it works that way now, though. @@ -11968,7 +11968,8 @@ tsubst_qualified_id (tree qualified_id, tree args, if (dependent_scope_p (scope)) { if (is_template) - expr = build_min_nt (TEMPLATE_ID_EXPR, expr, template_args); + expr = build_min_nt_loc (UNKNOWN_LOCATION, TEMPLATE_ID_EXPR, +expr, template_args); Here we should be able to retain the location from the TEMPLATE_ID_EXPR we took apart earlier. Jason
Re: [PATCH][ARM] Improve 64-bit shifts (non-NEON)
On 16/05/12 11:25, Ramana Radhakrishnan wrote: Ok with those changes. Hi Ramana, Here's an update rebased and modified as requested. Can you please confirm that the comments explain what you wanted to know, and then I will commit it. Thanks Andrew 2012-05-16 Andrew Stubbs gcc/ * config/arm/arm-protos.h (arm_emit_coreregs_64bit_shift): New prototype. * config/arm/arm.c (arm_emit_coreregs_64bit_shift): New function. * config/arm/arm.md (ashldi3): Use arm_emit_coreregs_64bit_shift. (ashrdi3,lshrdi3): Likewise. (arm_cond_branch): Remove '*' to enable gen_arm_cond_branch. diff --git a/gcc/config/arm/arm-protos.h b/gcc/config/arm/arm-protos.h index cb74d70..b338470 100644 --- a/gcc/config/arm/arm-protos.h +++ b/gcc/config/arm/arm-protos.h @@ -245,6 +245,9 @@ struct tune_params extern const struct tune_params *current_tune; extern int vfp3_const_double_for_fract_bits (rtx); + +extern void arm_emit_coreregs_64bit_shift (enum rtx_code, rtx, rtx, rtx, rtx, + rtx); #endif /* RTX_CODE */ extern void arm_expand_vec_perm (rtx target, rtx op0, rtx op1, rtx sel); diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c index 2c62c51..3ad4c75 100644 --- a/gcc/config/arm/arm.c +++ b/gcc/config/arm/arm.c @@ -25933,4 +25933,256 @@ arm_autoinc_modes_ok_p (enum machine_mode mode, enum arm_auto_incmodes code) return false; } +/* The default expansion of general 64-bit shifts in core-regs is suboptimal, + on ARM, since we know that shifts by negative amounts are no-ops. + Additionally, the default expansion code is not available or suitable + for post-reload insn splits (this can occur when the register allocator + chooses not to do a shift in NEON). + + This function is used in both initial expand and post-reload splits, and + handles all kinds of 64-bit shifts. + + Input requirements: +- It is safe for the input and output to be the same register, but + early-clobber rules apply for the shift amount and scratch registers. +- Shift by register requires both scratch registers. Shift by a constant + less than 32 in Thumb2 mode requires SCRATCH1 only. In all other cases + the scratch registers may be NULL. +- Ashiftrt by a register also clobbers the CC register. */ +void +arm_emit_coreregs_64bit_shift (enum rtx_code code, rtx out, rtx in, + rtx amount, rtx scratch1, rtx scratch2) +{ + rtx out_high = gen_highpart (SImode, out); + rtx out_low = gen_lowpart (SImode, out); + rtx in_high = gen_highpart (SImode, in); + rtx in_low = gen_lowpart (SImode, in); + + /* Terminology: + in = the register pair containing the input value. + out = the destination register pair. + up = the high- or low-part of each pair. + down = the opposite part to "up". + In a shift, we can consider bits to shift from "up"-stream to + "down"-stream, so in a left-shift "up" is the low-part and "down" + is the high-part of each register pair. */ + + rtx out_up = code == ASHIFT ? out_low : out_high; + rtx out_down = code == ASHIFT ? out_high : out_low; + rtx in_up = code == ASHIFT ? in_low : in_high; + rtx in_down = code == ASHIFT ? in_high : in_low; + + gcc_assert (code == ASHIFT || code == ASHIFTRT || code == LSHIFTRT); + gcc_assert (out + && (REG_P (out) || GET_CODE (out) == SUBREG) + && GET_MODE (out) == DImode); + gcc_assert (in + && (REG_P (in) || GET_CODE (in) == SUBREG) + && GET_MODE (in) == DImode); + gcc_assert (amount + && (((REG_P (amount) || GET_CODE (amount) == SUBREG) + && GET_MODE (amount) == SImode) + || CONST_INT_P (amount))); + gcc_assert (scratch1 == NULL + || (GET_CODE (scratch1) == SCRATCH) + || (GET_MODE (scratch1) == SImode + && REG_P (scratch1))); + gcc_assert (scratch2 == NULL + || (GET_CODE (scratch2) == SCRATCH) + || (GET_MODE (scratch2) == SImode + && REG_P (scratch2))); + gcc_assert (!REG_P (out) || !REG_P (amount) + || !HARD_REGISTER_P (out) + || (REGNO (out) != REGNO (amount) + && REGNO (out) + 1 != REGNO (amount))); + + /* Macros to make following code more readable. */ + #define SUB_32(DEST,SRC) \ + gen_addsi3 ((DEST), (SRC), gen_rtx_CONST_INT (VOIDmode, -32)) + #define RSB_32(DEST,SRC) \ + gen_subsi3 ((DEST), gen_rtx_CONST_INT (VOIDmode, 32), (SRC)) + #define SUB_S_32(DEST,SRC) \ + gen_addsi3_compare0 ((DEST), (SRC), \ + gen_rtx_CONST_INT (VOIDmode, -32)) + #define SET(DEST,SRC) \ + gen_rtx_SET (SImode, (DEST), (SRC)) + #define SHIFT(CODE,SRC,AMOUNT) \ + gen_rtx_fmt_ee ((CODE), SImode, (SRC), (AMOUNT)) + #define LSHIFT(CODE,SRC,AMOUNT) \ + gen_rtx_fmt_ee ((CODE) == ASHIFT ? ASHIFT : LSHIFTRT, \ + SImode, (SRC), (AMOUNT)) + #define REV_LSHIFT(CODE,SRC,AMOUNT) \ + gen_rtx_fmt_ee ((CODE) == ASHIFT ? LSHIFTRT : ASHIFT, \ + SImode, (SRC), (AMOUNT)) + #define ORR(A,B) \ + gen_rtx_IOR (SImode, (A), (B)) + #define BRANCH(COND,LABEL) \ + gen_arm_cond_branch ((LABEL), \
Re: trans-mem: functions making indirect calls are not transformed (issue6194061)
On 05/15/2012 02:33 PM, Patrick Marlier wrote: 2012-05-15 Dave Boutcher Patrick Marlier * trans-mem.c (ipa_tm_transform_clone): Transform functions with indirect calls. 2012-05-15 Patrick Marlier * gcc.dg/tm/indirect-2.c: New test. Ok for trunk and branch (if applicable). r~
Re: [PATCH][ARM] Improve 64-bit shifts (non-NEON)
On 16 May 2012 17:09, Andrew Stubbs wrote: > On 16/05/12 11:25, Ramana Radhakrishnan wrote: >> >> Ok with those changes. > > > Hi Ramana, > > Here's an update rebased and modified as requested. > > Can you please confirm that the comments explain what you wanted to know, > and then I will commit it. OK (if no regressions). Ramana
Re: [C++ Patch] PR 44516
On 16 May 2012 17:41, Jason Merrill wrote: > On 05/16/2012 06:54 AM, Paolo Carlini wrote: >> >> isn't the diagnostic machinery able to cope with UNKNOWN_LOCATION? By >> default should be interpreted as input_location, no? > > > That would make sense to me; I don't know if it works that way now, though. No, I don't think it works that way. In fact, if something printed in diagnostics has an unknown location, that seems a bug, because either it is some artificial construct that we should not be giving diagnostics about, or the location passed down is wrong. Of course, for release compilers, we could add a check in diagnostic_report_diagnostic() and use input_location instead. For development compilers we could have a gcc_checking_assert(location != UNKNOWN_LOCATION). But I am not sure what would happen for such a check. In any case, the general rule should be that input_location (or variants using that) should be only used in the parser (who actually knows what input_location is pointing at). Other functions should use a location coming from somewhere else (an argument or a tree). UNKNOWN_LOCATION should be used for anything that is artificial/compiler-generated. Of course, in some cases, we don't have a good location at the point that we want to set one (because we get unknown_location or the tree doesn't have a location), and getting the right location there may be more work that you want to do at the moment. So I would suggest that you simply look at what the underlying function does by default (most use input_location) and use that with a FIXME (or use the non-loc variant if you don't need to touch that function call). Cheers, Manuel.
Re: [C++ Patch] PR 44516
On Wed, May 16, 2012 at 11:56 AM, Manuel López-Ibáñez wrote: > On 16 May 2012 17:41, Jason Merrill wrote: >> On 05/16/2012 06:54 AM, Paolo Carlini wrote: >>> >>> isn't the diagnostic machinery able to cope with UNKNOWN_LOCATION? By >>> default should be interpreted as input_location, no? >> >> >> That would make sense to me; I don't know if it works that way now, though. > > No, I don't think it works that way. In fact, if something printed in > diagnostics has an unknown location, that seems a bug, because either > it is some artificial construct that we should not be giving > diagnostics about, or the location passed down is wrong. Of course, > for release compilers, we could add a check in > diagnostic_report_diagnostic() and use input_location instead. For > development compilers we could have a gcc_checking_assert(location != > UNKNOWN_LOCATION). But I am not sure what would happen for such a > check. hmm; triggering assertions while reporting diagnostics has usually been unfunny because this is the diagnostics: it is called when we think there is something wrong with our internal data structures. Rather, we should handle the situations gracefully. -- Gaby
Re: [C++ Patch] PR 44516
Hi > On 16 May 2012 17:41, Jason Merrill wrote: >> On 05/16/2012 06:54 AM, Paolo Carlini wrote: >>> >>> isn't the diagnostic machinery able to cope with UNKNOWN_LOCATION? By >>> default should be interpreted as input_location, no? >> >> >> That would make sense to me; I don't know if it works that way now, though. > > No, I don't think it works that way. In fact, if something printed in > diagnostics has an unknown location, that seems a bug, because either > it is some artificial construct that we should not be giving > diagnostics about, or the location passed down is wrong. Of course, > for release compilers, we could add a check in > diagnostic_report_diagnostic() and use input_location instead. For > development compilers we could have a gcc_checking_assert(location != > UNKNOWN_LOCATION). But I am not sure what would happen for such a > check. > > In any case, the general rule should be that input_location (or > variants using that) should be only used in the parser (who actually > knows what input_location is pointing at). Other functions should use > a location coming from somewhere else (an argument or a tree). > UNKNOWN_LOCATION should be used for anything that is > artificial/compiler-generated. Of course, in some cases, we don't have > a good location at the point that we want to set one (because we get > unknown_location or the tree doesn't have a location), and getting the > right location there may be more work that you want to do at the > moment. So I would suggest that you simply look at what the underlying > function does by default (most use input_location) and use that with a > FIXME (or use the non-loc variant if you don't need to touch that > function call). Thanks Manuel. So... what shoould I do here? Is the LOC_OR_HERE variant (+ the improvement just mentioned by Jason) right? Should I test and submit that? Jason? Thanks, Paolo
EnabledBy cleanups
Some cleanups in preparation for more EnabledBy goodness. Bootstrapped and regression tested. OK? 2012-05-16 Manuel López-Ibáñez c-family/ * c.opt (--pedantic-errors,-pedantic-errors): Do not handle here. * c-opts.c (c_common_handle_option): Do not handle explicitly Wreturn-type, Wwrite-strings, warn_ecpp, and -pedantic-errors. gcc/ * opts.c (common_handle_option): -pedantic-errors enables -Wpedantic. (enable_warning_as_error): Do not special case Wuninitialized. * optc-gen.awk: Add sanity checks. enabledy-cleanup.diff Description: Binary data
Re: Continue strict-volatile-bitfields fixes
Hi! Ping. On Wed, 09 May 2012 10:01:55 +0800, I wrote: > On Fri, 27 Apr 2012 10:29:06 +0200, Jakub Jelinek wrote: > > On Fri, Apr 27, 2012 at 12:42:41PM +0800, Thomas Schwinge wrote: > > > > > GET_MODE_BITSIZE (lmode)« (8 bits). (With the current sources, lmode > > > > > is > > > > > VOIDmode.) > > > > > > > > > > Is emmitting »BIT_FIELD_REF <*common, 32, 0> & 255« wrong in this > > > > > case, > > > > > or should a later optimization pass be able to figure out that > > > > > »BIT_FIELD_REF <*common, 32, 0> & 255« is in fact the same as > > > > > common->code, and then be able to conflate these? Any suggestions > > > > > where/how to tackle this? > > > > > > > > The BIT_FIELD_REF is somewhat of a red-herring. It is created by > > > > fold-const.c > > > > in optimize_bit_field_compare, code that I think should be removed > > > > completely. > > > > Or it needs to be made aware of strict-volatile bitfield and C++ memory > > > > model > > > > details. > > > > I'd actually very much prefer the latter, just disable > > optimize_bit_field_compare for strict-volatile bitfield mode and when > > avoiding load data races in C++ memory model (that isn't going to be > > default, right?). This optimization is useful, and it is solely about > > loads, so even C++ memory model usually shouldn't care. > > I can't comment on the C++ memory model bits, but I have now tested the > following patch (fixes the issue for SH, no regressions for ARM, x86): > > gcc/ > * fold-const.c (optimize_bit_field_compare): Abort early in the strict > volatile bitfields case. > > Index: fold-const.c > === > --- fold-const.c (revision 186856) > +++ fold-const.c (working copy) > @@ -3342,6 +3342,11 @@ optimize_bit_field_compare (location_t loc, enum t >tree mask; >tree offset; > > + /* In the strict volatile bitfields case, doing code changes here may > prevent > + other optimizations, in particular in a SLOW_BYTE_ACCESS setting. */ > + if (flag_strict_volatile_bitfields > 0) > +return 0; > + >/* Get all the information about the extractions being done. If the bit > size > if the same as the size of the underlying object, we aren't doing an > extraction at all and so can do nothing. We also don't want to Grüße, Thomas pgpyOY9LHZdAp.pgp Description: PGP signature
Re: [C++ Patch] PR 44516
On 16 May 2012 19:06, Gabriel Dos Reis wrote: > On Wed, May 16, 2012 at 11:56 AM, Manuel López-Ibáñez > wrote: >> On 16 May 2012 17:41, Jason Merrill wrote: >>> On 05/16/2012 06:54 AM, Paolo Carlini wrote: isn't the diagnostic machinery able to cope with UNKNOWN_LOCATION? By default should be interpreted as input_location, no? >>> >>> >>> That would make sense to me; I don't know if it works that way now, though. >> >> No, I don't think it works that way. In fact, if something printed in >> diagnostics has an unknown location, that seems a bug, because either >> it is some artificial construct that we should not be giving >> diagnostics about, or the location passed down is wrong. Of course, >> for release compilers, we could add a check in >> diagnostic_report_diagnostic() and use input_location instead. For >> development compilers we could have a gcc_checking_assert(location != >> UNKNOWN_LOCATION). But I am not sure what would happen for such a >> check. > > hmm; triggering assertions while reporting diagnostics has usually > been unfunny because this is the diagnostics: it is called when we > think there is something wrong with our internal data structures. > Rather, we should handle the situations gracefully. My proposal is to assert when checking is enabled (gcc_checking_assert instead of gcc_assert) and degrade gracefully when not (use input_location if given an UNKNOWN_LOCATION). The diagnostics that are called when something goes wrong internally do not use explicit locations (in fact, they use input_location which is often useless, but ok that is only a problem for GCC developers, not for users). Cheers, Manuel.
Re: [google/google-main] Fix regression - SUBTARGET_EXTRA_SPECS overridden by LINUX_GRTE_EXTRA_SPECS (issue 6016047)
Hi Jing, thanks! The SUBTARGET_EXTRA_SPECS is defined in config/i386/gnu-user.h In "linux.h", the original value of "SUBTARGET_EXTRA_SPECS" is overwritten by LINUX_GRTE_EXTRA_SPECS, which is not right! Instead, "SUBTARGET_EXTRA_SPECS" and "LINUX_GRTE_EXTRA_SPECS" must be concatenated. For every target, "SUBTARET_EXTRA_SPECS" has its own value, for example, for darwin, darwin.h just redefines SUBTARGET_EXTRA_SPECS, so the patch does not affect targets other than i386. Regards, -Han On Tue, May 15, 2012 at 3:26 PM, wrote: > > I suspect this patch would change specs of non-i386 platform. For > example, LINUX_GRTE_EXTRA_SPECS is not part of SUBTARGET_EXTRA_SPECS for > darwin. > > Can you tell where SUBTARGET_EXTRA_SPECS is firstly defined for chromeos > toolchain? > > > > > On 2012/05/14 18:32:17, shenhan wrote: >> >> On 2012/04/12 21:14:29, shenhan wrote: >> > Hi, the newest chrome gcc (from google-main) fails to linking > > anything, by >> >> > looking into specs file, it seems that 'link_emulation' section is > > missing in >> >> > specs. >> > >> > The problem I found is that SUBTARGET_EXTRA_SPECS, which is not > > empty for >> >> > chromeos, is overridden by "LINUX_GRTE_EXTRA_SPECS". >> > >> > Please review the proposed a patch. (Tested by buildit bootstrap). >> > >> > -Han > > >> Hi Jing and Rong, could you take a look at this? > > >> Thanks, >> -Han > > > > > http://codereview.appspot.com/6016047/ -- Han Shen | Software Engineer | shen...@google.com | +1-650-440-3330
Re: [google/gcc-4_6_3-mobile] Port r187569 from google/gcc-4_6 branch (issue 6210060)
On 2012/05/16 01:02:26, shenhan wrote: Hi Amhad and Jing, could you take a look at this CL at http://codereview.appspot.com/6210060/ Thanks, -Han Shouldn't this have a corresponding ChangeLog entry and a ChangeLog.google-gcc-4_6 entry? http://codereview.appspot.com/6210060/
Re: [patch] For alpha-vms, unset flag_jump_tables if flag_pic is nonzero
It turns out that there is a second target that doesn't define ASM_OUTPUT_ADDR_DIFF_ELT: pdp11. I'll fix that. The documentation is out of date as a consequence of this patch, because it still says that ASM_OUTPUT_ADDR_DIFF_ELT is optional which is no longer the case (or not unless you take some other actions as you did in the VAX port). paul
Re: [C++ Patch] PR 44516
Hi again, > In any case, the general rule should be that input_location (or > variants using that) should be only used in the parser (who actually > knows what input_location is pointing at). Other functions should use > a location coming from somewhere else (an argument or a tree). > UNKNOWN_LOCATION should be used for anything that is > artificial/compiler-generated. Thus, in short, I understand that what I'm trying to do with my patch using LOC_OR_HERE follows the practice, right? Currently we happily call from the build_x_* functions the build_min_nt function which doesn't set a location for the new node. But it's compiler generated stuff. Definitely we are not regressing on this. Otherwise the build_x_* functions may call build_new_op and the like, which may immediately emit errors and I'm proposing to use LOC_OR_HERE for those, thus we are never going to pass down an UNKNOWN_LOCATION. Honestly, from the point of view of your considerations, I don't see how my patch could cause regressions. Paolo
Re: [C++ Patch] PR 44516
On 16 May 2012 19:09, Paolo Carlini wrote: > Hi > >> On 16 May 2012 17:41, Jason Merrill wrote: >>> On 05/16/2012 06:54 AM, Paolo Carlini wrote: isn't the diagnostic machinery able to cope with UNKNOWN_LOCATION? By default should be interpreted as input_location, no? >>> >>> >>> That would make sense to me; I don't know if it works that way now, though. >> >> No, I don't think it works that way. In fact, if something printed in >> diagnostics has an unknown location, that seems a bug, because either >> it is some artificial construct that we should not be giving >> diagnostics about, or the location passed down is wrong. Of course, >> for release compilers, we could add a check in >> diagnostic_report_diagnostic() and use input_location instead. For >> development compilers we could have a gcc_checking_assert(location != >> UNKNOWN_LOCATION). But I am not sure what would happen for such a >> check. >> >> In any case, the general rule should be that input_location (or >> variants using that) should be only used in the parser (who actually >> knows what input_location is pointing at). Other functions should use >> a location coming from somewhere else (an argument or a tree). >> UNKNOWN_LOCATION should be used for anything that is >> artificial/compiler-generated. Of course, in some cases, we don't have >> a good location at the point that we want to set one (because we get >> unknown_location or the tree doesn't have a location), and getting the >> right location there may be more work that you want to do at the >> moment. So I would suggest that you simply look at what the underlying >> function does by default (most use input_location) and use that with a >> FIXME (or use the non-loc variant if you don't need to touch that >> function call). > > Thanks Manuel. So... what shoould I do here? Is the LOC_OR_HERE variant (+ > the improvement just mentioned by Jason) right? Should I test and submit > that? Jason? I cannot answer because I don't know what are the defaults. What does build_min_nt use by default? - return build_min_nt (COMPONENT_REF, object, name, NULL_TREE); + return build_min_nt_loc (UNKNOWN_LOCATION, COMPONENT_REF, +object, name, NULL_TREE); does it uses always UNKNOWN_LOCATION like above or you are changing the behaviour of this call? -expr = build_new_op (loc, code, LOOKUP_NORMAL, arg1, arg2, NULL_TREE, +expr = build_new_op (LOC_OR_HERE (loc), code, LOOKUP_NORMAL, +arg1, arg2, NULL_TREE, overload, complain); This doesn't seem correct to me because of the reasons Jason gave. If loc is unknown, you are changing the behaviour. - expr = build_new_op (input_location, ARRAY_REF, LOOKUP_NORMAL, arg1, + expr = build_new_op (LOC_OR_HERE (loc), ARRAY_REF, LOOKUP_NORMAL, arg1, arg2, NULL_TREE, /*overload=*/NULL, complain); This (and similar ones that change input_location to LOC_OR_HERE) seem correct to me because either you have a valid location or things stay as before. Of course, whether the valid location is more correct than input_location depends on the context. Which is the really tricky part, specially because during the transition neither option may be optimal. This is hard stuff, so I am very happy that there is someone brave enough to tackle it! ;-) Thanks! Cheers, Manuel.
Re: [RFC] PR 53063 encode group options in .opt files
On 2012/5/10 04:53 AM, Manuel López-Ibáñez wrote: > 2012-05-09 Manuel López-Ibáñez > > PR 53063 > gcc/ > * doc/options.texi (EnabledBy): Document > * opts.c: Include opts.h and options.h before tm.h. > (finish_options): Do not handle some sub-options here... > (common_handle_option): ... instead call common_handle_option_auto here. > * optc-gen.awk: Handle EnabledBy. > * opth-gen.awk: Declare common_handle_option_auto. > * common.opt (Wuninitialized): Use EnabledBy. Delete Init. > (Wmaybe-uninitialized): Likewise. > (Wunused-but-set-variable): Likewise. > (Wunused-function): Likewise. > (Wunused-label): Likewise. > (Wunused-value): Likewise. > (Wunused-variable): Likewise. > * opt-read.awk: Create opt_numbers array. > ada/ > * gcc-interface/misc.c (gnat_parse_file): Move before ... > (gnat_handle_option): ... this. Use handle_generated_option. > c-family/ > * c-opts.c (c_common_handle_option): Use handle_generated_option > to enable sub-options. > fortran/ > * options.c: Include diagnostics.h instead of > diagnostics-core.h. > (set_Wall): Do not see warn_unused here. > (gfc_handle_option): Set it here using handle_generated_option. I'm guessing these changes are the cause of a full C bootstrap (--disable-build-poststage1-with-cxx) failure I'm seeing on trunk. The *_handle_option_auto function prototypes are not seen in options.c, and -Werror -Wmissing-prototypes are in effect (oddly, such strict checking is not enforced in the default post-stage1 C++ bootstrap) Here's a simple one-liner Makefile.in change that seems to make things work. Okay to commit? Thanks, Chung-Lin 2012-05-17 Chung-Lin Tang * Makefile.in (options.c): Add options.h to included header files, before tm.h. Index: Makefile.in === --- Makefile.in (revision 187594) +++ Makefile.in (working copy) @@ -2121,7 +2121,7 @@ options.c: optionlist $(srcdir)/opt-functions.awk $(srcdir)/optc-gen.awk $(AWK) -f $(srcdir)/opt-functions.awk -f $(srcdir)/opt-read.awk \ -f $(srcdir)/optc-gen.awk \ - -v header_name="config.h system.h coretypes.h tm.h" < $< > $@ + -v header_name="config.h system.h coretypes.h options.h tm.h" < $< > $@ options-save.c: optionlist $(srcdir)/opt-functions.awk $(srcdir)/opt-read.awk \ $(srcdir)/optc-save-gen.awk
Re: [C++ Patch] PR 44516
Hi, > > I cannot answer because I don't know what are the defaults. What does > build_min_nt use by default? UNKNOWN_LOCATION. I think this answers most of your concerns. > -expr = build_new_op (loc, code, LOOKUP_NORMAL, arg1, arg2, NULL_TREE, > +expr = build_new_op (LOC_OR_HERE (loc), code, LOOKUP_NORMAL, > + arg1, arg2, NULL_TREE, > overload, complain); > > This doesn't seem correct to me because of the reasons Jason gave. If > loc is unknown, you are changing the behaviour. If loc is UNKNOWN I'm passing down an input_location which seems a definite improvement to me in the light of your last message, given that the called function may immediately error out and try to use that location in the error_at. But I'm not going to insist. > > - expr = build_new_op (input_location, ARRAY_REF, LOOKUP_NORMAL, arg1, > + expr = build_new_op (LOC_OR_HERE (loc), ARRAY_REF, LOOKUP_NORMAL, arg1, > arg2, NULL_TREE, /*overload=*/NULL, complain); > > This (and similar ones that change input_location to LOC_OR_HERE) seem > correct to me because either you have a valid location or things stay > as before. Of course, whether the valid location is more correct than > input_location depends on the context. Which is the really tricky > part, specially because during the transition neither option may be > optimal. Ok. Thanks! Paolo
Re: [RFC] PR 53063 encode group options in .opt files
On 16 May 2012 19:47, Chung-Lin Tang wrote: > On 2012/5/10 04:53 AM, Manuel López-Ibáñez wrote: >> 2012-05-09 Manuel López-Ibáñez >> >> PR 53063 >> gcc/ >> * doc/options.texi (EnabledBy): Document >> * opts.c: Include opts.h and options.h before tm.h. >> (finish_options): Do not handle some sub-options here... >> (common_handle_option): ... instead call common_handle_option_auto >> here. >> * optc-gen.awk: Handle EnabledBy. >> * opth-gen.awk: Declare common_handle_option_auto. >> * common.opt (Wuninitialized): Use EnabledBy. Delete Init. >> (Wmaybe-uninitialized): Likewise. >> (Wunused-but-set-variable): Likewise. >> (Wunused-function): Likewise. >> (Wunused-label): Likewise. >> (Wunused-value): Likewise. >> (Wunused-variable): Likewise. >> * opt-read.awk: Create opt_numbers array. >> ada/ >> * gcc-interface/misc.c (gnat_parse_file): Move before ... >> (gnat_handle_option): ... this. Use handle_generated_option. >> c-family/ >> * c-opts.c (c_common_handle_option): Use handle_generated_option >> to enable sub-options. >> fortran/ >> * options.c: Include diagnostics.h instead of >> diagnostics-core.h. >> (set_Wall): Do not see warn_unused here. >> (gfc_handle_option): Set it here using handle_generated_option. > > I'm guessing these changes are the cause of a full C bootstrap > (--disable-build-poststage1-with-cxx) failure I'm seeing on trunk. The > *_handle_option_auto function prototypes are not seen in options.c, and > -Werror -Wmissing-prototypes are in effect (oddly, such strict checking > is not enforced in the default post-stage1 C++ bootstrap) Yep, We should add -Wmissing-declarations to the post-stage1 flags, which also exists in C. Could you also add that to your patch? > Here's a simple one-liner Makefile.in change that seems to make things > work. Okay to commit? I cannot approve, but I guess it is either this or also declare the functions in options.c, which seems overkill. Cheers, Manuel.
Re: [C++ Patch] PR 44516
On 16 May 2012 19:53, Paolo Carlini wrote: >> - expr = build_new_op (loc, code, LOOKUP_NORMAL, arg1, arg2, NULL_TREE, >> + expr = build_new_op (LOC_OR_HERE (loc), code, LOOKUP_NORMAL, >> + arg1, arg2, NULL_TREE, >> overload, complain); >> >> This doesn't seem correct to me because of the reasons Jason gave. If >> loc is unknown, you are changing the behaviour. > > If loc is UNKNOWN I'm passing down an input_location which seems a definite > improvement to me in the light of your last message, given that the called > function may immediately error out and try to use that location in the > error_at. But I'm not going to insist. > But then, this should have happened before, no? Or do we get UNKNOWN_LOCATION now but not before? That would indicate something has changed in an upper level that is not quite right. The point is that some things may naturally have UNKNOWN_LOCATION, however, such things should never be used in diagnostics. That indicates a bug. So, if the caller is passing down loc == UNKNOWN_LOCATION here and this new operation is never artificial, then the caller is wrongly passing down UNKNOWN_LOCATION. In this particular case, you may have just uncovered a latent bug, and use the LOC_OR_HERE trick to defer fixing it for later. That seems OK to me, but then you should add a FIXME for such case, saying "This should be just LOC but something is passing down UNKNOWN_LOCATION and this breaks so and so" I remember well that we had such nasty cases when Aldy and me did the column-information mega-patch, and still we got some of the locations wrong. Cheers, Manuel.
Re: Turn check macros into functions. (issue6188088)
> "Lawrence" == Lawrence Crowl writes: Lawrence> The effect is that it now possible to get useful responses to gdb Lawrence> command like Lawrence> (gdb) print DECL_FUNCTION_CODE (decl) Doesn't this mean that if you have checking enabled, and you use the wrong macro on some tree, cc1 will crash? That seems like a distinct minus to me. Tom
Re: [C++ Patch] PR 44516
On 05/16/2012 01:15 PM, Manuel López-Ibáñez wrote: My proposal is to assert when checking is enabled (gcc_checking_assert instead of gcc_assert) and degrade gracefully when not (use input_location if given an UNKNOWN_LOCATION). That makes sense if we want to require the front end to move to explicit locations everywhere at once, but prevents a gradual transition. Currently diagnostics just use input_location. Paolo's patch passes in explicit locations to a bunch of functions, so now we can use those in diagnostics. But if for some reason we pass UNKNOWN_LOCATION in because some part of the compiler hasn't been updated yet, we get an ICE instead of the merely inaccurate location we had before. Jason
Re: [C++ Patch] PR 44516
Hi, > On 16 May 2012 19:53, Paolo Carlini wrote: >>> -expr = build_new_op (loc, code, LOOKUP_NORMAL, arg1, arg2, NULL_TREE, >>> +expr = build_new_op (LOC_OR_HERE (loc), code, LOOKUP_NORMAL, >>> + arg1, arg2, NULL_TREE, >>> overload, complain); >>> >>> This doesn't seem correct to me because of the reasons Jason gave. If >>> loc is unknown, you are changing the behaviour. >> >> If loc is UNKNOWN I'm passing down an input_location which seems a definite >> improvement to me in the light of your last message, given that the called >> function may immediately error out and try to use that location in the >> error_at. But I'm not going to insist. >> > > But then, this should have happened before, no? Or do we get > UNKNOWN_LOCATION now but not before? That would indicate something has > changed in an upper level that is not quite right. The point is that > some things may naturally have UNKNOWN_LOCATION, however, such things > should never be used in diagnostics. That indicates a bug. So, if the > caller is passing down loc == UNKNOWN_LOCATION here and this new > operation is never artificial, then the caller is wrongly passing down > UNKNOWN_LOCATION. > > In this particular case, you may have just uncovered a latent bug, and > use the LOC_OR_HERE trick to defer fixing it for later. That seems OK > to me, but then you should add a FIXME for such case, saying "This > should be just LOC but something is passing down UNKNOWN_LOCATION and > this breaks so and so" To clarify: I didn't do that out of "empirical necessity", because not doing it caused regression, or I had a new testcase which required it. I did it for consistency with all the other build_x_* functions, because I gathered that we never ever want to pass an UNKNOWN_LOCATION to a diagnostic function. But really I'm not going to insist, I can as well only in this build_x_* not use the LOC_OR_HERE trick, just, hey, let's agree on something which is a small but clear improvement and move to the next step in this (long, I'm afraid) path. Paolo
Re: PING PATCH: break lines in announce_function
> "Manuel" == Manuel López-Ibáñez writes: Manuel> Actually, I wonder how you (Richard) and other GCC hackers work with Manuel> and debug GCC, because it is a real pain in the ass. Manuel> * All the TREE_ macros don't work. Manuel> * __extension__ prevents GDB from evaluating many things. See that other thread... When I was debugging gcc regularly I added a bunch of "macro define"s to .gdbinit, and built with -g3. This fixed the macro problem and the __extension__ problem. Manuel> * Hitting auto-completion in GDB means staring at the window for 5-10 Manuel> minutes until it decides to stop listing stuff. Report completion bugs to gdb. There's only so much gdb can do here. But maybe we could have some mode to be more selective about what to complete. Tom
Re: PING PATCH: break lines in announce_function
> "Manuel" == Manuel López-Ibáñez writes: Manuel> It seems it will never work for statement expressions: Manuel> http://article.gmane.org/gmane.comp.gcc.devel/107339 It could be done, but it is non-trivial for sure. Manuel> I see how these small functions can quickly become annoying. Is it Manuel> really that hard to make GDB ignore some functions? There's a new "skip" feature in gdb specifically for this. It is in gdb 7.4. Tom
Re: EnabledBy cleanups
On Wed, 16 May 2012, Manuel L?pez-Ib??ez wrote: > Some cleanups in preparation for more EnabledBy goodness. > > Bootstrapped and regression tested. > > OK? > > 2012-05-16 Manuel L?pez-Ib??ez > > c-family/ > * c.opt (--pedantic-errors,-pedantic-errors): Do not handle here. > * c-opts.c (c_common_handle_option): Do not handle explicitly > Wreturn-type, Wwrite-strings, warn_ecpp, and -pedantic-errors. > gcc/ > * opts.c (common_handle_option): -pedantic-errors enables -Wpedantic. > (enable_warning_as_error): Do not special case Wuninitialized. > * optc-gen.awk: Add sanity checks. OK. -- Joseph S. Myers jos...@codesourcery.com
Re: [RFC] PR 53063 encode group options in .opt files
On Thu, 17 May 2012, Chung-Lin Tang wrote: > 2012-05-17 Chung-Lin Tang > > * Makefile.in (options.c): Add options.h to included header > files, before tm.h. OK. -- Joseph S. Myers jos...@codesourcery.com
[PATCH] Fix get_maxval_strlen for COND_EXPR after changing them to ternary
When COND_EXPR was changed from GIMPLE_SINGLE_RHS to GIMPLE_TERNARY_RHS, get_maxval_strlen was not updated for that changed. With a patch which has a late PHIOPT produce COND_EXPR, I saw a couple of regressions (pr23484-chk.c and strncpy-chk.c). This patch fixes get_maxval_strlen for that change. OK for the trunk and 4.7 branch? Bootstrapped and tested on x86_64-linux-gnu with no regressions. And in 4.7.0 with the patch for PHIOPT producing COND_EXPR on both x86_64-linux-gnu and mipsisa64-elf. Thanks, Andrew Pinski ChangeLog: * gimple-fold.c (get_maxval_strlen): Move COND_EXPR handling under GIMPLE_ASSIGN. Index: gimple-fold.c === --- gimple-fold.c (revision 187579) +++ gimple-fold.c (working copy) @@ -670,13 +670,10 @@ get_maxval_strlen (tree arg, tree *lengt if (TREE_CODE (arg) != SSA_NAME) { - if (TREE_CODE (arg) == COND_EXPR) -return get_maxval_strlen (COND_EXPR_THEN (arg), length, visited, type) - && get_maxval_strlen (COND_EXPR_ELSE (arg), length, visited, type); /* We can end up with &(*iftmp_1)[0] here as well, so handle it. */ - else if (TREE_CODE (arg) == ADDR_EXPR - && TREE_CODE (TREE_OPERAND (arg, 0)) == ARRAY_REF - && integer_zerop (TREE_OPERAND (TREE_OPERAND (arg, 0), 1))) + if (TREE_CODE (arg) == ADDR_EXPR + && TREE_CODE (TREE_OPERAND (arg, 0)) == ARRAY_REF + && integer_zerop (TREE_OPERAND (TREE_OPERAND (arg, 0), 1))) { tree aop0 = TREE_OPERAND (TREE_OPERAND (arg, 0), 0); if (TREE_CODE (aop0) == INDIRECT_REF @@ -736,6 +733,13 @@ get_maxval_strlen (tree arg, tree *lengt tree rhs = gimple_assign_rhs1 (def_stmt); return get_maxval_strlen (rhs, length, visited, type); } +else if (gimple_assign_rhs_code (def_stmt) == COND_EXPR) + { + tree op2 = gimple_assign_rhs2 (def_stmt); + tree op3 = gimple_assign_rhs3 (def_stmt); +return get_maxval_strlen (op2, length, visited, type) + && get_maxval_strlen (op3, length, visited, type); + } return false; case GIMPLE_PHI:
Ping: [PATCH] Hoist adjacent pointer loads
Ping. Thanks, Bill On Thu, 2012-05-03 at 09:33 -0500, William J. Schmidt wrote: > This patch was posted for comment back in February during stage 4. It > addresses a performance issue noted in the EEMBC routelookup benchmark > on a common idiom: > > if (...) > x = y->left; > else > x = y->right; > > If the two loads can be hoisted out of the if/else, the if/else can be > replaced by a conditional move instruction on architectures that support > one. Because this speculates one of the loads, the patch constrains the > optimization to avoid introducing page faults. > > Bootstrapped and regression tested on powerpc-unknown-linux-gnu with no > new failures. The patch provides significant improvement to the > routelookup benchmark, and is neutral on SPEC cpu2000/cpu2006. > > One question is what optimization level should be required for this. > Because of the speculation, -O3 might be in order. I don't believe > -Ofast is required as there is no potential correctness issue involved. > Right now the patch doesn't check the optimization level (like the rest > of the phi-opt transforms), which is likely a poor choice. > > Ok for trunk? > > Thanks, > Bill > > > 2012-05-03 Bill Schmidt > > * tree-ssa-phiopt.c (tree_ssa_phiopt_worker): Add argument to forward > declaration. > (hoist_adjacent_loads, gate_hoist_loads): New forward declarations. > (tree_ssa_phiopt): Call gate_hoist_loads. > (tree_ssa_cs_elim): Add parm to tree_ssa_phiopt_worker call. > (tree_ssa_phiopt_worker): Add do_hoist_loads to formal arg list; call > hoist_adjacent_loads. > (local_reg_dependence): New function. > (local_mem_dependence): Likewise. > (hoist_adjacent_loads): Likewise. > (gate_hoist_loads): Likewise. > * common.opt (fhoist-adjacent-loads): New switch. > * Makefile.in (tree-ssa-phiopt.o): Added dependencies. > * params.def (PARAM_MIN_CMOVE_STRUCT_ALIGN): New param. > > > Index: gcc/tree-ssa-phiopt.c > === > --- gcc/tree-ssa-phiopt.c (revision 187057) > +++ gcc/tree-ssa-phiopt.c (working copy) > @@ -37,9 +37,17 @@ along with GCC; see the file COPYING3. If not see > #include "cfgloop.h" > #include "tree-data-ref.h" > #include "tree-pretty-print.h" > +#include "gimple-pretty-print.h" > +#include "insn-config.h" > +#include "expr.h" > +#include "optabs.h" > > +#ifndef HAVE_conditional_move > +#define HAVE_conditional_move (0) > +#endif > + > static unsigned int tree_ssa_phiopt (void); > -static unsigned int tree_ssa_phiopt_worker (bool); > +static unsigned int tree_ssa_phiopt_worker (bool, bool); > static bool conditional_replacement (basic_block, basic_block, >edge, edge, gimple, tree, tree); > static int value_replacement (basic_block, basic_block, > @@ -53,6 +61,9 @@ static bool cond_store_replacement (basic_block, b > static bool cond_if_else_store_replacement (basic_block, basic_block, > basic_block); > static struct pointer_set_t * get_non_trapping (void); > static void replace_phi_edge_with_variable (basic_block, edge, gimple, tree); > +static void hoist_adjacent_loads (basic_block, basic_block, > + basic_block, basic_block); > +static bool gate_hoist_loads (void); > > /* This pass tries to replaces an if-then-else block with an > assignment. We have four kinds of transformations. Some of these > @@ -138,12 +149,56 @@ static void replace_phi_edge_with_variable (basic_ > bb2: > x = PHI ; > > - A similar transformation is done for MAX_EXPR. */ > + A similar transformation is done for MAX_EXPR. > > + > + This pass also performs a fifth transformation of a slightly different > + flavor. > + > + Adjacent Load Hoisting > + -- > + > + This transformation replaces > + > + bb0: > + if (...) goto bb2; else goto bb1; > + bb1: > + x1 = ().field1; > + goto bb3; > + bb2: > + x2 = ().field2; > + bb3: > + # x = PHI ; > + > + with > + > + bb0: > + x1 = ().field1; > + x2 = ().field2; > + if (...) goto bb2; else goto bb1; > + bb1: > + goto bb3; > + bb2: > + bb3: > + # x = PHI ; > + > + The purpose of this transformation is to enable generation of conditional > + move instructions such as Intel CMOVE or PowerPC ISEL. Because one of > + the loads is speculative, the transformation is restricted to very > + specific cases to avoid introducing a page fault. We are looking for > + the common idiom: > + > + if (...) > + x = y->left; > + else > + x = y->right; > + > + where left and right are typically adjacent pointers in a tree structure. > */ > + > static unsigned int > tree_ssa_phiopt (void) > { > - return tree_ssa_phiopt_worker (false); > + return tree_ssa_phiopt_worker (false, gate_hoist_loads ());
[PATCH] Try to expand COND_EXPR using addcc
Hi, This is just like my previous patch to expand COND_EXPR using conditional moves but this time using addcc instead. I had to fix a bug in emit_conditional_add where it was swapping op2 and op3 which can never happen. Also the documentation for both emit_conditional_add and add@var{mode}cc was in correct in saying the first (non comparison) operand was the result when the condition was true, it should have been when it was false. So it does op2 if cond is false and op2+op3 when the cond is true. This actually makes better sense than what the documentation was before too. OK? Bootstrapped and tested on x86_64-linux-gnu? Thanks, Andrew Pinski ChangeLog: * optabs.c (emit_conditional_add): Correct comment about the arguments. Remove code which might swap op2 and op3 since they cannot be swapped. * expr.c (get_condition_from_operand): New function. (get_def_noter_for_expr_with_code): New function. (expand_cond_expr_using_addcc): New function. (expand_cond_expr_using_cmove): Use get_condition_from_operand instead of doing it inline. (expand_expr_real_2 ): Call expand_cond_expr_using_addcc before trying conditional moves. * doc/md.texi (add@var{mode}cc): Fix document about how the arguments are used. Index: optabs.c === --- optabs.c(revision 187579) +++ optabs.c(working copy) @@ -4565,7 +4565,7 @@ can_conditionally_move_p (enum machine_m the mode to use should they be constants. If it is VOIDmode, they cannot both be constants. - OP2 should be stored in TARGET if the comparison is true, otherwise OP2+OP3 + OP2 should be stored in TARGET if the comparison is false, otherwise OP2+OP3 should be stored there. MODE is the mode to use should they be constants. If it is VOIDmode, they cannot both be constants. @@ -4579,7 +4579,6 @@ emit_conditional_add (rtx target, enum r { rtx tem, comparison, last; enum insn_code icode; - enum rtx_code reversed; /* If one operand is constant, make it the second one. Only do this if the other operand is not constant as well. */ @@ -4603,16 +4602,6 @@ emit_conditional_add (rtx target, enum r if (cmode == VOIDmode) cmode = GET_MODE (op0); - if (swap_commutative_operands_p (op2, op3) - && ((reversed = reversed_comparison_code_parts (code, op0, op1, NULL)) - != UNKNOWN)) -{ - tem = op2; - op2 = op3; - op3 = tem; - code = reversed; -} - if (mode == VOIDmode) mode = GET_MODE (op2); Index: expr.c === --- expr.c (revision 187579) +++ expr.c (working copy) @@ -7867,6 +7867,181 @@ expand_expr_real (tree exp, rtx target, return ret; } +/* Try to get the condition expression from the OP operand. Setting the OP0 + to the lhs and OP1 to the rhs and CODE to the code of the condition. */ + +static void +get_condition_from_operand (tree op, tree *op0, tree *op1, +enum tree_code *code) +{ + gimple srcstmt; + + if (TREE_CODE (op) == SSA_NAME + && (srcstmt = get_def_for_expr_class (op, tcc_comparison))) +{ + *op0 = gimple_assign_rhs1 (srcstmt); + *op1 = gimple_assign_rhs2 (srcstmt); + *code = gimple_assign_rhs_code (srcstmt); +} + else if (TREE_CODE (op) == SSA_NAME + && (srcstmt = get_def_for_expr (op, BIT_NOT_EXPR))) +{ + *op0 = gimple_assign_rhs1 (srcstmt); + *op1 = fold_convert (TREE_TYPE (op), integer_zero_node); + *code = EQ_EXPR; +} + else if (TREE_CODE_CLASS (TREE_CODE (op)) == tcc_comparison) +{ + *op0 = TREE_OPERAND (op, 0); + *op1 = TREE_OPERAND (op, 1); + *code = TREE_CODE (op); +} + /* If it is neither, just assume OP != 0. */ + else +{ + *op0 = op; + *op1 = fold_convert (TREE_TYPE (op), integer_zero_node); + *code = NE_EXPR; +} +} + +/* Given a ssa_name in NAME see if it was defined by an assignment and + set CODE to be the code and ARG1 to the first operand on the rhs and ARG2 + to the second operand on the rhs. */ + +static gimple +get_def_noter_for_expr_with_code (tree name, enum tree_code code) +{ + gimple def; + + if (TREE_CODE (name) == SSA_NAME) +{ + def = SSA_NAME_DEF_STMT (name); + + if (def && is_gimple_assign (def) + && gimple_assign_rhs_code (def) == code) +return def; +} + return NULL; +} + +/* Try to expand the conditional expression which is represented by + TREEOP0 ? TREEOP1 : TREEOP2 using conditonal adds. If succeseds + return the rtl reg which represents the result. Otherwise return + NULL_RTL. */ + +static rtx +expand_cond_expr_using_addcc (tree treeop0, tree treeop1, tree treeop2) +{ + tree diff; + rtx op1, op2, op00, op01, temp; + tree cmpop0, cmpop1, type; + enum tree_code cmpcode; + enum rtx_code comparison_code; + enum machine_mode comparison_mode, mode; + rtx seq; + int unsignedp; + gimple srcstmt; + +
[C++ Patch] Produce canonical names for debug info without changing normal pretty-printing (issue6215052)
This patch adds new flags and defines such that the C++ decl pretty printer prints both canonical dwarf names for decls without perturbing normal error message output. It addresses the issues with the earlier patches submitted as: http://gcc.gnu.org/ml/gcc-patches/2012-05/msg00516.html http://gcc.gnu.org/ml/gcc-patches/2012-05/msg00512.html Which are withdrawn. This patch requires no changes to the testsuite and does not produce visible changes to gcc's output except to dwarf consumers, which will now all agree on the names of functions. Tested with a full bootstrap. OK for mainline? Sterling 2012-05-16 Sterling Augustine * gcc/c-family/c-pretty-print.h (pp_c_flag_gnu_v3): New enumerator. * gcc/c-family/c-pretty-print.c (pp_c_specifier_qualifier_list): Check it at both the start and end of the function. * gcc/cp/cp-tree.h (TFF_MATCH_GNU_V3_DEMANGLER): Define and comment. * gcc/cp/error.c (dump_decl): Print appropriate string for anonymous namespace based on pp_c_flag_gnu_v3. (decl_as_string): Set cxx_pp->flags based on TFF_MATCH_GNU_V3_DEMANGLER. (lang_decl_name): Handle unnamed namespace decls. * gcc/cp/cp-lang.c (cxx_dwarf_name): Call decl_as_string for namespace decls. Index: gcc/c-family/c-pretty-print.c === --- gcc/c-family/c-pretty-print.c (revision 187603) +++ gcc/c-family/c-pretty-print.c (working copy) @@ -446,8 +446,9 @@ pp_c_specifier_qualifier_list (c_pretty_printer *p { const enum tree_code code = TREE_CODE (t); - if (TREE_CODE (t) != POINTER_TYPE) + if (!(pp->flags & pp_c_flag_gnu_v3) && TREE_CODE (t) != POINTER_TYPE) pp_c_type_qualifier_list (pp, t); + switch (code) { case REFERENCE_TYPE: @@ -494,6 +495,8 @@ pp_c_specifier_qualifier_list (c_pretty_printer *p pp_simple_type_specifier (pp, t); break; } + if ((pp->flags & pp_c_flag_gnu_v3) && TREE_CODE (t) != POINTER_TYPE) +pp_c_type_qualifier_list (pp, t); } /* parameter-type-list: Index: gcc/c-family/c-pretty-print.h === --- gcc/c-family/c-pretty-print.h (revision 187603) +++ gcc/c-family/c-pretty-print.h (working copy) @@ -30,7 +30,8 @@ along with GCC; see the file COPYING3. If not see typedef enum { pp_c_flag_abstract = 1 << 1, - pp_c_flag_last_bit = 2 + pp_c_flag_last_bit = 2, + pp_c_flag_gnu_v3 = 4 } pp_c_pretty_print_flags; Index: gcc/cp/error.c === --- gcc/cp/error.c (revision 187603) +++ gcc/cp/error.c (working copy) @@ -1028,7 +1028,12 @@ dump_decl (tree t, int flags) dump_scope (CP_DECL_CONTEXT (t), flags); flags &= ~TFF_UNQUALIFIED_NAME; if (DECL_NAME (t) == NULL_TREE) - pp_cxx_ws_string (cxx_pp, M_("{anonymous}")); +{ + if (!(pp_c_base (cxx_pp)->flags & pp_c_flag_gnu_v3)) +pp_cxx_ws_string (cxx_pp, M_("{anonymous}")); + else +pp_cxx_ws_string (cxx_pp, M_("(anonymous namespace)")); +} else pp_cxx_tree_identifier (cxx_pp, DECL_NAME (t)); } @@ -2561,6 +2566,8 @@ decl_as_string (tree decl, int flags) { reinit_cxx_pp (); pp_translate_identifiers (cxx_pp) = false; + if (flags & TFF_MATCH_GNU_V3_DEMANGLER) +pp_c_base (cxx_pp)->flags |= pp_c_flag_gnu_v3; dump_decl (decl, flags); return pp_formatted_text (cxx_pp); } @@ -2596,6 +2603,9 @@ lang_decl_name (tree decl, int v, bool translate) if (TREE_CODE (decl) == FUNCTION_DECL) dump_function_name (decl, TFF_PLAIN_IDENTIFIER); + else if ((DECL_NAME (decl) == NULL_TREE) + && TREE_CODE (decl) == NAMESPACE_DECL) +dump_decl (decl, TFF_PLAIN_IDENTIFIER); else dump_decl (DECL_NAME (decl), TFF_PLAIN_IDENTIFIER); Index: gcc/cp/cp-lang.c === --- gcc/cp/cp-lang.c(revision 187603) +++ gcc/cp/cp-lang.c(working copy) @@ -120,8 +120,14 @@ cxx_dwarf_name (tree t, int verbosity) if (verbosity >= 2) return decl_as_string (t, TFF_DECL_SPECIFIERS | TFF_UNQUALIFIED_NAME - | TFF_NO_OMIT_DEFAULT_TEMPLATE_ARGUMENTS); + | TFF_NO_OMIT_DEFAULT_TEMPLATE_ARGUMENTS + | TFF_MATCH_GNU_V3_DEMANGLER); + /* decl_as_string handles namespaces--especially anonymous ones--more + appropriately for debugging than cxx_printable_name. But + cxx_printable_name handles templates and global ctors and dtors better. */ + if (TREE_CODE (t) == NAMESPACE_DECL) +return decl_as_string (t, TFF_MATCH_GNU_V3_DEMANGLER); return cxx_printable_name (t, verbosity); } Index: gcc/cp/cp-tree.h
Re: [PATCH] Fix get_maxval_strlen for COND_EXPR after changing them to ternary
On Wed, May 16, 2012 at 12:34:18PM -0700, Andrew Pinski wrote: > When COND_EXPR was changed from GIMPLE_SINGLE_RHS to > GIMPLE_TERNARY_RHS, get_maxval_strlen was not updated for that > changed. With a patch which has a late PHIOPT produce COND_EXPR, I > saw a couple of regressions (pr23484-chk.c and strncpy-chk.c). > > This patch fixes get_maxval_strlen for that change. > > OK for the trunk and 4.7 branch? Bootstrapped and tested on > x86_64-linux-gnu with no regressions. And in 4.7.0 with the patch for > PHIOPT producing COND_EXPR on both x86_64-linux-gnu and mipsisa64-elf. Ok for both, but please fix up whitespace (on all lines you've changed replace 8 spaces with tab, you use tabs in some lines and spaces in other lines). > ChangeLog: > * gimple-fold.c (get_maxval_strlen): Move COND_EXPR handling under > GIMPLE_ASSIGN. Jakub
Re: Turn check macros into functions. (issue6188088)
On 5/16/12, Richard Guenther wrote: > On May 16, 2012 Diego Novillo wrote: > > On 12-05-16 09:00 , Richard Guenther wrote: > > > On May 16, 2012 Diego Novillo wrote: > > > > On 12-05-16 05:41 , Richard Guenther wrote: > > > > > What's the reason for templating these functions? > > > > > They all take trees as parameter!? The check templates replicate the behavior of the check macros, which carefully return the type of their argument. In essence, the check macros were templates. > > > > True. I don't recall what Lawrence had in mind, but I > > > > remember that by using templates here, you don't need to > > > > deal with the mess of distinguishing tree from const_tree, > > > > and having to force constness out with ugly casts. > > > > > > Well, but you get no typesafety for it in return. You can > > > simply provide a const_tree overload. > > > > There's less typing if you use the template variant. Not sure > > why you say there is less type safety with templates. > > Because it accepts any type as tree argument? It's of course not > less type safety than using macros, but less type safety compared > to not using templates. The overload works if the only types are tree and const_tree. In the future, we may wish to refine the types so that, for example, we can have a pointer to a decl and ask if it is a var decl. The implementation approach is to make small steps. It's a tradeoff between small steps that may show only incremental value, or big steps that risk rejection. In addition, merging is easier with small steps. > > > With using templates you are also forced to retain these > > > functions in the header file - another thing that I suppose > > > you guys were about to "fix"? It's after all debugging code. > > > > No, templated functions must always stay in the header file. > > There is no changing that. > > If they ain't templates they are not templates. And thus do not > need to stay in the header. Not sure what you are after here ;) Personally, I would like to get to the point where we take advantage of static typing and the vast bulk of these checks are eliminated. > > > > Additionally, templates are producing slightly smaller > > > > code than the non-template variant (about 0.2% smaller). > > > > I'm not actually sure why this happens, but it's consistent > > > > across all binaries. > > > > > > Well, what did you compare? Make sure to not have > > > -fkeep-inline-functions, otherwise you get all bodies as > > > compared to only the instantiated bodies. > > > > Two bootstrapped compilers built exactly the same, except one > > was using the template version, the other using the straight > > inline functions with const_tree parameters and CONST_CAST_TREE > > in return values. > > That's of course not exactly the same. The checking fns should > be able to unconditionally use const_tree anyway. The two versions are inline templates functions versus macros. I expect the non-template version would be roughly the same size as the template version. -- Lawrence Crowl
m32r-rtems libgcc config missing crtinit/fini
Hi When moving stuff into libgcc, a line for m32r-rtems got lost. This is PR53314. Is this OK for the head and 4.7? 2012-05-16 Joel Sherrill * config.host (m32r-*-rtems*): Include crtinit.o and crtfinit.o as extra_parts. diff --git a/libgcc/config.host b/libgcc/config.host index 14c705b..b79f321 100644 --- a/libgcc/config.host +++ b/libgcc/config.host @@ -679,6 +682,7 @@ m32r-*-elf*) ;; m32r-*-rtems*) tmake_file="$tmake_file m32r/t-m32r t-fdpbit" + extra_parts="$extra_parts crtinit.o crtfini.o" ;; m32rle-*-elf*) tmake_file=t-fdpbit -- Joel Sherrill, Ph.D. Director of Research& Development joel.sherr...@oarcorp.comOn-Line Applications Research Ask me about RTEMS: a free RTOS Huntsville AL 35805 Support Available (256) 722-9985
Re: Turn check macros into functions. (issue6188088)
On 5/16/12, Tom Tromey wrote: >> "Lawrence" == Lawrence Crowl writes: > > Lawrence> The effect is that it now possible to get useful responses > Lawrence> to gdb command like > Lawrence> (gdb) print DECL_FUNCTION_CODE (decl) > > Doesn't this mean that if you have checking enabled, and you use the > wrong macro on some tree, cc1 will crash? That seems like a distinct > minus to me. Yes, it does mean that, but it is a net overall improvement. The use scenario is in copying an expression from the source to gdb to figure out its value. In this scenario, you typically are not going to have the wrong tree type. With the macro, gdb refuses to evaluate the print even when the code is correct. It complains about extensions or gets confused. These extensions are not on gdb's list of things to implement. The only way to "make it work" with the macro is to define overriding macros in gdb. If you do that, the patch introduces no behavioral change. So, the patch is a net positive. -- Lawrence Crowl
Re: [patch] Fix debug info of nested inline functions
> Right, and that's why we want your change to split the nested function > into abstract and concrete instances. But then it should be fine to > attach the abstract instance to the abstract parent normally, I would > think. Indeed, this works, but I need to use function_possibly_abstracted_p instead of cgraph_function_possibly_inlined_p in gen_subprogram_die to get DW_AT_inline. Revised patch attached. It still generates the same (fixed) debug info for the reduced testcase. I'll do a full testing cycle if you're happy with it. * dwarf2out.c (function_possibly_abstracted_p): New static function. (gen_subprogram_die): Use it function_possibly_abstracted_p in lieu of cgraph_function_possibly_inlined_p. (gen_inlined_subroutine_die): Return if the origin is to be ignored. (process_scope_var): Do not emit concrete instances of abstracted nested functions from here. (gen_decl_die): Emit the abstract instance if the function is possibly abstracted and not only possibly inlined. (dwarf2out_finish): Find the first non-abstract parent instance and attach concrete instances on the limbo list to it. -- Eric Botcazou Index: dwarf2out.c === --- dwarf2out.c (revision 187533) +++ dwarf2out.c (working copy) @@ -16586,6 +16586,22 @@ gen_call_site_die (tree decl, dw_die_ref return die; } +/* Return true if an abstract instance of function DECL can be generated in + the debug information. */ + +static bool +function_possibly_abstracted_p (tree decl) +{ + while (decl) +{ + if (cgraph_function_possibly_inlined_p (decl)) + return true; + decl = decl_function_context (decl); +} + + return false; +} + /* Generate a DIE to represent a declared function (either file-scope or block-local). */ @@ -16738,14 +16754,14 @@ gen_subprogram_die (tree decl, dw_die_re { if (DECL_DECLARED_INLINE_P (decl)) { - if (cgraph_function_possibly_inlined_p (decl)) + if (function_possibly_abstracted_p (decl)) add_AT_unsigned (subr_die, DW_AT_inline, DW_INL_declared_inlined); else add_AT_unsigned (subr_die, DW_AT_inline, DW_INL_declared_not_inlined); } else { - if (cgraph_function_possibly_inlined_p (decl)) + if (function_possibly_abstracted_p (decl)) add_AT_unsigned (subr_die, DW_AT_inline, DW_INL_inlined); else add_AT_unsigned (subr_die, DW_AT_inline, DW_INL_not_inlined); @@ -17674,6 +17690,8 @@ gen_inlined_subroutine_die (tree stmt, d gcc_assert (! BLOCK_ABSTRACT (stmt)); decl = block_ultimate_origin (stmt); + if (DECL_IGNORED_P (decl)) +return; /* Emit info for the abstract instance first, if we haven't yet. We must emit this even if the block is abstract, otherwise when we @@ -18615,6 +18633,7 @@ gen_block_die (tree stmt, dw_die_ref con /* Process variable DECL (or variable with origin ORIGIN) within block STMT and add it to CONTEXT_DIE. */ + static void process_scope_var (tree stmt, tree decl, tree origin, dw_die_ref context_die) { @@ -18632,8 +18651,15 @@ process_scope_var (tree stmt, tree decl, if (die != NULL && die->die_parent == NULL) add_child_die (context_die, die); else if (TREE_CODE (decl_or_origin) == IMPORTED_DECL) -dwarf2out_imported_module_or_decl_1 (decl_or_origin, DECL_NAME (decl_or_origin), +dwarf2out_imported_module_or_decl_1 (decl_or_origin, + DECL_NAME (decl_or_origin), stmt, context_die); + /* Do not emit concrete instances of abstracted nested functions within + concrete instances of parent functions. */ + else if (TREE_CODE (decl_or_origin) == FUNCTION_DECL + && die + && get_AT (die, DW_AT_inline)) +; else gen_decl_die (decl, origin, context_die); } @@ -18980,11 +19006,11 @@ gen_decl_die (tree decl, tree origin, dw ? DECL_ORIGIN (origin) : DECL_ABSTRACT_ORIGIN (decl)); - /* If we're emitting an out-of-line copy of an inline function, + /* If we're emitting an out-of-line copy of an abstracted function, emit info for the abstract instance and set up to refer to it. */ - else if (cgraph_function_possibly_inlined_p (decl) - && ! DECL_ABSTRACT (decl) - && ! class_or_namespace_scope_p (context_die) + else if (!DECL_ABSTRACT (decl) + && function_possibly_abstracted_p (decl) + && !class_or_namespace_scope_p (context_die) /* dwarf2out_abstract_function won't emit a die if this is just a declaration. We must avoid setting DECL_ABSTRACT_ORIGIN in that case, because that works only if we have a die. */ @@ -21982,8 +22008,19 @@ dwarf2out_finish (const char *filename) { dw_die_ref origin = get_AT_ref (die, DW_AT_abstract_origin); - if (origin && origin->die_parent) - add_child_die (origin->die_parent, die); + if (origin) + { + /* Find the first non-abstract parent instance. */
Re: [patch] For alpha-vms, unset flag_jump_tables if flag_pic is nonzero
On May 16, 2012, at 1:27 PM, wrote: > It turns out that there is a second target that doesn't define > ASM_OUTPUT_ADDR_DIFF_ELT: pdp11. I'll fix that. > > The documentation is out of date as a consequence of this patch, because it > still says that ASM_OUTPUT_ADDR_DIFF_ELT is optional which is no longer the > case (or not unless you take some other actions as you did in the VAX port). > > paul > Now I'm really confused. I defined the missing macro, and now I no longer get an ICE -- but I don't see any output from that macro either. No jump tables with -fpic, instead I get what looks like the no table jump available case (decision tree). Why would that be? paul
Re: [google/gcc-4_6_3-mobile] Port r187569 from google/gcc-4_6 branch (issue 6210060)
On 2012/05/16 17:27:59, asharif1 wrote: On 2012/05/16 01:02:26, shenhan wrote: > Hi Amhad and Jing, could you take a look at this CL at > http://codereview.appspot.com/6210060/ > > Thanks, > -Han Shouldn't this have a corresponding ChangeLog entry and a ChangeLog.google-gcc-4_6 entry? Hi Ahmad, thanks! Done recreating the patch from "svn merge -c". Regards, -Han http://codereview.appspot.com/6210060/
Re: [PATCH] PR rtl-optimization/53352
I meant to CC the RTL optimization maintainers before ... On 05/15/2012 02:39 PM, Meador Inge wrote: > Hi All, > > As reported in PR rtl-optimization/53352 CSE currently trips up on a > paradoxical subreg case. When compiling for ARM GNU/Linux with -O3 > the expanded RTL of interest looks like: > > (insn 12 11 13 3 (set (reg:SI 140) > (lshiftrt:SI (reg/v:SI 135 [ tmp1 ]) > (const_int 16 [0x10]))) > (nil)) > > (insn 13 12 14 3 (set (reg:QI 141) > (subreg:QI (reg:SI 140) 0)) > (nil)) > > (insn 14 13 15 3 (set (reg:SI 142) > (subreg:SI (reg:QI 141) 0)) > (nil)) > > (insn 15 14 16 3 (set (reg:SI 134 [ tmp1$2 ]) > (and:SI (reg:SI 142) > (const_int 255 [0xff]))) > (nil)) > > ... > > (insn 29 28 30 3 (set (reg:SI 0 r0) > (const_int 0 [0])) > (nil)) > > after "cse1" things look like: > > (insn 12 11 13 2 (set (reg:SI 140) > (const_int 65280 [0xff00])) > (nil)) > > (insn 13 12 14 2 (set (reg:QI 141) > (subreg:QI (reg:SI 140) 0)) > (expr_list:REG_EQUAL (const_int 0 [0]) > (nil))) > > ;; This is *not* equal to zero because the upper > ;; two bytes are undefined. > (insn 14 13 15 2 (set (reg:SI 142) > (subreg:SI (reg:QI 141) 0)) > (expr_list:REG_EQUAL (const_int 0 [0]) > (nil))) > > (insn 15 14 16 2 (set (reg:SI 134 [ tmp1$2 ]) > (reg:SI 142)) > (expr_list:REG_EQUAL (const_int 0 [0]) > (nil))) > > ... > > (insn 29 28 30 2 (set (reg:SI 0 r0) > (reg:SI 142)) > (expr_list:REG_EQUAL (const_int 0 [0]) > (nil))) > > I believe the REG_EQUAL note on the set involving a paradoxical subreg > is incorrect. It eventually causes 0xFF00 to be passed to the function > 'foo'. > > The attached patch fixes the issue by skipping the paradoxical subreg > in 'equiv_constant'. Compiler bootstrapped for i686-pc-linux-gnu and > full GCC test runs for i686-pc-linux-gnu and arm-none-linux-gnueabi (no > regressions). OK? (If this is OK, then can someone commit for me. I don't > have write access). > > gcc/ > > 2012-05-15 Meador Inge > > PR rtl-optimization/53352 > > * cse.c (equiv_constant): Ignore paradoxical subregs. > > gcc/testsuite/ > > 2012-05-15 Meador Inge > > PR rtl-optimization/53352 > > * gcc.dg/pr53352.c: New test. > > > > pr53352.patch > > > Index: gcc/testsuite/gcc.dg/pr53352.c > === > --- gcc/testsuite/gcc.dg/pr53352.c(revision 0) > +++ gcc/testsuite/gcc.dg/pr53352.c(revision 0) > @@ -0,0 +1,41 @@ > +/* { dg-do run } */ > +/* { dg-options "-O1" } */ > + > +#include > + > +typedef union > +{ > + struct > + { > +unsigned char a; > +unsigned char b; > +unsigned char c; > +unsigned char d; > + } parts; > + unsigned long whole; > +} T; > + > +T *g_t; > + > +void bar (unsigned long x) > +{ > + if (x != 0) > +abort (); > +} > + > +int main () > +{ > + T one; > + T two; > + T tmp1, tmp2; > + > + one.whole = 0xFFE0E0E0; > + two.whole = 0xFF00; > + tmp1.parts = two.parts; > + tmp2.parts = one.parts; > + tmp2.parts.c = tmp1.parts.c; > + one.parts = tmp2.parts; > + > + g_t = &one; > + bar (0); > +} > Index: gcc/cse.c > === > --- gcc/cse.c (revision 187470) > +++ gcc/cse.c (working copy) > @@ -3786,8 +3786,11 @@ equiv_constant (rtx x) > } > } > > - /* Otherwise see if we already have a constant for the inner REG. */ > + /* Otherwise see if we already have a constant for the inner REG. > + Don't bother with paradoxical subregs because we have no way > + of knowing what the upper bytes are. */ >if (REG_P (SUBREG_REG (x)) > + && (GET_MODE_SIZE (mode) <= GET_MODE_SIZE (imode)) > && (new_rtx = equiv_constant (SUBREG_REG (x))) != 0) > return simplify_subreg (mode, new_rtx, imode, SUBREG_BYTE (x)); > -- Meador Inge CodeSourcery / Mentor Embedded http://www.mentor.com/embedded-software
Re: [google/gcc-4_6_3-mobile] Port r187569 from google/gcc-4_6 branch (issue 6210060)
On 2012/05/16 22:11:08, shenhan wrote: On 2012/05/16 17:27:59, asharif1 wrote: > On 2012/05/16 01:02:26, shenhan wrote: > > Hi Amhad and Jing, could you take a look at this CL at > > http://codereview.appspot.com/6210060/ > > > > Thanks, > > -Han > > Shouldn't this have a corresponding ChangeLog entry and a > ChangeLog.google-gcc-4_6 entry? Hi Ahmad, thanks! Done recreating the patch from "svn merge -c". Regards, -Han Thanks a lot, shenhan. lgtm. http://codereview.appspot.com/6210060/
Re: [PATCH] PR rtl-optimization/53352
On Thu, May 17, 2012, Meador Inge wrote: >> ;; This is *not* equal to zero because the upper >> ;; two bytes are undefined. >> (insn 14 13 15 2 (set (reg:SI 142) >> (subreg:SI (reg:QI 141) 0)) >> (expr_list:REG_EQUAL (const_int 0 [0]) >> (nil))) Hmm, this is what doc/rtl.texi has to say about paradoxical subregs as rvalues: The high-order bits of rvalues are in the following circumstances: * subreg regs The upper bits are defined when SUBREG_PROMOTED_VAR_P is true. SUBREG_PROMOTED_UNSIGNED_P describes what the upper bits hold. The way I read this, suggests your patch allow simplify_subreg on paradoxical subregs if SUBREG_PROMOTED_VAR_P is true. But I'm not exactly an RTL expert, and I'm not 100% sure if this part of doc applies here. ;-) Ciao! Steven