public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
* Re: [vta->trunk] VTA merge
@ 2009-09-02 13:06 Dominique Dhumieres
  2009-09-03  5:41 ` Alexandre Oliva
  0 siblings, 1 reply; 52+ messages in thread
From: Dominique Dhumieres @ 2009-09-02 13:06 UTC (permalink / raw)
  To: gcc-patches; +Cc: aoliva

This is likely the cause of pr41224.

TIA

Dominique

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [vta->trunk] VTA merge
  2009-09-02 13:06 [vta->trunk] VTA merge Dominique Dhumieres
@ 2009-09-03  5:41 ` Alexandre Oliva
  2009-09-03  5:52   ` Alexandre Oliva
  0 siblings, 1 reply; 52+ messages in thread
From: Alexandre Oliva @ 2009-09-03  5:41 UTC (permalink / raw)
  To: Dominique Dhumieres, Rainer Orth; +Cc: gcc-patches

[-- Attachment #1: Type: text/plain, Size: 245 bytes --]

On Sep  2, 2009, dominiq@lps.ens.fr (Dominique Dhumieres) wrote:

> This is likely the cause of pr41224.

And pr41228.

This is a patch that should fix it.  I'm checking it in now.  Please
confirm whether it fixes the problem for you.

Thanks,


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: vta-bootstrap-debug-default-test.patch --]
[-- Type: text/x-diff, Size: 8547 bytes --]

for  ChangeLog
from  Alexandre Oliva  <aoliva@redhat.com>

	* configure.ac (--with-build-config): New.  Set BUILD_CONFIG.
	Default to bootstrap-debug only if compare-debug works.
	* configure: Rebuilt.
	* Makefile.tpl: Make BUILD_CONFIG configure-configurable.
	* Makefile.in: Rebuilt.

Index: Makefile.in
===================================================================
--- Makefile.in.orig	2009-09-02 15:05:57.000000000 -0300
+++ Makefile.in	2009-09-02 15:06:35.000000000 -0300
@@ -771,7 +771,7 @@ EXTRA_GCC_FLAGS = \
 GCC_FLAGS_TO_PASS = $(BASE_FLAGS_TO_PASS) $(EXTRA_HOST_FLAGS) $(EXTRA_GCC_FLAGS)
 
 @if gcc
-BUILD_CONFIG = bootstrap-debug
+BUILD_CONFIG = @BUILD_CONFIG@
 ifneq ($(BUILD_CONFIG),)
 include $(foreach CONFIG, $(BUILD_CONFIG), $(srcdir)/config/$(CONFIG).mk)
 endif
Index: Makefile.tpl
===================================================================
--- Makefile.tpl.orig	2009-09-02 15:05:56.000000000 -0300
+++ Makefile.tpl	2009-09-02 15:06:23.000000000 -0300
@@ -619,7 +619,7 @@ EXTRA_GCC_FLAGS = \
 GCC_FLAGS_TO_PASS = $(BASE_FLAGS_TO_PASS) $(EXTRA_HOST_FLAGS) $(EXTRA_GCC_FLAGS)
 
 @if gcc
-BUILD_CONFIG = bootstrap-debug
+BUILD_CONFIG = @BUILD_CONFIG@
 ifneq ($(BUILD_CONFIG),)
 include $(foreach CONFIG, $(BUILD_CONFIG), $(srcdir)/config/$(CONFIG).mk)
 endif
Index: configure
===================================================================
--- configure.orig	2009-09-02 15:05:58.000000000 -0300
+++ configure	2009-09-03 01:51:33.000000000 -0300
@@ -631,6 +631,7 @@ build_tooldir
 tooldir
 GCC_SHLIB_SUBDIR
 RPATH_ENVVAR
+BUILD_CONFIG
 CXXFLAGS_FOR_TARGET
 CFLAGS_FOR_TARGET
 DEBUG_PREFIX_CFLAGS_FOR_TARGET
@@ -6569,13 +6570,42 @@ $as_echo "$as_me: WARNING: trying to boo
 esac
 
 # Adjust the toplevel makefile according to whether bootstrap was selected.
-case "$enable_bootstrap" in
+case $enable_bootstrap in
   yes)
-    bootstrap_suffix=bootstrap ;;
+    bootstrap_suffix=bootstrap
+    BUILD_CONFIG=bootstrap-debug
+    ;;
   no)
-    bootstrap_suffix=no-bootstrap ;;
+    bootstrap_suffix=no-bootstrap
+    BUILD_CONFIG=
+    ;;
 esac
 
+{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for default BUILD_CONFIG" >&5
+$as_echo_n "checking for default BUILD_CONFIG... " >&6; }
+if test "x${with_build_config}" != x; then
+  BUILD_CONFIG=$with_build_config
+else
+  case $BUILD_CONFIG in
+  bootstrap-debug)
+    if echo "int f (void) { return 0; }" > conftest.c &&
+       ${CC} -c conftest.c &&
+       mv conftest.o conftest.o.g0 &&
+       ${CC} -c -g conftest.c &&
+       mv conftest.o conftest.o.g &&
+       ${srcdir}/contrib/compare-debug conftest.o.g0 conftest.o.g; then
+      :
+    else
+      BUILD_CONFIG=
+    fi
+    rm -f conftest.c conftest.o conftest.o.g0 conftest.o.g
+    ;;
+  esac
+fi
+{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $BUILD_CONFIG" >&5
+$as_echo "$BUILD_CONFIG" >&6; }
+
+
 for module in ${build_configdirs} ; do
   if test -z "${no_recursion}" \
      && test -f ${build_subdir}/${module}/Makefile; then
Index: configure.ac
===================================================================
--- configure.ac.orig	2009-09-02 15:05:57.000000000 -0300
+++ configure.ac	2009-09-02 15:16:42.000000000 -0300
@@ -2459,13 +2459,40 @@ case "$have_compiler:$host:$target:$enab
 esac
 
 # Adjust the toplevel makefile according to whether bootstrap was selected.
-case "$enable_bootstrap" in
+case $enable_bootstrap in
   yes)
-    bootstrap_suffix=bootstrap ;;
+    bootstrap_suffix=bootstrap
+    BUILD_CONFIG=bootstrap-debug
+    ;;
   no)
-    bootstrap_suffix=no-bootstrap ;;
+    bootstrap_suffix=no-bootstrap
+    BUILD_CONFIG=
+    ;;
 esac
 
+AC_MSG_CHECKING(for default BUILD_CONFIG)
+if test "x${with_build_config}" != x; then
+  BUILD_CONFIG=$with_build_config
+else
+  case $BUILD_CONFIG in
+  bootstrap-debug)
+    if echo "int f (void) { return 0; }" > conftest.c &&
+       ${CC} -c conftest.c &&
+       mv conftest.o conftest.o.g0 &&
+       ${CC} -c -g conftest.c &&
+       mv conftest.o conftest.o.g &&
+       ${srcdir}/contrib/compare-debug conftest.o.g0 conftest.o.g; then
+      :
+    else
+      BUILD_CONFIG=
+    fi
+    rm -f conftest.c conftest.o conftest.o.g0 conftest.o.g
+    ;;
+  esac
+fi
+AC_MSG_RESULT($BUILD_CONFIG)
+AC_SUBST(BUILD_CONFIG)
+
 for module in ${build_configdirs} ; do
   if test -z "${no_recursion}" \
      && test -f ${build_subdir}/${module}/Makefile; then
Index: config/bootstrap-debug-big.mk
===================================================================
--- config/bootstrap-debug-big.mk.orig	2009-09-03 01:58:52.000000000 -0300
+++ config/bootstrap-debug-big.mk	2009-09-03 01:59:18.000000000 -0300
@@ -3,6 +3,6 @@
 # stage3, it generates dumps during stage2 and stage3, saving them all
 # until the final compare.
 
-STAGE2_CFLAGS += -gtoggle -fdump-final-insns
+STAGE2_CFLAGS += -fdump-final-insns
 STAGE3_CFLAGS += -fdump-final-insns
 do-compare = $(SHELL) $(srcdir)/contrib/compare-debug $$f1 $$f2
Index: config/bootstrap-debug-lean.mk
===================================================================
--- config/bootstrap-debug-lean.mk.orig	2009-09-03 01:58:52.000000000 -0300
+++ config/bootstrap-debug-lean.mk	2009-09-03 01:59:28.000000000 -0300
@@ -6,6 +6,6 @@
 # dumping and recompilation during stage3.  bootstrap-debug-big can
 # avoid the recompilation, if plenty of disk space is available.
 
-STAGE2_CFLAGS += -gtoggle -fcompare-debug=
+STAGE2_CFLAGS += -fcompare-debug=
 STAGE3_CFLAGS += -fcompare-debug
 do-compare = $(SHELL) $(srcdir)/contrib/compare-debug $$f1 $$f2
Index: gcc/doc/install.texi
===================================================================
--- gcc/doc/install.texi.orig	2009-09-03 01:53:03.000000000 -0300
+++ gcc/doc/install.texi	2009-09-03 01:58:17.000000000 -0300
@@ -2080,11 +2080,13 @@ the one you are building on: for example
 @code{powerpc64-unknown-linux-gnu} host.  In this case, pass
 @option{--enable-bootstrap} to the configure script.
 
-@code{BUILD_CONFIG} can be used to bring in additional customization to
-the build.  It can be set to a whitespace-separated list of names.  For
-each such @code{NAME}, top-level @file{config/@code{NAME}.mk} will be
-included by the top-level @file{Makefile}, bringing in any settings it
-contains.  Some examples are:
+@code{BUILD_CONFIG} can be used to bring in additional customization
+to the build.  It can be set to a whitespace-separated list of names.
+For each such @code{NAME}, top-level @file{config/@code{NAME}.mk} will
+be included by the top-level @file{Makefile}, bringing in any settings
+it contains.  The default @code{BUILD_CONFIG} can be set using the
+configure option @option{--with-build-config=@code{NAME}...}.  Some
+examples of supported build configurations are:
 
 @table @asis
 @item @samp{bootstrap-O1}
@@ -2097,19 +2099,22 @@ Analogous to @code{bootstrap-O1}.
 
 @item @samp{bootstrap-debug}
 Verifies that the compiler generates the same executable code, whether
-or not it is asked to emit debug information.  To this end, this option
-builds stage2 host programs without debug information, and uses
+or not it is asked to emit debug information.  To this end, this
+option builds stage2 host programs without debug information, and uses
 @file{contrib/compare-debug} to compare them with the stripped stage3
 object files.  If @code{BOOT_CFLAGS} is overridden so as to not enable
 debug information, stage2 will have it, and stage3 won't.  This option
-is enabled by default when GCC bootstrapping is enabled: in addition to
-better test coverage, it makes default bootstraps faster and leaner.
+is enabled by default when GCC bootstrapping is enabled, if
+@code{strip} can turn object files compiled with and without debug
+info into identical object files.  In addition to better test
+coverage, this option makes default bootstraps faster and leaner.
 
 @item @samp{bootstrap-debug-big}
-In addition to the checking performed by @code{bootstrap-debug}, this
-option saves internal compiler dumps during stage2 and stage3 and
-compares them as well, which helps catch additional potential problems,
-but at a great cost in terms of disk space.
+Rather than comparing stripped object files, as in
+@code{bootstrap-debug}, this option saves internal compiler dumps
+during stage2 and stage3 and compares them as well, which helps catch
+additional potential problems, but at a great cost in terms of disk
+space.  It can be specified in addition to @samp{bootstrap-debug}.
 
 @item @samp{bootstrap-debug-lean}
 This option saves disk space compared with @code{bootstrap-debug-big},

[-- Attachment #3: Type: text/plain, Size: 258 bytes --]



-- 
Alexandre Oliva, freedom fighter    http://FSFLA.org/~lxoliva/
You must be the change you wish to see in the world. -- Gandhi
Be Free! -- http://FSFLA.org/   FSF Latin America board member
Free Software Evangelist      Red Hat Brazil Compiler Engineer

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [vta->trunk] VTA merge
  2009-09-03  5:41 ` Alexandre Oliva
@ 2009-09-03  5:52   ` Alexandre Oliva
  2009-09-03  9:21     ` Rainer Orth
                       ` (2 more replies)
  0 siblings, 3 replies; 52+ messages in thread
From: Alexandre Oliva @ 2009-09-03  5:52 UTC (permalink / raw)
  To: Dominique Dhumieres; +Cc: Rainer Orth, gcc-patches

[-- Attachment #1: Type: text/plain, Size: 450 bytes --]

On Sep  3, 2009, Alexandre Oliva <aoliva@redhat.com> wrote:

> On Sep  2, 2009, dominiq@lps.ens.fr (Dominique Dhumieres) wrote:
>> This is likely the cause of pr41224.

> And pr41228.

> This is a patch that should fix it.  I'm checking it in now.  Please
> confirm whether it fixes the problem for you.

Sorry, I posted an outdated patch.

Here's one that has complete ChangeLog entries and additional fixes for
the bootstrap-debug* BUILD_CONFIGs.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: vta-bootstrap-debug-default-test.patch --]
[-- Type: text/x-diff, Size: 9714 bytes --]

for  ChangeLog
from  Alexandre Oliva  <aoliva@redhat.com>

	* configure.ac (--with-build-config): New.  Set BUILD_CONFIG.
	Default to bootstrap-debug only if compare-debug works.
	* configure: Rebuilt.
	* Makefile.tpl: Make BUILD_CONFIG configure-configurable.
	* Makefile.in: Rebuilt.

for  contrib/ChangeLog
from  Alexandre Oliva  <aoliva@redhat.com>

	* bootstrap-debug-big.mk (STAGE2_CFLAGS): Drop -gtoggle.
	* bootstrap-debug-lean.mk: Update comments.
	(STAGE2_CFLAGS): Likewise.
	(do-compare): Don't override.

for  gcc/ChangeLog
from  Alexandre Oliva  <aoliva@redhat.com>

	* doc/invoke.texi (BUILD_CONFIG): Document --with-build-config.
	(bootstrap-debug): Explain conditions in which it becomes default.
	(bootstrap-debug-big): Rather than duplicate bootstrap-debug,
	make it add to it.

Index: Makefile.in
===================================================================
--- Makefile.in.orig	2009-09-03 02:42:45.000000000 -0300
+++ Makefile.in	2009-09-03 02:46:00.000000000 -0300
@@ -771,7 +771,7 @@ EXTRA_GCC_FLAGS = \
 GCC_FLAGS_TO_PASS = $(BASE_FLAGS_TO_PASS) $(EXTRA_HOST_FLAGS) $(EXTRA_GCC_FLAGS)
 
 @if gcc
-BUILD_CONFIG = bootstrap-debug
+BUILD_CONFIG = @BUILD_CONFIG@
 ifneq ($(BUILD_CONFIG),)
 include $(foreach CONFIG, $(BUILD_CONFIG), $(srcdir)/config/$(CONFIG).mk)
 endif
Index: Makefile.tpl
===================================================================
--- Makefile.tpl.orig	2009-09-03 02:42:45.000000000 -0300
+++ Makefile.tpl	2009-09-03 02:46:00.000000000 -0300
@@ -619,7 +619,7 @@ EXTRA_GCC_FLAGS = \
 GCC_FLAGS_TO_PASS = $(BASE_FLAGS_TO_PASS) $(EXTRA_HOST_FLAGS) $(EXTRA_GCC_FLAGS)
 
 @if gcc
-BUILD_CONFIG = bootstrap-debug
+BUILD_CONFIG = @BUILD_CONFIG@
 ifneq ($(BUILD_CONFIG),)
 include $(foreach CONFIG, $(BUILD_CONFIG), $(srcdir)/config/$(CONFIG).mk)
 endif
Index: configure
===================================================================
--- configure.orig	2009-09-03 02:42:45.000000000 -0300
+++ configure	2009-09-03 02:46:00.000000000 -0300
@@ -631,6 +631,7 @@ build_tooldir
 tooldir
 GCC_SHLIB_SUBDIR
 RPATH_ENVVAR
+BUILD_CONFIG
 CXXFLAGS_FOR_TARGET
 CFLAGS_FOR_TARGET
 DEBUG_PREFIX_CFLAGS_FOR_TARGET
@@ -6569,13 +6570,42 @@ $as_echo "$as_me: WARNING: trying to boo
 esac
 
 # Adjust the toplevel makefile according to whether bootstrap was selected.
-case "$enable_bootstrap" in
+case $enable_bootstrap in
   yes)
-    bootstrap_suffix=bootstrap ;;
+    bootstrap_suffix=bootstrap
+    BUILD_CONFIG=bootstrap-debug
+    ;;
   no)
-    bootstrap_suffix=no-bootstrap ;;
+    bootstrap_suffix=no-bootstrap
+    BUILD_CONFIG=
+    ;;
 esac
 
+{ $as_echo "$as_me:${as_lineno-$LINENO}: checking for default BUILD_CONFIG" >&5
+$as_echo_n "checking for default BUILD_CONFIG... " >&6; }
+if test "x${with_build_config}" != x; then
+  BUILD_CONFIG=$with_build_config
+else
+  case $BUILD_CONFIG in
+  bootstrap-debug)
+    if echo "int f (void) { return 0; }" > conftest.c &&
+       ${CC} -c conftest.c &&
+       mv conftest.o conftest.o.g0 &&
+       ${CC} -c -g conftest.c &&
+       mv conftest.o conftest.o.g &&
+       ${srcdir}/contrib/compare-debug conftest.o.g0 conftest.o.g; then
+      :
+    else
+      BUILD_CONFIG=
+    fi
+    rm -f conftest.c conftest.o conftest.o.g0 conftest.o.g
+    ;;
+  esac
+fi
+{ $as_echo "$as_me:${as_lineno-$LINENO}: result: $BUILD_CONFIG" >&5
+$as_echo "$BUILD_CONFIG" >&6; }
+
+
 for module in ${build_configdirs} ; do
   if test -z "${no_recursion}" \
      && test -f ${build_subdir}/${module}/Makefile; then
Index: configure.ac
===================================================================
--- configure.ac.orig	2009-09-03 02:42:45.000000000 -0300
+++ configure.ac	2009-09-03 02:46:00.000000000 -0300
@@ -2459,13 +2459,40 @@ case "$have_compiler:$host:$target:$enab
 esac
 
 # Adjust the toplevel makefile according to whether bootstrap was selected.
-case "$enable_bootstrap" in
+case $enable_bootstrap in
   yes)
-    bootstrap_suffix=bootstrap ;;
+    bootstrap_suffix=bootstrap
+    BUILD_CONFIG=bootstrap-debug
+    ;;
   no)
-    bootstrap_suffix=no-bootstrap ;;
+    bootstrap_suffix=no-bootstrap
+    BUILD_CONFIG=
+    ;;
 esac
 
+AC_MSG_CHECKING(for default BUILD_CONFIG)
+if test "x${with_build_config}" != x; then
+  BUILD_CONFIG=$with_build_config
+else
+  case $BUILD_CONFIG in
+  bootstrap-debug)
+    if echo "int f (void) { return 0; }" > conftest.c &&
+       ${CC} -c conftest.c &&
+       mv conftest.o conftest.o.g0 &&
+       ${CC} -c -g conftest.c &&
+       mv conftest.o conftest.o.g &&
+       ${srcdir}/contrib/compare-debug conftest.o.g0 conftest.o.g; then
+      :
+    else
+      BUILD_CONFIG=
+    fi
+    rm -f conftest.c conftest.o conftest.o.g0 conftest.o.g
+    ;;
+  esac
+fi
+AC_MSG_RESULT($BUILD_CONFIG)
+AC_SUBST(BUILD_CONFIG)
+
 for module in ${build_configdirs} ; do
   if test -z "${no_recursion}" \
      && test -f ${build_subdir}/${module}/Makefile; then
Index: config/bootstrap-debug-big.mk
===================================================================
--- config/bootstrap-debug-big.mk.orig	2009-09-03 02:42:45.000000000 -0300
+++ config/bootstrap-debug-big.mk	2009-09-03 02:46:00.000000000 -0300
@@ -3,6 +3,6 @@
 # stage3, it generates dumps during stage2 and stage3, saving them all
 # until the final compare.
 
-STAGE2_CFLAGS += -gtoggle -fdump-final-insns
+STAGE2_CFLAGS += -fdump-final-insns
 STAGE3_CFLAGS += -fdump-final-insns
 do-compare = $(SHELL) $(srcdir)/contrib/compare-debug $$f1 $$f2
Index: config/bootstrap-debug-lean.mk
===================================================================
--- config/bootstrap-debug-lean.mk.orig	2009-09-03 02:42:45.000000000 -0300
+++ config/bootstrap-debug-lean.mk	2009-09-03 02:47:20.000000000 -0300
@@ -1,11 +1,11 @@
-# This BUILD_CONFIG option is a bit like bootstrap-debug, but in
-# addition to comparing stripped object files, it also compares
-# compiler internal state during stage3.
+# This BUILD_CONFIG option is a bit like bootstrap-debug, but rather
+# than comparing stripped object files, it compares compiler internal
+# state during stage3.  Both can be used simultaneously.
 
-# This makes it slower than bootstrap-debug, for there's additional
-# dumping and recompilation during stage3.  bootstrap-debug-big can
-# avoid the recompilation, if plenty of disk space is available.
+# This makes it slower than bootstrap-debug alone, for there's
+# additional dumping and recompilation during stage3.
+# bootstrap-debug-big can avoid the recompilation, if plenty of disk
+# space is available.
 
-STAGE2_CFLAGS += -gtoggle -fcompare-debug=
+STAGE2_CFLAGS += -fcompare-debug=
 STAGE3_CFLAGS += -fcompare-debug
-do-compare = $(SHELL) $(srcdir)/contrib/compare-debug $$f1 $$f2
Index: gcc/doc/install.texi
===================================================================
--- gcc/doc/install.texi.orig	2009-09-03 02:42:45.000000000 -0300
+++ gcc/doc/install.texi	2009-09-03 02:46:00.000000000 -0300
@@ -2080,11 +2080,13 @@ the one you are building on: for example
 @code{powerpc64-unknown-linux-gnu} host.  In this case, pass
 @option{--enable-bootstrap} to the configure script.
 
-@code{BUILD_CONFIG} can be used to bring in additional customization to
-the build.  It can be set to a whitespace-separated list of names.  For
-each such @code{NAME}, top-level @file{config/@code{NAME}.mk} will be
-included by the top-level @file{Makefile}, bringing in any settings it
-contains.  Some examples are:
+@code{BUILD_CONFIG} can be used to bring in additional customization
+to the build.  It can be set to a whitespace-separated list of names.
+For each such @code{NAME}, top-level @file{config/@code{NAME}.mk} will
+be included by the top-level @file{Makefile}, bringing in any settings
+it contains.  The default @code{BUILD_CONFIG} can be set using the
+configure option @option{--with-build-config=@code{NAME}...}.  Some
+examples of supported build configurations are:
 
 @table @asis
 @item @samp{bootstrap-O1}
@@ -2097,19 +2099,22 @@ Analogous to @code{bootstrap-O1}.
 
 @item @samp{bootstrap-debug}
 Verifies that the compiler generates the same executable code, whether
-or not it is asked to emit debug information.  To this end, this option
-builds stage2 host programs without debug information, and uses
+or not it is asked to emit debug information.  To this end, this
+option builds stage2 host programs without debug information, and uses
 @file{contrib/compare-debug} to compare them with the stripped stage3
 object files.  If @code{BOOT_CFLAGS} is overridden so as to not enable
 debug information, stage2 will have it, and stage3 won't.  This option
-is enabled by default when GCC bootstrapping is enabled: in addition to
-better test coverage, it makes default bootstraps faster and leaner.
+is enabled by default when GCC bootstrapping is enabled, if
+@code{strip} can turn object files compiled with and without debug
+info into identical object files.  In addition to better test
+coverage, this option makes default bootstraps faster and leaner.
 
 @item @samp{bootstrap-debug-big}
-In addition to the checking performed by @code{bootstrap-debug}, this
-option saves internal compiler dumps during stage2 and stage3 and
-compares them as well, which helps catch additional potential problems,
-but at a great cost in terms of disk space.
+Rather than comparing stripped object files, as in
+@code{bootstrap-debug}, this option saves internal compiler dumps
+during stage2 and stage3 and compares them as well, which helps catch
+additional potential problems, but at a great cost in terms of disk
+space.  It can be specified in addition to @samp{bootstrap-debug}.
 
 @item @samp{bootstrap-debug-lean}
 This option saves disk space compared with @code{bootstrap-debug-big},

[-- Attachment #3: Type: text/plain, Size: 257 bytes --]


-- 
Alexandre Oliva, freedom fighter    http://FSFLA.org/~lxoliva/
You must be the change you wish to see in the world. -- Gandhi
Be Free! -- http://FSFLA.org/   FSF Latin America board member
Free Software Evangelist      Red Hat Brazil Compiler Engineer

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [vta->trunk] VTA merge
  2009-09-03  5:52   ` Alexandre Oliva
@ 2009-09-03  9:21     ` Rainer Orth
  2009-09-03 21:24       ` Alexandre Oliva
  2009-09-03 19:05     ` Ralf Wildenhues
  2009-09-03 21:49     ` Alexandre Oliva
  2 siblings, 1 reply; 52+ messages in thread
From: Rainer Orth @ 2009-09-03  9:21 UTC (permalink / raw)
  To: Alexandre Oliva; +Cc: Dominique Dhumieres, gcc-patches

Alexandre Oliva writes:

