Re: [PATCH RFA] build: drop target libs from LD_LIBRARY_PATH [PR105688]

[Paolo Bonzini]

On 2/8/24 18:16, Jason Merrill wrote:




Hmm.  In stage 1, when we build with the system gcc, I'd think we want 
the just-built gnat1 to find the system libgcc.


In stage 2, when we build with the stage 1 gcc, we want the just-built 
gnat1 to find the stage 1 libgcc.


In neither case do we want it to find the libgcc from the current stage.

So it seems to me that what we want is for stage2+ LD_LIBRARY_PATH to 
include the TARGET_LIB_PATH from the previous stage.  Something like 
the below, on top of the earlier patch.


Does this make sense?  Does it work on Darwin?


Oops, that was broken, please consider this one instead:


Yes, this one makes sense (and the current code would not work since it 
lacks the prev- prefix on TARGET_LIB_PATH).


Paolo

Re: [PATCH v3 01/18] clean up SHLIB so subshells are not needed

[Paolo Bonzini]

Il 20/08/2013 15:58, Tom Tromey ha scritto:
> This changes the handling of SHLIB so that it is inlined into
> DRIVER_DEFINES.  This is ok because SHLIB is defined in a Makefile
> fragment that is included by the generated Makefile.
> 
> The rationale for this is that it simplifies some .o targets, so that
> we can share more code.
> 
>   * Makefile.in (DRIVER_DEFINES): Use $(and), not shell code,
>   to add -DENABLE_SHARED_LIBGCC.
>   (gcc.o): Don't use subshell.
> 
>   * Make-lang.in (c/gccspec.o): Don't use subshell.
> 
>   * Make-lang.in (g++spec.o): Don't use subshell.
> 
>   * Make-lang.in (gfortranspec.o): Don't use subshell.
> 
>   * Make-lang.in (gospec.o): Don't use subshell.
> 
>   * Make-lang.in (jvspec.o): Don't use subshell.
> ---
>  gcc/Makefile.in  | 5 ++---
>  gcc/c/Make-lang.in   | 3 +--
>  gcc/cp/Make-lang.in  | 3 +--
>  gcc/fortran/Make-lang.in | 3 +--
>  gcc/go/Make-lang.in  | 3 +--
>  gcc/java/Make-lang.in| 3 +--
>  6 files changed, 7 insertions(+), 13 deletions(-)
> 
> diff --git a/gcc/Makefile.in b/gcc/Makefile.in
> index 704ca10..8d2fd23 100644
> --- a/gcc/Makefile.in
> +++ b/gcc/Makefile.in
> @@ -2059,16 +2059,15 @@ DRIVER_DEFINES = \
>-DTOOLDIR_BASE_PREFIX=\"$(libsubdir_to_prefix)$(prefix_to_exec_prefix)\" \
>@TARGET_SYSTEM_ROOT_DEFINE@ \
>$(VALGRIND_DRIVER_DEFINES) \
> -  `test "X$${SHLIB}" = "X" || test "@enable_shared@" != "yes" || echo 
> "-DENABLE_SHARED_LIBGCC"` \
> +  $(and $(SHLIB),$(filter yes,@enable_shared@),-DENABLE_SHARED_LIBGCC) \
>-DCONFIGURE_SPECS="\"@CONFIGURE_SPECS@\""
>  
>  gcc.o: gcc.c $(CONFIG_H) $(SYSTEM_H) coretypes.h $(TM_H) intl.h multilib.h \
>  Makefile $(lang_specs_files) specs.h prefix.h $(GCC_H) $(FLAGS_H) \
>  configargs.h $(OBSTACK_H) $(OPTS_H) $(DIAGNOSTIC_H) $(VEC_H) $(PARAMS_H)
> - (SHLIB='$(SHLIB)'; \
>   $(COMPILER) $(ALL_COMPILERFLAGS) $(ALL_CPPFLAGS) \
>$(DRIVER_DEFINES) \
> -  -c $(srcdir)/gcc.c $(OUTPUT_OPTION))
> +  -c $(srcdir)/gcc.c $(OUTPUT_OPTION)
>  
>  specs.h : s-specs ; @true
>  s-specs : Makefile
> diff --git a/gcc/c/Make-lang.in b/gcc/c/Make-lang.in
> index 1161742..86deb1d 100644
> --- a/gcc/c/Make-lang.in
> +++ b/gcc/c/Make-lang.in
> @@ -46,10 +46,9 @@ c: cc1$(exeext)
>  # is to cc1 as e.g. g++ is to cc1plus, or gfortran is to f951).
>  c/gccspec.o: c/gccspec.c $(CONFIG_H) $(SYSTEM_H) coretypes.h $(TM_H) 
> $(GCC_H) \
>  $(OPTS_H)
> - (SHLIB='$(SHLIB)'; \
>   $(COMPILER) $(ALL_COMPILERFLAGS) $(ALL_CPPFLAGS) \
>$(DRIVER_DEFINES) \
> -  -c $(srcdir)/c/gccspec.c $(OUTPUT_OPTION))
> +  -c $(srcdir)/c/gccspec.c $(OUTPUT_OPTION)
>  
>  # The C compiler itself.
>  
> diff --git a/gcc/cp/Make-lang.in b/gcc/cp/Make-lang.in
> index 2c1774f..263daa5 100644
> --- a/gcc/cp/Make-lang.in
> +++ b/gcc/cp/Make-lang.in
> @@ -53,9 +53,8 @@ c++: cc1plus$(exeext)
>  
>  g++spec.o: $(srcdir)/cp/g++spec.c $(SYSTEM_H) coretypes.h $(TM_H) $(GCC_H) \
>  $(CONFIG_H) $(OPTS_H)
> - (SHLIB='$(SHLIB)'; \
>   $(COMPILER) -c $(ALL_COMPILERFLAGS) $(ALL_CPPFLAGS) $(DRIVER_DEFINES) \
> - $(INCLUDES) $(srcdir)/cp/g++spec.c)
> + $(INCLUDES) $(srcdir)/cp/g++spec.c
>  
>  # Create the compiler driver for g++.
>  GXX_OBJS = $(GCC_OBJS) g++spec.o
> diff --git a/gcc/fortran/Make-lang.in b/gcc/fortran/Make-lang.in
> index ee70423..035a1d5 100644
> --- a/gcc/fortran/Make-lang.in
> +++ b/gcc/fortran/Make-lang.in
> @@ -78,9 +78,8 @@ fortran: f951$(exeext)
>  
>  gfortranspec.o: $(srcdir)/fortran/gfortranspec.c $(SYSTEM_H) $(TM_H) 
> $(GCC_H) \
>   $(CONFIG_H) coretypes.h intl.h $(OPTS_H)
> - (SHLIB='$(SHLIB)'; \
>   $(COMPILER) -c $(ALL_COMPILERFLAGS) $(ALL_CPPFLAGS) $(DRIVER_DEFINES) \
> - $(INCLUDES) $(srcdir)/fortran/gfortranspec.c)
> + $(INCLUDES) $(srcdir)/fortran/gfortranspec.c
>  
>  # Create the compiler driver gfortran.
>  GFORTRAN_D_OBJS = $(GCC_OBJS) gfortranspec.o
> diff --git a/gcc/go/Make-lang.in b/gcc/go/Make-lang.in
> index 3cb18d6..015ed7a 100644
> --- a/gcc/go/Make-lang.in
> +++ b/gcc/go/Make-lang.in
> @@ -32,9 +32,8 @@ go: go1$(exeext)
>  
>  gospec.o: $(srcdir)/go/gospec.c $(SYSTEM_H) coretypes.h $(TM_H) $(GCC_H) \
>  $(CONFIG_H) opts.h
> - (SHLIB='$(SHLIB)'; \
>   $(COMPILER) -c $(ALL_COMPILERFLAGS) $(ALL_CPPFLAGS) $(DRIVER_DEFINES) \
> - $(INCLUDES) $(srcdir)/go/gospec.c)
> + $(INCLUDES) $(srcdir)/go/gospec.c
>  
>  GCCGO_OBJS = $(GCC_OBJS) gospec.o
>  gccgo$(exeext): $(GCCGO_OBJS) $(EXTRA_GCC_OBJS) libcommon-target.a $(LIBDEPS)
> diff --git a/gcc/java/Make-lang.in b/gcc/java/Make-lang.in
> index 8a6210f..078f6d4 100644
> --- a/gcc/java/Make-lang.in
> +++ b/gcc/java/Make-lang.in
> @@ -57,9 +57,8 @@ JAVA_TARGET_INDEPENDENT_BIN_TOOLS = jcf-dump
>  
>  jvspec.o: $(srcdir)/java/jvspec.c $(SYSTEM_H) coretypes.h $(TM_H) \
>$(GCC_H) $(CONFIG_H) java/jcf.h java/javaop.h $(OPTS_H)
> - (SHLIB='$(SHLIB)'; \
>   $(COMPILER) -c $(ALL_COMPILE

Re: [patch 1/2] tree-flow.h restructuring

[Paolo Bonzini]

Il 16/09/2013 16:39, Tom Tromey ha scritto:
> Richard> Where's that automatic dependency patch blocked ...
> 
> No build machinery maintainer has reviewed it.

/me gets out of coma...

Someone said "automatic dependency" and "build machinery maintainer" in
the same sentence???

Paolo

Re: [PATCH v3 03/18] move generated_files order-only dependency later

[Paolo Bonzini]

Il 20/08/2013 15:59, Tom Tromey ha scritto:
> There is an order-only dependency in gcc/Makefile.in that tries to
> build the generated files before compiling regular objects.  However,
> this appears too early, and so at the time it is seen by make,
> GCOV_OBJS and GCOV_DUMP_OBJS have not yet been set.
> 
> This patch fixes the problem and prevents any future problems of this
> nature by moving the order-only dependency to the end of Makefile.in.
> 
> This also adds lto-wrapper.o to ALL_HOST_BACKEND_OBJS.
> 
>   * Makefile.in (ALL_HOST_BACKEND_OBJS): Add lto-wrapper.o.
>   ($(ALL_HOST_OBJS)): Move order-only dependency to end
>   of file.
> ---
>  gcc/Makefile.in | 13 +++--
>  1 file changed, 7 insertions(+), 6 deletions(-)
> 
> diff --git a/gcc/Makefile.in b/gcc/Makefile.in
> index 415f085..172ab4d 100644
> --- a/gcc/Makefile.in
> +++ b/gcc/Makefile.in
> @@ -1496,7 +1496,8 @@ ALL_HOST_FRONTEND_OBJS = $(foreach 
> v,$(CONFIG_LANGUAGES),$($(v)_OBJS))
>  
>  ALL_HOST_BACKEND_OBJS = $(GCC_OBJS) $(OBJS) $(OBJS-libcommon) \
>$(OBJS-libcommon-target) @TREEBROWSER@ main.o c-family/cppspec.o \
> -  $(COLLECT2_OBJS) $(EXTRA_GCC_OBJS) $(GCOV_OBJS) $(GCOV_DUMP_OBJS)
> +  $(COLLECT2_OBJS) $(EXTRA_GCC_OBJS) $(GCOV_OBJS) $(GCOV_DUMP_OBJS) \
> +  lto-wrapper.o
>  
>  # This lists all host object files, whether they are included in this
>  # compilation or not.
> @@ -3884,11 +3885,6 @@ generated_files = config.h tm.h $(TM_P_H) $(TM_H) 
> multilib.h \
> options.h target-hooks-def.h insn-opinit.h \
> common/common-target-hooks-def.h pass-instances.def
>  
> -# In order for parallel make to really start compiling the expensive
> -# objects from $(OBJS) as early as possible, build all their
> -# prerequisites strictly before all objects.
> -$(ALL_HOST_OBJS) : | $(generated_files)
> -
>  #
>  # How to compile object files to run on the build machine.
>  
> @@ -5381,3 +5377,8 @@ po/gcc.pot: force
>   $(MAKE) srcextra
>   AWK=$(AWK) $(SHELL) $(srcdir)/po/exgettext \
>   $(XGETTEXT) gcc $(srcdir)
> +
> +# In order for parallel make to really start compiling the expensive
> +# objects from $(OBJS) as early as possible, build all their
> +# prerequisites strictly before all objects.
> +$(ALL_HOST_OBJS) : | $(generated_files)
> 

Ok.

Paolo

Re: [PATCH v3 04/18] add configury

[Paolo Bonzini]

Il 20/08/2013 15:59, Tom Tromey ha scritto:
> This adds the configury needed for automatic dependency tracking.  It
> also adds some bits to the Makefile so we can begin converting
> (removing) explicit dependencies.
> 
>   * Makefile.in (CCDEPMODE, DEPDIR, depcomp, COMPILE.base)
>   (COMPILE, POSTCOMPILE): New variables.
>   (.cc.o .c.o): Use COMPILE, POSTCOMPILE.
>   (DEPFILES): New variable.
>   Include ".Po" files.
>   * configure.ac: Add checks for dependency checking.
>   * configure, aclocal.m4: Regenerate.
> ---
>  gcc/Makefile.in  |  28 ++-
>  gcc/aclocal.m4   |   2 +
>  gcc/configure| 148 
> ++-
>  gcc/configure.ac |  16 ++
>  4 files changed, 191 insertions(+), 3 deletions(-)
> 
> diff --git a/gcc/Makefile.in b/gcc/Makefile.in
> index 172ab4d..e1de7ba 100644
> --- a/gcc/Makefile.in
> +++ b/gcc/Makefile.in
> @@ -308,6 +308,11 @@ write_entries_to_file = $(shell rm -f $(2) || :) $(shell 
> touch $(2)) \
>  # UNSORTED
>  # 
>  
> +# Dependency tracking stuff.
> +CCDEPMODE = @CCDEPMODE@
> +DEPDIR = @DEPDIR@
> +depcomp = $(SHELL) $(srcdir)/../depcomp
> +
>  # Some compilers can't handle cc -c blah.c -o foo/blah.o.
>  # In stage2 and beyond, we force this to "-o $@" since we know we're using 
> gcc.
>  OUTPUT_OPTION = @OUTPUT_OPTION@
> @@ -1068,8 +1073,19 @@ INCLUDES = -I. -I$(@D) -I$(srcdir) -I$(srcdir)/$(@D) \
>  $(CPPINC) $(GMPINC) $(DECNUMINC) $(BACKTRACEINC) \
>  $(CLOOGINC) $(ISLINC)
>  
> +COMPILE.base = $(COMPILER) -c $(ALL_COMPILERFLAGS) $(ALL_CPPFLAGS) -o $@
> +ifeq ($(CCDEPMODE),depmode=gcc3)
> +COMPILE = $(COMPILE.base) -MT $@ -MMD -MP -MF $(*D)/$(DEPDIR)/$(*F).TPo
> +POSTCOMPILE = @mv $(*D)/$(DEPDIR)/$(*F).TPo $(*D)/$(DEPDIR)/$(*F).Po
> +else
> +COMPILE = source='$<' object='$@' libtool=no \
> +DEPDIR=$(DEPDIR) $(CCDEPMODE) $(depcomp) $(COMPILE.base)
> +POSTCOMPILE =
> +endif
> +
>  .cc.o .c.o:
> - $(COMPILER) -c $(ALL_COMPILERFLAGS) $(ALL_CPPFLAGS) $< $(OUTPUT_OPTION)
> + $(COMPILE) $<
> + $(POSTCOMPILE)
>  
>  #
>  # Support for additional languages (other than C).
> @@ -5378,7 +5394,17 @@ po/gcc.pot: force
>   AWK=$(AWK) $(SHELL) $(srcdir)/po/exgettext \
>   $(XGETTEXT) gcc $(srcdir)
>  
> +#
> +
> +# Dependency information.
> +
>  # In order for parallel make to really start compiling the expensive
>  # objects from $(OBJS) as early as possible, build all their
>  # prerequisites strictly before all objects.
>  $(ALL_HOST_OBJS) : | $(generated_files)
> +
> +# Include the auto-generated dependencies for all host objects.
> +DEPFILES = \
> +  $(foreach obj,$(ALL_HOST_OBJS),\
> +$(dir $(obj))$(DEPDIR)/$(patsubst %.o,%.Po,$(notdir $(obj
> +-include $(DEPFILES)
> diff --git a/gcc/aclocal.m4 b/gcc/aclocal.m4
> index 1f3ce9d..e7cfd7d 100644
> --- a/gcc/aclocal.m4
> +++ b/gcc/aclocal.m4
> @@ -101,10 +101,12 @@ m4_define([AC_PROG_CC],
>  
>  m4_include([../config/acx.m4])
>  m4_include([../config/codeset.m4])
> +m4_include([../config/depstand.m4])
>  m4_include([../config/dfp.m4])
>  m4_include([../config/gettext-sister.m4])
>  m4_include([../config/iconv.m4])
>  m4_include([../config/lcmessage.m4])
> +m4_include([../config/lead-dot.m4])
>  m4_include([../config/lib-ld.m4])
>  m4_include([../config/lib-link.m4])
>  m4_include([../config/lib-prefix.m4])
> diff --git a/gcc/configure b/gcc/configure
> index 1e9be8e..c53a930 100755
> --- a/gcc/configure
> +++ b/gcc/configure
> @@ -735,6 +735,9 @@ LDEXP_LIB
>  EXTRA_GCC_LIBS
>  GNAT_LIBEXC
>  COLLECT2_LIBS
> +CCDEPMODE
> +DEPDIR
> +am__leading_dot
>  CXXCPP
>  AR
>  NM
> @@ -8873,6 +8876,136 @@ ac_compiler_gnu=$ac_cv_c_compiler_gnu
>  
>  
>  # 
> +# Dependency checking.
> +# 
> +
> +ac_ext=cpp
> +ac_cpp='$CXXCPP $CPPFLAGS'
> +ac_compile='$CXX -c $CXXFLAGS $CPPFLAGS conftest.$ac_ext >&5'
> +ac_link='$CXX -o conftest$ac_exeext $CXXFLAGS $CPPFLAGS $LDFLAGS 
> conftest.$ac_ext $LIBS >&5'
> +ac_compiler_gnu=$ac_cv_cxx_compiler_gnu
> +
> +rm -rf .tst 2>/dev/null
> +mkdir .tst 2>/dev/null
> +if test -d .tst; then
> +  am__leading_dot=.
> +else
> +  am__leading_dot=_
> +fi
> +rmdir .tst 2>/dev/null
> +
> +DEPDIR="${am__leading_dot}deps"
> +
> +ac_config_commands="$ac_config_commands depdir"
> +
> +
> +ac_config_commands="$ac_config_commands gccdepdir"
> +
> +
> +depcc="$CC"   am_compiler_list=
> +
> +am_depcomp=$ac_aux_dir/depcomp
> +{ $as_echo "$as_me:${as_lineno-$LINENO}: checking dependency style of 
> $depcc" >&5
> +$as_echo_n "checking dependency style of $depcc... " >&6; }
> +if test "${am_cv_CC_dependencies_compiler_type+set}" = set; then :
> +  $as_echo_n "(cached) " >&6
> +else
> +  if test -f "$am_depcomp"; then
> +  # We make a subdir and do the tests there.  Otherwise we can end up
> +  # making bogus files that we don't know about and never remove.  For
> +  # instance it was reported that on HP-UX the gcc test will end up
> +  # making a dummy file named `D' -- becaus

Re: [PATCH v3 10/18] Fix up c-family targets

[Paolo Bonzini]

Il 20/08/2013 15:59, Tom Tromey ha scritto:
> This removes manual dependencies for the c-family .o files.
> 
>   * Makefile.in (c-family/cppspec.o, c-family/c-common.o)
>   (c-family/c-cppbuiltin.o, c-family/c-dump.o, c-family/c-format.o)
>   (c-family/c-gimplify.o, c-family/c-lex.o, c-family/c-omp.o)
>   (c-family/c-opts.o, c-family/c-pch.o, c-family/c-ppoutput.o)
>   (c-family/c-pragma.o, c-family/c-pretty-print.o)
>   (c-family/c-semantics.o, c-family/c-ada-spec.o)
>   (c-family/array-notation-common.o, c-family/stub-objc.o): Remove.
> ---
>  gcc/Makefile.in | 76 
> -
>  1 file changed, 76 deletions(-)
> 
> diff --git a/gcc/Makefile.in b/gcc/Makefile.in
> index 79eb9f1..6edaa00 100644
> --- a/gcc/Makefile.in
> +++ b/gcc/Makefile.in
> @@ -1963,87 +1963,11 @@ lto-wrapper.o: lto-wrapper.c $(CONFIG_H) $(SYSTEM_H) 
> coretypes.h intl.h \
>   $(OBSTACK_H) $(DIAGNOSTIC_H) $(OPTS_H) $(OPTIONS_H)
>  
>  # Files used by all variants of C or by the stand-alone pre-processor.
> -c-family/cppspec.o: c-family/cppspec.c $(CONFIG_H) $(SYSTEM_H) coretypes.h \
> -$(TM_H) $(GCC_H) $(OPTS_H)
> -
> -c-family/c-common.o : c-family/c-common.c $(CONFIG_H) $(SYSTEM_H) 
> coretypes.h \
> - $(TM_H) $(TREE_H) \
> - $(OBSTACK_H) $(C_COMMON_H) $(FLAGS_H) toplev.h output.h $(C_PRAGMA_H) \
> - $(GGC_H) builtin-types.def builtin-attrs.def \
> - $(DIAGNOSTIC_H) langhooks.h c-family/c-objc.h \
> - $(TARGET_H) tree-iterator.h langhooks.h tree-mudflap.h \
> - intl.h $(OPTS_H) $(CPPLIB_H) $(TREE_INLINE_H) $(HASHTAB_H) \
> - $(BUILTINS_DEF) $(CGRAPH_H) $(TARGET_DEF_H) \
> - gt-c-family-c-common.h $(COMMON_TARGET_H)
> -
> -c-family/c-cppbuiltin.o : c-family/c-cppbuiltin.c $(CONFIG_H) $(SYSTEM_H) \
> - coretypes.h $(TM_H) $(TREE_H) version.h $(C_COMMON_H) $(C_PRAGMA_H) \
> - $(FLAGS_H) output.h $(TREE_H) $(TARGET_H) $(COMMON_TARGET_H) \
> - $(TM_P_H) debug.h $(CPP_ID_DATA_H) cppbuiltin.h
> -
> -c-family/c-dump.o : c-family/c-dump.c $(CONFIG_H) $(SYSTEM_H) coretypes.h \
> - $(TM_H) $(TREE_H) $(TREE_DUMP_H)
> -
> -c-family/c-format.o : c-family/c-format.c c-family/c-format.h \
> - $(CONFIG_H) $(SYSTEM_H) coretypes.h $(TM_H) $(TREE_H) langhooks.h \
> - $(C_COMMON_H) $(FLAGS_H) intl.h $(C_TARGET_H) \
> - $(DIAGNOSTIC_CORE_H) alloc-pool.h c-family/c-objc.h
> -
> -c-family/c-gimplify.o : c-family/c-gimplify.c $(CONFIG_H) $(SYSTEM_H) 
> $(TREE_H) \
> - $(C_COMMON_H) $(DIAGNOSTIC_CORE_H) $(GIMPLE_H) \
> - $(FLAGS_H) langhooks.h $(LANGHOOKS_DEF_H) \
> - $(TM_H) coretypes.h $(C_PRETTY_PRINT_H) $(CGRAPH_H) \
> - $(DUMPFILE_H) $(TREE_INLINE_H)
> -
> -c-family/c-lex.o : c-family/c-lex.c $(CONFIG_H) $(SYSTEM_H) coretypes.h \
> - $(TM_H) $(TREE_H) $(FIXED_VALUE_H) debug.h $(C_COMMON_H) 
> $(SPLAY_TREE_H) \
> - $(C_PRAGMA_H) $(INPUT_H) intl.h $(FLAGS_H) \
> - $(CPPLIB_H) $(TARGET_H) $(TIMEVAR_H)
> -
> -c-family/c-omp.o : c-family/c-omp.c $(CONFIG_H) $(SYSTEM_H) coretypes.h \
> - $(TREE_H) $(C_COMMON_H) $(GIMPLE_H) langhooks.h
>  
>  CFLAGS-c-family/c-opts.o += @TARGET_SYSTEM_ROOT_DEFINE@
> -c-family/c-opts.o : c-family/c-opts.c $(CONFIG_H) $(SYSTEM_H) coretypes.h \
> -$(TREE_H) $(C_PRAGMA_H) $(FLAGS_H) toplev.h langhooks.h \
> -$(DIAGNOSTIC_H) intl.h debug.h $(C_COMMON_H) $(C_TARGET_H) \
> -$(OPTS_H) $(OPTIONS_H) $(MKDEPS_H) incpath.h cppdefault.h
>  
>  CFLAGS-c-family/c-pch.o += -DHOST_MACHINE=\"$(host)\" \
>   -DTARGET_MACHINE=\"$(target)\"
> -c-family/c-pch.o : c-family/c-pch.c $(CONFIG_H) $(SYSTEM_H) coretypes.h \
> - $(CPPLIB_H) $(TREE_H) $(C_COMMON_H) output.h $(C_PRAGMA_H) \
> - $(GGC_H) debug.h langhooks.h $(FLAGS_H) hosthooks.h version.h \
> - $(TARGET_H) $(OPTS_H) $(TIMEVAR_H)
> -
> -c-family/c-ppoutput.o : c-family/c-ppoutput.c $(CONFIG_H) $(SYSTEM_H) \
> - coretypes.h $(C_COMMON_H) $(TREE_H) $(CPPLIB_H) $(CPP_INTERNAL_H) \
> - $(C_PRAGMA_H)
> -
> -c-family/c-pragma.o: c-family/c-pragma.c $(CONFIG_H) $(SYSTEM_H) coretypes.h 
> \
> - $(TM_H) $(TREE_H) $(FUNCTION_H) $(C_PRAGMA_H) output.h \
> - $(TM_P_H) $(C_COMMON_H) $(TARGET_H) $(CPPLIB_H) $(FLAGS_H) \
> - $(DIAGNOSTIC_H) $(OPTS_H) $(PLUGINS_H) \
> - gt-c-family-c-pragma.h
> -
> -c-family/c-pretty-print.o : c-family/c-pretty-print.c $(C_PRETTY_PRINT_H) \
> - $(CONFIG_H) $(SYSTEM_H) coretypes.h $(TM_H) \
> - $(DIAGNOSTIC_H) tree-iterator.h intl.h $(TREE_PRETTY_PRINT_H)
> -
> -c-family/c-semantics.o : c-family/c-semantics.c $(CONFIG_H) $(SYSTEM_H) \
> - coretypes.h $(TM_H) $(TREE_H) $(FLAGS_H) \
> - $(C_COMMON_H) $(FUNCTION_H) langhooks.h $(SPLAY_TREE_H) $(TIMEVAR_H) \
> - tree-iterator.h
> -
> -c-family/c-ada-spec.o : c-family/c-ada-spec.c c-family/c-ada-spec.h \
> - $(CONFIG_H) $(SYSTEM_H) $(TREE_H) $(CPP_ID_DATA_H) $(TM_H) \
> - coretypes.h tree-iterator.h $(DUMPFILE_H)
> -
> -c-family/array-notation-common.o : c-family/array

