Re: [PATCH] Detect loops in find_comparison_args
Il 21/05/2012 19:21, Andrew Jenner ha scritto: Hi Paolo, On 5/21/2012 10:12 AM, Paolo Bonzini wrote: That's pretty heavy-weight. Perhaps you can try the usual algorithm of looking at x-next and x-next-next? That would only detect cycles of length 1 and 2 though. While that would cover all the testcases we currently know about, I wanted to eliminate the entire pattern of hangs. Otherwise it's only a question of time until someone hits a cycle of length = 3. Sorry, I meant x = x-next and y = y-next-next. That's the tortoise and hare algorithm you referred to, I think. If you extract the body of the loop into a new function find_comparison_args_1, the code would not be much more complicated. Paolo
Re: [DF] Generate REFs in REGNO order
Il 21/05/2012 19:49, Dimitrios Apostolou ha scritto:
Thanks for reviewing, in the meantime I'll try to figure out why this
patch doesn't offer any speed-up on ppc64 (doesn't break anything
though), so expect a followup by tomorrow.
Perhaps you hit this?
else if (GET_CODE (XEXP (note, 0)) == CLOBBER)
{
if (REG_P (XEXP (XEXP (note, 0), 0)))
{
unsigned int regno = REGNO (XEXP (XEXP (note, 0), 0));
if (!TEST_HARD_REG_BIT (defs_generated, regno))
df_defs_record (collection_rec, XEXP (note, 0), bb,
insn_info, flags);
}
Paolo
Re: [DF] Generate REFs in REGNO order
Il 20/05/2012 20:50, Dimitrios Apostolou ha scritto: +static void +df_find_hard_reg_defs_1 (rtx *loc, basic_block bb, + int flags, HARD_REG_SET *defs) BB and FLAGS are not needed here, I am changing this and rebootstrapping. Paolo
Re: [PATCH preprocessor, diagnostics] PR preprocessor/53229 - Fix diagnostics location when pasting tokens
Jason Merrill ja...@redhat.com writes: On 05/21/2012 10:07 AM, Dodji Seketeli wrote: Jason Merrillja...@redhat.com writes: When do we not want to set invocation_location if we're beginning to expand a macro? If M itself involves expanding another macro M', we are supposed to call the (internal function) cpp_get_token_1 that does not set set_invocation_location. So that the only expansion point recorded is the one for M, not the one for M'. But if we're already in a macro expansion (if context-c.macro is set) cpp_get_token_1 doesn't set invocation_location even if set_invocation_location is true. So we should only get the expansion point for M even if set_invocation_location is always true. The problem is that cpp_get_token_1 can be called when we are at the beginning of a macro expansion (inside enter_macro_expansion, called from cpp_get_token_1), *before* context-c.macro is set. This happens e.g, when we call funlike_invocation_p to know if the current macro is function-like or not. In that case, if we don't have the pfile-set_invocation_location flag, we'll record the expansion point of M', if M' is an argument of the function-like macro M. -- Dodji
[Patch ARM] Fix PR target/53334
Hi,
This appears to fix the problem with PR53334 and deals with the
fallout of not validating properly operands to movsicc. In addition I
took the opportunity of merging all the other places where we
validized such transforms into a single function and dealt with the
fallout accordingly. It also helps some of the tests in the testsuite
which were previously ICE'ing as a result of the previous patch to now
get fixed up.
Tested cross with trunk for arm-linux-gnueabi with armv7-a
(arm/thumb), armv5t multilibs for C,C++,
Committed.
regards.
Ramana
2012-05-22 Ramana Radhakrishnan ramana.radhakrish...@linaro.org
PR target/53334
* config/arm/arm-protos.h (arm_validize_comparison): Declare.
* config/arm/arm.c (arm_validize_comparison): Define.
* config/arm/arm.md (cbranchsi4): Cleanup expansion and use
arm_validize_comparison.
(cbranchdi4): Likewise.
(cstoredi4): Likewise.
(movsicc): Likewise.
(movsfcc): Likewise.
(movdfcc): Likewise.
Index: gcc/config/arm/arm.c
===
--- gcc/config/arm/arm.c(revision 187760)
+++ gcc/config/arm/arm.c(working copy)
@@ -26185,4 +26185,54 @@
#undef BRANCH
}
+
+/* Returns true if a valid comparison operation and makes
+ the operands in a form that is valid. */
+bool
+arm_validize_comparison (rtx *comparison, rtx * op1, rtx * op2)
+{
+ enum rtx_code code = GET_CODE (*comparison);
+ enum rtx_code canonical_code;
+ enum machine_mode mode = (GET_MODE (*op1) == VOIDmode)
+? GET_MODE (*op2) : GET_MODE (*op1);
+
+ gcc_assert (GET_MODE (*op1) != VOIDmode || GET_MODE (*op2) != VOIDmode);
+
+ if (code == UNEQ || code == LTGT)
+return false;
+
+ canonical_code = arm_canonicalize_comparison (code, op1, op2);
+ PUT_CODE (*comparison, canonical_code);
+
+ switch (mode)
+{
+case SImode:
+ if (!arm_add_operand (*op1, mode))
+ *op1 = force_reg (mode, *op1);
+ if (!arm_add_operand (*op2, mode))
+ *op2 = force_reg (mode, *op2);
+ return true;
+
+case DImode:
+ if (!cmpdi_operand (*op1, mode))
+ *op1 = force_reg (mode, *op1);
+ if (!cmpdi_operand (*op2, mode))
+ *op2 = force_reg (mode, *op2);
+ return true;
+
+case SFmode:
+case DFmode:
+ if (!arm_float_compare_operand (*op1, mode))
+ *op1 = force_reg (mode, *op1);
+ if (!arm_float_compare_operand (*op2, mode))
+ *op2 = force_reg (mode, *op2);
+ return true;
+default:
+ break;
+}
+
+ return false;
+
+}
+
#include gt-arm.h
Index: gcc/config/arm/arm-protos.h
===
--- gcc/config/arm/arm-protos.h (revision 187760)
+++ gcc/config/arm/arm-protos.h (working copy)
@@ -248,6 +248,7 @@
extern void arm_emit_coreregs_64bit_shift (enum rtx_code, rtx, rtx, rtx, rtx,
rtx);
+extern bool arm_validize_comparison (rtx *, rtx *, rtx *);
#endif /* RTX_CODE */
extern void arm_expand_vec_perm (rtx target, rtx op0, rtx op1, rtx sel);
Index: gcc/config/arm/arm.md
===
--- gcc/config/arm/arm.md (revision 187760)
+++ gcc/config/arm/arm.md (working copy)
@@ -6977,12 +6977,12 @@
(match_operand:SI 2 nonmemory_operand )])
(label_ref (match_operand 3 ))
(pc)))]
- TARGET_THUMB1 || TARGET_32BIT
+ TARGET_EITHER
if (!TARGET_THUMB1)
{
- if (!arm_add_operand (operands[2], SImode))
- operands[2] = force_reg (SImode, operands[2]);
+ if (!arm_validize_comparison (operands[0], operands[1], operands[2]))
+FAIL;
emit_jump_insn (gen_cbranch_cc (operands[0], operands[1], operands[2],
operands[3]));
DONE;
@@ -7054,33 +7054,13 @@
(pc)))]
TARGET_32BIT
{
- rtx swap = NULL_RTX;
- enum rtx_code code = GET_CODE (operands[0]);
-
/* We should not have two constants. */
gcc_assert (GET_MODE (operands[1]) == DImode
|| GET_MODE (operands[2]) == DImode);
-/* Flip unimplemented DImode comparisons to a form that
- arm_gen_compare_reg can handle. */
- switch (code)
- {
- case GT:
- swap = gen_rtx_LT (VOIDmode, operands[2], operands[1]); break;
- case LE:
- swap = gen_rtx_GE (VOIDmode, operands[2], operands[1]); break;
- case GTU:
- swap = gen_rtx_LTU (VOIDmode, operands[2], operands[1]); break;
- case LEU:
- swap = gen_rtx_GEU (VOIDmode, operands[2], operands[1]); break;
- default:
- break;
- }
- if (swap)
- emit_jump_insn (gen_cbranch_cc (swap, operands[2], operands[1],
- operands[3]));
- else
- emit_jump_insn (gen_cbranch_cc (operands[0], operands[1], operands[2],
+ if
Re: [PATCH][1/n] referenced-vars TLC
On Mon, 21 May 2012, Michael Matz wrote:
Hi,
On Mon, 21 May 2012, Richard Guenther wrote:
This removes the code that makes us walk DECL_INITIAL (recursively)
on add_referenced_var - one source of compile-time hogs in the past
and not strictly necessary.
For this to be a compile time hog, ...
if (referenced_var_check_and_insert (var))
-{
- /* Scan DECL_INITIAL for pointer variables as they may contain
-address arithmetic referencing the address of other
-variables. As we are only interested in directly referenced
-globals or referenced locals restrict this to initializers
-than can refer to local variables. */
- if (DECL_INITIAL (var)
- DECL_CONTEXT (var) == current_function_decl)
- walk_tree (DECL_INITIAL (var), find_vars_r, NULL, 0);
... DECL_INITIAL must be non-zero often. Two cases:
a) the transformation (presumably gimplification) that transformed
DECL_INITIALIZER into explicit statements left DECL_INITIALIZER
uselessly set
b) not all (local) DECL_INITIALIZERs are transformed into explicit
statements
If the former, then the above really is dead code as you rely on, though
it probably would be good to zero out DECL_INITIALIZER at transformation
time. In case b) your patch is incorrect as it would leave out marking
things as referenced which are referenced from only that initializer.
So, which is it? :)
Depends on how you define the set of referenced vars. DECL_INITIALs
remain for static variables only, automatic variables DECL_INITIAL
are either gimplified to a series of assignments or to a static const
variable with DECL_INITIAL and an aggregate init statement.
In a further patch of this series I want to arrive at the point where
we never add static variables to referenced-vars. This intermediate
patch makes it only ever add static variables to referenced-vars if
they are directly referenced (well, referenced). The
previous compile-time hog testcases were solved by the
DECL_CONTEXT () == current_function_decl check (otherwise you'd end
up with globals in all functions referenced vars list, even if not
referenced directly). But that change already would have been
wrong dependent on the definition of referenced-vars.
Richard.
Fix fixinclude's configure{,.ac}
Dear all, an --enable-maintainers-build fails here with: configure.ac:99: error: possibly undefined macro: gcc_AC_FUNC_MMAP_BLACKLIST If this token and others are legitimate, please use m4_pattern_allow. See the Autoconf documentation. make[3]: *** [fixincludes/configure] Error 1 Looking at Tristan's commit Rev. 186106 of 2012-04-03, the change gcc_ - GCC_ is missing. It also looks as if the fixinclude/ config files where not regenerated, even though there was a later patch by Tristan which touches fixinclude's config files (Rev. 186759 of 2012-04-24). OK for the trunk? (I think it is really obvious.) Tobias fixincludes/ 2012-05-22 Tobias Burnus bur...@net-b.de * configure.ac: Use GCC_AC_FUNC_MMAP_BLACKLIST instead of gcc_AC_FUNC_MMAP_BLACKLIST. * aclocal.m4: Regenerate. * configure: Regenerate. diff --git a/fixincludes/configure.ac b/fixincludes/configure.ac index f1fb2ff..f8f352f 100644 --- a/fixincludes/configure.ac +++ b/fixincludes/configure.ac @@ -96,7 +96,7 @@ AC_CHECK_DECLS(m4_split(m4_normalize(fixincludes_UNLOCKED_FUNCS))) AC_C_CONST # Checks for library functions. -gcc_AC_FUNC_MMAP_BLACKLIST +GCC_AC_FUNC_MMAP_BLACKLIST AC_MSG_CHECKING([whether to enable maintainer-specific portions of Makefiles]) AC_ARG_ENABLE(maintainer-mode, diff --git a/fixincludes/aclocal.m4 b/fixincludes/aclocal.m4 index b23541c..c5d05a3 100644 --- a/fixincludes/aclocal.m4 +++ b/fixincludes/aclocal.m4 @@ -1,7 +1,8 @@ -# generated automatically by aclocal 1.11.1 -*- Autoconf -*- +# generated automatically by aclocal 1.11.5 -*- Autoconf -*- # Copyright (C) 1996, 1997, 1998, 1999, 2000, 2001, 2002, 2003, 2004, -# 2005, 2006, 2007, 2008, 2009 Free Software Foundation, Inc. +# 2005, 2006, 2007, 2008, 2009, 2010, 2011 Free Software Foundation, +# Inc. # This file is free software; the Free Software Foundation # gives unlimited permission to copy and/or distribute it, # with or without modifications, as long as this notice is preserved. @@ -12,6 +13,6 @@ # PARTICULAR PURPOSE. m4_include([../config/acx.m4]) +m4_include([../config/mmap.m4]) m4_include([../config/override.m4]) m4_include([../config/warnings.m4]) -m4_include([../gcc/acinclude.m4]) diff --git a/fixincludes/configure b/fixincludes/configure index ea889b8..4836cd8 100755 --- a/fixincludes/configure +++ b/fixincludes/configure @@ -5222,7 +5222,7 @@ else # read() to the same fd. The only system known to have a problem here # is VMS, where text files have record structure. case $host_os in - vms* | ultrix*) + *vms* | ultrix*) gcc_cv_func_mmap_file=no ;; *) gcc_cv_func_mmap_file=yes;; @@ -5246,7 +5246,7 @@ else # Systems known to be in this category are Windows (all variants), # VMS, and Darwin. case $host_os in - vms* | cygwin* | pe | mingw* | darwin* | ultrix* | hpux10* | hpux11.00) + *vms* | cygwin* | pe | mingw* | darwin* | ultrix* | hpux10* | hpux11.00) gcc_cv_func_mmap_dev_zero=no ;; *) gcc_cv_func_mmap_dev_zero=yes;; @@ -5303,7 +5303,7 @@ else # above for use of /dev/zero. # Systems known to be in this category are Windows, VMS, and SCO Unix. case $host_os in - vms* | cygwin* | pe | mingw* | sco* | udk* ) + *vms* | cygwin* | pe | mingw* | sco* | udk* ) gcc_cv_func_mmap_anon=no ;; *) gcc_cv_func_mmap_anon=yes;;
[PATCH] Fix PR53437
This fixes PR53437, at -O0 as we are not having debug stmts there,
we need a dummy init stmt.
Bootstrapped and tested on x86_64-unknown-linux-gnu applied.
Richard.
2012-05-21 Richard Guenther rguent...@suse.de
PR middle-end/53437
* tree-inline.c (setup_one_parameter): Create a dummy init
statement for unused parameters when not optimizing.
Index: gcc/tree-inline.c
===
--- gcc/tree-inline.c (revision 187709)
+++ gcc/tree-inline.c (working copy)
@@ -2701,7 +2701,8 @@ setup_one_parameter (copy_body_data *id,
STRIP_USELESS_TYPE_CONVERSION (rhs);
/* If we are in SSA form properly remap the default definition
- or omit the initialization if the parameter is unused. */
+ or assign to a dummy SSA name if the parameter is unused and
+we are not optimizing. */
if (gimple_in_ssa_p (cfun) is_gimple_reg (p))
{
if (def)
@@ -2711,6 +2712,11 @@ setup_one_parameter (copy_body_data *id,
SSA_NAME_IS_DEFAULT_DEF (def) = 0;
set_default_def (var, NULL);
}
+ else if (!optimize)
+ {
+ def = make_ssa_name (var, NULL);
+ init_stmt = gimple_build_assign (def, rhs);
+ }
}
else
init_stmt = gimple_build_assign (var, rhs);
Re: Fix fixinclude's configure{,.ac}
On May 22, 2012, at 11:20 AM, Tobias Burnus wrote: Dear all, an --enable-maintainers-build fails here with: configure.ac:99: error: possibly undefined macro: gcc_AC_FUNC_MMAP_BLACKLIST If this token and others are legitimate, please use m4_pattern_allow. See the Autoconf documentation. make[3]: *** [fixincludes/configure] Error 1 Looking at Tristan's commit Rev. 186106 of 2012-04-03, the change gcc_ - GCC_ is missing. It also looks as if the fixinclude/ config files where not regenerated, even though there was a later patch by Tristan which touches fixinclude's config files (Rev. 186759 of 2012-04-24). Indeed, I failed to notice that fixinclude was using ../gcc/config.m4. I think it would also make sense to remove '-I ../gcc' from ACLOCAL_AMFLAGS of fixincludes/Makefile.in Thank you for having noticed and fixed this bug, Tristan. [ Bruce CC as he is the maintained of fixinclude] OK for the trunk? (I think it is really obvious.) Tobias fixinclude.diff
Re: [Patch, Fortran] PR53389 realloc-on-assignment: Memory leak and wrong double evaluation (4.6-4.8 regression)
Dear Tobias, OK for trunk. As you say, the testcase does no harm and so should be retained. Thanks for the patch. Cheers Paul On 21 May 2012 19:51, Tobias Burnus bur...@net-b.de wrote: First: I have another rather simple patch, which still needs to be reviewed: http://gcc.gnu.org/ml/fortran/2012-05/msg00086.html * * * When a realloc on assignment is done, gfortran first obtains the descriptor by calling realloc_lhs_loop_for_fcn_call and then does the actual function call using gfc_conv_function_expr. However, realloc_lhs_loop_for_fcn_call already causes that the RHS is evaluated (via gfc_add_loop_ss_code). The double evaluation of an inner function call, causes memory leaks - and double evaluation (correctness/performance issue). The following code solves the issue. I am not 100% sure that it correctly does so, but it solved the problem and the testsuite passed. (It might obsolete the check in se-direct_byref of gfc_conv_procedure_call.) The test case is a bit pointless as it will only fail if one uses valgrind on it; thus, one could consider committing the patch even without. However, it might not harm to have it in the test suite. Build and regtested on x86-64-linux. OK for the trunk and 4.6/4.7? Tobias -- The knack of flying is learning how to throw yourself at the ground and miss. --Hitchhikers Guide to the Galaxy
Re: [PATCH] relax constraint on integral-offset conversions (PR53336)
On 05/16/2012 11:43 AM, Richard Guenther wrote: On Wed, May 16, 2012 at 9:19 AM, Paolo Bonzinibonz...@gnu.org wrote: OFFSET_TYPE is treated as an integral type for the purpose of conversion in fold-const.c. However, the GIMPLE verifier disagrees, leading to verification errors when a cast from boolean to offset type is gimplified. Bootstrapped/regtested x86_64-pc-linux-gnu, ok for mainline? I see this patch is now installed and I'm afraid is causing regressions in the c++ testsuite: today I see many unexpected fails having to do with pointers to members. Thanks, Paolo.
Re: [PATCH] relax constraint on integral-offset conversions (PR53336)
On Tue, May 22, 2012 at 12:26 PM, Paolo Carlini paolo.carl...@oracle.com wrote: On 05/16/2012 11:43 AM, Richard Guenther wrote: On Wed, May 16, 2012 at 9:19 AM, Paolo Bonzinibonz...@gnu.org wrote: OFFSET_TYPE is treated as an integral type for the purpose of conversion in fold-const.c. However, the GIMPLE verifier disagrees, leading to verification errors when a cast from boolean to offset type is gimplified. Bootstrapped/regtested x86_64-pc-linux-gnu, ok for mainline? I see this patch is now installed and I'm afraid is causing regressions in the c++ testsuite: today I see many unexpected fails having to do with pointers to members. Can't be this patch though, it only lets more things pass the verifier. Unless - /* Allow conversion from integer to offset type and vice versa. */ + /* Allow conversion from integral to offset type and vice versa. */ if ((TREE_CODE (lhs_type) == OFFSET_TYPE - TREE_CODE (rhs1_type) == INTEGER_TYPE) + INTEGRAL_TYPE_P (rhs1_type)) || (TREE_CODE (lhs_type) == INTEGER_TYPE -TREE_CODE (rhs1_type) == OFFSET_TYPE)) +INTEGRAL_TYPE_P (rhs1_type))) the latter looks like a typo - INTEGRAL_TYPE_P should be used for lhs_type, not rhs1_type. Richard. Thanks, Paolo.
Re: [PATCH] relax constraint on integral-offset conversions (PR53336)
On 05/22/2012 12:38 PM, Richard Guenther wrote: Can't be this patch though, it only lets more things pass the verifier. Unless - /* Allow conversion from integer to offset type and vice versa. */ + /* Allow conversion from integral to offset type and vice versa. */ if ((TREE_CODE (lhs_type) == OFFSET_TYPE - TREE_CODE (rhs1_type) == INTEGER_TYPE) + INTEGRAL_TYPE_P (rhs1_type)) || (TREE_CODE (lhs_type) == INTEGER_TYPE - TREE_CODE (rhs1_type) == OFFSET_TYPE)) + INTEGRAL_TYPE_P (rhs1_type))) the latter looks like a typo - INTEGRAL_TYPE_P should be used for lhs_type, not rhs1_type. Ok, thanks. If nobody beats me later today I will regtest and in case commit what you are suggesting. Paolo.
Re: [PATCH] Improved re-association of signed arithmetic (was: Inefficient end-of-loop value computation)
On Fri, May 18, 2012 at 10:27 PM, Ulrich Weigand uweig...@de.ibm.com wrote:
Richard Guenther wrote:
On Thu, Mar 8, 2012 at 3:29 PM, Ulrich Weigand uweig...@de.ibm.com wrote:
- Should I try to improve forwprop to handle casts and additional re-
association cases until it handles the above expression?
Yes, ideally by trying to sub-divide this task into separate profitable
transforms. Maybe Andrew can chime in here?
I finally got some time to look into this in detail. The various special-
case transforms in associate_plusminus all transform a plus/minus expression
tree into either a single operand, a negated operand, or a single plus or
minus of two operands. This is valid as long as we can prove that the
newly introduced expression can never overflow (if we're doing signed
arithmetic). This is true in those special cases due to either:
- If all sub-expressions of the contracted plus/minus tree are themselves
performed in signed arithmetic and thus are guaranteed to never overflow,
their result equals the true arithmetic result if we were to perform
the computation in the actual integers with unlimited precision.
Now, true arithmetic is associative. Thus, if we re-associate the
expression tree such that everything cancels out and just a single
operation A +/- B (or -A) remains, the true result of that operation
must equal the true result of the original expression -- which means
in particular that it lies within the range of the underlying data
type, and hence that final operation itself cannot overflow.
- In the case of introducing a negation, we only need to be sure that
its operand can never be the minimum value of its type range. This
can on occasion be proven via a form of value range tracking.
For example, when performing the transformation ~A + 1 -- -A, we note
that ~A cannot be INT_MAX, since we can add 1 to it without overflow;
and therefore A cannot be INT_MIN.
Now, using those two rules, we can actually prove many more cases where
reassociation is possible. For example, we can transform (A + (B + C)) - C
to A + B (due to the first rule). We can also transform the original
expression resulting from end-of-loop value computation
(A + 1) + (int) ((unsigned) ~A + (unsigned) B)
into just B -- this can never overflow since we aren't even introducing
any new expression.
The following patch rewrites associate_plusminus to remove all the
explicitly coded special cases, and instead performs a scan of the
plus/minus tree similar to what is done in tree-ssa-reassoc (and also
in simplify-rtx for that matter). If this results in an expression
tree that collapses to just a single operand, or just a single newly
introduced operation, and -in the latter case- one of the two rules
above ensure the new operation is safe, the transformation is performed.
This still passes all reassoc tests, and in fact allows to remove XFAILs
from two of them. It also catches the end-of-loop value computation case.
Tested on i386-linux with no regressions.
OK for mainline?
The point of the special-cases in forwprop was to make them fast to
detect - forwprop should be a pattern-matching thing, much like
combine on RTL.
So, instead of changing forwprop this way can you adjust tree-ssa-reassoc.c
to (again) associate !TYPE_OVERFLOW_WRAPS operations but make
sure we throw away results that would possibly introduce undefined overflow
for !TYPE_OVERFLOW_WRAPS types? There is a reassoc pass after
loop optimizations, so that should fix it as well, no?
Thanks,
Richard.
Bye,
Ulrich
ChangeLog:
gcc/
* tree-ssa-forwprop.c (enum plus_minus_op_range,
struct plus_minus_op_data, struct plus_minus_data): New types.
(next_plus_minus_op, remove_plus_minus_op, add_plus_minus_op,
add_plus_minus_ops, setup_plus_minus_ops): New functions.
(associate_plusminus): Reimplement.
gcc/testsuite/
* gcc.dg/tree-ssa/reassoc-2.c: Remove XFAIL.
* gcc.dg/tree-ssa/reassoc-9.c: Update test, remove XFAIL.
* gcc.dg/tree-ssa/reassoc-23.c: Update test.
* gcc.dg/tree-ssa/reassoc-26.c: New test.
Index: gcc/testsuite/gcc.dg/tree-ssa/reassoc-23.c
===
*** gcc/testsuite/gcc.dg/tree-ssa/reassoc-23.c (revision 187653)
--- gcc/testsuite/gcc.dg/tree-ssa/reassoc-23.c (working copy)
***
*** 1,5
/* { dg-do compile } */
! /* { dg-options -O2 -fdump-tree-reassoc1 } */
unsigned int
foo(unsigned int a, unsigned int b, unsigned int c, unsigned int d,
--- 1,5
/* { dg-do compile } */
! /* { dg-options -O2 -fdump-tree-optimized } */
unsigned int
foo(unsigned int a, unsigned int b, unsigned int c, unsigned int d,
*** foo(unsigned int a, unsigned int b, unsi
*** 13,17
return e;
}
! /* { dg-final { scan-tree-dump-times = 20 1 reassoc1} } */
! /* { dg-final {
Re: [PATCH] relax constraint on integral-offset conversions (PR53336)
On Tue, May 22, 2012 at 1:09 PM, Paolo Carlini paolo.carl...@oracle.com wrote: On 05/22/2012 12:38 PM, Richard Guenther wrote: Can't be this patch though, it only lets more things pass the verifier. Unless - /* Allow conversion from integer to offset type and vice versa. */ + /* Allow conversion from integral to offset type and vice versa. */ if ((TREE_CODE (lhs_type) == OFFSET_TYPE - TREE_CODE (rhs1_type) == INTEGER_TYPE) + INTEGRAL_TYPE_P (rhs1_type)) || (TREE_CODE (lhs_type) == INTEGER_TYPE - TREE_CODE (rhs1_type) == OFFSET_TYPE)) + INTEGRAL_TYPE_P (rhs1_type))) the latter looks like a typo - INTEGRAL_TYPE_P should be used for lhs_type, not rhs1_type. Ok, thanks. If nobody beats me later today I will regtest and in case commit what you are suggesting. I think it's quite obvious even ... thus I'll commit it without testing. Heh. Richard. Paolo.
[PATCH][3/n] referenced-vars TLC
This is the next patch in the series to move us to the state not
having global variables in referenced-vars but restricting the
reverse lookup UID - DECL to automatic variables (the main
consumer is the SSA rewriter - which obviously only needs autos).
It introduces a DECL flag to mark virtual operands so we can
let those pass through (for the SSA renamer again). The benefit
is also a cheaper is_gimple_reg which is used to decide whether
you are faced with a virtual SSA name or a real one.
On the way this changes the virtual var creation to work on
a specific function, thereby avoiding a cfun switch in omp lowering.
It also removes some unused cruft.
Bootstrapped on x86_64-unknown-linux-gnu, testing in progress.
Richard.
2012-05-22 Richard Guenther rguent...@suse.de
* tree.h (VAR_DECL_IS_VIRTUAL_OPERAND): New.
(init_function_for_compilation): Remove.
* tree-dfa.c (find_vars_r): Take struct function argument.
(find_referenced_vars_in): Adjust.
* tree-ssa-operands.c (clobber_stats): Remove.
(create_vop_var): Take struct function argument. Mark
virtual operand with VAR_DECL_IS_VIRTUAL_OPERAND.
(init_ssa_operands): Take struct function argument.
(fini_ssa_operands): Do not dump dead stats.
* tree-ssa-operands.h (init_ssa_operands): Take struct function
argument.
* cgraphunit.c (init_lowered_empty_function): Adjust.
* lto-streamer-in.c (input_cfg): Likewise.
* tree-inline.c (initialize_cfun): Likewise.
* tree-into-ssa.c (rewrite_into_ssa): Likewise.
* omp-low.c (expand_omp_taskreg): Likewise. Avoid switching
cfun.
* gimple.c (is_gimple_reg): Optimize the SSA_NAME case,
virtual operands are not registers.
Index: gcc/tree.h
===
*** gcc/tree.h (revision 187767)
--- gcc/tree.h (working copy)
*** struct GTY(()) tree_common {
*** 689,694
--- 689,697
TYPE_SATURATING in
all types
+VAR_DECL_IS_VIRTUAL_OPERAND in
+ VAR_DECL
+
nowarning_flag:
TREE_NO_WARNING in
*** extern void decl_fini_priority_insert (t
*** ,3338
--- 3336,3344
libraries. */
#define MAX_RESERVED_INIT_PRIORITY 100
+ #define VAR_DECL_IS_VIRTUAL_OPERAND(NODE) \
+ (VAR_DECL_CHECK (NODE)-base.saturating_flag)
+
#define DECL_VAR_ANN_PTR(NODE) \
(TREE_CODE (NODE) == VAR_DECL ? (NODE)-var_decl.ann \
: TREE_CODE (NODE) == PARM_DECL ? (NODE)-parm_decl.ann \
*** extern void stack_protect_prologue (void
*** 5537,5543
extern void stack_protect_epilogue (void);
extern void init_dummy_function_start (void);
extern void expand_dummy_function_end (void);
- extern unsigned int init_function_for_compilation (void);
extern void allocate_struct_function (tree, bool);
extern void push_struct_function (tree fndecl);
extern void init_function_start (tree);
--- 5543,5548
Index: gcc/tree-ssa-operands.c
===
*** gcc/tree-ssa-operands.c (revision 187767)
--- gcc/tree-ssa-operands.c (working copy)
*** along with GCC; see the file COPYING3.
*** 76,109
operand vector for VUSE, then the new vector will also be modified
such that it contains 'a_5' rather than 'a'. */
- /* Structure storing statistics on how many call clobbers we have, and
-how many where avoided. */
-
- static struct
- {
- /* Number of call-clobbered ops we attempt to add to calls in
- add_call_clobbered_mem_symbols. */
- unsigned int clobbered_vars;
-
- /* Number of write-clobbers (VDEFs) avoided by using
- not_written information. */
- unsigned int static_write_clobbers_avoided;
-
- /* Number of reads (VUSEs) avoided by using not_read information. */
- unsigned int static_read_clobbers_avoided;
-
- /* Number of write-clobbers avoided because the variable can't escape to
- this call. */
- unsigned int unescapable_clobbers_avoided;
-
- /* Number of read-only uses we attempt to add to calls in
- add_call_read_mem_symbols. */
- unsigned int readonly_clobbers;
-
- /* Number of read-only uses we avoid using not_read information. */
- unsigned int static_readonly_clobbers_avoided;
- } clobber_stats;
-
/* Flags to describe operand properties in helpers. */
--- 76,81
*** ssa_operands_active (void)
*** 186,196
representative of all of the virtual operands FUD chain. */
static void
! create_vop_var (void)
{
tree global_var;
! gcc_assert (cfun-gimple_df-vop == NULL_TREE);
global_var = build_decl (BUILTINS_LOCATION, VAR_DECL,
get_identifier (.MEM),
--- 158,168
representative of all of the virtual operands FUD chain. */
static void
! create_vop_var (struct function *fn)
{
Re: [PATCH] Improved re-association of signed arithmetic (was: Inefficient end-of-loop value computation)
Richard Guenther wrote: On Fri, May 18, 2012 at 10:27 PM, Ulrich Weigand uweig...@de.ibm.com wrote: The following patch rewrites associate_plusminus to remove all the explicitly coded special cases, and instead performs a scan of the plus/minus tree similar to what is done in tree-ssa-reassoc (and also in simplify-rtx for that matter). If this results in an expression tree that collapses to just a single operand, or just a single newly introduced operation, and -in the latter case- one of the two rules above ensure the new operation is safe, the transformation is performed. This still passes all reassoc tests, and in fact allows to remove XFAILs from two of them. It also catches the end-of-loop value computation case. Tested on i386-linux with no regressions. OK for mainline? The point of the special-cases in forwprop was to make them fast to detect - forwprop should be a pattern-matching thing, much like combine on RTL. Well, the problem is that you can really make the decision whether or not reassociation is allowed after you've seen the whole plus-minus tree. For example, it is valid to transform (a + (b + c)) - c into a + b -- but the only potential intermediate transform, a + (b + c) into (a + b) + c, is of course not valid in general. It only becomes valid due to the outer context ... - c in which it is executed ... So, instead of changing forwprop this way can you adjust tree-ssa-reassoc.c to (again) associate !TYPE_OVERFLOW_WRAPS operations but make sure we throw away results that would possibly introduce undefined overflow for !TYPE_OVERFLOW_WRAPS types? There is a reassoc pass after loop optimizations, so that should fix it as well, no? I had thought of that as well. But it is not quite that simple -- the problem is that tree-ssa-reassoc.c as part of its core algorithm reassociates expressions all the time while even still building up the tree, see e.g. linearize_expr or break_up_subtract. Those steps may all be invalid in general, but we only know whether that is true at the very end, once we've built up the full tree -- but at that point it is already too late. I guess it might be possible to re-work tree-ssa-reassoc to *first* build up the tree without changing any statements, then make the decision whether we can re-associate, and only then actually perform modifications. I'll have to think about that a bit more. If we manage to do that, would you then suggest we should remove the associate_plusminus phase in tree-ssa-forwprop.c again? Bye, Ulrich -- Dr. Ulrich Weigand GNU Toolchain for Linux on System z and Cell BE ulrich.weig...@de.ibm.com
Re: [PATCH] Improved re-association of signed arithmetic (was: Inefficient end-of-loop value computation)
On Tue, May 22, 2012 at 1:58 PM, Ulrich Weigand uweig...@de.ibm.com wrote: Richard Guenther wrote: On Fri, May 18, 2012 at 10:27 PM, Ulrich Weigand uweig...@de.ibm.com wrote: The following patch rewrites associate_plusminus to remove all the explicitly coded special cases, and instead performs a scan of the plus/minus tree similar to what is done in tree-ssa-reassoc (and also in simplify-rtx for that matter). If this results in an expression tree that collapses to just a single operand, or just a single newly introduced operation, and -in the latter case- one of the two rules above ensure the new operation is safe, the transformation is performed. This still passes all reassoc tests, and in fact allows to remove XFAILs from two of them. It also catches the end-of-loop value computation case. Tested on i386-linux with no regressions. OK for mainline? The point of the special-cases in forwprop was to make them fast to detect - forwprop should be a pattern-matching thing, much like combine on RTL. Well, the problem is that you can really make the decision whether or not reassociation is allowed after you've seen the whole plus-minus tree. For example, it is valid to transform (a + (b + c)) - c into a + b -- but the only potential intermediate transform, a + (b + c) into (a + b) + c, is of course not valid in general. It only becomes valid due to the outer context ... - c in which it is executed ... So, instead of changing forwprop this way can you adjust tree-ssa-reassoc.c to (again) associate !TYPE_OVERFLOW_WRAPS operations but make sure we throw away results that would possibly introduce undefined overflow for !TYPE_OVERFLOW_WRAPS types? There is a reassoc pass after loop optimizations, so that should fix it as well, no? I had thought of that as well. But it is not quite that simple -- the problem is that tree-ssa-reassoc.c as part of its core algorithm reassociates expressions all the time while even still building up the tree, see e.g. linearize_expr or break_up_subtract. Those steps may all be invalid in general, but we only know whether that is true at the very end, once we've built up the full tree -- but at that point it is already too late. Hmm, really? I had not realized it does something it cannot undo later ... but well ISTR patches floating around for re-organizing how we do the break_up_subtract / negate stuff. Micha? I guess it might be possible to re-work tree-ssa-reassoc to *first* build up the tree without changing any statements, then make the decision whether we can re-associate, and only then actually perform modifications. I'll have to think about that a bit more. Yes, I think that's what we want. If we manage to do that, would you then suggest we should remove the associate_plusminus phase in tree-ssa-forwprop.c again? Not sure about removing it - simplifying the simple cases early enough might be useful. But yes, I installed them all to avoid regressing too much as I fixed reassoc not to associate expressions with undefined overflow. Richard. Bye, Ulrich -- Dr. Ulrich Weigand GNU Toolchain for Linux on System z and Cell BE ulrich.weig...@de.ibm.com
Re: [PATCH] Improved re-association of signed arithmetic (was: Inefficient end-of-loop value computation)
On Tue, May 22, 2012 at 2:04 PM, Richard Guenther richard.guent...@gmail.com wrote: On Tue, May 22, 2012 at 1:58 PM, Ulrich Weigand uweig...@de.ibm.com wrote: Richard Guenther wrote: On Fri, May 18, 2012 at 10:27 PM, Ulrich Weigand uweig...@de.ibm.com wrote: The following patch rewrites associate_plusminus to remove all the explicitly coded special cases, and instead performs a scan of the plus/minus tree similar to what is done in tree-ssa-reassoc (and also in simplify-rtx for that matter). If this results in an expression tree that collapses to just a single operand, or just a single newly introduced operation, and -in the latter case- one of the two rules above ensure the new operation is safe, the transformation is performed. This still passes all reassoc tests, and in fact allows to remove XFAILs from two of them. It also catches the end-of-loop value computation case. Tested on i386-linux with no regressions. OK for mainline? The point of the special-cases in forwprop was to make them fast to detect - forwprop should be a pattern-matching thing, much like combine on RTL. Well, the problem is that you can really make the decision whether or not reassociation is allowed after you've seen the whole plus-minus tree. For example, it is valid to transform (a + (b + c)) - c into a + b -- but the only potential intermediate transform, a + (b + c) into (a + b) + c, is of course not valid in general. It only becomes valid due to the outer context ... - c in which it is executed ... So, instead of changing forwprop this way can you adjust tree-ssa-reassoc.c to (again) associate !TYPE_OVERFLOW_WRAPS operations but make sure we throw away results that would possibly introduce undefined overflow for !TYPE_OVERFLOW_WRAPS types? There is a reassoc pass after loop optimizations, so that should fix it as well, no? I had thought of that as well. But it is not quite that simple -- the problem is that tree-ssa-reassoc.c as part of its core algorithm reassociates expressions all the time while even still building up the tree, see e.g. linearize_expr or break_up_subtract. Those steps may all be invalid in general, but we only know whether that is true at the very end, once we've built up the full tree -- but at that point it is already too late. Hmm, really? I had not realized it does something it cannot undo later ... but well ISTR patches floating around for re-organizing how we do the break_up_subtract / negate stuff. Micha? I guess it might be possible to re-work tree-ssa-reassoc to *first* build up the tree without changing any statements, then make the decision whether we can re-associate, and only then actually perform modifications. I'll have to think about that a bit more. Yes, I think that's what we want. If we manage to do that, would you then suggest we should remove the associate_plusminus phase in tree-ssa-forwprop.c again? Not sure about removing it - simplifying the simple cases early enough might be useful. But yes, I installed them all to avoid regressing too much as I fixed reassoc not to associate expressions with undefined overflow. Btw, reassoc (and your patch?) would have the issue that for a + (b + c) - a it would yield b + c as result which is not an atom (but still ok, since it is a pre-existing value that is computed in the IL already). The simple forwprop matching catches this as it doesn't even look into the (b + c) expression but just sees a temporary as atom here. Richard.
Re: [C++ Patch] PR 44516
Hi, On 05/17/2012 10:29 PM, Iain Sandoe wrote: On 17 May 2012, at 21:16, Mike Stump wrote: On May 17, 2012, at 12:53 PM, Paolo Carlini wrote: On 05/17/2012 09:47 PM, Jason Merrill wrote: On 05/17/2012 05:06 AM, Paolo Carlini wrote: On 05/17/2012 10:33 AM, Gabriel Dos Reis wrote: I am still puzzled by why we need to assert, as opposed to just ignore, unless we have a plan to make a wholesale move -- but even there I am bit nervous. Eh, don't ask me ;) Anyway, in terms of testing that we aren't screwing up anything in the C++ front-end, the testsuite just passed with the below p3 attached. That's good. Yep, that's what the assert is for: testing that we aren't screwing up anything in the C++ front end. If it fires, it lets us know there's something still to fix. Sounds like it looks good so far. If you like, I can install p3 now, but I think it would be a pity if we can't have the warning_at bit because of that lone use in the ocbj front-end of an explicit 'warning_at (0' (in objc-gnu-runtime-abi-01.c). Maybe Mike has something to suggest? Gosh, I'm not wedded to even having that warning. :-) The compiler knows what it has to do for codegen, it can eat and ignore the flag silently for all I care. I'd ask Nicola or Iain if they have any thoughts. One could also reasonably use: inform (UNKNOWN_LOCATION, ); for it, if that helps. For my part, I'd prefer inform (UNKNOWN_LOCATION as is used below .. and, hopefully everywhere else, where new stuff has been introduced. Ok, thanks. Today I wanted to concretely test and post a patch but I noticed this other objc bit in objc-next-runtime-abi-01.c which I don't know how to handle due to the OPT_Wall gate: warning_at (UNKNOWN_LOCATION, OPT_Wall, %-fobjc-sjlj-exceptions% is the only supported exceptions system for %-fnext-runtime% with %-fobjc-abi-version% 2); If the objc maintainers could help with those uses of warning_at (0 in the objc tree, I have a patch ready for the asserts proper. Thanks, Paolo.
Re: [PATCH] Improved re-association of signed arithmetic (was: Inefficient end-of-loop value computation)
Hi, On Tue, 22 May 2012, Richard Guenther wrote: I had thought of that as well. But it is not quite that simple -- the problem is that tree-ssa-reassoc.c as part of its core algorithm reassociates expressions all the time while even still building up the tree, see e.g. linearize_expr or break_up_subtract. Those steps may all be invalid in general, but we only know whether that is true at the very end, once we've built up the full tree -- but at that point it is already too late. Hmm, really? Yep, reassoc rewrites already at linearize time. I think that could be changed without too much hassle. I had not realized it does something it cannot undo later ... but well ISTR patches floating around for re-organizing how we do the break_up_subtract / negate stuff. Micha? Yes, I have an incomplete patch for that, but it didn't touch the current fundamental implementation detail of tree-reassoc, namely touching statements at analyze time already. Ciao, Michael.
Re: [PATCH] Improved re-association of signed arithmetic (was: Inefficient end-of-loop value computation)
Richard Guenther wrote: Btw, reassoc (and your patch?) would have the issue that for a + (b + c) - a it would yield b + c as result which is not an atom (but still ok, since it is a pre-existing value that is computed in the IL already). The simple forwprop matching catches this as it doesn't even look into the (b + c) expression but just sees a temporary as atom here. Yes, that is a problem, and my patch partially solves it by being very eager in performing optimizations and checking whether we have found a simplified solution. For example, in your case we'd see y = b + c x = a + y r = x - a and then, when looking at r: - push x and -a onto the operand stack - notice no simplification yet - expand x one stage into a and y - immediately cancel a and -a when pushing onto the stack - notice the stack is now just y, which represents a simplification - stop This is not a *complete* solution, since by expanding x we might miss solutions we could have seen only when expanding -a instead; we'd really have to search the full solution space or back-track an already performed expansion. I have not implemented this due to concerns about combinatorial explosion ... Solving this problem in the context of the existing tree-ssa-reassoc algorithm is indeed yet another issue that I need to think about. Bye, Ulrich -- Dr. Ulrich Weigand GNU Toolchain for Linux on System z and Cell BE ulrich.weig...@de.ibm.com
Re: [PATCH] Improved re-association of signed arithmetic (was: Inefficient end-of-loop value computation)
On Tue, May 22, 2012 at 2:50 PM, Ulrich Weigand uweig...@de.ibm.com wrote: Richard Guenther wrote: Btw, reassoc (and your patch?) would have the issue that for a + (b + c) - a it would yield b + c as result which is not an atom (but still ok, since it is a pre-existing value that is computed in the IL already). The simple forwprop matching catches this as it doesn't even look into the (b + c) expression but just sees a temporary as atom here. Yes, that is a problem, and my patch partially solves it by being very eager in performing optimizations and checking whether we have found a simplified solution. For example, in your case we'd see y = b + c x = a + y r = x - a and then, when looking at r: - push x and -a onto the operand stack - notice no simplification yet - expand x one stage into a and y - immediately cancel a and -a when pushing onto the stack - notice the stack is now just y, which represents a simplification - stop This is not a *complete* solution, since by expanding x we might miss solutions we could have seen only when expanding -a instead; we'd really have to search the full solution space or back-track an already performed expansion. I have not implemented this due to concerns about combinatorial explosion ... Solving this problem in the context of the existing tree-ssa-reassoc algorithm is indeed yet another issue that I need to think about. I suppose we'd have to go the full way making the reassoc intermediate IL a tree and not just a linear list of operands ... which incidentially also would pave the way for supporting multiple operations. Each operation node could then link to a stmt (or have an SSA name) that represents its value as long as we have it. Associating would simply remove that link, forcing a new stmt computation. Re-structuring reassoc that way would also make its IL and optimizations possibly usable by other passes that construct expressions, allowing optimizing them without relying on fold and later gimplification ... Richard. Bye, Ulrich -- Dr. Ulrich Weigand GNU Toolchain for Linux on System z and Cell BE ulrich.weig...@de.ibm.com
[Patch, libgfortran] PR53445/53444 - Fix compilation on VxWorks
The attached patches fix compilation issues on VxWorks.
a) VxWorks has strerror_r but contrary to POSIX, the function in VxWorks
(at least in older versions) takes only two arguments: errnum and buf
and not also the buffer length. I added a configure check for that variant.
b) There is no sticky bit on VxWorks - which is now ignored via an #ifdef.
The patch of (b) is the bug reporter's and, thus, tested on VxWorks.
(a) and (b) have been successfully bootstrapped on x86-64-linux.
OK for the trunk?
(I have regenerated the files with the latest 1.11 automake (1.11.5).
Before the committal, I will re-generate them with 1.11.1.)
Tobias
2012-05-22 Tobias Burnus bur...@net-b.de
PR libfortran/53445
* intrinsics/chmod.c (chmod_func): Ignore S_ISVTX on VxWorks.
diff --git a/libgfortran/intrinsics/chmod.c b/libgfortran/intrinsics/chmod.c
index 9156303..e8a81d5 100644
--- a/libgfortran/intrinsics/chmod.c
+++ b/libgfortran/intrinsics/chmod.c
@@ -459,17 +459,19 @@ clause_done:
if ((ugo[2] || honor_umask) !rwxXstugo[8])
file_mode = (file_mode ~(S_IROTH | S_IWOTH | S_IXOTH))
| (new_mode (S_IROTH | S_IWOTH | S_IXOTH));
+#ifndef __VXWORKS__
if (is_dir rwxXstugo[5])
file_mode |= S_ISVTX;
else if (!is_dir)
file_mode = ~S_ISVTX;
#endif
+#endif
}
else if (set_mode == 2)
{
/* Clear '-'. */
file_mode = ~new_mode;
-#ifndef __MINGW32__
+#if !defined( __MINGW32__) !defined (__VXWORKS__)
if (rwxXstugo[5] || !is_dir)
file_mode = ~S_ISVTX;
#endif
@@ -477,7 +479,7 @@ clause_done:
else if (set_mode == 3)
{
file_mode |= new_mode;
-#ifndef __MINGW32__
+#if !defined (__MINGW32__) !defined (__VXWORKS__)
if (rwxXstugo[5] is_dir)
file_mode |= S_ISVTX;
else if (!is_dir)
Re: fix cross build
On 05/21/12 11:03, Richard Guenther wrote:
Hmm - I think this papers over the issue that the CONSTRUCTOR is not
properly gimplified - it still contains a TARGET_EXPR which is not valid GIMPLE.
Why is that TARGET_EXPR not gimplified?
As far as I can make out, it just doesn't look inside the constructor.
0 gimplify_expr (expr_p=0xb7e90394, pre_p=0xbfffe514, post_p=0xbfffd08c,
gimple_test_f=0x84b0a80 is_gimple_min_lval(tree_node*), fallback=3) at
../../src/gcc/gimplify.c:7356
#1 0x084f5882 in gimplify_compound_lval (expr_p=0xb7e9cb94, pre_p=0xbfffe514,
post_p=0xbfffd08c, fallback=1)
at ../../src/gcc/gimplify.c:2259
#2 0x08519878 in gimplify_expr (expr_p=0xb7e9cb94, pre_p=0xbfffe514,
post_p=0xbfffd08c,
gimple_test_f=0x84eaf9f is_gimple_reg_rhs_or_call(tree), fallback=1) at
../../src/gcc/gimplify.c:7081
#3 0x085087f7 in gimplify_modify_expr (expr_p=0xbfffd6d0, pre_p=0xbfffe514,
post_p=0xbfffd08c, want_value=false)
at ../../src/gcc/gimplify.c:4843
The switch case at that stack frame is for CONSTRUCTORs. It has the comment:
/* Don't reduce this in place; let gimplify_init_constructor work its
magic. Buf if we're just elaborating this for side effects, just
gimplify any element that has side-effects. */
But we never enter G_I_C before the ICE.
On the second time we get here, fallback has the value 1 at that point
(fb_rvalue), so neither if condition is satisified, and we end up at
ret = GS_ALL_DONE;
we then proceed down to enter:
else if ((fallback fb_rvalue) is_gimple_reg_rhs_or_call (*expr_p))
{
/* An rvalue will do. Assign the gimplified expression into a
new temporary TMP and replace the original expression with
TMP. First, make sure that the expression has a type so that
it can be assigned into a temporary. */
gcc_assert (!VOID_TYPE_P (TREE_TYPE (*expr_p)));
*expr_p = get_formal_tmp_var (*expr_p, pre_p);
}
and ICE inside GFTV when it attempts to generate the hash.
AFAICT, changing the test case to:
char *str = ((union {const char * _q; char * _nq;})
((const char *)((num_string)
-string.str)))._nq;
(i.e. removing the stmt-expr) results in the same logical flow as above, but
there's no TARGET_EXPR inside the constructor.
I'm not sure how it's supposed to work, so I'm a little lost.
Re: [driver, LTO Patch]: Resurrect user specs support
On Mon, 21 May 2012, Christian Bruel wrote: 1) Lazily check the flag validation until all command line spec files are read. For this purpose, 'read_specs' records specs, to be analyzed with 'file_spec_p'. Such flags have 'live_cond' = SWITCH_USER I like the idea of allowing flags mentioned in user specs but not other specs - but not the implementation using this new live_cond. There are a lot of places in gcc.c that set validated, and I can't convince myself that this implementation will ensure they all take correct account of where the relevant spec (if any) came from without causing any other change to how the driver behaves. For example, I don't see any change to the setting of validated for % and % in do_spec_1 to account for where the spec came from. Instead, I think that any function that sets validated based on a spec should be passed the information about whether it's a user spec or not. So validate_all_switches would need to pass that down to validate_switches_from_spec, for example - and do_spec_1 would also need to get that information. -- Joseph S. Myers jos...@codesourcery.com
Re: divide 64-bit by constant for 32-bit target machines
Hi,
Here is the new version of the patch
I have fixed two errors in the previous version,
on mips32 the compiler could not expand division and terminated
with ICE, this change fixed the issue:
/* Extrating the higher part of the sum */
tmp = gen_highpart (SImode, tmp);
tmp = force_reg (SImode, tmp);
+ tmp = force_reg (SImode, tmp);
+ tmp = convert_to_mode (DImode, tmp, 1);
and another error on i686 and mips32r2: I found that overflow
check in multiplication was not working. For example
0xfe34b4190a392b23/257 produced wrong result. Following change
resolved the issue:
-emit_store_flag_force (c, GT, u0, tmp, DImode, 1, -1);
+emit_store_flag_force (c, GT, u0, tmp, DImode, 1, 1);
Tested this new version of the patch on -none-linux-gnu with arm-7l,
mips-32r2 (74k), i686 without new regressions.
On Thu, May 3, 2012 at 5:40 PM, Richard Earnshaw rearn...@arm.com wrote:
On 03/05/12 11:27, Dinar Temirbulatov wrote:
This clearly needs more work.
Comments: Need to end with a period and two spaces.
Binary Operators: Need to be surrounded with white space.
sorry for this, hope I resolved such issues with the new version.
As Andrew Haley has already said, some documentation of the algorithm is
needed.
General documentation for the issue could be found here
gmplib.org/~tege/divcnst-pldi94.pdf.
Multiplication to get higher 128-bit was developed by me and Alexey
Kravets, I attached C version of the algorithm.
Why is this restricted to BITS_PER_WORD == 32?
I am checking here that we are generating code for 32-bit target with
32-bit wide general propose registers, and with 64-bit wide registers
usually there is an instruction to get the higher 64-bit of 64x64-bit
multiplication cheaply.
Costs: This code clearly needs to understand the relative cost of doing
division this way; there is a limit to the amount of inline expansion
that we should tolerate, particularly at -O2 and it's not clear that
this will be much better than a library call if we don't have a widening
multiply operation (as is true for older ARM chips, and presumably some
other CPUs). In essence, you need to work out the cost of a divide
instruction (just as rtx costs for this) and the approximate cost of the
expanded algorithm.
Added cost calculation.
Another issue that worries me is the number of live DImode values; on
machines with few registers this could cause excessive spilling to start
happening.
The cost calculation suppose to take this into account.
I also wonder whether it would be beneficial to generate custom
functions for division by specific constants (using this algorithm) and
then call those functions rather than the standard lib-call. On ELF
systems the functions can marked as hidden and put into common sections
so that we only end up with one instance of each function in a program.
yes, I think it is a good approach, I could redo my patch with such
intrinsic function implementation with pre-shift, post-shift, and
64-bit constant as function parameters.
Finally, do you have a copyright assignment with the FSF? We can't use
this code without one.
yes, I do have a copyright assignment with the FSF.
Also I am in process of implementing signed integer 64-bit division by constant.
thanks, Dinar.
18.patch
Description: Binary data
unsigned long long mul(unsigned long long a, unsigned long long b)
{
unsigned long long x1=a32;
unsigned long long x0=a0x;
unsigned long long y1=b32;
unsigned long long y0=b0x;
unsigned long long z2, z0, tmp, u0, u1;
unsigned char c=0;
z2=x1*y1;
z0=x0*y0;
u0=x0*y1+(z032);
u1=x1*y0;
tmp = (u0+u1);
c = (tmp u0) || (tmp u1);
return z2+(tmp32)+(((unsigned long long)c)32);
}
Re: fix cross build
On Tue, May 22, 2012 at 3:24 PM, Nathan Sidwell nat...@acm.org wrote:
On 05/21/12 11:03, Richard Guenther wrote:
Hmm - I think this papers over the issue that the CONSTRUCTOR is not
properly gimplified - it still contains a TARGET_EXPR which is not valid
GIMPLE.
Why is that TARGET_EXPR not gimplified?
As far as I can make out, it just doesn't look inside the constructor.
0 gimplify_expr (expr_p=0xb7e90394, pre_p=0xbfffe514, post_p=0xbfffd08c,
gimple_test_f=0x84b0a80 is_gimple_min_lval(tree_node*), fallback=3) at
../../src/gcc/gimplify.c:7356
#1 0x084f5882 in gimplify_compound_lval (expr_p=0xb7e9cb94,
pre_p=0xbfffe514, post_p=0xbfffd08c, fallback=1)
at ../../src/gcc/gimplify.c:2259
#2 0x08519878 in gimplify_expr (expr_p=0xb7e9cb94, pre_p=0xbfffe514,
post_p=0xbfffd08c,
gimple_test_f=0x84eaf9f is_gimple_reg_rhs_or_call(tree), fallback=1) at
../../src/gcc/gimplify.c:7081
#3 0x085087f7 in gimplify_modify_expr (expr_p=0xbfffd6d0, pre_p=0xbfffe514,
post_p=0xbfffd08c, want_value=false)
at ../../src/gcc/gimplify.c:4843
The switch case at that stack frame is for CONSTRUCTORs. It has the
comment:
/* Don't reduce this in place; let gimplify_init_constructor work its
magic. Buf if we're just elaborating this for side effects,
just
gimplify any element that has side-effects. */
But we never enter G_I_C before the ICE.
On the second time we get here, fallback has the value 1 at that point
(fb_rvalue), so neither if condition is satisified, and we end up at
ret = GS_ALL_DONE;
we then proceed down to enter:
else if ((fallback fb_rvalue) is_gimple_reg_rhs_or_call (*expr_p))
{
/* An rvalue will do. Assign the gimplified expression into a
new temporary TMP and replace the original expression with
TMP. First, make sure that the expression has a type so that
it can be assigned into a temporary. */
gcc_assert (!VOID_TYPE_P (TREE_TYPE (*expr_p)));
*expr_p = get_formal_tmp_var (*expr_p, pre_p);
}
and ICE inside GFTV when it attempts to generate the hash.
AFAICT, changing the test case to:
char *str = ((union {const char * _q; char * _nq;})
((const char *)((num_string)
-string.str)))._nq;
(i.e. removing the stmt-expr) results in the same logical flow as above, but
there's no TARGET_EXPR inside the constructor.
I'm not sure how it's supposed to work, so I'm a little lost.
Hm, ok. I see what happens. The issue is that the CONSTRUCTOR has
elements with TREE_SIDE_EFFECTS set but is itself not TREE_SIDE_EFFECTS.
Which would have avoided all this mess in lookup_tmp_var. I suppose
for duplicating the initializer you could even generate wrong-code with your fix
in(?). Now, we never set TREE_SIDE_EFFECTS from build_constructor
(but only TREE_CONSTANT), so the fix could either be to fix that or to
exclude CONSTRUCTORs in lookup_tmp_var like
Index: gcc/gimplify.c
===
--- gcc/gimplify.c (revision 187772)
+++ gcc/gimplify.c (working copy)
@@ -514,7 +514,8 @@ lookup_tmp_var (tree val, bool is_formal
block, which means it will go into memory, causing much extra
work in reload and final and poorer code generation, outweighing
the extra memory allocation here. */
- if (!optimize || !is_formal || TREE_SIDE_EFFECTS (val))
+ if (!optimize || !is_formal || TREE_SIDE_EFFECTS (val)
+ || TREE_CODE (val) == CONSTRUCTOR)
ret = create_tmp_from_val (val);
else
{
after all lookup_tmp_var performs a simple-minded CSE here.
But I wonder why CONSTRUCTORs do not inherit TREE_SIDE_EFFECTS
properly ...
Richard.
[PATCH] rs6000.c missing target hook
During one of the target hook conversions,
rs6000_aix_asm_output_dwarf_table_ref() retained a use of
TARGET_STRIP_NAME_ENCODING macro instead of the target hook, which
broke when Alan cleaned up rs6000.c. Fixed thusly.
- David
* config/rs6000/rs6000.c (rs6000_aix_asm_output_dwarf_table_ref): Use
strip_name_encoding target hook.
Index: rs6000.c
===
--- rs6000.c(revision 187756)
+++ rs6000.c(working copy)
@@ -18392,7 +18392,7 @@
rs6000_aix_asm_output_dwarf_table_ref (char * frame_table_label)
{
fprintf (asm_out_file, \t.ref %s\n,
- TARGET_STRIP_NAME_ENCODING (frame_table_label));
+ (* targetm.strip_name_encoding) (frame_table_label));
}
/* This ties together stack memory (MEM with an alias set of frame_alias_set)
Re: [Patch, libgfortran] PR53445/53444 - Fix compilation on VxWorks
On 05/22/2012 03:06 PM, Tobias Burnus wrote:
The attached patches fix compilation issues on VxWorks.
a) VxWorks has strerror_r but contrary to POSIX, the function in
VxWorks (at least in older versions) takes only two arguments: errnum
and buf and not also the buffer length. I added a configure check for
that variant.
I forgot to attach that patch. Now with patch and automake 1.11.1 for
the generated files.
Tobias
2012-05-22 Tobias Burnus bur...@net-b.de
PR libfortran/53444
* acinclude.m4 (LIBGFOR_CHECK_STRERROR_R): Add configure checks for
two- and three-argument versions of strerror_r.
* configure.ac (LIBGFOR_CHECK_STRERROR_R): Use it.
* runtime/error.c (gf_strerror): Handle two-argument version
of strerror_r.
* config.h.in: Regenerate.
* configure: Regenerate.
diff --git a/libgfortran/runtime/error.c b/libgfortran/runtime/error.c
index 7e523bc..8e8d80d 100644
--- a/libgfortran/runtime/error.c
+++ b/libgfortran/runtime/error.c
@@ -212,6 +212,7 @@ gf_strerror (int errnum,
size_t buflen __attribute__((unused)))
{
#ifdef HAVE_STRERROR_R
+ /* POSIX returns an int, GNU a char*. */
return
__builtin_choose_expr (__builtin_classify_type (strerror_r (0, buf, 0))
== 5,
@@ -219,6 +220,14 @@ gf_strerror (int errnum,
strerror_r (errnum, buf, buflen),
/* POSIX strerror_r () */
(strerror_r (errnum, buf, buflen), buf));
+#elif defined(HAVE_STRERROR_R_2ARGS)
+ return
+__builtin_choose_expr (__builtin_classify_type (strerror_r (0, buf))
+ == 5,
+ /* char*-returning strerror_r() */
+ strerror_r (errnum, buf),
+ /* int-returning strerror_r () */
+ (strerror_r (errnum, buf), buf));
#else
/* strerror () is not necessarily thread-safe, but should at least
be available everywhere. */
diff --git a/libgfortran/configure.ac b/libgfortran/configure.ac
index 41bef72..fc58a5c 100644
--- a/libgfortran/configure.ac
+++ b/libgfortran/configure.ac
@@ -265,9 +265,12 @@ AC_CHECK_FUNCS_ONCE(getrusage times mkstemp strtof strtold snprintf \
ftruncate chsize chdir getlogin gethostname kill link symlink sleep ttyname \
alarm access fork execl wait setmode execve pipe dup2 close \
strcasestr getrlimit gettimeofday stat fstat lstat getpwuid vsnprintf dup \
-getcwd localtime_r gmtime_r strerror_r getpwuid_r ttyname_r clock_gettime \
+getcwd localtime_r gmtime_r getpwuid_r ttyname_r clock_gettime \
readlink getgid getpid getppid getuid geteuid umask getegid __secure_getenv)
+# Check strerror_r, cannot be above as versions with two and three arguments exist
+LIBGFOR_CHECK_STRERROR_R
+
# Check for C99 (and other IEEE) math functions
GCC_CHECK_MATH_FUNC([acosf])
GCC_CHECK_MATH_FUNC([acos])
diff --git a/libgfortran/acinclude.m4 b/libgfortran/acinclude.m4
index 1b11e6a..d101261 100644
--- a/libgfortran/acinclude.m4
+++ b/libgfortran/acinclude.m4
@@ -362,3 +362,29 @@ AC_DEFUN([LIBGFOR_CHECK_FLOAT128], [
dnl We need a conditional for the Makefile
AM_CONDITIONAL(LIBGFOR_BUILD_QUAD, [test x$libgfor_cv_have_float128 = xyes])
])
+
+
+dnl Check whether we have strerror_r
+AC_DEFUN([LIBGFOR_CHECK_STRERROR_R], [
+ dnl Check for three-argument POSIX version of strerror_r
+ ac_save_CFLAGS=$CFLAGS
+ CFLAGS=-Wimplicit-function-declaration -Werror
+ AC_TRY_COMPILE([#define _GNU_SOURCE 1
+ #include string.h
+ #include locale.h],
+ [char s[128]; strerror_r(5, s, 128);],
+ AC_DEFINE(HAVE_STRERROR_R, 1,
+ [Define if strerror_r is available in string.h.]),)
+ CFLAGS=$ac_save_CFLAGS
+
+ dnl Check for two-argument version of strerror_r (e.g. for VxWorks)
+ ac_save_CFLAGS=$CFLAGS
+ CFLAGS=-Wimplicit-function-declaration -Werror
+ AC_TRY_COMPILE([#define _GNU_SOURCE 1
+ #include string.h
+ #include locale.h],
+ [char s[128]; strerror_r(5, s);],
+ AC_DEFINE(HAVE_STRERROR_R_2ARGS, 1,
+ [Define if strerror_r takes two arguments and is available in string.h.]),)
+ CFLAGS=$ac_save_CFLAGS
+])
diff --git a/libgfortran/config.h.in b/libgfortran/config.h.in
index b75fa96..43ba025 100644
--- a/libgfortran/config.h.in
+++ b/libgfortran/config.h.in
@@ -696,9 +696,12 @@
/* Define to 1 if you have the `strcasestr' function. */
#undef HAVE_STRCASESTR
-/* Define to 1 if you have the `strerror_r' function. */
+/* Define if strerror_r is available in string.h. */
#undef HAVE_STRERROR_R
+/* Define if strerror_r takes two arguments and is available in string.h. */
+#undef HAVE_STRERROR_R_2ARGS
+
/* Define to 1 if you have the strings.h header file. */
#undef HAVE_STRINGS_H
diff --git a/libgfortran/configure b/libgfortran/configure
index 80bfe84..ca33870 100755
--- a/libgfortran/configure
+++ b/libgfortran/configure
@@ -2581,7 +2581,6 @@ as_fn_append ac_func_list dup
as_fn_append ac_func_list getcwd
as_fn_append ac_func_list localtime_r
as_fn_append ac_func_list gmtime_r
-as_fn_append ac_func_list strerror_r
as_fn_append ac_func_list getpwuid_r
as_fn_append ac_func_list ttyname_r
Re: [PATCH preprocessor, diagnostics] PR preprocessor/53229 - Fix diagnostics location when pasting tokens
On 05/22/2012 05:04 AM, Dodji Seketeli wrote: The problem is that cpp_get_token_1 can be called when we are at the beginning of a macro expansion (inside enter_macro_expansion, called from cpp_get_token_1), *before* context-c.macro is set. This happens e.g, when we call funlike_invocation_p to know if the current macro is function-like or not. OK, sounds like we need some additional code to handle that. I guess we could do something in funlike_invocation_p to prevent cpp_get_token_1 from setting invocation_location, or change the check we use to decide whether or not we already have an invocation location, perhaps by looking at invocation_location itself (and clearing it when we finish a macro). Jason
Re: fix cross build
On 05/22/12 15:12, Richard Guenther wrote: thanks! But I wonder why CONSTRUCTORs do not inherit TREE_SIDE_EFFECTS properly ... yeah, that would seem to be the error. Looking ...
Re: Request new specs string token for multilib_os_dir
On Fri, 27 Apr 2012, Steven Drake wrote: 2012-04-27 Steven Drake s...@netbsd.org * gcc.c (do_spec_1): Add %M spec token to output multilib_os_dir. Thanks, I've committed this patch with the addition of %M to the comment in gcc.c that documents specs. -- Joseph S. Myers jos...@codesourcery.com
Re: [C++ Patch] PR 44516
On 05/22/2012 08:22 AM, Paolo Carlini wrote: Ok, thanks. Today I wanted to concretely test and post a patch but I noticed this other objc bit in objc-next-runtime-abi-01.c which I don't know how to handle due to the OPT_Wall gate: warning_at (UNKNOWN_LOCATION, OPT_Wall, %-fobjc-sjlj-exceptions% is the only supported exceptions system for %-fnext-runtime% with %-fobjc-abi-version% 2); If the objc maintainers could help with those uses of warning_at (0 in the objc tree, I have a patch ready for the asserts proper. What if we just changed UNKNOWN_LOCATION to BUILTINS_LOCATION for these diagnostics? Jason
Re: divide 64-bit by constant for 32-bit target machines
On 05/22/12 07:05, Dinar Temirbulatov wrote: + if ((size - 1 BITS_PER_WORD +BITS_PER_WORD == 32 mode == DImode) Do note that this patch will not go in with hard-coded SImode and DImode references. Which, really, you do not even need. GET_MODE_BITSIZE (mode) == 2*BITS_PER_WORD is what you wanted for testing for double-word-ness, and word_mode is a good substitute for SImode here. + /* Splitting the 64-bit constant for the higher and the lower parts. */ + y0 = gen_rtx_CONST_INT (DImode, dUINT32_MAX); + y1 = gen_rtx_CONST_INT (DImode, d32); Use gen_int_mode. + x1 = convert_to_mode (DImode, x1, 1); + x0 = convert_to_mode (DImode, x0, 1); + + /* Splitting the 64-bit constant for the higher and the lower parts. */ + y0 = gen_rtx_CONST_INT (DImode, dUINT32_MAX); + y1 = gen_rtx_CONST_INT (DImode, d32); + + z2 = gen_reg_rtx (DImode); + u0 = gen_reg_rtx (DImode); + + /* Unsigned multiplication of the higher multiplier part + and the higher constant part. */ + z2 = expand_mult(DImode, x1, y1, z2, 1); + /* Unsigned multiplication of the lower multiplier part + and the higher constant part. */ + u0 = expand_mult(DImode, x0, y1, u0, 1); I'm fairly sure you really want to be using expand_widening_mult here, rather than using convert_to_mode first. While combine may be able to re-generate a widening multiply out of this sequence, there's no sense making it work too hard. r~
Re: [C++ Patch] PR 44516
On 05/22/2012 05:42 PM, Jason Merrill wrote:
On 05/22/2012 08:22 AM, Paolo Carlini wrote:
Ok, thanks. Today I wanted to concretely test and post a patch but I
noticed this other objc bit in objc-next-runtime-abi-01.c which I don't
know how to handle due to the OPT_Wall gate:
warning_at (UNKNOWN_LOCATION, OPT_Wall,
%-fobjc-sjlj-exceptions% is the only supported exceptions
system for %-fnext-runtime% with %-fobjc-abi-version% 2);
If the objc maintainers could help with those uses of warning_at (0 in
the objc tree, I have a patch ready for the asserts proper.
What if we just changed UNKNOWN_LOCATION to BUILTINS_LOCATION for
these diagnostics?
Maybe... I don't know really, what happens with that location.
Alternately, I bootstrapped and I'm finishing testing the below. Let me
know, I don't have a strong opinion at the moment: if we can
consistently use inform for all those general messages provided by objc,
I think it would be good, I don't know how bad is adding warn_all.
Thanks,
Paolo.
///
2012-05-22 Paolo Carlini paolo.carl...@oracle.com
* diagnostic.c (warning_at, error_n, error_at): Assert location !=
UNKNOWN_LOCATION.
/c-family
2012-05-22 Paolo Carlini paolo.carl...@oracle.com
* c.opt [Wall]: Define warn_all.
/objc
2012-05-22 Paolo Carlini paolo.carl...@oracle.com
* objc-gnu-runtime-abi-01.c (objc_gnu_runtime_abi_01_init): Use
inform instead of warning_at (0; minor formatting fixes.
* objc-next-runtime-abi-01.c (objc_next_runtime_abi_01_init):
Likewise.
Index: diagnostic.c
===
--- diagnostic.c(revision 187760)
+++ diagnostic.c(working copy)
@@ -798,6 +798,8 @@ warning_at (location_t location, int opt, const ch
diagnostic_info diagnostic;
va_list ap;
bool ret;
+
+ gcc_assert (location != UNKNOWN_LOCATION);
va_start (ap, gmsgid);
diagnostic_set_info (diagnostic, gmsgid, ap, location, DK_WARNING);
@@ -881,6 +883,8 @@ error_n (location_t location, int n, const char *s
diagnostic_info diagnostic;
va_list ap;
+ gcc_assert (location != UNKNOWN_LOCATION);
+
va_start (ap, plural_gmsgid);
diagnostic_set_info_translated (diagnostic,
ngettext (singular_gmsgid, plural_gmsgid, n),
@@ -891,13 +895,15 @@ error_n (location_t location, int n, const char *s
/* Same as ebove, but use location LOC instead of input_location. */
void
-error_at (location_t loc, const char *gmsgid, ...)
+error_at (location_t location, const char *gmsgid, ...)
{
diagnostic_info diagnostic;
va_list ap;
+ gcc_assert (location != UNKNOWN_LOCATION);
+
va_start (ap, gmsgid);
- diagnostic_set_info (diagnostic, gmsgid, ap, loc, DK_ERROR);
+ diagnostic_set_info (diagnostic, gmsgid, ap, location, DK_ERROR);
report_diagnostic (diagnostic);
va_end (ap);
}
Index: c-family/c.opt
===
--- c-family/c.opt (revision 187760)
+++ c-family/c.opt (working copy)
@@ -265,7 +265,7 @@ C ObjC C++ ObjC++ Var(warn_address) Warning
Warn about suspicious uses of memory addresses
Wall
-C ObjC C++ ObjC++ Warning
+C ObjC C++ ObjC++ Var(warn_all) Warning
Enable most warning messages
Wassign-intercept
Index: objc/objc-next-runtime-abi-01.c
===
--- objc/objc-next-runtime-abi-01.c (revision 187760)
+++ objc/objc-next-runtime-abi-01.c (working copy)
@@ -146,12 +146,11 @@ bool
objc_next_runtime_abi_01_init (objc_runtime_hooks *rthooks)
{
if (flag_objc_exceptions
- !flag_objc_sjlj_exceptions)
-{
- warning_at (UNKNOWN_LOCATION, OPT_Wall,
- %-fobjc-sjlj-exceptions% is the only supported exceptions
- system for %-fnext-runtime% with %-fobjc-abi-version%
2);
-}
+ !flag_objc_sjlj_exceptions
+ warn_all)
+inform (UNKNOWN_LOCATION,
+ %-fobjc-sjlj-exceptions% is the only supported exceptions
+ system for %-fnext-runtime% with %-fobjc-abi-version% 2);
rthooks-initialize = next_runtime_01_initialize;
rthooks-default_constant_string_class_name = DEF_CONSTANT_STRING_CLASS_NAME;
Index: objc/objc-gnu-runtime-abi-01.c
===
--- objc/objc-gnu-runtime-abi-01.c (revision 187760)
+++ objc/objc-gnu-runtime-abi-01.c (working copy)
@@ -1,5 +1,5 @@
/* GNU Runtime ABI version 8
- Copyright (C) 2011 Free Software Foundation, Inc.
+ Copyright (C) 2011, 2012 Free Software Foundation, Inc.
Contributed by Iain Sandoe (split from objc-act.c)
This file is part of GCC.
@@ -128,15 +128,17 @@ objc_gnu_runtime_abi_01_init (objc_runtime_hooks *
/* GNU runtime does not need the compiler to change code in order to do GC.
*/
if (flag_objc_gc)
{
- warning_at (0, 0, %-fobjc-gc% is ignored for
Re: [C++ Patch] PR 44516
On 05/22/2012 05:49 PM, Paolo Carlini wrote: On 05/22/2012 05:42 PM, Jason Merrill wrote: On 05/22/2012 08:22 AM, Paolo Carlini wrote: Ok, thanks. Today I wanted to concretely test and post a patch but I noticed this other objc bit in objc-next-runtime-abi-01.c which I don't know how to handle due to the OPT_Wall gate: warning_at (UNKNOWN_LOCATION, OPT_Wall, %-fobjc-sjlj-exceptions% is the only supported exceptions system for %-fnext-runtime% with %-fobjc-abi-version% 2); If the objc maintainers could help with those uses of warning_at (0 in the objc tree, I have a patch ready for the asserts proper. What if we just changed UNKNOWN_LOCATION to BUILTINS_LOCATION for these diagnostics? Maybe... I don't know really, what happens with that location. Alternately, I bootstrapped and I'm finishing testing the below. Let me know, I don't have a strong opinion at the moment: if we can consistently use inform for all those general messages provided by objc, I think it would be good, I don't know how bad is adding warn_all. Sh*it, doesn't work, no idea why. FAIL: gcc.dg/opts-1.c (internal compiler error) FAIL: gcc.dg/opts-1.c -fno-abi-version (test for errors, line ) FAIL: gcc.dg/opts-1.c -fno-lto-compression-level (test for errors, line ) FAIL: gcc.dg/opts-1.c -fno-tree-parallelize-loops (test for errors, line ) FAIL: gcc.dg/opts-1.c -Wno-strict-overflow (test for errors, line ) FAIL: gcc.dg/opts-1.c -Wno-strict-aliasing (test for errors, line ) FAIL: gcc.dg/opts-1.c (test for excess errors) FAIL: gcc.dg/opts-5.c (internal compiler error) FAIL: gcc.dg/opts-5.c (test for errors, line ) FAIL: gcc.dg/opts-5.c (test for excess errors) FAIL: gcc.dg/opts-6.c (internal compiler error) FAIL: gcc.dg/opts-6.c (test for errors, line ) FAIL: gcc.dg/opts-6.c (test for excess errors) There is stuff we expect to print at line 0. I'm really annoyed. Paolo. Paolo.
Re: [DF] Generate REFs in REGNO order
On Tue, 22 May 2012, Paolo Bonzini wrote:
Il 21/05/2012 19:49, Dimitrios Apostolou ha scritto:
Thanks for reviewing, in the meantime I'll try to figure out why this
patch doesn't offer any speed-up on ppc64 (doesn't break anything
though), so expect a followup by tomorrow.
Perhaps you hit this?
else if (GET_CODE (XEXP (note, 0)) == CLOBBER)
{
if (REG_P (XEXP (XEXP (note, 0), 0)))
{
unsigned int regno = REGNO (XEXP (XEXP (note, 0), 0));
if (!TEST_HARD_REG_BIT (defs_generated, regno))
df_defs_record (collection_rec, XEXP (note, 0), bb,
insn_info, flags);
}
You are right, and I noticed that if we reverse (actually put straight)
the loop for the PARALLEL defs inside df_defs_record() then the speedup
stands for both x86 and ppc64.
The following patch was tested on x86, do you think it is meaningful for
the generic case?
--- gcc/df-scan.c 2012-05-06 04:08:43 +
+++ gcc/df-scan.c 2012-05-22 13:08:33 +
@@ -3010,7 +3010,7 @@ df_defs_record (struct df_collection_rec
break;
case PARALLEL:
- for (i = XVECLEN (x, 0) - 1; i = 0; i--)
+ for (i = 0; i XVECLEN (x, 0); i++)
df_defs_record (collection_rec, XVECEXP (x, 0, i),
bb, insn_info, flags);
break;
Thanks,
Dimitris
Re: [PATCH] Hoist adjacent pointer loads
Here's a revision of the hoist-adjacent-loads patch. Besides hopefully
addressing all your comments, I added a gate of at least -O2 for this
transformation. Let me know if you prefer a different minimum opt
level.
I'm still running SPEC tests to make sure there are no regressions when
opening this up to non-pointer arguments. The code bootstraps on
powerpc64-unknown-linux-gnu with no regressions. Assuming the SPEC
numbers come out as expected, is this ok?
Thanks,
Bill
2012-05-22 Bill Schmidt wschm...@linux.vnet.ibm.com
* tree-ssa-phiopt.c (tree_ssa_phiopt_worker): Add argument to forward
declaration.
(hoist_adjacent_loads, gate_hoist_loads): New forward declarations.
(tree_ssa_phiopt): Call gate_hoist_loads.
(tree_ssa_cs_elim): Add parm to tree_ssa_phiopt_worker call.
(tree_ssa_phiopt_worker): Add do_hoist_loads to formal arg list; call
hoist_adjacent_loads.
(local_mem_dependence): New function.
(hoist_adjacent_loads): Likewise.
(gate_hoist_loads): Likewise.
* common.opt (fhoist-adjacent-loads): New switch.
* Makefile.in (tree-ssa-phiopt.o): Added dependencies.
* params.def (PARAM_MIN_CMOVE_STRUCT_ALIGN): New param.
Index: gcc/tree-ssa-phiopt.c
===
--- gcc/tree-ssa-phiopt.c (revision 187728)
+++ gcc/tree-ssa-phiopt.c (working copy)
@@ -37,9 +37,17 @@ along with GCC; see the file COPYING3. If not see
#include cfgloop.h
#include tree-data-ref.h
#include tree-pretty-print.h
+#include gimple-pretty-print.h
+#include insn-config.h
+#include expr.h
+#include optabs.h
+#ifndef HAVE_conditional_move
+#define HAVE_conditional_move (0)
+#endif
+
static unsigned int tree_ssa_phiopt (void);
-static unsigned int tree_ssa_phiopt_worker (bool);
+static unsigned int tree_ssa_phiopt_worker (bool, bool);
static bool conditional_replacement (basic_block, basic_block,
edge, edge, gimple, tree, tree);
static int value_replacement (basic_block, basic_block,
@@ -53,6 +61,9 @@ static bool cond_store_replacement (basic_block, b
static bool cond_if_else_store_replacement (basic_block, basic_block,
basic_block);
static struct pointer_set_t * get_non_trapping (void);
static void replace_phi_edge_with_variable (basic_block, edge, gimple, tree);
+static void hoist_adjacent_loads (basic_block, basic_block,
+ basic_block, basic_block);
+static bool gate_hoist_loads (void);
/* This pass tries to replaces an if-then-else block with an
assignment. We have four kinds of transformations. Some of these
@@ -138,12 +149,56 @@ static void replace_phi_edge_with_variable (basic_
bb2:
x = PHI x' (bb0), ...;
- A similar transformation is done for MAX_EXPR. */
+ A similar transformation is done for MAX_EXPR.
+
+ This pass also performs a fifth transformation of a slightly different
+ flavor.
+
+ Adjacent Load Hoisting
+ --
+
+ This transformation replaces
+
+ bb0:
+ if (...) goto bb2; else goto bb1;
+ bb1:
+ x1 = (expr).field1;
+ goto bb3;
+ bb2:
+ x2 = (expr).field2;
+ bb3:
+ # x = PHI x1, x2;
+
+ with
+
+ bb0:
+ x1 = (expr).field1;
+ x2 = (expr).field2;
+ if (...) goto bb2; else goto bb1;
+ bb1:
+ goto bb3;
+ bb2:
+ bb3:
+ # x = PHI x1, x2;
+
+ The purpose of this transformation is to enable generation of conditional
+ move instructions such as Intel CMOVE or PowerPC ISEL. Because one of
+ the loads is speculative, the transformation is restricted to very
+ specific cases to avoid introducing a page fault. We are looking for
+ the common idiom:
+
+ if (...)
+ x = y-left;
+ else
+ x = y-right;
+
+ where left and right are typically adjacent pointers in a tree structure.
*/
+
static unsigned int
tree_ssa_phiopt (void)
{
- return tree_ssa_phiopt_worker (false);
+ return tree_ssa_phiopt_worker (false, gate_hoist_loads ());
}
/* This pass tries to transform conditional stores into unconditional
@@ -190,7 +245,7 @@ tree_ssa_phiopt (void)
static unsigned int
tree_ssa_cs_elim (void)
{
- return tree_ssa_phiopt_worker (true);
+ return tree_ssa_phiopt_worker (true, false);
}
/* Return the singleton PHI in the SEQ of PHIs for edges E0 and E1. */
@@ -227,9 +282,11 @@ static tree condstoretemp;
/* The core routine of conditional store replacement and normal
phi optimizations. Both share much of the infrastructure in how
to match applicable basic block patterns. DO_STORE_ELIM is true
- when we want to do conditional store replacement, false otherwise. */
+ when we want to do conditional store replacement, false otherwise.
+ DO_HOIST_LOADS is true when we want to hoist adjacent loads out
+ of diamond control flow patterns, false otherwise. */
static unsigned int
Re: [C++ Patch] PR 44516
On 05/22/2012 11:55 AM, Paolo Carlini wrote: FAIL: gcc.dg/opts-1.c (internal compiler error) FAIL: gcc.dg/opts-1.c -fno-abi-version (test for errors, line ) FAIL: gcc.dg/opts-1.c -fno-lto-compression-level (test for errors, line ) FAIL: gcc.dg/opts-1.c -fno-tree-parallelize-loops (test for errors, line ) FAIL: gcc.dg/opts-1.c -Wno-strict-overflow (test for errors, line ) FAIL: gcc.dg/opts-1.c -Wno-strict-aliasing (test for errors, line ) FAIL: gcc.dg/opts-1.c (test for excess errors) FAIL: gcc.dg/opts-5.c (internal compiler error) FAIL: gcc.dg/opts-5.c (test for errors, line ) FAIL: gcc.dg/opts-5.c (test for excess errors) FAIL: gcc.dg/opts-6.c (internal compiler error) FAIL: gcc.dg/opts-6.c (test for errors, line ) FAIL: gcc.dg/opts-6.c (test for excess errors) That all looks like diagnostics about options that don't work properly together, much like the objc issues. I think it makes sense to switch all such diagnostics over to BUILTINS_LOCATION, because we do know the location of the thing we're complaining about: it's on the command line. Jason
Re: [C++ Patch] PR 44516
On 05/22/2012 06:42 PM, Jason Merrill wrote: On 05/22/2012 11:55 AM, Paolo Carlini wrote: FAIL: gcc.dg/opts-1.c (internal compiler error) FAIL: gcc.dg/opts-1.c -fno-abi-version (test for errors, line ) FAIL: gcc.dg/opts-1.c -fno-lto-compression-level (test for errors, line ) FAIL: gcc.dg/opts-1.c -fno-tree-parallelize-loops (test for errors, line ) FAIL: gcc.dg/opts-1.c -Wno-strict-overflow (test for errors, line ) FAIL: gcc.dg/opts-1.c -Wno-strict-aliasing (test for errors, line ) FAIL: gcc.dg/opts-1.c (test for excess errors) FAIL: gcc.dg/opts-5.c (internal compiler error) FAIL: gcc.dg/opts-5.c (test for errors, line ) FAIL: gcc.dg/opts-5.c (test for excess errors) FAIL: gcc.dg/opts-6.c (internal compiler error) FAIL: gcc.dg/opts-6.c (test for errors, line ) FAIL: gcc.dg/opts-6.c (test for excess errors) That all looks like diagnostics about options that don't work properly together, much like the objc issues. I think it makes sense to switch all such diagnostics over to BUILTINS_LOCATION, because we do know the location of the thing we're complaining about: it's on the command line. Ok, I didn't notice before that we want to do this in the driver too, besides objc. For sure now I see a warning_at (0 in opts-global.c. Let's see what happens if I find all of them and I replace UNKNOWN_LOCATION with BUILTINS_LOCATION. Thanks, Paolo.
Re: [C++ RFH / Patch] PR 53159
Hi, On 05/22/2012 12:53 AM, Jason Merrill wrote: On 05/21/2012 04:18 PM, Paolo Carlini wrote: -check_narrowing (type, init); +check_narrowing (type, perform_implicit_conversion + (type, tinit, tf_none)); Any changes should go inside check_narrowing rather than outside. I think the best fix if ftype is a class would be to find the implicit conversion sequence from ftype to type, then drop the second standard conversion sequence and just perform the user-defined conversion, then continue with the rest of check_narrowing. The conversion part should be a new function in call.c. Let's see if I understand in detail the structure of the new function (beyond all the simplifications possible because the target is a scalar, etc): we want something quite similar to implicit_conversion, but at the end of it, instead of using build_user_type_conversion_1 - which returns a conversion and implies that the caller passes it to convert_like_real - something like build_user_type_conversion (maybe exactly build_user_type_conversion) Do I understand correctly? Thanks, Paolo.
libgo patch committed: Use getcontext, not setjmp, to save regs for GC
When starting a system call, the libgo library needs to save the registers in a place where the garbage collector can see them, so that if the registers happen to be pointers the objects to which they point will not be collected. I thought that I could simply use setjmp for that. Unfortunately, I just managed to uncover the fact that setjmp mangles the register values, so that the garbage collector does not understand them. This means that a particularly poorly timed garbage collection can discard values used by a system call. This patch changes libgo to use getcontext as well, which should work at least until getcontext also gets pointer mangling. It appears that in the long run we will need processor-specific context save and restore routines in libgo. Bootstrapped and ran Go testsuite on x86_64-unknown-linux-gnu. Committed to mainline and 4.7 branch. Ian diff -r edb476bbcabd libgo/runtime/proc.c --- a/libgo/runtime/proc.c Fri May 18 13:14:19 2012 -0700 +++ b/libgo/runtime/proc.c Tue May 22 09:51:13 2012 -0700 @@ -1239,9 +1239,7 @@ // Save the registers in the g structure so that any pointers // held in registers will be seen by the garbage collector. - // We could use getcontext here, but setjmp is more efficient - // because it doesn't need to save the signal mask. - setjmp(g-gcregs); + getcontext(g-gcregs); g-status = Gsyscall; @@ -1299,7 +1297,7 @@ gp-gcstack = nil; #endif gp-gcnext_sp = nil; - runtime_memclr(gp-gcregs, sizeof gp-gcregs); + runtime_memclr(gp-gcregs, sizeof gp-gcregs); if(m-profilehz 0) runtime_setprof(true); @@ -1328,7 +1326,7 @@ gp-gcstack = nil; #endif gp-gcnext_sp = nil; - runtime_memclr(gp-gcregs, sizeof gp-gcregs); + runtime_memclr(gp-gcregs, sizeof gp-gcregs); } // Allocate a new g, with a stack big enough for stacksize bytes. diff -r edb476bbcabd libgo/runtime/runtime.h --- a/libgo/runtime/runtime.h Fri May 18 13:14:19 2012 -0700 +++ b/libgo/runtime/runtime.h Tue May 22 09:51:13 2012 -0700 @@ -7,7 +7,6 @@ #include config.h #include go-assert.h -#include setjmp.h #include signal.h #include stdio.h #include stdlib.h @@ -128,7 +127,7 @@ void* gcnext_segment; void* gcnext_sp; void* gcinitial_sp; - jmp_buf gcregs; + ucontext_t gcregs; byte* entry; // initial function G* alllink; // on allg void* param; // passed parameter on wakeup
Re: [C++ RFH / Patch] PR 53159
On 05/22/2012 12:52 PM, Paolo Carlini wrote: Let's see if I understand in detail the structure of the new function (beyond all the simplifications possible because the target is a scalar, etc): we want something quite similar to implicit_conversion, but at the end of it, instead of using build_user_type_conversion_1 - which returns a conversion and implies that the caller passes it to convert_like_real - something like build_user_type_conversion (maybe exactly build_user_type_conversion) Do I understand correctly? We want to call convert_like_real on the conversion sequence in conv at the end of build_user_type_conversion_1: that is, the conversion sequence before the call to merge_conversion_sequences. Jason
Re: trans-mem: make sure clones for functions referenced indirectly are marked as needed (issue6201064)
Hi Dave. Without this patch we generate calls to TM_GETTMCLONE for functions called indirectly, but we don't actually store the clone mapping in the clone table because we think the functions are not needed. Compiles fine, dies at runtime. See GCC Bugzilla – Bug 53008 Have you taken a look at?: http://gcc.gnu.org/contribute.html In particular, you should include a testcase, and have tested your patch as suggested in the above link. Also, it is customary to include the bugzilla number in the subject as so: Subject: [PR middle-end/53008] trans-mem: make sure clones Similarly, the ChangeLog entry should reference PR middle-end/53008. This makes the final commit to be referenced automagically from the PR itself. All this makes it easier to review your patch. Thanks. Aldy
Re: [Patch]: Fix very large frame bug on i386
On 05/15/12 02:10, Tristan Gingold wrote: 2012-05-15 Tristan Gingold ging...@adacore.com * config/i386/i386.c (struct ix86_frame): Remove unused frame field. (ix86_compute_frame_layout): Fix type of stack_alignment_needed and preferred_alignment. Ok with a test case. Even if it's a scan-assember sort of test. r~
Re: [Patch]: Fix typo in common/config/ia64/ia64-common.c
On 05/15/12 01:52, Tristan Gingold wrote: 2012-05-14 Tristan Gingold ging...@adacore.com * common/config/ia64/ia64-common.c (ia64_except_unwind_info): Fix typo. Ok. r~
Re: [Dwarf Patch] Improve pubnames and pubtypes generation. (issue 6197069)
On 05/21/2012 02:39 PM, Cary Coutant wrote:
The DW_AT_GNU_pubnames/pubtypes attributes serve two purposes: (1)
they let the linker know which CUs have pubnames pubtypes sections,
and (2) they let us know that the pubnames and pubtypes are reliable
(relative to the incomplete sections that GCC has generated in the
past). When building the .gdb_index, the linker starts with the top
compile_unit DIE of the CU. If there are pubnames/pubtypes attributes,
it can go read those sections; otherwise, it needs to parse the whole
DIE tree (slow) to extract the information needed for .gdb_index. It
would be possible to use the pointer from the pubnames section to the
CU, but that's a reverse pointer, and it would take time to set up the
links to figure out which pubnames/pubtypes sections go with which
CUs.
I would expect the linker to start by processing the pubnames/pubtypes
sections, and then if it wants to look through the CUs as well it
already knows which ones it can skip.
The presence or absence of an attribute seems like a fragile way to
determine whether or not particular debug info is sufficiently reliable.
If a consumer wants to make decisions based on changing behavior in
different versions of a producer, it could look at the DW_AT_producer
string.
I'm also puzzled by what the proposal says about .debug_types. Why would a
pubtypes entry point to a CU that doesn't actually have a definition of the
type? Is it so that the consumer knows which .dwo to look in? Perhaps we
could do something else with the sig8 instead of pointing to the wrong unit.
Yes, it's to find the .dwo. I never did anything to accomodate
.debug_types sections with .debug_pubtypes -- probably a more
substantial change to the format is in order, but I wanted to save
that for what I expect to be a complete revamping of the accelerated
lookup tables in DWARF.
Hmm. Looking at the code, it seems that the pubtypes entries for these
types will point to the main CU, but with the offset in the type_unit,
so a consumer trying to follow the pointer will find garbage. If you
want to have a pubtypes entry, it needs to point to a type skeleton to
avoid crashing standard-conforming consumers.
The DWARF standard says that pubnames is for names with global scope;
this change would add namespace-scope statics as well.
I think it was generally agreed that it's ultimately a
quality-of-implementation issue where the producer and consumer need
to agree on what's useful there.
OK.
Really? GDB wants to look up built-in types in the index rather than just
knowing what int means?
It doesn't actually always mean the same thing, though. It might be
less surprising if you ask that about some other base type like
double (and consider that GCC has options like -fshort-double).
Also, GDB can narrow the set of CUs that it has to read sometimes by
knowing which CUs actually contain a declaration of a given base type
(again, it's easier to see the advantage for double than for int).
OK.
Why bypass the DECL_NAMELESS check?
So objects within anonymous namespaces get pubnames:
namespace { void some_function...
Hmm, it would give the anonymous namespace itself a pubname, but I don't see
how it would make any difference to objects within the namespace.
The items within an anonymous namespace need to be indexed so that a
GDB user can just type some_function instead of (anonymous
namespace)::some_function.
Yes, I agree that we want to put some_function in pubnames. I still
don't see how putting the anonymous namespace itself in pubnames helps
at all. As far as pubnames is concerned, the anonymous namespace should
be transparent, and some_function should appear unqualified.
Jason
Re: [Dwarf Patch] Improve pubnames and pubtypes generation. (issue 6197069)
On Tue, May 22, 2012 at 10:19 AM, Jason Merrill ja...@redhat.com wrote: I'll let Cary handle the other questions. Yes, I agree that we want to put some_function in pubnames. I still don't see how putting the anonymous namespace itself in pubnames helps at all. As far as pubnames is concerned, the anonymous namespace should be transparent, and some_function should appear unqualified. Anonymous namespaces are funky in general, but gdb would like to be able to do things like tab completion on: (gdb) b '(anonymous namespace):: (gdb) b 'foo::(anonymous namespace):: Without being able to lookup anonymous namespaces, gdb has no idea where to continue from. This works today without pubnames because gdb goes and constructs a big list that includes anonymous namespaces by hand. Without anonymous namespaces in the index, gdb would either lose that functionality, or would have to parse the dwarf to find them. If it does that, then the index isn't very helpful.
Re: [C++ RFH / Patch] PR 53159
On 05/22/2012 07:02 PM, Jason Merrill wrote: On 05/22/2012 12:52 PM, Paolo Carlini wrote: Let's see if I understand in detail the structure of the new function (beyond all the simplifications possible because the target is a scalar, etc): we want something quite similar to implicit_conversion, but at the end of it, instead of using build_user_type_conversion_1 - which returns a conversion and implies that the caller passes it to convert_like_real - something like build_user_type_conversion (maybe exactly build_user_type_conversion) Do I understand correctly? We want to call convert_like_real on the conversion sequence in conv at the end of build_user_type_conversion_1: that is, the conversion sequence before the call to merge_conversion_sequences. Thanks, now I understand the general idea. Paolo.
Re: [PATCH, libatomic] Fix cut and paste errors in libat_test_and_set
On 05/19/12 14:25, John David Anglin wrote: 2012-05-19 John David Anglin dave.ang...@nrc-cnrc.gc.ca PR other/53231 * tas_n.c (libat_test_and_set): Correct return. Remove unused variable. Ok. r~
Re: [PATCH] rs6000.c missing target hook
On May 22, 2012, at 7:15 AM, David Edelsohn wrote:
* config/rs6000/rs6000.c (rs6000_aix_asm_output_dwarf_table_ref): Use
strip_name_encoding target hook.
Index: rs6000.c
===
--- rs6000.c(revision 187756)
+++ rs6000.c(working copy)
@@ -18392,7 +18392,7 @@
rs6000_aix_asm_output_dwarf_table_ref (char * frame_table_label)
{
fprintf (asm_out_file, \t.ref %s\n,
- TARGET_STRIP_NAME_ENCODING (frame_table_label));
+ (* targetm.strip_name_encoding) (frame_table_label));
Any reason to not remove (* and )?
Re: [PATCH 1/2] SH epilogue unwind, dwarf2 pass changes
On 05/18/12 03:48, Chung-Lin Tang wrote: Hi, I found a few changes were needed to the dwarf2 pass when trying to implement epilogue unwind for SH, mainly that the current handling of annulled-taken branches does not seem correct; the delay slot insn should be handled in a manner similar to an insn in the fallthru block. Cross-tested on SH and MIPS, and bootstrapped/tested on x86_64. Thanks, Chung-Lin 2012-05-18 Chung-Lin Tang clt...@codesourcery.com * dwarf2cfi.c (scan_trace): Update CFA before exiting scan. Handle annulled-taken branch (!INSN_FROM_TARGET_P) case. Do you have a test case for this? r~
Merge gcc-4_7-branch to gccgo branch
I've merged gcc-4_7-branch revision 187778 to the gccgo branch. Ian
Re: [PATCH 1/2] SH epilogue unwind, dwarf2 pass changes
On 05/18/12 03:48, Chung-Lin Tang wrote:
@@ -2401,6 +2401,7 @@ scan_trace (dw_trace_info *trace)
{
/* Propagate across fallthru edges. */
dwarf2out_flush_queued_reg_saves ();
+ def_cfa_1 (this_cfa);
maybe_record_trace_start (insn, NULL);
break;
}
@@ -2455,6 +2456,18 @@ scan_trace (dw_trace_info *trace)
cur_cfa = this_cfa;
continue;
}
+ else
+ {
+ /* If ELT is a annulled branch-taken instruction (i.e.
executed
+ only when branch is not taken), the args_size and CFA
should
+ not change through the jump. */
+ create_trace_edges (control);
+
+ /* Update and continue with the trace. */
+ add_cfi_insn = insn;
+ scan_insn_after (elt);
+ continue;
+ }
I think the def_cfa_1 is misplaced. It should be immediately before
that last continue. That mirrors the sort of flow you get via the
other paths through the loop.
r~
Re: [Dwarf Patch] Improve pubnames and pubtypes generation. (issue 6197069)
I would expect the linker to start by processing the pubnames/pubtypes sections, and then if it wants to look through the CUs as well it already knows which ones it can skip. The presence or absence of an attribute seems like a fragile way to determine whether or not particular debug info is sufficiently reliable. If a consumer wants to make decisions based on changing behavior in different versions of a producer, it could look at the DW_AT_producer string. In the context of Tom's original suggestion to add .gdb_index, I suggested fixing the problems with .debug_pubnames and using the producer string to distinguish. Tom had several objections to the use of the producer string for that: http://sourceware.org/ml/archer/2009-q4/msg00065.html I'm simply trying to do something that's fairly simple and that works. These attributes are GNU extensions, and I do not intend to propose these as part of the DWARF 5 standard -- they're stopgap measures until we have a better plan for accelerated lookup tables. Starting with the pubnames sections then going back and reading the debug_info sections would require an extra table and a lookup for each CU to see if it's already been covered by a pubnames section. Building the .gdb_index in gold is still too slow, but these attributes help make it a bit faster than it was without them. Are these two attributes a showstopper issue for you? Hmm. Looking at the code, it seems that the pubtypes entries for these types will point to the main CU, but with the offset in the type_unit, so a consumer trying to follow the pointer will find garbage. If you want to have a pubtypes entry, it needs to point to a type skeleton to avoid crashing standard-conforming consumers. Yes, I understand that's broken, but there are no consumers at this point that make any use of that offset. Would it be acceptable if we just put 0 there? (Given that I expect .debug_pub* to go away soon, I don't think it's worth the trouble of filling in the offset with anything more meaningful.) -cary
Re: [C++ Patch] PR 44516
On May 22, 2012, at 8:42 AM, Jason Merrill wrote: On 05/22/2012 08:22 AM, Paolo Carlini wrote: Ok, thanks. Today I wanted to concretely test and post a patch but I noticed this other objc bit in objc-next-runtime-abi-01.c which I don't know how to handle due to the OPT_Wall gate: warning_at (UNKNOWN_LOCATION, OPT_Wall, %-fobjc-sjlj-exceptions% is the only supported exceptions system for %-fnext-runtime% with %-fobjc-abi-version% 2); If the objc maintainers could help with those uses of warning_at (0 in the objc tree, I have a patch ready for the asserts proper. What if we just changed UNKNOWN_LOCATION to BUILTINS_LOCATION for these diagnostics? I thought about that, but only after wanting to use COMMANDLINE_LOCATION, looking around and not finding it If everyone else is using BUILTINS_LOCATION, that's fine, but just seems wrong. UNKNOWN almost seems like a better match at first blush.
Re: [C++ Patch] PR 44516
On 05/22/2012 08:03 PM, Mike Stump wrote: On May 22, 2012, at 8:42 AM, Jason Merrill wrote: On 05/22/2012 08:22 AM, Paolo Carlini wrote: Ok, thanks. Today I wanted to concretely test and post a patch but I noticed this other objc bit in objc-next-runtime-abi-01.c which I don't know how to handle due to the OPT_Wall gate: warning_at (UNKNOWN_LOCATION, OPT_Wall, %-fobjc-sjlj-exceptions% is the only supported exceptions system for %-fnext-runtime% with %-fobjc-abi-version%2); If the objc maintainers could help with those uses of warning_at (0 in the objc tree, I have a patch ready for the asserts proper. What if we just changed UNKNOWN_LOCATION to BUILTINS_LOCATION for these diagnostics? I thought about that, but only after wanting to use COMMANDLINE_LOCATION, looking around and not finding it If everyone else is using BUILTINS_LOCATION, that's fine, but just seems wrong. UNKNOWN almost seems like a better match at first blush. BUILTINS_LOCATION sounds a bit weird to me too. I guess, if we do that we need clear comments. And in any case, the issue, as stupid as it seems in principle, turns out to be quite tricky, because the driver really expects UNKNOWN_LOCATION in quite a few places, there are even asserts. Just doing searchreplace for the UNKNOWN_LOCATIONs passed by warning_at and error_at doesn't work. Paolo.
Re: [Dwarf Patch] Improve pubnames and pubtypes generation. (issue 6197069)
On 05/22/2012 01:27 PM, Sterling Augustine wrote: Anonymous namespaces are funky in general, but gdb would like to be able to do things like tab completion on: (gdb) b '(anonymous namespace):: (gdb) b 'foo::(anonymous namespace):: Without being able to lookup anonymous namespaces, gdb has no idea where to continue from. Ah. I guess that makes sense. On 05/22/2012 01:49 PM, Cary Coutant wrote: In the context of Tom's original suggestion to add .gdb_index, I suggested fixing the problems with .debug_pubnames and using the producer string to distinguish. Tom had several objections to the use of the producer string for that: http://sourceware.org/ml/archer/2009-q4/msg00065.html Sure, his point about backporting patches is a strong argument. I'm simply trying to do something that's fairly simple and that works. These attributes are GNU extensions, and I do not intend to propose these as part of the DWARF 5 standard -- they're stopgap measures until we have a better plan for accelerated lookup tables. Starting with the pubnames sections then going back and reading the debug_info sections would require an extra table and a lookup for each CU to see if it's already been covered by a pubnames section. Yes, but I would expect this table lookup to be faster than going to the disk to read the CU DIE and abbrev in order to check for the attribute. OTOH, I suppose you need to read it anyway if you want to check somehow whether you should trust the pub* information. Building the .gdb_index in gold is still too slow, but these attributes help make it a bit faster than it was without them. Are these two attributes a showstopper issue for you? No, I'm just reluctant to add more relocations. Could we make them just flags? Hmm. Looking at the code, it seems that the pubtypes entries for these types will point to the main CU, but with the offset in the type_unit, so a consumer trying to follow the pointer will find garbage. If you want to have a pubtypes entry, it needs to point to a type skeleton to avoid crashing standard-conforming consumers. Yes, I understand that's broken, but there are no consumers at this point that make any use of that offset. Would it be acceptable if we just put 0 there? (Given that I expect .debug_pub* to go away soon, I don't think it's worth the trouble of filling in the offset with anything more meaningful.) I wouldn't expect it to be much harder to put the skeleton there than plain 0. Jason
Re: [Patch, libgfortran] PR53445/53444 - Fix compilation on VxWorks
On Tue, May 22, 2012 at 5:57 PM, Tobias Burnus bur...@net-b.de wrote: On 05/22/2012 03:06 PM, Tobias Burnus wrote: The attached patches fix compilation issues on VxWorks. a) VxWorks has strerror_r but contrary to POSIX, the function in VxWorks (at least in older versions) takes only two arguments: errnum and buf and not also the buffer length. I added a configure check for that variant. I forgot to attach that patch. Now with patch and automake 1.11.1 for the generated files. Tobias For the a) patch (strerror_r): The configure.ac diff occurs twice in the patch, and the patch file has DOS line endings. Also, based on some googling the vxworks 2-arg strerror_r returns OK or ERROR (an enum, I presume). So the trick with builtin_choose_expr is both wrong and unnecessary. Thus I'd replace +#elif defined(HAVE_STRERROR_R_2ARGS) + return +__builtin_choose_expr (__builtin_classify_type (strerror_r (0, buf)) + == 5, + /* char*-returning strerror_r() */ + strerror_r (errnum, buf), + /* int-returning strerror_r () */ + (strerror_r (errnum, buf), buf)); with #elif defined(HAVE_STRERROR_R_2ARGS) if (strerror_r (errnum, buf) == OK) return buf; return NULL; Otherwise Ok. For the b) patch (sticky bit): Also DOS line endings. If the patch is verbatim by the OP it would perhaps be nice to include his name in the ChangeLog as well (it's small enough that AFAICS it goes below the trivial limit, thus no requirement for copyright assignment etc.). Otherwise Ok. Thanks for the patches! -- Janne Blomqvist
Re: [Patch, libgfortran] PR53445/53444 - Fix compilation on VxWorks
On Tue, May 22, 2012 at 10:33 PM, Janne Blomqvist blomqvist.ja...@gmail.com wrote: On Tue, May 22, 2012 at 5:57 PM, Tobias Burnus bur...@net-b.de wrote: On 05/22/2012 03:06 PM, Tobias Burnus wrote: The attached patches fix compilation issues on VxWorks. a) VxWorks has strerror_r but contrary to POSIX, the function in VxWorks (at least in older versions) takes only two arguments: errnum and buf and not also the buffer length. I added a configure check for that variant. I forgot to attach that patch. Now with patch and automake 1.11.1 for the generated files. Tobias For the a) patch (strerror_r): The configure.ac diff occurs twice in the patch, and the patch file has DOS line endings. Also, based on some googling the vxworks 2-arg strerror_r returns OK or ERROR (an enum, I presume). So the trick with builtin_choose_expr is both wrong and unnecessary. Thus I'd replace +#elif defined(HAVE_STRERROR_R_2ARGS) + return + __builtin_choose_expr (__builtin_classify_type (strerror_r (0, buf)) + == 5, + /* char*-returning strerror_r() */ + strerror_r (errnum, buf), + /* int-returning strerror_r () */ + (strerror_r (errnum, buf), buf)); with #elif defined(HAVE_STRERROR_R_2ARGS) if (strerror_r (errnum, buf) == OK) return buf; return NULL; Googling some more, it seems the vxworks STATUS is just a typedef for int, so I guess the original patch works. Also the error checking is not useful here, so we could do just #elif defined(HAVE_STRERROR_R_2ARGS) strerror_r (errnum, buf); return buf; -- Janne Blomqvist
Re: [DF] Generate REFs in REGNO order
Il 22/05/2012 18:26, Dimitrios Apostolou ha scritto: You are right, and I noticed that if we reverse (actually put straight) the loop for the PARALLEL defs inside df_defs_record() then the speedup stands for both x86 and ppc64. The following patch was tested on x86, do you think it is meaningful for the generic case? It's really a lottery, but if it doesn't make things worse for x86 why not. Paolo --- gcc/df-scan.c 2012-05-06 04:08:43 + +++ gcc/df-scan.c 2012-05-22 13:08:33 + @@ -3010,7 +3010,7 @@ df_defs_record (struct df_collection_rec break; case PARALLEL: - for (i = XVECLEN (x, 0) - 1; i = 0; i--) + for (i = 0; i XVECLEN (x, 0); i++) df_defs_record (collection_rec, XVECEXP (x, 0, i), bb, insn_info, flags); break;
Re: [Dwarf Patch] Improve pubnames and pubtypes generation. (issue 6197069)
Yes, but I would expect this table lookup to be faster than going to the disk to read the CU DIE and abbrev in order to check for the attribute. OTOH, I suppose you need to read it anyway if you want to check somehow whether you should trust the pub* information. Right. I also need to read the top-level DIE to get the range list for the CU. Building the .gdb_index in gold is still too slow, but these attributes help make it a bit faster than it was without them. Are these two attributes a showstopper issue for you? No, I'm just reluctant to add more relocations. Could we make them just flags? That might be workable. Let me take a look at the gold changes I'd need to make for that. They don't have to be relocations, though -- since they're only used by the linker, a raw section-relative offset would be sufficient. Yes, I understand that's broken, but there are no consumers at this point that make any use of that offset. Would it be acceptable if we just put 0 there? (Given that I expect .debug_pub* to go away soon, I don't think it's worth the trouble of filling in the offset with anything more meaningful.) I wouldn't expect it to be much harder to put the skeleton there than plain 0. I was concerned that we might not always have a skeleton type to point to, but I confess I haven't looked closely enough to know whether that's a valid concern. -cary
Re: [Patch, libgfortran] PR53445/53444 - Fix compilation on VxWorks
On 05/22/2012 03:42 PM, Janne Blomqvist wrote: On Tue, May 22, 2012 at 10:33 PM, Janne Blomqvist blomqvist.ja...@gmail.com wrote: On Tue, May 22, 2012 at 5:57 PM, Tobias Burnusbur...@net-b.de wrote: On 05/22/2012 03:06 PM, Tobias Burnus wrote: The attached patches fix compilation issues on VxWorks. a) VxWorks has strerror_r but contrary to POSIX, the function in VxWorks (at least in older versions) takes only two arguments: errnum and buf and not also the buffer length. I added a configure check for that variant. I forgot to attach that patch. Now with patch and automake 1.11.1 for the generated files. Tobias For the a) patch (strerror_r): The configure.ac diff occurs twice in the patch, and the patch file has DOS line endings. Also, based on some googling the vxworks 2-arg strerror_r returns OK or ERROR (an enum, I presume). So the trick with builtin_choose_expr is both wrong and unnecessary. Thus I'd replace +#elif defined(HAVE_STRERROR_R_2ARGS) + return +__builtin_choose_expr (__builtin_classify_type (strerror_r (0, buf)) + == 5, + /* char*-returning strerror_r() */ + strerror_r (errnum, buf), + /* int-returning strerror_r () */ + (strerror_r (errnum, buf), buf)); with #elif defined(HAVE_STRERROR_R_2ARGS) if (strerror_r (errnum, buf) == OK) return buf; return NULL; Googling some more, it seems the vxworks STATUS is just a typedef for int, so I guess the original patch works. Also the error checking is not useful here, so we could do just #elif defined(HAVE_STRERROR_R_2ARGS) strerror_r (errnum, buf); return buf; All the proposed solutions work for me. Robert Mason
Re: [DF] Generate REFs in REGNO order
On Tue, 22 May 2012, Paolo Bonzini wrote: Il 22/05/2012 18:26, Dimitrios Apostolou ha scritto: You are right, and I noticed that if we reverse (actually put straight) the loop for the PARALLEL defs inside df_defs_record() then the speedup stands for both x86 and ppc64. The following patch was tested on x86, do you think it is meaningful for the generic case? It's really a lottery, but if it doesn't make things worse for x86 why not. It doesn't so please apply, even though I'll try understanding in depth and see if I can think of a proper way of generating DEFs in proper order. Dimitris
Re: [PATCH] Try to expand COND_EXPR using addcc
On 05/16/2012 12:48 PM, Andrew Pinski wrote:
+ /* Handle 0/1 specially because boolean types and precision of one types,
+ will cause the diff to always be 1. Note this really should have
+ simplified before reaching here. */
+ /* A ? 1 : 0 is just (type)(A!=0). */
+ if (integer_zerop (treeop2) integer_onep (treeop1))
+{
+ tree t = fold_convert (TREE_TYPE (treeop0), integer_zero_node);
+ tree tmp = fold_build2 (NE_EXPR, TREE_TYPE (treeop0), treeop0, t);
+ tmp = fold_convert (type, tmp);
+ return expand_normal (tmp);
+}
+
+ /* A ? 0 : 1 is just (type)(A==0). */
+ if (integer_zerop (treeop1) integer_onep (treeop0))
+{
+ tree t = fold_convert (TREE_TYPE (treeop0), integer_zero_node);
+ tree tmp = fold_build2 (EQ_EXPR, TREE_TYPE (treeop0), treeop0, t);
+ tmp = fold_convert (type, tmp);
+ return expand_normal (tmp);
+}
Well, why *don't* you handle them before here? I completely agree that
they seem out of place in an _addcc function.
+ /* A ? CST1 : CST2 can be expanded as CST2 + (!A)*(CST1 - CST2) */
+ if (TREE_CODE (treeop1) == INTEGER_CST
+ TREE_CODE (treeop2) == INTEGER_CST)
+diff = int_const_binop (MINUS_EXPR, treeop1, treeop2);
Here you're bypassing the large amount of logic we've got in
e.g. ix86_expand_int_movcc for handling exactly this case, and
in more clever ways than you're doing here.
Please continue to defer to cmov for this.
+ /* A ? b : b+c can be expanded as b + (!A)*(c) */
+ /* A ? b + c : b can be expanded as b + (!A)*(c) */
What has MULT got to do with it? I think these comments are just confusing.
r~
[Patch] Add AC_ARG_ENABLE for libstdc++-v3
This patch adds an AC_ARG_ENABLE option to build/not build
libstdc++-v3. I wrote the patch in order to allow the user to override
the automatic disable for libstdc++-v3 for certain targets.
Patch is against configure.ac in trunk.
Robert Mason
From 69c085f3742e94501f4a202d1321e143afe9a115 Mon Sep 17 00:00:00 2001
From: rbmj r...@verizon.net
Date: Tue, 22 May 2012 16:32:02 -0400
Subject: [PATCH] Added --enable-libstdc++-v3 option.
---
configure.ac | 38 +-
1 files changed, 25 insertions(+), 13 deletions(-)
diff --git a/configure.ac b/configure.ac
index 378e9f5..c06b400 100644
--- a/configure.ac
+++ b/configure.ac
@@ -425,6 +425,15 @@ AC_ARG_ENABLE(libssp,
ENABLE_LIBSSP=$enableval,
ENABLE_LIBSSP=yes)
+AC_ARG_ENABLE(libstdc++-v3,
+AS_HELP_STRING([--disable-libstdc++-v3],
+ [do not build libstdc++-v3 directory]),
+ENABLE_LIBSTDCXX=$enableval,
+ENABLE_LIBSTDCXX=default)
+if test ${ENABLE_LIBSTDCXX} = no ; then
+ noconfigdirs=$noconfigdirs libstdc++-v3
+fi
+
# Save it here so that, even in case of --enable-libgcj, if the Java
# front-end isn't enabled, we still get libgcj disabled.
libgcj_saved=$libgcj
@@ -544,19 +553,22 @@ case ${target} in
esac
# Disable libstdc++-v3 for some systems.
-case ${target} in
- *-*-vxworks*)
-# VxWorks uses the Dinkumware C++ library.
-noconfigdirs=$noconfigdirs target-libstdc++-v3
-;;
- arm*-wince-pe*)
-# the C++ libraries don't build on top of CE's C libraries
-noconfigdirs=$noconfigdirs target-libstdc++-v3
-;;
- avr-*-*)
-noconfigdirs=$noconfigdirs target-libstdc++-v3
-;;
-esac
+# Allow user to override this if they pass --enable-libstdc++-v3
+if test ${ENABLE_LIBSTDCXX} = default ; then
+ case ${target} in
+*-*-vxworks*)
+ # VxWorks uses the Dinkumware C++ library.
+ noconfigdirs=$noconfigdirs target-libstdc++-v3
+ ;;
+arm*-wince-pe*)
+ # the C++ libraries don't build on top of CE's C libraries
+ noconfigdirs=$noconfigdirs target-libstdc++-v3
+ ;;
+avr-*-*)
+ noconfigdirs=$noconfigdirs target-libstdc++-v3
+ ;;
+ esac
+fi
# Disable Fortran for some systems.
case ${target} in
--
1.7.5.4
Re: [google/gcc-4_6] More Fission updates (issue6219049)
On 12-05-18 17:38 , Cary Coutant wrote: @@ -5820,6 +5825,14 @@ const struct gcc_debug_hooks dwarf2_debu representation is done after the entire program has been compiled. The types below are used to describe the internal representation. */ +/* Whether to generate the DWARF accelerator tables in .debug_pubnames + and .debug_pubtypes. This is configured per-target, but can be + overridden by the -gpubnames or -gno-pubnames options. */ + +#define want_pubnames (debug_generate_pub_sections != -1 \ + ? debug_generate_pub_sections \ + : targetm.want_debug_pub_sections) + Minor nit. Make it a static inline function? OK otherwise. Diego.
Re: [C++ Patch] PR 44516
Hi again, On 05/22/2012 06:42 PM, Jason Merrill wrote: On 05/22/2012 11:55 AM, Paolo Carlini wrote: FAIL: gcc.dg/opts-1.c (internal compiler error) FAIL: gcc.dg/opts-1.c -fno-abi-version (test for errors, line ) FAIL: gcc.dg/opts-1.c -fno-lto-compression-level (test for errors, line ) FAIL: gcc.dg/opts-1.c -fno-tree-parallelize-loops (test for errors, line ) FAIL: gcc.dg/opts-1.c -Wno-strict-overflow (test for errors, line ) FAIL: gcc.dg/opts-1.c -Wno-strict-aliasing (test for errors, line ) FAIL: gcc.dg/opts-1.c (test for excess errors) FAIL: gcc.dg/opts-5.c (internal compiler error) FAIL: gcc.dg/opts-5.c (test for errors, line ) FAIL: gcc.dg/opts-5.c (test for excess errors) FAIL: gcc.dg/opts-6.c (internal compiler error) FAIL: gcc.dg/opts-6.c (test for errors, line ) FAIL: gcc.dg/opts-6.c (test for excess errors) That all looks like diagnostics about options that don't work properly together, much like the objc issues. I think it makes sense to switch all such diagnostics over to BUILTINS_LOCATION, because we do know the location of the thing we're complaining about: it's on the command line. Uhhm, I have an out of the blue idea, so please excuse me if for some obvious reason doesn't make sense: don't we have a global variable saying where we are in the compiler pipeline? If we have one, it would seem to me pretty neat to assert that *a front end* cannot pass UNKNOWN_LOCATION to warning_at, error_at and the like. I think it would work also for objc, because, AFAICS, the warning_at are in the runtime (and in that specific case we know how to handle the problem anyway) Paolo.
Re: [google/gcc-4_6] More Fission updates (issue6219049)
On 12-05-21 20:15 , Cary Coutant wrote: This patch is for the google/gcc-4_6 branch. Fission improvements and bug fixes. Adds new DW_OP_GNU_const_index to handle TLS offsets in debug info. Adds -gpubnames/-gno-pubnames options to explicitly request .debug_pubnames/pubtypes sections. Adds style parameter to C/C++ pretty-printer so that we can get canonical pubnames without affecting diagnostic output. Bootstrapped and tested on x86_64. 2012-05-18 Sterling Augustinesaugust...@google.com Cary Coutantccout...@google.com include/ * dwarf2.h: Add DW_OP_GNU_const_index. gcc/ * opts.c (finish_options): -gfission implies -gpubnames. (common_handle_option): Pass empty arg string to set_debug_level. * common.opt (gno-fission): New option. (gfission): Remove JoinedOrMissing, add RejectNegative. (gno-pubnames, gpubnames): New options. * target.def (want_debug_pub_sections): Change default to false. * gcc.c (check_live_switch): Check -g options for -gno- options. * c-family/c-pretty-print.c (pp_c_specifier_qualifier_list): Add support for gnu_v3 style. * c-family/c-pretty-print.h (pp_c_flag_gnu_v3): New enum constant. * cp/error.c (dump_decl): Add support for gnu_v3 style. (decl_as_string): Likewise. (lang_decl_name): Likewise. * cp/cp-lang.c (cxx_dwarf_name): Likewise. * cp/cp-tree.h (enum overload_flags): Add TFF_MATCH_GNU_V3_DEMANGLER. * dwarf2out.c (dwarf_stack_op_name): Add DW_OP_GNU_const_index. (size_of_loc_descr): Likewise. (output_loc_operands): Likewise. (output_loc_operands_raw): Likewise. (want_pubnames): New macro. (dw_addr_op): New function. (new_addr_loc_descr): Call dw_addr_op. (add_AT_pubnames): Add DW_AT_GNU_pubnames/pubtypes only if generating .debug_pubnames/pubtypes sections. (add_pubname_string): Check for -gpubnames option. (add_pubname): Likewise. (add_pubtype): Likewise. (output_pubnames): Likewise. (mem_loc_descriptor): Call new_addr_loc_desc for TLS vars. (loc_list_from_tree): Likewise. (output_addr_table): Handle DW_OP_GNU_const_index. Add missing newline. (hash_loc_operands): Add DW_OP_GNU_const_index. (compare_loc_operands): Likewise. * testsuite/g++.old-deja/g++.pt/memtemp77.C: Revert earlier change to expected results. * testsuite/g++.dg/ext/pretty3.C: Likewise. * testsuite/g++.dg/warn/Wuninitializable-member.C: Likewise. * testsuite/g++.dg/warn/pr35711.C: Likewise. * testsuite/g++.dg/pr44486.C: Likewise. OK. Diego.
libgo patch committed: Tweak runtime.Callers for Go 1 compatibility
In the Go 1 release of the other Go compiler, runtime.Callers had what I
would describe as an off-by-one error. We can't change it now, because
that would break Go 1 backward compatibility. This patch changes libgo
to have the same off-by-one error. Bootstrapped and ran Go testsuite on
x86_64-unknown-linux-gnu. Committed to mainline and 4.7 branch.
Ian
diff -r d99f020243e7 libgo/runtime/go-callers.c
--- a/libgo/runtime/go-callers.c Tue May 22 09:54:14 2012 -0700
+++ b/libgo/runtime/go-callers.c Tue May 22 14:50:06 2012 -0700
@@ -72,5 +72,8 @@
int
Callers (int skip, struct __go_open_array pc)
{
- return runtime_callers (skip, (uintptr *) pc.__values, pc.__count);
+ /* In the Go 1 release runtime.Callers has an off-by-one error,
+ which we can not correct because it would break backward
+ compatibility. Adjust SKIP here to be compatible. */
+ return runtime_callers (skip - 1, (uintptr *) pc.__values, pc.__count);
}
Re: [Dwarf Patch] Improve pubnames and pubtypes generation. (issue 6197069)
On Tue, May 22, 2012 at 01:04:15PM -0700, Cary Coutant wrote: That might be workable. Let me take a look at the gold changes I'd need to make for that. They don't have to be relocations, though -- since they're only used by the linker, a raw section-relative offset would be sufficient. But if they are just flags, they can be DW_FORM_flag_present and waste space just in .debug_abbrev, not in .debug_info. Jakub
Re: [google/gcc-4_6] More Fission updates (issue6219049)
+/* Whether to generate the DWARF accelerator tables in .debug_pubnames + and .debug_pubtypes. This is configured per-target, but can be + overridden by the -gpubnames or -gno-pubnames options. */ + +#define want_pubnames (debug_generate_pub_sections != -1 \ + ? debug_generate_pub_sections \ + : targetm.want_debug_pub_sections) + Minor nit. Make it a static inline function? OK otherwise. Done. I've updated the patch with this, and slipped in one more bug fix to output pubnames for all inline functions. Tests are running now. I'll send the updated patch shortly. Thanks, -cary
Re: [PATCH] rs6000.c missing target hook
On Tue, May 22, 2012 at 1:31 PM, Mike Stump mikest...@comcast.net wrote: fprintf (asm_out_file, \t.ref %s\n, - TARGET_STRIP_NAME_ENCODING (frame_table_label)); + (* targetm.strip_name_encoding) (frame_table_label)); Any reason to not remove (* and )? I wanted to be consistent with the rest of the file, which uses that style. - David
[C++ Patch] PR 29185
Hi,
some years ago Martin lamented that we weren't consistently warning
about deleting member arrays vs arrays. A fix seems simple and passes
bootstrap and testing on x86_64-linux. Note I have to change D to E
because dump_decl cannot cope with COMPONENT_REFs.
Thanks,
Paolo.
//
/cp
2012-05-23 Paolo Carlini paolo.carl...@oracle.com
PR c++/29185
* decl2.c (delete_sanity): Extend 'deleting array' warning to
member arrays.
/testsuite
2012-05-23 Paolo Carlini paolo.carl...@oracle.com
PR c++/29185
* g++.dg/warn/delete-array-1.C: New.
Index: testsuite/g++.dg/warn/delete-array-1.C
===
--- testsuite/g++.dg/warn/delete-array-1.C (revision 0)
+++ testsuite/g++.dg/warn/delete-array-1.C (revision 0)
@@ -0,0 +1,11 @@
+// PR c++/29185
+
+int a [1];
+struct S { int a [1]; } s;
+
+void foo (S *p)
+{
+ delete a;// { dg-warning deleting array }
+ delete s.a; // { dg-warning deleting array }
+ delete p-a; // { dg-warning deleting array }
+}
Index: cp/decl2.c
===
--- cp/decl2.c (revision 187780)
+++ cp/decl2.c (working copy)
@@ -438,9 +438,10 @@ delete_sanity (tree exp, tree size, bool doing_vec
}
/* An array can't have been allocated by new, so complain. */
- if (TREE_CODE (exp) == VAR_DECL
+ if ((TREE_CODE (exp) == VAR_DECL
+ || TREE_CODE (exp) == COMPONENT_REF)
TREE_CODE (TREE_TYPE (exp)) == ARRAY_TYPE)
-warning (0, deleting array %q#D, exp);
+warning (0, deleting array %q#E, exp);
t = build_expr_type_conversion (WANT_POINTER, exp, true);
[google/gcc-4_6] More Fission updates (revised) (issue6219049)
[Revised to address review comments and to fix a bug we found late:
We've changed want_pubnames to a static inline function, and changed
the pubnames output to include (ironically enough) inline functions.]
This patch is for the google/gcc-4_6 branch.
Fission improvements and bug fixes. Adds new DW_OP_GNU_const_index to
handle TLS offsets in debug info. Adds -gpubnames/-gno-pubnames options
to explicitly request .debug_pubnames/pubtypes sections. Adds style
parameter to C/C++ pretty-printer so that we can get canonical pubnames
without affecting diagnostic output.
Bootstrapped and tested on x86_64.
2012-05-21 Sterling Augustine saugust...@google.com
Cary Coutant ccout...@google.com
include/
* dwarf2.h: Add DW_OP_GNU_const_index.
gcc/
* opts.c (finish_options): -gfission implies -gpubnames.
(common_handle_option): Pass empty arg string to set_debug_level.
* common.opt (gno-fission): New option.
(gfission): Remove JoinedOrMissing, add RejectNegative.
(gno-pubnames, gpubnames): New options.
* target.def (want_debug_pub_sections): Change default to false.
* gcc.c (check_live_switch): Check -g options for -gno- options.
* c-family/c-pretty-print.c (pp_c_specifier_qualifier_list): Add
support for gnu_v3 style.
* c-family/c-pretty-print.h (pp_c_flag_gnu_v3): New enum constant.
* cp/error.c (dump_decl): Add support for gnu_v3 style.
(decl_as_string): Likewise.
(lang_decl_name): Likewise.
* cp/cp-lang.c (cxx_dwarf_name): Likewise.
* cp/cp-tree.h (enum overload_flags): Add TFF_MATCH_GNU_V3_DEMANGLER.
* dwarf2out.c (dwarf_stack_op_name): Add DW_OP_GNU_const_index.
(size_of_loc_descr): Likewise.
(output_loc_operands): Likewise.
(output_loc_operands_raw): Likewise.
(dw_addr_op): New function.
(new_addr_loc_descr): Call dw_addr_op.
(want_pubnames): New function.
(add_AT_pubnames): Add DW_AT_GNU_pubnames/pubtypes only if
generating .debug_pubnames/pubtypes sections.
(add_pubname_string): Check for -gpubnames option.
(add_pubname): Likewise.
(add_pubtype): Likewise.
(output_pubnames): Likewise.
(mem_loc_descriptor): Call new_addr_loc_desc for TLS vars.
(loc_list_from_tree): Likewise.
(gen_subprogram_die): Output pubnames for all inlined functions.
(output_addr_table): Handle DW_OP_GNU_const_index. Add missing
newline.
(hash_loc_operands): Add DW_OP_GNU_const_index.
(compare_loc_operands): Likewise.
* testsuite/g++.old-deja/g++.pt/memtemp77.C: Revert earlier change
to expected results.
* testsuite/g++.dg/ext/pretty3.C: Likewise.
* testsuite/g++.dg/warn/Wuninitializable-member.C: Likewise.
* testsuite/g++.dg/warn/pr35711.C: Likewise.
* testsuite/g++.dg/pr44486.C: Likewise.
Index: include/dwarf2.h
===
--- include/dwarf2.h(revision 187751)
+++ include/dwarf2.h(working copy)
@@ -546,8 +546,9 @@ enum dwarf_location_atom
DW_OP_GNU_uninit = 0xf0,
DW_OP_GNU_encoded_addr = 0xf1,
DW_OP_GNU_implicit_pointer = 0xf2,
-/* Extension for Fission. See http://gcc.gnu.org/wiki/DebugFission. */
+/* Extensions for Fission. See http://gcc.gnu.org/wiki/DebugFission. */
DW_OP_GNU_addr_index = 0xfb,
+DW_OP_GNU_const_index = 0xfc,
/* HP extensions. */
DW_OP_HP_unknown = 0xe0, /* Ouch, the same as GNU_push_tls_address. */
DW_OP_HP_is_value= 0xe1,
Index: gcc/c-family/c-pretty-print.c
===
--- gcc/c-family/c-pretty-print.c (revision 187751)
+++ gcc/c-family/c-pretty-print.c (working copy)
@@ -445,6 +445,9 @@ pp_c_specifier_qualifier_list (c_pretty_
{
const enum tree_code code = TREE_CODE (t);
+ if (!(pp-flags pp_c_flag_gnu_v3) TREE_CODE (t) != POINTER_TYPE)
+pp_c_type_qualifier_list (pp, t);
+
switch (code)
{
case REFERENCE_TYPE:
@@ -489,7 +492,7 @@ pp_c_specifier_qualifier_list (c_pretty_
pp_simple_type_specifier (pp, t);
break;
}
- if (TREE_CODE (t) != POINTER_TYPE)
+ if ((pp-flags pp_c_flag_gnu_v3) TREE_CODE (t) != POINTER_TYPE)
pp_c_type_qualifier_list (pp, t);
}
Index: gcc/c-family/c-pretty-print.h
===
--- gcc/c-family/c-pretty-print.h (revision 187751)
+++ gcc/c-family/c-pretty-print.h (working copy)
@@ -30,7 +30,8 @@ along with GCC; see the file COPYING3.
typedef enum
{
pp_c_flag_abstract = 1 1,
- pp_c_flag_last_bit = 2
+ pp_c_flag_last_bit = 2,
+ pp_c_flag_gnu_v3 = 4
} pp_c_pretty_print_flags;
Index: gcc/target.def
===
--- gcc/target.def
Re: [C++ Patch] PR 29185
On Tue, May 22, 2012 at 8:25 PM, Paolo Carlini paolo.carl...@oracle.com wrote: Hi, some years ago Martin lamented that we weren't consistently warning about deleting member arrays vs arrays. A fix seems simple and passes bootstrap and testing on x86_64-linux. Note I have to change D to E because dump_decl cannot cope with COMPONENT_REFs. dump_decl can't handle COMPONENT_REF because it is an expression. I suspect you want to look at the operand 1 instead of changing D to E.
Re: [PATCH, 4.6] Fix PR53170: missing target c++11 selector
On 21/05/12 21:14, Paolo Carlini wrote:
On 05/21/2012 01:45 AM, Michael Hope wrote:
The testsuite for PR52796 uses the 'target c++11' selector which doesn't exist
in 4.6.
This patch backports the selector, clearing the 'ERROR:
g++.dg/cpp0x/variadic-value1.C:
syntax error in target selector target c++11 for dg-do 2 run { target c++11 }
'
errors which have appeared in recent 4.6 builds.
Tested on x86_64-linux-gnu with no regressions. Changes the ERROR to
UNSUPPORTED.
OK for 4.6?
To be honest, when I saw the issue I thought we wanted simply to not use the target
selector at all in the branch and simply add a // { dg-options -std=c++11 } I
thought somebody would commit the change as obvious ;)
Sure. The version below changes the selector for explicit flags that match
those passed by trunk.
OK for 4.6?
-- Michael (who's not keen enough to commit as obvious)
2012-05-23 Michael Hope michael.h...@linaro.org
PR PR52796
* g++.dg/cpp0x/variadic-value1.C: Change selector for explicit
options.
diff --git a/gcc/testsuite/g++.dg/cpp0x/variadic-value1.C
b/gcc/testsuite/g++.dg/cpp0x/variadic-value1.C
index 179919a..301bd54 100644
--- a/gcc/testsuite/g++.dg/cpp0x/variadic-value1.C
+++ b/gcc/testsuite/g++.dg/cpp0x/variadic-value1.C
@@ -1,5 +1,5 @@
// PR c++/52796
-// { dg-do run { target c++11 } }
+// { dg-options -std=c++0x -pedantic-errors }
inline void *operator new(__SIZE_TYPE__ s, void *p) { return p; }
Re: [C++ Patch] PR 44516
On 05/22/2012 05:18 PM, Paolo Carlini wrote: Uhhm, I have an out of the blue idea, so please excuse me if for some obvious reason doesn't make sense: don't we have a global variable saying where we are in the compiler pipeline? I'm not sure, but the question led me to thinking about only asserting if input_location != UNKNOWN_LOCATION. Would that work? Jason
Re: [C++ Patch] PR 29185
On 05/22/2012 09:25 PM, Paolo Carlini wrote: some years ago Martin lamented that we weren't consistently warning about deleting member arrays vs arrays. I wonder why we look at the code of exp at all. Surely deleting any expression with array type is questionable. Jason
ping*2: Fix gcc.dg/lower-subreg-1.c failure (was: [C Patch]: pr52543)
From: Hans-Peter Nilsson h...@axis.com Date: Wed, 16 May 2012 08:24:41 +0200 From: Hans-Peter Nilsson h...@axis.com Date: Wed, 9 May 2012 08:02:25 +0200 Ping. I missed the PR number decoration on the ChangeLog entry: PR rtl-optimization/53176 * rtlanal.c (rtx_cost): Adjust default cost for X with a UNITS_PER_WORD factor for all X according to the size of its mode, not just for SUBREGs with untieable modes. http://gcc.gnu.org/ml/gcc-patches/2012-05/msg00609.html Ping again... brgds, H-P