> On Sep  3, 2009, Alexandre Oliva <aoliva@redhat.com> wrote:
> 
> > On Sep  2, 2009, dominiq@lps.ens.fr (Dominique Dhumieres) wrote:
> >> This is likely the cause of pr41224.
> 
> > And pr41228.
> 
> > This is a patch that should fix it.  I'm checking it in now.  Please
> > confirm whether it fixes the problem for you.
> 
> Sorry, I posted an outdated patch.
> 
> Here's one that has complete ChangeLog entries and additional fixes for
> the bootstrap-debug* BUILD_CONFIGs.

unfortunately, on sparc-sun-solaris2.11 the sparcv9 stage1 libgcc fails to
build:

/vol/gcc/src/gcc-dist/libgcc/../gcc/libgcc2.c: In function '__divtc3':
/vol/gcc/src/gcc-dist/libgcc/../gcc/libgcc2.c:1948:1: internal compiler error: in loc_cmp, at var-tracking.c:2456

i386-pc-solaris2.10 (already in stage2) and mips-sgi-irix6.5 bootstraps are
still running.

	Rainer

-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [vta->trunk] VTA merge
  2009-09-03  5:52   ` Alexandre Oliva
  2009-09-03  9:21     ` Rainer Orth
@ 2009-09-03 19:05     ` Ralf Wildenhues
  2009-09-03 21:46       ` Alexandre Oliva
  2009-09-03 21:49     ` Alexandre Oliva
  2 siblings, 1 reply; 52+ messages in thread
From: Ralf Wildenhues @ 2009-09-03 19:05 UTC (permalink / raw)
  To: Alexandre Oliva; +Cc: Dominique Dhumieres, Rainer Orth, gcc-patches

Hi Alexandre,

* Alexandre Oliva wrote on Thu, Sep 03, 2009 at 07:52:20AM CEST:
> for  ChangeLog
> from  Alexandre Oliva  <aoliva@redhat.com>
> 
> 	* configure.ac (--with-build-config): New.  Set BUILD_CONFIG.
> 	Default to bootstrap-debug only if compare-debug works.
> 	* configure: Rebuilt.
> 	* Makefile.tpl: Make BUILD_CONFIG configure-configurable.
> 	* Makefile.in: Rebuilt.

This patch exposes nonportable code in compare-debug:
Solaris 10 grep does not grok -e:

checking for default BUILD_CONFIG... conftest.o.g0.stripped conftest.o.g.stripped differ: char 48, line 1
grep: illegal option -- e
grep: illegal option -- \[*section-\]*headers
Usage: grep -hblcnsviw pattern file . . .
grep: illegal option -- e
grep: illegal option -- \[*section-\]*headers
Usage: grep -hblcnsviw pattern file . . .
grep: illegal option -- e
grep: illegal option -- \[*section-\]*headers
Usage: grep -hblcnsviw pattern file . . .
stripping off .eh_frame, then retrying
grep: illegal option -- e
grep: illegal option -- remove-section
Usage: grep -hblcnsviw pattern file . . .
grep: illegal option -- e
grep: illegal option -- remove-section
Usage: grep -hblcnsviw pattern file . . .
failed to strip off .eh_frame
conftest.o.g0.stripped conftest.o.g.stripped differ: char 48, line 1

Cheers,
Ralf

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [vta->trunk] VTA merge
  2009-09-03  9:21     ` Rainer Orth
@ 2009-09-03 21:24       ` Alexandre Oliva
  2009-09-04 11:07         ` Rainer Orth
  0 siblings, 1 reply; 52+ messages in thread
From: Alexandre Oliva @ 2009-09-03 21:24 UTC (permalink / raw)
  To: Rainer Orth; +Cc: Dominique Dhumieres, gcc-patches

On Sep  3, 2009, Rainer Orth <ro@techfak.uni-bielefeld.de> wrote:

> unfortunately, on sparc-sun-solaris2.11 the sparcv9 stage1 libgcc fails to
> build:

> /vol/gcc/src/gcc-dist/libgcc/../gcc/libgcc2.c: In function '__divtc3':
> /vol/gcc/src/gcc-dist/libgcc/../gcc/libgcc2.c:1948:1: internal compiler error: in loc_cmp, at var-tracking.c:2456

You got a preprocessed testcase you could send me (PVT is fine), please?

Thanks in advance,

-- 
Alexandre Oliva, freedom fighter    http://FSFLA.org/~lxoliva/
You must be the change you wish to see in the world. -- Gandhi
Be Free! -- http://FSFLA.org/   FSF Latin America board member
Free Software Evangelist      Red Hat Brazil Compiler Engineer

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [vta->trunk] VTA merge
  2009-09-03 19:05     ` Ralf Wildenhues
@ 2009-09-03 21:46       ` Alexandre Oliva
  0 siblings, 0 replies; 52+ messages in thread
From: Alexandre Oliva @ 2009-09-03 21:46 UTC (permalink / raw)
  To: Ralf Wildenhues; +Cc: Dominique Dhumieres, Rainer Orth, gcc-patches

[-- Attachment #1: Type: text/plain, Size: 301 bytes --]

On Sep  3, 2009, Ralf Wildenhues <Ralf.Wildenhues@gmx.de> wrote:

> This patch exposes nonportable code in compare-debug:
> Solaris 10 grep does not grok -e:

Ugh.  How fast portability memory fades! :-(

Thanks, here's a patch that should address this.  I'll check it in once
I'm done with testing.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: contrib-compare-debug-grep-port.patch --]
[-- Type: text/x-diff, Size: 1555 bytes --]

for  contrib/ChangeLog
from  Alexandre Oliva  <aoliva@redhat.com>

	* compare-debug: Grep for blank before dash to avoid grep -e.

Index: contrib/compare-debug
===================================================================
--- contrib/compare-debug.orig	2009-09-03 18:40:04.000000000 -0300
+++ contrib/compare-debug	2009-09-03 18:40:12.000000000 -0300
@@ -78,7 +78,7 @@ else
   cmp2=
 
   for t in objdump readelf eu-readelf; do
-    if ($t --help) 2>&1 | grep -e '--\[*section-\]*headers' > /dev/null; then
+    if ($t --help) 2>&1 | grep ' --\[*section-\]*headers' > /dev/null; then
       cmd=$t
 
       $cmd --section-headers "$1.$suf1" | grep '\.eh_frame' > /dev/null
@@ -109,13 +109,13 @@ else
 
     echo stripping off .eh_frame, then retrying >&2
 
-    if (objcopy -v) 2>&1 | grep -e "--remove-section" > /dev/null; then
+    if (objcopy -v) 2>&1 | grep ' --remove-section' > /dev/null; then
       objcopy --remove-section .eh_frame --remove-section .rel.eh_frame --remove-section .rela.eh_frame "$1.$suf1" "$1.$suf3"
       mv "$1.$suf3" "$1.$suf1"
 
       objcopy --remove-section .eh_frame --remove-section .rel.eh_frame --remove-section .rela.eh_frame "$2.$suf2" "$2.$suf4"
       mv "$2.$suf4" "$2.$suf2"
-    elif (strip --help) 2>&1 | grep -e --remove-section > /dev/null; then
+    elif (strip --help) 2>&1 | grep ' --remove-section' > /dev/null; then
       cp "$1.$suf1" "$1.$suf3"
       strip --remove-section .eh_frame --remove-section .rel.eh_frame --remove-section .rela.eh_frame "$1.$suf3"
       mv "$1.$suf3" "$1.$suf1"

[-- Attachment #3: Type: text/plain, Size: 257 bytes --]


-- 
Alexandre Oliva, freedom fighter    http://FSFLA.org/~lxoliva/
You must be the change you wish to see in the world. -- Gandhi
Be Free! -- http://FSFLA.org/   FSF Latin America board member
Free Software Evangelist      Red Hat Brazil Compiler Engineer

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [vta->trunk] VTA merge
  2009-09-03  5:52   ` Alexandre Oliva
  2009-09-03  9:21     ` Rainer Orth
  2009-09-03 19:05     ` Ralf Wildenhues
@ 2009-09-03 21:49     ` Alexandre Oliva
  2 siblings, 0 replies; 52+ messages in thread
From: Alexandre Oliva @ 2009-09-03 21:49 UTC (permalink / raw)
  To: Dominique Dhumieres; +Cc: Rainer Orth, gcc-patches

[-- Attachment #1: Type: text/plain, Size: 599 bytes --]

On Sep  3, 2009, Alexandre Oliva <aoliva@redhat.com> wrote:

> 	* configure.ac (--with-build-config): New.  Set BUILD_CONFIG.
> 	Default to bootstrap-debug only if compare-debug works.
> 	* configure: Rebuilt.

Here's a followup patch that avoids noisy include warnings from make if
--with-build-config or --without-build-config are passed in the
configure command line, because config/yes.mk and config/no.mk do not
exist.

--with-build-config is essentially a nop, whereas --without-build-config
effectively disables the default build configuration.

I'll check it in once I'm done with testing.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: toplev-with-build-config.patch --]
[-- Type: text/x-diff, Size: 2147 bytes --]

for  ChangeLog
from  Alexandre Oliva  <aoliva@redhat.com>

	* configure.ac (with-build-config): Document.  Handle without.
	Handle missing argument.
	* configure: Rebuilt.

Index: configure
===================================================================
--- configure.orig	2009-09-03 16:19:41.000000000 -0300
+++ configure	2009-09-03 16:25:19.000000000 -0300
@@ -769,6 +769,7 @@ enable_objc_gc
 with_build_sysroot
 with_debug_prefix_map
 enable_bootstrap
+with_build_config
 enable_serial_configure
 with_build_time_tools
 enable_maintainer_mode
@@ -1498,6 +1499,8 @@ Optional Packages:
                           use sysroot as the system root during the build
   --with-debug-prefix-map='A=B C=D ...'
                              map A to B, C to D ... in debug information
+--with-build-config='NAME NAME2...'
+                          Use config/NAME.mk build configuration
   --with-build-time-tools=PATH
                           use given path to find target tools during the build
 
@@ -6583,6 +6586,17 @@ esac
 
 { $as_echo "$as_me:${as_lineno-$LINENO}: checking for default BUILD_CONFIG" >&5
 $as_echo_n "checking for default BUILD_CONFIG... " >&6; }
+
+
+# Check whether --with-build-config was given.
+if test "${with_build_config+set}" = set; then :
+  withval=$with_build_config; case $with_build_config in
+   yes) with_build_config= ;;
+   no) with_build_config= BUILD_CONFIG= ;;
+   esac
+fi
+
+
 if test "x${with_build_config}" != x; then
   BUILD_CONFIG=$with_build_config
 else
Index: configure.ac
===================================================================
--- configure.ac.orig	2009-09-03 16:19:41.000000000 -0300
+++ configure.ac	2009-09-03 16:25:16.000000000 -0300
@@ -2471,6 +2471,15 @@ case $enable_bootstrap in
 esac
 
 AC_MSG_CHECKING(for default BUILD_CONFIG)
+
+AC_ARG_WITH([build-config],
+  [--with-build-config='NAME NAME2...'
+                          Use config/NAME.mk build configuration],
+  [case $with_build_config in
+   yes) with_build_config= ;;
+   no) with_build_config= BUILD_CONFIG= ;;
+   esac])
+
 if test "x${with_build_config}" != x; then
   BUILD_CONFIG=$with_build_config
 else

[-- Attachment #3: Type: text/plain, Size: 257 bytes --]


-- 
Alexandre Oliva, freedom fighter    http://FSFLA.org/~lxoliva/
You must be the change you wish to see in the world. -- Gandhi
Be Free! -- http://FSFLA.org/   FSF Latin America board member
Free Software Evangelist      Red Hat Brazil Compiler Engineer

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [vta->trunk] VTA merge
  2009-09-03 21:24       ` Alexandre Oliva
@ 2009-09-04 11:07         ` Rainer Orth
  0 siblings, 0 replies; 52+ messages in thread
From: Rainer Orth @ 2009-09-04 11:07 UTC (permalink / raw)
  To: Alexandre Oliva; +Cc: Dominique Dhumieres, gcc-patches

Alexandre Oliva writes:

> On Sep  3, 2009, Rainer Orth <ro@techfak.uni-bielefeld.de> wrote:
> 
> > unfortunately, on sparc-sun-solaris2.11 the sparcv9 stage1 libgcc fails to
> > build:
> 
> > /vol/gcc/src/gcc-dist/libgcc/../gcc/libgcc2.c: In function '__divtc3':
> > /vol/gcc/src/gcc-dist/libgcc/../gcc/libgcc2.c:1948:1: internal compiler error: in loc_cmp, at var-tracking.c:2456
> 
> You got a preprocessed testcase you could send me (PVT is fine), please?

That one is gone as ov rev 151416, but instead of two comparison failures

gcc/objc/objc-act.o differs
gcc/c-common.o differs

one still remains:

gcc/c-common.o differs

I'll attach the stage2 and stage3 object files to PR bootstrap/41241 as
Jakub requested.

	Rainer

-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [vta->trunk] VTA merge
  2009-09-16 15:48                   ` Jakub Jelinek
@ 2009-09-16 15:56                     ` Richard Guenther
  0 siblings, 0 replies; 52+ messages in thread
From: Richard Guenther @ 2009-09-16 15:56 UTC (permalink / raw)
  To: Jakub Jelinek
  Cc: Alexandre Oliva, Tom Tromey, Joseph S. Myers, Jack Howarth,
	Steven Bosscher, gcc-patches

On Wed, Sep 16, 2009 at 5:47 PM, Jakub Jelinek <jakub@redhat.com> wrote:
> On Thu, Sep 03, 2009 at 04:14:32AM -0300, Alexandre Oliva wrote:
>> On Sep  2, 2009, Tom Tromey <tromey@redhat.com> wrote:
>>
>> > FWIW I changed my mind on this topic.  I was swayed by your argument
>> > about compatibility: the alternative to emitting DW_OP_*_value seems to
>> > be to emit nothing.  Either way an older debugger is just not going to
>> > be able to print the value.
>>
>> I think the important question is whether older debuggers would be able
>> to deal gracefully with these location entries or constant values.  I
>> understand they're supposed to, and I have no evidence that GDB isn't,
>> so I offer this patch to revert the last-minute addition of tests for
>> dwarf_version >= 4 in the VTA merge patch.
>>
>> Should I check it in so that we can get a better idea of what, if
>> anything, breaks?
>
>> for  gcc/ChangeLog
>> from  Alexandre Oliva  <aoliva@redhat.com>
>>
>>       * dwarf2out.c (loc_descriptor): Emit DW_OP_stack_value and
>>       DW_OP_implicit_value even without dwarf_version 4.
>
> Could somebody please review this?
> I believe we should emit them by default, if the debugger doesn't understand
> them, it will just not be able to find out what value particular variable
> has, but without the ops it won't be able to do so either.
>
> Currently, limiting it to dwarf_version >= 4 has 2 problems:
> 1) it is not used by default, so far fewer people will benefit
> 2) -gdwarf-4 has other side effects, e.g. changing headers of .debug_info
>   and other sections, which is IMHO ATM undesirable for dwarf4, because
>   while DW_OP_{stack,implicit}_value can be considered stable at this
>   point, the whole standard is still in development and we don't know yet
>   what all version 4 in .debug_info etc. will imply.

The patch is ok if nobody objects until tomorrow.

Thanks,
Richard.

>        Jakub
>

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [vta->trunk] VTA merge
  2009-09-03  7:14                 ` Alexandre Oliva
  2009-09-03 15:36                   ` Tom Tromey
@ 2009-09-16 15:48                   ` Jakub Jelinek
  2009-09-16 15:56                     ` Richard Guenther
  1 sibling, 1 reply; 52+ messages in thread
From: Jakub Jelinek @ 2009-09-16 15:48 UTC (permalink / raw)
  To: Alexandre Oliva
  Cc: Tom Tromey, Joseph S. Myers, Jack Howarth, Steven Bosscher,
	Richard Guenther, gcc-patches

On Thu, Sep 03, 2009 at 04:14:32AM -0300, Alexandre Oliva wrote:
> On Sep  2, 2009, Tom Tromey <tromey@redhat.com> wrote:
> 
> > FWIW I changed my mind on this topic.  I was swayed by your argument
> > about compatibility: the alternative to emitting DW_OP_*_value seems to
> > be to emit nothing.  Either way an older debugger is just not going to
> > be able to print the value.
> 
> I think the important question is whether older debuggers would be able
> to deal gracefully with these location entries or constant values.  I
> understand they're supposed to, and I have no evidence that GDB isn't,
> so I offer this patch to revert the last-minute addition of tests for
> dwarf_version >= 4 in the VTA merge patch.
> 
> Should I check it in so that we can get a better idea of what, if
> anything, breaks?

> for  gcc/ChangeLog
> from  Alexandre Oliva  <aoliva@redhat.com>
> 
> 	* dwarf2out.c (loc_descriptor): Emit DW_OP_stack_value and
> 	DW_OP_implicit_value even without dwarf_version 4.

Could somebody please review this?
I believe we should emit them by default, if the debugger doesn't understand
them, it will just not be able to find out what value particular variable
has, but without the ops it won't be able to do so either.

Currently, limiting it to dwarf_version >= 4 has 2 problems:
1) it is not used by default, so far fewer people will benefit
2) -gdwarf-4 has other side effects, e.g. changing headers of .debug_info
   and other sections, which is IMHO ATM undesirable for dwarf4, because
   while DW_OP_{stack,implicit}_value can be considered stable at this
   point, the whole standard is still in development and we don't know yet
   what all version 4 in .debug_info etc. will imply.

	Jakub

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [vta->trunk] VTA merge
  2009-09-04  7:21                       ` Jakub Jelinek
@ 2009-09-04 13:37                         ` Mark Mitchell
  0 siblings, 0 replies; 52+ messages in thread
From: Mark Mitchell @ 2009-09-04 13:37 UTC (permalink / raw)
  To: Jakub Jelinek
  Cc: Tom Tromey, Alexandre Oliva, Joseph S. Myers, Jack Howarth,
	Steven Bosscher, Richard Guenther, gcc-patches

Jakub Jelinek wrote:

>>> I don't know about other debuggers, but GDB will issue an error like:
>>>
>>>     Unhandled dwarf expression opcode 0x9e
>>>
>>> if you try to print a variable whose location uses that opcode.
>> That's unfortunate.  Obviously, we can't do anything about older
>> versions of GDB, but can we change GDB so that it doesn't complain and
>> just silently ignores the expression, unless in some debugging mode? 
> 
> Yes, it is unfortunate.  But we can't retroactively change released GDBs,
> and the upcoming GDB will understand it.  And you need the upcoming GDB to
> debug GCC 4.5 compiled code anyway, because older GDBs mishandle
> DW_CFA_restore_state which is now used all around in .eh_frame, otherwise
> backtraces really don't work.

Yes, I understand; nothing we can do for 4.5.  But, I'd like to hear
that we're fixing GDB so that if GCC (or some other compiler) introduces
new DWARF opcodes in future we won't have a similar problem.

-- 
Mark Mitchell
CodeSourcery
mark@codesourcery.com
(650) 331-3385 x713

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [vta->trunk] VTA merge
  2009-09-04  4:58                     ` Mark Mitchell
@ 2009-09-04  7:21                       ` Jakub Jelinek
  2009-09-04 13:37                         ` Mark Mitchell
  0 siblings, 1 reply; 52+ messages in thread
From: Jakub Jelinek @ 2009-09-04  7:21 UTC (permalink / raw)
  To: Mark Mitchell
  Cc: Tom Tromey, Alexandre Oliva, Joseph S. Myers, Jack Howarth,
	Steven Bosscher, Richard Guenther, gcc-patches

On Thu, Sep 03, 2009 at 06:16:30PM -0700, Mark Mitchell wrote:
> Tom Tromey wrote:
> 
> > I don't know about other debuggers, but GDB will issue an error like:
> > 
> >     Unhandled dwarf expression opcode 0x9e
> > 
> > if you try to print a variable whose location uses that opcode.
> 
> That's unfortunate.  Obviously, we can't do anything about older
> versions of GDB, but can we change GDB so that it doesn't complain and
> just silently ignores the expression, unless in some debugging mode?  If
> I'm a user, and I have some compiler that spits out weird DWARF GDB
> doesn't understand, scary messages aren't helping me.

Yes, it is unfortunate.  But we can't retroactively change released GDBs,
and the upcoming GDB will understand it.  And you need the upcoming GDB to
debug GCC 4.5 compiled code anyway, because older GDBs mishandle
DW_CFA_restore_state which is now used all around in .eh_frame, otherwise
backtraces really don't work.

	Jakub

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [vta->trunk] VTA merge
  2009-09-03 15:36                   ` Tom Tromey
@ 2009-09-04  4:58                     ` Mark Mitchell
  2009-09-04  7:21                       ` Jakub Jelinek
  0 siblings, 1 reply; 52+ messages in thread
From: Mark Mitchell @ 2009-09-04  4:58 UTC (permalink / raw)
  To: Tom Tromey
  Cc: Alexandre Oliva, Joseph S. Myers, Jack Howarth, Steven Bosscher,
	Richard Guenther, gcc-patches

Tom Tromey wrote:

> I don't know about other debuggers, but GDB will issue an error like:
> 
>     Unhandled dwarf expression opcode 0x9e
> 
> if you try to print a variable whose location uses that opcode.

That's unfortunate.  Obviously, we can't do anything about older
versions of GDB, but can we change GDB so that it doesn't complain and
just silently ignores the expression, unless in some debugging mode?  If
I'm a user, and I have some compiler that spits out weird DWARF GDB
doesn't understand, scary messages aren't helping me.

-- 
Mark Mitchell
CodeSourcery
mark@codesourcery.com
(650) 331-3385 x713

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [vta->trunk] VTA merge
  2009-09-03  7:14                 ` Alexandre Oliva
@ 2009-09-03 15:36                   ` Tom Tromey
  2009-09-04  4:58                     ` Mark Mitchell
  2009-09-16 15:48                   ` Jakub Jelinek
  1 sibling, 1 reply; 52+ messages in thread
From: Tom Tromey @ 2009-09-03 15:36 UTC (permalink / raw)
  To: Alexandre Oliva
  Cc: Joseph S. Myers, Jack Howarth, Steven Bosscher, Richard Guenther,
	gcc-patches

>>>>> "Alexandre" == Alexandre Oliva <aoliva@redhat.com> writes:

Alexandre> I think the important question is whether older debuggers
Alexandre> would be able to deal gracefully with these location entries
Alexandre> or constant values.  I understand they're supposed to, and I
Alexandre> have no evidence that GDB isn't, so I offer this patch to
Alexandre> revert the last-minute addition of tests for dwarf_version >=
Alexandre> 4 in the VTA merge patch.

I don't know about other debuggers, but GDB will issue an error like:

    Unhandled dwarf expression opcode 0x9e

if you try to print a variable whose location uses that opcode.

You can see that with GDB trunk today :) because the DW_OP_*_value patch
is still pending.

Tom

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [vta->trunk] VTA merge
  2009-09-03  9:27 Uros Bizjak
@ 2009-09-03 11:11 ` Rainer Orth
  0 siblings, 0 replies; 52+ messages in thread
From: Rainer Orth @ 2009-09-03 11:11 UTC (permalink / raw)
  To: Uros Bizjak; +Cc: gcc-patches, Alexandre Oliva, Dominique Dhumieres

Uros Bizjak writes:

> > unfortunately, on sparc-sun-solaris2.11 the sparcv9 stage1 libgcc fails to build:
> 
> > /vol/gcc/src/gcc-dist/libgcc/../gcc/libgcc2.c: In function '__divtc3':
> > /vol/gcc/src/gcc-dist/libgcc/../gcc/libgcc2.c:1948:1: internal compiler error: in loc_cmp, at var-tracking.c:2456
> 
> PR 41238.

Thanks.  mips-sgi-irix6.5 is broken as well in the mabi=64 stage1 libgcc:

/vol/gcc/src/gcc-dist/libgcc/../gcc/libgcc2.c: In function '__lshrti3':
/vol/gcc/src/gcc-dist/libgcc/../gcc/libgcc2.c:431:1: error: unrecognizable
insn:
(debug_insn 14 13 15 3 /vol/gcc/src/gcc-dist/libgcc/../gcc/libgcc2.c:414
(var_location:DI bm (reg/v:DI 3 $3 [orig:193 bm ] [193])) -1 (nil))
/vol/gcc/src/gcc-dist/libgcc/../gcc/libgcc2.c:431:1: internal compiler error:
in get_attr_got, at config/mips/mips.md:455

See PR target/41240.

	Rainer

-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [vta->trunk] VTA merge
  2009-09-03  7:07             ` Alexandre Oliva
@ 2009-09-03 11:02               ` Richard Guenther
  0 siblings, 0 replies; 52+ messages in thread
From: Richard Guenther @ 2009-09-03 11:02 UTC (permalink / raw)
  To: Alexandre Oliva; +Cc: gcc-patches, jakub

On Thu, Sep 3, 2009 at 9:07 AM, Alexandre Oliva<aoliva@redhat.com> wrote:
> On Sep  1, 2009, Alexandre Oliva <aoliva@redhat.com> wrote:
>
>> Here's the patch I intend to check in tonight.
>
> In a last-minute (ok, make that last couple of hours ;-) review of the
> patch, I realized tree-ssa-loop-ivopts.c was working too hard, dropping
> uses of an SSA DEF early, instead of letting them be propagated into
> debug stmts by the later-introduced code in release_ssa_name.
>
> This patch reverts that (hopefully) unnecessary change.  Ok to install,
> if it passes regression testing?

Ok.

Can you please post new patches in new threads?  It gets very confusing
and I'm not sure if new patches subsume others in this thread.

Thanks,
Richard.

>
>
>
> --
> Alexandre Oliva, freedom fighter    http://FSFLA.org/~lxoliva/
> You must be the change you wish to see in the world. -- Gandhi
> Be Free! -- http://FSFLA.org/   FSF Latin America board member
> Free Software Evangelist      Red Hat Brazil Compiler Engineer
>
>

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [vta->trunk] VTA merge
@ 2009-09-03  9:27 Uros Bizjak
  2009-09-03 11:11 ` Rainer Orth
  0 siblings, 1 reply; 52+ messages in thread