Re: [PATCH v3 13/18] convert LTO to automatic dependencies

[Paolo Bonzini]

Il 20/08/2013 15:59, Tom Tromey ha scritto:
> This converts LTO.
> 
> This also fixes a latent buglet in lto/Make-lang.in.  lto_OBJS should
> hold all the objects for a language, but LTO never defined this.
> 
>   * Make-lang.in (LTO_H, LINKER_PLUGIN_API_H, LTO_TREE_H)
>   (lto/lto-lang.o, lto/lto.o, lto/lto-partition.o)
>   (lto/lto-object.o): Remove.
> ---
>  gcc/lto/Make-lang.in | 27 +--
>  1 file changed, 1 insertion(+), 26 deletions(-)
> 
> diff --git a/gcc/lto/Make-lang.in b/gcc/lto/Make-lang.in
> index 1acd176..ddef3bf 100644
> --- a/gcc/lto/Make-lang.in
> +++ b/gcc/lto/Make-lang.in
> @@ -23,10 +23,7 @@
>  LTO_EXE = lto1$(exeext)
>  # The LTO-specific object files inclued in $(LTO_EXE).
>  LTO_OBJS = lto/lto-lang.o lto/lto.o lto/lto-object.o attribs.o 
> lto/lto-partition.o
> -LTO_H = lto/lto.h $(HASHTAB_H)
> -LINKER_PLUGIN_API_H = $(srcdir)/../include/plugin-api.h
> -LTO_TREE_H = lto/lto-tree.h $(LINKER_PLUGIN_API_H)
> -
> +lto_OBJS = $(LTO_OBJS)
>  
>  # Rules
>  
> @@ -74,27 +71,5 @@ $(LTO_EXE): $(LTO_OBJS) $(BACKEND) $(LIBDEPS)
>   +$(LLINKER) $(ALL_LINKERFLAGS) $(LDFLAGS) -o $@ \
>   $(LTO_OBJS) $(BACKEND) $(BACKENDLIBS) $(LIBS)
>  
> -# Dependencies
> -lto/lto-lang.o: lto/lto-lang.c $(CONFIG_H) coretypes.h debug.h \
> - flags.h $(GGC_H) langhooks.h $(LANGHOOKS_DEF_H) $(SYSTEM_H) \
> - $(TARGET_H) $(LTO_H) $(GIMPLE_H) gtype-lto.h gt-lto-lto-lang.h \
> - $(EXPR_H) $(LTO_STREAMER_H)
> -lto/lto.o: lto/lto.c $(CONFIG_H) $(SYSTEM_H) coretypes.h $(OPTS_H) \
> - toplev.h $(TREE_H) $(TREE_FLOW_H) $(DIAGNOSTIC_CORE_H) $(TM_H) \
> - $(CGRAPH_H) $(GGC_H) tree-ssa-operands.h $(TREE_PASS_H) \
> - langhooks.h $(VEC_H) $(BITMAP_H) pointer-set.h $(IPA_PROP_H) \
> - $(COMMON_H) debug.h $(GIMPLE_H) $(LTO_H) $(LTO_TREE_H) \
> - $(LTO_TAGS_H) $(LTO_STREAMER_H) $(SPLAY_TREE_H) gt-lto-lto.h \
> - $(TREE_STREAMER_H) $(DATA_STREAMER_H) lto/lto-partition.h \
> - $(CONTEXT_H) $(PIPELINE_H)
> -lto/lto-partition.o: lto/lto-partition.c $(CONFIG_H) $(SYSTEM_H) coretypes.h 
> \
> - toplev.h $(TREE_H) $(TM_H) \
> - $(CGRAPH_H) $(TIMEVAR_H) \
> - $(LTO_STREAMER_H) $(SPLAY_TREE_H) gt-lto-lto.h $(PARAMS_H) \
> - ipa-inline.h $(IPA_UTILS_H) lto/lto-partition.h
> -lto/lto-object.o: lto/lto-object.c $(CONFIG_H) $(SYSTEM_H) coretypes.h \
> - $(DIAGNOSTIC_CORE_H) $(LTO_H) $(TM_H) $(LTO_STREAMER_H) \
> - ../include/simple-object.h
> -
>  # LTO testing is done as part of C/C++/Fortran etc. testing.
>  check-lto:
> 

Ok.

Paolo

Re: [PATCH v3 14/18] remove explicit dependencies

[Paolo Bonzini]

Il 20/08/2013 15:59, Tom Tromey ha scritto:
> This removes most of the explicit dependencies for host objects.
> It also fixes a few rules to use $(COMPILE) in the process.
> 
> build objects are not affected, and are one reason that the various _H
> macros are not yet removed.
> 
>   * Makefile.in (graph.o, sbitmap.o, sparseset.o, gcc-ar.o)
>   (gcc-ranlib.o, gcc-nm.o, collect2.o, collect2-aix.o, tlink.o)
>   (lto-wrapper.o, attribs.o, incpath.o)
>   (prefix.o, gcc.o, options.o, options-save.o, version.o)
>   (gtype-desc.o, trans-mem.o, ggc-common.o, ggc-page.o, ggc-none.o)
>   (stringpool.o, convert.o, double-int.o, lto-compress.o)
>   (data-streamer-in.o, data-streamer-out.o, data-streamer.o)
>   (gimple-streamer-in.o, gimple-streamer-out.o, tree-streamer.o)
>   (tree-streamer-in.o, tree-streamer-out.o, streamer-hooks.o)
>   (lto-cgraph.o, lto-streamer-in.o, lto-streamer-out.o)
>   (lto-section-in.o, lto-section-out.o, lto-symtab.o, lto-opts.o)
>   (lto-streamer.o, langhooks.o, test-dump.o, tree.o, tree-dump.o)
>   (tree-inline.o, print-tree.o, stor-layout.o, asan.o, tsan.o)
>   (tree-ssa-tail-merge.o, tree-ssa-structalias.o, tree-ssa-uninit.o)
>   (tree-ssa.o, tree-into-ssa.o, tree-ssa-ter.o, tree-ssa-coalesce.o)
>   (tree-outof-ssa.o, tree-ssa-dse.o, tree-ssa-forwprop.o)
>   (tree-ssa-phiprop.o, tree-ssa-ifcombine.o, tree-ssa-phiopt.o)
>   (tree-nrv.o, tree-ssa-copy.o, tree-ssa-propagate.o)
>   (tree-ssa-dom.o, tree-ssa-uncprop.o, tree-ssa-threadedge.o)
>   (tree-ssa-threadupdate.o, tree-ssanames.o, tree-phinodes.o)
>   (domwalk.o, tree-ssa-live.o, tree-ssa-copyrename.o)
>   (tree-ssa-pre.o, tree-ssa-sccvn.o)
>   (gimple-ssa-strength-reduction.o, tree-vrp.o, tree-cfg.o)
>   (tree-cfgcleanup.o, tree-tailcall.o, tree-ssa-sink.o)
>   (tree-nested.o, tree-if-conv.o, tree-iterator.o, tree-dfa.o)
>   (tree-ssa-operands.o, tree-eh.o, tree-ssa-loop.o)
>   (tree-ssa-loop-unswitch.o, tree-ssa-address.o)
>   (tree-ssa-loop-niter.o, tree-ssa-loop-ivcanon.o)
>   (tree-ssa-loop-ch.o, tree-ssa-loop-prefetch.o, tree-predcom.o)
>   (tree-ssa-loop-ivopts.o, tree-affine.o, tree-ssa-loop-manip.o)
>   (tree-ssa-loop-im.o, tree-ssa-math-opts.o, tree-ssa-alias.o)
>   (tree-ssa-reassoc.o, tree-optimize.o, gimplify.o)
>   (gimple-iterator.o, gimple-fold.o, gimple-low.o, omp-low.o)
>   (tree-browser.o, omega.o, tree-chrec.o, tree-scalar-evolution.o)
>   (tree-data-ref.o, sese.o, graphite.o, graphite-blocking.o)
>   (graphite-clast-to-gimple.o, graphite-dependences.o)
>   (graphite-interchange.o, graphite-poly.o)
>   (graphite-scop-detection.o, graphite-sese-to-poly.o)
>   (graphite-optimize-isl.o, tree-vect-loop.o)
>   (tree-vect-loop-manip.o, tree-vect-patterns.o, tree-vect-slp.o)
>   (tree-vect-stmts.o, tree-vect-data-refs.o, tree-vectorizer.o)
>   (tree-loop-distribution.o, tree-parloops.o, tree-stdarg.o)
>   (tree-object-size.o, internal-fn.o, gimple.o)
>   (gimple-pretty-print.o, tree-mudflap.o, tree-nomudflap.o)
>   (tree-pretty-print.o, tree-diagnostic.o, fold-const.o)
>   (diagnostic.o, diagnostic-color.o, opts.o, opts-global.o)
>   (opts-common.o, targhooks.o, common/common-targhooks.o, input.o)
>   (toplev.o, hwint.o, plugin.o, main.o, host-default.o)
>   (rtl-error.o, rtl.o, print-rtl.o, rtlanal.o, varasm.o, function.o)
>   (statistics.o, stmt.o, except.o, expr.o, dojump.o, builtins.o)
>   (calls.o, expmed.o, explow.o, optabs.o, dbxout.o, debug.o)
>   (sdbout.o, dwarf2out.o, dwarf2cfi.o, dwarf2asm.o, vmsdbgout.o)
>   (xcoffout.o, godump.o, emit-rtl.o, real.o, realmpfr.o, dfp.o)
>   (fixed-value.o, jump.o, simplify-rtx.o, symtab.o, cgraph.o)
>   (cgraphunit.o, cgraphclones.o, cgraphbuild.o, varpool.o, ipa.o)
>   (ipa-prop.o, ipa-ref.o, ipa-cp.o, ipa-split.o, ipa-inline.o)
>   (ipa-inline-analysis.o, ipa-inline-transform.o, ipa-utils.o)
>   (ipa-reference.o, ipa-pure-const.o, coverage.o, cselib.o, cse.o)
>   (dce.o, dumpfile.o, dse.o, fwprop.o, web.o, ree.o, cprop.o)
>   (gcse.o, store-motion.o, resource.o, lcm.o, mode-switching.o)
>   (tree-ssa-dce.o, tree-call-cdce.o, tree-ssa-ccp.o)
>   (tree-ssa-strlen.o, tree-sra.o, tree-switch-conversion.o)
>   (tree-complex.o, tree-emutls.o, tree-vect-generic.o, df-core.o)
>   (df-problems.o, df-scan.o, regstat.o, valtrack.o, var-tracking.o)
>   (profile.o, mcf.o, tree-profile.o, value-prof.o, loop-doloop.o)
>   (alloc-pool.o, auto-inc-dec.o, cfg.o, cfghooks.o, cfgexpand.o)
>   (cfgrtl.o, cfganal.o, cfgbuild.o, cfgcleanup.o, cfgloop.o)
>   (cfgloopanal.o, graphds.o, loop-iv.o, loop-invariant.o)
>   (cfgloopmanip.o, loop-init.o, loop-unswitch.o, loop-unroll.o)
>   (dominance.o, et-forest.o, combine.o, reginfo.o, bitmap.o, vec.o)
>   (hash-table.o, reload.o, reload1.o, rtlhooks.o, postreload.o)
> 

Re: [PATCH v3 15/18] make out_object_file rule use automatic dependencies

[Paolo Bonzini]

Il 20/08/2013 15:59, Tom Tromey ha scritto:
> This is a small change to make out_object_file use automatic
> dependencies.
> 
>   * Makefile.in ($(out_object_file)): Use COMPILE and POSTCOMPILE.
> ---
>  gcc/Makefile.in | 12 +++-
>  1 file changed, 3 insertions(+), 9 deletions(-)
> 
> diff --git a/gcc/Makefile.in b/gcc/Makefile.in
> index b22e8a8..c8b7b65 100644
> --- a/gcc/Makefile.in
> +++ b/gcc/Makefile.in
> @@ -2028,15 +2028,9 @@ pass-instances.def: $(srcdir)/passes.def 
> $(srcdir)/gen-pass-instances.awk
>   $(AWK) -f $(srcdir)/gen-pass-instances.awk \
> $(srcdir)/passes.def > pass-instances.def
>  
> -$(out_object_file): $(out_file) $(CONFIG_H) coretypes.h $(TM_H) $(TREE_H) \
> -   $(RTL_H) $(REGS_H) hard-reg-set.h insn-config.h conditions.h \
> -   output.h $(INSN_ATTR_H) $(SYSTEM_H) toplev.h $(DIAGNOSTIC_CORE_H) \
> -   $(TARGET_H) $(LIBFUNCS_H) $(TARGET_DEF_H) $(FUNCTION_H) $(SCHED_INT_H) \
> -   $(TM_P_H) $(EXPR_H) langhooks.h $(GGC_H) $(OPTABS_H) $(REAL_H) \
> -   tm-constrs.h $(GIMPLE_H) $(DF_H) cselib.h $(COMMON_TARGET_H) hw-doloop.h \
> -   regrename.h
> - $(COMPILER) -c $(ALL_COMPILERFLAGS) $(ALL_CPPFLAGS) \
> - $(out_file) $(OUTPUT_OPTION)
> +$(out_object_file): $(out_file)
> + $(COMPILE) $<
> + $(POSTCOMPILE)
>  
>  $(common_out_object_file): $(common_out_file)
>   $(COMPILE) $<
> 

Ok.

Paolo

Re: [PATCH v3 16/18] remove last reference to TREE_GIMPLE_H

[Paolo Bonzini]

Il 20/08/2013 15:59, Tom Tromey ha scritto:
> There is a single reference to TREE_GIMPLE_H in the source tree.
> Since it is not defined anywhere, we might as well remove the use.
> 
>   * config/i386/t-i386 (i386.o): Don't use TREE_GIMPLE_H.
> ---
>  gcc/config/i386/t-i386 | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/gcc/config/i386/t-i386 b/gcc/config/i386/t-i386
> index 07624cc..a26edda 100644
> --- a/gcc/config/i386/t-i386
> +++ b/gcc/config/i386/t-i386
> @@ -22,7 +22,7 @@ i386.o: $(CONFIG_H) $(SYSTEM_H) coretypes.h dumpfile.h 
> $(TM_H) \
>$(INSN_ATTR_H) $(FLAGS_H) $(C_COMMON_H) except.h $(FUNCTION_H) \
>$(RECOG_H) $(EXPR_H) $(OPTABS_H) toplev.h $(BASIC_BLOCK_H) \
>$(GGC_H) $(TARGET_H) $(TARGET_DEF_H) langhooks.h $(CGRAPH_H) \
> -  $(TREE_GIMPLE_H) $(DWARF2_H) $(DF_H) tm-constrs.h $(PARAMS_H) \
> +  $(DWARF2_H) $(DF_H) tm-constrs.h $(PARAMS_H) \
>i386-builtin-types.inc debug.h dwarf2out.h sbitmap.h $(FIBHEAP_H) \
>$(OPTS_H) $(DIAGNOSTIC_H) $(COMMON_TARGET_H) $(CONTEXT_H) $(PASS_MANAGER_H)
>  
> 

Could it be for gimple.h?

Paolo

Re: [PATCH v3 17/18] remove last definition of CROSS_FLOAT_H

[Paolo Bonzini]

Il 20/08/2013 15:59, Tom Tromey ha scritto:
> There is a single definition of CROSS_FLOAT_H in the source.
> As far as I can tell, this is never used anywhere.
> So, this patch removes it.
> 
>   * config/mcore/t-mcore (CROSS_FLOAT_H): Remove.
> ---
>  gcc/config/mcore/t-mcore | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/gcc/config/mcore/t-mcore b/gcc/config/mcore/t-mcore
> index 92fefb1..e5f37ee 100644
> --- a/gcc/config/mcore/t-mcore
> +++ b/gcc/config/mcore/t-mcore
> @@ -16,9 +16,6 @@
>  # along with GCC; see the file COPYING3.  If not see
>  # .
>  
> -# We have values for float.h.
> -CROSS_FLOAT_H = $(srcdir)/config/mcore/gfloat.h
> -
>  # If support for -m4align is ever re-enabled then comment out the
>  # following line and uncomment the multilib lines below.
>  
> 

Ok.

Paolo

Re: [PATCH v3 18/18] remove unused macros

[Paolo Bonzini]

Il 20/08/2013 15:59, Tom Tromey ha scritto:
> I used this perl script to find unused _H macros in the Makefile.  I
> deleted the definitions it reported and re-ran the script, until there
> was no more output.
> 
> The script also makes note of _H variables which are used but never
> defined.  That is how I found the TREE_GIMPLE_H use, fixed earlier in
> the series.
> 
> Once Ada and the various config files are migrated to automatic
> dependencies, we can run this script again and remove more things.
> 
> If you find yourself needing to add a new _H variable -- don't!
> Instead, take the few seconds to verify that automatic dependencies
> work in this case, and remove the manual dependencies for whatever
> object you are dealing with.
> 
> 
> 
> while (<>) {
> chomp;
> if (m/^(([A-Z0-9_]+)_H)\s*:?=\s/) {
>   $saw_def{$1} = $ARGV . ":" . $.;
> }
> 
> while (m/\$[({](([A-Z0-9_]+)_H)[)}](.*)/) {
>   $saw_use{$1} = 1;
>   $_ = $3;
> }
> } continue {
> close ARGV if eof;
> }
> 
> foreach $key (sort keys %saw_use) {
> if (! defined $saw_def{$key}) {
>   print "use without a def of ", $key, "\n";
> }
> }
> 
> foreach $key (sort keys %saw_def) {
> if (! defined $saw_use{$key}) {
>   print $saw_def{$key}, ": no use of ", $key, "\n";
> }
> }
> 
> 
>   * Makefile.in (PARTITION_H, LTO_SYMTAB_H, COMMON_TARGET_DEF_H)
>   (RTL_ERROR_H, TRANS_MEM_H, COVERAGE_H, DEMANGLE_H, ALIAS_H)
>   (SCHED_INT_H, SEL_SCHED_IR_H, SEL_SCHED_DUMP_H, VALTRACK_H, DDG_H)
>   (GGC_INTERNAL_H, DECNUM_H, BACKTRACE_H, MKDEPS_H, TREE_HASHER_H)
>   (TREE_SSA_LIVE_H, SSAEXPAND_H, DWARF2OUT_H, SCEV_H, OMEGA_H)
>   (TREE_DATA_REF_H, IRA_INT_H, LRA_INT_H, DBGCNT_H, DATA_STREAMER_H)
>   (GIMPLE_STREAMER_H, TREE_STREAMER_H, STREAMER_HOOKS_H)
>   (TREE_VECTORIZER_H, IPA_INLINE_H, GSTAB_H, LIBFUNCS_H)
>   (GRAPHITE_HTAB_H): Remove.
> ---
>  gcc/Makefile.in | 46 --
>  1 file changed, 46 deletions(-)
> 
> diff --git a/gcc/Makefile.in b/gcc/Makefile.in
> index c8b7b65..b645dc6 100644
> --- a/gcc/Makefile.in
> +++ b/gcc/Makefile.in
> @@ -446,7 +446,6 @@ HASHTAB_H   = $(srcdir)/../include/hashtab.h
>  OBSTACK_H   = $(srcdir)/../include/obstack.h
>  SPLAY_TREE_H= $(srcdir)/../include/splay-tree.h
>  FIBHEAP_H   = $(srcdir)/../include/fibheap.h
> -PARTITION_H = $(srcdir)/../include/partition.h
>  MD5_H= $(srcdir)/../include/md5.h
>  DWARF2_H= $(srcdir)/../include/dwarf2.h $(srcdir)/../include/dwarf2.def
>  XREGEX_H= $(srcdir)/../include/xregex.h
> @@ -454,7 +453,6 @@ FNMATCH_H   = $(srcdir)/../include/fnmatch.h
>  
>  # Linker plugin API headers
>  LINKER_PLUGIN_API_H = $(srcdir)/../include/plugin-api.h
> -LTO_SYMTAB_H = $(srcdir)/../include/lto-symtab.h
>  
>  # Default native SYSTEM_HEADER_DIR, to be overridden by targets.
>  NATIVE_SYSTEM_HEADER_DIR = @NATIVE_SYSTEM_HEADER_DIR@
> @@ -863,14 +861,11 @@ LANGHOOKS_DEF_H = langhooks-def.h $(HOOKS_H)
>  TARGET_DEF_H = target-def.h target-hooks-def.h $(HOOKS_H) targhooks.h
>  C_TARGET_DEF_H = c-family/c-target-def.h c-family/c-target-hooks-def.h \
>$(TREE_H) $(C_COMMON_H) $(HOOKS_H) common/common-targhooks.h
> -COMMON_TARGET_DEF_H = common/common-target-def.h \
> -  common/common-target-hooks-def.h $(HOOKS_H)
>  RTL_BASE_H = coretypes.h rtl.h rtl.def $(MACHMODE_H) reg-notes.def \
>insn-notes.def $(INPUT_H) $(REAL_H) statistics.h $(VEC_H) \
>$(FIXED_VALUE_H) alias.h $(HASHTAB_H)
>  FIXED_VALUE_H = fixed-value.h $(MACHMODE_H) double-int.h
>  RTL_H = $(RTL_BASE_H) $(FLAGS_H) genrtl.h
> -RTL_ERROR_H = rtl-error.h $(RTL_H) $(DIAGNOSTIC_CORE_H)
>  READ_MD_H = $(OBSTACK_H) $(HASHTAB_H) read-md.h
>  PARAMS_H = params.h params.def
>  BUILTINS_DEF = builtins.def sync-builtins.def omp-builtins.def \
> @@ -888,12 +883,8 @@ BASIC_BLOCK_H = basic-block.h $(PREDICT_H) $(VEC_H) 
> $(FUNCTION_H) \
>  GIMPLE_H = gimple.h gimple.def gsstruct.def pointer-set.h $(VEC_H) \
>   $(GGC_H) $(BASIC_BLOCK_H) $(TREE_H) tree-ssa-operands.h \
>   tree-ssa-alias.h $(INTERNAL_FN_H) $(HASH_TABLE_H)
> -TRANS_MEM_H = trans-mem.h
>  GCOV_IO_H = gcov-io.h gcov-iov.h auto-host.h
> -COVERAGE_H = coverage.h $(GCOV_IO_H)
> -DEMANGLE_H = $(srcdir)/../include/demangle.h
>  RECOG_H = recog.h
> -ALIAS_H = alias.h
>  EMIT_RTL_H = emit-rtl.h
>  FLAGS_H = flags.h flag-types.h $(OPTIONS_H)
>  OPTIONS_H = options.h flag-types.h $(OPTIONS_H_EXTRA)
> @@ -902,11 +893,6 @@ FUNCTION_H = function.h $(HASHTAB_H) $(TM_H) 
> hard-reg-set.h \
>  EXPR_H = expr.h insn-config.h $(FUNCTION_H) $(RTL_H) $(FLAGS_H) $(TREE_H) 
> $(MACHMODE_H) $(EMIT_RTL_H)
>  OPTABS_H = optabs.h insn-codes.h insn-opinit.h
>  REGS_H = regs.h $(MACHMODE_H) hard-reg-set.h
> -SCHED_INT_H = sched-int.h $(INSN_ATTR_H) $(BASIC_BLOCK_H) $(RTL_H) $(DF_H) \
> - $(REGSET_H)
> -SEL_SCHED_IR_H = sel-sched

Re: [PATCH v3 12/18] convert the Go front end to automatic dependencies

[Paolo Bonzini]

Il 26/08/2013 18:09, Tom Tromey ha scritto:
>> "Ian" == Ian Lance Taylor  writes:
> 
> Ian> I assume that dropping $(OUTPUT_OPTION) is correct--I haven't looked
> Ian> at the new definition of $(COMPILE).
> 
> I believe the depcomp script takes care of this.

I think that would be the "compile" script, not depcomp.  But I suspect
that this has been broken for a while, since the C front-end require
$(OUTPUT_OPTION).

You are listed as the author of the "compile" script in Automake, do you
remember which compilers need it?  I'm too young for that (that's
something I can say less and less :))...

Paolo

Re: [PATCH v3 02/18] update generated_files

[Paolo Bonzini]

Il 20/08/2013 15:59, Tom Tromey ha scritto:
> A few generated files were not mentioned in the generated_files
> variable.  This fixes the problem.
> 
>   * Makefile.in (generated_files): Add options.h,
>   target-hooks-def.h, insn-opinit.h,
>   common/common-target-hooks-def.h, pass-instances.def.
> ---
>  gcc/Makefile.in | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/gcc/Makefile.in b/gcc/Makefile.in
> index 8d2fd23..415f085 100644
> --- a/gcc/Makefile.in
> +++ b/gcc/Makefile.in
> @@ -3880,7 +3880,9 @@ s-gtype: build/gengtype$(build_exeext) $(filter-out 
> [%], $(GTFILES)) \
>  generated_files = config.h tm.h $(TM_P_H) $(TM_H) multilib.h \
> $(simple_generated_h) specs.h \
> tree-check.h genrtl.h insn-modes.h tm-preds.h tm-constrs.h \
> -   $(ALL_GTFILES_H) gtype-desc.c gtype-desc.h gcov-iov.h
> +   $(ALL_GTFILES_H) gtype-desc.c gtype-desc.h gcov-iov.h \
> +   options.h target-hooks-def.h insn-opinit.h \
> +   common/common-target-hooks-def.h pass-instances.def
>  
>  # In order for parallel make to really start compiling the expensive
>  # objects from $(OBJS) as early as possible, build all their
> 

Ok.

Paolo

Re: [PATCH v3 05/18] convert the C front end to automatic dependencies

[Paolo Bonzini]

Il 20/08/2013 15:59, Tom Tromey ha scritto:
> This converts the C front end.
> 
> Note that this fixes a latent bug in gcc/Makefile.in's definition of
> C_TREE_H.  This is needed to avoid breaking this build with this
> patch.
> 
>   * Makefile.in (C_TREE_H): Reference c/c-tree.h.
> 
>   * Make-lang.in (c/gccspec.o): Remove.
>   (CFLAGS-c/gccspec.o): New variable.
>   (cc1-checksum.o, C_TREE_H, c/c-aux-info.o, c/c-convert.o)
>   (c/c-decl.o, c/c-errors.o, c/c-lang.o, c/c-objc-common.o)
>   (c/c-parser.o, c/c-typeck.o, c/c-array-notation.o): Remove.

As I wrote in another reply, I suspect this breaks bootstrap with
compilers that do not support "-c -o", if it is not broken yet.

Can you put before patch 4 another that defines OUTPUT_OPTION to "-o $@"
unconditionally and removes all traces of NO_MINUS_C_MINUS_O?

The series looks okay to me with that change.

Paolo

Re: [PATCH v3 12/18] convert the Go front end to automatic dependencies

[Paolo Bonzini]

Il 16/09/2013 19:23, Tom Tromey ha scritto:
> Tom> I can try a test build using a setting for CC that rejects -c -o.
> 
> I did this and the build died pretty early on:
> 
>   /home/tromey/Space/Trunk/Git/bin/my-cc -c -DHAVE_CONFIG_H -g -g -I. 
> -I../../gcc/libiberty/../include  -W -Wall -Wwrite-strings -Wc++-compat 
> -Wstrict-prototypes -pedantic  -fpic ../../gcc/libiberty/regex.c -o 
> pic/regex.o; \
> else true; fi
> no -c -o support
> make[3]: *** [regex.o] Error 1
> make[3]: Leaving directory `/home/tromey/Space/Trunk/Git/build/libiberty'
> make[2]: *** [all-stage1-libiberty] Error 2
> 
> 
> In this case the --enable-shared for libiberty was added automatically
> because LTO defaults to enabled.  Adding --disable-lto lets it get
> further:
> 
> /home/tromey/Space/Trunk/Git/bin/my-cc -DPACKAGE_NAME=\"\" 
> -DPACKAGE_TARNAME=\"\" -DPACKAGE_VERSION=\"\" -DPACKAGE_STRING=\"\" 
> -DPACKAGE_BUGREPORT=\"\" -DPACKAGE_URL=\"\" -DPACKAGE=\"zlib\" 
> -DVERSION=\"1.1.4\" -DSTDC_HEADERS=1 -DHAVE_SYS_TYPES_H=1 -DHAVE_SYS_STAT_H=1 
> -DHAVE_STDLIB_H=1 -DHAVE_STRING_H=1 -DHAVE_MEMORY_H=1 -DHAVE_STRINGS_H=1 
> -DHAVE_INTTYPES_H=1 -DHAVE_STDINT_H=1 -DHAVE_UNISTD_H=1 -DHAVE_DLFCN_H=1 
> -DLT_OBJDIR=\".libs/\" -DHAVE_STDLIB_H=1 -DHAVE_UNISTD_H=1 
> -DHAVE_GETPAGESIZE=1 -DHAVE_MMAP=1 -DHAVE_MEMCPY=1 -DHAVE_STRERROR=1 
> -DHAVE_UNISTD_H=1 -I. -I../../gcc/zlib   -g  -g -c -o libz_a-adler32.o `test 
> -f 'adler32.c' || echo '../../gcc/zlib/'`adler32.c
> no -c -o support
> make[3]: *** [libz_a-adler32.o] Error 1
> make[3]: Leaving directory `/home/tromey/Space/Trunk/Git/build/zlib'
> make[2]: *** [all-stage1-zlib] Error 2
> 
> 
> And indeed, zlib's configure.ac does not invoke AC_PROG_CC_C_O.
> 
> So, maybe this has been broken a long time.

Likely.  Zapping OUTPUT_OPTION sounds like the right thing to do.

Paolo

Re: [PATCH v3 05/18] convert the C front end to automatic dependencies

[Paolo Bonzini]

Il 16/09/2013 22:58, Tom Tromey ha scritto:
> Paolo> The series looks okay to me with that change.
> 
> Two last questions while I'm testing my rebase --
> 
> First, do you mind if I resend the whole series?  Otherwise I can try to
> pick out just the patches that have changed, plus the additional
> patches.
> 
> Second, should I get a separate ok for the changes in the fortran
> directory?  Or does your approval here cover those?

I didn't review them because I saw front-end maintainers were looking at
their own patches, but I can approve them when you resend the whole series.

Paolo

Re: [PATCH v4 00/20] resurrect automatic dependencies

[Paolo Bonzini]

Il 25/09/2013 06:37, Alexandre Oliva ha scritto:
> On Sep 23, 2013, Tom Tromey  wrote:
> 
>> First, I believe I've addressed all of the comments in Paolo's review.
> 
> That's my feeling as well.  Sorry that I didn't manage to go through
> earlier versions of the patch before; I've just reviewed v4 plus the
> discussions around v3.
> 
> Save for one detail, I'm entirely comfortable and quite happy with the
> entire patchset.  The detail is the removal of support for compilers
> without -c -o.  However, since it has long been broken and Paolo
> suggested we drop it altogether, I'm going along with that plan.

Yeah, if zlib can afford not having support for that, and also
considering that we're now talking of a C++ compiler without -c -o
(which is probably even more rare)... than I really think we can drop that.

> As far as I'm concerned, the patch is approved, but I suggest giving
> Paolo a couple more days to chime in, since he reviewed the previous
> version.

The series is good!

Thanks,

Paolo

Re: [PATCH v4 04/20] add configury

[Paolo Bonzini]