From: Uros Bizjak @ 2009-09-03  9:27 UTC (permalink / raw)
  To: gcc-patches; +Cc: Rainer Orth, Alexandre Oliva, Dominique Dhumieres

Hello!

> unfortunately, on sparc-sun-solaris2.11 the sparcv9 stage1 libgcc fails to build:

> /vol/gcc/src/gcc-dist/libgcc/../gcc/libgcc2.c: In function '__divtc3':
> /vol/gcc/src/gcc-dist/libgcc/../gcc/libgcc2.c:1948:1: internal compiler error: in loc_cmp, at var-tracking.c:2456

PR 41238.

Uros.

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [vta->trunk] VTA merge
  2009-09-02 17:46               ` Tom Tromey
@ 2009-09-03  7:14                 ` Alexandre Oliva
  2009-09-03 15:36                   ` Tom Tromey
  2009-09-16 15:48                   ` Jakub Jelinek
  0 siblings, 2 replies; 52+ messages in thread
From: Alexandre Oliva @ 2009-09-03  7:14 UTC (permalink / raw)
  To: Tom Tromey
  Cc: Joseph S. Myers, Jack Howarth, Steven Bosscher, Richard Guenther,
	gcc-patches

[-- Attachment #1: Type: text/plain, Size: 717 bytes --]

On Sep  2, 2009, Tom Tromey <tromey@redhat.com> wrote:

> FWIW I changed my mind on this topic.  I was swayed by your argument
> about compatibility: the alternative to emitting DW_OP_*_value seems to
> be to emit nothing.  Either way an older debugger is just not going to
> be able to print the value.

I think the important question is whether older debuggers would be able
to deal gracefully with these location entries or constant values.  I
understand they're supposed to, and I have no evidence that GDB isn't,
so I offer this patch to revert the last-minute addition of tests for
dwarf_version >= 4 in the VTA merge patch.

Should I check it in so that we can get a better idea of what, if
anything, breaks?


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: vta-dwarf2out-values-without-v4.patch --]
[-- Type: text/x-diff, Size: 2124 bytes --]

for  gcc/ChangeLog
from  Alexandre Oliva  <aoliva@redhat.com>

	* dwarf2out.c (loc_descriptor): Emit DW_OP_stack_value and
	DW_OP_implicit_value even without dwarf_version 4.