Il 27/09/2013 21:45, Gerald Pfeifer ha scritto:
> I believe this may be breaking all my testers on FreeBSD 
> (i386-unknown-freebsd10.0 for example).  The timing of when this
> patchset went in fits pretty much when my builds started to break
> and I am wondering about some code.
> 
> Here is the failure mode:
> 
> gmake[2]: Entering directory `/scratch/tmp/gerald/OBJ-0927-1848/gcc'
> g++ -c  -DIN_GCC_FRONTEND -g -O2 -DIN_GCC   -fno-exceptions -fno-rtti
> -fasynchronous-unwind-tables -W -Wall -Wno-narrowing -Wwrite-strings
> -Wcast-qual -Wmissing-format-attribute -pedantic -Wno-long-long
> -Wno-variadic-macros -Wno-overlength-strings -Werror -fno-common
> -DHAVE_CONFIG_H -I. -Ic -I/scratch/tmp/gerald/gcc-HEAD/gcc ...[-I 
> options]...
> -o c/c-lang.o -MT c/c-lang.o -MMD -MP -MF c/.deps/c-lang.TPo
> /scratch/tmp/gerald/gcc-HEAD/gcc/c/c-lang.c
> cc1plus: error: unrecognized command line option "-Wno-narrowing"
> gmake[2]: *** [c/c-lang.o] Error 1
> gmake[1]: *** [install-gcc] Error 2
> gmake: *** [install] Error 2
> 
> The issue is the invocation of g++ (the old system compiler, not what
> we built) with -Wno-narrowing (a new option).

Why is install building anything?

Paolo

Re: [build] Update t-sparc, t-sol2 etc. for automatic dependencies

[Paolo Bonzini]

Il 02/10/2013 12:39, Rainer Orth ha scritto:
> Inspired by the t-i386 changes, the following patch moves SPARC and
> Solaris files over to automatic dependencies.
> 
> Bootstrapped without regression on sparc-sun-solaris2.11, verified that
> dependencies were generated for affected files.
> 
> Ok for mainline?
> 
>   Rainer
> 
> 
> 2013-10-01  Rainer Orth  
> 
>   * config/t-sol2 (sol2-c.o): Remove header dependencies.
>   Use $(COMPILE) and $(POSTCOMPILE).
>   (sol2-cxx.o): Likewise.
>   (sol2-stubs.o): Likewise.
>   (sol2.o): Likewise.
>   * config/x-solaris (host-solaris.o): Likewise.
> 
>   * config/sparc/t-sparc (sparc.o): Remove.
>   (sparc-c.o): Remove header dependencies.
>   Use $(COMPILE) and $(POSTCOMPILE).
>   * config/sparc/x-sparc: Likewise.
> 
> 
> 
> 

Sure.

Paolo

Re: RFC: LRA for x86/x86-64 [3/9]

[Paolo Bonzini]

Il 28/09/2012 00:57, Vladimir Makarov ha scritto:
> LRA creates a lot of new pseudos.  So the following patch implements
> ahead allocation reg info information which is important for LRA
> compilation speed.
> 
> 2012-09-27  Vladimir Makarov  
> 
> * reginfo.c (max_regno_since_last_resize): New.
> (reg_preferred_class, reg_alternate_class): Add assert.
> (allocate_reg_info): Initialize allocated reg info.
> (resize_reg_info): Make bigger reg_info and initialize new memory.
> (reginfo_init): Initialize max_regno_since_last_resize.
> (setup_reg_classes): Change assert.
> 

Is this considered dataflow stuff?  If so, I also want to approve part
of LRA! :)

This is ok.

Paolo

Re: [PATCH, i386]: Implement atomic_fetch_sub

[Paolo Bonzini]

Il 03/08/2012 17:08, Richard Henderson ha scritto:
> On 2012-08-03 08:01, Uros Bizjak wrote:
>> On Fri, Aug 3, 2012 at 4:40 PM, Richard Henderson  wrote:
>>> On 2012-08-03 01:51, Uros Bizjak wrote:
 The same reasoning goes for dynamic negation: for neg %eax,%eax value
 0x8000 stays the same, but we have changed (x)sub to an (x)add in
 the code stream.
>>>
>>> So?  Did you think the xadd will trap?
>>
>> No, but can we ignore the fact that we changed xsub -0x8000, mem
>> to xadd -0x08000, mem?
> 
> Yes, since it'll have the same effect on the bits.

In fact we can even use this trick for "xxor"...

Paolo

Re: RFC: LRA for x86/x86-64 [0/9]

[Paolo Bonzini]

Il 02/10/2012 09:28, Steven Bosscher ha scritto:
>>   My experience shows that these lists are usually 1-2 elements. Although in
>> > this case, there are pseudos with huge number elements (hundreeds).  I 
>> > tried
>> > -fweb for this tests because it can decrease the number elements but GCC (I
>> > don't know what pass) scales even worse: after 20 min of waiting and when
>> > virt memory achieved 20GB I stoped it.
> Ouch :-)
> 
> The webizer itself never even runs, the compiler blows up somewhere
> during the df_analyze call from web_main. The issue here is probably
> in the DF_UD_CHAIN problem or in the DF_RD problem.

/me is glad to have fixed fwprop when his GCC contribution time was more
than 1-2 days per year...

Unfortunately, the fwprop solution (actually a rewrite) was very
specific to the problem and cannot be reused in other parts of the compiler.

I guess here it is where we could experiment with region-based
optimization.  If a loop (including the parent dummy loop) is too big,
ignore it and only do LRS on smaller loops inside it.  Reaching
definitions is insanely expensive on an entire function, but works well
on smaller loops.

Perhaps something similar could be applied also to IRA/LRA.

Paolo

Re: RFC: LRA for x86/x86-64 [0/9]

[Paolo Bonzini]

Il 02/10/2012 10:49, Steven Bosscher ha scritto:
> On Tue, Oct 2, 2012 at 10:29 AM, Paolo Bonzini  wrote:
>> Il 02/10/2012 09:28, Steven Bosscher ha scritto:
>>>>   My experience shows that these lists are usually 1-2 elements. Although 
>>>> in
>>>>> this case, there are pseudos with huge number elements (hundreeds).  I 
>>>>> tried
>>>>> -fweb for this tests because it can decrease the number elements but GCC 
>>>>> (I
>>>>> don't know what pass) scales even worse: after 20 min of waiting and when
>>>>> virt memory achieved 20GB I stoped it.
>>> Ouch :-)
>>>
>>> The webizer itself never even runs, the compiler blows up somewhere
>>> during the df_analyze call from web_main. The issue here is probably
>>> in the DF_UD_CHAIN problem or in the DF_RD problem.
>>
>> /me is glad to have fixed fwprop when his GCC contribution time was more
>> than 1-2 days per year...
> 
> I thought you spent more time on GCC nowadays, working for Red Hat?

No, I work on QEMU most of the time. :)  Knowing myself, if I had
GCC-related assignments you'd see me _a lot_ on upstream mailing lists!

>> Unfortunately, the fwprop solution (actually a rewrite) was very
>> specific to the problem and cannot be reused in other parts of the compiler.
> 
> That'd be too bad... But is this really true? I thought you had
> something done that builds chains only for USEs reached by multiple
> DEFs? That's the only interesting kind for web, too.

No, it's the other way round.  I have a dataflow problem that recognizes
USEs reached by multiple DEFs, so that I can use a dominator walk to
build singleton def-use chains.  It's very similar to how you build SSA,
but punting instead of inserting phis.

Another solution is to build factored use-def chains for web, and use
them instead of RD.  In the end it's not very different from regional
live range splitting, since the phi functions factor out the state of
the pass at loop (that is region) boundaries.  I thought you had looked
at FUD chains years ago?

> FWIW: part of the problem for this particular test case is that there
> are many registers with partial defs (vector registers) and the RD
> problem doesn't (and probably cannot) keep track of one partial
> def/use killing another partial def/use.

So they are subregs of regs?  Perhaps they could be represented with
VEC_MERGE to break the live range:

 (set (reg:V4SI 94) (vec_merge:V4SI (reg:V4SI 94)
(const_vector:V4SI [(const_int 0)
(const_int 0)
(const_int 0)
(reg:SI 95)])
(const_int 7)))

And then reload, or something after reload, would know how to split
these when spilling V4SI to memory.

Paolo

Re: [PATCH v2, rtl-optimization]: Fix PR54457, [x32] Fail to combine 64bit index + constant

[Paolo Bonzini]

Il 30/09/2012 11:02, Richard Sandiford ha scritto:
> Uros Bizjak  writes:
>> On Thu, Sep 27, 2012 at 8:20 PM, Jakub Jelinek  wrote:
>>> On Thu, Sep 27, 2012 at 08:04:58PM +0200, Uros Bizjak wrote:
 After some off-line discussion with Richard, attached is v2 of the patch.

 2012-09-27  Uros Bizjak  

 PR rtl-optimization/54457
 * simplify-rtx.c (simplify_subreg):
   Simplify (subreg:SI (op:DI ((x:DI) (y:DI)), 0)
   to (op:SI (subreg:SI (x:DI) 0) (subreg:SI (x:DI) 0)).
>>>
>>> Is that a good idea even for WORD_REGISTER_OPERATIONS targets?
>>
>> I have bootstrapped and regtested [1] the patch on
>> alphaev68-pc-linux-gnu, a WORD_REGISTER_OPERATIONS target, and there
>> were no additional failures.
> 
> Thanks.  Given Jakub's question/concern, I'd really prefer a third
> opinion rather than approving it myself, but... OK if no-one objects
> within 24hrs.

I used to have a patch doing roughly the same thing (for more operations
but not MULT).  I never submitted because I didn't have the time to
audit all targets after changing the canonicalization.

Perhaps you can take the best of both worlds.

Paolo
change canonicalization

Index: gcc/Makefile.in
===
--- gcc/Makefile.in	(branch pr39726)
+++ gcc/Makefile.in	(working copy)
@@ -2844,7 +2844,7 @@ jump.o : jump.c $(CONFIG_H) $(SYSTEM_H) 
 simplify-rtx.o : simplify-rtx.c $(CONFIG_H) $(SYSTEM_H) coretypes.h $(TM_H) \
$(RTL_H) $(REGS_H) hard-reg-set.h $(FLAGS_H) $(REAL_H) insn-config.h \
$(RECOG_H) $(EXPR_H) $(TOPLEV_H) output.h $(FUNCTION_H) $(GGC_H) $(TM_P_H) \
-   $(TREE_H) $(TARGET_H)
+   $(TREE_H) $(TARGET_H) $(OPTABS_H)
 cgraph.o : cgraph.c $(CONFIG_H) $(SYSTEM_H) coretypes.h $(TM_H) $(TREE_H) \
langhooks.h $(TOPLEV_H) $(FLAGS_H) $(GGC_H) $(TARGET_H) $(CGRAPH_H) \
gt-cgraph.h output.h intl.h $(BASIC_BLOCK_H) debug.h $(HASHTAB_H) \
Index: gcc/simplify-rtx.c
===
--- gcc/simplify-rtx.c	(branch pr39726)
+++ gcc/simplify-rtx.c	(working copy)
@@ -39,6 +39,7 @@ along with GCC; see the file COPYING3.  
 #include "output.h"
 #include "ggc.h"
 #include "target.h"
+#include "optabs.h"
 
 /* Simplification and canonicalization of RTL.  */
 
@@ -5191,6 +5192,8 @@ rtx
 simplify_subreg (enum machine_mode outermode, rtx op,
 		 enum machine_mode innermode, unsigned int byte)
 {
+  int is_lowpart;
+
   /* Little bit of sanity checking.  */
   gcc_assert (innermode != VOIDmode);
   gcc_assert (outermode != VOIDmode);
@@ -5300,11 +5303,13 @@ simplify_subreg (enum machine_mode outer
   return NULL_RTX;
 }
 
+  is_lowpart = subreg_lowpart_offset (outermode, innermode) == byte;
+
   /* Merge implicit and explicit truncations.  */
 
   if (GET_CODE (op) == TRUNCATE
   && GET_MODE_SIZE (outermode) < GET_MODE_SIZE (innermode)
-  && subreg_lowpart_offset (outermode, innermode) == byte)
+  && is_lowpart)
 return simplify_gen_unary (TRUNCATE, outermode, XEXP (op, 0),
 			   GET_MODE (XEXP (op, 0)));
 
@@ -5343,7 +5348,7 @@ simplify_subreg (enum machine_mode outer
 	 The information is used only by alias analysis that can not
 	 grog partial register anyway.  */
 
-	  if (subreg_lowpart_offset (outermode, innermode) == byte)
+	  if (is_lowpart)
 	ORIGINAL_REGNO (x) = ORIGINAL_REGNO (op);
 	  return x;
 	}
@@ -5393,6 +5398,51 @@ simplify_subreg (enum machine_mode outer
   return NULL_RTX;
 }
 
+  /* Try to move a subreg inside an arithmetic operation.  */
+  if (is_lowpart && ARITHMETIC_P (op)
+  && GET_MODE_CLASS (outermode) == MODE_INT
+  && GET_MODE_CLASS (innermode) == MODE_INT)
+{
+  enum insn_code ic;
+  enum machine_mode cnt_mode;
+  switch (GET_CODE (op))
+	{
+	case ABS:
+	case NEG:
+	  return simplify_gen_unary (GET_CODE (op), outermode,
+ rtl_hooks.gen_lowpart_no_emit
+  (outermode, XEXP (op, 0)),
+ outermode);
+
+	case ASHIFT:
+	  /* i386 always uses QImode for the shift count.  Get the
+	 appropriate mode from the optab.  */
+	  ic = ashl_optab->handlers[outermode].insn_code;
+	  if (ic != CODE_FOR_nothing)
+	cnt_mode = insn_data[ic].operand[2].mode;
+	  else
+	cnt_mode = outermode;
+	  return simplify_gen_binary (GET_CODE (op), outermode,
+  rtl_hooks.gen_lowpart_no_emit
+   (outermode, XEXP (op, 0)),
+  rtl_hooks.gen_lowpart_no_emit
+   (cnt_mode, XEXP (op, 1)));
+
+	case PLUS:
+	case MINUS:
+	case AND:
+	case IOR:
+	case XOR:
+	  return simplify_gen_binary (GET_CODE (op), outermode,
+  rtl_hooks.gen_lowpart_no_emit
+   (outermode, XEXP (op, 0)),
+  rtl_hooks.gen_lowpart_no_emit
+   (outermode, XEXP (op, 1)));
+	default:
+	  break;
+	}
+}
+
   /* Optimize SUBREG truncations of zero and sign extended values.  */
   if ((GET_CODE (op) == ZERO_EXTEND
|| GET_CODE (op) == SIGN_EXTEND)
Index: gcc/combine.c

Re: [patch] Add option to compute "reaching and live definitions"

[Paolo Bonzini]

Il 07/10/2012 19:18, Steven Bosscher ha scritto:
> Hello,
> 
> The attached patch adds a DF changeable flag to compute a subset of
> reaching definitions that are also live at the program points they
> reach. This is an idea I discussed with Paolo many years ago already,
> but until today it hadn't really ever been close to the top of my todo
> list, but trying to compile the test case for PR54146 with -fweb
> finally changed that :-)
> 
> The idea is to prune the DF_RD_OUT set of each basic block by
> registers live in DF_LR_OUT. I've implemented this pruning with the
> same approach as the sparse formulation of RD dataflow, expanding the
> regs in DF_LR_OUT to the corresponding set of DEFs and using that set
> to mask out dead DEFs in DF_RD_OUT. This is a convenient formulation
> because DF_LR is already expressed in terms of regnos (like
> sparse_kill & friends), and the formulation also works fine for the
> dense formulation, of course.
> 
> The effect on compile time for a set of cc1-i files is negligible (not
> measurable, anyway), but for crazy large test cases like PR54146 this
> patch is the difference between triggering out-of-memory or completing
> the pass (at least -fweb, probably also the other affected passes).
> 
> Bootstrapped&tested on powerpc64-unknown-linux-gnu. OK for trunk?

Ok.

I wonder if we actually need the non-pruned version anywhere...

Paolo

> df_rd_pruned.diff
> 
>   * bitmap.h (bitmap_and_into): Update prototype.
>   * bitmap.c (bitmap_and_into): Return true if the target bitmap
>   changed, false otherwise.
> 
>   * df.h (df_dump_insn_problem_function): New function type.
>   (struct df_problem): Add two functions, to dump just before and
>   just after an insn.
>   (DF_RD_PRUNE_DEAD_DEFS): New changable flag.
>   (df_dump_insn_top, df_dump_insn_bottom): New prototypes.
>   * df-core (df_dump_region): Use dump_bb.
>   (df_dump_bb_problem_data): New function.
>   (df_dump_top, df_dump_bottom): Rewrite using df_dump_bb_problem_data.
>   (df_dump_insn_problem_data): New function.
>   (df_dump_insn_top, df_dump_insn_bottom): New functions.
>   * df-scan.c (problem_SCAN): Add NULL fields for new members.
>   * df-problems.c (df_rd_local_compute): Ignore hard registers if
>   DF_NO_HARD_REGS is in effect.
>   (df_rd_transfer_function): If DF_RD_PRUNE_DEAD_DEFS is in effect,
>   prune reaching defs using the LR problem.
>   (df_rd_start_dump): Fix dumping of DEFs map.
>   (df_rd_dump_defs_set): New function.
>   (df_rd_top_dump, df_rd_bottom_dump): Use it.
>   (problem_RD): Add NULL fields for new members.
>   (problem_LR, problem_LIVE): Likewise.
>   (df_chain_bb_dump): New function.
>   (df_chain_top_dump): Dump only for artificial DEFs and USEs,
>   using df_chain_bb_dump.
>   (df_chain_bottom_dump): Likewise.
>   (df_chain_insn_top_dump, df_chain_insn_bottom_dump): New functions.
>   (problem_CHAIN): Add them as new members.
>   (problem_WORD_LR, problem_NOTE): Add NULL fields for new members.
>   (problem_MD): Likewise.
>   * cfgrtl.c (rtl_dump_bb): Use df_dump_insn_top and df_dump_insn_bottom.
>   (print_rtl_with_bb): Likewise.
> 
>   * dce.c (init_dce): Use DF_RD_PRUNE_DEAD_DEFS.
>   * loop-invariant.c (find_defs): Likewise.
>   * loop-iv.c (iv_analysis_loop_init): Likewise.
>   * ree.c (find_and_remove_re): Likewise.
>   * web.c (web_main): Likewise.
> 
> Index: bitmap.h
> ===
> --- bitmap.h  (revision 192106)
> +++ bitmap.h  (working copy)
> @@ -224,7 +224,7 @@ extern unsigned long bitmap_count_bits (const_bitm
> are three operand versions that to not destroy the source bitmaps.
> The operations supported are &, & ~, |, ^.  */
>  extern void bitmap_and (bitmap, const_bitmap, const_bitmap);
> -extern void bitmap_and_into (bitmap, const_bitmap);
> +extern bool bitmap_and_into (bitmap, const_bitmap);
>  extern bool bitmap_and_compl (bitmap, const_bitmap, const_bitmap);
>  extern bool bitmap_and_compl_into (bitmap, const_bitmap);
>  #define bitmap_compl_and(DST, A, B) bitmap_and_compl (DST, B, A)
> Index: bitmap.c
> ===
> --- bitmap.c  (revision 192106)
> +++ bitmap.c  (working copy)
> @@ -916,17 +916,18 @@ bitmap_and (bitmap dst, const_bitmap a, const_bitm
>  dst->indx = dst->current->indx;
>  }
>  
> -/* A &= B.  */
> +/* A &= B.  Return true if A changed.  */
>  
> -void
> +bool
>  bitmap_and_into (bitmap a, const_bitmap b)
>  {
>bitmap_element *a_elt = a->first;
>const bitmap_element *b_elt = b->first;
>bitmap_element *next;
> +  bool changed = false;
>  
>if (a == b)
> -return;
> +return false;
>  
>while (a_elt && b_elt)
>  {
> @@ -935,6 +936,7 @@ bitmap_and_into (bitmap a, const_bitmap b)
> next = a_elt->next;
> bitmap_element_free (a

Re: Fix twolf -funroll-loops -O3 miscompilation (a semi-latent web.c bug)

[Paolo Bonzini]

Il 13/10/2012 00:25, Steven Bosscher ha scritto:
> On Fri, Oct 12, 2012 at 11:16 PM, Jan Hubicka  wrote:
>>> On Fri, Oct 12, 2012 at 10:44 PM, Jan Hubicka  wrote:
  1) computing liveness with REG_EQUAL included prior RD that means a lot
 of shuffling of REG_DEAD notes
>>>
>>> I was already working on a patch for this. I'll send it here later tonight.
>>
>> Great, thanks!  This is probably most sensible approach even if we will need 
>> to
>> recompute liveness before/after webizer.
> 
> I don't think we have to touch the liveness sets. We can compute an
> extra set of registers live only for REG_EQUAL/REG_EQUIV notes.
> Attached is what I had in mind. Untested, etc. it's late (and the
> Yankees are playing) so I'll get back to properly testing this
> tomorrow.

Can we just simulate liveness for web, and drop REG_EQUAL/REG_EQUIV
notes that refer to a dead pseudo?

Paolo

Re: Ping: RFA: Fix OP_INOUT handling of web.c:union_match_dups

[Paolo Bonzini]

Il 15/10/2012 07:10, Joern Rennecke ha scritto:
> 2012-10-02  Joern Rennecke  
> 
> * web.c (union_match_dups): Properly handle OP_INOUT match_dups.
> 
> http://gcc.gnu.org/ml/gcc-patches/2012-10/msg00189.html
> 

Ok.

Paolo

Re: Fix twolf -funroll-loops -O3 miscompilation (a semi-latent web.c bug)

[Paolo Bonzini]

Il 14/10/2012 22:59, Steven Bosscher ha scritto:
> On Sun, Oct 14, 2012 at 9:02 AM, Paolo Bonzini wrote:
>> Can we just simulate liveness for web, and drop REG_EQUAL/REG_EQUIV
>> notes that refer to a dead pseudo?
> 
> I don't think we want to do that. A REG_EQUAL/REG_EQUIV note can use a
> pseudo that isn't live and still be valid. Consider a simple example
> like this:
> 
> a = b + 3
> // b dies here
> c = a {REG_EQUAL b+3}
> 
> The REG_EQUAL note is valid and may help optimization. Removing it
> just because b is dead at that point would be unnecessarily
> pessimistic.

I disagree that it is valid.  At least it is risky to consider it valid,
because a pass that simulates liveness might end up doing something
wrong because of that note.  If simulation is done backwards, it doesn't
even require any interaction with REG_DEAD notes.

> I also don't want to compute DF_LR taking EQ_USES into account as real
> uses for liveness, because that involves recomputing and enlarging the
> DF_LR sets (all of them, both globally and locally) before LR&RD and
> after LR&RD. That's why I implemented the quick-and-dirty liveness
> computation for the notes: It's non-intrusive on DF_LR and it's cheap.

Yes, I agree on that part of the implementation. :)

Paolo

Re: Fix twolf -funroll-loops -O3 miscompilation (a semi-latent web.c bug)

[Paolo Bonzini]

Il 15/10/2012 10:13, Steven Bosscher ha scritto:
> > I disagree that it is valid.  At least it is risky to consider it valid,
> > because a pass that simulates liveness might end up doing something
> > wrong because of that note.  If simulation is done backwards, it doesn't
> > even require any interaction with REG_DEAD notes.
> 
> In any case, if web doesn't properly rename the register in the
> REG_EQUAL note (which it doesn't do without my patch) and we declare
> such a note invalid, then we should remove the note. You're right that
> GCC ends up doing something wrong, that's why Honza's test case fails.
> 
> I think we should come to a conclusion of this discussion: Either we
> drop the notes (e.g. by re-computing the DF_NOTE problem after web) or
> we update them, like my patch does. I prefer the patch I proposed
> because it re-instates the behavior GCC had before.

I prefer to declare the notes invalid and drop the notes.

Paolo

Re: Fix twolf -funroll-loops -O3 miscompilation (a semi-latent web.c bug)

[Paolo Bonzini]

Il 15/10/2012 10:37, Steven Bosscher ha scritto:
>> I prefer to declare the notes invalid and drop the notes.
> Then, afaic, our only option is to drop them all in web, as per attached 
> patch.
>
> I strongly disagree with this approach though. It destroys information
> that is correct, that we had before DF_RD_PRUNE_DEAD_DEFS, that we can
> update, and that helps with optimization. With renaming these notes
> are valid, and do not refer to dead regs

I agree it is bad.  But I do not understand the last sentence: I
suppose you mean that _without_ renaming these notes are valid, on the
other hand it is normal that some of the notes will be dropped if you
shorten live ranges.

Without removing all of the notes you can do something like this:

- drop the deferred rescanning from web.c.  Instead, make replace_ref
return a bool and call df_insn_rescan manually from web_main.

- attribute new registers to webs in a separate pass that happens
before rewriting, and compute a special version of LR_IN/LR_OUT that
uses the rewritten registers.

- process instructions in reverse order; before starting the visit of
a basic block, initialize the local LR bitmap with the rewritten
LR_OUT of the previous step

- after rewriting and scanning each statement, simulate liveness using
the new defs and uses.

- after rewriting each statement, look for EQ_USES referring to
registers that are dead just before the statement, and delete
REG_EQUAL notes if this is the case

Paolo

> This whole discussion about notes being dead has gone in completely
> the wrong direction. With renaming these notes are valid, and do not
> refer to dead regs. Perhaps you could be convinced if you look at
> Honza's test case with the patch of r192413 reverted.
> The test case still fails with --param max-unroll-times=3, that makes
> visualizing the problem easier.

Re: Fix twolf -funroll-loops -O3 miscompilation (a semi-latent web.c bug)

[Paolo Bonzini]

Il 15/10/2012 14:53, Steven Bosscher ha scritto:
> I think I've shown above that we're all looking at the wrong pass...

I think you have... so we want a patch like this?

Index: df-problems.c
===
--- df-problems.c   (revisione 183719)
+++ df-problems.c   (copia locale)
@@ -3480,6 +3485,18 @@ df_note_bb_compute (unsigned int bb_inde
}
}
 
+  for (use_rec = DF_INSN_UID_EQ_USES (uid); *use_rec; use_rec++)
+   {
+ df_ref use = *use_rec;
+ unsigned int uregno = DF_REF_REGNO (use);
+
+ if (!bitmap_bit_p (live, uregno))
+{
+  remove_note (insn, find_reg_equal_equiv_note (insn));
+  break;
+}
+   }
+
   if (debug_insn == -1)
{
  /* ??? We could probably do better here, replacing dead

Paolo

Re: Fix twolf -funroll-loops -O3 miscompilation (a semi-latent web.c bug)

[Paolo Bonzini]

Il 16/10/2012 12:35, Steven Bosscher ha scritto:
> On Tue, Oct 16, 2012 at 12:15 PM, Steven Bosscher wrote:
>> On Mon, Oct 15, 2012 at 3:26 PM, Steven Bosscher wrote:
>>> On Mon, Oct 15, 2012 at 3:21 PM, Paolo Bonzini wrote:
>>>> Il 15/10/2012 14:53, Steven Bosscher ha scritto:
>>>>> I think I've shown above that we're all looking at the wrong pass...
>>>>
>>>> I think you have... so we want a patch like this?
>>>
>>> I don't think so. df_kill_notes is already supposed to take care of this.
>>
>> But it doesn't because if the SET_DEST of an insn is the same as the
>> register dieing in the insn's REG_EQUAL note, the reg is live at the
>> end of the insn and so the note stays:
>>
>> Breakpoint 2, df_kill_notes (insn=0x75e3e7e0, live=0x7fffda90)
>> at ../../trunk/gcc/df-problems.c:2833
>> 2833  rtx *pprev = ®_NOTES (insn);
>> 1: debug_rtx(insn) = (insn 79 50 52 8 (set (reg:DI 72 [ ivtmp.17D.1758 ])
>> (reg:DI 103 [ ivtmp.17D.1758 ])) -1
>>  (expr_list:REG_DEAD (reg:DI 103 [ ivtmp.17D.1758 ])
>> (expr_list:REG_EQUAL (plus:DI (reg:DI 72 [ ivtmp.17D.1758 ])
>> (const_int 24 [0x18]))
>> (nil
>> void
>> (gdb) undisp 1
>> (gdb) p debug_bitmap(live)
>>
>> first = 0x1627200 current = 0x1627200 indx = 0
>> 0x1627200 next = (nil) prev = (nil) indx = 0
>> bits = { 6 7 16 20 72 82 85 87 }
>> $2 = void
>> (gdb)
>>
>>
>> So GCC should be looking at whether the reg in the REG_EQUAL note is
>> dead *before* the insn.
>>
>> Bottom line: This is a bug in df_kill_notes.

Yep, and it could cause wrong code generation, couldn't it?  Because the
new (reg:DI 72) is obviously not equal to (plus:DI (reg:DI 72)
(const_int 24)), but it could be propagated to subsequent insns.

So I think this patch should be backported to all release branches after
regstrapping.

> I think this should fix it. Can't test it right now, so help
> appreciated (Honza: hint hint! ;-)

Ok after bootstrap, regtest and checking that it actually fixes the bug.

Paolo

Re: [ping] couple of fixes

[Paolo Bonzini]

Il 19/10/2012 19:01, Eric Botcazou ha scritto:
> PR bootstrap/54820 (stage #1 bootstrap failure)
>   http://gcc.gnu.org/ml/gcc-patches/2012-10/msg01093.html

This one is okay, thanks.

Paolo

Re: [PATCH v4 04/20] add configury

[Paolo Bonzini]

Il 15/10/2013 00:28, Gerald Pfeifer ha scritto:
>  - The problem is not actually the set of flags being used.  On a 
>different tester clang is the bootstrap compiler, and this is
>also the one invoked for `gmake install`.  If anything, shouldn't
>the just built compiler be used _if_ anything is left to compile?

Yes---but then, the question remains as to why anything is built at all.

Paolo

Re: AIX: Dependency problem with xlC/g++ combination

[Paolo Bonzini]

Il 24/10/2013 03:17, Tom Tromey ha scritto:
> Jan-Benedict>   I'd like to get some comments on this patch, which works
> Jan-Benedict> for me for stage-1 builds with gcc/g++ (on a amd64-linux
> Jan-Benedict> box) as well as with xlC/g++ on gcc111 (the mentioned AIX
> Jan-Benedict> machine):
> 
> FWIW, I believe this patch is correct.
> However, I cannot approve it.

The patch is okay indeed.

Paolo

Re: [PATCH 1/n] Add conditional compare support

[Paolo Bonzini]

Il 18/09/2013 11:45, Zhenqiang Chen ha scritto:
> +;; Expand conditional compare according to the instruction patterns.
> +(define_expand "conditional_compare"
> +  [(set (match_operand 0 "s_register_operand" "")
> + (match_operator 1 ""
> +  [(match_operator:SI 2 "arm_comparison_operator"
> +   [(match_operand:SI 3 "s_register_operand" "")
> +(match_operand:SI 4 "arm_add_operand" "")])

My first reaction to this is that this operand should only match
const0_rtx, and operands 2/3 should be the same mode as operand 0 rather
than SImode.  In particular, perhaps they could be CCmode or BImode on
some architectures.

But then Richard is right that this would look a lot like the old
compare + branch patterns.

So either you could do this at expansion time using hooks, like Richard
mentioned, or you could do a pattern matching pass that is specific to
this pattern and combines sets with MODE_CC destinations across multiple
basic blocks.

Paolo


> +  (match_operator:SI 5 "arm_comparison_operator"
> +   [(match_operand:SI 6 "s_register_operand" "")
> +(match_operand:SI 7 "arm_add_operand" "")])]))]
> +  "TARGET_ARM || TARGET_THUMB2"
> +  "{
> + enum machine_mode mode;
> + HOST_WIDE_INT cond_or;
> + rtx par;
> + enum rtx_code code = GET_CODE (operands[1]);
> + cond_or = code == AND? DOM_CC_X_AND_Y : DOM_CC_X_OR_Y;
> +
> + mode = arm_select_dominance_cc_mode (operands[2],
> +   operands[5],
> +   cond_or);
> + /* To match and/ior_scc_scc, mode should not be CCmode.  */
> + if (mode == CCmode)
> +   FAIL;
> +
> + operands[2] = gen_rtx_fmt_ee (GET_CODE (operands[2]),
> +GET_MODE (operands[3]),
> +operands[3], operands[4]);
> + operands[5] = gen_rtx_fmt_ee (GET_CODE (operands[5]),
> +GET_MODE (operands[5]),
> +operands[6], operands[7]);
> + operands[1] = gen_rtx_fmt_ee (GET_CODE (operands[1]), SImode,
> +operands[2], operands[5]);
> +
> +  /* Generate insn to match and/ior_scc_scc.  */
> +  par = gen_rtx_PARALLEL (VOIDmode, rtvec_alloc (2));
> +  XVECEXP (par, 0, 0) = gen_rtx_SET (GET_MODE (operands[0]),
> + operands[0], operands[1]);
> +  XVECEXP (par, 0, 1) = gen_rtx_CLOBBER (CCmode,
> +  gen_rtx_REG (CCmode, CC_REGNUM));
> +
> +  emit_insn (par);
> +
> + DONE;
> +  }"

Re: PATCH to use -Wno-format during stage1

[Paolo Bonzini]

Il 30/10/2013 16:47, Jason Merrill ha scritto:
> I find -Wformat warnings about unknown % codes when building with an
> older GCC to be mildly annoying noise; this patch avoids them by passing
> -Wno-format during stage 1.
> 
> Tested x86_64-pc-linux-gnu.  Is this OK for trunk?

Ok.

Re: [wide-int] Add fast path for hosts with HWI widening multiplication

[Paolo Bonzini]

Il 02/12/2013 20:34, Richard Sandiford ha scritto:
>>> >> I followed Joseph's suggestion and reused longlong.h.  I copied it from
>>> >> libgcc rather than glibc since it seemed better for GCC to have a single
>>> >> version across both gcc/ and libgcc/.  I can put it in include/ if that
>>> >> seems better.
>> >
>> > Actually copying complex code like this does not seem maintainable.  I
>> > think there needs to be only one copy in the GCC sources.  If that
>> > requires moving it back from libgcc to gcc, or moving it to include,
>> > do that.
> OK, will do, but which do you prefer?

libgcc/ should not use gcc/ sources too much.  Please put it in include/.

Paolo

Re: [PATCH][07/10] -fuse-caller-save - Use collected register usage information

[Paolo Bonzini]

Il 06/12/2013 01:56, Tom de Vries ha scritto:
>>
>>
>> This patch series adds analysis of register usage of functions for
>> usage by IRA.
>> The original post is here
>> ( http://gcc.gnu.org/ml/gcc-patches/2013-01/msg01234.html ).
>>
>> This patch uses the information of which registers are clobbered by a
>> call
>> in IRA and df-scan.
>>
>> Bootstrapped and reg-tested on x86_64, Ada inclusive. Build and
>> reg-tested on
>> mips, arm, ppc and sh.
>>
>> Can you approve the df-scan part for trunk?
>>
> 
> Paolo,
> 
> Ping. df-scan part OK for stage1?

Yes, thanks.

Paolo

Re: [RFC, patch] Detect lack of 32-bit devel environment on x86_64-linux targets

[Paolo Bonzini]

Il 09/12/2013 11:46, Gerald Pfeifer ha scritto:
> Hmm, it looks like this has not been approved/applied, but I also
> have not seen any NACK.
> 
> This does address an annoying (and hard for novices to understand)
> roadblock for someone installing GCC manually.  Can this go in?

I'm not sure why this should be different for x86_64 compared to all
other bi-arch toolchains?

I think the right place for this is a "Non-bugs" section in the
installation manual.

Paolo

Re: [RFC, patch] Detect lack of 32-bit devel environment on x86_64-linux targets

[Paolo Bonzini]

Il 09/12/2013 12:08, Gerald Pfeifer ha scritto:
> On Mon, 9 Dec 2013, FX wrote:
>> > Look at this as a diagnostics bug: our current diagnostics for this 
>> > pretty common situation sucks. It comes late in the compilation, and 
>> > the message itself isn’t helpful.
> Totally seconded.
> 
> Paolo, I have been running into this myself and was confused at
> first.  If that happens to me, building GCC for some 20 years,
> imagine a new user.
> 
> This is really something to be identified at configure time,
> with a reasonable error message.
> 
> If FX's specific patch is not ideal yet, can you help rework it?
> That would be great.

The patch is okay, but if other architecture maintainers could add
similar checks for their ports (SPARC and PPC, I guess), it would be nice.

Paolo

[PATCH] PR/66760, ipa-inline-analysis.c compile-time hog

[Paolo Bonzini]

From: bonz...@gnu.org

In this PR, a lot of time is spent doing the same ipa_load_from_parm_agg
query over and over.  Luckily a memoization scheme is already there, it's
just not used by ipa-inline-analysis.c.  The patch moves the cache struct
(struct func_body_info) to ipa-prop.h and modify ipa-inline-analysis.c.
On some testcases from PR26854 the "alias stmt walking" timevar goes
off the profile while it used to be 30-70%.

Bootstrapped (regtest in progress) on x86_64-pc-linux-gnu.

Please commit the patch for me if approved, as I don't have anymore
the key I used to use for gcc.gnu.org.  One of these days I'll send
my new SSH public key to the overseers.

Paolo

* ipa-inline-analysis.c (unmodified_parm_or_parm_agg_item): Accept
struct func_body_info* instead of struct ipa_node_params*, expecting
fbi->info to be filled in.  Replace throughout.  Adjust call to
ipa_load_from_parm_agg.
(set_cond_stmt_execution_predicate): Accept struct func_body_info*
instead of struct ipa_node_params*.  Adjust calls to other functions
so that they pass either fbi or fbi->info.
(set_switch_stmt_execution_predicate): Likewise.
(will_be_nonconstant_predicate): Likewise.
(compute_bb_predicates): Likewise.
(estimate_function_body_sizes): Move asserts earlier.  Fill in
struct func_body_info, replace parms_info with fbi.info.  Adjust
calls to functions that now accept struct func_body_info.
* ipa-prop.c (struct param_aa_status, struct ipa_bb_info,
struct func_body_info).  Move to ipa-prop.h.
(ipa_load_from_parm_agg_1): Rename to ipa_load_from_parm_agg,
remove static.  Adjust callers.
(ipa_load_from_parm_agg): Remove.
* ipa-prop.h (struct param_aa_status, struct ipa_bb_info,
struct func_body_info).  Move from ipa-prop.c.
(ipa_load_from_parm_agg): Adjust prototype.

diff --git a/gcc/ipa-inline-analysis.c b/gcc/ipa-inline-analysis.c
index d5dbfbd..81a6860 100644
--- a/gcc/ipa-inline-analysis.c
+++ b/gcc/ipa-inline-analysis.c
@@ -1574,7 +1574,7 @@ unmodified_parm (gimple stmt, tree op)
loaded.  */
 
 static bool
-unmodified_parm_or_parm_agg_item (struct ipa_node_params *info,
+unmodified_parm_or_parm_agg_item (struct func_body_info *fbi,
  gimple stmt, tree op, int *index_p,
  struct agg_position_info *aggpos)
 {
@@ -1583,7 +1583,7 @@ unmodified_parm_or_parm_agg_item (struct ipa_node_params 
*info,
   gcc_checking_assert (aggpos);
   if (res)
 {
-  *index_p = ipa_get_param_decl_index (info, res);
+  *index_p = ipa_get_param_decl_index (fbi->info, res);
   if (*index_p < 0)
return false;
   aggpos->agg_contents = false;
@@ -1599,13 +1599,14 @@ unmodified_parm_or_parm_agg_item (struct 
ipa_node_params *info,
   stmt = SSA_NAME_DEF_STMT (op);
   op = gimple_assign_rhs1 (stmt);
   if (!REFERENCE_CLASS_P (op))
-   return unmodified_parm_or_parm_agg_item (info, stmt, op, index_p,
+   return unmodified_parm_or_parm_agg_item (fbi, stmt, op, index_p,
 aggpos);
 }
 
   aggpos->agg_contents = true;
-  return ipa_load_from_parm_agg (info, stmt, op, index_p, &aggpos->offset,
-&aggpos->by_ref);
+  return ipa_load_from_parm_agg (fbi, fbi->info->descriptors,
+stmt, op, index_p, &aggpos->offset,
+NULL, &aggpos->by_ref);
 }
 
 /* See if statement might disappear after inlining.
@@ -1744,7 +1745,7 @@ eliminated_by_inlining_prob (gimple stmt)
predicates to the CFG edges.   */
 
 static void
-set_cond_stmt_execution_predicate (struct ipa_node_params *info,
+set_cond_stmt_execution_predicate (struct func_body_info *fbi,
   struct inline_summary *summary,
   basic_block bb)
 {
@@ -1767,7 +1768,7 @@ set_cond_stmt_execution_predicate (struct ipa_node_params 
*info,
   /* TODO: handle conditionals like
  var = op0 < 4;
  if (var != 0).  */
-  if (unmodified_parm_or_parm_agg_item (info, last, op, &index, &aggpos))
+  if (unmodified_parm_or_parm_agg_item (fbi, last, op, &index, &aggpos))
 {
   code = gimple_cond_code (last);
   inverted_code = invert_tree_comparison (code, HONOR_NANS (op));
@@ -1810,8 +1811,7 @@ set_cond_stmt_execution_predicate (struct ipa_node_params 
*info,
   || gimple_call_num_args (set_stmt) != 1)
 return;
   op2 = gimple_call_arg (set_stmt, 0);
-  if (!unmodified_parm_or_parm_agg_item
-  (info, set_stmt, op2, &index, &aggpos))
+  if (!unmodified_parm_or_parm_agg_item (fbi, set_stmt, op2, &index, &aggpos))
 return;
   FOR_EACH_EDGE (e, ei, bb->succs) if (e->flags & EDGE_FALSE_VALUE)
 {
@@ -1827,7 +1827,7 @@ set_cond_stmt_execution_predicate (struct ipa_node_params 
*info,
predicates to the CFG edges.   */
 
 static void
-set_switch_stmt_execution_predicate (struct ipa_node_params *info,
+set_switch_stmt_execution_predicate (struct func_body_info *fbi,
  

Re: [PATCH] PR/66760, ipa-inline-analysis.c compile-time hog

[Paolo Bonzini]

On 13/07/2015 13:55, Martin Jambor wrote:
> I can't approve it, but FWIW, I'm generally fine with the patch.
> Although the original idea was to share one func_body_info in between
> ipa-cp and ipa-inline analyses, this is certainly better than what we
> have now and perhaps even good enough generally.

Ah, so you'd add a pointer to cgraph_node?  I only have a question
then---does cgraph_node have a "destructor" where I can free the
func_body_info?  Or is there no such thing?

> The only semi-issue I have is the name of func_body_info.  If it is
> going to be exposed in a header file, perhaps it should get an ipa_
> prefix.

That's a patch as large as this one.  I can do the rename later, and
maybe have it preapproved to convince me to get the new public key in
place. :)

> I also think that its initialization should be put into a
> common function, but that is somethig I can do as a followup, if need
> be.

Tried doing that now...  it seems better to do it together with adding a
func_body_info* to cgraph_node*, so that the initialization is done
lazily in cgraph_node::get_func_body_info.

Paolo

Re: [PATCH] PR/66760, ipa-inline-analysis.c compile-time hog

[Paolo Bonzini]

On 13/07/2015 14:34, Martin Jambor wrote:
> You might want to use Martin's shiny new
> function_summary class in symbol-summary.c.  That is a mechanism
> specifically designed to append to a cgraph_node information specific
> to an optimization pass (or two, as ipa-cp and ipa-inline already both
> use a few of them).  Unfortunately, the class is not very well
> documented but you should be able to figure out how to use it from
> other code using them.
> 
> If you then always deallocate everything there at the end of
> ipa-inline analysis, you'll get exactly the right life-time for the
> data.

Good.  I might as well merge func_body_info and ipa_node_params then, so
I already have ipa_node_params_sum.  WDYT?

Paolo

Re: [PATCH] PR/66760, ipa-inline-analysis.c compile-time hog

[Paolo Bonzini]

On 13/07/2015 15:45, Richard Biener wrote:
> It would be nice to have a patch that can be backported to the GCC 5 branch
> as well.  We can improve this on trunk as followup,no?

The patch I've already posted can be backported. O:-)

Paolo

Re: [PATCH] PR/66760, ipa-inline-analysis.c compile-time hog

[Paolo Bonzini]

On 15/07/2015 18:01, Martin Jambor wrote:
> > So unless Martin objects consider the patch approved for trunk and for
> > backporting after 5.2 is released and trunk shows no issues.
> > 
> > Martin - can you take care of committing if you are fine with it?
> 
> I have commitred the patch to trunk (and hopefully will not forget to
> backport it once 5 branch reopens).  I am testing the following
> hopefully obvious followup, which I plan to commit tomorrow.  It is
> just mechanical renaming of the newly exported structures to give them
> ipa prefix.

Thanks Martin!

Paolo

Re: [PATCH 5/9] i386: Add address spaces for fs/gs segments

[Paolo Bonzini]

On 08/10/2015 06:59, Richard Henderson wrote:
> +/* Address space support.
> +
> +   This is not "far pointers" in the 16-bit sense, but an easy way
> +   to use %fs and %gs segment prefixes.  Therefore:
> +
> +(a) All address spaces have the same modes,
> +(b) All address spaces have the same addresss forms,
> +(c) While %fs and %gs are technically subsets of the generic
> +address space, they are probably not subsets of each other.
> +(d) Since we have no access to the segment base register values
> +without resorting to a system call, we cannot convert a
> +non-default address space to a default address space.
> +Therefore we do not claim %fs or %gs are subsets of generic.

rdfsbase and rdgsbase are potentially accessible to userspace too, so I
think %fs or %gs should be considered subsets of generic.

Paolo

> +   Therefore, we need not override any of the address space hooks.  */

Re: [PATCH] PR 68192 Export AIX TLS symbols

[Paolo Bonzini]

On 05/11/2015 17:28, David Edelsohn wrote:
> [Explicitly copying build maintainers.]
> 
> Paolo and Alexandre,
> 
> Could you review and help with this patch?
> 
> TLS symbols in AIX display a new, different symbol type in nm output.
> Libtool explicitly creates a list of exported symbols for shared
> libraries using nm and does not recognize the new TLS symbols, so
> those symbols are not exported.
> 
> This is a regression for TLS support on AIX.
> 
> This patch updates libtool.m4 in GCC and configure for libstdc++-v3,
> libgfortran, and libgomp.  I would like to apply the patch to GCC
> while I simultaneously work with the Libtool community to correct the
> bug upstream.  I also would like to backport this to GCC 5.2 and GCC
> 4.9.x.

I think it's okay to wait for the patch to be upstream.

I can help committing the patch once it is.

Paolo

> I have not been able to run the correct versions of autoconf to
> regenerate configure directly.  I either can edit the files directly
> or I would appreciate someone helping me to regenerate configure in
> all library directories.
> 
> Bootstrapped on powerpc-ibm-aix7.1.0.0.
> 
> * libtool.m4 (export_symbols_cmds) [AIX]: Add global TLS "L" symbols.
> * libstdc++-v3/configure: Regenerate.
> * libgfortran/configure: Regenerate.
> * libgomp/configure: Regenerate.
> 
> Thanks, David
> 

Index: libtool.m4
===
--- libtool.m4  (revision 229706)
+++ libtool.m4  (working copy)
@@ -4230,7 +4230,7 @@
 if $NM -V 2>&1 | $GREP 'GNU' > /dev/null; then
   _LT_TAGVAR(export_symbols_cmds, $1)='$NM -Bpg $libobjs $convenience | 
awk '\''{ if (((\$ 2 == "T") || (\$ 2 == "D") || (\$ 2 == "B") || (\$ 2 == 
"W")) && ([substr](\$ 3,1,1) != ".")) { print \$ 3 } }'\'' | sort -u > 
$export_symbols'
 else
-  _LT_TAGVAR(export_symbols_cmds, $1)='$NM -BCpg $libobjs $convenience | 
awk '\''{ if (((\$ 2 == "T") || (\$ 2 == "D") || (\$ 2 == "B")) && ([substr](\$ 
3,1,1) != ".")) { print \$ 3 } }'\'' | sort -u > $export_symbols'
+  _LT_TAGVAR(export_symbols_cmds, $1)='$NM -BCpg $libobjs $convenience | 
awk '\''{ if (((\$ 2 == "T") || (\$ 2 == "D") || (\$ 2 == "B") || (\$ 2 == 
"L")) && ([substr](\$ 3,1,1) != ".")) { print \$ 3 } }'\'' | sort -u > 
$export_symbols'
 fi
 ;;
   pw32*)
@@ -4641,7 +4641,7 @@
if $NM -V 2>&1 | $GREP 'GNU' > /dev/null; then
  _LT_TAGVAR(export_symbols_cmds, $1)='$NM -Bpg $libobjs $convenience | 
awk '\''{ if (((\$ 2 == "T") || (\$ 2 == "D") || (\$ 2 == "B") || (\$ 2 == 
"W")) && ([substr](\$ 3,1,1) != ".")) { print \$ 3 } }'\'' | sort -u > 
$export_symbols'
else
- _LT_TAGVAR(export_symbols_cmds, $1)='$NM -BCpg $libobjs $convenience 
| awk '\''{ if (((\$ 2 == "T") || (\$ 2 == "D") || (\$ 2 == "B")) && 
([substr](\$ 3,1,1) != ".")) { print \$ 3 } }'\'' | sort -u > $export_symbols'
+ _LT_TAGVAR(export_symbols_cmds, $1)='$NM -BCpg $libobjs $convenience 
| awk '\''{ if (((\$ 2 == "T") || (\$ 2 == "D") || (\$ 2 == "B") || (\$ 2 == 
"L")) && ([substr](\$ 3,1,1) != ".")) { print \$ 3 } }'\'' | sort -u > 
$export_symbols'
fi
aix_use_runtimelinking=no
 
Index: libstdc++-v3/configure
===
--- libstdc++-v3/configure  (revision 229706)
+++ libstdc++-v3/configure  (working copy)
@@ -9539,7 +9539,7 @@
if $NM -V 2>&1 | $GREP 'GNU' > /dev/null; then
  export_symbols_cmds='$NM -Bpg $libobjs $convenience | awk '\''{ if 
(((\$ 2 == "T") || (\$ 2 == "D") || (\$ 2 == "B") || (\$ 2 == "W")) && 
(substr(\$ 3,1,1) != ".")) { print \$ 3 } }'\'' | sort -u > $export_symbols'
else
- export_symbols_cmds='$NM -BCpg $libobjs $convenience | awk '\''{ if 
(((\$ 2 == "T") || (\$ 2 == "D") || (\$ 2 == "B")) && (substr(\$ 3,1,1) != 
".")) { print \$ 3 } }'\'' | sort -u > $export_symbols'
+ export_symbols_cmds='$NM -BCpg $libobjs $convenience | awk '\''{ if 
(((\$ 2 == "T") || (\$ 2 == "D") || (\$ 2 == "B") || (\$ 2 == "L")) && 
(substr(\$ 3,1,1) != ".")) { print \$ 3 } }'\'' | sort -u > $export_symbols'
fi
aix_use_runtimelinking=no
 
@@ -14058,7 +14058,7 @@
 if $NM -V 2>&1 | $GREP 'GNU' > /dev/null; then
   export_symbols_cmds_CXX='$NM -Bpg $libobjs $convenience | awk '\''{ if 
(((\$ 2 == "T") || (\$ 2 == "D") || (\$ 2 == "B") || (\$ 2 == "W")) && 
(substr(\$ 3,1,1) != ".")) { print \$ 3 } }'\'' | sort -u > $export_symbols'
 else
-  export_symbols_cmds_CXX='$NM -BCpg $libobjs $convenience | awk '\''{ if 
(((\$ 2 == "T") || (\$ 2 == "D") || (\$ 2 == "B")) && (substr(\$ 3,1,1) != 
".")) { print \$ 3 } }'\'' | sort -u > $export_symbols'
+  export_symbols_cmds_CXX='$NM -BCpg $libobjs $convenience | awk '\''{ if 
(((\$ 2 == "T") || (\$ 2 == "D") || (\$ 2 == "B") || (\$ 2 == "L")) && 
(substr(\$ 3,1,1) != ".")) { print \$ 3 } }'\'' | sort -u > $export_symbols'
 fi
 ;;
   pw32*)
Index: libgfortran/configure

Re: [PATCH] PR 68192 Export AIX TLS symbols

[Paolo Bonzini]

On 05/11/2015 17:38, David Edelsohn wrote:
> On Thu, Nov 5, 2015 at 8:34 AM, Paolo Bonzini  wrote:
>>
>>
>> On 05/11/2015 17:28, David Edelsohn wrote:
>>> [Explicitly copying build maintainers.]
>>>
>>> Paolo and Alexandre,
>>>
>>> Could you review and help with this patch?
>>>
>>> TLS symbols in AIX display a new, different symbol type in nm output.
>>> Libtool explicitly creates a list of exported symbols for shared
>>> libraries using nm and does not recognize the new TLS symbols, so
>>> those symbols are not exported.
>>>
>>> This is a regression for TLS support on AIX.
>>>
>>> This patch updates libtool.m4 in GCC and configure for libstdc++-v3,
>>> libgfortran, and libgomp.  I would like to apply the patch to GCC
>>> while I simultaneously work with the Libtool community to correct the
>>> bug upstream.  I also would like to backport this to GCC 5.2 and GCC
>>> 4.9.x.
>>
>> I think it's okay to wait for the patch to be upstream.
>>
>> I can help committing the patch once it is.
> 
> The patch MUST go into GCC 5.3 release, which Richi has announced that
> he wants to release around the beginning of Stage 3.  The patch should
> not go into GCC 5 branch without going into trunk.

What is blocking the patch from being included in libtool in the next
week or two?

> This patch cannot
> wait unless you want to block GCC 5.3.

What blocks GCC 5.3 is decided by the release manager.  As a build
maintainer, I am not going to ack the patch for trunk until it is
included in Libtool, but I'm happy to be overridden by the GCC 5 release
manager.

Paolo

[PATCH] Clarify that -fwrapv covers all signed arithmetic overflow

[Paolo Bonzini]

GCC's -fwrapv option does not affect code generation for shifts
because currently GCC does not rely on the fact that certain
signed shifts trigger undefined behavior.  However, the definition
of signed arithmetic overflow does extend to shifts; it is only
code generation that is limited to addition, subtraction and
multiplication.

Make the documentation of -fwrapv consistent with the existing
text under -fstrict-overflow ("Using '-fwrapv' means that integer
signed overflow is fully defined: it wraps.").

Ok for trunk and the branches?

Paolo

* doc/invoke.texi (Optimize Options): Clarify the effect of -fwrapv.

Index: doc/invoke.texi
===
--- doc/invoke.texi (revision 227511)
+++ doc/invoke.texi (working copy)
@@ -23705,9 +23705,10 @@
 @item -fwrapv
 @opindex fwrapv
 This option instructs the compiler to assume that signed arithmetic
-overflow of addition, subtraction and multiplication wraps around
-using twos-complement representation.  This flag enables some optimizations
-and disables others.  This option is enabled by default for the Java
+overflow wraps around using twos-complement representation.
+This flag affects code generation for addition, subtraction
+and multiplication, enabling some optimizations
+and disabling others.  This option is enabled by default for the Java
 front end, as required by the Java language specification.
 The options @option{-ftrapv} and @option{-fwrapv} override each other, so using
 @option{-ftrapv} @option{-fwrapv} on the command-line results in

Re: [PATCH] Clarify that -fwrapv covers all signed arithmetic overflow

[Paolo Bonzini]

On 17/11/2015 13:58, Joseph Myers wrote:
>> > GCC's -fwrapv option does not affect code generation for shifts
>> > because currently GCC does not rely on the fact that certain
>> > signed shifts trigger undefined behavior.  However, the definition
>> > of signed arithmetic overflow does extend to shifts; it is only
>> > code generation that is limited to addition, subtraction and
>> > multiplication.
> It is part of the GNU C language, independent of -fwrapv, that shifts 
> where the shift amount is in the range [0, width - 1] are fully defined 
> (although ubsan will still sanitize those not defined in ISO C)

-fwrapv should probably disable those checks, just like it disables e.g.
"signed integer overflow: 1 + 2147483647 cannot be represented in type
'int'".

> - they are 
> considered to be defined in terms of bits, not integers, so overflow is 
> not a meaningful concept for them.

Can you suggest a wording for "if the GNU C language definition changes
[which, no matter how unlikely, is explicitly not ruled out by the
manual] -fwrapv will be extended to signed shifts, and shifts of
negative numbers would return A*2^B whenever the result fits in the type"?

Paolo

> -fwrapv *should* however affect division and modulo -1, although it 
> doesn't at present (see bug 30484).

[RFC PATCH] Do not sanitize left shifts for -fwrapv

[Paolo Bonzini]

Left shifts into the sign bit is a kind of overflow, and the
standard chooses to treat left shifts of negative values the
same way.

However, the -fwrapv option modifies the language to one where
integers are defined as two's complement---which also defines
entirely the behavior of shifts.  Disable sanitization of left
shifts when -fwrapv is in effect.

This needs test cases of course, but I wanted to be sure in advance
whether this is an acceptable change and whether it is considered
a bug (thus acceptable for stage 3).  The same change was proposed
for LLVM at https://llvm.org/bugs/show_bug.cgi?id=25552.

Paolo

* c-family/c-ubsan.c (ubsan_instrument_shift): Disable sanitization
of left shifts for wrapping signed types as well.


Index: c-family/c-ubsan.c
===
--- c-family/c-ubsan.c  (revision 227511)
+++ c-family/c-ubsan.c  (working copy)
@@ -150,7 +150,7 @@
  (unsigned) x >> (uprecm1 - y)
  if non-zero, is undefined.  */
   if (code == LSHIFT_EXPR
-  && !TYPE_UNSIGNED (type0)
+  && !TYPE_OVERFLOW_WRAPS (type0)
   && flag_isoc99)
 {
   tree x = fold_build2 (MINUS_EXPR, op1_utype, uprecm1,
@@ -165,7 +165,7 @@
  x < 0 || ((unsigned) x >> (uprecm1 - y))
  if > 1, is undefined.  */
   if (code == LSHIFT_EXPR
-  && !TYPE_UNSIGNED (type0)
+  && !TYPE_OVERFLOW_WRAPS (type0)
   && (cxx_dialect >= cxx11))
 {
   tree x = fold_build2 (MINUS_EXPR, op1_utype, uprecm1,

Re: [PATCH] Clarify that -fwrapv covers all signed arithmetic overflow

[Paolo Bonzini]

On 17/11/2015 16:27, Joseph Myers wrote:
> > Can you suggest a wording for "if the GNU C language definition changes
> > [which, no matter how unlikely, is explicitly not ruled out by the
> > manual] -fwrapv will be extended to signed shifts, and shifts of
> > negative numbers would return A*2^B whenever the result fits in the type"?
>
> I don't think we can usefully say how a hypothetical change in one area 
> would or would not affect a particular option.

I agree.  That is why I phrased my original patch in the other way,
assuming that overflow _can_ be defined for signed left shifts but it is
not treated as undefined.  My definition of overflow for signed left
shifts would be shifting a 1 into or out of the sign bit.

However, I understood that you don't want to define overflow of signed
left shifts.

The reason why I am proposing this patch is that the current
documentation has a sort of catch-22:

* it doesn't promise that GCC will never rely on undefined behavior
rules for signed left shifts

* it says that -fwrapv affects add/sub/mult

This means that GCC has no future-proof option for projects that wish to
rely on definedness of signed left shifts.

In fact, as you mentioned, ubsan _already_ provides a case where GCC
does not treat left shift as an operation on the bit representation.
This makes it even more important to define such an option _now_ and to
make ubsan respect it (for which I've also sent an RFC patch earlier today).

Paolo

Re: [PATCH] Clarify that -fwrapv covers all signed arithmetic overflow

[Paolo Bonzini]

On 17/11/2015 17:02, Joseph Myers wrote:
> On Tue, 17 Nov 2015, Paolo Bonzini wrote:
> 
>> * it doesn't promise that GCC will never rely on undefined behavior
>> rules for signed left shifts
> 
> I think we should remove the ", but this is subject to change" in 
> implement-c.texi (while replacing it with noting that ubsan will still 
> diagnose such cases, and they will also be diagnosed where constant 
> expressions are required).

That's great.  I'll send a patch.

Paolo

Re: [PATCH] Clarify that -fwrapv covers all signed arithmetic overflow

[Paolo Bonzini]

On 17/11/2015 17:02, Joseph Myers wrote:
> On Tue, 17 Nov 2015, Paolo Bonzini wrote:
> 
>> * it doesn't promise that GCC will never rely on undefined behavior
>> rules for signed left shifts
> 
> I think we should remove the ", but this is subject to change" in 
> implement-c.texi (while replacing it with noting that ubsan will still 
> diagnose such cases, and they will also be diagnosed where constant 
> expressions are required).

... hmm, are you sure?  None of the following warn for me

int x = -1 << 2;
int y = 1 << 31;
int z = 2 << 31;

I tried with any combination of -ansi, -pedantic, -std=cXX,
-fsanitize=undefined.

Paolo

Re: GCC 5.3 Status Report (2015-11-20)

[Paolo Bonzini]

On 20/11/2015 14:14, David Edelsohn wrote:
> On Fri, Nov 20, 2015 at 7:53 AM, Richard Biener  wrote:
>>
>> Status
>> ==
>>
>> We plan to do a GCC 5.3 release candidate at the end of next week
>> followed by the actual release a week after that.
>>
>> So now is the time to look at your regression bugs in bugzilla and
>> do some backporting for things already fixed on trunk.
> 
> I'm still waiting for approval of the libtool change to support AIX
> TLS symbols (PR 68192).  There has been no response on Libtool patches
> mailing list.
> 
> I again request permission to apply the patches to GCC trunk and 5-branch.

Let me look around for a libtool committer tomorrow, I'll get back to you.

Paolo

Re: [PATCH] PR 68192 Export AIX TLS symbols

[Paolo Bonzini]

Eric, can you help committing this patch to libtool?

http://article.gmane.org/gmane.comp.gcc.patches/356566

Thanks,

Paolo

On 03/11/2015 15:23, David Edelsohn wrote:
> TLS symbols in AIX display a new, different symbol type in nm output.
> Libtool explicitly creates a list of exported symbols for shared
> libraries using nm and does not recognize the new TLS symbols, so
> those symbols are not exported.
> 
> This is a regression for TLS support on AIX.
> 
> This patch updates libtool.m4 in GCC and configure for libstdc++-v3,
> libgfortran, and libgomp.  I would like to apply the patch to GCC
> while I simultaneously work with the Libtool community to correct the
> bug upstream.  I also would like to backport this to GCC 5.2 and GCC
> 4.9.x.
> 
> I have not been able to run the correct versions of autoconf to
> regenerate configure directly.  I either can edit the files directly
> or I would appreciate someone helping me to regenerate configure in
> all library directories.
> 
> Bootstrapped on powerpc-ibm-aix7.1.0.0.
> 
> * libtool.m4 (export_symbols_cmds) [AIX]: Add global TLS "L" symbols.
> * libstdc++-v3/configure: Regenerate.
> * libgfortran/configure: Regenerate.
> * libgomp/configure: Regenerate.
> 
> Thanks, David
> 

[PATCH v2] Clarify promises about undefined behavior with signed <

[Paolo Bonzini]

GCC's -fwrapv option does not affect code generation for shifts
because currently GCC does not rely on the fact that certain
signed shifts trigger undefined behavior.  However, the definition
of signed arithmetic overflow does extend to shifts; it is only
code generation that is limited to addition, subtraction and
multiplication.

Make the documentation of -fwrapv consistent with the existing
text under -fstrict-overflow ("Using '-fwrapv' means that integer
signed overflow is fully defined: it wraps.").

Ok for trunk, and for GCC 5 branch after 5.3 is released?

Paolo

* doc/implement-c.texi (Integers Implementation): Make GCC's promises
about signed left shift stronger and clarify the cases when they're
broken.

Index: doc/implement-c.texi
===
--- doc/implement-c.texi(revision 230466)
+++ doc/implement-c.texi(working copy)
@@ -266,9 +266,11 @@
 immediately above the highest-value value bit.  Signed @samp{>>} acts
 on negative numbers by sign extension.
 
-GCC does not use the latitude given in C99 and C11 only to treat certain
-aspects of signed @samp{<<} as undefined, but this is subject to
-change.
+As an extension to the C language, GCC does not use the latitude given in
+C99 and C11 only to treat certain aspects of signed @samp{<<} as undefined.
+However, @option{-fsanitize=shift} (and @option{-fsanitize=undefined}) will
+diagnose such cases.  They are also diagnosed where constant
+expressions are required.
 
 @item
 @cite{The sign of the remainder on integer division (C90 6.3.5).}

[PATCH] Do not sanitize left shifts for -fwrapv (PR68418)

[Paolo Bonzini]

Left shifts into the sign bit is a kind of overflow, and the
standard chooses to treat left shifts of negative values the
same way.

However, the -fwrapv option modifies the language to one where
integers are defined as two's complement---which also defines
entirely the behavior of shifts.  Disable sanitization of left
shifts when -fwrapv is in effect.  The same change was proposed
for LLVM at https://llvm.org/bugs/show_bug.cgi?id=25552.

Bootstrapped/regtested x86_64-pc-linux-gnu.  Ok for trunk, and for
GCC 5 branch after 5.3 is released?

Thanks,

Paolo

gcc:
PR sanitizer/68418
* c-family/c-ubsan.c (ubsan_instrument_shift): Disable
sanitization of left shifts for wrapping signed types as well.

gcc/testsuite:
PR sanitizer/68418
* gcc.dg/ubsan/c99-wrapv-shift-1.c,
gcc.dg/ubsan/c99-wrapv-shift-2.c: New testcases.

Index: c-family/c-ubsan.c
===
--- c-family/c-ubsan.c  (revision 230466)
+++ c-family/c-ubsan.c  (working copy)
@@ -128,7 +128,7 @@
  (unsigned) x >> (uprecm1 - y)
  if non-zero, is undefined.  */
   if (code == LSHIFT_EXPR
-  && !TYPE_UNSIGNED (type0)
+  && !TYPE_OVERFLOW_WRAPS (type0)
   && flag_isoc99)
 {
   tree x = fold_build2 (MINUS_EXPR, op1_utype, uprecm1,
@@ -143,7 +143,7 @@
  x < 0 || ((unsigned) x >> (uprecm1 - y))
  if > 1, is undefined.  */
   if (code == LSHIFT_EXPR
-  && !TYPE_UNSIGNED (type0)
+  && !TYPE_OVERFLOW_WRAPS (type0)
   && (cxx_dialect >= cxx11))
 {
   tree x = fold_build2 (MINUS_EXPR, op1_utype, uprecm1,
Index: testsuite/gcc.dg/ubsan/c99-wrapv-shift-1.c
===
--- testsuite/gcc.dg/ubsan/c99-wrapv-shift-1.c  (revision 0)
+++ testsuite/gcc.dg/ubsan/c99-wrapv-shift-1.c  (working copy)
@@ -0,0 +1,9 @@
+/* { dg-do run } */
+/* { dg-options "-fsanitize=shift -fwrapv -w -std=c99" } */
+
+int
+main (void)
+{
+  int a = -42;
+  a << 1;
+}
Index: testsuite/gcc.dg/ubsan/c99-wrapv-shift-2.c
===
--- testsuite/gcc.dg/ubsan/c99-wrapv-shift-2.c  (revision 0)
+++ testsuite/gcc.dg/ubsan/c99-wrapv-shift-2.c  (working copy)
@@ -0,0 +1,9 @@
+/* { dg-do run } */
+/* { dg-options "-fsanitize=shift -fwrapv -w -std=c99" } */
+
+int
+main (void)
+{
+  int a = 1;
+  a <<= 31;
+}

Re: GCC 5.3 Status Report (2015-11-20)

[Paolo Bonzini]

On 24/11/2015 16:57, David Edelsohn wrote:
> > > We plan to do a GCC 5.3 release candidate at the end of next week
> > > followed by the actual release a week after that.
> > >
> > > So now is the time to look at your regression bugs in bugzilla and
> > > do some backporting for things already fixed on trunk.
> >
> > I'm still waiting for approval of the libtool change to support AIX
> > TLS symbols (PR 68192).  There has been no response on Libtool patches
> > mailing list.
> >
> > I again request permission to apply the patches to GCC trunk and 5-branch.
> >
> > Let me look around for a libtool committer tomorrow, I'll get back to you.
>
> Any progress?

Patch committed to upstream libtool, thanks for your understanding.

Paolo

[PATCH] -Wshift-overflow: Warn for shifting sign bit out of a negative number

[Paolo Bonzini]

maybe_warn_shift_overflow is checking for patterns such as (1 << 31)
and not warning for them.

However, if the shifted value is negative, a shift by a non-zero
amount will always shift *out* of the sign bit rather than into it.
Thus it should be warned about, even if the value only requires
one bit more than the precision of the LHS.

Ok for trunk?

Paolo

gcc:
* c-family/c-common.c (maybe_warn_shift_overflow): Warn on all
overflows if shifting 1 out of the sign bit.

gcc/testsuite:
* c-c++-common/Wshift-overflow-1.c: Test shifting 1 out of the sign bit.
* c-c++-common/Wshift-overflow-2.c: Test shifting 1 out of the sign bit.
* c-c++-common/Wshift-overflow-3.c: Test shifting 1 out of the sign bit.
* c-c++-common/Wshift-overflow-4.c: Test shifting 1 out of the sign bit.
* c-c++-common/Wshift-overflow-6.c: Test shifting 1 out of the sign bit.
* c-c++-common/Wshift-overflow-7.c: Test shifting 1 out of the sign bit.

Index: c-family/c-common.c
===
--- c-family/c-common.c (revision 230930)
+++ c-family/c-common.c (working copy)
@@ -12631,8 +12631,11 @@ maybe_warn_shift_overflow (location_t loc, tree op
 
   unsigned int min_prec = (wi::min_precision (op0, SIGNED)
   + TREE_INT_CST_LOW (op1));
-  /* Handle the left-shifting 1 into the sign bit case.  */
-  if (min_prec == prec0 + 1)
+  /* Handle the case of left-shifting 1 into the sign bit.
+   * However, shifting 1 _out_ of the sign bit, as in
+   * INT_MIN << 1, is considered an overflow.
+   */
+  if (!tree_int_cst_sign_bit(op0) && min_prec == prec0 + 1)
 {
   /* Never warn for C++14 onwards.  */
   if (cxx_dialect >= cxx14)
Index: testsuite/c-c++-common/Wshift-overflow-1.c
===
--- testsuite/c-c++-common/Wshift-overflow-1.c  (revision 230930)
+++ testsuite/c-c++-common/Wshift-overflow-1.c  (working copy)
@@ -8,6 +8,9 @@
 #define LLONGM1 (sizeof (long long) * __CHAR_BIT__ - 1)
 #define LLONGM2 (sizeof (long long) * __CHAR_BIT__ - 2)
 
+#define INT_MIN (-__INT_MAX__-1)
+#define LONG_LONG_MIN (-__LONG_LONG_MAX__-1)
+
 int i1 = 1 << INTM1;
 int i2 = 9 << INTM1; /* { dg-warning "requires 36 bits to represent" } */
 int i3 = 10 << INTM2; /* { dg-warning "requires 35 bits to represent" } */
@@ -18,6 +21,7 @@
 int i8 = -10 << INTM2; /* { dg-warning "requires 35 bits to represent" } */
 int i9 = -__INT_MAX__ << 2; /* { dg-warning "requires 34 bits to represent" } 
*/
 int i10 = -__INT_MAX__ << INTM1; /* { dg-warning "requires 63 bits to 
represent" } */
+int i11 = INT_MIN << 1; /* { dg-warning "requires 33 bits to represent" } */
 
 int r1 = 1 >> INTM1;
 int r2 = 9 >> INTM1;
@@ -46,6 +50,7 @@
 long long int l8 = -10LL << LLONGM2; /* { dg-warning "requires 67 bits to 
represent" } */
 long long int l9 = -__LONG_LONG_MAX__ << 2; /* { dg-warning "requires 66 bits 
to represent" } */
 long long int l10 = -__LONG_LONG_MAX__ << LLONGM1; /* { dg-warning "requires 
127 bits to represent" } */
+long long int l11 = LONG_LONG_MIN << 1; /* { dg-warning "requires 65 bits to 
represent" } */
 
 void
 fn (void)
Index: testsuite/c-c++-common/Wshift-overflow-2.c
===
--- testsuite/c-c++-common/Wshift-overflow-2.c  (revision 230930)
+++ testsuite/c-c++-common/Wshift-overflow-2.c  (working copy)
@@ -8,6 +8,9 @@
 #define LLONGM1 (sizeof (long long) * __CHAR_BIT__ - 1)
 #define LLONGM2 (sizeof (long long) * __CHAR_BIT__ - 2)
 
+#define INT_MIN (-__INT_MAX__-1)
+#define LONG_LONG_MIN (-__LONG_LONG_MAX__-1)
+
 int i1 = 1 << INTM1;
 int i2 = 9 << INTM1;
 int i3 = 10 << INTM2;
@@ -18,6 +21,7 @@
 int i8 = -10 << INTM2;
 int i9 = -__INT_MAX__ << 2;
 int i10 = -__INT_MAX__ << INTM1;
+int i11 = INT_MIN << 1;
 
 int r1 = 1 >> INTM1;
 int r2 = 9 >> INTM1;
@@ -46,6 +50,7 @@
 long long int l8 = -10LL << LLONGM2;
 long long int l9 = -__LONG_LONG_MAX__ << 2;
 long long int l10 = -__LONG_LONG_MAX__ << LLONGM1;
+long long int l11 = LONG_LONG_MIN << 1;
 
 void
 fn (void)
Index: testsuite/c-c++-common/Wshift-overflow-3.c
===
--- testsuite/c-c++-common/Wshift-overflow-3.c  (revision 230930)
+++ testsuite/c-c++-common/Wshift-overflow-3.c  (working copy)
@@ -9,6 +9,9 @@
 #define LLONGM1 (sizeof (long long) * __CHAR_BIT__ - 1)
 #define LLONGM2 (sizeof (long long) * __CHAR_BIT__ - 2)
 
+#define INT_MIN (-__INT_MAX__-1)
+#define LONG_LONG_MIN (-__LONG_LONG_MAX__-1)
+
 int i1 = 1 << INTM1;
 int i2 = 9 << INTM1; /* { dg-warning "requires 36 bits to represent" } */
 int i3 = 10 << INTM2; /* { dg-warning "requires 35 bits to represent" } */
@@ -19,6 +22,7 @@
 int i8 = -10 << INTM2; /* { dg-warning "requires 35 bits to represent" } */
 int i9 = -__INT_MAX__ << 2; /* { dg-warning "requires 34 bits to represent" } 
*/
 int i10 = -__INT_MAX__ << INTM1; /* { dg-warning "requires 63 bits to 
represent" } */
+int i11 = INT_MIN << 1; /*

Re: [PATCH] Do not sanitize left shifts for -fwrapv (PR68418)

[Paolo Bonzini]

On 25/11/2015 14:55, Paolo Bonzini wrote:
> Left shifts into the sign bit is a kind of overflow, and the
> standard chooses to treat left shifts of negative values the
> same way.
> 
> However, the -fwrapv option modifies the language to one where
> integers are defined as two's complement---which also defines
> entirely the behavior of shifts.  Disable sanitization of left
> shifts when -fwrapv is in effect.  The same change was proposed
> for LLVM at https://llvm.org/bugs/show_bug.cgi?id=25552.
> 
> Bootstrapped/regtested x86_64-pc-linux-gnu.  Ok for trunk, and for
> GCC 5 branch after 5.3 is released?
> 
> Thanks,
> 
> Paolo
> 
> gcc:
>   PR sanitizer/68418
>   * c-family/c-ubsan.c (ubsan_instrument_shift): Disable
>   sanitization of left shifts for wrapping signed types as well.
> 
> gcc/testsuite:
>   PR sanitizer/68418
>   * gcc.dg/ubsan/c99-wrapv-shift-1.c,
>   gcc.dg/ubsan/c99-wrapv-shift-2.c: New testcases.
> 
> Index: c-family/c-ubsan.c
> ===
> --- c-family/c-ubsan.c(revision 230466)
> +++ c-family/c-ubsan.c(working copy)
> @@ -128,7 +128,7 @@
>   (unsigned) x >> (uprecm1 - y)
>   if non-zero, is undefined.  */
>if (code == LSHIFT_EXPR
> -  && !TYPE_UNSIGNED (type0)
> +  && !TYPE_OVERFLOW_WRAPS (type0)
>&& flag_isoc99)
>  {
>tree x = fold_build2 (MINUS_EXPR, op1_utype, uprecm1,
> @@ -143,7 +143,7 @@
>   x < 0 || ((unsigned) x >> (uprecm1 - y))
>   if > 1, is undefined.  */
>if (code == LSHIFT_EXPR
> -  && !TYPE_UNSIGNED (type0)
> +  && !TYPE_OVERFLOW_WRAPS (type0)
>&& (cxx_dialect >= cxx11))
>  {
>tree x = fold_build2 (MINUS_EXPR, op1_utype, uprecm1,
> Index: testsuite/gcc.dg/ubsan/c99-wrapv-shift-1.c
> ===
> --- testsuite/gcc.dg/ubsan/c99-wrapv-shift-1.c(revision 0)
> +++ testsuite/gcc.dg/ubsan/c99-wrapv-shift-1.c(working copy)
> @@ -0,0 +1,9 @@
> +/* { dg-do run } */
> +/* { dg-options "-fsanitize=shift -fwrapv -w -std=c99" } */
> +
> +int
> +main (void)
> +{
> +  int a = -42;
> +  a << 1;
> +}
> Index: testsuite/gcc.dg/ubsan/c99-wrapv-shift-2.c
> ===
> --- testsuite/gcc.dg/ubsan/c99-wrapv-shift-2.c(revision 0)
> +++ testsuite/gcc.dg/ubsan/c99-wrapv-shift-2.c(working copy)
> @@ -0,0 +1,9 @@
> +/* { dg-do run } */
> +/* { dg-options "-fsanitize=shift -fwrapv -w -std=c99" } */
> +
> +int
> +main (void)
> +{
> +  int a = 1;
> +  a <<= 31;
> +}
> 

Ping?

Paolo

Re: [PATCH] Do not sanitize left shifts for -fwrapv (PR68418)

[Paolo Bonzini]

> >> gcc:
> >>PR sanitizer/68418
> >>* c-family/c-ubsan.c (ubsan_instrument_shift): Disable
> >>sanitization of left shifts for wrapping signed types as well.
> >>
> >> gcc/testsuite:
> >>PR sanitizer/68418
> >>* gcc.dg/ubsan/c99-wrapv-shift-1.c,
> >>gcc.dg/ubsan/c99-wrapv-shift-2.c: New testcases.
> Doesn't this change how pointer types are handled?

Why would pointer types be shifted at all (at the ubsan level,
which is basically the AST)?

Paolo

Re: [PATCH] Do not sanitize left shifts for -fwrapv (PR68418)

[Paolo Bonzini]

On 04/12/2015 23:48, Jeff Law wrote:
>>
>> Why would pointer types be shifted at all (at the ubsan level,
>> which is basically the AST)?
> BTW, if you argument is that we can never get into this code with a
> shift of a pointer object, I'd like to see some kind of analysis to back
> up that assertion -- which could be as simple as pointing to FE code
> that issues an error if the user tried to do something like shift a
> pointer object.

You're right, I should have qualified that better.  And actually there
is an issue with this patch, though it is not pointers.

There are only two call sites for ubsan_instrument_shift.
In c/c-typeck.c:

  /* Remember whether we're doing << or >>.  */
  bool doing_shift = false;

  /* The expression codes of the data types of the arguments tell us
 whether the arguments are integers, floating, pointers, etc.  */
  code0 = TREE_CODE (type0);
  code1 = TREE_CODE (type1);

  switch (code)
{
...
case RSHIFT_EXPR:
  ...
  else if ((code0 == INTEGER_TYPE || code0 == FIXED_POINT_TYPE)
   && code1 == INTEGER_TYPE)
{
  doing_shift = true;
  ...
}
  ...
case LSHIFT_EXPR:
  ...
  else if ((code0 == INTEGER_TYPE || code0 == FIXED_POINT_TYPE)
   && code1 == INTEGER_TYPE)
{
  doing_shift = true;
  ...
}
  ...
}
  ...
  if ((flag_sanitize & (SANITIZE_SHIFT | SANITIZE_DIVIDE
| SANITIZE_FLOAT_DIVIDE))
  && do_ubsan_in_current_function ()
  && (doing_div_or_mod || doing_shift)
  && !require_constant_value)
{
  /* OP0 and/or OP1 might have side-effects.  */
  op0 = c_save_expr (op0);
  op1 = c_save_expr (op1);
  op0 = c_fully_fold (op0, false, NULL);
  op1 = c_fully_fold (op1, false, NULL);
  ...
  else if (doing_shift && (flag_sanitize & SANITIZE_SHIFT))
instrument_expr = ubsan_instrument_shift (location, code, op0, op1);
}

cp/typeck.c is the same but it doesn't handle code0 == FIXED_POINT_TYPE.

But FIXED_POINT_TYPE is not an integral type, and thus it would fail the
TYPE_OVERFLOW_WRAPS check with my patch.  I'll post an updated patch that
also removes all instrumentation in the case of fixed point types, similar
to instrument_si_overflow.

Thanks for the careful review!

Paolo

[PATCH v2] Do not sanitize left shifts for -fwrapv (PR68418)

[Paolo Bonzini]

Left shifts into the sign bit is a kind of overflow, and the
standard chooses to treat left shifts of negative values the
same way.

However, the -fwrapv option modifies the language to one where
integers are defined as two's complement---which also defines
entirely the behavior of shifts.  Disable sanitization of left
shifts when -fwrapv is in effect, using the same logic as
instrument_si_overflow.  The same change was proposed
for LLVM at https://llvm.org/bugs/show_bug.cgi?id=25552.

Bootstrapped/regtested x86_64-pc-linux-gnu.  Ok for trunk, and for
GCC 5 branch after 5.3 is released?

Thanks,

Paolo

gcc:
PR sanitizer/68418
* c-family/c-ubsan.c (ubsan_instrument_shift): Disable
sanitization of left shifts for wrapping signed types as well.

gcc/testsuite:
PR sanitizer/68418
* gcc.dg/ubsan/c99-wrapv-shift-1.c,
gcc.dg/ubsan/c99-wrapv-shift-2.c: New testcases.

Index: c-family/c-ubsan.c
===
--- c-family/c-ubsan.c  (revision 231289)
+++ c-family/c-ubsan.c  (working copy)
@@ -124,12 +124,17 @@ ubsan_instrument_shift (location_t loc, enum tree_
   t = fold_convert_loc (loc, op1_utype, op1);
   t = fold_build2 (GT_EXPR, boolean_type_node, t, uprecm1);
 
+  /* If this is not a signed operation, don't perform overflow checks.
+ Also punt on bit-fields.  */
+  if (!INTEGRAL_TYPE_P (type0)
+  || TYPE_OVERFLOW_WRAPS (type0)
+  || GET_MODE_BITSIZE (TYPE_MODE (type0)) != TYPE_PRECISION (type0))
+;
+
   /* For signed x << y, in C99/C11, the following:
  (unsigned) x >> (uprecm1 - y)
  if non-zero, is undefined.  */
-  if (code == LSHIFT_EXPR
-  && !TYPE_UNSIGNED (type0)
-  && flag_isoc99)
+  else if (code == LSHIFT_EXPR && flag_isoc99 && cxx_dialect < cxx11)
 {
   tree x = fold_build2 (MINUS_EXPR, op1_utype, uprecm1,
fold_convert (op1_utype, unshare_expr (op1)));
@@ -142,9 +147,7 @@ ubsan_instrument_shift (location_t loc, enum tree_
   /* For signed x << y, in C++11 and later, the following:
  x < 0 || ((unsigned) x >> (uprecm1 - y))
  if > 1, is undefined.  */
-  if (code == LSHIFT_EXPR
-  && !TYPE_UNSIGNED (type0)
-  && (cxx_dialect >= cxx11))
+  else if (code == LSHIFT_EXPR && cxx_dialect >= cxx11)
 {
   tree x = fold_build2 (MINUS_EXPR, op1_utype, uprecm1,
fold_convert (op1_utype, unshare_expr (op1)));
Index: testsuite/gcc.dg/ubsan/c99-wrapv-shift-1.c
===
--- testsuite/gcc.dg/ubsan/c99-wrapv-shift-1.c  (revision 0)
+++ testsuite/gcc.dg/ubsan/c99-wrapv-shift-1.c  (working copy)
@@ -0,0 +1,9 @@
+/* { dg-do run } */
+/* { dg-options "-fsanitize=shift -fwrapv -w -std=c99" } */
+
+int
+main (void)
+{
+  int a = -42;
+  a << 1;
+}
Index: testsuite/gcc.dg/ubsan/c99-wrapv-shift-2.c
===
--- testsuite/gcc.dg/ubsan/c99-wrapv-shift-2.c  (revision 0)
+++ testsuite/gcc.dg/ubsan/c99-wrapv-shift-2.c  (working copy)
@@ -0,0 +1,9 @@
+/* { dg-do run } */
+/* { dg-options "-fsanitize=shift -fwrapv -w -std=c99" } */
+
+int
+main (void)
+{
+  int a = 1;
+  a <<= 31;
+}

[PATCH] Improve dead code elimination with -fsanitize=address (PR84307)

[Paolo Bonzini]

Hi all,

in this PR, a dead reference to a function pointer cannot be optimized
out by the compiler because some ASAN_MARK UNPOISON calls, which are
placed before the store, cause the containing struct to escape.
(Without -fsanitize=address, the dead code is eliminated by the first
DSE pass).

The fix, which works at least for this testcase, is to copy part of the
sanopt dead code elimination pass early, so that the compiler can see
fewer UNPOISON calls.  I am not sure this is general enough, due to
the very limited data flow analysis done by sanitize_asan_mark_unpoison.
Another possibility which I considered but did not implement is to mark
the UNPOISON calls so that they do not cause the parameter to escape.

Any suggestions on how to proceed here (or approval is welcome too :))?

Paolo

2018-02-09  Paolo Bonzini  

* passes.def: add pass_sandce before first DSE pass.
* sanopt.c (pass_data_sandce, pass_sandce, make_pass_sandce): New.
* tree-pass.h (make_pass_sandce): Declare it.

testsuite:
2018-02-09  Paolo Bonzini  

PR sanitizer/84307
* gcc.dg/asan/pr84307.c: New test.

diff --git a/gcc/passes.def b/gcc/passes.def
index 9802f08..180df50 100644
--- a/gcc/passes.def
+++ b/gcc/passes.def
@@ -244,6 +244,7 @@ along with GCC; see the file COPYING3.  If not see
 only examines PHIs to discover const/copy propagation
 opportunities.  */
   NEXT_PASS (pass_phi_only_cprop);
+  NEXT_PASS (pass_sandce);
   NEXT_PASS (pass_dse);
   NEXT_PASS (pass_reassoc, true /* insert_powi_p */);
   NEXT_PASS (pass_dce);
diff --git a/gcc/sanopt.c b/gcc/sanopt.c
index cd94638..23b8e6e 100644
--- a/gcc/sanopt.c
+++ b/gcc/sanopt.c
@@ -906,6 +906,32 @@ sanopt_optimize (function *fun, bool *contains_asan_mark)
 
 namespace {
 
+const pass_data pass_data_sandce =
+{
+  GIMPLE_PASS, /* type */
+  "sandce", /* name */
+  OPTGROUP_NONE, /* optinfo_flags */
+  TV_NONE, /* tv_id */
+  ( PROP_ssa | PROP_cfg | PROP_gimple_leh ), /* properties_required */
+  0, /* properties_provided */
+  0, /* properties_destroyed */
+  0, /* todo_flags_start */
+  TODO_update_ssa, /* todo_flags_finish */
+};
+
+class pass_sandce : public gimple_opt_pass
+{
+public:
+  pass_sandce (gcc::context *ctxt)
+: gimple_opt_pass (pass_data_sandce, ctxt)
+  {}
+
+  /* opt_pass methods: */
+  virtual bool gate (function *) { return flag_sanitize & SANITIZE_ADDRESS; }
+  virtual unsigned int execute (function *);
+
+}; // class pass_sanopt
+
 const pass_data pass_data_sanopt =
 {
   GIMPLE_PASS, /* type */
@@ -1244,6 +1270,31 @@ sanitize_rewrite_addressable_params (function *fun)
 }
 
 unsigned int
+pass_sandce::execute (function *fun)
+{
+  basic_block bb;
+  bool contains_asan_mark = false;
+
+  /* Try to remove redundant unpoisoning.  */
+  gimple_stmt_iterator gsi;
+  FOR_EACH_BB_FN (bb, fun)
+for (gsi = gsi_start_bb (bb); !gsi_end_p (gsi); gsi_next (&gsi))
+  {
+   gimple *stmt = gsi_stmt (gsi);
+   if (gimple_call_internal_p (stmt, IFN_ASAN_MARK))
+ {
+   contains_asan_mark = true;
+   break;
+ }
+  }
+
+  if (contains_asan_mark)
+sanitize_asan_mark_unpoison ();
+
+  return 0;
+}
+
+unsigned int
 pass_sanopt::execute (function *fun)
 {
   basic_block bb;
@@ -1367,6 +1418,12 @@ pass_sanopt::execute (function *fun)
 } // anon namespace
 
 gimple_opt_pass *
+make_pass_sandce (gcc::context *ctxt)
+{
+  return new pass_sandce (ctxt);
+}
+
+gimple_opt_pass *
 make_pass_sanopt (gcc::context *ctxt)
 {
   return new pass_sanopt (ctxt);
diff --git a/gcc/tree-pass.h b/gcc/tree-pass.h
index 93a6a99..d5eb055 100644
--- a/gcc/tree-pass.h
+++ b/gcc/tree-pass.h
@@ -350,6 +350,7 @@ extern gimple_opt_pass *make_pass_tsan (gcc::context *ctxt);
 extern gimple_opt_pass *make_pass_tsan_O0 (gcc::context *ctxt);
 extern gimple_opt_pass *make_pass_sancov (gcc::context *ctxt);
 extern gimple_opt_pass *make_pass_sancov_O0 (gcc::context *ctxt);
+extern gimple_opt_pass *make_pass_sandce (gcc::context *ctxt);
 extern gimple_opt_pass *make_pass_lower_cf (gcc::context *ctxt);
 extern gimple_opt_pass *make_pass_refactor_eh (gcc::context *ctxt);
 extern gimple_opt_pass *make_pass_lower_eh (gcc::context *ctxt);
diff --git a/gcc/testsuite/gcc.dg/asan/pr84307.c 
b/gcc/testsuite/gcc.dg/asan/pr84307.c
new file mode 100644
index 000..6e1a197
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/asan/pr84307.c
@@ -0,0 +1,21 @@
+/* PR middle-end/83185 */
+/* { dg-do link } */
+/* { dg-options "-O1" } */
+
+struct f {
+  void (*func)(void);
+};
+
+extern void link_error(void);
+extern int printf(const char *f, ...);
+
+static inline struct f *gimme_null(struct f *result)
+{
+  return 0;
+}
+
+int main(int argc, char **argv)
+{
+  struct f *x = gimme_null(&(struct f) { .func = link_error });
+  printf("%p", x);
+}

Re: [PATCH] Improve dead code elimination with -fsanitize=address (PR84307)

[Paolo Bonzini]

On 09/02/2018 17:40, Richard Biener wrote:
> On February 9, 2018 5:08:24 PM GMT+01:00, Paolo Bonzini  
> wrote:
>> Another possibility which I considered but did not implement is to mark
>> the UNPOISON calls so that they do not cause the parameter to escape.
> 
> I'd do this, thus assign proper fnspec attributes to the asan functions. 

Hmm, actually that might be as simple as fixing a typo:

diff --git a/gcc/internal-fn.def b/gcc/internal-fn.def
index 5970d0e..15d6151 100644
--- a/gcc/internal-fn.def
+++ b/gcc/internal-fn.def
@@ -255,8 +255,8 @@ DEF_INTERNAL_FN (UBSAN_PTR, ECF_LEAF | ECF_NOTHROW, ".R.")
 DEF_INTERNAL_FN (UBSAN_OBJECT_SIZE, ECF_LEAF | ECF_NOTHROW, NULL)
 DEF_INTERNAL_FN (ABNORMAL_DISPATCHER, ECF_NORETURN, NULL)
 DEF_INTERNAL_FN (BUILTIN_EXPECT, ECF_CONST | ECF_LEAF | ECF_NOTHROW, NULL)
-DEF_INTERNAL_FN (ASAN_CHECK, ECF_TM_PURE | ECF_LEAF | ECF_NOTHROW, ".R...")
-DEF_INTERNAL_FN (ASAN_MARK, ECF_LEAF | ECF_NOTHROW, ".R..")
+DEF_INTERNAL_FN (ASAN_CHECK, ECF_TM_PURE | ECF_LEAF | ECF_NOTHROW, "..R..")
+DEF_INTERNAL_FN (ASAN_MARK, ECF_LEAF | ECF_NOTHROW, "..W.")
 DEF_INTERNAL_FN (ASAN_POISON, ECF_LEAF | ECF_NOTHROW | ECF_NOVOPS, NULL)
 DEF_INTERNAL_FN (ASAN_POISON_USE, ECF_LEAF | ECF_NOTHROW | ECF_NOVOPS, NULL)
 DEF_INTERNAL_FN (ADD_OVERFLOW, ECF_CONST | ECF_LEAF | ECF_NOTHROW, NULL)

which indeed fixes the testcase and seems not to break asan.exp.
'W' is needed to avoid breaking the pr78541.c testcase, and I think it
makes sense since ASAN_MARK is "writing" the state of the object
(in the test case FRE moves a dereference across a poisoning).

I'll look at it next week.  Someone maybe should take a look at ubsan
fnspecs too.

Paolo

Re: [PATCH] Improve dead code elimination with -fsanitize=address (PR84307)

[Paolo Bonzini]

On 12/02/2018 09:56, Richard Biener wrote:
>>> I think it does, for both ASAN_CHECK and ASAN_MARK the pointer argument
>>> is the second one, the first one is an integer argument with flags.
>>> And ASAN_MARK, both poison and unpoison, works kind like a clobber on
>>> the
>>> referenced variable, before unpoison it is generally inaccessible and
>>> after
>>> poison too.
>> Ah, indeed.
> Which was an approval as well, in case you want to push this right now.

Oh cool.  I was going to look at ubsan builtins too, I'll post that
separately.  Ok for GCC 7 too?

Thanks,

Paolo

Re: [PATCH] Improve dead code elimination with -fsanitize=address (PR84307)

[Paolo Bonzini]

On 09/02/2018 19:07, Jakub Jelinek wrote:
> On Fri, Feb 09, 2018 at 07:01:08PM +0100, Richard Biener wrote:
>>> which indeed fixes the testcase and seems not to break asan.exp.
>>
>> Huh. Need to double check why that makes sense ;) 
> 
> I think it does, for both ASAN_CHECK and ASAN_MARK the pointer argument
> is the second one, the first one is an integer argument with flags.
> And ASAN_MARK, both poison and unpoison, works kind like a clobber on the
> referenced variable, before unpoison it is generally inaccessible and after
> poison too.

This was too optimistic. :(

In use-after-scope-types-1.C, after the patch FRE+DSE are able to
optimize away the problematic read.  In general it seems to me that the
sanitizer passes should be before DSE if we want ASAN builtins to have
precise info, otherwise some reads or stores might not be
instrumented---GCC was being lucky here.

The obvious change here is:

Index: passes.def
===
--- passes.def  (revision 257584)
+++ passes.def  (working copy)
@@ -95,6 +95,9 @@
  NEXT_PASS (pass_fre);
  NEXT_PASS (pass_early_vrp);
  NEXT_PASS (pass_merge_phi);
+ NEXT_PASS (pass_sancov);
+ NEXT_PASS (pass_asan);
+ NEXT_PASS (pass_tsan);
   NEXT_PASS (pass_dse);
  NEXT_PASS (pass_cd_dce);
  NEXT_PASS (pass_early_ipa_sra);
@@ -259,9 +262,6 @@
   NEXT_PASS (pass_walloca, false);
   NEXT_PASS (pass_pre);
   NEXT_PASS (pass_sink_code);
-  NEXT_PASS (pass_sancov);
-  NEXT_PASS (pass_asan);
-  NEXT_PASS (pass_tsan);
   NEXT_PASS (pass_dce);
   /* Pass group that runs when 1) enabled, 2) there are loops
 in the function.  Make sure to run pass_fix_loops before

which seems to work (this time for real... not sure what went wrong in
my previous testing) but it's a pretty large change that I'd like to run
by you guys before posting it.

Paolo

Re: Fallout: PR84340

[Paolo Bonzini]

On 13/02/2018 10:32, Martin Liška wrote:
> Hello.
> 
> It caused PR84340, I'm suggesting following fix:
> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84340#c3

I don't think EAF_DIRECT is the issue.  You could think of ASAN_MARK as
writing a global variable, which it can do because it's not const.

The issue is that the ASAN_CHECK doesn't exist at early DSE time, and
thus causes the store to disappear.

Paolo

Re: [PATCH] Improve dead code elimination with -fsanitize=address (PR84307)

[Paolo Bonzini]

On 13/02/2018 00:49, Jakub Jelinek wrote:
> On Tue, Feb 13, 2018 at 12:41:28AM +0100, Paolo Bonzini wrote:
>> On 09/02/2018 19:07, Jakub Jelinek wrote:
>>> On Fri, Feb 09, 2018 at 07:01:08PM +0100, Richard Biener wrote:
>>>>> which indeed fixes the testcase and seems not to break asan.exp.
>>>>
>>>> Huh. Need to double check why that makes sense ;) 
>>>
>>> I think it does, for both ASAN_CHECK and ASAN_MARK the pointer argument
>>> is the second one, the first one is an integer argument with flags.
>>> And ASAN_MARK, both poison and unpoison, works kind like a clobber on the
>>> referenced variable, before unpoison it is generally inaccessible and after
>>> poison too.
>>
>> This was too optimistic. :(
>>
>> In use-after-scope-types-1.C, after the patch FRE+DSE are able to
>> optimize away the problematic read.  In general it seems to me that the
>> sanitizer passes should be before DSE if we want ASAN builtins to have
>> precise info, otherwise some reads or stores might not be
>> instrumented---GCC was being lucky here.
>>
>> The obvious change here is:
> 
> That is way too early, I don't think this is a good idea.
> Certainly not in stage4.

Certainly, for now I'll revert.

But can you expand on why it's too early?  Indeed I suppose it may
affect inlining decisions, on the other hand it seems dangerous to apply
instrumentation after pretty much any optimization pass.

Thanks,

Paolo

Re: [PATCH] Improve dead code elimination with -fsanitize=address (PR84307)

[Paolo Bonzini]

On 13/02/2018 14:00, Jakub Jelinek wrote:
>> Certainly, for now I'll revert.
> Reversion is not the right thing, the "fn spec" attributes were clearly
> incorrect. So, we should change them to something more conservative that
> will work.

That would only be "all dots", that is no fnspec at all.  Martin
suggested removing EAF_DIRECT, but I don't think I agree with his
reasoning.  Besides, aliasing doesn't see the shadow memory at all (see
call_may_clobber_ref_p_1), so it's okay to ignore it for the sake of
fnspecs.

>> But can you expand on why it's too early?  Indeed I suppose it may
>> affect inlining decisions, on the other hand it seems dangerous to apply
>> instrumentation after pretty much any optimization pass.
>
> It will prevent pretty much all optimizations.  We don't want -O2
> -fsanitize=address to be unusably slow, if people want to catch everything,
> they can always use -O0 -fsanitize=address.  The current placement of the
> passes has been a result of long discussions if I remember well.

I'm not sure it will be that bad, together with the fnspec.  Consider
that PR84340 is latent in current GCC; the testcases work because GCC
thinks that the &x pointer escaped, and thus treated the stores as not
dead.  In other words, -fsanitize=address -O2 _currently_ lacks an awful
lot of aliasing-based optimizations such as DSE, because all variables
are marked as escaping after the initial ASAN_MARK(UNPOISON, &var, sz).

With some luck (that we can ascertain between now and stage 1) the
negative effects of pass placement balance with the positive effects of
the fnspec.  But I agree that it requires some discussion and benchmarking.

Paolo

Re: Fallout: PR84340

[Paolo Bonzini]

On 13/02/2018 14:35, Jakub Jelinek wrote:
> On Tue, Feb 13, 2018 at 12:21:55PM +0100, Jakub Jelinek wrote:
>> On Tue, Feb 13, 2018 at 12:15:36PM +0100, Paolo Bonzini wrote:
>>> The issue is that the ASAN_CHECK doesn't exist at early DSE time, and
>>> thus causes the store to disappear.
>>
>> If it was DSE removing the stores before asan pass, then it would FAIL
>> before as well.
> 
> Sorry, while ASAN_CHECK is introduced late, ASAN_MARK is present there
> already from the gimplification.

Yeah, and it keeps everything alive.

> For use after scope, I guess a lot of the stores after end of scope
> (rather than reads) are something DSE could consider removing.
> So, shall we just disable DSE on vars where their address "escapes"
> through ASAN_MARK when -fsanitize-address-use-after-scope?

But the stores _are_ dead; it's only the ASAN_CHECK that isn't.  Hence
the idea of doing the entire instrumentation very early.

> Generally, dead stores could be eliminable when stored before the
> corresponding ASAN_MARK poison (but even ASAN_MARK with "..W.." will
> prevent those) and uneliminable when stored after ASAN_MARK poison.
> 
> For the "fn spec" for now, I'd just go with "..R.." for ASAN_CHECK and
> NULL for ASAN_MARK for now.

I'm a bit scared of that even, :) especially in stage4.  If you think
it's safe enough, I can give it a shot, but honestly I wouldn't have
much time to deal with the fallout now (hence the quick revert).

Paolo

Re: [PATCH] make has_gate and has_execute useless

[Paolo Bonzini]

Il 08/11/2013 10:37, Richard Biener ha scritto:
>   /* If we have a gate, combine the properties that we could have with
>  and without the pass being examined.  */
>   if (pass->has_gate)
> properties &= new_properties;
>   else
> properties = new_properties;
> 
> which I don't understand (and you just removed all properties handling there).

The properties argument to register_dump_files_1 is indeed dead code; it
is never used except to compute new_properties which is also dead
(because it is simply the argument in a recursive call).

I seem to recall it was needed back when pass->type didn't exist; it
used PROP_rtl to find whether a pass was tree or gimple, or something
like that.  The above test and "&=" were needed for this to work at all
optimization levels, but I don't recall the details and as said above
it's all dead anyway.

The argument to register_dump_files is used.  To remove it, one could
change all_*_passes from a pointer to a "dummy" pass, and set
properties_required in the initializer for the dummy pass.

Paolo

Re: Clean up configure glibc version detection, add --with-glibc-version

[Paolo Bonzini]

Il 06/11/2013 19:33, Joseph S. Myers ha scritto:
> +dnl GCC_GLIBC_VERSION_GTE_IFELSE(MAJOR, MINOR, IF-TRUE, IF-FALSE)
> +dnl -
> +dnl If the target glibc version ($glibc_version_major.$glibc_version_minor)
> +dnl is at least MAJOR.MINOR, call IF-TRUE, otherwise call IF-FALSE.
> +AC_DEFUN([GCC_GLIBC_VERSION_GTE_IFELSE],
> +[
> +if test $glibc_version_major -gt $1 \
> +  || ( test $glibc_version_major -eq $1 && test $glibc_version_minor -ge $2 
> );
> +then
> +  :
> +  $3
> +else
> +  :
> +  $4
> +fi])

Ok, but please use AS_IF here.

Paolo

Re: [libcilkrts, build] Only use visibility if supported

[Paolo Bonzini]

Il 14/11/2013 15:08, Rainer Orth ha scritto:
> libcilkrts.so fails to link on Solaris 9/x86 with Sun as since this
> configuration lacks visibility support:
> 
> Text relocation remains referenced
> against symbol  offset  in file
> __cilkrts_get_tls_worker0x41.libs/cilk-abi.o
> __cilkrts_get_tls_worker0x254   .libs/cilk-abi.o
> __cilkrts_get_tls_worker0x5c9   .libs/cilk-abi.o
> __cilkrts_get_tls_worker0x609   .libs/cilk-abi.o
> __cilkrts_get_tls_worker0x6e7   .libs/cilk-abi.o
> __cilkrts_get_tls_worker0x774   .libs/cilk-abi.o
> __cilkrts_get_tls_worker0x271   .libs/cilk-abi-cilk-for.o
> __cilkrts_get_tls_worker0x361   .libs/cilk-abi-cilk-for.o
> __cilkrts_get_tls_worker0x10.libs/cilk_api.o
> __cilkrts_get_tls_worker0xc1.libs/cilk_api.o
> __cilkrts_get_tls_worker0x194   .libs/cilk_api.o
> __cilkrts_get_tls_worker0x241   .libs/cilk_api.o
> __cilkrts_get_tls_worker0x324   .libs/cilk_api.o
> __cilkrts_get_tls_worker0x706   .libs/reducer_impl.o
> __cilkrts_get_tls_worker0x893   .libs/reducer_impl.o
> [...]
> __cilkrts_bump_worker_rank_internal 0x32c   .libs/cilk_api.o
> __cilkrts_init_worker_sysdep0x262c  .libs/scheduler.o
> ld: fatal: relocations remain against allocatable but non-writable sections
> collect2: error: ld returned 1 exit status
> make[2]: *** [libcilkrts.la] Error 1
> 
> During the build, one sees many warnings like this:
> 
> /vol/gcc/src/hg/trunk/local/libcilkrts/runtime/cilk-abi.c: In function 
> '__cilkrt
> s_enter_frame_fast':
> /vol/gcc/src/hg/trunk/local/libcilkrts/runtime/cilk-abi.c:144:1: warning: 
> visibi
> lity attribute not supported in this configuration; ignored [-Wattributes]
> 
> This can be cured by actually checking if a given configuration supports
> __attribute((visibility)) instead of just assuming gcc on Unix systems
> does.  The following patch does just that.
> 
> Tested by reconfiguring and rebuilding libcilkrts on i386-pc-solaris2.9
> (which now builds) and i386-pc-solaris2.11 (where elfdump -s still show
> many symbols as protected as they should be).
> 
> Ok for mainline?
> 
>   Rainer
> 
> 
> 2013-11-14  Rainer Orth  
> 
>   * configure.ac (libcilkrts_cv_have_attribute_visibility): Check
>   for __attribute__((visibility)).
>   * configure: Regenerate.
>   * include/cilk/common.h (CILK_EXPORT, CILK_EXPORT_DATA): Only use
>   __attribute__((visibility)) if HAVE_ATTRIBUTE_VISIBILITY.
> 
> 
> 
> 

Any chance to move the test to config/?

Otherwise ok.

Paolo

Re: [build] Enable libcilkrts multilib build on Solaris

[Paolo Bonzini]

Il 13/11/2013 15:06, Rainer Orth ha scritto:
> 
> This happens because there's no installed amd64 libgcc_s.so.1 on the
> system, and toplevel Makefile only sets LD_LIBRARY_PATH for the default
> multilib.  Initially, I thought that there were something special going
> on, but it turned out that other runtime libs containing C++ code don't
> have this problem, which can easily be cured by the following patch.  It
> allowed a C++-only i386-pc-solaris2.10 bootstrap to finish, make check
> currently running.  I'm currently also running an
> x86_64-unknown-linux-gnu bootstrap to make sure nothing breaks there.
> 
> Ok for mainline if those pass?

Yes.

> Btw., I noticed a couple of other anomalies:
> 
> * configure.ac has
> 
>   GCC_LIBSTDCXX_RAW_CXX_FLAGS
> 
>   but does nothing with the result: Makefile.in substitutes the results,
>   but that's it.  Also, toplevel Makefile.tpl should set raw_cxx=true it
>   this were useful, which it doesn't do as well.

Yes, looks like cut-and-paste.

Paolo

> * MAINTAINERS doesn't list a maintainer for libcilkrts.
> 
> * I believe there should be a libcilkrts bugzilla category, intead of
>   having to use other.

Re: Enale -fno-fat-lto-objects by default

[Paolo Bonzini]

Il 18/11/2013 20:09, Jan Hubicka ha scritto:
>>> > > this patch switches the default for fat-lto-objects as was documented 
>>> > > for a while.
>>> > > -ffat-lto-objects doubles compilation time and often makes users to not 
>>> > > notice that
>>> > > LTO was not used at all (because they forgot to use gcc-ar/gcc-nm 
>>> > > plugins).
>>> > > 
>>> > > Sadly I had to add -ffat-lto-objects to bootstrap. This is because I do 
>>> > > not know
>>> > > how to convince our build machinery to use gcc-ar/gcc-nm during the 
>>> > > stage2+
>> > 
>> > I've posted a minimal patch set for slim-lto-bootstrap last year, see:
>> > http://thread.gmane.org/gmane.comp.gcc.patches/270842
>> > 
>> > If there's interest I could repost it.
> It would be really nice to have it in indeed.  I think we do not really need
> lto-bootstrap.mk and slim-lto-bootstrap.mk, but otherwise the patch seems easy
> enough and would save quite some of lto bootstrap testing time...

Patches 1 and 2 should go upstream first.

Patch 3 in the series is wrong because Makefile.in is a generated file.
 The message does not explain why it is necessary, and it is probably
working around a bug elsewhere.

For patch 4, I agree with Jan that we do not need a separate configuration.

Paolo

Re: Enale -fno-fat-lto-objects by default

[Paolo Bonzini]

Il 19/11/2013 11:05, Markus Trippelsdorf ha scritto:
> On 2013.11.19 at 09:44 +0100, Paolo Bonzini wrote:
>> Il 18/11/2013 20:09, Jan Hubicka ha scritto:
>>>>>>> this patch switches the default for fat-lto-objects as was documented 
>>>>>>> for a while.
>>>>>>> -ffat-lto-objects doubles compilation time and often makes users to not 
>>>>>>> notice that
>>>>>>> LTO was not used at all (because they forgot to use gcc-ar/gcc-nm 
>>>>>>> plugins).
>>>>>>>
>>>>>>> Sadly I had to add -ffat-lto-objects to bootstrap. This is because I do 
>>>>>>> not know
>>>>>>> how to convince our build machinery to use gcc-ar/gcc-nm during the 
>>>>>>> stage2+
>>>>>
>>>>> I've posted a minimal patch set for slim-lto-bootstrap last year, see:
>>>>> http://thread.gmane.org/gmane.comp.gcc.patches/270842
>>>>>
>>>>> If there's interest I could repost it.
>>> It would be really nice to have it in indeed.  I think we do not really need
>>> lto-bootstrap.mk and slim-lto-bootstrap.mk, but otherwise the patch seems 
>>> easy
>>> enough and would save quite some of lto bootstrap testing time...
>>
>> Patches 1 and 2 should go upstream first.
> 
> OK, but where is upstream?
> Please note that a general libtool update would fix this issue, too.

Ah, so they're already upstream.

> So, maybe it is just time to upgrade libtool everywhere in gnu-land?

Yes, that would be better but no need to do that now.

>> Patch 3 in the series is wrong because Makefile.in is a generated file.
>>  The message does not explain why it is necessary, and it is probably
>> working around a bug elsewhere.
>> For patch 4, I agree with Jan that we do not need a separate configuration.
> 
> The problem is that fixincl links with libiberty.a:
> 
> /var/tmp/gcc_build_dir/./gcc/xgcc -B/var/tmp/gcc_build_dir/./gcc/
> -B/usr/x86_64-pc-linux-gnu/bin/ -B/usr/x86_64-pc-linux-gnu/lib/ -isystem
> /usr/x86_64-pc-linux-gnu/include -isystem
> /usr/x86_64-pc-linux-gnu/sys-include-O2 -pipe -static-libstdc++
> -static-libgcc  -o fixincl fixincl.o fixtests.o fixfixes.o server.o
> procopen.o fixlib.o fixopts.o ../libiberty/libiberty.a
> 
> And this archive consists of object files with LTO sections only. So we
> need to find a way to pass -fuse-linker-plugin to the invocation above.

Then -fuse-linker-plugin should be added to the LDFLAGS (not CFLAGS) for
all host modules, as in "LDFLAGS += -fuse-linker-plugin".  Other host
modules than fixincludes could also use libiberty or another
bootstrapped host library (libbfd is one, I think), and would require
the same fix.

Paolo

Re: Enale -fno-fat-lto-objects by default

[Paolo Bonzini]

Il 19/11/2013 12:19, Markus Trippelsdorf ha scritto:
>> > 
>>> > > So, maybe it is just time to upgrade libtool everywhere in gnu-land?
>> > 
>> > Yes, that would be better but no need to do that now.
> So would Patches 1 and 2 be OK in the interim?

Yes.  And Jan's answer suggests that patch 3 is not necessary at all now.

Paolo

Re: Add slim-lto support to gcc's build machinery

[Paolo Bonzini]

Il 20/11/2013 08:23, Markus Trippelsdorf ha scritto:
> Hi,
> 
> now that slim-lto objects are enabled by default, it would be nice to
> use them when building gcc with bootstrap-lto. The following patch
> implements the handling of these objects in gcc's build machinery.
> (Once -fuse-linker-plugin is made the default, -ffat-lto-objects in
> config/bootstrap-lto.mk could be dropped.)
> 
> LTO-Bootstrapped on x86_64-linux (with default -fuse-linker-plugin).
> The patch was already approved by Paolo Bonzini.
> 
> Please apply, because I don't have access.

Note that you need to regenerate all users of libtool.m4.  Please post a
patch _with_ the regeneration so that whoever applies it won't screw up.

Paolo

> Thanks.
> 
> 2013-11-20  Markus Trippelsdorf  
> 
>   * libtool.m4 : Handle slim-lto objects.
>   * ltmain.sh: Handle lto options.
> 
> diff --git a/libtool.m4 b/libtool.m4
> index 797468f02a5a..c55b6eba7a94 100644
> --- a/libtool.m4
> +++ b/libtool.m4
> @@ -3440,6 +3440,7 @@ for ac_symprfx in "" "_"; do
>else
>  lt_cv_sys_global_symbol_pipe="sed -n -e 's/^.*[[  
> ]]\($symcode$symcode*\)[[   ]][[
> ]]*$ac_symprfx$sympat$opt_cr$/$symxfrm/p'"
>fi
> +  lt_cv_sys_global_symbol_pipe="$lt_cv_sys_global_symbol_pipe | sed '/ 
> __gnu_lto/d'"
>  
># Check to see that the pipe works correctly.
>pipe_works=no
> @@ -4459,7 +4460,7 @@ _LT_EOF
>if $LD --help 2>&1 | $EGREP ': supported targets:.* elf' > /dev/null \
>&& test "$tmp_diet" = no
>then
> - tmp_addflag=
> + tmp_addflag=' $pic_flag'
>   tmp_sharedflag='-shared'
>   case $cc_basename,$host_cpu in
>  pgcc*)   # Portland Group C compiler
> @@ -5525,8 +5526,8 @@ if test "$_lt_caught_CXX_error" != yes; then
># Check if GNU C++ uses GNU ld as the underlying linker, since the
># archiving commands below assume that GNU ld is being used.
>if test "$with_gnu_ld" = yes; then
> -_LT_TAGVAR(archive_cmds, $1)='$CC -shared -nostdlib $predep_objects 
> $libobjs $deplibs $postdep_objects $compiler_flags ${wl}-soname $wl$soname -o 
> $lib'
> -_LT_TAGVAR(archive_expsym_cmds, $1)='$CC -shared -nostdlib 
> $predep_objects $libobjs $deplibs $postdep_objects $compiler_flags 
> ${wl}-soname $wl$soname ${wl}-retain-symbols-file $wl$export_symbols -o $lib'
> +_LT_TAGVAR(archive_cmds, $1)='$CC $pic_flag -shared -nostdlib 
> $predep_objects $libobjs $deplibs $postdep_objects $compiler_flags 
> ${wl}-soname $wl$soname -o $lib'
> +_LT_TAGVAR(archive_expsym_cmds, $1)='$CC $pic_flag -shared -nostdlib 
> $predep_objects $libobjs $deplibs $postdep_objects $compiler_flags 
> ${wl}-soname $wl$soname ${wl}-retain-symbols-file $wl$export_symbols -o $lib'
>  
>  _LT_TAGVAR(hardcode_libdir_flag_spec, $1)='${wl}-rpath ${wl}$libdir'
>  _LT_TAGVAR(export_dynamic_flag_spec, $1)='${wl}--export-dynamic'
> @@ -6503,6 +6504,13 @@ public class foo {
>  };
>  _LT_EOF
>  ])
> +
> +_lt_libdeps_save_CFLAGS=$CFLAGS
> +case "$CC $CFLAGS " in #(
> +*\ -flto*\ *) CFLAGS="$CFLAGS -fno-lto" ;;
> +*\ -fwhopr*\ *) CFLAGS="$CFLAGS -fno-whopr" ;;
> +esac
> +
>  dnl Parse the compiler output and extract the necessary
>  dnl objects, libraries and library flags.
>  if AC_TRY_EVAL(ac_compile); then
> @@ -6551,6 +6559,7 @@ if AC_TRY_EVAL(ac_compile); then
> fi
> ;;
>  
> +*.lto.$objext) ;; # Ignore GCC LTO objects
>  *.$objext)
> # This assumes that the test object file only shows up
> # once in the compiler output.
> @@ -6586,6 +6595,7 @@ else
>  fi
>  
>  $RM -f confest.$objext
> +CFLAGS=$_lt_libdeps_save_CFLAGS
>  
>  # PORTME: override above test on systems where it is broken
>  m4_if([$1], [CXX],
> diff --git a/ltmain.sh b/ltmain.sh
> index a03433f17894..2e0910194f24 100644
> --- a/ltmain.sh
> +++ b/ltmain.sh
> @@ -4980,7 +4980,8 @@ func_mode_link ()
># @file GCC response files
># -tp=* Portland pgcc target processor selection
>-64|-mips[0-9]|-r[0-9][0-9]*|-xarch=*|-xtarget=*|+DA*|+DD*|-q*|-m*| \
> -  -t[45]*|-txscale*|-p|-pg|--coverage|-fprofile-*|-F*|@*|-tp=*)
> +  -t[45]*|-txscale*|-p|-pg|--coverage|-fprofile-*|-F*|@*|-tp=*| \
> +  -O*|-flto*|-fwhopr|-fuse-linker-plugin)
>  func_quote_for_eval "$arg"
>   arg="$func_quote_for_eval_result"
>  func_append compile_command " $arg"
> 

Re: [build] Enable libcilkrts multilib build on Solaris

[Paolo Bonzini]

Il 22/11/2013 13:14, Rainer Orth ha scritto:
> I'm including the following in this weekend's bootstraps (Solaris and
> Linux).
> 
> 2013-11-22  Rainer Orth  
> 
>   * configure.ac (GCC_LIBSTDCXX_RAW_CXX_FLAGS): Remove.
>   * configure: Regenerate.
> 

Ok if it passes.

Paolo

Re: [libgcc, build] Suppress some warnings for soft-fp files

[Paolo Bonzini]

Il 25/11/2013 16:45, Rainer Orth ha scritto:
> Uros prompted me to look into why we were still getting warnings
> compiling the soft-fp code in libgcc despite this in config/t-softfp:
> 
> $(soft-fp-objects) : INTERNAL_CFLAGS += -Wno-missing-prototypes 
> -Wno-type-limit
> s
> 
> It turned out that soft-fp-objects still included the $srcdir prefix.
> It seems my libgcc migration missed some chunk of code to strip that,
> although I still see the warnings on the 4.6 branch (i.e. before the
> migration).
> 
> Anyway, the following patch fixes this.  Bootstrapped without
> regressions on i386-pc-solaris2.11 and x86_64-unknown-linux-gnu and
> verified the warnings are indeed gone.
> 
> Ok for mainline?

Ok.

Paolo

Re: libffi patch RFA: Pass -Qunused-arguments for asm files

[Paolo Bonzini]

Il 30/09/2014 02:12, Ian Lance Taylor ha scritto:
> Similar to a recent patch to libgo, this patch to the libffi configure
> script checks whether the compiler support -Qunused-arguments.  If it
> does, it passes -Qunused-arguments when invoking the compiler on .s
> files.  This is because the clang driver complains by default when given
> useless arguments, such as -I options when compiling a .s file.  This
> somewhat annoying behaviour works poorly with configure scripts.  The
> -Qunused-arguments option disables it.  Bootstrapped and ran libffi and
> libgo tests on x86_64-unknown-linux-gnu.
> 
> OK for mainline?
> 
> Ian
> 
> 
> 2014-09-29  Ian Lance Taylor  
> 
>   * configure.ac: If the compiler supports -Qunused-arguments, use
>   it when running the compiler on .s files.
>   * configure: Regenerated.

Ok.

Paolo

Re: [build, driver] RFC: Support compressed debug sections

[Paolo Bonzini]

Il 26/06/2014 15:16, Rainer Orth ha scritto:

Hi Gerald,

sorry for the delay, I've been away for a couple of days.


On Tue, 3 Jun 2014, Rainer Orth wrote:

It's been another week, and I still need approval for the build, doc,
and Darwin changes:

https://gcc.gnu.org/ml/gcc-patches/2014-05/msg01860.html


On the doc side, things are fine.

Just a suggestion or two:

+Produce compressed debug sections in DWARF format, if that is supported.

Supported by what?


By the toolchain used.  TBH, I just copied that fragment from various
other debug options (-gstabs, -gcoff, -gdwarf-N).  Given the precedent
and the verbosity of a more detailed explanation, I'd leave this as is.


"doesn't" -> "does not", especially given the emphasis we want to make
here.


Good point, fixed.


And could the "If the linker doesn't support writing compressed debug
sections, the option is rejected.  Otherwise, if the assembler doesn't
support them, @option{-gz} is silently ignored when producing object
files." be moved to the very end, or is this only applicable to the
case where no type has been specified?


No, you're right: it's better to first explain the values for type in
the working case, then explain potential error scenarios.

The section now reads

@item -gz@r{[}=@var{type}@r{]}
@opindex gz
Produce compressed debug sections in DWARF format, if that is supported.
If @var{type} is not given, the default type depends on the capabilities
of the assembler and linker used.  @var{type} may be one of
@option{none} (don't compress debug sections), @option{zlib} (use zlib
compression in ELF gABI format), or @option{zlib-gnu} (use zlib
compression in traditional GNU format).  If the linker doesn't support
writing compressed debug sections, the option is rejected.  Otherwise,
if the assembler does not support them, @option{-gz} is silently ignored
when producing object files.

Thanks for your comments.

I'm still missing review of the build parts after three weeks and
several reminders, though.  Paolo, Nathanael, Alexandre, could one of
you please have a look?


Build changes are good, thanks.

Paolo

Re: [PATCH] top-level for libvtv: use normal (not raw_cxx) target exports

[Paolo Bonzini]

On 09/06/2015 16:22, Michael Haubenwallner wrote:
> Hi build machinery maintainers,
> 
> since we always build the C++ compiler now, I fail to see the need to still
> use RAW_CXX_TARGET_EXPORTS for libvtv.
> 
> The situation to expose the problem is:
> * Use a multilib-enabled x86_64-linux box.
> * Use a 64-bit (multilib-disabled) bootstrap compiler (binary image).
> $ configure --enable-multilib --with-system-zlib
> $ make bootstrap
> 
> When it comes to build the 32-bit libvtv, it breaks because of using
> "CC=/build/prev-gcc/xgcc -m32" "CXX=g++ -m32", while it should use
> "CC=/build/prev-gcc/xgcc -m32" "CXX=/build/prev-gcc/xg++ -m32" instead.
> 
> However, I'm not sure about the general question behind:
> Should it work to bootstrap the multilib-compiler using a non-multilib one?
> 
> This also needs above configure flags to work around two more but minor 
> issues,
> which I'm unsure about whether I can/should fix at all:
> * --enable-multilib: Without this, the "user friendly check" is breaking,
> since https://gcc.gnu.org/viewcvs/gcc?view=revision&revision=205975

Why is it breaking?

> * --with-system-zlib: Without this, --enable-multilib tries to build a
> 32-bit zlib with "CC=/build/32/./prev-gcc/xgcc"

Ouch, that's a separate bug...  Arguably --with-system-zlib should be
the default these days (and should have been for 10 years or so).

The patch is ok.

Paolo

Re: [PR25530] Convert (unsigned t / 2) * 2 into (unsigned t & ~1)

[Paolo Bonzini]

On 07/07/2015 11:08, Richard Biener wrote:
> Also I am not sure ceil_div and floor_div can be handled this way.
> (5 /[ceil] 2) * 2 == 6 but you compute it as 4.  So I am only convinced
> trunc_div works this way.

Of course also floor_div for unsigned arguments.

For signed arguments, ceil_div works if the operand is known-negative
and floor_div if known-positive.

Perhaps you could optimize these cases to trunc_div first (or maybe it's
already done...)?

Paolo

Re: [PATCH, FT32] initial support

[Paolo Bonzini]

On 03/02/2015 07:05, Andrew Pinski wrote:
> Likewise of:
> +(define_insn "abssi2"
> +  [(set (match_operand:SI 0 "register_operand" "=r")
> + (abs:SI (match_operand:SI 1 "register_operand" "r")))
> +   (clobber (match_scratch:SI 2 "=&r"))]
> +  ""
> +  "ashr.l\t%2,%1,31\;xor.l\t%0,%1,%2\;sub.l\t%0,%0,%2")
> 

optabs.c's expand_abs_nojump already knows this trick:

/* If this machine has expensive jumps, we can do integer absolute
   value of X as (((signed) x >> (W-1)) ^ x) - ((signed) x >> (W-1)),
   where W is the width of MODE. */

So if you define BRANCH_COST to be 2 or more there should be no need for
this pattern at all.

Paolo

Re: patch to fix rtl documentation for new floating point comparisons

[Paolo Bonzini]

On 10/02/2015 22:46, Joseph Myers wrote:
> It may make sense to define LTGT as exactly !UNEQ, and so quiet, but the 
> choice of definition is a matter of what's convenient for the 
> implementation (and which choice you make determines which existing code 
> in GCC should be considered incorrect).

It would be different from e.g. !UNLT and GE differing only in that UNLT
is quiet and GE is signaling.  So it makes sense to me to keep LTGT as
signaling.

Paolo

Re: PATCH: fix breakage from "[PATCH] Fix genmatch linking"

[Paolo Bonzini]

On 10/24/2014 06:32 AM, Hans-Peter Nilsson wrote:
> It seems "more correct" to just disable the config.cache sharing
> between the differently-configured build-subdirectories, as is
> already is done for host-libraries and target-libraries, even if
> that may slow down the builds.

Yes, please do.

Re: libcc1

[Paolo Bonzini]

On 10/29/2014 11:31 AM, Jakub Jelinek wrote:
> It would be nice to have libcc1 built just once, not bootstrap it, but
> it is a build module, is that possible?
> In toplevel configure.ac I'm seeing:   
> host_tools="texinfo flex bison binutils gas ld fixincludes gcc cgen sid sim 
> gdb gprof etc expect dejagnu m4 utils guile fastjar gnattools libcc1"

Stuff such as texinfo and flex is in host_tools just as a relic of the
old Cygnus tree.

fixincludes is in there for running it after installation.

The ones that matter in the common case are biuntils, gas, ld, gcc, gdb,
gnattools and of course libcc1.

> shouldn't libcc1 be in build_tools instead?
> I mean, it is a library meant to be dlopened by gdb and gcc
> plugin that uses that library, so in canadian-cross should be
> for the build target, where the resulting compiler will be run
> and where gdb will be run.

That is host, not build.  Build is the system you are on.

Say you're cross-building a native mingw compiler and debugger:

build = i686-pc-linux-gnu
host = i686-pc-mingw (or whatever they use these days)
target = i686-pc-mingw

You cannot link build-libcc1 (for i686-pc-linux-gnu) into host-gcc or
host-gdb.

But you surely know this, so perhaps it's me who is missing something.

Re: [PATCH 5/5] add libcc1

[Paolo Bonzini]

On 10/29/2014 11:28 AM, Jakub Jelinek wrote:
> If this passes bootstrap/regtest, is it ok for trunk (should fix
> two bootstrap issues)?  Is the
> https://gcc.gnu.org/ml/gcc-patches/2014-10/msg02936.html
> patch ok too (that one already tested; another bootstrap issue)?

Both seem okay, though I'd have to look at the whole thread to
understand what libcc1 is. :)

Just two questions:

1) what's the issue that you need to disable asan for?

2) why is GMPLIB not handled in the same way?

Paolo

Re: libcc1

[Paolo Bonzini]

On 10/29/2014 11:45 AM, Jakub Jelinek wrote:
> On Wed, Oct 29, 2014 at 11:37:26AM +0100, Paolo Bonzini wrote:
>> On 10/29/2014 11:31 AM, Jakub Jelinek wrote:
>>> shouldn't libcc1 be in build_tools instead?
>>> I mean, it is a library meant to be dlopened by gdb and gcc
>>> plugin that uses that library, so in canadian-cross should be
>>> for the build target, where the resulting compiler will be run
>>> and where gdb will be run.
>>
>> That is host, not build.  Build is the system you are on.
> 
> Oops, sorry, mixed that, sure, it should be host tool then.
> 
> So without the first two hunks and third hunk changed so that it
> doesn't bootstrap it?  Doesn't that mean that when bootstrapping
> natively it will be built by the system compiler rather than the
> newly built compiler?

IIRC it will be built after stage3 completes, with the just-bootstrapped
compiler.

> I think fixincludes is only built during
> stage1 normally, we don't need libcc1 during stage1/stage2 unless
> not bootstrapping, it is needed just for installation and testing.
> 
> --- configure.ac  2014-10-28 14:39:53.018852391 +0100
> +++ configure.ac  2014-10-29 11:43:19.873216226 +0100
> @@ -2677,6 +2677,7 @@ for module in ${configdirs} ; do
>fi
>case ${module},${bootstrap_fixincludes} in
>  fixincludes,no) host_bootstrap_suffix=no-bootstrap ;;
> +libcc1,*) host_bootstrap_suffix=no-bootstrap ;;
>  *) host_bootstrap_suffix=$bootstrap_suffix ;;
>esac
>extrasub_host="$extrasub_host

This makes sense.

Paolo

Re: [PATCH 5/5] add libcc1

[Paolo Bonzini]

On 10/29/2014 11:51 AM, Jakub Jelinek wrote:
> On Wed, Oct 29, 2014 at 11:37:42AM +0100, Paolo Bonzini wrote:
>>
>>
>> On 10/29/2014 11:28 AM, Jakub Jelinek wrote:
>>> If this passes bootstrap/regtest, is it ok for trunk (should fix
>>> two bootstrap issues)?  Is the
>>> https://gcc.gnu.org/ml/gcc-patches/2014-10/msg02936.html
>>> patch ok too (that one already tested; another bootstrap issue)?
>>
>> Both seem okay, though I'd have to look at the whole thread to
>> understand what libcc1 is. :)
> 
> It is a library for communication between the debugger and
> a GCC plugin (and the plugin itself).  So, the library is
> dlopened into GDB and the plugin that links against that library
> is dlopened by GCC when GDB asks the library it dlopened to
> run the compiler with the plugin.
> 
>> Just two questions:
>>
>> 1) what's the issue that you need to disable asan for?
> 
> -fsanitize=address generally doesn't work or doesn't work too well,
> if the binary is not built with -fsanitize=address, but shared library
> dlopened into it is.  So, we want to avoid instrumenting plugins
> that way (we already don't instrument lto-plugin for that reason,
> because ld might not be asan instrumented, and libcc1 is similar case,
> when gdb dlopens the library, it might not be instrumented either).

Thanks for explaining.  I can see intuitively why that could be a problem...

>> 2) why is GMPLIB not handled in the same way?
> 
> The only problem is that system.h includes gmp.h, so we need a way
> to find that header.  I think libcc1 doesn't use any functions from gmp
> itself, so if gmp.h can be included, GMPLIB isn't really needed.

Ah, got it.  Is it hard to move the inclusion to the actual users?

Paolo

Re: libcc1

[Paolo Bonzini]

On 10/29/2014 11:58 AM, Phil Muldoon wrote:
> My archaeology into the source repository has not revealed why
> we needed bootstrap.  Perhaps we included it out of a sense of
> paranoia for testing.  I've CC'd Tom on this, so he may have an
> opinion or insight.  From my point of view, I see no value in
> bootstrapping libcc1 now.  It's not a required build to bootstrap GCC.

Then I agree, I don't think it needs to be bootstrapped.

Paolo

Re: [PATCH 5/5] add libcc1

[Paolo Bonzini]

On 10/29/2014 11:59 AM, Jakub Jelinek wrote:
>> > Ah, got it.  Is it hard to move the inclusion to the actual users?
> I think it is hard.  I think it has been moved to system.h very much
> intentionally, as including gmp.h only in selected headers was causing lots
> of troubles, e.g. because of #pragma GCC poison at the end of system.h,
> I believe some gmp.h versions were using some poisoned symbols.
> system.h doesn't include gmp.h if -DGENERATOR_FILE, but libcc1 is not a
> generator, so that is not appropriate, it can use various other GCC headers
> that are not suitable for generators.  GMPINC has been suggested by Joseph,
> I'd think if we ever need also GMPLIB, we'd clearly see it as link failures
> of libcc1 first and could add it only when really needed.

Fair enough, thanks!

Paolo

Re: libcc1

[Paolo Bonzini]

On 10/29/2014 09:10 PM, Jakub Jelinek wrote:
> On Wed, Oct 29, 2014 at 11:45:51AM +0100, Jakub Jelinek wrote:
>> On Wed, Oct 29, 2014 at 11:37:26AM +0100, Paolo Bonzini wrote:
>>> On 10/29/2014 11:31 AM, Jakub Jelinek wrote:
>>>> shouldn't libcc1 be in build_tools instead?
>>>> I mean, it is a library meant to be dlopened by gdb and gcc
>>>> plugin that uses that library, so in canadian-cross should be
>>>> for the build target, where the resulting compiler will be run
>>>> and where gdb will be run.
>>>
>>> That is host, not build.  Build is the system you are on.
>>
>> Oops, sorry, mixed that, sure, it should be host tool then.
>>
>> So without the first two hunks and third hunk changed so that it
>> doesn't bootstrap it?  Doesn't that mean that when bootstrapping
>> natively it will be built by the system compiler rather than the
>> newly built compiler?  I think fixincludes is only built during
>> stage1 normally, we don't need libcc1 during stage1/stage2 unless
>> not bootstrapping, it is needed just for installation and testing.
>>
>> --- configure.ac 2014-10-28 14:39:53.018852391 +0100
>> +++ configure.ac 2014-10-29 11:43:19.873216226 +0100
>> @@ -2677,6 +2677,7 @@ for module in ${configdirs} ; do
>>fi
>>case ${module},${bootstrap_fixincludes} in
>>  fixincludes,no) host_bootstrap_suffix=no-bootstrap ;;
>> +libcc1,*) host_bootstrap_suffix=no-bootstrap ;;
>>  *) host_bootstrap_suffix=$bootstrap_suffix ;;
>>esac
>>extrasub_host="$extrasub_host
> 
> Makefile.def has:
> host_modules= { module= libcc1; bootstrap=true;
> extra_configure_flags=--enable-shared; };
> wonder if that bootstrap=true; is desirable there.

No, it shouldn't be there

Paolo

[PATCH] cpp/c: Add -Wexpansion-to-defined

[Paolo Bonzini]

clang recently added a new warning -Wexpansion-to-defined, which
warns when `defined' is used outside a #if expression (including the
case of a macro that is then used in a #if expression).

While I disagree with their inclusion of the warning to -Wall, I think
it is a good addition overall.  First, it is a logical extension of the
existing warning for breaking defined across a macro and its caller.
Second, it is good to make these warnings for `defined' available with
a command-line option other than -pedantic.

This patch adds -Wexpansion-to-defined, and enables it automatically
for both -pedantic (as before) and -Wextra.

Bootstrapped and regression tested x86_64-pc-linux-gnu, ok?

Paolo

libcpp:
2016-08-09  Paolo Bonzini  

* include/cpplib.h (struct cpp_options): Add new member
warn_expansion_to_defined.
(CPP_W_EXPANSION_TO_DEFINED): New enum member.
* expr.c (parse_defined): Warn for all uses of "defined"
in macros, and tie warning to CPP_W_EXPANSION_TO_DEFINED.
* system.h (HAVE_DESIGNATED_INITIALIZERS): Do not use
"defined" in macros.

gcc:
2016-08-09  Paolo Bonzini  

* system.h (HAVE_DESIGNATED_INITIALIZERS,
HAVE_DESIGNATED_UNION_INITIALIZERS): Do not use
"defined" in macros.

gcc/c-family:
2016-08-09  Paolo Bonzini  

* c-opts (sanitize_cpp_opts): Compute default value
for -Wexpansion-to-defined.
* c.opt (Wexpansion-to-defined): New.

gcc/doc:
2016-08-09  Paolo Bonzini  

* cpp.texi (Defined): Mention -Wexpansion-to-defined.
* cppopts.texi (Invocation): Document -Wexpansion-to-defined.
* invoke.texi (Warning Options): Document -Wexpansion-to-defined.

gcc/testsuite:
2016-08-09  Paolo Bonzini  

* gcc.dg/cpp/defined.c: Mark newly introduced warnings.
* gcc.dg/cpp/defined-Wexpansion-to-defined.c,
gcc.dg/cpp/defined-Wextra-Wno-expansion-to-defined.c,
gcc.dg/cpp/defined-Wextra.c,
gcc.dg/cpp/defined-Wno-expansion-to-defined.c: New testcases.

Index: gcc/c-family/c-opts.c
===
--- gcc/c-family/c-opts.c   (revision 239276)
+++ gcc/c-family/c-opts.c   (working copy)
@@ -1256,6 +1256,10 @@ sanitize_cpp_opts (void)
   cpp_opts->unsigned_char = !flag_signed_char;
   cpp_opts->stdc_0_in_system_headers = STDC_0_IN_SYSTEM_HEADERS;
 
+  cpp_opts->warn_expansion_to_defined = cpp_warn_expansion_to_defined;
+  if (cpp_warn_expansion_to_defined == -1)
+cpp_warn_expansion_to_defined = pedantic || extra_warnings;
+
   /* Wlong-long is disabled by default. It is enabled by:
   [-Wpedantic | -Wtraditional] -std=[gnu|c]++98 ; or
   [-Wpedantic | -Wtraditional] -std=non-c99 
Index: gcc/c-family/c.opt
===
--- gcc/c-family/c.opt  (revision 239276)
+++ gcc/c-family/c.opt  (working copy)
@@ -506,6 +506,10 @@ Wdouble-promotion
 C ObjC C++ ObjC++ Var(warn_double_promotion) Warning
 Warn about implicit conversions from \"float\" to \"double\".
 
+Wexpansion-to-defined
+C ObjC C++ ObjC++ CppReason(CPP_W_EXPANSION_TO_DEFINED) 
Var(cpp_warn_expansion_to_defined) Init(-1) Warning
+Warn if an undefined macro is used in an #if directive.
+
 Wimplicit-function-declaration
 C ObjC Var(warn_implicit_function_declaration) Init(-1) Warning 
LangEnabledBy(C ObjC,Wimplicit)
 Warn about implicit function declarations.
Index: gcc/doc/cpp.texi
===
--- gcc/doc/cpp.texi(revision 239276)
+++ gcc/doc/cpp.texi(working copy)
@@ -3342,7 +3342,8 @@ If the @code{defined} operator appears as a result
 the C standard says the behavior is undefined.  GNU cpp treats it as a
 genuine @code{defined} operator and evaluates it normally.  It will warn
 wherever your code uses this feature if you use the command-line option
-@option{-pedantic}, since other compilers may handle it differently.
+@option{-pedantic}, since other compilers may handle it differently.  The
+warning can also be enabled individually with @option{-Wexpansion-to-defined}.
 
 @node Else
 @subsection Else
Index: gcc/doc/cppopts.texi
===
--- gcc/doc/cppopts.texi(revision 239276)
+++ gcc/doc/cppopts.texi(working copy)
@@ -120,6 +120,12 @@ Warn whenever an identifier which is not a macro i
 @samp{#if} directive, outside of @samp{defined}.  Such identifiers are
 replaced with zero.
 
+@item -Wexpansion-to-defined
+@opindex Wexpansion-to-defined
+Warn whenever @samp{defined} is encountered in the expansion of a macro
+(including the case where the macro is expanded by an @samp{#if} directive).
+Such usage is not portable.
+
 @item -Wunused-macros
 @opindex Wunused-macros
 Warn about macros defined 

Re: [PATCH] cpp/c: Add -Wexpansion-to-defined

[Paolo Bonzini]

On 09/08/2016 20:30, Manuel López-Ibáñez wrote:
>>
>>
>> +  cpp_opts->warn_expansion_to_defined = cpp_warn_expansion_to_defined;
>> +  if (cpp_warn_expansion_to_defined == -1)
>> +cpp_warn_expansion_to_defined = pedantic || extra_warnings;
>> +
> 
> Instead of the above, plase use LangEnabledBy() or EnabledBy() in c.opt.
> See Wendif-labels and other examples. Then, you do not need Init(-1).

This causes this to produce an error if -pedantic-errors is provided,
because -pedantic-errors does

  control_warning_option (OPT_Wpedantic, DK_ERROR, NULL, value,
  loc, lang_mask,
  handlers, opts, opts_set,
  dc);

and this enables -Wexpansion-to-defined at DK_ERROR level.  Bug or fix?

Paolo