Index: gcc/dwarf2out.c
===================================================================
--- gcc/dwarf2out.c.orig	2009-09-03 04:09:03.000000000 -0300
+++ gcc/dwarf2out.c	2009-09-03 04:09:21.000000000 -0300
@@ -11744,7 +11744,7 @@ loc_descriptor (rtx rtl, enum machine_mo
       break;
 
     case CONST_INT:
-      if (mode != VOIDmode && mode != BLKmode && dwarf_version >= 4)
+      if (mode != VOIDmode && mode != BLKmode)
         {
           HOST_WIDE_INT i = INTVAL (rtl);
           int litsize;
@@ -11798,7 +11798,7 @@ loc_descriptor (rtx rtl, enum machine_mo
       break;
 
     case CONST_DOUBLE:
-      if (mode != VOIDmode && dwarf_version >= 4)
+      if (mode != VOIDmode)
 	{
 	  /* Note that a CONST_DOUBLE rtx could represent either an integer
 	     or a floating-point constant.  A CONST_DOUBLE is used whenever
@@ -11829,7 +11829,7 @@ loc_descriptor (rtx rtl, enum machine_mo
       break;
 
     case CONST_VECTOR:
-      if (mode != VOIDmode && dwarf_version >= 4)
+      if (mode != VOIDmode)
 	{
 	  unsigned int elt_size = GET_MODE_UNIT_SIZE (GET_MODE (rtl));
 	  unsigned int length = CONST_VECTOR_NUNITS (rtl);
@@ -11917,8 +11917,7 @@ loc_descriptor (rtx rtl, enum machine_mo
 	  && SYMBOL_REF_TLS_MODEL (rtl) != TLS_MODEL_NONE)
 	break;
     case LABEL_REF:
-      if (mode != VOIDmode && GET_MODE_SIZE (mode) == DWARF2_ADDR_SIZE
-	  && dwarf_version >= 4)
+      if (mode != VOIDmode && GET_MODE_SIZE (mode) == DWARF2_ADDR_SIZE)
 	{
 	  loc_result = new_loc_descr (DW_OP_implicit_value,
 				      DWARF2_ADDR_SIZE, 0);
@@ -11930,8 +11929,7 @@ loc_descriptor (rtx rtl, enum machine_mo
 
     default:
       if (GET_MODE_CLASS (mode) == MODE_INT && GET_MODE (rtl) == mode
-	  && GET_MODE_SIZE (GET_MODE (rtl)) <= DWARF2_ADDR_SIZE
-	  && dwarf_version >= 4)
+	  && GET_MODE_SIZE (GET_MODE (rtl)) <= DWARF2_ADDR_SIZE)
 	{
 	  /* Value expression.  */
 	  loc_result = mem_loc_descriptor (rtl, VOIDmode, initialized);

[-- Attachment #3: Type: text/plain, Size: 258 bytes --]



-- 
Alexandre Oliva, freedom fighter    http://FSFLA.org/~lxoliva/
You must be the change you wish to see in the world. -- Gandhi
Be Free! -- http://FSFLA.org/   FSF Latin America board member
Free Software Evangelist      Red Hat Brazil Compiler Engineer

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [vta->trunk] VTA merge
  2009-09-01 18:25           ` Alexandre Oliva
  2009-09-01 20:09             ` Joseph S. Myers
@ 2009-09-03  7:07             ` Alexandre Oliva
  2009-09-03 11:02               ` Richard Guenther
  1 sibling, 1 reply; 52+ messages in thread
From: Alexandre Oliva @ 2009-09-03  7:07 UTC (permalink / raw)
  To: Richard Guenther, gcc-patches, jakub

[-- Attachment #1: Type: text/plain, Size: 492 bytes --]

On Sep  1, 2009, Alexandre Oliva <aoliva@redhat.com> wrote:

> Here's the patch I intend to check in tonight.

In a last-minute (ok, make that last couple of hours ;-) review of the
patch, I realized tree-ssa-loop-ivopts.c was working too hard, dropping
uses of an SSA DEF early, instead of letting them be propagated into
debug stmts by the later-introduced code in release_ssa_name.

This patch reverts that (hopefully) unnecessary change.  Ok to install,
if it passes regression testing?


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: vta-loop-ivopts-propagate-on-release.patch --]
[-- Type: text/x-diff, Size: 1065 bytes --]

for  gcc/ChangeLog
from  Alexandre Oliva  <aoliva@redhat.com>

	* tree-ssa-loop-ivopts.c (remove_unused_ivs): Let release_ssa_name
	propagate removed DEFs into debug uses.

Index: gcc/tree-ssa-loop-ivopts.c
===================================================================
--- gcc/tree-ssa-loop-ivopts.c.orig	2009-09-01 23:40:27.000000000 -0300
+++ gcc/tree-ssa-loop-ivopts.c	2009-09-03 03:02:45.000000000 -0300
@@ -5622,24 +5622,7 @@ remove_unused_ivs (struct ivopts_data *d
 	  && !info->inv_id
 	  && !info->iv->have_use_for
 	  && !info->preserve_biv)
-	{
-	  if (MAY_HAVE_DEBUG_STMTS)
-	    {
-	      gimple stmt;
-	      imm_use_iterator iter;
-
-	      FOR_EACH_IMM_USE_STMT (stmt, iter, info->iv->ssa_name)
-		{
-		  if (!gimple_debug_bind_p (stmt))
-		    continue;
-
-		  /* ??? We can probably do better than this.  */
-		  gimple_debug_bind_reset_value (stmt);
-		  update_stmt (stmt);
-		}
-	    }
-	  remove_statement (SSA_NAME_DEF_STMT (info->iv->ssa_name), true);
-	}
+	remove_statement (SSA_NAME_DEF_STMT (info->iv->ssa_name), true);
     }
 }
 

[-- Attachment #3: Type: text/plain, Size: 258 bytes --]



-- 
Alexandre Oliva, freedom fighter    http://FSFLA.org/~lxoliva/
You must be the change you wish to see in the world. -- Gandhi
Be Free! -- http://FSFLA.org/   FSF Latin America board member
Free Software Evangelist      Red Hat Brazil Compiler Engineer

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [vta->trunk] VTA merge
  2009-09-02 14:11                   ` H.J. Lu
@ 2009-09-03  6:50                     ` Alexandre Oliva
  0 siblings, 0 replies; 52+ messages in thread
From: Alexandre Oliva @ 2009-09-03  6:50 UTC (permalink / raw)
  To: H.J. Lu, Joseph S. Myers
  Cc: NightStrike, Richard Guenther, gcc-patches, jakub

[-- Attachment #1: Type: text/plain, Size: 619 bytes --]

On Sep  2, 2009, "H.J. Lu" <hjl.tools@gmail.com> wrote:

> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=41226

I'm testing the first patch below to fix the warnings about conversion
of pointer types to integral types of different width.  Joseph, I also
fixed (I hope) the unixisms you pointed out.  I hope I caught them all,
save for the expectation of concurrency between GDB and the program
under test.

I'm also testing the second patch below to fix the regression on the
pr40753.c testcase.  I've already tested it, but without the patch that
enabled VTA by default.

Are these ok to install if they pass?

Thanks,


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: vta-guality-silence-warning.patch --]
[-- Type: text/x-diff, Size: 4308 bytes --]

for gcc/testsuite/ChangeLog
from  Alexandre Oliva  <aoliva@redhat.com>

	* gcc.dg/guality/guality.h: Include stdint.h.  Drop unnecessary
	unistd.h, sys/types.h and sys/wait.h.
	(gualchk_t): New.
	(GUALCVT): New.
	(GUALCHKXPR, GUALCHKVAL, GUALCHKFLA): Use it.
	(GUALITY_GDB_REDIRECT): New.
	(GUALITY_GDB_ARGS): Use it.

Index: gcc/testsuite/gcc.dg/guality/guality.h
===================================================================
--- gcc/testsuite/gcc.dg/guality/guality.h.orig	2009-09-03 03:38:04.000000000 -0300
+++ gcc/testsuite/gcc.dg/guality/guality.h	2009-09-03 03:40:44.000000000 -0300
@@ -21,9 +21,7 @@ along with GCC; see the file COPYING3.  
 #include <stdio.h>
 #include <stdlib.h>
 #include <string.h>
-#include <unistd.h>
-#include <sys/types.h>
-#include <sys/wait.h>
+#include <stdint.h>
 
 /* This is a first cut at checking that debug information matches
    run-time.  The idea is to annotate programs with GUALCHK* macros
@@ -56,8 +54,20 @@ along with GCC; see the file COPYING3.  
    so that __FILE__ and __LINE__ will be usable to identify them.
 */
 
+/* This is the type we use to pass values to guality_check.  */
+
+typedef intmax_t gualchk_t;
+
+/* Convert a pointer or integral type to the widest integral type,
+   as expected by guality_check.  */
+
+#define GUALCVT(val)						\
+  ((gualchk_t)__builtin_choose_expr				\
+   (__builtin_types_compatible_p (__typeof (val), gualchk_t),	\
+    (val), (intptr_t)(val)))
+
 /* Attach a debugger to the current process and verify that the string
-   EXPR, evaluated by the debugger, yields the long long number VAL.
+   EXPR, evaluated by the debugger, yields the gualchk_t number VAL.
    If the debugger cannot compute the expression, say because the
    variable is unavailable, this will count as an error, unless unkok
    is nonzero.  */
@@ -73,13 +83,13 @@ along with GCC; see the file COPYING3.  
    guality_check, although not necessarily after the call.  */
 
 #define GUALCHKXPR(expr) \
-  GUALCHKXPRVAL (#expr, (long long)(expr), 1)
+  GUALCHKXPRVAL (#expr, GUALCVT (expr), 1)
 
 /* Same as GUALCHKXPR, but issue an error if the variable is optimized
    away.  */
 
 #define GUALCHKVAL(expr) \
-  GUALCHKXPRVAL (#expr, (long long)(expr), 0)
+  GUALCHKXPRVAL (#expr, GUALCVT (expr), 0)
 
 /* Check that a debugger knows that EXPR evaluates to the run-time
    value of EXPR.  Unknown values are marked as errors, because the
@@ -91,7 +101,7 @@ along with GCC; see the file COPYING3.  
 #define GUALCHKFLA(expr) do {					\
     __typeof(expr) volatile __preserve_after;			\
     __typeof(expr) __preserve_before = (expr);			\
-    GUALCHKXPRVAL (#expr, (long long)(__preserve_before), 0);	\
+    GUALCHKXPRVAL (#expr, GUALCVT (__preserve_before), 0);	\
     __preserve_after = __preserve_before;			\
     asm ("" : : "m" (__preserve_after));			\
   } while (0)
@@ -119,7 +129,14 @@ along with GCC; see the file COPYING3.  
 
 static const char *guality_gdb_command;
 #define GUALITY_GDB_DEFAULT "gdb"
-#define GUALITY_GDB_ARGS " -nx -nw --quiet > /dev/null 2>&1"
+#if defined(__unix)
+# define GUALITY_GDB_REDIRECT " > /dev/null 2>&1"
+#elif defined (_WIN32) || defined (MSDOS)
+# define GUALITY_GDB_REDIRECT " > nul"
+#else
+# define GUALITY_GDB_REDRECT ""
+#endif
+#define GUALITY_GDB_ARGS " -nx -nw --quiet" GUALITY_GDB_REDIRECT
 
 /* Kinds of results communicated as exit status from child process
    that runs gdb to the parent process that's being monitored.  */
@@ -155,7 +172,7 @@ int volatile guality_attached;
 extern int guality_main (int argc, char *argv[]);
 
 static void __attribute__((noinline))
-guality_check (const char *name, long long value, int unknown_ok);
+guality_check (const char *name, gualchk_t value, int unknown_ok);
 
 /* Set things up, run guality_main, then print a summary and quit.  */
 
@@ -228,7 +245,7 @@ continue\n\
    have an UNRESOLVED.  Otherwise, it's a FAIL.  */
 
 static void __attribute__((noinline))
-guality_check (const char *name, long long value, int unknown_ok)
+guality_check (const char *name, gualchk_t value, int unknown_ok)
 {
   int result;
 
@@ -236,7 +253,7 @@ guality_check (const char *name, long lo
     return;
 
   {
-    volatile long long xvalue = -1;
+    volatile gualchk_t xvalue = -1;
     volatile int unavailable = 0;
     if (name)
       {

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #3: vta-unregress-pr40753.patch --]
[-- Type: text/x-diff, Size: 2051 bytes --]

for  gcc/ChangeLog
from  Alexandre Oliva  <aoliva@redhat.com>

	* var-tracking.c (dv_is_decl_p): Adjust NULL behavior to match
	comment.  Use switch statement to catch overlaps between rtx
	and tree codes.  Accept FUNCTION_DECLs in addition to those in...
	(IS_DECL_CODE): ... here. Remove.
	(check_value_is_not_decl): Remove.
	(dv_from_decl, dv_from_value): Check after conversion.

Index: gcc/var-tracking.c
===================================================================
--- gcc/var-tracking.c.orig	2009-09-02 11:46:43.000000000 -0300
+++ gcc/var-tracking.c	2009-09-02 12:11:55.000000000 -0300
@@ -723,12 +723,24 @@ static inline bool
 dv_is_decl_p (decl_or_value dv)
 {
   if (!dv)
-    return false;
+    return true;
 
-  if (GET_CODE ((rtx)dv) == VALUE)
-    return false;
+  /* Make sure relevant codes don't overlap.  */
+  switch ((int)TREE_CODE ((tree)dv))
+    {
+    case (int)VAR_DECL:
+    case (int)PARM_DECL:
+    case (int)RESULT_DECL:
+    case (int)FUNCTION_DECL:
+    case (int)COMPONENT_REF:
+      return true;
 
-  return true;
+    case (int)VALUE:
+      return false;
+
+    default:
+      gcc_unreachable ();
+    }
 }
 
 /* Return true if a decl_or_value is a VALUE rtl.  */
@@ -790,21 +802,13 @@ dv_pool (decl_or_value dv)
   return dv_onepart_p (dv) ? valvar_pool : var_pool;
 }
 
-#define IS_DECL_CODE(C) ((C) == VAR_DECL || (C) == PARM_DECL \
-			 || (C) == RESULT_DECL || (C) == COMPONENT_REF)
-
-/* Check that VALUE won't ever look like a DECL.  */
-static char check_value_is_not_decl [(!IS_DECL_CODE ((enum tree_code)VALUE))
-				     ? 1 : -1] ATTRIBUTE_UNUSED;
-
-
 /* Build a decl_or_value out of a decl.  */
 static inline decl_or_value
 dv_from_decl (tree decl)
 {
   decl_or_value dv;
-  gcc_assert (!decl || IS_DECL_CODE (TREE_CODE (decl)));
   dv = decl;
+  gcc_assert (dv_is_decl_p (dv));
   return dv;
 }
 
@@ -813,8 +817,8 @@ static inline decl_or_value
 dv_from_value (rtx value)
 {
   decl_or_value dv;
-  gcc_assert (value);
   dv = value;
+  gcc_assert (dv_is_value_p (dv));
   return dv;
 }
 

[-- Attachment #4: Type: text/plain, Size: 257 bytes --]


-- 
Alexandre Oliva, freedom fighter    http://FSFLA.org/~lxoliva/
You must be the change you wish to see in the world. -- Gandhi
Be Free! -- http://FSFLA.org/   FSF Latin America board member
Free Software Evangelist      Red Hat Brazil Compiler Engineer

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [vta->trunk] VTA merge
  2009-09-01 21:17             ` Alexandre Oliva
@ 2009-09-02 17:46               ` Tom Tromey
  2009-09-03  7:14                 ` Alexandre Oliva
  0 siblings, 1 reply; 52+ messages in thread
From: Tom Tromey @ 2009-09-02 17:46 UTC (permalink / raw)
  To: Alexandre Oliva
  Cc: Joseph S. Myers, Jack Howarth, Steven Bosscher, Richard Guenther,
	gcc-patches

>>>>> "Alexandre" == Alexandre Oliva <aoliva@redhat.com> writes:

Alexandre> I actually thought DW_OP_stack_value and DW_OP_implicit_value
Alexandre> might be somehow related with this, which is why I disabled
Alexandre> their use for dwarf_version < 4, but investigation showed it
Alexandre> to be unrelated, but I'll leave the decision on whether to
Alexandre> retain this restriction for someone else to make.

FWIW I changed my mind on this topic.  I was swayed by your argument
about compatibility: the alternative to emitting DW_OP_*_value seems to
be to emit nothing.  Either way an older debugger is just not going to
be able to print the value.

Tom

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [vta->trunk] VTA merge
  2009-09-02 17:17                     ` Jakub Jelinek
@ 2009-09-02 17:41                       ` Jakub Jelinek
  0 siblings, 0 replies; 52+ messages in thread
From: Jakub Jelinek @ 2009-09-02 17:41 UTC (permalink / raw)
  To: Alexandre Oliva
  Cc: Joseph S. Myers, Janis Johnson, NightStrike, Richard Guenther,
	gcc-patches

On Wed, Sep 02, 2009 at 07:17:25PM +0200, Jakub Jelinek wrote:
> And here is a more fancy one which also executes it and checks if global
> variable can be verified.  Perhaps the first
> check_no_compiler_messages_nocache is redundant now and perhaps I should
> check that [string match "" $lines] is 1 before actually calling gcc_load
> on it.

Simplified version:

2009-09-02  Jakub Jelinek  <jakub@redhat.com>

	* gcc.dg/guality/guality.exp: Only run guality tests if a trivial
	testcase using guality.h compiles and links and if a global variable
	can be verified by gdb.

--- gcc/testsuite/gcc.dg/guality/guality.exp.jj	2009-09-02 08:29:54.000000000 +0200
+++ gcc/testsuite/gcc.dg/guality/guality.exp	2009-09-02 19:37:49.000000000 +0200
@@ -2,6 +2,30 @@
 
 load_lib gcc-dg.exp
 
+proc check_guality {args} {
+    set result [eval check_compile guality_check executable $args "-g -O0"]
+    set lines [lindex $result 0]
+    set output [lindex $result 1]
+    set ret 0
+    if {[string match "" $lines]} {
+      set execout [gcc_load "./$output"]
+      set ret [string match "*1 PASS, 0 FAIL, 0 UNRESOLVED*" $execout]
+    }
+    remote_file build delete $output
+    return $ret
+}
+
 dg-init
-gcc-dg-runtest [lsort [glob $srcdir/$subdir/*.c]] ""
+
+if {[check_guality "
+  #include \"$srcdir/$subdir/guality.h\"
+  volatile long int varl = 6;
+  int main (int argc, char *argv\[\])
+  {
+    GUALCHKVAL (varl);
+    return 0;
+  }
+"]} {
+  gcc-dg-runtest [lsort [glob $srcdir/$subdir/*.c]] ""
+}
 dg-finish


	Jakub

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [vta->trunk] VTA merge
  2009-09-02 16:34                   ` Jakub Jelinek
@ 2009-09-02 17:17                     ` Jakub Jelinek
  2009-09-02 17:41                       ` Jakub Jelinek
  0 siblings, 1 reply; 52+ messages in thread
From: Jakub Jelinek @ 2009-09-02 17:17 UTC (permalink / raw)
  To: Alexandre Oliva
  Cc: Joseph S. Myers, Janis Johnson, NightStrike, Richard Guenther,
	gcc-patches

On Wed, Sep 02, 2009 at 06:33:39PM +0200, Jakub Jelinek wrote:
> On Tue, Sep 01, 2009 at 09:40:26PM +0000, Joseph S. Myers wrote:
> > On Tue, 1 Sep 2009, Alexandre Oliva wrote:
> > 
> > > > (along with various other Unixisms 
> > > > in the testcase that may not even work on all supported native systems), 
> > > 
> > > Now that's surprising to me.  What is it that you're referring to?
> > 
> > sys/wait.h, used in guality.h, does not exist for MinGW, which is a 
> > supported native system on the list of secondary platforms for 4.5.  "> 
> > /dev/null 2>&1" is Unix syntax; the null device is NUL on Windows.  Some 
> > other things I thought would be Unixisms (such as getpid) do seem to be 
> > present on MinGW after all, though I don't know about any other non-Unix 
> > native systems GCC may support (VMS?).
> 
> That can be partily handled by something like:

And here is a more fancy one which also executes it and checks if global
variable can be verified.  Perhaps the first
check_no_compiler_messages_nocache is redundant now and perhaps I should
check that [string match "" $lines] is 1 before actually calling gcc_load
on it.

2009-09-02  Jakub Jelinek  <jakub@redhat.com>

	* gcc.dg/guality/guality.exp: Only run guality tests if a trivial
	testcase using guality.h compiles and links and if a global variable
	can be verified by gdb.

--- gcc/testsuite/gcc.dg/guality/guality.exp.jj	2009-09-02 08:29:54.000000000 +0200
+++ gcc/testsuite/gcc.dg/guality/guality.exp	2009-09-02 19:10:54.000000000 +0200
@@ -2,6 +2,29 @@
 
 load_lib gcc-dg.exp
 
+proc check_guality {args} {
+    set result [eval check_compile guality_check executable $args "-g -O0"]
+    set lines [lindex $result 0]
+    set output [lindex $result 1]
+    set execout [gcc_load "./$output"]
+    remote_file build delete $output
+    return [string match "" $lines] && [string match "*1 PASS, 0 FAIL, 0 UNRESOLVED*" $execout]
+}
+
 dg-init
-gcc-dg-runtest [lsort [glob $srcdir/$subdir/*.c]] ""
+
+if { [check_no_compiler_messages_nocache guality_trivial executable "
+  #include \"$srcdir/$subdir/guality.h\"
+  int main (int argc, char *argv\[\]) { return 0; }
+"] && [check_guality "
+  #include \"$srcdir/$subdir/guality.h\"
+  volatile long int varl = 6;
+  int main (int argc, char *argv\[\])
+  {
+    GUALCHKVAL (varl);
+    return 0;
+  }
+"]} {
+  gcc-dg-runtest [lsort [glob $srcdir/$subdir/*.c]] ""
+}
 dg-finish


	Jakub

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [vta->trunk] VTA merge
  2009-09-01 21:40                 ` Joseph S. Myers
@ 2009-09-02 16:34                   ` Jakub Jelinek
  2009-09-02 17:17                     ` Jakub Jelinek
  0 siblings, 1 reply; 52+ messages in thread
From: Jakub Jelinek @ 2009-09-02 16:34 UTC (permalink / raw)
  To: Joseph S. Myers, Janis Johnson
  Cc: Alexandre Oliva, NightStrike, Richard Guenther, gcc-patches

On Tue, Sep 01, 2009 at 09:40:26PM +0000, Joseph S. Myers wrote:
> On Tue, 1 Sep 2009, Alexandre Oliva wrote:
> 
> > > (along with various other Unixisms 
> > > in the testcase that may not even work on all supported native systems), 
> > 
> > Now that's surprising to me.  What is it that you're referring to?
> 
> sys/wait.h, used in guality.h, does not exist for MinGW, which is a 
> supported native system on the list of secondary platforms for 4.5.  "> 
> /dev/null 2>&1" is Unix syntax; the null device is NUL on Windows.  Some 
> other things I thought would be Unixisms (such as getpid) do seem to be 
> present on MinGW after all, though I don't know about any other non-Unix 
> native systems GCC may support (VMS?).

That can be partily handled by something like:

2009-09-02  Jakub Jelinek  <jakub@redhat.com>

	* gcc.dg/guality/guality.exp: Only run guality tests if a trivial
	testcase using guality.h compiles and links.

--- gcc/testsuite/gcc.dg/guality/guality.exp.jj	2009-09-02 08:29:54.000000000 +0200
+++ gcc/testsuite/gcc.dg/guality/guality.exp	2009-09-02 18:22:11.000000000 +0200
@@ -3,5 +3,11 @@
 load_lib gcc-dg.exp
 
 dg-init
-gcc-dg-runtest [lsort [glob $srcdir/$subdir/*.c]] ""
+
+if { [check_no_compiler_messages_nocache guality_trivial executable "
+  #include \"$srcdir/$subdir/guality.h\"
+  int main (int argc, char *argv\[\]) { return 0; }
+"] } {
+  gcc-dg-runtest [lsort [glob $srcdir/$subdir/*.c]] ""
+}
 dg-finish

(i.e. don't run any of the tests at all if we can't compile/link a trivial
program using guality.h (i.e. popen isn't available, sys/wait.h, ...)).
As a second check we could perhaps
check_compile similar trivial program which would try e.g. verify a value
of a global variable using GDB, then execute it on the target and check if
it passed and only if it passed run the normal VTA tests.

	Jakub

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [vta->trunk] VTA merge
  2009-09-02  2:50                 ` Alexandre Oliva
@ 2009-09-02 14:11                   ` H.J. Lu
  2009-09-03  6:50                     ` Alexandre Oliva
  0 siblings, 1 reply; 52+ messages in thread
From: H.J. Lu @ 2009-09-02 14:11 UTC (permalink / raw)
  To: Alexandre Oliva
  Cc: Joseph S. Myers, NightStrike, Richard Guenther, gcc-patches, jakub

On Tue, Sep 1, 2009 at 7:49 PM, Alexandre Oliva<aoliva@redhat.com> wrote:
> On Sep  1, 2009, Alexandre Oliva <aoliva@redhat.com> wrote:
>
>> On Sep  1, 2009, "Joseph S. Myers" <joseph@codesourcery.com> wrote:
>>> you need to make guality.exp not try to run any tests for non-native
>>> testing,
>
>> Sorry, I was pretty sure I'd already done that, but it's obvious I
>> haven't.  This is bad.  I'll prepare a follow up patch to address this,
>> and post it for approval before I install the merge patch.
>
> Given your considerations about isnative, that I don't quite follow yet,
> and the fact that with VTA disabled by default, the tests can't possibly
> pass, I'm installing this patch for the time being.
>
>

This caused:

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=41226


-- 
H.J.

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [vta->trunk] VTA merge
  2009-09-02  2:43   ` Alexandre Oliva
@ 2009-09-02 11:37     ` Paolo Bonzini
  0 siblings, 0 replies; 52+ messages in thread
From: Paolo Bonzini @ 2009-09-02 11:37 UTC (permalink / raw)
  To: Alexandre Oliva; +Cc: Richard Guenther, gcc-patches

On 09/02/2009 04:43 AM, Alexandre Oliva wrote:
> On Aug 30, 2009, Richard Guenther<richard.guenther@gmail.com>  wrote:
>
>> You need (if you didn't already get) approval for the C++ parts,
>> the testsuite parts, the build system changes and the RTL
>> middle-end pieces.
>
> I'm a maintainer for the build machinery, so I'm going ahead without
> explicit approval for them.

Yeah, of course you don't need it.

Paolo

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [vta->trunk] VTA merge
  2009-09-01 21:25               ` Alexandre Oliva
  2009-09-01 21:40                 ` Joseph S. Myers
@ 2009-09-02  2:50                 ` Alexandre Oliva
  2009-09-02 14:11                   ` H.J. Lu
  1 sibling, 1 reply; 52+ messages in thread
From: Alexandre Oliva @ 2009-09-02  2:50 UTC (permalink / raw)
  To: Joseph S. Myers; +Cc: NightStrike, Richard Guenther, gcc-patches, jakub

[-- Attachment #1: Type: text/plain, Size: 620 bytes --]

On Sep  1, 2009, Alexandre Oliva <aoliva@redhat.com> wrote:

> On Sep  1, 2009, "Joseph S. Myers" <joseph@codesourcery.com> wrote:
>> you need to make guality.exp not try to run any tests for non-native
>> testing,

> Sorry, I was pretty sure I'd already done that, but it's obvious I
> haven't.  This is bad.  I'll prepare a follow up patch to address this,
> and post it for approval before I install the merge patch.

Given your considerations about isnative, that I don't quite follow yet,
and the fact that with VTA disabled by default, the tests can't possibly
pass, I'm installing this patch for the time being.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: vta-testsuite-xfail.patch --]
[-- Type: text/x-diff, Size: 990 bytes --]

for  gcc/testsuite/ChangeLog
from  Alexandre Oliva  <aoliva@redhat.com>

	* gcc.dg/guality/guality.c: Expect to fail for now.
	* gcc.dg/guality/example.c: Likewise.

Index: gcc/testsuite/gcc.dg/guality/example.c
===================================================================
--- gcc/testsuite/gcc.dg/guality/example.c.orig	2009-09-01 23:45:50.000000000 -0300
+++ gcc/testsuite/gcc.dg/guality/example.c	2009-09-01 23:46:09.000000000 -0300
@@ -1,4 +1,4 @@
-/* { dg-do run } */
+/* { dg-do run { xfail *-*-* } } */
 /* { dg-options "-g" } */
 
 #define GUALITY_DONT_FORCE_LIVE_AFTER -1
Index: gcc/testsuite/gcc.dg/guality/guality.c
===================================================================
--- gcc/testsuite/gcc.dg/guality/guality.c.orig	2009-09-01 23:45:50.000000000 -0300
+++ gcc/testsuite/gcc.dg/guality/guality.c	2009-09-01 23:45:55.000000000 -0300
@@ -1,4 +1,4 @@
-/* { dg-do run } */
+/* { dg-do run { xfail *-*-* } } */
 /* { dg-options "-g" } */
 
 #include "guality.h"

[-- Attachment #3: Type: text/plain, Size: 257 bytes --]


-- 
Alexandre Oliva, freedom fighter    http://FSFLA.org/~lxoliva/
You must be the change you wish to see in the world. -- Gandhi
Be Free! -- http://FSFLA.org/   FSF Latin America board member
Free Software Evangelist      Red Hat Brazil Compiler Engineer

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [vta->trunk] VTA merge
  2009-08-30 17:18 ` Richard Guenther
  2009-08-30 20:44   ` Tom Tromey
  2009-08-31 16:39   ` Alexandre Oliva
@ 2009-09-02  2:43   ` Alexandre Oliva
  2009-09-02 11:37     ` Paolo Bonzini
  2 siblings, 1 reply; 52+ messages in thread
From: Alexandre Oliva @ 2009-09-02  2:43 UTC (permalink / raw)
  To: Richard Guenther; +Cc: gcc-patches

On Aug 30, 2009, Richard Guenther <richard.guenther@gmail.com> wrote:

> You need (if you didn't already get) approval for the C++ parts,
> the testsuite parts, the build system changes and the RTL
> middle-end pieces.

I'm a maintainer for the build machinery, so I'm going ahead without
explicit approval for them.  RTH approved all of the other bits
mentioned above, although some of them were approved in private replies
or IRC.

We've been frozen for 16 hours now, so I'm going ahead and checking in
the vta patchset.

I'll wait another 24 hours before installing the patch that enables VTA
by default.

-- 
Alexandre Oliva, freedom fighter    http://FSFLA.org/~lxoliva/
You must be the change you wish to see in the world. -- Gandhi
Be Free! -- http://FSFLA.org/   FSF Latin America board member
Free Software Evangelist      Red Hat Brazil Compiler Engineer

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [vta->trunk] VTA merge
  2009-09-01 21:25               ` Alexandre Oliva
@ 2009-09-01 21:40                 ` Joseph S. Myers
  2009-09-02 16:34                   ` Jakub Jelinek
  2009-09-02  2:50                 ` Alexandre Oliva
  1 sibling, 1 reply; 52+ messages in thread
From: Joseph S. Myers @ 2009-09-01 21:40 UTC (permalink / raw)
  To: Alexandre Oliva; +Cc: NightStrike, Richard Guenther, gcc-patches, jakub

On Tue, 1 Sep 2009, Alexandre Oliva wrote:

> > (along with various other Unixisms 
> > in the testcase that may not even work on all supported native systems), 
> 
> Now that's surprising to me.  What is it that you're referring to?

sys/wait.h, used in guality.h, does not exist for MinGW, which is a 
supported native system on the list of secondary platforms for 4.5.  "> 
/dev/null 2>&1" is Unix syntax; the null device is NUL on Windows.  Some 
other things I thought would be Unixisms (such as getpid) do seem to be 
present on MinGW after all, though I don't know about any other non-Unix 
native systems GCC may support (VMS?).

> > you need to make guality.exp not try to run any tests for non-native
> > testing,
> 
> Sorry, I was pretty sure I'd already done that, but it's obvious I
> haven't.  This is bad.  I'll prepare a follow up patch to address this,
> and post it for approval before I install the merge patch.

Note that DejaGnu's "isnative" is not the right thing here, since that 
tests whether build == target, and what's relevant in this case is whether 
host == target (I see no reason the existing tests should not work for 
testing the case build != host == target).

-- 
Joseph S. Myers
joseph@codesourcery.com

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [vta->trunk] VTA merge
  2009-09-01 20:09             ` Joseph S. Myers
@ 2009-09-01 21:25               ` Alexandre Oliva
  2009-09-01 21:40                 ` Joseph S. Myers
  2009-09-02  2:50                 ` Alexandre Oliva
  0 siblings, 2 replies; 52+ messages in thread
From: Alexandre Oliva @ 2009-09-01 21:25 UTC (permalink / raw)
  To: Joseph S. Myers; +Cc: NightStrike, Richard Guenther, gcc-patches, jakub

On Sep  1, 2009, "Joseph S. Myers" <joseph@codesourcery.com> wrote:

> On Tue, 1 Sep 2009, Alexandre Oliva wrote:
>> Here's the patch I intend to check in tonight.  It is the combination of

> Since you haven't fixed the previously discussed testsuite problems with 
> the testsuite assuming it can create a GDB process (on the host) with 
> popen from a testcase (on the target)

It actually tries to run GDB on the target, but I get your point.
That's a known limitation of the current testing infrastructure, that I
hope can be addressed in a followup patch.  Sorry that this hasn't been
very high priority to me.  I have still been working on features and
maintaining the branch up to date.  I'll now have more time to complete
the features and address this issue.

> (along with various other Unixisms 
> in the testcase that may not even work on all supported native systems), 

Now that's surprising to me.  What is it that you're referring to?

> you need to make guality.exp not try to run any tests for non-native
> testing,

Sorry, I was pretty sure I'd already done that, but it's obvious I
haven't.  This is bad.  I'll prepare a follow up patch to address this,
and post it for approval before I install the merge patch.

Thanks for reminding me of these important issues.

-- 
Alexandre Oliva, freedom fighter    http://FSFLA.org/~lxoliva/
You must be the change you wish to see in the world. -- Gandhi
Be Free! -- http://FSFLA.org/   FSF Latin America board member
Free Software Evangelist      Red Hat Brazil Compiler Engineer

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [vta->trunk] VTA merge
  2009-08-31  6:41           ` Joseph S. Myers
@ 2009-09-01 21:17             ` Alexandre Oliva
  2009-09-02 17:46               ` Tom Tromey
  0 siblings, 1 reply; 52+ messages in thread
From: Alexandre Oliva @ 2009-09-01 21:17 UTC (permalink / raw)
  To: Joseph S. Myers
  Cc: Jack Howarth, Steven Bosscher, Tom Tromey, Richard Guenther, gcc-patches

On Aug 30, 2009, "Joseph S. Myers" <joseph@codesourcery.com> wrote:

> It seems likely that GDB 7.0 will be out well before GCC 4.5 and could
> be a suitable GDB version to specify.

And, in case it isn't, GDB 6.8 needs these two patches in order to deal
with code generated by the GCC trunk today, before the VTA merge:

- the patch in http://sourceware.org/bugzilla/show_bug.cgi?id=10275 for
  DW_CFA_restore_state

- http://sourceware.org/ml/gdb-patches/2009-06/msg00247.html for
  DW_LNE_set_discriminator

This means that anyone wishing to test VTA for optimized programs in the
trunk will need a GDB with these two patches for a reasonable debugging
experience.  Without them, you get broken stack traces and unrelated
line numbers.

I actually thought DW_OP_stack_value and DW_OP_implicit_value might be
somehow related with this, which is why I disabled their use for
dwarf_version < 4, but investigation showed it to be unrelated, but I'll
leave the decision on whether to retain this restriction for someone
else to make.

Both problems are fixed in the GDB trunk.  I thank Jan Kratochvil for
tracking down the two patches required to make GDB 6.8 work with GCC
trunk, before or after VTA.

-- 
Alexandre Oliva, freedom fighter    http://FSFLA.org/~lxoliva/
You must be the change you wish to see in the world. -- Gandhi
Be Free! -- http://FSFLA.org/   FSF Latin America board member
Free Software Evangelist      Red Hat Brazil Compiler Engineer

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [vta->trunk] VTA merge
  2009-09-01 18:25           ` Alexandre Oliva
@ 2009-09-01 20:09             ` Joseph S. Myers
  2009-09-01 21:25               ` Alexandre Oliva
  2009-09-03  7:07             ` Alexandre Oliva
  1 sibling, 1 reply; 52+ messages in thread
From: Joseph S. Myers @ 2009-09-01 20:09 UTC (permalink / raw)
  To: Alexandre Oliva; +Cc: NightStrike, Richard Guenther, gcc-patches, jakub

On Tue, 1 Sep 2009, Alexandre Oliva wrote:

> Here's the patch I intend to check in tonight.  It is the combination of

Since you haven't fixed the previously discussed testsuite problems with 
the testsuite assuming it can create a GDB process (on the host) with 
popen from a testcase (on the target) (along with various other Unixisms 
in the testcase that may not even work on all supported native systems), 
instead of using DejaGnu's interfaces to start GDB on the host (the 
precise GDB command being determined by site.exp), you need to make 
guality.exp not try to run any tests for non-native testing, and probably 
for native MinGW as well, to avoid introducing spurious test failures.

-- 
Joseph S. Myers
joseph@codesourcery.com

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [vta->trunk] VTA merge
  2009-09-01 15:16         ` NightStrike
@ 2009-09-01 18:25           ` Alexandre Oliva
  2009-09-01 20:09             ` Joseph S. Myers
  2009-09-03  7:07             ` Alexandre Oliva
  0 siblings, 2 replies; 52+ messages in thread
From: Alexandre Oliva @ 2009-09-01 18:25 UTC (permalink / raw)
  To: NightStrike; +Cc: Richard Guenther, gcc-patches, jakub

[-- Attachment #1: Type: text/plain, Size: 1138 bytes --]

On Sep  1, 2009, NightStrike <nightstrike@gmail.com> wrote:

> On Mon, Aug 31, 2009 at 6:30 PM, Alexandre Oliva<aoliva@redhat.com> wrote:
>> How does this patch, on top of vta-patchset.patch, look?  (ChangeLog
>> still pending) It has just finished building stage1 on x86_64-linux-gnu,
>> but it's otherwise untested.

> Well, your original patchset doesn't apply cleanly.

True.  I suppose the extraneous i got there by accident (I wasn't used
to the keyboard/touchpad combination of the Lemote Yeeloong), and
the rejected portions of fwprop.c had already been installed
independently.

> So, can you provide a complete patch that I can test on mingw
> targets?

Here's the patch I intend to check in tonight.  It is the combination of
the original (and fixed) vta-patchset.patch.bz2,
vta-patchset-cleanup.patch, and the vta-cfgexpand-assign-rhs-const.patch
that was initially rejected, then ended up being preferred over the
suggested alternative.

I'll shortly post and install the final (with backports, ChangeLogs and
all) vta-patchset-cleanup.patch patches for the VTA branch and for the
VTA4.4 branch.


[-- Attachment #2: vta-patchset.patch.bz2 --]
[-- Type: application/octet-stream, Size: 121732 bytes --]

[-- Attachment #3: Type: text/plain, Size: 257 bytes --]


-- 
Alexandre Oliva, freedom fighter    http://FSFLA.org/~lxoliva/
You must be the change you wish to see in the world. -- Gandhi
Be Free! -- http://FSFLA.org/   FSF Latin America board member
Free Software Evangelist      Red Hat Brazil Compiler Engineer

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [vta->trunk] VTA merge
  2009-08-31 23:22       ` Alexandre Oliva
       [not found]         ` <4A9C5373.6060201@redhat.com>
@ 2009-09-01 15:16         ` NightStrike
  2009-09-01 18:25           ` Alexandre Oliva
  1 sibling, 1 reply; 52+ messages in thread
From: NightStrike @ 2009-09-01 15:16 UTC (permalink / raw)
  To: Alexandre Oliva; +Cc: Richard Guenther, gcc-patches, jakub

On Mon, Aug 31, 2009 at 6:30 PM, Alexandre Oliva<aoliva@redhat.com> wrote:
> How does this patch, on top of vta-patchset.patch, look?  (ChangeLog
> still pending) It has just finished building stage1 on x86_64-linux-gnu,
> but it's otherwise untested.

Well, your original patchset doesn't apply cleanly.

There's a typo in the original here:

patch: **** malformed patch at line 18175: i+      vt_debug_insns_local (true);

I deleted the 'i' at the beginning of the line.


Also, there's this:
2 out of 4 hunks FAILED -- saving rejects to file gcc/fwprop.c.rej


I'm guessing that you want your cleanup patch to be applied after
applying your initial patch.  However, your initial patch doesn't
apply.  So, can you provide a complete patch that I can test on mingw
targets?  As opposed to broken first patches and cleanup patches that
won't apply on top of them?

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [vta->trunk] VTA merge
  2009-09-01  4:42           ` Alexandre Oliva
@ 2009-09-01  8:54             ` Richard Guenther
  0 siblings, 0 replies; 52+ messages in thread
From: Richard Guenther @ 2009-09-01  8:54 UTC (permalink / raw)
  To: Alexandre Oliva; +Cc: Richard Henderson, gcc-patches, jakub

On Tue, Sep 1, 2009 at 6:42 AM, Alexandre Oliva<aoliva@redhat.com> wrote:
> On Aug 31, 2009, Richard Henderson <rth@redhat.com> wrote:
>
>>> +      if (!id->debug_stmts)
>>> +        id->debug_stmts = VEC_alloc (gimple, heap, 1);
>>> +      VEC_safe_push (gimple, heap, id->debug_stmts, copy);
>
>> Note that VEC_safe_push will also handle the initial
>> allocation of the VEC from NULL.  It's quite handy.
>
> Aah, indeed, this makes room for a few simplifications.  Even more so
> because id is zero initialized.
>
> Here's the revised patch I'm testing now, with a few additional
> adjustments.  It has already completed bootstrap on i686-pc-linux-gnu.

Looks good to me.  Well, let's just go forward with it - we obviously disagree
about the added abstraction at this point.  I'm looking forward to you
adding other kinds of debug instructions in the near future.  Or at least
a hint on what they could be.

Thanks,
Richard.

>
>
> --
> Alexandre Oliva, freedom fighter    http://FSFLA.org/~lxoliva/
> You must be the change you wish to see in the world. -- Gandhi
> Be Free! -- http://FSFLA.org/   FSF Latin America board member
> Free Software Evangelist      Red Hat Brazil Compiler Engineer
>
>

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [vta->trunk] VTA merge
       [not found]         ` <4A9C5373.6060201@redhat.com>
@ 2009-09-01  4:42           ` Alexandre Oliva
  2009-09-01  8:54             ` Richard Guenther
  0 siblings, 1 reply; 52+ messages in thread
From: Alexandre Oliva @ 2009-09-01  4:42 UTC (permalink / raw)
  To: Richard Henderson; +Cc: Richard Guenther, gcc-patches, jakub

[-- Attachment #1: Type: text/plain, Size: 555 bytes --]

On Aug 31, 2009, Richard Henderson <rth@redhat.com> wrote:

>> +	  if (!id->debug_stmts)
>> +	    id->debug_stmts = VEC_alloc (gimple, heap, 1);
>> +	  VEC_safe_push (gimple, heap, id->debug_stmts, copy);

> Note that VEC_safe_push will also handle the initial
> allocation of the VEC from NULL.  It's quite handy.

Aah, indeed, this makes room for a few simplifications.  Even more so
because id is zero initialized.

Here's the revised patch I'm testing now, with a few additional
adjustments.  It has already completed bootstrap on i686-pc-linux-gnu.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: vta-patchset-cleanup.patch --]
[-- Type: text/x-diff, Size: 22988 bytes --]

Removed:
	* fwprop.c (try_fwprop_subst): Deal with NULL set.

Index: gcc/cse.c
===================================================================
--- gcc/cse.c.orig	2009-08-31 21:28:48.000000000 -0300
+++ gcc/cse.c	2009-08-31 21:28:57.000000000 -0300
@@ -6246,8 +6246,7 @@ cse_extended_basic_block (struct cse_bas
 
 	     FIXME: This is a real kludge and needs to be done some other
 		    way.  */
-	  if (INSN_P (insn)
-	      && !DEBUG_INSN_P (insn)
+	  if (NONDEBUG_INSN_P (insn)
 	      && num_insns++ > PARAM_VALUE (PARAM_MAX_CSE_INSNS))
 	    {
 	      flush_hash_table ();
Index: gcc/Makefile.in
===================================================================
--- gcc/Makefile.in.orig	2009-08-31 21:28:51.000000000 -0300
+++ gcc/Makefile.in	2009-08-31 21:28:57.000000000 -0300
@@ -911,7 +911,7 @@ SCEV_H = tree-scalar-evolution.h $(GGC_H
 LAMBDA_H = lambda.h $(TREE_H) vec.h $(GGC_H)
 TREE_DATA_REF_H = tree-data-ref.h $(LAMBDA_H) omega.h graphds.h $(SCEV_H)
 VARRAY_H = varray.h $(MACHMODE_H) $(SYSTEM_H) coretypes.h $(TM_H)
-TREE_INLINE_H = tree-inline.h pointer-set.h
+TREE_INLINE_H = tree-inline.h $(GIMPLE_H)
 REAL_H = real.h $(MACHMODE_H)
 IRA_INT_H = ira.h ira-int.h $(CFGLOOP_H) alloc-pool.h
 DBGCNT_H = dbgcnt.h dbgcnt.def
Index: gcc/tree-inline.c
===================================================================
--- gcc/tree-inline.c.orig	2009-08-31 21:28:51.000000000 -0300
+++ gcc/tree-inline.c	2009-08-31 22:53:15.000000000 -0300
@@ -1335,7 +1335,7 @@ remap_gimple_stmt (gimple stmt, copy_bod
 	  copy = gimple_build_debug_bind (gimple_debug_bind_get_var (stmt),
 					  gimple_debug_bind_get_value (stmt),
 					  stmt);
-	  VARRAY_PUSH_GENERIC_PTR (id->debug_stmts, copy);
+	  VEC_safe_push (gimple, heap, id->debug_stmts, copy);
 	  return copy;
 	}
       else
@@ -2122,20 +2122,22 @@ copy_debug_stmt (gimple stmt, copy_body_
 
 /* Process deferred debug stmts.  In order to give values better odds
    of being successfully remapped, we delay the processing of debug
-   stmts.  */
+   stmts until all other stmts that might require remapping are
+   processed.  */
 
 static void
 copy_debug_stmts (copy_body_data *id)
 {
-  size_t i, e;
+  size_t i;
+  gimple stmt;
 
   if (!id->debug_stmts)
     return;
 
-  for (i = 0, e = VARRAY_ACTIVE_SIZE (id->debug_stmts); i < e; i++)
-    copy_debug_stmt ((gimple) VARRAY_GENERIC_PTR (id->debug_stmts, i), id);
+  for (i = 0; VEC_iterate (gimple, id->debug_stmts, i, stmt); i++)
+    copy_debug_stmt (stmt, id);
 
-  VARRAY_POP_ALL (id->debug_stmts);
+  VEC_free (gimple, heap, id->debug_stmts);
 }
 
 /* Make a copy of the body of SRC_FN so that it can be inserted inline in
@@ -2226,8 +2228,6 @@ insert_init_debug_bind (copy_body_data *
 	gsi_insert_before (&gsi, note, GSI_SAME_STMT);
     }
 
-  mark_symbols_for_renaming (note);
-
   return note;
 }
 
@@ -3882,8 +3882,6 @@ optimize_inline_calls (tree fn)
   id.transform_return_to_modify = true;
   id.transform_lang_insert_block = NULL;
   id.statements_to_fold = pointer_set_create ();
-  if (MAY_HAVE_DEBUG_STMTS)
-    VARRAY_GENERIC_PTR_INIT (id.debug_stmts, 8, "debug_stmt");
 
   push_gimplify_context (&gctx);
 
@@ -3921,6 +3919,8 @@ optimize_inline_calls (tree fn)
   fold_marked_statements (last, id.statements_to_fold);
   pointer_set_destroy (id.statements_to_fold);
   
+  gcc_assert (!id.debug_stmts);
+
   /* Renumber the (code) basic_blocks consecutively.  */
   compact_blocks ();
   /* Renumber the lexical scoping (non-code) blocks consecutively.  */
@@ -4736,9 +4736,6 @@ tree_function_versioning (tree old_decl,
   /* Generate a new name for the new version. */
   id.statements_to_fold = pointer_set_create ();
 
-  if (MAY_HAVE_DEBUG_STMTS)
-    VARRAY_GENERIC_PTR_INIT (id.debug_stmts, 8, "debug_stmt");
-  
   id.decl_map = pointer_map_create ();
   id.debug_map = NULL;
   id.src_fn = old_decl;
@@ -4875,6 +4872,7 @@ tree_function_versioning (tree old_decl,
   free_dominance_info (CDI_DOMINATORS);
   free_dominance_info (CDI_POST_DOMINATORS);
 
+  gcc_assert (!id.debug_stmts);
   VEC_free (gimple, heap, init_stmts);
   pop_cfun ();
   current_function_decl = old_current_function_decl;
Index: gcc/tree-inline.h
===================================================================
--- gcc/tree-inline.h.orig	2009-08-31 21:28:52.000000000 -0300
+++ gcc/tree-inline.h	2009-08-31 21:28:57.000000000 -0300
@@ -22,8 +22,7 @@ along with GCC; see the file COPYING3.  
 #ifndef GCC_TREE_INLINE_H
 #define GCC_TREE_INLINE_H
 
-#include "varray.h"
-#include "pointer-set.h"
+#include "gimple.h"
 
 struct cgraph_edge;
 
@@ -120,7 +119,7 @@ typedef struct copy_body_data
   struct basic_block_def *entry_bb;
 
   /* Debug statements that need processing.  */
-  varray_type debug_stmts;
+  VEC(gimple,heap) *debug_stmts;
 
   /* A map from local declarations in the inlined function to
      equivalents in the function into which it is being inlined, where
Index: gcc/cfgexpand.c
===================================================================
--- gcc/cfgexpand.c.orig	2009-08-31 21:28:54.000000000 -0300
+++ gcc/cfgexpand.c	2009-08-31 21:28:57.000000000 -0300
@@ -3055,7 +3055,7 @@ expand_gimple_basic_block (basic_block b
 	  if (new_bb)
 	    return new_bb;
 	}
-      else if (is_gimple_debug (stmt))
+      else if (gimple_debug_bind_p (stmt))
 	{
 	  location_t sloc = get_curr_insn_source_location ();
 	  tree sblock = get_curr_insn_block ();
@@ -3102,7 +3102,7 @@ expand_gimple_basic_block (basic_block b
 	      if (gsi_end_p (nsi))
 		break;
 	      stmt = gsi_stmt (nsi);
-	      if (!is_gimple_debug (stmt))
+	      if (!gimple_debug_bind_p (stmt))
 		break;
 	    }
 
Index: gcc/dwarf2out.c
===================================================================
--- gcc/dwarf2out.c.orig	2009-08-31 21:28:49.000000000 -0300
+++ gcc/dwarf2out.c	2009-08-31 21:28:57.000000000 -0300
@@ -11541,7 +11541,7 @@ loc_descriptor (rtx rtl, enum machine_mo
       break;
 
     case CONST_INT:
-      if (mode != VOIDmode && mode != BLKmode)
+      if (mode != VOIDmode && mode != BLKmode && dwarf_version >= 4)
         {
           HOST_WIDE_INT i = INTVAL (rtl);
           int litsize;
@@ -11595,7 +11595,7 @@ loc_descriptor (rtx rtl, enum machine_mo
       break;
 
     case CONST_DOUBLE:
-      if (mode != VOIDmode)
+      if (mode != VOIDmode && dwarf_version >= 4)
 	{
 	  /* Note that a CONST_DOUBLE rtx could represent either an integer
 	     or a floating-point constant.  A CONST_DOUBLE is used whenever
@@ -11626,7 +11626,7 @@ loc_descriptor (rtx rtl, enum machine_mo
       break;
 
     case CONST_VECTOR:
-      if (mode != VOIDmode)
+      if (mode != VOIDmode && dwarf_version >= 4)
 	{
 	  unsigned int elt_size = GET_MODE_UNIT_SIZE (GET_MODE (rtl));
 	  unsigned int length = CONST_VECTOR_NUNITS (rtl);
@@ -11714,7 +11714,8 @@ loc_descriptor (rtx rtl, enum machine_mo
 	  && SYMBOL_REF_TLS_MODEL (rtl) != TLS_MODEL_NONE)
 	break;
     case LABEL_REF:
-      if (mode != VOIDmode && GET_MODE_SIZE (mode) == DWARF2_ADDR_SIZE)
+      if (mode != VOIDmode && GET_MODE_SIZE (mode) == DWARF2_ADDR_SIZE
+	  && dwarf_version >= 4)
 	{
 	  loc_result = new_loc_descr (DW_OP_implicit_value,
 				      DWARF2_ADDR_SIZE, 0);
@@ -11726,7 +11727,8 @@ loc_descriptor (rtx rtl, enum machine_mo
 
     default:
       if (GET_MODE_CLASS (mode) == MODE_INT && GET_MODE (rtl) == mode
-	  && GET_MODE_SIZE (GET_MODE (rtl)) <= DWARF2_ADDR_SIZE)
+	  && GET_MODE_SIZE (GET_MODE (rtl)) <= DWARF2_ADDR_SIZE
+	  && dwarf_version >= 4)
 	{
 	  /* Value expression.  */
 	  loc_result = mem_loc_descriptor (rtl, VOIDmode, initialized);
Index: gcc/expr.c
===================================================================
--- gcc/expr.c.orig	2009-08-31 21:28:51.000000000 -0300
+++ gcc/expr.c	2009-08-31 21:28:58.000000000 -0300
@@ -6002,7 +6002,7 @@ get_inner_reference (tree exp, HOST_WIDE
 
   /* Compute cumulative bit-offset for nested component-refs and array-refs,
      and find the ultimate containing object.  */
-  do
+  while (1)
     {
       switch (TREE_CODE (exp))
 	{
@@ -6081,7 +6081,6 @@ get_inner_reference (tree exp, HOST_WIDE
 
       exp = TREE_OPERAND (exp, 0);
     }
-  while (exp);
  done:
 
   /* If OFFSET is constant, see if we can return the whole thing as a
Index: gcc/var-tracking.c
===================================================================
--- gcc/var-tracking.c.orig	2009-08-31 21:28:51.000000000 -0300
+++ gcc/var-tracking.c	2009-08-31 21:28:58.000000000 -0300
@@ -2742,25 +2742,8 @@ canonicalize_values_mark (void **slot, v
 	    decl_or_value odv = dv_from_value (node->loc);
 	    void **oslot = shared_hash_find_slot_noinsert (set->vars, odv);
 
-#if 0 && ENABLE_CHECKING
-	    variable ovar;
-	    location_chain onode;
-
-	    gcc_assert (oslot);
-	    ovar = (variable)*oslot;
-	    gcc_assert (ovar->n_var_parts == 1);
-	    for (onode = ovar->var_part[0].loc_chain; onode;
-		 onode = onode->next)
-	      if (onode->loc == val)
-		break;
-
-	    gcc_assert (onode);
-
-	    /* ??? Remove this in case the assertion above never fails.  */
-	    if (!onode)
-#endif
-	      oslot = set_slot_part (set, val, oslot, odv, 0,
-				     node->init, NULL_RTX);
+	    oslot = set_slot_part (set, val, oslot, odv, 0,
+				   node->init, NULL_RTX);
 
 	    VALUE_RECURSED_INTO (node->loc) = true;
 	  }
@@ -2971,23 +2954,8 @@ canonicalize_values_star (void **slot, v
       }
 
   if (val)
-    {
-#if 0 && ENABLE_CHECKING
-      variable cvar = (variable)*cslot;
-
-      gcc_assert (cvar->n_var_parts == 1);
-      for (node = cvar->var_part[0].loc_chain; node; node = node->next)
-	if (node->loc == val)
-	  break;
-
-      gcc_assert (node);
-
-      /* ??? Remove this in case the assertion above never fails.  */
-      if (!node)
-#endif
-	cslot = set_slot_part (set, val, cslot, cdv, 0,
-			       VAR_INIT_STATUS_INITIALIZED, NULL_RTX);
-    }
+    cslot = set_slot_part (set, val, cslot, cdv, 0,
+			   VAR_INIT_STATUS_INITIALIZED, NULL_RTX);
 
   slot = clobber_slot_part (set, cval, slot, 0, NULL);
 
@@ -3582,65 +3550,6 @@ variable_post_merge_new_vals (void **slo
 	      *curp = att->next;
 	      pool_free (attrs_pool, att);
 	    }
-#if 0 /* Don't push constants to values.  If you remove this, adjust
-	 the corresponding comment containing 'push constants to
-	 values' below.  */
-	  else if (GET_CODE (node->loc) == CONST_INT
-		   || GET_CODE (node->loc) == CONST_FIXED
-		   || GET_CODE (node->loc) == CONST_DOUBLE
-		   || GET_CODE (node->loc) == SYMBOL_REF)
-	    {
-	      decl_or_value cdv;
-	      rtx cval;
-	      cselib_val *v;
-	      void **oslot;
-
-	      if (var->refcount != 1)
-		{
-		  slot = unshare_variable (set, slot, var,
-					   VAR_INIT_STATUS_INITIALIZED);
-		  var = (variable)*slot;
-		  goto restart;
-		}
-
-	      v = cselib_lookup (node->loc,
-				 TYPE_MODE (TREE_TYPE (dv_as_decl (var->dv))),
-				 1);
-
-	      if (dump_file)
-		{
-		  fprintf (dump_file, "%s new value %i for ",
-			   cselib_preserved_value_p (v)
-			   ? "Reused" : "Created", v->value);
-		  print_rtl_single (dump_file, node->loc);
-		  fputc ('\n', dump_file);
-		}
-
-	      cselib_preserve_value (v);
-	      cval = v->val_rtx;
-	      cdv = dv_from_value (cval);
-
-	      oslot = shared_hash_find_slot_noinsert (set->vars, cdv);
-	      if (oslot)
-		oslot = set_slot_part (set, node->loc, oslot, cdv, 0,
-				       VAR_INIT_STATUS_INITIALIZED,
-				       NULL_RTX);
-	      else
-		{
-		  if (!*dfpm->permp)
-		    {
-		      *dfpm->permp = XNEW (dataflow_set);
-		      dataflow_set_init (*dfpm->permp);
-		    }
-
-		  set_variable_part (*dfpm->permp, node->loc, cdv, 0,
-				     VAR_INIT_STATUS_INITIALIZED, NULL,
-				     NO_INSERT);
-		}
-	      node->loc = cval;
-	      check_dupes = true;
-	    }
-#endif
 	}
 
       if (check_dupes)
@@ -7079,14 +6988,13 @@ vt_emit_notes (void)
   htab_traverse (shared_hash_htab (cur.vars),
 		 emit_notes_for_differences_1,
 		 shared_hash_htab (empty_shared_hash));
+  if (MAY_HAVE_DEBUG_INSNS)
+    gcc_assert (htab_elements (value_chains) == 0);
 #endif
   dataflow_set_destroy (&cur);
 
   if (MAY_HAVE_DEBUG_INSNS)
-    {
-      gcc_assert (htab_elements (value_chains) == 0);
-      VEC_free (variable, heap, changed_variables_stack);
-    }
+    VEC_free (variable, heap, changed_variables_stack);
 
   emit_notes = false;
 }
Index: gcc/gimple.h
===================================================================
--- gcc/gimple.h.orig	2009-08-31 21:28:48.000000000 -0300
+++ gcc/gimple.h	2009-08-31 21:28:58.000000000 -0300
@@ -114,9 +114,15 @@ enum gf_mask {
     GF_OMP_RETURN_NOWAIT	= 1 << 0,
 
     GF_OMP_SECTION_LAST		= 1 << 0,
-    GF_PREDICT_TAKEN		= 1 << 15,
+    GF_PREDICT_TAKEN		= 1 << 15
+};
 
-    GIMPLE_DEBUG_BIND		= 0
+/* Currently, there's only one type of gimple debug stmt.  Others are
+   envisioned, for example, to enable the generation of is_stmt notes
+   in line number information, to mark sequence points, etc.  This
+   subcode is to be used to tell them apart.  */
+enum gimple_debug_subcode {
+  GIMPLE_DEBUG_BIND = 0
 };
 
 /* Masks for selecting a pass local flag (PLF) to work on.  These
Index: gcc/tree-cfg.c
===================================================================
--- gcc/tree-cfg.c.orig	2009-08-31 21:28:49.000000000 -0300
+++ gcc/tree-cfg.c	2009-08-31 21:28:58.000000000 -0300
@@ -1395,6 +1395,49 @@ gimple_can_merge_blocks_p (basic_block a
   return true;
 }
 
+/* Return true if the var whose chain of uses starts at PTR has no
+   nondebug uses.  */
+bool
+has_zero_uses_1 (const ssa_use_operand_t *head)
+{
+  const ssa_use_operand_t *ptr;
+
+  for (ptr = head->next; ptr != head; ptr = ptr->next)
+    if (!is_gimple_debug (USE_STMT (ptr)))
+      return false;
+
+  return true;
+}
+
+/* Return true if the var whose chain of uses starts at PTR has a
+   single nondebug use.  Set USE_P and STMT to that single nondebug
+   use, if so, or to NULL otherwise.  */
+bool
+single_imm_use_1 (const ssa_use_operand_t *head,
+		  use_operand_p *use_p, gimple *stmt)
+{
+  ssa_use_operand_t *ptr, *single_use = 0;
+
+  for (ptr = head->next; ptr != head; ptr = ptr->next)
+    if (!is_gimple_debug (USE_STMT (ptr)))
+      {
+	if (single_use)
+	  {
+	    single_use = NULL;
+	    break;
+	  }
+	single_use = ptr;
+      }
+
+  if (use_p)
+    *use_p = single_use;
+
+  if (stmt)
+    *stmt = single_use ? single_use->loc.stmt : NULL;
+
+  return !!single_use;
+}
+
 /* Replaces all uses of NAME by VAL.  */
 
 void
@@ -4137,7 +4180,12 @@ verify_gimple_phi (gimple stmt)
 static bool
 verify_gimple_debug (gimple stmt ATTRIBUTE_UNUSED)
 {
-  /* ??? FIXME.  */
+  /* There isn't much that could be wrong in a gimple debug stmt.  A
+     gimple debug bind stmt, for example, maps a tree, that's usually
+     a VAR_DECL or a PARM_DECL, but that could also be some scalarized
+     component or member of an aggregate type, to another tree, that
+     can be an arbitrary expression.  These stmts expand into debug
+     insns, and are converted to debug notes by var-tracking.c.  */
   return false;
 }
 
Index: gcc/tree-flow-inline.h
===================================================================
--- gcc/tree-flow-inline.h.orig	2009-08-31 21:28:50.000000000 -0300
+++ gcc/tree-flow-inline.h	2009-08-31 21:28:58.000000000 -0300
@@ -358,100 +358,88 @@ next_readonly_imm_use (imm_use_iterator 
   return imm->imm_use;
 }
 
-/* Return true if VAR has no uses.  */
+/* tree-cfg.c */
+extern bool has_zero_uses_1 (const ssa_use_operand_t *head);
+extern bool single_imm_use_1 (const ssa_use_operand_t *head,
+			      use_operand_p *use_p, gimple *stmt);
+
+/* Return true if VAR has no nondebug uses.  */
 static inline bool
 has_zero_uses (const_tree var)
 {
-  const ssa_use_operand_t *ptr, *start;
-  bool ret;
+  const ssa_use_operand_t *const ptr = &(SSA_NAME_IMM_USE_NODE (var));
 
-  ptr = &(SSA_NAME_IMM_USE_NODE (var));
-  /* A single use means there is no items in the list.  */
-  ret = (ptr == ptr->next);
-
-  if (ret || !MAY_HAVE_DEBUG_STMTS)
-    return ret;
-
-  start = ptr;
-  for (ptr = start->next; ptr != start; ptr = ptr->next)
-    if (!is_gimple_debug (USE_STMT (ptr)))
-      return false;
-  return true;
+  /* A single use_operand means there is no items in the list.  */
+  if (ptr == ptr->next)
+    return true;
+
+  /* If there are debug stmts, we have to look at each use and see
+     whether there are any nondebug uses.  */
+  if (!MAY_HAVE_DEBUG_STMTS)
+    return false;
+
+  return has_zero_uses_1 (ptr);
 }
 
-/* Return true if VAR has a single use.  */
+/* Return true if VAR has a single nondebug use.  */
 static inline bool
 has_single_use (const_tree var)
 {
-  const ssa_use_operand_t *ptr, *start;
-  bool ret;
+  const ssa_use_operand_t *const ptr = &(SSA_NAME_IMM_USE_NODE (var));
 
-  ptr = &(SSA_NAME_IMM_USE_NODE (var));
-  /* A single use means there is one item in the list.  */
-  ret = (ptr != ptr->next && ptr == ptr->next->next);
+  /* If there aren't any uses whatsoever, we're done.  */
+  if (ptr == ptr->next)
+    return false;
 
-  if (ret)
+  /* If there's a single use, check that it's not a debug stmt.  */
+  if (ptr == ptr->next->next)
     return !is_gimple_debug (USE_STMT (ptr->next));
-  else if (!MAY_HAVE_DEBUG_STMTS)
-    return ret;
 
-  start = ptr;
-  for (ptr = start->next; ptr != start; ptr = ptr->next)
-    if (!is_gimple_debug (USE_STMT (ptr)))
-      {
-	if (ret)
-	  return false;
-	ret = true;
-      }
-  return ret;
+  /* If there are debug stmts, we have to look at each of them.  */
+  if (!MAY_HAVE_DEBUG_STMTS)
+    return false;
+
+  return single_imm_use_1 (ptr, NULL, NULL);
 }
 
 
-/* If VAR has only a single immediate use, return true, and set USE_P and STMT
-   to the use pointer and stmt of occurrence.  */
+/* If VAR has only a single immediate nondebug use, return true, and
+   set USE_P and STMT to the use pointer and stmt of occurrence.  */
 static inline bool
 single_imm_use (const_tree var, use_operand_p *use_p, gimple *stmt)
 {
-  const ssa_use_operand_t *ptr;
-  bool ret;
-
-  ptr = &(SSA_NAME_IMM_USE_NODE (var));
-
-  ret = ptr != ptr->next && ptr == ptr->next->next;
+  const ssa_use_operand_t *const ptr = &(SSA_NAME_IMM_USE_NODE (var));
 
-  if (ret)
-    ret = !is_gimple_debug (USE_STMT (ptr->next));
-  else if (MAY_HAVE_DEBUG_STMTS)
+  /* If there aren't any uses whatsoever, we're done.  */
+  if (ptr == ptr->next)
     {
-      const ssa_use_operand_t *start = ptr, *prev = ptr, *single_use_prev = 0;
-
-      for (ptr = start->next; ptr != start; prev = ptr, ptr = ptr->next)
-	if (!is_gimple_debug (USE_STMT (ptr)))
-	  {
-	    if (ret)
-	      {
-		ret = false;
-		break;
-	      }
-	    ret = true;
-	    single_use_prev = prev;
-	  }
-
-      ptr = single_use_prev;
+    return_false:
+      *use_p = NULL_USE_OPERAND_P;
+      *stmt = NULL;
+      return false;
     }
 
-  if (ret)
+  /* If there's a single use, check that it's not a debug stmt.  */
+  if (ptr == ptr->next->next)
     {
-      *use_p = ptr->next;
-      *stmt = ptr->next->loc.stmt;
-      return true;
+      if (!is_gimple_debug (USE_STMT (ptr->next)))
+	{
+	  *use_p = ptr->next;
+	  *stmt = ptr->next->loc.stmt;
+	  return true;
+	}
+      else
+	goto return_false;
     }
-  *use_p = NULL_USE_OPERAND_P;
-  *stmt = NULL;
-  return false;
+
+  /* If there are debug stmts, we have to look at each of them.  */
+  if (!MAY_HAVE_DEBUG_STMTS)
+    goto return_false;
+
+  return single_imm_use_1 (ptr, use_p, stmt);
 }
 
-/* Return the number of immediate uses of VAR.  */
+/* Return the number of nondebug immediate uses of VAR.  */
 static inline unsigned int
 num_imm_uses (const_tree var)
 {
Index: gcc/tree-ssa-loop-ivopts.c
===================================================================
--- gcc/tree-ssa-loop-ivopts.c.orig	2009-08-31 21:28:51.000000000 -0300
+++ gcc/tree-ssa-loop-ivopts.c	2009-08-31 21:28:58.000000000 -0300
@@ -1850,7 +1850,7 @@ find_interesting_uses (struct ivopts_dat
 	find_interesting_uses_stmt (data, gsi_stmt (bsi));
       for (bsi = gsi_start_bb (bb); !gsi_end_p (bsi); gsi_next (&bsi))
 	if (!is_gimple_debug (gsi_stmt (bsi)))
-	find_interesting_uses_stmt (data, gsi_stmt (bsi));
+	  find_interesting_uses_stmt (data, gsi_stmt (bsi));
     }
 
   if (dump_file && (dump_flags & TDF_DETAILS))
Index: gcc/tree-ssa.c
===================================================================
--- gcc/tree-ssa.c.orig	2009-08-31 21:28:49.000000000 -0300
+++ gcc/tree-ssa.c	2009-08-31 21:28:58.000000000 -0300
@@ -262,12 +262,6 @@ target_for_debug_bind (tree var)
   if (DECL_HAS_VALUE_EXPR_P (var))
     return target_for_debug_bind (DECL_VALUE_EXPR (var));
 
-#if 0
-  /* Should we deal with DECL_DEBUG_EXPR_IS_FROM as well?  */
-  if (DECL_DEBUG_EXPR_IS_FROM (var))
-    return target_for_debug_bind (DECL_DEBUG_EXPR (var));
-#endif
-
   if (DECL_IGNORED_P (var))
     return NULL_TREE;
 
@@ -439,7 +433,7 @@ propagate_defs_into_debug_stmts (gimple 
   if (!MAY_HAVE_DEBUG_STMTS)
     return;
 
-  FOR_EACH_SSA_DEF_OPERAND (def_p, def, op_iter, SSA_OP_ALL_DEFS)
+  FOR_EACH_SSA_DEF_OPERAND (def_p, def, op_iter, SSA_OP_DEF)
     {
       tree var = DEF_FROM_PTR (def_p);
 
Index: gcc/doc/invoke.texi
===================================================================
--- gcc/doc/invoke.texi.orig	2009-08-31 21:28:50.000000000 -0300
+++ gcc/doc/invoke.texi	2009-08-31 21:28:58.000000000 -0300
@@ -4398,11 +4398,14 @@ assembler (GAS) to fail with an error.
 @opindex gdwarf-@var{version}
 Produce debugging information in DWARF format (if that is
 supported).  This is the format used by DBX on IRIX 6.  The value
-of @var{version} may be either 2 or 3; the default version is 2.
+of @var{version} may be either 2, 3 or 4; the default version is 2.
 
 Note that with DWARF version 2 some ports require, and will always
 use, some non-conflicting DWARF 3 extensions in the unwind tables.
 
+Version 4 may require GDB 7.0 and @option{-fvar-tracking-assignments}
+for maximum benefit.
+
 @item -gvms
 @opindex gvms
 Produce debugging information in VMS debug format (if that is
@@ -5456,7 +5459,7 @@ the debug info format supports it.
 Annotate assignments to user variables early in the compilation and
 attempt to carry the annotations over throughout the compilation all the
 way to the end, in an attempt to improve debug information while
-optimizing.
+optimizing.  Use of @option{-gdwarf-4} is recommended along with it.
 
 It can be enabled even if var-tracking is disabled, in which case
 annotations will be created and maintained, but discarded at the end.
Index: gcc/opts.c
===================================================================
--- gcc/opts.c.orig	2009-08-31 21:28:50.000000000 -0300
+++ gcc/opts.c	2009-08-31 21:28:58.000000000 -0300
@@ -2054,7 +2054,7 @@ common_handle_option (size_t scode, cons
       break;
 
     case OPT_gdwarf_:
-      if (value < 2 || value > 3)
+      if (value < 2 || value > 4)
 	error ("dwarf version %d is not supported", value);
       else
 	dwarf_version = value;

[-- Attachment #3: Type: text/plain, Size: 257 bytes --]


-- 
Alexandre Oliva, freedom fighter    http://FSFLA.org/~lxoliva/
You must be the change you wish to see in the world. -- Gandhi
Be Free! -- http://FSFLA.org/   FSF Latin America board member
Free Software Evangelist      Red Hat Brazil Compiler Engineer

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [vta->trunk] VTA merge
  2009-08-31 16:45     ` Richard Guenther
@ 2009-08-31 23:22       ` Alexandre Oliva
       [not found]         ` <4A9C5373.6060201@redhat.com>
  2009-09-01 15:16         ` NightStrike
  0 siblings, 2 replies; 52+ messages in thread
From: Alexandre Oliva @ 2009-08-31 23:22 UTC (permalink / raw)
  To: Richard Guenther; +Cc: gcc-patches, jakub

[-- Attachment #1: Type: text/plain, Size: 3631 bytes --]

On Aug 31, 2009, Richard Guenther <richard.guenther@gmail.com> wrote:

> On Mon, Aug 31, 2009 at 10:42 AM, Alexandre Oliva<aoliva@redhat.com> wrote:
>> On Aug 30, 2009, Richard Guenther <richard.guenther@gmail.com> wrote:
>>> +/* Nonzero if is_gimple_debug() may possibly hold.  */
>>> +#define MAY_HAVE_DEBUG_STMTS    (flag_var_tracking_assignments)
>> 
>>> I don't remember exactly the outcome of the last discussion, but I
>>> at least remember asking you to
>>> s/MAY_HAVE_DEBUG_STMTS/flag_var_tracking_assignments/g

>> You did, and I pointed out that would have to be changed back as soon as
>> I added another type of debug stmt that I have planned.  The abstraction
>> is ready for that, and it is a solid abstraction.

> No, it says MAY_HAVE_DEBUG_STMTS, not MAY_HAVE_DEBUG_BIND.

Of course, because the abstraction is for debug stmts, not only for
debug bind stmts.

> Frakly I don't think it is sound to introduce abstraction before you make
> use of it.

?!?

The very power of abstraction in software engineering is to enable the
underlying implementation to change without having to change the
interface.  That's what I'm talking about here.  I have laid the ground
to evolve this code base with the addition of other kinds of debug
stmts, and prepared the code to deal with it.

Only very few cases actually deal with the specifics debug bind stmts,
and those are the ones that use gimple_debug_bind_p.  All the others
just know that they're to ignore any kind of debug stmt.

> I cannot see how at some places using is_gimple_debug
> and at some places using gimple_debug_bind_p can be correct.

I think that's just because you expect them to be synonymous, but they
aren't.  Debug stmts are a more general class than debug bind stmts.  I
envision other kinds of debug stmts that will be part of the code
stream, but that ought to be disregarded wherever debug bind stmts are.
The example I brought up back then was that of statement separators for
is_stmt generation in line number information.  is_gimple_debug() will
cover them, whereas gimple_debug_bind_p() won't, because they don't have
the same internal structure.

That said, I found one piece of code in cfgexpand that used
is_gimple_debug(), but that assumed a gimple_debug_bind_p() structure,
and fixed it.  There's another in tree-ssa.c, but if we ever add any
other debug stmt that contains USEs, the assertion will hit and remind
us to fix it.

> Once you introduce some other debug stmt variant how can you decide
> what and why one or the other is correct.

That's exactly the power of the abstraction.

>>> Note that the GIMPLE_DEBUG_BIND as gf_mask is even more bogus
>>> that the previous additional tree code.  Just use literal zero in the
>>> single place where it is necessary and remove the debug bind
>>> vs. debug stmt confusion.
>> 
>> Likewise.  (Why do you say it is bogus?)

> Because you amend a bit flags enum with something completely
> different?

An enum already used for subcodes didn't feel bogus to me, but I've just
introduced enum gimple_debug_subcode.

> where up to the ptr == ptr->next->next check you'd inline the thing.

ACK

> It would be useful to have automatic testers pick up the rev before
> the merge, the merge and then followup changes.  So I think a short
> freeze won't hurt.

I see.  I'll announce the freeze once I get approval for these bits.

How does this patch, on top of vta-patchset.patch, look?  (ChangeLog
still pending) It has just finished building stage1 on x86_64-linux-gnu,
but it's otherwise untested.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: vta-patchset-cleanup.patch --]
[-- Type: text/x-diff, Size: 23576 bytes --]

Removed:
	* fwprop.c (try_fwprop_subst): Deal with NULL set.

Index: gcc/cse.c
===================================================================
--- gcc/cse.c.orig	2009-08-31 16:52:55.000000000 -0300
+++ gcc/cse.c	2009-08-31 16:55:45.000000000 -0300
@@ -6246,8 +6246,7 @@ cse_extended_basic_block (struct cse_bas
 
 	     FIXME: This is a real kludge and needs to be done some other
 		    way.  */
-	  if (INSN_P (insn)
-	      && !DEBUG_INSN_P (insn)
+	  if (NONDEBUG_INSN_P (insn)
 	      && num_insns++ > PARAM_VALUE (PARAM_MAX_CSE_INSNS))
 	    {
 	      flush_hash_table ();
Index: gcc/regmove.c
===================================================================
--- gcc/regmove.c.orig	2009-08-31 16:52:56.000000000 -0300
+++ gcc/regmove.c	2009-08-31 16:55:45.000000000 -0300
@@ -1198,8 +1198,6 @@ regmove_backward_pass (void)
 
 		  break;
 		}
-	      else if (num_changes_pending () > 0)
-		cancel_changes (0);
 	    }
 
 	  /* If we weren't able to replace any of the alternatives, try an
Index: gcc/Makefile.in
===================================================================
--- gcc/Makefile.in.orig	2009-08-31 17:04:36.000000000 -0300
+++ gcc/Makefile.in	2009-08-31 19:09:33.000000000 -0300
@@ -911,7 +911,7 @@ SCEV_H = tree-scalar-evolution.h $(GGC_H
 LAMBDA_H = lambda.h $(TREE_H) vec.h $(GGC_H)
 TREE_DATA_REF_H = tree-data-ref.h $(LAMBDA_H) omega.h graphds.h $(SCEV_H)
 VARRAY_H = varray.h $(MACHMODE_H) $(SYSTEM_H) coretypes.h $(TM_H)
-TREE_INLINE_H = tree-inline.h pointer-set.h
+TREE_INLINE_H = tree-inline.h $(GIMPLE_H)
 REAL_H = real.h $(MACHMODE_H)
 IRA_INT_H = ira.h ira-int.h $(CFGLOOP_H) alloc-pool.h
 DBGCNT_H = dbgcnt.h dbgcnt.def
Index: gcc/tree-inline.c
===================================================================
--- gcc/tree-inline.c.orig	2009-08-31 17:01:44.000000000 -0300
+++ gcc/tree-inline.c	2009-08-31 18:56:28.000000000 -0300
@@ -1335,7 +1335,9 @@ remap_gimple_stmt (gimple stmt, copy_bod
 	  copy = gimple_build_debug_bind (gimple_debug_bind_get_var (stmt),
 					  gimple_debug_bind_get_value (stmt),
 					  stmt);
-	  VARRAY_PUSH_GENERIC_PTR (id->debug_stmts, copy);
+	  if (!id->debug_stmts)
+	    id->debug_stmts = VEC_alloc (gimple, heap, 1);
+	  VEC_safe_push (gimple, heap, id->debug_stmts, copy);
 	  return copy;
 	}
       else
@@ -2122,20 +2124,22 @@ copy_debug_stmt (gimple stmt, copy_body_
 
 /* Process deferred debug stmts.  In order to give values better odds
    of being successfully remapped, we delay the processing of debug
-   stmts.  */
+   stmts until all other stmts that might require remapping are
+   processed.  */
 
 static void
 copy_debug_stmts (copy_body_data *id)
 {
-  size_t i, e;
+  size_t i;
+  gimple stmt;
 
   if (!id->debug_stmts)
     return;
 
-  for (i = 0, e = VARRAY_ACTIVE_SIZE (id->debug_stmts); i < e; i++)
-    copy_debug_stmt ((gimple) VARRAY_GENERIC_PTR (id->debug_stmts, i), id);
+  for (i = 0; VEC_iterate (gimple, id->debug_stmts, i, stmt); i++)
+    copy_debug_stmt (stmt, id);
 
-  VARRAY_POP_ALL (id->debug_stmts);
+  VEC_truncate (gimple, id->debug_stmts, 0);
 }
 
 /* Make a copy of the body of SRC_FN so that it can be inserted inline in
@@ -2226,8 +2230,6 @@ insert_init_debug_bind (copy_body_data *
 	gsi_insert_before (&gsi, note, GSI_SAME_STMT);
     }
 
-  mark_symbols_for_renaming (note);
-
   return note;
 }
 
@@ -3883,7 +3885,7 @@ optimize_inline_calls (tree fn)
   id.transform_lang_insert_block = NULL;
   id.statements_to_fold = pointer_set_create ();
   if (MAY_HAVE_DEBUG_STMTS)
-    VARRAY_GENERIC_PTR_INIT (id.debug_stmts, 8, "debug_stmt");
+    id.debug_stmts = VEC_alloc (gimple, heap, 0);
 
   push_gimplify_context (&gctx);
 
@@ -3921,6 +3923,8 @@ optimize_inline_calls (tree fn)
   fold_marked_statements (last, id.statements_to_fold);
   pointer_set_destroy (id.statements_to_fold);
   
+  VEC_free (gimple, heap, id.debug_stmts);
+
   /* Renumber the (code) basic_blocks consecutively.  */
   compact_blocks ();
   /* Renumber the lexical scoping (non-code) blocks consecutively.  */
@@ -4736,8 +4740,7 @@ tree_function_versioning (tree old_decl,
   /* Generate a new name for the new version. */
   id.statements_to_fold = pointer_set_create ();
 
-  if (MAY_HAVE_DEBUG_STMTS)
-    VARRAY_GENERIC_PTR_INIT (id.debug_stmts, 8, "debug_stmt");
+  id.debug_stmts = VEC_alloc (gimple, heap, 0);
   
   id.decl_map = pointer_map_create ();
   id.debug_map = NULL;
@@ -4875,6 +4878,7 @@ tree_function_versioning (tree old_decl,
   free_dominance_info (CDI_DOMINATORS);
   free_dominance_info (CDI_POST_DOMINATORS);
 
+  VEC_free (gimple, heap, id.debug_stmts);
   VEC_free (gimple, heap, init_stmts);
   pop_cfun ();
   current_function_decl = old_current_function_decl;
Index: gcc/tree-inline.h
===================================================================
--- gcc/tree-inline.h.orig	2009-08-31 17:01:44.000000000 -0300
+++ gcc/tree-inline.h	2009-08-31 19:09:39.000000000 -0300
@@ -22,8 +22,7 @@ along with GCC; see the file COPYING3.  
 #ifndef GCC_TREE_INLINE_H
 #define GCC_TREE_INLINE_H
 
-#include "varray.h"
-#include "pointer-set.h"
+#include "gimple.h"
 
 struct cgraph_edge;
 
@@ -120,7 +119,7 @@ typedef struct copy_body_data
   struct basic_block_def *entry_bb;
 
   /* Debug statements that need processing.  */
-  varray_type debug_stmts;
+  VEC(gimple,heap) *debug_stmts;
 
   /* A map from local declarations in the inlined function to
      equivalents in the function into which it is being inlined, where
Index: gcc/cfgexpand.c
===================================================================
--- gcc/cfgexpand.c.orig	2009-08-31 17:46:50.000000000 -0300
+++ gcc/cfgexpand.c	2009-08-31 17:47:38.000000000 -0300
@@ -3049,7 +3049,7 @@ expand_gimple_basic_block (basic_block b
 	  if (new_bb)
 	    return new_bb;
 	}
-      else if (is_gimple_debug (stmt))
+      else if (gimple_debug_bind_p (stmt))
 	{
 	  location_t sloc = get_curr_insn_source_location ();
 	  tree sblock = get_curr_insn_block ();
@@ -3096,7 +3096,7 @@ expand_gimple_basic_block (basic_block b
 	      if (gsi_end_p (nsi))
 		break;
 	      stmt = gsi_stmt (nsi);
-	      if (!is_gimple_debug (stmt))
+	      if (!gimple_debug_bind_p (stmt))
 		break;
 	    }
 
Index: gcc/dwarf2out.c
===================================================================
--- gcc/dwarf2out.c.orig	2009-08-31 17:38:31.000000000 -0300
+++ gcc/dwarf2out.c	2009-08-31 17:40:11.000000000 -0300
@@ -11541,7 +11541,7 @@ loc_descriptor (rtx rtl, enum machine_mo
       break;
 
     case CONST_INT:
-      if (mode != VOIDmode && mode != BLKmode)
+      if (mode != VOIDmode && mode != BLKmode && dwarf_version >= 4)
         {
           HOST_WIDE_INT i = INTVAL (rtl);
           int litsize;
@@ -11595,7 +11595,7 @@ loc_descriptor (rtx rtl, enum machine_mo
       break;
 
     case CONST_DOUBLE:
-      if (mode != VOIDmode)
+      if (mode != VOIDmode && dwarf_version >= 4)
 	{
 	  /* Note that a CONST_DOUBLE rtx could represent either an integer
 	     or a floating-point constant.  A CONST_DOUBLE is used whenever
@@ -11626,7 +11626,7 @@ loc_descriptor (rtx rtl, enum machine_mo
       break;
 
     case CONST_VECTOR:
-      if (mode != VOIDmode)
+      if (mode != VOIDmode && dwarf_version >= 4)
 	{
 	  unsigned int elt_size = GET_MODE_UNIT_SIZE (GET_MODE (rtl));
 	  unsigned int length = CONST_VECTOR_NUNITS (rtl);
@@ -11714,7 +11714,8 @@ loc_descriptor (rtx rtl, enum machine_mo
 	  && SYMBOL_REF_TLS_MODEL (rtl) != TLS_MODEL_NONE)
 	break;
     case LABEL_REF:
-      if (mode != VOIDmode && GET_MODE_SIZE (mode) == DWARF2_ADDR_SIZE)
+      if (mode != VOIDmode && GET_MODE_SIZE (mode) == DWARF2_ADDR_SIZE
+	  && dwarf_version >= 4)
 	{
 	  loc_result = new_loc_descr (DW_OP_implicit_value,
 				      DWARF2_ADDR_SIZE, 0);
@@ -11726,7 +11727,8 @@ loc_descriptor (rtx rtl, enum machine_mo
 
     default:
       if (GET_MODE_CLASS (mode) == MODE_INT && GET_MODE (rtl) == mode
-	  && GET_MODE_SIZE (GET_MODE (rtl)) <= DWARF2_ADDR_SIZE)
+	  && GET_MODE_SIZE (GET_MODE (rtl)) <= DWARF2_ADDR_SIZE
+	  && dwarf_version >= 4)
 	{
 	  /* Value expression.  */
 	  loc_result = mem_loc_descriptor (rtl, VOIDmode, initialized);
Index: gcc/expr.c
===================================================================
--- gcc/expr.c.orig	2009-08-31 17:36:54.000000000 -0300
+++ gcc/expr.c	2009-08-31 17:37:00.000000000 -0300
@@ -6002,7 +6002,7 @@ get_inner_reference (tree exp, HOST_WIDE
 
   /* Compute cumulative bit-offset for nested component-refs and array-refs,
      and find the ultimate containing object.  */
-  do
+  while (1)
     {
       switch (TREE_CODE (exp))
 	{
@@ -6081,7 +6081,6 @@ get_inner_reference (tree exp, HOST_WIDE
 
       exp = TREE_OPERAND (exp, 0);
     }
-  while (exp);
  done:
 
   /* If OFFSET is constant, see if we can return the whole thing as a
Index: gcc/var-tracking.c
===================================================================
--- gcc/var-tracking.c.orig	2009-08-31 17:35:04.000000000 -0300
+++ gcc/var-tracking.c	2009-08-31 18:53:09.000000000 -0300
@@ -2742,25 +2742,8 @@ canonicalize_values_mark (void **slot, v
 	    decl_or_value odv = dv_from_value (node->loc);
 	    void **oslot = shared_hash_find_slot_noinsert (set->vars, odv);
 
-#if 0 && ENABLE_CHECKING
-	    variable ovar;
-	    location_chain onode;
-
-	    gcc_assert (oslot);
-	    ovar = (variable)*oslot;
-	    gcc_assert (ovar->n_var_parts == 1);
-	    for (onode = ovar->var_part[0].loc_chain; onode;
-		 onode = onode->next)
-	      if (onode->loc == val)
-		break;
-
-	    gcc_assert (onode);
-
-	    /* ??? Remove this in case the assertion above never fails.  */
-	    if (!onode)
-#endif
-	      oslot = set_slot_part (set, val, oslot, odv, 0,
-				     node->init, NULL_RTX);
+	    oslot = set_slot_part (set, val, oslot, odv, 0,
+				   node->init, NULL_RTX);
 
 	    VALUE_RECURSED_INTO (node->loc) = true;
 	  }
@@ -2971,23 +2954,8 @@ canonicalize_values_star (void **slot, v
       }
 
   if (val)
-    {
-#if 0 && ENABLE_CHECKING
-      variable cvar = (variable)*cslot;
-
-      gcc_assert (cvar->n_var_parts == 1);
-      for (node = cvar->var_part[0].loc_chain; node; node = node->next)
-	if (node->loc == val)
-	  break;
-
-      gcc_assert (node);
-
-      /* ??? Remove this in case the assertion above never fails.  */
-      if (!node)
-#endif
-	cslot = set_slot_part (set, val, cslot, cdv, 0,
-			       VAR_INIT_STATUS_INITIALIZED, NULL_RTX);
-    }
+    cslot = set_slot_part (set, val, cslot, cdv, 0,
+			   VAR_INIT_STATUS_INITIALIZED, NULL_RTX);
 
   slot = clobber_slot_part (set, cval, slot, 0, NULL);
 
@@ -3582,65 +3550,6 @@ variable_post_merge_new_vals (void **slo
 	      *curp = att->next;
 	      pool_free (attrs_pool, att);
 	    }
-#if 0 /* Don't push constants to values.  If you remove this, adjust
-	 the corresponding comment containing 'push constants to
-	 values' below.  */
-	  else if (GET_CODE (node->loc) == CONST_INT
-		   || GET_CODE (node->loc) == CONST_FIXED
-		   || GET_CODE (node->loc) == CONST_DOUBLE
-		   || GET_CODE (node->loc) == SYMBOL_REF)
-	    {
-	      decl_or_value cdv;
-	      rtx cval;
-	      cselib_val *v;
-	      void **oslot;
-
-	      if (var->refcount != 1)
-		{
-		  slot = unshare_variable (set, slot, var,
-					   VAR_INIT_STATUS_INITIALIZED);
-		  var = (variable)*slot;
-		  goto restart;
-		}
-
-	      v = cselib_lookup (node->loc,
-				 TYPE_MODE (TREE_TYPE (dv_as_decl (var->dv))),
-				 1);
-
-	      if (dump_file)
-		{
-		  fprintf (dump_file, "%s new value %i for ",
-			   cselib_preserved_value_p (v)
-			   ? "Reused" : "Created", v->value);
-		  print_rtl_single (dump_file, node->loc);
-		  fputc ('\n', dump_file);
-		}
-
-	      cselib_preserve_value (v);
-	      cval = v->val_rtx;
-	      cdv = dv_from_value (cval);
-
-	      oslot = shared_hash_find_slot_noinsert (set->vars, cdv);
-	      if (oslot)
-		oslot = set_slot_part (set, node->loc, oslot, cdv, 0,
-				       VAR_INIT_STATUS_INITIALIZED,
-				       NULL_RTX);
-	      else
-		{
-		  if (!*dfpm->permp)
-		    {
-		      *dfpm->permp = XNEW (dataflow_set);
-		      dataflow_set_init (*dfpm->permp);
-		    }
-
-		  set_variable_part (*dfpm->permp, node->loc, cdv, 0,
-				     VAR_INIT_STATUS_INITIALIZED, NULL,
-				     NO_INSERT);
-		}
-	      node->loc = cval;
-	      check_dupes = true;
-	    }
-#endif
 	}
 
       if (check_dupes)
@@ -7079,14 +6988,13 @@ vt_emit_notes (void)
   htab_traverse (shared_hash_htab (cur.vars),
 		 emit_notes_for_differences_1,
 		 shared_hash_htab (empty_shared_hash));
+  if (MAY_HAVE_DEBUG_INSNS)
+    gcc_assert (htab_elements (value_chains) == 0);
 #endif
   dataflow_set_destroy (&cur);
 
   if (MAY_HAVE_DEBUG_INSNS)
-    {
-      gcc_assert (htab_elements (value_chains) == 0);
-      VEC_free (variable, heap, changed_variables_stack);
-    }
+    VEC_free (variable, heap, changed_variables_stack);
 
   emit_notes = false;
 }
Index: gcc/gimple.h
===================================================================
--- gcc/gimple.h.orig	2009-08-31 18:08:29.000000000 -0300
+++ gcc/gimple.h	2009-08-31 18:11:46.000000000 -0300
@@ -114,9 +114,15 @@ enum gf_mask {
     GF_OMP_RETURN_NOWAIT	= 1 << 0,
 
     GF_OMP_SECTION_LAST		= 1 << 0,
-    GF_PREDICT_TAKEN		= 1 << 15,
+    GF_PREDICT_TAKEN		= 1 << 15
+};
 
-    GIMPLE_DEBUG_BIND		= 0
+/* Currently, there's only one type of gimple debug stmt.  Others are
+   envisioned, for example, to enable the generation of is_stmt notes
+   in line number information, to mark sequence points, etc.  This
+   subcode is to be used to tell them apart.  */
+enum gimple_debug_subcode {
+  GIMPLE_DEBUG_BIND = 0
 };
 
 /* Masks for selecting a pass local flag (PLF) to work on.  These
Index: gcc/tree-cfg.c
===================================================================
--- gcc/tree-cfg.c.orig	2009-08-31 18:21:24.000000000 -0300
+++ gcc/tree-cfg.c	2009-08-31 19:01:08.000000000 -0300
@@ -1395,6 +1395,50 @@ gimple_can_merge_blocks_p (basic_block a
   return true;
 }
 
+/* Return true if the var whose chain of uses starts at PTR has no
+   nondebug uses.  */
+bool
+has_zero_uses_1 (const ssa_use_operand_t *head)
+{
+  const ssa_use_operand_t *ptr;
+
+  for (ptr = head->next; ptr != head; ptr = ptr->next)
+    if (!is_gimple_debug (USE_STMT (ptr)))
+      return false;
+
+  return true;
+}
+
+/* Return true if the var whose chain of uses starts at PTR has a
+   single nondebug use.  Set USE_P and STMT to that single nondebug
+   use, if so, or to NULL otherwise.  */
+bool
+single_imm_use_1 (const ssa_use_operand_t *head,
+		  use_operand_p *use_p, gimple *stmt)
+{
+  const ssa_use_operand_t *single_use = 0;
+  const ssa_use_operand_t *ptr;
+
+  for (ptr = head->next; ptr != head; ptr = ptr->next)
+    if (!is_gimple_debug (USE_STMT (ptr)))
+      {
+	if (single_use)
+	  {
+	    single_use = NULL;
+	    break;
+	  }
+	single_use = ptr;
+      }
+
+  if (use_p)
+    *use_p = single_use;
+
+  if (stmt)
+    *stmt = single_use ? single_use->loc.stmt : NULL;
+
+  return !!single_use;
+}
+
 /* Replaces all uses of NAME by VAL.  */
 
 void
@@ -4137,7 +4181,12 @@ verify_gimple_phi (gimple stmt)
 static bool
 verify_gimple_debug (gimple stmt ATTRIBUTE_UNUSED)
 {
-  /* ??? FIXME.  */
+  /* There isn't much that could be wrong in a gimple debug stmt.  A
+     gimple debug bind stmt, for example, maps a tree, that's usually
+     a VAR_DECL or a PARM_DECL, but that could also be some scalarized
+     component or member of an aggregate type, to another tree, that
+     can be an arbitrary expression.  These stmts expand into debug
+     insns, and are converted to debug notes by var-tracking.c.  */
   return false;
 }
 
Index: gcc/tree-flow-inline.h
===================================================================
--- gcc/tree-flow-inline.h.orig	2009-08-31 18:20:57.000000000 -0300
+++ gcc/tree-flow-inline.h	2009-08-31 19:14:32.000000000 -0300
@@ -358,100 +358,88 @@ next_readonly_imm_use (imm_use_iterator 
   return imm->imm_use;
 }
 
-/* Return true if VAR has no uses.  */
+/* tree-cfg.c */
+extern bool has_zero_uses_1 (const ssa_use_operand_t *head);
+extern bool single_imm_use_1 (const ssa_use_operand_t *head,
+			      use_operand_p *use_p, gimple *stmt);
+
+/* Return true if VAR has no nondebug uses.  */
 static inline bool
 has_zero_uses (const_tree var)
 {
-  const ssa_use_operand_t *ptr, *start;
-  bool ret;
+  const ssa_use_operand_t *const ptr = &(SSA_NAME_IMM_USE_NODE (var));
 
-  ptr = &(SSA_NAME_IMM_USE_NODE (var));
-  /* A single use means there is no items in the list.  */
-  ret = (ptr == ptr->next);
-
-  if (ret || !MAY_HAVE_DEBUG_STMTS)
-    return ret;
-
-  start = ptr;
-  for (ptr = start->next; ptr != start; ptr = ptr->next)
-    if (!is_gimple_debug (USE_STMT (ptr)))
-      return false;
-  return true;
+  /* A single use_operand means there is no items in the list.  */
+  if (ptr == ptr->next)
+    return true;
+
+  /* If there are debug stmts, we have to look at each use and see
+     whether there are any nondebug uses.  */
+  if (!MAY_HAVE_DEBUG_STMTS)
+    return false;
+
+  return has_zero_uses_1 (ptr);
 }
 
-/* Return true if VAR has a single use.  */
+/* Return true if VAR has a single nondebug use.  */
 static inline bool
 has_single_use (const_tree var)
 {
-  const ssa_use_operand_t *ptr, *start;
-  bool ret;
+  const ssa_use_operand_t *const ptr = &(SSA_NAME_IMM_USE_NODE (var));
 
-  ptr = &(SSA_NAME_IMM_USE_NODE (var));
-  /* A single use means there is one item in the list.  */
-  ret = (ptr != ptr->next && ptr == ptr->next->next);
+  /* If there aren't any uses whatsoever, we're done.  */
+  if (ptr == ptr->next)
+    return false;
 
-  if (ret)
+  /* If there's a single use, check that it's not a debug stmt.  */
+  if (ptr == ptr->next->next)
     return !is_gimple_debug (USE_STMT (ptr->next));
-  else if (!MAY_HAVE_DEBUG_STMTS)
-    return ret;
 
-  start = ptr;
-  for (ptr = start->next; ptr != start; ptr = ptr->next)
-    if (!is_gimple_debug (USE_STMT (ptr)))
-      {
-	if (ret)
-	  return false;
-	ret = true;
-      }
-  return ret;
+  /* If there are debug stmts, we have to look at each of them.  */
+  if (!MAY_HAVE_DEBUG_STMTS)
+    return false;
+
+  return single_imm_use_1 (ptr, NULL, NULL);
 }
 
 
-/* If VAR has only a single immediate use, return true, and set USE_P and STMT
-   to the use pointer and stmt of occurrence.  */
+/* If VAR has only a single immediate nondebug use, return true, and
+   set USE_P and STMT to the use pointer and stmt of occurrence.  */
 static inline bool
 single_imm_use (const_tree var, use_operand_p *use_p, gimple *stmt)
 {
-  const ssa_use_operand_t *ptr;
-  bool ret;
-
-  ptr = &(SSA_NAME_IMM_USE_NODE (var));
-
-  ret = ptr != ptr->next && ptr == ptr->next->next;
+  const ssa_use_operand_t *const ptr = &(SSA_NAME_IMM_USE_NODE (var));
 
-  if (ret)
-    ret = !is_gimple_debug (USE_STMT (ptr->next));
-  else if (MAY_HAVE_DEBUG_STMTS)
+  /* If there aren't any uses whatsoever, we're done.  */
+  if (ptr == ptr->next)
     {
-      const ssa_use_operand_t *start = ptr, *prev = ptr, *single_use_prev = 0;
-
-      for (ptr = start->next; ptr != start; prev = ptr, ptr = ptr->next)
-	if (!is_gimple_debug (USE_STMT (ptr)))
-	  {
-	    if (ret)
-	      {
-		ret = false;
-		break;
-	      }
-	    ret = true;
-	    single_use_prev = prev;
-	  }
-
-      ptr = single_use_prev;
+    return_false:
+      *use_p = NULL_USE_OPERAND_P;
+      *stmt = NULL;
+      return false;
     }
 
-  if (ret)
+  /* If there's a single use, check that it's not a debug stmt.  */
+  if (ptr == ptr->next->next)
     {
-      *use_p = ptr->next;
-      *stmt = ptr->next->loc.stmt;
-      return true;
+      if (!is_gimple_debug (USE_STMT (ptr->next)))
+	{
+	  *use_p = ptr->next;
+	  *stmt = ptr->next->loc.stmt;
+	  return true;
+	}
+      else
+	goto return_false;
     }
-  *use_p = NULL_USE_OPERAND_P;
-  *stmt = NULL;
-  return false;
+
+  /* If there are debug stmts, we have to look at each of them.  */
+  if (!MAY_HAVE_DEBUG_STMTS)
+    goto return_false;
+
+  return single_imm_use_1 (ptr, use_p, stmt);
 }
 
-/* Return the number of immediate uses of VAR.  */
+/* Return the number of nondebug immediate uses of VAR.  */
 static inline unsigned int
 num_imm_uses (const_tree var)
 {
Index: gcc/tree-ssa-loop-ivopts.c
===================================================================
--- gcc/tree-ssa-loop-ivopts.c.orig	2009-08-31 17:55:38.000000000 -0300
+++ gcc/tree-ssa-loop-ivopts.c	2009-08-31 17:55:40.000000000 -0300
@@ -1850,7 +1850,7 @@ find_interesting_uses (struct ivopts_dat
 	find_interesting_uses_stmt (data, gsi_stmt (bsi));
       for (bsi = gsi_start_bb (bb); !gsi_end_p (bsi); gsi_next (&bsi))
 	if (!is_gimple_debug (gsi_stmt (bsi)))
-	find_interesting_uses_stmt (data, gsi_stmt (bsi));
+	  find_interesting_uses_stmt (data, gsi_stmt (bsi));
     }
 
   if (dump_file && (dump_flags & TDF_DETAILS))
Index: gcc/tree-ssa.c
===================================================================
--- gcc/tree-ssa.c.orig	2009-08-31 18:52:01.000000000 -0300
+++ gcc/tree-ssa.c	2009-08-31 19:16:07.000000000 -0300
@@ -262,12 +262,6 @@ target_for_debug_bind (tree var)
   if (DECL_HAS_VALUE_EXPR_P (var))
     return target_for_debug_bind (DECL_VALUE_EXPR (var));
 
-#if 0
-  /* Should we deal with DECL_DEBUG_EXPR_IS_FROM as well?  */
-  if (DECL_DEBUG_EXPR_IS_FROM (var))
-    return target_for_debug_bind (DECL_DEBUG_EXPR (var));
-#endif
-
   if (DECL_IGNORED_P (var))
     return NULL_TREE;
 
@@ -439,7 +433,7 @@ propagate_defs_into_debug_stmts (gimple 
   if (!MAY_HAVE_DEBUG_STMTS)
     return;
 
-  FOR_EACH_SSA_DEF_OPERAND (def_p, def, op_iter, SSA_OP_ALL_DEFS)
+  FOR_EACH_SSA_DEF_OPERAND (def_p, def, op_iter, SSA_OP_DEF)
     {
       tree var = DEF_FROM_PTR (def_p);
 
Index: gcc/doc/invoke.texi
===================================================================
--- gcc/doc/invoke.texi.orig	2009-08-31 19:27:45.000000000 -0300
+++ gcc/doc/invoke.texi	2009-08-31 19:29:42.000000000 -0300
@@ -4398,11 +4398,14 @@ assembler (GAS) to fail with an error.
 @opindex gdwarf-@var{version}
 Produce debugging information in DWARF format (if that is
 supported).  This is the format used by DBX on IRIX 6.  The value
-of @var{version} may be either 2 or 3; the default version is 2.
+of @var{version} may be either 2, 3 or 4; the default version is 2.
 
 Note that with DWARF version 2 some ports require, and will always
 use, some non-conflicting DWARF 3 extensions in the unwind tables.
 
+Version 4 may require GDB 7.0 and @option{-fvar-tracking-assignments}
+for maximum benefit.
+
 @item -gvms
 @opindex gvms
 Produce debugging information in VMS debug format (if that is
@@ -5456,7 +5459,7 @@ the debug info format supports it.
 Annotate assignments to user variables early in the compilation and
 attempt to carry the annotations over throughout the compilation all the
 way to the end, in an attempt to improve debug information while
-optimizing.
+optimizing.  Use of @option{-gdwarf-4} is recommended along with it.
 
 It can be enabled even if var-tracking is disabled, in which case
 annotations will be created and maintained, but discarded at the end.
Index: gcc/opts.c
===================================================================
--- gcc/opts.c.orig	2009-08-31 19:20:21.000000000 -0300
+++ gcc/opts.c	2009-08-31 19:20:25.000000000 -0300
@@ -2054,7 +2054,7 @@ common_handle_option (size_t scode, cons
       break;
 
     case OPT_gdwarf_:
-      if (value < 2 || value > 3)
+      if (value < 2 || value > 4)
 	error ("dwarf version %d is not supported", value);
       else
 	dwarf_version = value;

[-- Attachment #3: Type: text/plain, Size: 258 bytes --]



-- 
Alexandre Oliva, freedom fighter    http://FSFLA.org/~lxoliva/
You must be the change you wish to see in the world. -- Gandhi
Be Free! -- http://FSFLA.org/   FSF Latin America board member
Free Software Evangelist      Red Hat Brazil Compiler Engineer

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [vta->trunk] VTA merge
  2009-08-31  8:42 David Edelsohn
  2009-08-31 15:31 ` Jan Hubicka
  2009-08-31 16:34 ` Jakub Jelinek
@ 2009-08-31 17:03 ` Joseph S. Myers
  2 siblings, 0 replies; 52+ messages in thread
From: Joseph S. Myers @ 2009-08-31 17:03 UTC (permalink / raw)
  To: David Edelsohn
  Cc: Jack Howarth, Steven Bosscher, Tom Tromey, Richard Guenther,
	Alexandre Oliva, gcc-patches

On Sun, 30 Aug 2009, David Edelsohn wrote:

> Does this mean that older versions of GDB will not work with the new debugging
> information?  I do not expect older versions of GDB to use the additional
> information, but I expected that they at least would ignore it and
> work as before.

I would expect GCC to continue to generate debug information that is 
correctly formatted according to the relevant standard and in which any 
extensions to the standard are such that they can be ignored by consumers 
(including non-GDB debuggers) that do not understand them, but not 
necessarily to work around bugs in older versions of GDB or other 
consumers; whether to work around a particular bug would need to be 
decided on a case by case basis, taking account of the impact of the bug 
if not worked around and how old the versions with the bug are.  By 
specifying a minimum GDB version we may say that bugs only present in 
older versions need not be worked around.

-- 
Joseph S. Myers
joseph@codesourcery.com

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [vta->trunk] VTA merge
  2009-08-31 16:39   ` Alexandre Oliva
@ 2009-08-31 16:45     ` Richard Guenther
  2009-08-31 23:22       ` Alexandre Oliva
  0 siblings, 1 reply; 52+ messages in thread
From: Richard Guenther @ 2009-08-31 16:45 UTC (permalink / raw)
  To: Alexandre Oliva; +Cc: gcc-patches

On Mon, Aug 31, 2009 at 10:42 AM, Alexandre Oliva<aoliva@redhat.com> wrote:
> On Aug 30, 2009, Richard Guenther <richard.guenther@gmail.com> wrote:
>
>> On Sat, Aug 29, 2009 at 6:04 AM, Alexandre Oliva<aoliva@redhat.com> wrote:
>>         * expr.c (get_inner_reference): Tolerate NULL inner exp.
>
>> this should be no longer necessary after the alias-export merge.
>
> I'll check.
>
>>         * tree-inline.h: Include varray.h.
>
>> hmm.  You are using a VARRAY?  I'm sure I have complained about this
>> already - please use a VEC.
>
> I misunderstood your complaint as being only about the need for
> deferring the remapping of debug decls.  I'll switch to vecs.

Thanks.

>> Your changelog gets garbled starting with config/rs6000/rs6000.c - an
>> extra leading space on each line.
>
> Oops.  Thanks, fixed.
>
>> What gdb version is recommended to be able to benefit from
>> -fvar-tracking-assignments?
>
> Any gdb version should benefit.
>
>> In particular do you in this patchset output
>> expressions with the new dwarf4 stuff?
>
> Yup.  The code that Jakub added for DW_OP_implicit_value and
> DW_OP_stack_value is unconditional, and I expect this should be fine,
> for it is supposed to be backward-compatible.  Do you have any evidence
> that it isn't?

No.  It would be nice to have it documented which gdb version supports
them though.  It can be in a separate section, but I don't remember if
we have a general debugging overview doc.

>> Can you expand the documentation for the switch if that is the case?
>
> You mean the -fvar-tracking-assignments switch?  No, I don't think that
> would be apropriate.  I understand GCC might emit the new expression
> types even without VTA, given a suitable initializer for the variable.
>
>> How was this patch tested?  In particular which of the primary targets
>> passed bootstrap and regtesting?
>
> Sorry, I failed to include that information, but Jakub filled it in.
>
>> -fvar-tracking-assignments seems to be off by default.
>
> Yup.  There's a separate patch that turns it on by default, if you'd
> like to approve it.  I'd like to keep them separate, so that it's easy
> to revert to the current default in case we decide it shouldn't be
> enabled.

You can install it as followup - it's hereby approved.  And indeed we
can easily revert it.

>> +/* Nonzero if is_gimple_debug() may possibly hold.  */
>> +#define MAY_HAVE_DEBUG_STMTS    (flag_var_tracking_assignments)
>
>> I don't remember exactly the outcome of the last discussion, but I
>> at least remember asking you to
>> s/MAY_HAVE_DEBUG_STMTS/flag_var_tracking_assignments/g
>
> You did, and I pointed out that would have to be changed back as soon as
> I added another type of debug stmt that I have planned.  The abstraction
> is ready for that, and it is a solid abstraction.

No, it says MAY_HAVE_DEBUG_STMTS, not MAY_HAVE_DEBUG_BIND.

Frakly I don't think it is sound to introduce abstraction before you make
use of it.

>> this seems to be the only case (sofar) you check gimple_debug_bind_p
>> instead of is_gimple_debug.  As there is no difference at the moment
>> using sometimes one and sometimes the other looks bogus.  Please
>> eliminate gimple_debug_bind_p in favor of is_gimple_debug.
>
> Likewise.

Likewise?  I cannot see how at some places using is_gimple_debug
and at some places using gimple_debug_bind_p can be correct.  Once
you introduce some other debug stmt variant how can you decide what
and why one or the other is correct.

Please change all of them to is_gimple_debug.

As above, extra abstraction without a use is not ok.  It's easy to
re-instantiate
the abstraction if you need it later - at the moment it is just confusing
to people that have to deal with it.

Please remove it.  Thanks.

>> Note that the GIMPLE_DEBUG_BIND as gf_mask is even more bogus
>> that the previous additional tree code.  Just use literal zero in the
>> single place where it is necessary and remove the debug bind
>> vs. debug stmt confusion.
>
> Likewise.  (Why do you say it is bogus?)

Because you amend a bit flags enum with something completely
different?  If you in future re-introduce the abstraction simply create
a new enum, but don't reuse the flags one.

>> The has_zero_uses, has_single_use, etc. helpers no longer seem
>> suitable for inlining.  As a followup please split them into a inlined
>> head and a tail.
>
> ACK.
>
>> The debug rewrites in terms of the boolean previous result also makes
>> them hard to follow (as you didn't adjust the comments).
>
> I don't understand what you're saying here.  Can you try to say it using
> different words?

 /* Return true if VAR has a single use.  */
 static inline bool
 has_single_use (const_tree var)
 {
-  const ssa_use_operand_t *const ptr = &(SSA_NAME_IMM_USE_NODE (var));
+  const ssa_use_operand_t *ptr, *start;
+  bool ret;
+
+  ptr = &(SSA_NAME_IMM_USE_NODE (var));
   /* A single use means there is one item in the list.  */
-  return (ptr != ptr->next && ptr == ptr->next->next);
+  ret = (ptr != ptr->next && ptr == ptr->next->next);
+
+  if (ret)
+    return !is_gimple_debug (USE_STMT (ptr->next));
+  else if (!MAY_HAVE_DEBUG_STMTS)
+    return ret;
+
+  start = ptr;
+  for (ptr = start->next; ptr != start; ptr = ptr->next)
+    if (!is_gimple_debug (USE_STMT (ptr)))
+      {
+       if (ret)
+       ret = true;
+      }
+  return ret;
 }

The unchanged comment /* A single use means there is one item in the list.  */
is no longer true.  It should at least read /* A single non-debug use ... */.

Moreover I think a better way to write this is to do

 /* Return true if VAR has a single use.  */
 static inline bool
 has_single_use (const_tree var)
 {
-  const ssa_use_operand_t *const ptr = &(SSA_NAME_IMM_USE_NODE (var));
+  const ssa_use_operand_t *ptr, *start;
+  bool seen_nondebug = false;
+
+  ptr = &(SSA_NAME_IMM_USE_NODE (var));

   if (ptr == ptr->next)
     return false;

   /* A single non-debug use means there is one item in the list.  */
   if (ptr == ptr->next->next)
     return (!MAY_HAVE_DEBUG_STMTS || !is_gimple_debug (USE_STMT (ptr)));

+  start = ptr;
    while (ptr != start)
        {
           if (!is_gimple_debug (USE_STMT (ptr)))
             {
               if (seen_nondebug)
                 return false;
               seen_nondebug = true;
             }
            ptr = ptr->next;
        }

     return seen_nondebug;
}

where up to the ptr == ptr->next->next check you'd inline the thing.  The tail
loop can probably be shared in the various predicates if it just returns the
number of non-debug stmts.

>> +#if 0
>> +  /* Should we deal with DECL_DEBUG_EXPR_IS_FROM as well?  */
>> +  if (DECL_DEBUG_EXPR_IS_FROM (var))
>> +    return target_for_debug_bind (DECL_DEBUG_EXPR (var));
>> +#endif
>
>> remove #if 0 blocks.
>
> Uhh...  Ok.
>
>> +void
>> +propagate_defs_into_debug_stmts (gimple def, basic_block tobb,
>> +                                const gimple_stmt_iterator *togsip)
>> ...
>> +  FOR_EACH_SSA_DEF_OPERAND (def_p, def, op_iter, SSA_OP_ALL_DEFS)
>
>> why do you propagate virtual SSA_NAMEs here?
>
> Cut&pasto, I guess.
>
>> IMHO you want to use SSA_OP_DEF here
>
> Agreed.  It's not like it makes much of a difference in terms of
> correctness (it would never find debug stmts that use a virtual
> SSA_NAME), but it could bring some speed up.  Thanks.
>
>> (maybe this mistake is also somewhere else).
>
> This is the only addition of FOR_EACH_SSA_DEF_OPERAND in the patch.  Did
> you have something else in mind?

No, I just didn't want to go back and see if I missed another case.

>> +/* Process deferred debug stmts.  In order to give values better odds
>> +   of being successfully remapped, we delay the processing of debug
>> +   stmts.  */
>
>> "better odds of being successfully remapped"?  Well, ok ... I'll take
>> your word for it.
>
> Yup.  If you postpone the check for whether the decls have already been
> remapped till the end of the function, you get better odds of finding
> themit already mapped than deciding right away.
>
>> +insert_init_debug_bind (copy_body_data *id,
>> +                       basic_block bb, tree var, tree value,
>> +                       gimple base_stmt)
>> +{
>> ...
>> +  mark_symbols_for_renaming (note);
>
>> really?  I can't see why.  As I likely have asked this previously a
>> comment before the call looks useful.
>
> It might be a left-over from copying some code from elsewhere in a
> distant past.  I don't know why it might be necessary.  I'll try without
> it.

Thanks.

>> +/* Verify a gimple debug statement STMT.
>> +   Returns true if anything is wrong.  */
>> +
>> +static bool
>> +verify_gimple_debug (gimple stmt ATTRIBUTE_UNUSED)
>> +{
>> +  /* ??? FIXME.  */
>> +  return false;
>> +}
>
>> ;)  Please add some very basic stuff, like the lhs should be a plain
>> var or parm decl
>
> Why?  It might very well be a (scalarized) component ref.  We don't
> generate or handle that elsewhere, but there's no reason why it should
> be limited to what you suggest.
>
>> and the rhs should be (err, what should it be?).
>
> Anything whatsoever.  Or nothing.
>
>> Even a comment explaining what to check would be ok.
>
> Yeah, the FIXME is bad ;-)

A comment is fine with me - maybe with a pointer to the code that
will end up interpreting the stmts.

>> The tree middle-end changes are ok with the above changes
>> if all the bootstrapping and regtesting works out ok.
>
> Thanks.
>
>> You need (if you didn't already get) approval for the C++ parts,
>> the testsuite parts, the build system changes and the RTL
>> middle-end pieces.
>
> RTL is taken care of.  C++ and testsuite can be easily split out (they
> were, before), so I won't hold off the whole patch because of them.
>
>> Please announce the merge 24h in advance and ask the
>> trunk to be frozen.
>
> I don't quite see a need for a freeze.  This can be treated as a single
> (although big) patch check in.

It would be useful to have automatic testers pick up the rev before
the merge, the merge and then followup changes.  So I think a short
freeze won't hurt.

Richard.

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [vta->trunk] VTA merge
  2009-08-30 17:18 ` Richard Guenther
  2009-08-30 20:44   ` Tom Tromey
@ 2009-08-31 16:39   ` Alexandre Oliva
  2009-08-31 16:45     ` Richard Guenther
  2009-09-02  2:43   ` Alexandre Oliva
  2 siblings, 1 reply; 52+ messages in thread
From: Alexandre Oliva @ 2009-08-31 16:39 UTC (permalink / raw)
  To: Richard Guenther; +Cc: gcc-patches

On Aug 30, 2009, Richard Guenther <richard.guenther@gmail.com> wrote:

> On Sat, Aug 29, 2009 at 6:04 AM, Alexandre Oliva<aoliva@redhat.com> wrote:
>         * expr.c (get_inner_reference): Tolerate NULL inner exp.

> this should be no longer necessary after the alias-export merge.

I'll check.

>         * tree-inline.h: Include varray.h.

> hmm.  You are using a VARRAY?  I'm sure I have complained about this
> already - please use a VEC.

I misunderstood your complaint as being only about the need for
deferring the remapping of debug decls.  I'll switch to vecs.

> Your changelog gets garbled starting with config/rs6000/rs6000.c - an
> extra leading space on each line.

Oops.  Thanks, fixed.

> What gdb version is recommended to be able to benefit from
> -fvar-tracking-assignments?

Any gdb version should benefit.

> In particular do you in this patchset output
> expressions with the new dwarf4 stuff?

Yup.  The code that Jakub added for DW_OP_implicit_value and
DW_OP_stack_value is unconditional, and I expect this should be fine,
for it is supposed to be backward-compatible.  Do you have any evidence
that it isn't?

> Can you expand the documentation for the switch if that is the case?

You mean the -fvar-tracking-assignments switch?  No, I don't think that
would be apropriate.  I understand GCC might emit the new expression
types even without VTA, given a suitable initializer for the variable.

> How was this patch tested?  In particular which of the primary targets
> passed bootstrap and regtesting?

Sorry, I failed to include that information, but Jakub filled it in.

> -fvar-tracking-assignments seems to be off by default.

Yup.  There's a separate patch that turns it on by default, if you'd
like to approve it.  I'd like to keep them separate, so that it's easy
to revert to the current default in case we decide it shouldn't be
enabled.

> +/* Nonzero if is_gimple_debug() may possibly hold.  */
> +#define MAY_HAVE_DEBUG_STMTS    (flag_var_tracking_assignments)

> I don't remember exactly the outcome of the last discussion, but I
> at least remember asking you to
> s/MAY_HAVE_DEBUG_STMTS/flag_var_tracking_assignments/g

You did, and I pointed out that would have to be changed back as soon as
I added another type of debug stmt that I have planned.  The abstraction
is ready for that, and it is a solid abstraction.

> this seems to be the only case (sofar) you check gimple_debug_bind_p
> instead of is_gimple_debug.  As there is no difference at the moment
> using sometimes one and sometimes the other looks bogus.  Please
> eliminate gimple_debug_bind_p in favor of is_gimple_debug.

Likewise.

> Note that the GIMPLE_DEBUG_BIND as gf_mask is even more bogus
> that the previous additional tree code.  Just use literal zero in the
> single place where it is necessary and remove the debug bind
> vs. debug stmt confusion.

Likewise.  (Why do you say it is bogus?)

> The has_zero_uses, has_single_use, etc. helpers no longer seem
> suitable for inlining.  As a followup please split them into a inlined
> head and a tail.

ACK.

> The debug rewrites in terms of the boolean previous result also makes
> them hard to follow (as you didn't adjust the comments).

I don't understand what you're saying here.  Can you try to say it using
different words?

> +#if 0
> +  /* Should we deal with DECL_DEBUG_EXPR_IS_FROM as well?  */
> +  if (DECL_DEBUG_EXPR_IS_FROM (var))
> +    return target_for_debug_bind (DECL_DEBUG_EXPR (var));
> +#endif

> remove #if 0 blocks.

Uhh...  Ok.

> +void
> +propagate_defs_into_debug_stmts (gimple def, basic_block tobb,
> +                                const gimple_stmt_iterator *togsip)
> ...
> +  FOR_EACH_SSA_DEF_OPERAND (def_p, def, op_iter, SSA_OP_ALL_DEFS)

> why do you propagate virtual SSA_NAMEs here?

Cut&pasto, I guess.

> IMHO you want to use SSA_OP_DEF here

Agreed.  It's not like it makes much of a difference in terms of
correctness (it would never find debug stmts that use a virtual
SSA_NAME), but it could bring some speed up.  Thanks.

> (maybe this mistake is also somewhere else).

This is the only addition of FOR_EACH_SSA_DEF_OPERAND in the patch.  Did
you have something else in mind?

> +/* Process deferred debug stmts.  In order to give values better odds
> +   of being successfully remapped, we delay the processing of debug
> +   stmts.  */

> "better odds of being successfully remapped"?  Well, ok ... I'll take
> your word for it.

Yup.  If you postpone the check for whether the decls have already been
remapped till the end of the function, you get better odds of finding
themit already mapped than deciding right away.

> +insert_init_debug_bind (copy_body_data *id,
> +                       basic_block bb, tree var, tree value,
> +                       gimple base_stmt)
> +{
> ...
> +  mark_symbols_for_renaming (note);

> really?  I can't see why.  As I likely have asked this previously a
> comment before the call looks useful.

It might be a left-over from copying some code from elsewhere in a
distant past.  I don't know why it might be necessary.  I'll try without
it.

> +/* Verify a gimple debug statement STMT.
> +   Returns true if anything is wrong.  */
> +
> +static bool
> +verify_gimple_debug (gimple stmt ATTRIBUTE_UNUSED)
> +{
> +  /* ??? FIXME.  */
> +  return false;
> +}

> ;)  Please add some very basic stuff, like the lhs should be a plain
> var or parm decl

Why?  It might very well be a (scalarized) component ref.  We don't
generate or handle that elsewhere, but there's no reason why it should
be limited to what you suggest.

> and the rhs should be (err, what should it be?).

Anything whatsoever.  Or nothing.

> Even a comment explaining what to check would be ok.

Yeah, the FIXME is bad ;-)

> The tree middle-end changes are ok with the above changes
> if all the bootstrapping and regtesting works out ok.

Thanks.

> You need (if you didn't already get) approval for the C++ parts,
> the testsuite parts, the build system changes and the RTL
> middle-end pieces.

RTL is taken care of.  C++ and testsuite can be easily split out (they
were, before), so I won't hold off the whole patch because of them.

> Please announce the merge 24h in advance and ask the
> trunk to be frozen.

I don't quite see a need for a freeze.  This can be treated as a single
(although big) patch check in.

-- 
Alexandre Oliva, freedom fighter    http://FSFLA.org/~lxoliva/
You must be the change you wish to see in the world. -- Gandhi
Be Free! -- http://FSFLA.org/   FSF Latin America board member
Free Software Evangelist      Red Hat Brazil Compiler Engineer

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [vta->trunk] VTA merge
  2009-08-31  8:42 David Edelsohn
  2009-08-31 15:31 ` Jan Hubicka
@ 2009-08-31 16:34 ` Jakub Jelinek
  2009-08-31 17:03 ` Joseph S. Myers
  2 siblings, 0 replies; 52+ messages in thread
From: Jakub Jelinek @ 2009-08-31 16:34 UTC (permalink / raw)
  To: David Edelsohn
  Cc: Joseph S. Myers, Jack Howarth, Steven Bosscher, Tom Tromey,
	Richard Guenther, Alexandre Oliva, gcc-patches

On Sun, Aug 30, 2009 at 04:44:34PM -0400, David Edelsohn wrote:
> Does this mean that older versions of GDB will not work with the new debugging
> information?  I do not expect older versions of GDB to use the additional
> information, but I expected that they at least would ignore it and
> work as before.

With the GDB versions I've tried, GDB just won't be able to print the
variables that use the new operations in the location lists, like:
(gdb) p j
Unhandled dwarf expression opcode 0x9f
(gdb) p k
$1 = 6

Without VTA it would print:
$1 = <value optimized out>
instead.  That's both with
GNU gdb (GDB) Fedora (6.8.50.20090302-37.fc11)
and
GNU gdb (GDB) 6.8.50.20090723-cvs

> What is the effect of VTA on non-Dwarf debugging targets, such as AIX, which
> uses stabs-like debugging.  Again, I do not expect the additional information
> to be recorded in stabs, but I do expect stabs debugging to continue to work.

It should have no effect on non-DWARF debugging targets.  Similarly to
-fvar-tracking, it defaults to off on those targets, and even if you
explicitely enable the var tracking resp. var assignments tracking, it will
be tracked and then completely ignored during the debuginfo output.

	Jakub

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [vta->trunk] VTA merge
  2009-08-31  8:42 David Edelsohn
@ 2009-08-31 15:31 ` Jan Hubicka
  2009-08-31 16:34 ` Jakub Jelinek
  2009-08-31 17:03 ` Joseph S. Myers
  2 siblings, 0 replies; 52+ messages in thread
From: Jan Hubicka @ 2009-08-31 15:31 UTC (permalink / raw)
  To: David Edelsohn
  Cc: Joseph S. Myers, Jack Howarth, Steven Bosscher, Tom Tromey,
	Richard Guenther, Alexandre Oliva, gcc-patches

> > >    What I mean was for all platforms that might have older gdb's. When
> > > VTA is merged will there be any legacy support for older gdb's or will
> > > everyone have to upgrade gdb to debug code compiled in the gcc 4.5?
> >
> > I think it is reasonable for a given GCC version to declare a global
> > minimum version of GDB for debugging GCC-compiled code with GDB - and a
> > global minimum version of Binutils for compiling for systems using GNU
> > Binutils - and that declaring such versions based on recent stable
> > releases of those tools would be better than the present situation of a
> > few, probably inaccurate, explicit statements about versions for given
> > targets and likely many undocumented requirements.  It seems likely that
> > GDB 7.0 will be out well before GCC 4.5 and could be a suitable GDB
> > version to specify.
> >
> > It is for those who care about a particular system to ensure that FSF GDB
> > support for it works in current versions of FSF GDB rather than just in
> > vendor patched versions of some old version.  There are systems with GDB
> > support in vendor patched versions but not FSF GDB, but Darwin (at least
> > for IA32) is not one of them since AdaCore has added the required support
> > to FSF GDB.
> 
> Does this mean that older versions of GDB will not work with the new debugging
> information?  I do not expect older versions of GDB to use the additional
> information, but I expected that they at least would ignore it and
> work as before.

I can't speak much for VTA, but at least with my experience on inline
substitution and SRA tracking (I intend to submit too in next week(s).
It is pretty much complementary to VTA.) and older GDBs (up to all
official 6.x releases) is that sometimes the GDB dwarf interpretor got
confused by the (valid) dwarf2 GCC now produce.  This results in DWARF2
stack machine errors instead of "value optimized out" or wrong value
message that sounds pretty much acceptable to me.

THere are some new DWARF3 features, DW_bit_piece and DW_OP_value that
both VTA and my patches use that are not supported by current GDB
releases.  I certainly hope that GDB 7.0 will get it and I would like to
see DW_OP_value support enabled by default. It makes huge difference in
the expressive power (i.e. any optimized out value whose actual value
can be computed as expression can be done only using DW_OP_value).

But at least none in my work affects stabs or any other non-dwarf output
format unless we chose to implement there the VALUE_EXPR handling.

Honza

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [vta->trunk] VTA merge
@ 2009-08-31  8:42 David Edelsohn
  2009-08-31 15:31 ` Jan Hubicka
                   ` (2 more replies)
  0 siblings, 3 replies; 52+ messages in thread
From: David Edelsohn @ 2009-08-31  8:42 UTC (permalink / raw)
  To: Joseph S. Myers
  Cc: Jack Howarth, Steven Bosscher, Tom Tromey, Richard Guenther,
	Alexandre Oliva, gcc-patches

> >    What I mean was for all platforms that might have older gdb's. When
> > VTA is merged will there be any legacy support for older gdb's or will
> > everyone have to upgrade gdb to debug code compiled in the gcc 4.5?
>
> I think it is reasonable for a given GCC version to declare a global
> minimum version of GDB for debugging GCC-compiled code with GDB - and a
> global minimum version of Binutils for compiling for systems using GNU
> Binutils - and that declaring such versions based on recent stable
> releases of those tools would be better than the present situation of a
> few, probably inaccurate, explicit statements about versions for given
> targets and likely many undocumented requirements.  It seems likely that
> GDB 7.0 will be out well before GCC 4.5 and could be a suitable GDB
> version to specify.
>
> It is for those who care about a particular system to ensure that FSF GDB
> support for it works in current versions of FSF GDB rather than just in
> vendor patched versions of some old version.  There are systems with GDB
> support in vendor patched versions but not FSF GDB, but Darwin (at least
> for IA32) is not one of them since AdaCore has added the required support
> to FSF GDB.

Does this mean that older versions of GDB will not work with the new debugging
information?  I do not expect older versions of GDB to use the additional
information, but I expected that they at least would ignore it and
work as before.

What is the effect of VTA on non-Dwarf debugging targets, such as AIX, which
uses stabs-like debugging.  Again, I do not expect the additional information
to be recorded in stabs, but I do expect stabs debugging to continue to work.

Thanks, David

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [vta->trunk] VTA merge
  2009-08-31  6:30         ` Jack Howarth
@ 2009-08-31  6:41           ` Joseph S. Myers
  2009-09-01 21:17             ` Alexandre Oliva
  0 siblings, 1 reply; 52+ messages in thread
From: Joseph S. Myers @ 2009-08-31  6:41 UTC (permalink / raw)
  To: Jack Howarth
  Cc: Steven Bosscher, Tom Tromey, Richard Guenther, Alexandre Oliva,
	gcc-patches

On Sun, 30 Aug 2009, Jack Howarth wrote:

> On Sun, Aug 30, 2009 at 06:40:02PM +0200, Steven Bosscher wrote:
> > On Sun, Aug 30, 2009 at 5:56 PM, Jack Howarth<howarth@bromo.med.uc.edu> wrote:
> > >
> > > Will merging in VTA break the use of older gdb releases? Apple is still
> > > using the gdb 6.3.50-20050815 release for Snow Leopard (although it may
> > > well be patched newer than that indicates).
> > 
> > Well, no offense, but IMHO Apple is the last thing we should worry
> > about with this VTA stuff.
> > 
> > Ciao!
> > Steven
> 
> Steven,
>    What I mean was for all platforms that might have older gdb's. When
> VTA is merged will there be any legacy support for older gdb's or will
> everyone have to upgrade gdb to debug code compiled in the gcc 4.5?

I think it is reasonable for a given GCC version to declare a global 
minimum version of GDB for debugging GCC-compiled code with GDB - and a 
global minimum version of Binutils for compiling for systems using GNU 
Binutils - and that declaring such versions based on recent stable 
releases of those tools would be better than the present situation of a 
few, probably inaccurate, explicit statements about versions for given 
targets and likely many undocumented requirements.  It seems likely that 
GDB 7.0 will be out well before GCC 4.5 and could be a suitable GDB 
version to specify.

It is for those who care about a particular system to ensure that FSF GDB 
support for it works in current versions of FSF GDB rather than just in 
vendor patched versions of some old version.  There are systems with GDB 
support in vendor patched versions but not FSF GDB, but Darwin (at least 
for IA32) is not one of them since AdaCore has added the required support 
to FSF GDB.

-- 
Joseph S. Myers
joseph@codesourcery.com

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [vta->trunk] VTA merge
  2009-08-31  0:25       ` Steven Bosscher
@ 2009-08-31  6:30         ` Jack Howarth
  2009-08-31  6:41           ` Joseph S. Myers
  0 siblings, 1 reply; 52+ messages in thread
From: Jack Howarth @ 2009-08-31  6:30 UTC (permalink / raw)
  To: Steven Bosscher
  Cc: Tom Tromey, Richard Guenther, Alexandre Oliva, gcc-patches

On Sun, Aug 30, 2009 at 06:40:02PM +0200, Steven Bosscher wrote:
> On Sun, Aug 30, 2009 at 5:56 PM, Jack Howarth<howarth@bromo.med.uc.edu> wrote:
> >
> > Will merging in VTA break the use of older gdb releases? Apple is still
> > using the gdb 6.3.50-20050815 release for Snow Leopard (although it may
> > well be patched newer than that indicates).
> 
> Well, no offense, but IMHO Apple is the last thing we should worry
> about with this VTA stuff.
> 
> Ciao!
> Steven

Steven,
   What I mean was for all platforms that might have older gdb's. When
VTA is merged will there be any legacy support for older gdb's or will
everyone have to upgrade gdb to debug code compiled in the gcc 4.5?
             Jack

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [vta->trunk] VTA merge
  2009-08-30 22:33     ` Jack Howarth
@ 2009-08-31  0:25       ` Steven Bosscher
  2009-08-31  6:30         ` Jack Howarth
  0 siblings, 1 reply; 52+ messages in thread
From: Steven Bosscher @ 2009-08-31  0:25 UTC (permalink / raw)
  To: Jack Howarth; +Cc: Tom Tromey, Richard Guenther, Alexandre Oliva, gcc-patches

On Sun, Aug 30, 2009 at 5:56 PM, Jack Howarth<howarth@bromo.med.uc.edu> wrote:
>
> Will merging in VTA break the use of older gdb releases? Apple is still
> using the gdb 6.3.50-20050815 release for Snow Leopard (although it may
> well be patched newer than that indicates).

Well, no offense, but IMHO Apple is the last thing we should worry
about with this VTA stuff.

Ciao!
Steven

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [vta->trunk] VTA merge
  2009-08-30 20:44   ` Tom Tromey
@ 2009-08-30 22:33     ` Jack Howarth
  2009-08-31  0:25       ` Steven Bosscher
  0 siblings, 1 reply; 52+ messages in thread
From: Jack Howarth @ 2009-08-30 22:33 UTC (permalink / raw)
  To: Tom Tromey; +Cc: Richard Guenther, Alexandre Oliva, gcc-patches

On Sun, Aug 30, 2009 at 09:47:18AM -0600, Tom Tromey wrote:
> >>>>> "Richard" == Richard Guenther <richard.guenther@gmail.com> writes:
> 
> Richard> What gdb version is recommended to be able to benefit from
> Richard> -fvar-tracking-assignments?  In particular do you in this
> Richard> patchset output expressions with the new dwarf4 stuff?
> 
> The new DW_OP_*_value support is only on a branch in the archer
> repository for the time being.  I'll try to push it upstream soon.
> 
> Note that GCC trunk with -gdwarf-3 already requires an uncommitted gdb
> patch, due to the use of DW_OP_call_frame_cfa.  However, I didn't see
> any checks in this patch against dwarf_version, which seems a little
> suspect.  If those checks were added, then this patch would not make the
> choice-of-debugger situation any worse than it already is.
> 
> Tom

Will merging in VTA break the use of older gdb releases? Apple is still
using the gdb 6.3.50-20050815 release for Snow Leopard (although it may
well be patched newer than that indicates).
                  Jack

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [vta->trunk] VTA merge
  2009-08-30 17:18 ` Richard Guenther
@ 2009-08-30 20:44   ` Tom Tromey
  2009-08-30 22:33     ` Jack Howarth
  2009-08-31 16:39   ` Alexandre Oliva
  2009-09-02  2:43   ` Alexandre Oliva
  2 siblings, 1 reply; 52+ messages in thread
From: Tom Tromey @ 2009-08-30 20:44 UTC (permalink / raw)
  To: Richard Guenther; +Cc: Alexandre Oliva, gcc-patches

>>>>> "Richard" == Richard Guenther <richard.guenther@gmail.com> writes:

Richard> What gdb version is recommended to be able to benefit from
Richard> -fvar-tracking-assignments?  In particular do you in this
Richard> patchset output expressions with the new dwarf4 stuff?

The new DW_OP_*_value support is only on a branch in the archer
repository for the time being.  I'll try to push it upstream soon.

Note that GCC trunk with -gdwarf-3 already requires an uncommitted gdb
patch, due to the use of DW_OP_call_frame_cfa.  However, I didn't see
any checks in this patch against dwarf_version, which seems a little
suspect.  If those checks were added, then this patch would not make the
choice-of-debugger situation any worse than it already is.

Tom

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [vta->trunk] VTA merge
  2009-08-30  9:32 Alexandre Oliva
  2009-08-30 17:18 ` Richard Guenther
@ 2009-08-30 17:20 ` Jakub Jelinek
  1 sibling, 0 replies; 52+ messages in thread
From: Jakub Jelinek @ 2009-08-30 17:20 UTC (permalink / raw)
  To: Alexandre Oliva; +Cc: gcc-patches

On Sat, Aug 29, 2009 at 01:04:17AM -0300, Alexandre Oliva wrote:
> I've been on the road and off-line for nearly the whole week, but I
> managed to complete writing the ChangeLog entry for the VTA merge.  It's
> the longest ChangeLog I've ever seen, weighting some 36KB, for a 600KB
> patch.
> 
> It would be ideal to check it in as quickly as possible, because I'm
> going to be at home and on-line for at least the next two weeks, which
> should be plenty of time to fix any issues that might arise.
> 
> So, may I have confirmation of the various approvals I've already got,
> before I addressed the suggestions and requests in them?

I'd like to add that VTA branch has been bootstrapped/regtested recently on
at least x86_64-linux, i686-linux, powerpc64-linux (both
--with-cpu=default32 and defaulting to -m64), ia64-linux, s390x-linux and
arm-eabi.

	Jakub

^ permalink raw reply	[flat|nested] 52+ messages in thread

* Re: [vta->trunk] VTA merge
  2009-08-30  9:32 Alexandre Oliva
@ 2009-08-30 17:18 ` Richard Guenther
  2009-08-30 20:44   ` Tom Tromey
                     ` (2 more replies)
  2009-08-30 17:20 ` Jakub Jelinek
  1 sibling, 3 replies; 52+ messages in thread
From: Richard Guenther @ 2009-08-30 17:18 UTC (permalink / raw)
  To: Alexandre Oliva; +Cc: gcc-patches

On Sat, Aug 29, 2009 at 6:04 AM, Alexandre Oliva<aoliva@redhat.com> wrote:
> Hi,
>
> I've been on the road and off-line for nearly the whole week, but I
> managed to complete writing the ChangeLog entry for the VTA merge.  It's
> the longest ChangeLog I've ever seen, weighting some 36KB, for a 600KB
> patch.
>
> It would be ideal to check it in as quickly as possible, because I'm
> going to be at home and on-line for at least the next two weeks, which
> should be plenty of time to fix any issues that might arise.
>
> So, may I have confirmation of the various approvals I've already got,
> before I addressed the suggestions and requests in them?

        * expr.c (get_inner_reference): Tolerate NULL inner exp.

this should be no longer necessary after the alias-export merge.

        * tree-inline.h: Include varray.h.

hmm.  You are using a VARRAY?  I'm sure I have complained about this
already - please use a VEC.

Your changelog gets garbled starting with config/rs6000/rs6000.c - an
extra leading space on each line.

What gdb version is recommended to be able to benefit from
-fvar-tracking-assignments?  In particular do you in this patchset output
expressions with the new dwarf4 stuff?  Can you expand the documentation
for the switch if that is the case?

How was this patch tested?  In particular which of the primary targets
passed bootstrap and regtesting?

-fvar-tracking-assignments seems to be off by default.  Please consider
turing it on by default (together with -fvar-tracking) if -g is specified - at
least during the remainder of stage1 so we get testing coverage.

+/* Nonzero if is_gimple_debug() may possibly hold.  */
+#define MAY_HAVE_DEBUG_STMTS    (flag_var_tracking_assignments)

I don't remember exactly the outcome of the last discussion, but I
at least remember asking you to
s/MAY_HAVE_DEBUG_STMTS/flag_var_tracking_assignments/g

@@ -2526,6 +2526,11 @@ propagate_rhs_into_lhs (gimple stmt, tre
         be successful would be if the use occurs in an ASM_EXPR.  */
       FOR_EACH_IMM_USE_STMT (use_stmt, iter, lhs)
        {
+         /* Leave debug stmts alone.  If we succeed in propagating
+            all non-debug uses, we'll drop the DEF, and propagation
+            into debug stmts will occur then.  */
+         if (gimple_debug_bind_p (use_stmt))
+           continue;

this seems to be the only case (sofar) you check gimple_debug_bind_p
instead of is_gimple_debug.  As there is no difference at the moment
using sometimes one and sometimes the other looks bogus.  Please
eliminate gimple_debug_bind_p in favor of is_gimple_debug.

Note that the GIMPLE_DEBUG_BIND as gf_mask is even more bogus
that the previous additional tree code.  Just use literal zero in the
single place where it is necessary and remove the debug bind
vs. debug stmt confusion.

The has_zero_uses, has_single_use, etc. helpers no longer seem
suitable for inlining.  As a followup please split them into a inlined
head and a tail.  The debug rewrites in terms of the boolean previous
result also makes them hard to follow (as you didn't adjust the comments).

+#if 0
+  /* Should we deal with DECL_DEBUG_EXPR_IS_FROM as well?  */
+  if (DECL_DEBUG_EXPR_IS_FROM (var))
+    return target_for_debug_bind (DECL_DEBUG_EXPR (var));
+#endif

remove #if 0 blocks.

+void
+propagate_defs_into_debug_stmts (gimple def, basic_block tobb,
+                                const gimple_stmt_iterator *togsip)
...
+  FOR_EACH_SSA_DEF_OPERAND (def_p, def, op_iter, SSA_OP_ALL_DEFS)

why do you propagate virtual SSA_NAMEs here?  IMHO you want to
use SSA_OP_DEF here (maybe this mistake is also somewhere else).

+/* Process deferred debug stmts.  In order to give values better odds
+   of being successfully remapped, we delay the processing of debug
+   stmts.  */
+
+static void
+copy_debug_stmts (copy_body_data *id)
+{
+  size_t i, e;
+
+  if (!id->debug_stmts)
+    return;
+
+  for (i = 0, e = VARRAY_ACTIVE_SIZE (id->debug_stmts); i < e; i++)
+    copy_debug_stmt ((gimple) VARRAY_GENERIC_PTR (id->debug_stmts, i), id);
+

"better odds of being successfully remapped"?  Well, ok ... I'll take your word
for it.  As noted previously please use a VEC, not a VARRAY.

+insert_init_debug_bind (copy_body_data *id,
+                       basic_block bb, tree var, tree value,
+                       gimple base_stmt)
+{
...
+  mark_symbols_for_renaming (note);

really?  I can't see why.  As I likely have asked this previously a
comment before
the call looks useful.

+/* Verify a gimple debug statement STMT.
+   Returns true if anything is wrong.  */
+
+static bool
+verify_gimple_debug (gimple stmt ATTRIBUTE_UNUSED)
+{
+  /* ??? FIXME.  */
+  return false;
+}

;)  Please add some very basic stuff, like the lhs should be a plain
var or parm decl and the rhs should be (err, what should it be?).
Even a comment explaining what to check would be ok.

The tree middle-end changes are ok with the above changes
if all the bootstrapping and regtesting works out ok.

You need (if you didn't already get) approval for the C++ parts,
the testsuite parts, the build system changes and the RTL
middle-end pieces.

Please announce the merge 24h in advance and ask the
trunk to be frozen.

Thanks,
Richard.

^ permalink raw reply	[flat|nested] 52+ messages in thread

* [vta->trunk] VTA merge
@ 2009-08-30  9:32 Alexandre Oliva
  2009-08-30 17:18 ` Richard Guenther
  2009-08-30 17:20 ` Jakub Jelinek
  0 siblings, 2 replies; 52+ messages in thread
From: Alexandre Oliva @ 2009-08-30  9:32 UTC (permalink / raw)
  To: gcc-patches

[-- Attachment #1: Type: text/plain, Size: 576 bytes --]

Hi,

I've been on the road and off-line for nearly the whole week, but I
managed to complete writing the ChangeLog entry for the VTA merge.  It's
the longest ChangeLog I've ever seen, weighting some 36KB, for a 600KB
patch.

It would be ideal to check it in as quickly as possible, because I'm
going to be at home and on-line for at least the next two weeks, which
should be plenty of time to fix any issues that might arise.

So, may I have confirmation of the various approvals I've already got,
before I addressed the suggestions and requests in them?

Thanks in advance,


[-- Attachment #2: vta-patchset.patch.bz2 --]
[-- Type: application/octet-stream, Size: 121298 bytes --]

[-- Attachment #3: Type: text/plain, Size: 257 bytes --]


-- 
Alexandre Oliva, freedom fighter    http://FSFLA.org/~lxoliva/
You must be the change you wish to see in the world. -- Gandhi
Be Free! -- http://FSFLA.org/   FSF Latin America board member
Free Software Evangelist      Red Hat Brazil Compiler Engineer

^ permalink raw reply	[flat|nested] 52+ messages in thread

end of thread, other threads:[~2009-09-16 15:56 UTC | newest]

Thread overview: 52+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-09-02 13:06 [vta->trunk] VTA merge Dominique Dhumieres
2009-09-03  5:41 ` Alexandre Oliva
2009-09-03  5:52   ` Alexandre Oliva
2009-09-03  9:21     ` Rainer Orth
2009-09-03 21:24       ` Alexandre Oliva
2009-09-04 11:07         ` Rainer Orth
2009-09-03 19:05     ` Ralf Wildenhues
2009-09-03 21:46       ` Alexandre Oliva
2009-09-03 21:49     ` Alexandre Oliva
  -- strict thread matches above, loose matches on Subject: below --
2009-09-03  9:27 Uros Bizjak
2009-09-03 11:11 ` Rainer Orth
2009-08-31  8:42 David Edelsohn
2009-08-31 15:31 ` Jan Hubicka
2009-08-31 16:34 ` Jakub Jelinek
2009-08-31 17:03 ` Joseph S. Myers
2009-08-30  9:32 Alexandre Oliva
2009-08-30 17:18 ` Richard Guenther
2009-08-30 20:44   ` Tom Tromey
2009-08-30 22:33     ` Jack Howarth
2009-08-31  0:25       ` Steven Bosscher
2009-08-31  6:30         ` Jack Howarth
2009-08-31  6:41           ` Joseph S. Myers
2009-09-01 21:17             ` Alexandre Oliva
2009-09-02 17:46               ` Tom Tromey
2009-09-03  7:14                 ` Alexandre Oliva
2009-09-03 15:36                   ` Tom Tromey
2009-09-04  4:58                     ` Mark Mitchell
2009-09-04  7:21                       ` Jakub Jelinek
2009-09-04 13:37                         ` Mark Mitchell
2009-09-16 15:48                   ` Jakub Jelinek
2009-09-16 15:56                     ` Richard Guenther
2009-08-31 16:39   ` Alexandre Oliva
2009-08-31 16:45     ` Richard Guenther
2009-08-31 23:22       ` Alexandre Oliva
     [not found]         ` <4A9C5373.6060201@redhat.com>
2009-09-01  4:42           ` Alexandre Oliva
2009-09-01  8:54             ` Richard Guenther
2009-09-01 15:16         ` NightStrike
2009-09-01 18:25           ` Alexandre Oliva
2009-09-01 20:09             ` Joseph S. Myers
2009-09-01 21:25               ` Alexandre Oliva
2009-09-01 21:40                 ` Joseph S. Myers
2009-09-02 16:34                   ` Jakub Jelinek
2009-09-02 17:17                     ` Jakub Jelinek
2009-09-02 17:41                       ` Jakub Jelinek
2009-09-02  2:50                 ` Alexandre Oliva
2009-09-02 14:11                   ` H.J. Lu
2009-09-03  6:50                     ` Alexandre Oliva
2009-09-03  7:07             ` Alexandre Oliva
2009-09-03 11:02               ` Richard Guenther
2009-09-02  2:43   ` Alexandre Oliva
2009-09-02 11:37     ` Paolo Bonzini
2009-08-30 17:20 ` Jakub Jelinek

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).