[PATCH] Fix PR61306: improve handling of sign and cast in bswap
When bswap replace a bitwise expression involving a memory source by a load
possibly followed by a bswap, it is possible that the load has a size smaller
than that of the target expression where the bitwise expression was affected.
So some sort of cast is needed. But there might also be a difference between
the size of the expression that was affected and the size of the load. So 3
different sizes might be involved. Consider the following example from binutils:
bfd_vma
bfd_getl16 (const void *p)
{
const bfd_byte *addr = (*const bfd_byte *) p;
return (addr[1] 8) | addr[0];
}
Here the load will have a size of 16 bits, while the bitwise expression is an
int (don't ask me why) but is returned as a 64 bits quantity (bfd_vma maps to
the size of host registers). In this case we need 2 separate cast. One from 16
bit quantity to int with zero extension as the high bits are 0. It is always a
zero extension because bswap will not do anything in the presence of a sign
extension as depending on the initial value the result would be different
(maybe a bswap if positive number and random value if negative number). Then,
we need to cast respecting the extension that would have happen had we not
replaced the bitwise extension. Here since the bitwise expression is int, it
means we sign extend and then consider the content as being unsigned (bfd_vma
is an unsigned quantity).
When a bswap is necessary we need to do this double cast after doing the bswap
as the bswap must be done on the loaded value since a that's the size expected
by the bswap builtin. Finally, this patch also forbit any sign extension *in*
the bitwise expression as the result of the expression would then be
unpredictable (depend on the initial value).
The patch works this way:
1) prevent size extension of a bitwise expression
2) record the type of the bitwise expression instead of its size (the size can
be determined from the type)
3) use this type to perform a double cast as explained above
2014-05-30 Thomas Preud'homme thomas.preudho...@arm.com
PR tree-optimization/61306
* tree-ssa-math-opts.c (struct symbolic_number): Store type of
expression instead of its size.
(do_shift_rotate): Adapt to change in struct symbolic_number.
(verify_symbolic_number_p): Likewise.
(init_symbolic_number): Likewise.
(find_bswap_or_nop_1): Likewise. Also prevent optimization when the
result of the expressions is unpredictable due to sign extension.
(convert_via): New function to deal with the casting involved from the
loaded value to the target SSA.
(bswap_replace): Rename load_type in range_type to reflect it's the
type the memory accessed shall have before being casted. Select load
type according to whether a bswap needs to be done. Cast first to rhs
with zero extend and then to lhs with sign extend to keep semantic of
original stmts.
(pass_optimize_bswap::execute): Adapt to change in struct
symbolic_number. Decide if the range accessed should be signed or
unsigned before being casted to lhs type based on rhs type and size.
2014-05-29 Thomas Preud'homme thomas.preudho...@arm.com
* gcc.c-torture/execute/pr61306.c: New test.
Patch is in attachment. Is this ok for trunk?
Best regards,
Thomas
PR61306.1.0.diff
Description: Binary data
[PATCH] Fix PR61320: disable bswap for unaligned access on SLOW_UNALIGNED_ACCESS targets
Hi there, It seems from PR61320 that the bswap pass causes some problems when it replaces an OR expression by an unaligned access. Although it's not clear yet why the unaligned load does not go through the extract_bit_field codepath, it is necessary to provide a solution as this prevent sparc from bootstrapping. This patch takes the simple approach of cancelling the bswap optimization when the load that would replace the OR expression would be an unaligned load and the target has SLOW_UNALIGNED_ACCESS. In the long run this patch should be reverted as soon as the root cause of the current problem is found. The patch also rewrite the test to take into account the fact that the optimization is not done for some target. It also add some alignment hint so that more tests can be run even on STRICT_ALIGNMENT targets. ChangeLog changes are: *** gcc/ChangeLog *** 2014-06-03 Thomas Preud'homme thomas.preudho...@arm.com PR tree-optimization/61320 * tree-ssa-math-opts.c (bswap_replace): Cancel bswap optimization when load is unaligned and would be slow for this target. *** gcc/testsuite/ChangeLog *** 2014-06-03 Thomas Preud'homme thomas.preudho...@arm.com * gcc.dg/optimize-bswaphi-1.c: Make variables global when possible to enforce correct alignment and make the test work better on STRICT_ALIGNMENT targets. Also adjust dg-final selectors when alignment cannot be controlled (read_*_3 ()). * gcc.dg/optimize-bswapsi-2.c: Likewise. * gcc.dg/optimize-bswapdi-3.c: Likewise. Bootstrapped on x86_64-linux-gnu and no regression found in the testsuite. Patch is in attachment. It applies on top of the one for PR61306 in the email titled [PATCH] Fix PR61306: improve handling of sign and cast in bswap but can be trivially modified to apply directly on trunk should that patch (PR61306) need to be improved. Is this ok for trunk? Best regards, Thomas PR61320.1.0.diff Description: Binary data
Re: Fix a function decl in gfortran
Tobias Burnus wrote:
There are two functions groups called with mask, cf. iresolve.c's
gfc_resolve_minloc. Namely, without dim mmin0_kind_type and with
dim mmin1_kind_type. – The latter takes dim as additional
argument.
Thus, for mmin1_* the generated decl is fine, for mmin0_ is is not. I
do not immediately see where the dim argument is skipped – but some
similar special-case handling would then be also needed for the
function declaration.
To answer myself: gfc_conv_intrinsic_function sets se-ignore_optional
– and, thus, dim= is not passed as actual argument.
I think what's needed is to modify gfc_copy_formal_args_intr – called by
trans-intrinsic.c's gfc_get_symbol_for_expr – such that it optionally
ignores absent optionals, i.e. passing expr-value.function.actual and
walking it along the formal arguments, skipping the formal argument when
actual-expr == NULL.
Tobias
PS: I have attached a completely untested draft patch.
diff --git a/gcc/fortran/gfortran.h b/gcc/fortran/gfortran.h
index 3e5cdbd..1df79fd 100644
--- a/gcc/fortran/gfortran.h
+++ b/gcc/fortran/gfortran.h
@@ -2750,3 +2787,4 @@ bool gfc_type_compatible (gfc_typespec *, gfc_typespec *);
-void gfc_copy_formal_args_intr (gfc_symbol *, gfc_intrinsic_sym *);
+void gfc_copy_formal_args_intr (gfc_symbol *, gfc_intrinsic_sym *,
+gfc_actual_arglist *);
diff --git a/gcc/fortran/resolve.c b/gcc/fortran/resolve.c
index 7579573..d224d6e 100644
--- a/gcc/fortran/resolve.c
+++ b/gcc/fortran/resolve.c
@@ -1676,3 +1676,3 @@ gfc_resolve_intrinsic (gfc_symbol *sym, locus *loc)
- gfc_copy_formal_args_intr (sym, isym);
+ gfc_copy_formal_args_intr (sym, isym, NULL);
diff --git a/gcc/fortran/symbol.c b/gcc/fortran/symbol.c
index 3785c2e..bb5b440 100644
--- a/gcc/fortran/symbol.c
+++ b/gcc/fortran/symbol.c
@@ -4017,6 +4044,10 @@ add_proc_interface (gfc_symbol *sym, ifsrc source, gfc_formal_arglist *formal)
declaration statement (see match_proc_decl()) to create the formal
- args based on the args of a given named interface. */
+ args based on the args of a given named interface.
+
+ When an actual argument list is provided, skip the absent arguments.
+ To be used together with gfc_se-gnore_optional. */
void
-gfc_copy_formal_args_intr (gfc_symbol *dest, gfc_intrinsic_sym *src)
+gfc_copy_formal_args_intr (gfc_symbol *dest, gfc_intrinsic_sym *src,
+ gfc_actual_arglist *actual)
{
@@ -4027,2 +4058,3 @@ gfc_copy_formal_args_intr (gfc_symbol *dest, gfc_intrinsic_sym *src)
gfc_formal_arglist *formal_prev = NULL;
+ gfc_actual_arglist *act_arg = actual;
/* Save current namespace so we can change it for formal args. */
@@ -4037,2 +4069,13 @@ gfc_copy_formal_args_intr (gfc_symbol *dest, gfc_intrinsic_sym *src)
{
+ /* Skip absent arguments. */
+ if (actual)
+ {
+ gcc_assert (act_arg != NULL);
+ if (act_arg == NULL)
+ {
+ act_arg = act_arg-next;
+ continue;
+ }
+ act_arg = act_arg-next;
+ }
formal_arg = gfc_get_formal_arglist ();
diff --git a/gcc/fortran/trans-intrinsic.c b/gcc/fortran/trans-intrinsic.c
index a76d0f7..f89b45c 100644
--- a/gcc/fortran/trans-intrinsic.c
+++ b/gcc/fortran/trans-intrinsic.c
@@ -2396,3 +2769,4 @@ gfc_get_symbol_for_expr (gfc_expr * expr)
- gfc_copy_formal_args_intr (sym, expr-value.function.isym);
+ gfc_copy_formal_args_intr (sym, expr-value.function.isym,
+ expr-value.function.actual);
Re: [PATCH, i386, Pointer Bounds Checker 10/x] Partitions
On 06/02/14 05:50, Ilya Enkovich wrote: On 30 May 11:10, Jeff Law wrote: On 05/28/14 10:06, Ilya Enkovich wrote: Hi, This patch keeps instrumented and original versions together and preserve tranparent alias chain during symbol name privatization. Bootstrapped and tested on linux-x86_64. Thanks, Ilya -- gcc/ 2013-05-28 Ilya Enkovich ilya.enkov...@intel.com * lto/lto-partition.c (add_symbol_to_partition_1): Keep original and instrumented versions together. This part is OK. Note lto/ has its own ChangeLog, so put the ChangeLog entry there and don't use the lto/ prefix in the ChangeLog entry. (privatize_symbol_name): Restore transparent alias chain if required. What exactly are you doing here? The comment in the code doesn't really make it clear what you are doing or why. + /* We could change name which is a target of transparent alias + chain of instrumented function name. Fix alias chain if so .*/ So are you saying that we want to change the name? Or that it could have been changed and we have to adjust something because it was changed? I'm certainly not as familiar with the LTO stuff as I should be -- what is the purpose behing chaining the DECL_ASSEMBLER_NAME nodes? I'm just adjusting renaming to support alias chains. LTO renames functions to avoid two static functions with the same assembler name. Instrumented functions have names chained with original names using IDENTIFIER_TRANSPARENT_ALIAS. If name of the original function is privatized, then IDENTIFIER_TRANSPARENT_ALIAS still points to the old name which is wrong. My patch fixes it. jeff Here is fixed ChangeLog. Thanks, Ilya -- gcc/lto 2014-06-02 Ilya Enkovich ilya.enkov...@intel.com * lto-partition.c (add_symbol_to_partition_1): Keep original and instrumented versions together. (privatize_symbol_name): Restore transparent alias chain if required. OK. This is fine once all the other pointer checking bits are approved. jeff
[C PATCH] Better location for switch warnings (PR c/30020)
It is trivial to pass the location from c_add_case_label down to
check_case_bounds, so do that. With it, we instead of
i.c:4:3: warning: case label value is less than minimum value for type
switch (c) { case 42: case -1: return -1; };
^
output
i.c:4:25: warning: case label value is less than minimum value for type
switch (c) { case 42: case -1: return -1; };
^
which is better.
Tested x86_64, ok for trunk?
2014-06-04 Marek Polacek pola...@redhat.com
PR c/30020
* c-common.c (check_case_bounds): Add location parameter.
Use it.
(c_add_case_label): Pass loc to check_case_bounds.
* c-c++-common/pr30020.c: New test.
diff --git gcc/c-family/c-common.c gcc/c-family/c-common.c
index 6ec14fc..07a1798 100644
--- gcc/c-family/c-common.c
+++ gcc/c-family/c-common.c
@@ -301,7 +301,7 @@ struct visibility_flags visibility_options;
static tree c_fully_fold_internal (tree expr, bool, bool *, bool *);
static tree check_case_value (tree);
-static bool check_case_bounds (tree, tree, tree *, tree *);
+static bool check_case_bounds (location_t, tree, tree, tree *, tree *);
static tree handle_packed_attribute (tree *, tree, tree, int, bool *);
static tree handle_nocommon_attribute (tree *, tree, tree, int, bool *);
@@ -3355,7 +3355,7 @@ check_case_value (tree value)
untouched) or false if the label is out of range. */
static bool
-check_case_bounds (tree type, tree orig_type,
+check_case_bounds (location_t loc, tree type, tree orig_type,
tree *case_low_p, tree *case_high_p)
{
tree min_value, max_value;
@@ -3373,7 +3373,8 @@ check_case_bounds (tree type, tree orig_type,
if (tree_int_cst_compare (case_low, min_value) 0
tree_int_cst_compare (case_high, min_value) 0)
{
- warning (0, case label value is less than minimum value for type);
+ warning_at (loc, 0, case label value is less than minimum value
+ for type);
return false;
}
@@ -3381,7 +3382,7 @@ check_case_bounds (tree type, tree orig_type,
if (tree_int_cst_compare (case_low, max_value) 0
tree_int_cst_compare (case_high, max_value) 0)
{
- warning (0, case label value exceeds maximum value for type);
+ warning_at (loc, 0, case label value exceeds maximum value for type);
return false;
}
@@ -3389,8 +3390,8 @@ check_case_bounds (tree type, tree orig_type,
if (tree_int_cst_compare (case_high, min_value) = 0
tree_int_cst_compare (case_low, min_value) 0)
{
- warning (0, lower value in case label range
- less than minimum value for type);
+ warning_at (loc, 0, lower value in case label range
+ less than minimum value for type);
case_low = min_value;
}
@@ -3398,8 +3399,8 @@ check_case_bounds (tree type, tree orig_type,
if (tree_int_cst_compare (case_low, max_value) = 0
tree_int_cst_compare (case_high, max_value) 0)
{
- warning (0, upper value in case label range
- exceeds maximum value for type);
+ warning_at (loc, 0, upper value in case label range
+ exceeds maximum value for type);
case_high = max_value;
}
@@ -6014,7 +6015,7 @@ c_add_case_label (location_t loc, splay_tree cases, tree
cond, tree orig_type,
expression. If both low_value and high_value are out of range,
don't insert the case label and return NULL_TREE. */
if (low_value
- !check_case_bounds (type, orig_type,
+ !check_case_bounds (loc, type, orig_type,
low_value, high_value ? high_value : NULL))
return NULL_TREE;
diff --git gcc/testsuite/c-c++-common/pr30020.c
gcc/testsuite/c-c++-common/pr30020.c
index e69de29..b8082cd 100644
--- gcc/testsuite/c-c++-common/pr30020.c
+++ gcc/testsuite/c-c++-common/pr30020.c
@@ -0,0 +1,12 @@
+/* PR c/30020 */
+/* { dg-do compile } */
+
+int
+foo (unsigned char c)
+{
+ switch (c) { case 42: case -1: return -1; }; /* { dg-warning 25:case label
value } */
+ switch (c) { case 42: case 300: return -1; }; /* { dg-warning 25:case label
value } */
+ switch (c) { case 42: case -1 ... 2: return -1; }; /* { dg-warning 25:lower
value in case } */
+ switch (c) { case 42: case 250 ... 300: return -1; }; /* { dg-warning
25:upper value in case } */
+ return 0;
+}
Marek
Re: [PATCH, Pointer Bounds Checker 13/x] Early versioning
On 06/03/14 03:29, Richard Biener wrote:
On Tue, Jun 3, 2014 at 7:55 AM, Ilya Enkovich enkovich@gmail.com wrote:
2014-06-02 21:27 GMT+04:00 Jeff Law l...@redhat.com:
On 06/02/14 04:48, Ilya Enkovich wrote:
Hmm, so if I understand things correctly, src_fun has no loop
structures attached, thus there's nothing to copy. Presumably at
some later point we build loop structures for the copy from scratch?
I suppose it is just a simple bug with absent NULL pointer check. Here is
original code:
/* Duplicate the loop tree, if available and wanted. */
if (loops_for_fn (src_cfun) != NULL
current_loops != NULL)
{
copy_loops (id, entry_block_map-loop_father,
get_loop (src_cfun, 0));
/* Defer to cfgcleanup to update loop-father fields of
basic-blocks. */
loops_state_set (LOOPS_NEED_FIXUP);
}
/* If the loop tree in the source function needed fixup, mark the
destination loop tree for fixup, too. */
if (loops_for_fn (src_cfun)-state LOOPS_NEED_FIXUP)
loops_state_set (LOOPS_NEED_FIXUP);
As you may see we have check for absent loops structure in the first
if-statement and no check in the second one. I hit segfault and added the
check.
Downthread you indicated you're not in SSA form which might explain the
inconsistency here. If so, then we need to make sure that the loop df
structures do get set up properly later.
That is what init_data_structures pass will do for us as Richard pointed. Right?
loops are set up during the CFG construction and thus are available everywhere.
Which would argue that the hunk that checks for the loop tree's
existence before accessing it should not be needed. Ilya -- is it
possible you hit this prior to Richi's work to build the loop structures
as part of CFG construction and maintain them throughout compilation.
the df structures are set up in init_data_structures pass which is run before
going into SSA form (I'd like to somehow cleanup that area).
OK. So this part should be approved since we've established this code
is running prior to going into SSA form.
jeff
[PATCH]Code size optimization for the fmul/fdiv and dmul/ddiv function in libgcc
Hi there, In libgcc the file ieee754-sf.S and ieee754-df.S have some function pairs which will be bundled into one .o file and sharing the same .text section. For example, the fmul and fdiv, the libgcc makefile will build them into one .o file and archived into libgcc.a. So when user only call single float point multiply functions, the fdiv function will also be linked, and as fmul and fdiv share the same .text section, linker option --gc-sections or -flot can’t remove the dead code. So the optimization just separates the function pair(fmul/fdiv and dmul/ddiv) into different sections, following the naming pattern of –ffunction-sections(.text.__functionname), through which the unused sections of fdiv/ddiv can be eliminated through option --gcc-sections when users only use fmul/dmul.The solution is to add a conditional statement in the macro FUNC_START, which will conditional change the section of a function from .text to .text.__\name. when compiling with the L_arm_muldivsf3 or L_arm_muldivdf3 macro. There are 3 parts: mul, div and common. This patch puts mul and common together, so that user's multiply won't pull-in div, butuser's div will still pull-in mul. It is reasonable because size of mul is far smaller than size of div. ChangLog changes are: ***gcc/libgcc/ChangeLog*** 2014-05-28 Tony Wang tony.w...@arm.com * config/arm/lib1funcs.S (FUNC_START): Add conditional section redefine for macro L_arm_muldivsf3 and L_arm_muldivdf3 Bootstrapped on x86_64-linux-gnu and no regression found in the testsuite. Patch is in attachment. The code reduction for thumb2 on cortex-m3 is: 1. When user only use single float point multiply: fmul+fdiv = fmul will have a code size reduction of 318 bytes. 2. When user only use double float point multiply: dmul+ddiv = dmul will have a code size reduction of 474 bytes. BR, Tony diff --git a/libgcc/config/arm/lib1funcs.S b/libgcc/config/arm/lib1funcs.S index b617137..0454bc8 100644 --- a/libgcc/config/arm/lib1funcs.S +++ b/libgcc/config/arm/lib1funcs.S @@ -419,7 +419,11 @@ SYM (\name): #endif .macro FUNC_START name +#if defined (L_arm_muldivsf3) || defined (L_arm_muldivdf3) + .section.text.__\name,ax,%progbits +#else .text +#endif .globl SYM (__\name) TYPE (__\name) .align 0 @@ -468,7 +472,11 @@ _L__\name: #define EQUIV .thumb_set #else .macro ARM_FUNC_START name +#if defined (L_arm_muldivsf3) || defined (L_arm_muldivdf3) + .section.text.__\name,ax,%progbits +#else .text +#endif .globl SYM (__\name) TYPE (__\name) .align 0
Re: [PATCH, i386, Pointer Bounds Checker 17/x] Pointer bounds constants support
On 06/02/14 04:25, Ilya Enkovich wrote: Hi, This patch adds support for pointer bounds constants to be used as DECL_INITIAL for constant bounds (like zero bounds). Bootstrapped and tested on linux-x86_64. Thanks, Ilya -- gcc/ 2014-05-30 Ilya Enkovich ilya.enkov...@intel.com * emit-rtl.c (immed_double_const): Support MODE_POINTER_BOUNDS. (init_emit_once): Build pointer bounds zero constants. * explow.c (trunc_int_for_mode): Likewise. * varpool.c (ctor_for_folding): Do not fold constant bounds vars. * varasm.c (output_constant_pool_2): Support MODE_POINTER_BOUNDS. * config/i386/i386.c (ix86_legitimate_constant_p): Mark bounds constant as not valid. [ ... ] @@ -5875,6 +5876,11 @@ init_emit_once (void) if (STORE_FLAG_VALUE == 1) const_tiny_rtx[1][(int) BImode] = const1_rtx; + for (mode = GET_CLASS_NARROWEST_MODE (MODE_POINTER_BOUNDS); + mode != VOIDmode; + mode = GET_MODE_WIDER_MODE (mode)) +const_tiny_rtx[0][mode] = immed_double_const (0, 0, mode); I'm pretty sure GET_CLASS_NARROWEST_MODE should be taking a class, not a mode as its argument. So something is clearly wrong here... jeff
Re: [PATCH, Pointer Bounds Checker 21/x] Weakrefs output
On 06/02/14 09:22, Ilya Enkovich wrote: Hi, This patch prevents output of both instrumented and not instrumented weakref variants. Thanks, Ilya -- gcc/ 2014-06-02 Ilya Enkovich ilya.enkov...@intel.com * cgraphunit.c (output_weakrefs): If there are both instrumented and original versions, output only one of them. OK. Jeff
Re: [C PATCH] Better location for switch warnings (PR c/30020)
On 06/04/14 00:46, Marek Polacek wrote:
It is trivial to pass the location from c_add_case_label down to
check_case_bounds, so do that. With it, we instead of
i.c:4:3: warning: case label value is less than minimum value for type
switch (c) { case 42: case -1: return -1; };
^
output
i.c:4:25: warning: case label value is less than minimum value for type
switch (c) { case 42: case -1: return -1; };
^
which is better.
Tested x86_64, ok for trunk?
2014-06-04 Marek Polacek pola...@redhat.com
PR c/30020
* c-common.c (check_case_bounds): Add location parameter.
Use it.
(c_add_case_label): Pass loc to check_case_bounds.
* c-c++-common/pr30020.c: New test.
OK.
ISTM that additional patches of this nature (add location parameter to
get better diagnostics) ought to be considered pre-approved.
jeff
Re: [PATCH, Pointer Bounds Checker 23/x] Function split
On 06/03/14 01:10, Ilya Enkovich wrote: Hi, This patch does not allow splitting in case bounds are returned until retutrned bounds are supported. It also propagates instrumentation marks for generated call and function. Bootstrapped and tested on linux-x86_64. Thanks, Ilya -- gcc/ 2014-06-03 Ilya Enkovich ilya.enkov...@intel.com * ipa-split.c: Include tree-chkp.h. (consider_split): Do not split when return bounds. (split_function): Propagate Pointer Bounds Checker instrumentation marks. It's a hack. There's no reason we can't support this. So I'll approve on the condition that you do look at removing this limitation in the future. jeff
Re: [C PATCH] Don't reject valid code with _Alignas (PR c/61053)
On 06/02/14 00:52, Marek Polacek wrote:
Ping.
On Sun, May 25, 2014 at 11:30:51AM +0200, Marek Polacek wrote:
On Mon, May 05, 2014 at 10:27:03PM +0200, Marek Polacek wrote:
In this PR the issue is that we reject (valid) code such as
_Alignas (long long) long long foo;
with -m32, because we trip this condition:
alignas_align = 1U declspecs-align_log;
if (alignas_align TYPE_ALIGN_UNIT (type))
{
if (name)
error_at (loc, %_Alignas% specifiers cannot reduce
alignment of %qE, name);
and error later on, since alignas_align is 4 (correct, see PR52023 for
why), but TYPE_ALIGN_UNIT of long long is 8. I think TYPE_ALIGN_UNIT
is wrong here as that won't give us minimal alignment required.
In c_sizeof_or_alignof_type we already have the code to compute such
minimal alignment so I just moved the code to a separate function
and used that instead of TYPE_ALIGN_UNIT.
Note that the test is run only on i?86 and x86_64, because we can't (?)
easily determine which target requires what alignment.
Regtested/bootstrapped on x86_64-unknown-linux-gnu and
powerpc64-unknown-linux-gnu, ok for trunk?
Can I backport this one to 4.9?
Yes, backporting to 4.9 is fine.
jeff
Re: [build, doc, testsuite] Centralise clearing hardware capabilities with Sun ld
On 05/27/14 02:04, Rainer Orth wrote: It's been a week since I've submitted the patch, so far having received approval for the testsuite parts only. While I don't need approval for the Solaris-only parts, it would be good if the doc and build changes could be checked by the respective maintainers. One might argue that they are Solaris-only, thus falling under my maintainership, but better safe than sorry. I think those bits fall under your Solaris maintainership umbrella. While Jakub may have complained in the older threads about the concept of using hwcaps like this, as the Solaris maintainer you can make that choice for that platform. Jeff
Re: [PATCH] Fix PR middle-end/61141
On 05/24/14 12:39, John David Anglin wrote: The insns are being deleted in final after the assembly output has being done. So, for example, pa_output_call is never called with a delayed branch sequence containing a NOTE_INSN_DELETED. OK. That's good to know. I scanned final and final_scan_insn and see a couple places where we delete insns, but if I'm reading the code correctly, they would result in seeing the NOTE_INSN_DELETED rather than a real insn in the output routines. So what code during final is deleting these insns? Jeff
Re: Fix a function decl in gfortran
Still untested patch, but I cannot resist pointing out stupid
typos by myself.
I intent to tests the build and test the patch - and then to
commit it as obvious. If you see problems with this approach
please scream now.
On Wed, Jun 04, 2014 at 08:13:44AM +0200, Tobias Burnus wrote:
+ To be used together with gfc_se-gnore_optional. */
s/gnore/ignore/
+ if (actual)
+ {
+ gcc_assert (act_arg != NULL);
+ if (act_arg == NULL)
s/act_arg == NULL/act_arg-expr == NULL/
Tobias
Re: [PATCH, Pointer Bounds Checker 6/x] New static constructor types
On 04/16/14 06:33, Ilya Enkovich wrote: Hi, This patch add new static constructor types used by Pointer Bounds Checker. It was approved earlier for 4.9 and I'll assume patch is OK for trunk if no objections arise. Patch was bootstrapped and tested for linux-x86_64. Thanks, Ilya -- gcc/ 2014-04-16 Ilya Enkovich ilya.enkov...@intel.com * ipa.c (cgraph_build_static_cdtor_1): Support contructors with chkp ctor and bnd_legacy attributes. * gimplify.c (gimplify_init_constructor): Avoid infinite loop during gimplification of bounds initializer. This is OK for the trunk. Per Richi's request, please hold off committing until the entire series is approved. Thanks, jeff
Re: PR61385: phiopt drops some PHIs
On Tue, 3 Jun 2014, Jeff Law wrote:
On 06/03/14 08:08, Richard Biener wrote:
All arguments get the same value (and the PHI in middle-bb is surely
a singleton?), so it's way better to re-materialize the PHI as a
gimple assignment at the start of the basic block. If they are
singletons (which I expect), the easiest way is to propagate their
single argument into all uses and simply remove them.
I was all for propagating, but replace_uses_by calls fold_stmt on the
using statements (that's normal). If I first move the statement and then
replace, I may call fold_stmt on a stmt that isn't dominated by the defs
of all its arguments. If I first replace, the statement I am supposed to
move may have been modified so I need to find out what happened to it. In
the end, the assignment seems easier and safer (and not too bad since this
case almost never happens in practice).
We certainly want to get rid of them :-)
I'd start by finding out which pass left the degenerate, ensure it's not
marked as SSA_NAME_OCCURS_IN_ABNORMAL_PHI, then propagate it away.
If it's a systemic problem that a particular pass can leave degenerate PHIs,
then you might want to schedule the phi-only propagator to run after that
pass. It does const/copy propagation seeding from degenerate PHIs, so it
ought to be very fast.
This one comes from VRP2 apparently. But it isn't the only pass that
produces singleton phis. I didn't look at them closely, but I assume LIM
is normal, DOM maybe less so, etc.
bootstrap+testsuite on x86_64-linux-gnu as usual.
2014-06-04 Marc Glisse marc.gli...@inria.fr
PR tree-optimization/61385
gcc/
* tree-ssa-phiopt.c (value_replacement): Replace singleton PHIs
with assignments.
gcc/testsuite/
* gcc.dg/tree-ssa/pr61385.c: New file.
--
Marc GlisseIndex: gcc/testsuite/gcc.dg/tree-ssa/pr61385.c
===
--- gcc/testsuite/gcc.dg/tree-ssa/pr61385.c (revision 0)
+++ gcc/testsuite/gcc.dg/tree-ssa/pr61385.c (working copy)
@@ -0,0 +1,43 @@
+/* { dg-do compile } */
+/* { dg-options -O2 } */
+
+#define assert(x) if (!(x)) __builtin_abort ()
+
+int a, b, c, d, e, f, g;
+
+int
+fn1 ()
+{
+ int *h = c;
+ for (; c 1; c++)
+{
+ int *i = a, *k = a;
+ f = 0;
+ if (b)
+ return 0;
+ if (*h)
+ {
+ int **j = i;
+ *j = 0;
+ d = 0;
+ }
+ else
+ g = e = 0;
+ if (*h)
+ {
+ int **l = k;
+ *l = g;
+ }
+ d = *h;
+ assert (k == a || k);
+ assert (i);
+}
+ return 0;
+}
+
+int
+main ()
+{
+ fn1 ();
+ return 0;
+}
Index: gcc/tree-ssa-phiopt.c
===
--- gcc/tree-ssa-phiopt.c (revision 211178)
+++ gcc/tree-ssa-phiopt.c (working copy)
@@ -877,21 +877,35 @@ value_replacement (basic_block cond_bb,
operand_equal_for_phi_arg_p (rhs2, cond_lhs)
neutral_element_p (code_def, cond_rhs, true))
|| (arg1 == rhs2
operand_equal_for_phi_arg_p (rhs1, cond_lhs)
neutral_element_p (code_def, cond_rhs, false))
|| (operand_equal_for_phi_arg_p (arg1, cond_rhs)
(operand_equal_for_phi_arg_p (rhs2, cond_lhs)
|| operand_equal_for_phi_arg_p (rhs1, cond_lhs))
absorbing_element_p (code_def, cond_rhs
{
+ /* Any PHI node is a singleton and we can replace it with an assignment.
+We don't just call replace_uses_by because it calls fold_stmt and it
+becomes hard to make all the right adjustments in the right order. */
gsi = gsi_for_stmt (cond);
+ gimple_stmt_iterator psi = gsi_start_phis (middle_bb);
+ while (!gsi_end_p (psi))
+ {
+ gimple phi_moving = gsi_stmt (psi);
+ tree phi_res = gimple_phi_result (phi_moving);
+ tree phi_arg = gimple_phi_arg_def (phi_moving, 0);
+ gsi_remove (psi, false);
+ gsi_insert_before (gsi, gimple_build_assign (phi_res, phi_arg),
+GSI_SAME_STMT);
+ }
+
gimple_stmt_iterator gsi_from = gsi_for_stmt (assign);
gsi_move_before (gsi_from, gsi);
replace_phi_edge_with_variable (cond_bb, e1, phi, lhs);
return 2;
}
return 0;
}
/* The function minmax_replacement does the main work of doing the minmax
Re: [PATCH, Pointer Bounds Checker 5/x] Attributes
On 04/16/14 06:26, Ilya Enkovich wrote: Hi, This patch introduces attributes used by Pointer Bounds Checker. Comparing to what was approved for 4.9, this one has additional attribute 'bnd_instrument' to be used for selective instrumentation. Bootstrapped and tested on linux-x86_64. OK for trunk? Thanks, Ilya -- gcc/ 2014-04-16 Ilya Enkovich ilya.enkov...@intel.com * c-family/c-common.c (handle_bnd_variable_size_attribute): New. (handle_bnd_legacy): New. (handle_bnd_instrument): New. (c_common_attribute_table): Add bnd_variable_size, bnd_legacy and bnd_instrument. * doc/extend.texi: Document bnd_variable_size, bnd_legacy and bnd_instrument attributes. This is fine for the trunk, but per Richi's request please hold off committing until the entire series is approved. Thanks, jeff
Re: PR61385: phiopt drops some PHIs
On 06/04/14 01:46, Marc Glisse wrote: On Tue, 3 Jun 2014, Jeff Law wrote: On 06/03/14 08:08, Richard Biener wrote: All arguments get the same value (and the PHI in middle-bb is surely a singleton?), so it's way better to re-materialize the PHI as a gimple assignment at the start of the basic block. If they are singletons (which I expect), the easiest way is to propagate their single argument into all uses and simply remove them. I was all for propagating, but replace_uses_by calls fold_stmt on the using statements (that's normal). If I first move the statement and then replace, I may call fold_stmt on a stmt that isn't dominated by the defs of all its arguments. If I first replace, the statement I am supposed to move may have been modified so I need to find out what happened to it. In the end, the assignment seems easier and safer (and not too bad since this case almost never happens in practice). We certainly want to get rid of them :-) I'd start by finding out which pass left the degenerate, ensure it's not marked as SSA_NAME_OCCURS_IN_ABNORMAL_PHI, then propagate it away. If it's a systemic problem that a particular pass can leave degenerate PHIs, then you might want to schedule the phi-only propagator to run after that pass. It does const/copy propagation seeding from degenerate PHIs, so it ought to be very fast. This one comes from VRP2 apparently. But it isn't the only pass that produces singleton phis. I didn't look at them closely, but I assume LIM is normal, DOM maybe less so, etc. DOM VRP can create them via jump threading. Right now we only fire up the pass_phi_only_cprop after DOM though. DOM runs not too long after LIM and it will propagate away the degenerates as well. One could argue that propagating away the degenerates ought to occur immediately before any phi-opt pass. bootstrap+testsuite on x86_64-linux-gnu as usual. 2014-06-04 Marc Glisse marc.gli...@inria.fr PR tree-optimization/61385 gcc/ * tree-ssa-phiopt.c (value_replacement): Replace singleton PHIs with assignments. gcc/testsuite/ * gcc.dg/tree-ssa/pr61385.c: New file. Don't you have to verify neither the source nor the destination are marked with SSA_NAME_OCCURS_IN_ABNORMAL_PHI? And if so, you can't propagate away the PHI (which may mean we need to avoid the optimization in that case). I also believe that when turning a PHI into an assignment you have to preserve the parallel nature of the PHI. If the result of one PHI is used as an input in another PHI, then you have to work harder (see the out-of-ssa code). Perhaps this can never be the case with a degenerate. Jeff
Re: [PATCH, Pointer Bounds Checker 13/x] Early versioning
2014-06-04 10:46 GMT+04:00 Jeff Law l...@redhat.com:
On 06/03/14 03:29, Richard Biener wrote:
On Tue, Jun 3, 2014 at 7:55 AM, Ilya Enkovich enkovich@gmail.com
wrote:
2014-06-02 21:27 GMT+04:00 Jeff Law l...@redhat.com:
On 06/02/14 04:48, Ilya Enkovich wrote:
Hmm, so if I understand things correctly, src_fun has no loop
structures attached, thus there's nothing to copy. Presumably at
some later point we build loop structures for the copy from scratch?
I suppose it is just a simple bug with absent NULL pointer check. Here
is
original code:
/* Duplicate the loop tree, if available and wanted. */
if (loops_for_fn (src_cfun) != NULL
current_loops != NULL)
{
copy_loops (id, entry_block_map-loop_father,
get_loop (src_cfun, 0));
/* Defer to cfgcleanup to update loop-father fields of
basic-blocks. */
loops_state_set (LOOPS_NEED_FIXUP);
}
/* If the loop tree in the source function needed fixup, mark the
destination loop tree for fixup, too. */
if (loops_for_fn (src_cfun)-state LOOPS_NEED_FIXUP)
loops_state_set (LOOPS_NEED_FIXUP);
As you may see we have check for absent loops structure in the first
if-statement and no check in the second one. I hit segfault and added
the
check.
Downthread you indicated you're not in SSA form which might explain the
inconsistency here. If so, then we need to make sure that the loop df
structures do get set up properly later.
That is what init_data_structures pass will do for us as Richard pointed.
Right?
loops are set up during the CFG construction and thus are available
everywhere.
Which would argue that the hunk that checks for the loop tree's existence
before accessing it should not be needed. Ilya -- is it possible you hit
this prior to Richi's work to build the loop structures as part of CFG
construction and maintain them throughout compilation.
CFG build pass is in lowering stage and happens before versioning.
Probably I just hit some bug and hide it with my check. I'll try to
remove this check and reproduce a segfault to see why it happens.
Ilya
the df structures are set up in init_data_structures pass which is run
before
going into SSA form (I'd like to somehow cleanup that area).
OK. So this part should be approved since we've established this code is
running prior to going into SSA form.
jeff
Re: [build, doc, testsuite] Centralise clearing hardware capabilities with Sun ld
Jeff Law l...@redhat.com writes: On 05/27/14 02:04, Rainer Orth wrote: It's been a week since I've submitted the patch, so far having received approval for the testsuite parts only. While I don't need approval for the Solaris-only parts, it would be good if the doc and build changes could be checked by the respective maintainers. One might argue that they are Solaris-only, thus falling under my maintainership, but better safe than sorry. I think those bits fall under your Solaris maintainership umbrella. While Jakub may have complained in the older threads about the concept of using hwcaps like this, as the Solaris maintainer you can make that choice for that platform. I did already. As I've explained before, I believe leaving hwcaps enabled with as by default is better than turning them off globally. Thanks. Rainer -- - Rainer Orth, Center for Biotechnology, Bielefeld University
Re: [patch, testsuite] Fix PR 49191 with test for STRICT_ALIGNMENT
On 06/08/11 22:51, Steve Ellcey wrote:
On Wed, 2011-06-08 at 22:19 +0200, Jakub Jelinek wrote:
On Wed, Jun 08, 2011 at 01:16:26PM -0700, Steve Ellcey wrote:
On Wed, 2011-06-08 at 22:08 +0200, Jakub Jelinek wrote:
On Wed, Jun 08, 2011 at 01:03:53PM -0700, Steve Ellcey wrote:
2011-06-08 Steve Ellcey s...@cup.hp.com
PR middle-end/49191
* lib/target-supports.exp (check_effective_target_strict_align): New.
* gcc.dg/memcpy-3.c: Add dg-require-effective-target strict_align.
Isn't the test backwards, i.e. doesn't
{ dg-require-effective-target strict_align }
actually mean !STRICT_ALIGNMENT?
You are right. What do you think I should do, reverse the test or just
rename it? I would be inclined to just rename it, maybe:
dg-require-effective-target non_strict_align
Yeah, either that, or reverse the test and then
{ dg-do run { target { !strict_align } } } */
(if that is the right syntax).
Jakub
Renaming it seems simpler. If we reverse the test, I can't just return
the result of check_no_compiler_messages, I need to save the result and
reverse the return value. Then in the actual tests we can't use
dg-require-effective-target but need to modify the dg-do line to do the
check and it will probably always include the '!' to (re-)negate the
check. It can be done but renaming the test seems a lot simpler.
I just happened to notice that the (no longer new) non_strict_align
keyword lacks both a comment in
target-supports.exp and documentation in sourcebuild.texi.
Could you please add both?
Thanks.
Rainer
Re: [build, driver] RFC: Support compressed debug sections
Mike Stump mikest...@comcast.net writes: On Jun 3, 2014, at 3:40 AM, Rainer Orth r...@cebitec.uni-bielefeld.de wrote: It's been another week, and I still need approval for the build, doc, and Darwin changes: So, the darwin bits look trivial enough, if the entire scheme is what people want to do. My question would be, why do we want an option for this? If the scheme works, why not just turn it on unconditionally? If it doesn’t work, why add it? If it isn’t good, why add it? If it is good, why not do it? It works in very specific circumstances, like assembler and linker versions. If it is just to reach compatibility with the debugger, then I’d rather either just mandate a certain debugger or autoconf for what the current debugger supports. As of late people seem to just break the debugging experience with non-updated gdbs and assume that a newer gdb is used. You cannot do that: unlike the assembler and linker used, which are often hardcoded into gcc, the debugger can easily be changed below the compiler's feet, so to speak. Besides, on several platforms, you have more than one debugger available (like gdb and dbx, or others), so this isn't an option. Apart from that, the debugging experience when e.g. emitting very recent DWARF extensions and trying to use them with a gdb that doesn't understand them usually leads to some debug info missing. In this case, emitting compressed debug with a debugger that cannot read it leads to the debugger claiming (correctly, from its point of view) that there's no debugging info present. I don't want to tell users who come complaining `I compiled with -g, but my debugger tells me there's no debug info present': `look, your debugger lies, it is present, but it cannot read it'. That's a lot worse than the DWARF extensions scenario above. On top of all that, compressed debug is a tradeoff: in some cases it may be worth it to save space on debug info if disk space is at a premium for some reason (e.g. for release builds), but in others you want to compile as fast as possible, but assembling and linking compressed debug takes more CPU time. Otherwise we could just as well default to -Os, telling our users it's better for them since it generates faster and smaller code, not minding the compile time cost and worse debugging experience. I at least don't wont to patronize our users like this. Rainer -- - Rainer Orth, Center for Biotechnology, Bielefeld University
Re: [PATCH][AARCH64]Support full addressing modes for ldr/str in vectorization scenarios
On 4 June 2014 03:11, Bin.Cheng amker.ch...@gmail.com wrote: Yes, If there is a PR, I can evaluate how this can help and ask release maintainer for approval. I'll reduce the test case and create one shortly
Re: PR61385: phiopt drops some PHIs
On Tue, Jun 3, 2014 at 10:30 PM, Marc Glisse marc.gli...@inria.fr wrote:
On Tue, 3 Jun 2014, Richard Biener wrote:
On Tue, Jun 3, 2014 at 3:48 PM, Marc Glisse marc.gli...@inria.fr wrote:
Hello,
apparently it is possible to have a PHI in the middle basic block of
value_replacement, so I need to move it as well when I move the statement
and remove the block.
Bootstrap and testsuite on x86_64-linux-gnu (re-running for various
reasons
but they had completed successfully yesterday).
2014-06-03 Marc Glisse marc.gli...@inria.fr
PR tree-optimization/61385
gcc/
* tree-ssa-phiopt.c (value_replacement): Copy PHI nodes before
removing the basic block.
gcc/testsuite/
* gcc.dg/tree-ssa/pr61385.c: New file.
--
Marc Glisse
Index: gcc/testsuite/gcc.dg/tree-ssa/pr61385.c
===
--- gcc/testsuite/gcc.dg/tree-ssa/pr61385.c (revision 0)
+++ gcc/testsuite/gcc.dg/tree-ssa/pr61385.c (working copy)
@@ -0,0 +1,43 @@
+/* { dg-do compile } */
+/* { dg-options -O2 } */
+
+#define assert(x) if (!(x)) __builtin_abort ()
+
+int a, b, c, d, e, f, g;
+
+int
+fn1 ()
+{
+ int *h = c;
+ for (; c 1; c++)
+{
+ int *i = a, *k = a;
+ f = 0;
+ if (b)
+ return 0;
+ if (*h)
+ {
+ int **j = i;
+ *j = 0;
+ d = 0;
+ }
+ else
+ g = e = 0;
+ if (*h)
+ {
+ int **l = k;
+ *l = g;
+ }
+ d = *h;
+ assert (k == a || k);
+ assert (i);
+}
+ return 0;
+}
+
+int
+main ()
+{
+ fn1 ();
+ return 0;
+}
Index: gcc/tree-ssa-phiopt.c
===
--- gcc/tree-ssa-phiopt.c (revision 211178)
+++ gcc/tree-ssa-phiopt.c (working copy)
@@ -877,20 +877,39 @@ value_replacement (basic_block cond_bb,
operand_equal_for_phi_arg_p (rhs2, cond_lhs)
neutral_element_p (code_def, cond_rhs, true))
|| (arg1 == rhs2
operand_equal_for_phi_arg_p (rhs1, cond_lhs)
neutral_element_p (code_def, cond_rhs, false))
|| (operand_equal_for_phi_arg_p (arg1, cond_rhs)
(operand_equal_for_phi_arg_p (rhs2, cond_lhs)
|| operand_equal_for_phi_arg_p (rhs1, cond_lhs))
absorbing_element_p (code_def, cond_rhs
{
+ /* Move potential PHI nodes. */
+ gimple_stmt_iterator psi = gsi_start_phis (middle_bb);
+ while (!gsi_end_p (psi))
+ {
+ gimple phi_moving = gsi_stmt (psi);
+ gimple newphi = create_phi_node (gimple_phi_result
(phi_moving),
+ cond_bb);
+ int nargs = cond_bb-preds-length();
+ location_t loc = gimple_phi_arg_location (phi_moving, 0);
+ tree phi_arg = gimple_phi_arg_def (phi_moving, 0);
+ for (int i = 0; i nargs; ++i)
+ {
+ edge e = (*cond_bb-preds)[i];
+ add_phi_arg (newphi, phi_arg, e, loc);
All arguments get the same value (and the PHI in middle-bb is surely
a singleton?),
Yes, there is a single incoming edge to middle-bb.
so it's way better to re-materialize the PHI as a
gimple assignment at the start of the basic block.
I thought there might be a reason why it was a PHI and not an assignment so
I wasn't sure doing that would be ok. It is indeed much easier, so I'll do
that...
Maybe it's leftover of loop closed ssa form, but I guess it's useless
at the stage of phiopt.
Thanks,
bin
If they are
singletons (which I expect), the easiest way is to propagate their
single argument into all uses and simply remove them.
... or indeed that, now that I have found a function called replace_uses_by
which should do exactly what I need.
Thanks,
--
Marc Glisse
--
Best Regards.
RE: [PATCH ARM] Improve ARM memset inlining
Ping^4. The original thread is https://gcc.gnu.org/ml/gcc-patches/2014-05/msg00182.html, also there is some info at https://gcc.gnu.org/ml/gcc-patches/2014-05/msg00182.html in the same thread. Thanks, bin -Original Message- From: gcc-patches-ow...@gcc.gnu.org [mailto:gcc-patches- ow...@gcc.gnu.org] On Behalf Of bin.cheng Sent: Wednesday, May 28, 2014 4:53 PM To: Richard Earnshaw Cc: gcc-patches List Subject: RE: [PATCH ARM] Improve ARM memset inlining Ping^3 -Original Message- From: Bin.Cheng [mailto:amker.ch...@gmail.com] Sent: Monday, May 19, 2014 2:40 PM To: Bin Cheng Cc: Richard Earnshaw; gcc-patches List Subject: Re: [PATCH ARM] Improve ARM memset inlining Ping^2 Thanks, bin On Mon, May 12, 2014 at 11:17 AM, Bin.Cheng amker.ch...@gmail.com wrote: Ping. Thanks, bin On Tue, May 6, 2014 at 12:59 PM, bin.cheng bin.ch...@arm.com wrote: Precisely, I configured gcc with options --with-arch=armv7-a --with-cpu|--with-tune=cortex-a9. I read gcc documents and realized that -mcpu is ignored when -march is specified. I don't know why gcc acts in this manner, but it leads to inconsistent configuration/command line behavior. If we configure GCC with --with-arch=armv7-a --with-cpu=cortex-a9, then only -march=armv7-a is passed to cc1. If we compile with -march=armv7-a -mcpu=cortex-a9, then gcc works fine and passes -march=armv7-a -mcpu=cortex-a9 to cc1. Even more weird cc1 warns that switch -mcpu=cortex-m4 conflicts with -march=armv7-m switch. Thanks, bin -- Best Regards. -- Best Regards.
Re: Eliminate write-only variables
On Tue, Jun 3, 2014 at 6:19 PM, Jan Hubicka hubi...@ucw.cz wrote: On Mon, Jun 2, 2014 at 8:59 PM, Jan Hubicka hubi...@ucw.cz wrote: Yeah, I discussed this with martin today on irc. For aliasing we'd like to know whether a decl possibly has its address taken. Currently we only trust TREE_ADDRESSABLE for statics - and lto might change those to hidden visibility public :( So we want deck_referred_to_by_single_function OK, I will implement this one in IPA mini-pass. It is easy. This property changes by clonning and inlining. What Martin wants to use it for? (I.e. my plan would be to compute it as last IPA pass making it useless for IPA analysispropagation) He doesn't want to use it but we talked and he said he'd have a look. Note that it's important the decls are not refered to by global initializers either. Why is a new pass necessary - can't the IPA reference maintaining code update some flag in the varpool magically? If the code to add/remove reference was updating the flag, it would became out of date as we remove callgraph during the late compilation (we do not keep references for functions we already compiled). I do not want the flags to be computed before end of IPA queue so they won't become out of date as we clone/inline. We basically need to walk references and check that they are all functions either one given function F or a clones inlined to it. Assuming that function does not have unbounded number of references to given variale, this is basically constant time test. and deck_may_have_address_taken. We currently clear TREE_ADDRESSABLE for statics that have no address taken at WPA time and that ought to keep the flag cleared at ltrans (I think I even made testcase for this) What I think we could improve is to not consider string functions as ADDR operations for this analysis, i.e. it is common to only memset to 0. How may_have_address_taken would differ here? I want tree.h:may_be_aliased to return false if a decl doesn't have its address taken. We don't trust TREE_ADDRESSABLE for TREE_PUBLIC/DECL_EXTERNAL decls because other TUs may take the address. I want the check to be replaced with sth more symtab aware, that is, bring in benefits from LTO (and at the same time be not confused by statics brought public with hidden visibility). I see, are you sure you need to ignore TREE_ADDRESSABLE for PUBLIC/EXTERNAL? I belive frontends are resposible to set them for all data that may have address taken (even externally) and IPA code maintains it - we clear the flagonly for variables that are static. Not sure - we've always not trusted this flag on public/externals. Probably because there could be aliases to them that have their address taken? (well, that's true for locals as well ...) That said, having a varpool way of querying whether a decl may be aliased by a pointer would be nice (with bonus points of handling the alias case). Or we can just set the flag to true for externally visible vars ealry in IPA queue if this is false. Well, let me do a simple check (remove the restriction and bootstrap/test). Richard. Honza Richard. Honza
[PATCH] Adding myself to Write After Approval in MAINTAINERS
Hi all, I forgot to add myself to the MAINTAINERS file when I got Write After Approval access. This patch does just this (already commited as specified on the website): diff --git a/ChangeLog b/ChangeLog index d35b315..80e9600 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,7 @@ +2014-06-04 Thomas Preud'homme thomas.preudho...@arm.com + + * MAINTAINERS (Write After Approval): Add myself. + 2014-06-03 Andrew Bennett andrew.benn...@imgtec.com * MAINTAINERS (Write After Approval): Add myself. diff --git a/MAINTAINERS b/MAINTAINERS index 12c123d..71206e7 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -501,6 +501,7 @@ Paul Pluzhnikov ppluzhni...@google.com Marek Polacek pola...@redhat.com Antoniu Popantoniu@gmail.com Vidya Praveen vidyaprav...@arm.com +Thomas Preud'homme thomas.preudho...@arm.com Vladimir Prus vladi...@codesourcery.com Yao Qi y...@codesourcery.com Jerry Quinnjlqu...@optonline.net
Re: [PATCH] Fix PR61306: improve handling of sign and cast in bswap
On Wed, Jun 4, 2014 at 7:59 AM, Thomas Preud'homme
thomas.preudho...@arm.com wrote:
When bswap replace a bitwise expression involving a memory source by a load
possibly followed by a bswap, it is possible that the load has a size smaller
than that of the target expression where the bitwise expression was affected.
So some sort of cast is needed. But there might also be a difference between
the size of the expression that was affected and the size of the load. So 3
different sizes might be involved. Consider the following example from
binutils:
bfd_vma
bfd_getl16 (const void *p)
{
const bfd_byte *addr = (*const bfd_byte *) p;
return (addr[1] 8) | addr[0];
}
Here the load will have a size of 16 bits, while the bitwise expression is an
int (don't ask me why) but is returned as a 64 bits quantity (bfd_vma maps to
the size of host registers). In this case we need 2 separate cast. One from
16 bit quantity to int with zero extension as the high bits are 0. It is
always a zero extension because bswap will not do anything in the presence of
a sign extension as depending on the initial value the result would be
different (maybe a bswap if positive number and random value if negative
number). Then, we need to cast respecting the extension that would have
happen had we not replaced the bitwise extension. Here since the bitwise
expression is int, it means we sign extend and then consider the content as
being unsigned (bfd_vma is an unsigned quantity).
When a bswap is necessary we need to do this double cast after doing the
bswap as the bswap must be done on the loaded value since a that's the size
expected by the bswap builtin. Finally, this patch also forbit any sign
extension *in* the bitwise expression as the result of the expression would
then be unpredictable (depend on the initial value).
The patch works this way:
1) prevent size extension of a bitwise expression
2) record the type of the bitwise expression instead of its size (the size
can be determined from the type)
3) use this type to perform a double cast as explained above
2014-05-30 Thomas Preud'homme thomas.preudho...@arm.com
PR tree-optimization/61306
* tree-ssa-math-opts.c (struct symbolic_number): Store type of
expression instead of its size.
(do_shift_rotate): Adapt to change in struct symbolic_number.
(verify_symbolic_number_p): Likewise.
(init_symbolic_number): Likewise.
(find_bswap_or_nop_1): Likewise. Also prevent optimization when the
result of the expressions is unpredictable due to sign extension.
(convert_via): New function to deal with the casting involved from the
loaded value to the target SSA.
(bswap_replace): Rename load_type in range_type to reflect it's the
type the memory accessed shall have before being casted. Select load
type according to whether a bswap needs to be done. Cast first to rhs
with zero extend and then to lhs with sign extend to keep semantic of
original stmts.
(pass_optimize_bswap::execute): Adapt to change in struct
symbolic_number. Decide if the range accessed should be signed or
unsigned before being casted to lhs type based on rhs type and size.
2014-05-29 Thomas Preud'homme thomas.preudho...@arm.com
* gcc.c-torture/execute/pr61306.c: New test.
Patch is in attachment. Is this ok for trunk?
I'd rather change the comparisons
- if (n-size (int)sizeof (int64_t))
-n-n = ((uint64_t)1 (n-size * BITS_PER_UNIT)) - 1;
+ if (bitsize / BITS_PER_UNIT (int)sizeof (int64_t))
+n-n = ((uint64_t)1 bitsize) - 1;
to work in bits, thus bitsize 8 * sizeof (int64_t) (note that using
BITS_PER_UNIT is bogus here - you are dealing with 8-bit bytes
on the host, not whatever the target uses). Otherwise it smells
like truncation may lose bits (probably not in practice).
+ /* Sign extension: result is dependent on the value. */
+ if (!TYPE_UNSIGNED (type) !TYPE_UNSIGNED (n-type)
+type_size TYPE_PRECISION (n-type))
+ return NULL_TREE;
whether it's sign-extended depends solely on the sign of the
converted entity, so I'm not sure why you are restricting this
to signed n-type. Consider
signed char *p;
((unsigned int)p[0]) 8 | ((unsigned int)p[1]) 16
the loads are still sign-extended but n-type is unsigned.
I'm failing to get why you possibly need two casts ... you should
only need one, from the bswap/load result to the final type
(zero-extended as you say - so the load type should simply be
unsigned which it is already).
So I think that the testcase in the patch is fixed already by
doing the n-type change (and a proper sign-extension detection).
Can you please split that part out?
That range_type and convert_via looks wrong and unnecessary to me,
and it doesn't look like you have a testcase excercising it?
Thanks,
Richard.
Re: [PATCH] Fix PR61320: disable bswap for unaligned access on SLOW_UNALIGNED_ACCESS targets
On Wed, Jun 4, 2014 at 8:07 AM, Thomas Preud'homme thomas.preudho...@arm.com wrote: Hi there, It seems from PR61320 that the bswap pass causes some problems when it replaces an OR expression by an unaligned access. Although it's not clear yet why the unaligned load does not go through the extract_bit_field codepath, it is necessary to provide a solution as this prevent sparc from bootstrapping. This patch takes the simple approach of cancelling the bswap optimization when the load that would replace the OR expression would be an unaligned load and the target has SLOW_UNALIGNED_ACCESS. In the long run this patch should be reverted as soon as the root cause of the current problem is found. The patch also rewrite the test to take into account the fact that the optimization is not done for some target. It also add some alignment hint so that more tests can be run even on STRICT_ALIGNMENT targets. ChangeLog changes are: *** gcc/ChangeLog *** 2014-06-03 Thomas Preud'homme thomas.preudho...@arm.com PR tree-optimization/61320 * tree-ssa-math-opts.c (bswap_replace): Cancel bswap optimization when load is unaligned and would be slow for this target. *** gcc/testsuite/ChangeLog *** 2014-06-03 Thomas Preud'homme thomas.preudho...@arm.com * gcc.dg/optimize-bswaphi-1.c: Make variables global when possible to enforce correct alignment and make the test work better on STRICT_ALIGNMENT targets. Also adjust dg-final selectors when alignment cannot be controlled (read_*_3 ()). * gcc.dg/optimize-bswapsi-2.c: Likewise. * gcc.dg/optimize-bswapdi-3.c: Likewise. Bootstrapped on x86_64-linux-gnu and no regression found in the testsuite. Patch is in attachment. It applies on top of the one for PR61306 in the email titled [PATCH] Fix PR61306: improve handling of sign and cast in bswap but can be trivially modified to apply directly on trunk should that patch (PR61306) need to be improved. Is this ok for trunk? I'd rather wait for the other bug to be fixed properly (given this patch is ontop of that). Btw, + if (align GET_MODE_ALIGNMENT (TYPE_MODE (load_type)) + SLOW_UNALIGNED_ACCESS (TYPE_MODE (load_type), align)) return false; goes to the next line. I too belive that presenting a wider possibly unaligned load to the expander is good and gives it a chance to produce optimal code for the target. I think that PR61320 is probably a dup of PR61306. Still adding the align GET_MODE_ALIGNMENT (TYPE_MODE (load_type)) condition is correct here, aligned loads are fine even in the bswap code-path. Richard. Best regards, Thomas
Re: [PATCH, i386, Pointer Bounds Checker 10/x] Partitions
On Tue, Jun 3, 2014 at 11:24 PM, Jeff Law l...@redhat.com wrote: On 06/02/14 05:41, Richard Biener wrote: this should be all moved to the symbol table level. (and IDENTIFIER_NODE shouldn't have to have tree_common.chain and thus become smaller). Which ought to be independent of the pointer checking work. There's been some proposals for making first class symbol tables in GCC, but nothing that's made any progress at this point. Maybe I'll sick David on it at some point. You probably missed that we now have such symbol table (ok, it still lacks labels and CONST_DECLs). Richard. jeff
Re: [PATCH] Fix PR54733 Optimize endian independent load/store
On Wed, Jun 4, 2014 at 9:04 AM, Thomas Preud'homme thomas.preudho...@arm.com wrote: From: Christophe Lyon [mailto:christophe.l...@linaro.org] On 29 May 2014 11:58, Thomas Preud'homme thomas.preudho...@arm.com wrote: Does the patch solve the problem you had? What about you Christophe? Hi Thomas, After a quick test, it looks OK to me. Great. What about you Andreas? Does it work fine for you? If yes, is this ok for trunk? Ok. Thanks, Richard. Best regards, Thomas
Re: [PATCH, Pointer Bounds Checker 6/x] New static constructor types
On Wed, Apr 16, 2014 at 2:33 PM, Ilya Enkovich enkovich@gmail.com wrote:
Hi,
This patch add new static constructor types used by Pointer Bounds Checker.
It was approved earlier for 4.9 and I'll assume patch is OK for trunk if no
objections arise.
Patch was bootstrapped and tested for linux-x86_64.
Thanks,
Ilya
--
gcc/
2014-04-16 Ilya Enkovich ilya.enkov...@intel.com
* ipa.c (cgraph_build_static_cdtor_1): Support contructors
with chkp ctor and bnd_legacy attributes.
* gimplify.c (gimplify_init_constructor): Avoid infinite
loop during gimplification of bounds initializer.
diff --git a/gcc/gimplify.c b/gcc/gimplify.c
index 7441784..67ab515 100644
--- a/gcc/gimplify.c
+++ b/gcc/gimplify.c
@@ -3803,10 +3803,19 @@ gimplify_init_constructor (tree *expr_p, gimple_seq
*pre_p, gimple_seq *post_p,
individual element initialization. Also don't do this for small
all-zero initializers (which aren't big enough to merit
clearing), and don't try to make bitwise copies of
- TREE_ADDRESSABLE types. */
+ TREE_ADDRESSABLE types.
+
+ We cannot apply such transformation when compiling chkp static
+ initializer because creation of initializer image in the memory
+ will require static initialization of bounds for it. It should
+ result in another gimplification of similar initializer and we
+ may fall into infinite loop. */
if (valid_const_initializer
!(cleared || num_nonzero_elements == 0)
-!TREE_ADDRESSABLE (type))
+!TREE_ADDRESSABLE (type)
+(!current_function_decl
+ || !lookup_attribute (chkp ctor,
+ DECL_ATTRIBUTES
(current_function_decl
Simply make the type TREE_ADDRESSABLE?
{
HOST_WIDE_INT size = int_size_in_bytes (type);
unsigned int align;
diff --git a/gcc/ipa.c b/gcc/ipa.c
index 26e9b03..5ab3aed 100644
--- a/gcc/ipa.c
+++ b/gcc/ipa.c
@@ -1345,9 +1345,11 @@ make_pass_ipa_whole_program_visibility (gcc::context
*ctxt)
}
/* Generate and emit a static constructor or destructor. WHICH must
- be one of 'I' (for a constructor) or 'D' (for a destructor). BODY
- is a STATEMENT_LIST containing GENERIC statements. PRIORITY is the
- initialization priority for this constructor or destructor.
+ be one of 'I' (for a constructor), 'D' (for a destructor), 'P'
+ (for chp static vars constructor) or 'B' (for chkp static bounds
+ constructor). BODY is a STATEMENT_LIST containing GENERIC
+ statements. PRIORITY is the initialization priority for this
+ constructor or destructor.
FINAL specify whether the externally visible name for collect2 should
be produced. */
@@ -1406,6 +1408,20 @@ cgraph_build_static_cdtor_1 (char which, tree body,
int priority, bool final)
DECL_STATIC_CONSTRUCTOR (decl) = 1;
decl_init_priority_insert (decl, priority);
break;
+case 'P':
+ DECL_STATIC_CONSTRUCTOR (decl) = 1;
+ DECL_ATTRIBUTES (decl) = tree_cons (get_identifier (chkp ctor),
+ NULL,
+ NULL_TREE);
Ick. Please try to avoid using attributes for this. Rather adjust the
caller of this function to set a flag in the cgraph node.
So I don't like this patch at all.
Richard.
+ decl_init_priority_insert (decl, priority);
+ break;
+case 'B':
+ DECL_STATIC_CONSTRUCTOR (decl) = 1;
+ DECL_ATTRIBUTES (decl) = tree_cons (get_identifier (bnd_legacy),
+ NULL,
+ NULL_TREE);
+ decl_init_priority_insert (decl, priority);
+ break;
case 'D':
DECL_STATIC_DESTRUCTOR (decl) = 1;
decl_fini_priority_insert (decl, priority);
@@ -1423,9 +1439,11 @@ cgraph_build_static_cdtor_1 (char which, tree body,
int priority, bool final)
}
/* Generate and emit a static constructor or destructor. WHICH must
- be one of 'I' (for a constructor) or 'D' (for a destructor). BODY
- is a STATEMENT_LIST containing GENERIC statements. PRIORITY is the
- initialization priority for this constructor or destructor. */
+ be one of 'I' (for a constructor), 'D' (for a destructor), 'P'
+ (for chkp static vars constructor) or 'B' (for chkp static bounds
+ constructor). BODY is a STATEMENT_LIST containing GENERIC
+ statements. PRIORITY is the initialization priority for this
+ constructor or destructor. */
void
cgraph_build_static_cdtor (char which, tree body, int priority)
Re: [PATCH, Pointer Bounds Checker 13/x] Early versioning
On Wed, Jun 4, 2014 at 8:46 AM, Jeff Law l...@redhat.com wrote:
On 06/03/14 03:29, Richard Biener wrote:
On Tue, Jun 3, 2014 at 7:55 AM, Ilya Enkovich enkovich@gmail.com
wrote:
2014-06-02 21:27 GMT+04:00 Jeff Law l...@redhat.com:
On 06/02/14 04:48, Ilya Enkovich wrote:
Hmm, so if I understand things correctly, src_fun has no loop
structures attached, thus there's nothing to copy. Presumably at
some later point we build loop structures for the copy from scratch?
I suppose it is just a simple bug with absent NULL pointer check. Here
is
original code:
/* Duplicate the loop tree, if available and wanted. */
if (loops_for_fn (src_cfun) != NULL
current_loops != NULL)
{
copy_loops (id, entry_block_map-loop_father,
get_loop (src_cfun, 0));
/* Defer to cfgcleanup to update loop-father fields of
basic-blocks. */
loops_state_set (LOOPS_NEED_FIXUP);
}
/* If the loop tree in the source function needed fixup, mark the
destination loop tree for fixup, too. */
if (loops_for_fn (src_cfun)-state LOOPS_NEED_FIXUP)
loops_state_set (LOOPS_NEED_FIXUP);
As you may see we have check for absent loops structure in the first
if-statement and no check in the second one. I hit segfault and added
the
check.
Downthread you indicated you're not in SSA form which might explain the
inconsistency here. If so, then we need to make sure that the loop df
structures do get set up properly later.
That is what init_data_structures pass will do for us as Richard pointed.
Right?
loops are set up during the CFG construction and thus are available
everywhere.
Which would argue that the hunk that checks for the loop tree's existence
before accessing it should not be needed. Ilya -- is it possible you hit
this prior to Richi's work to build the loop structures as part of CFG
construction and maintain them throughout compilation.
That's likely. It's still on my list of janitor things to do to remove all
those if (current_loops) checks ...
the df structures are set up in init_data_structures pass which is run
before
going into SSA form (I'd like to somehow cleanup that area).
OK. So this part should be approved since we've established this code is
running prior to going into SSA form.
Agreed.
Thanks,
Richard.
jeff
Re: PR61385: phiopt drops some PHIs
On Wed, Jun 4, 2014 at 9:59 AM, Jeff Law l...@redhat.com wrote: On 06/04/14 01:46, Marc Glisse wrote: On Tue, 3 Jun 2014, Jeff Law wrote: On 06/03/14 08:08, Richard Biener wrote: All arguments get the same value (and the PHI in middle-bb is surely a singleton?), so it's way better to re-materialize the PHI as a gimple assignment at the start of the basic block. If they are singletons (which I expect), the easiest way is to propagate their single argument into all uses and simply remove them. I was all for propagating, but replace_uses_by calls fold_stmt on the using statements (that's normal). If I first move the statement and then replace, I may call fold_stmt on a stmt that isn't dominated by the defs of all its arguments. If I first replace, the statement I am supposed to move may have been modified so I need to find out what happened to it. In the end, the assignment seems easier and safer (and not too bad since this case almost never happens in practice). We certainly want to get rid of them :-) I'd start by finding out which pass left the degenerate, ensure it's not marked as SSA_NAME_OCCURS_IN_ABNORMAL_PHI, then propagate it away. If it's a systemic problem that a particular pass can leave degenerate PHIs, then you might want to schedule the phi-only propagator to run after that pass. It does const/copy propagation seeding from degenerate PHIs, so it ought to be very fast. This one comes from VRP2 apparently. But it isn't the only pass that produces singleton phis. I didn't look at them closely, but I assume LIM is normal, DOM maybe less so, etc. DOM VRP can create them via jump threading. Right now we only fire up the pass_phi_only_cprop after DOM though. Right, if we want to get rid of the degenerates early we have to run pass_phi_only_cprop after VRP as well. DOM runs not too long after LIM and it will propagate away the degenerates as well. Not sure where LIM would create degenerate PHIs, but I guess you simply see loop-closed PHIs here which are degenerate. One could argue that propagating away the degenerates ought to occur immediately before any phi-opt pass. I'd say we should run phi_only_cprop after the first VRPs instead and move the 2nd VRP before the existing phi_only_cprop pass. I'll do the appropriate change. bootstrap+testsuite on x86_64-linux-gnu as usual. 2014-06-04 Marc Glisse marc.gli...@inria.fr PR tree-optimization/61385 gcc/ * tree-ssa-phiopt.c (value_replacement): Replace singleton PHIs with assignments. gcc/testsuite/ * gcc.dg/tree-ssa/pr61385.c: New file. Don't you have to verify neither the source nor the destination are marked with SSA_NAME_OCCURS_IN_ABNORMAL_PHI? And if so, you can't propagate away the PHI (which may mean we need to avoid the optimization in that case). I also believe that when turning a PHI into an assignment you have to preserve the parallel nature of the PHI. If the result of one PHI is used as an input in another PHI, then you have to work harder (see the out-of-ssa code). Perhaps this can never be the case with a degenerate. Hmm, indeed that can happen. So I'd say we should instead simply bail out if the middle-bb has a PHI node. Simple and safe. And with the pass ordering fixes above we shouldn't see degenerates anyway. Richard. Jeff
[PATCH][1/2] Improve DSE
This first patch improves DSE by improving the handling of
references with non-invariant addresses such as
a-b[i].c in stmt_kills_ref_p_1.
Bootstrapped on x86_64-unknown-linux-gnu, testing in progress.
Richard.
2014-06-04 Richard Biener rguent...@suse.de
* tree-ssa-alias.c (stmt_may_clobber_ref_p): Improve handling
of accesses with non-invariant address.
* gcc.dg/tree-ssa/ssa-dse-16.c: New testcase.
Index: gcc/tree-ssa-alias.c
===
*** gcc/tree-ssa-alias.c(revision 211213)
--- gcc/tree-ssa-alias.c(working copy)
*** stmt_may_clobber_ref_p (gimple stmt, tre
*** 2174,2184
static bool
stmt_kills_ref_p_1 (gimple stmt, ao_ref *ref)
{
! /* For a must-alias check we need to be able to constrain
! the access properly.
! FIXME: except for BUILTIN_FREE. */
! if (!ao_ref_base (ref)
! || ref-max_size == -1)
return false;
if (gimple_has_lhs (stmt)
--- 2174,2180
static bool
stmt_kills_ref_p_1 (gimple stmt, ao_ref *ref)
{
! if (!ao_ref_base (ref))
return false;
if (gimple_has_lhs (stmt)
*** stmt_kills_ref_p_1 (gimple stmt, ao_ref
*** 2191,2199
might throw as well. */
!stmt_can_throw_internal (stmt))
{
! tree base, lhs = gimple_get_lhs (stmt);
HOST_WIDE_INT size, offset, max_size, ref_offset = ref-offset;
! base = get_ref_base_and_extent (lhs, offset, size, max_size);
/* We can get MEM[symbol: sZ, index: D.8862_1] here,
so base == ref-base does not always hold. */
if (base != ref-base)
--- 2187,2237
might throw as well. */
!stmt_can_throw_internal (stmt))
{
! tree lhs = gimple_get_lhs (stmt);
! /* If LHS is literally a base of the access we are done. */
! if (ref-ref)
! {
! tree base = ref-ref;
! if (handled_component_p (base))
! {
! tree saved_lhs0 = NULL_TREE;
! if (handled_component_p (lhs))
! {
! saved_lhs0 = TREE_OPERAND (lhs, 0);
! TREE_OPERAND (lhs, 0) = integer_zero_node;
! }
! do
! {
! /* Just compare the outermost handled component, if
!they are equal we have found a possible common
!base. */
! tree saved_base0 = TREE_OPERAND (base, 0);
! TREE_OPERAND (base, 0) = integer_zero_node;
! bool res = operand_equal_p (lhs, base, 0);
! TREE_OPERAND (base, 0) = saved_base0;
! if (res)
! break;
! /* Otherwise drop handled components of the access. */
! base = saved_base0;
! }
! while (handled_component_p (base));
! if (saved_lhs0)
! TREE_OPERAND (lhs, 0) = saved_lhs0;
! }
! /* Finally check if lhs is equal or equal to the base candidate
!of the access. */
! if (operand_equal_p (lhs, base, 0))
! return true;
! }
!
! /* Now look for non-literal equal bases with the restriction of
! handling constant offset and size. */
! /* For a must-alias check we need to be able to constrain
!the access properly. */
! if (ref-max_size == -1)
! return false;
HOST_WIDE_INT size, offset, max_size, ref_offset = ref-offset;
! tree base = get_ref_base_and_extent (lhs, offset, size, max_size);
/* We can get MEM[symbol: sZ, index: D.8862_1] here,
so base == ref-base does not always hold. */
if (base != ref-base)
*** stmt_kills_ref_p_1 (gimple stmt, ao_ref
*** 2261,2266
--- 2299,2308
case BUILT_IN_MEMMOVE_CHK:
case BUILT_IN_MEMSET_CHK:
{
+ /* For a must-alias check we need to be able to constrain
+the access properly. */
+ if (ref-max_size == -1)
+ return false;
tree dest = gimple_call_arg (stmt, 0);
tree len = gimple_call_arg (stmt, 2);
if (!tree_fits_shwi_p (len))
Index: gcc/testsuite/gcc.dg/tree-ssa/ssa-dse-16.c
===
--- gcc/testsuite/gcc.dg/tree-ssa/ssa-dse-16.c (revision 0)
+++ gcc/testsuite/gcc.dg/tree-ssa/ssa-dse-16.c (working copy)
@@ -0,0 +1,18 @@
+/* { dg-do compile } */
+/* { dg-options -O2 -fdump-tree-dse1-details } */
+
+struct X { struct A { int a[2]; } b[10]; };
+void foo (struct X *x, int i)
+{
+ struct A a;
+ /* Confuse SRA here with using a variable index, otherwise it will mess
+with the IL too much. */
+ a.a[i] = 3;
+ a.a[1] = 0;
+ /* The following store is dead. */
+ x-b[i].a[0] = 1;
+ x-b[i] = a;
+}
+
+/* { dg-final { scan-tree-dump Deleted dead store dse1 } } */
RE: [PATCH] Fix PR61306: improve handling of sign and cast in bswap
From: Richard Biener [mailto:richard.guent...@gmail.com] I'd rather change the comparisons - if (n-size (int)sizeof (int64_t)) -n-n = ((uint64_t)1 (n-size * BITS_PER_UNIT)) - 1; + if (bitsize / BITS_PER_UNIT (int)sizeof (int64_t)) +n-n = ((uint64_t)1 bitsize) - 1; to work in bits, thus bitsize 8 * sizeof (int64_t) (note that using BITS_PER_UNIT is bogus here - you are dealing with 8-bit bytes on the host, not whatever the target uses). Otherwise it smells like truncation may lose bits (probably not in practice). Ah yes, right. + /* Sign extension: result is dependent on the value. */ + if (!TYPE_UNSIGNED (type) !TYPE_UNSIGNED (n-type) +type_size TYPE_PRECISION (n-type)) + return NULL_TREE; whether it's sign-extended depends solely on the sign of the converted entity, so I'm not sure why you are restricting this to signed n-type. Consider signed char *p; ((unsigned int)p[0]) 8 | ((unsigned int)p[1]) 16 the loads are still sign-extended but n-type is unsigned. Indeed, I understood it for convert_via (the requirement to be unsigned) but got it wrong here. I'm failing to get why you possibly need two casts ... you should only need one, from the bswap/load result to the final type (zero-extended as you say - so the load type should simply be unsigned which it is already). Because of the type of the shift constant, the bitwise expression is usually of type int. However, if you write a function that does a 32 bit load in host endianness (like a 32 bit little endian load on x86_64) with a result of size 64 bits, then you need to sign extend, since the source type is signed. This is a situation I encountered in bfd_getl32 in binutils I think. Now if you consider bfd_getl16 instead a direct sign extension is out of the question. Suppose bfd_getl16 is called to read from a memory address that contains 0xff 0xfe. The bitwise expression would thus be equivalent to the value 0xfeff since it's of type int. Then after the sign extension to 64bits you'd have 0xfeff. But after replacing the bitwise expression you end up with a 16bit load into a 16bit SSA variable. If you sign extend that directly to 64 bits you'll end up with 0xfeff which is wrong. But if you zero extend to an int value (the type of the bitwise OR expression) and then sign extend to the target type you'll have the correct result. But you're right, we can do simpler by sign extending if load size == size of bitwise expression and zero extend else. The change of load_type to range_type would still be needed because in case of a load + bswap it's better to load in the same type as the type of the parameter of bswap. After bswap you'd need to convert to a signed or unsigned value according to the logic above (load size == size of bitwise expression) In the original statement, the bitwise OR expression would have the 2 bytes of higher weight be 0 while the 2 bytes of lower weight would be the value read from memory. The result of the sign extension would be So I think that the testcase in the patch is fixed already by doing the n-type change (and a proper sign-extension detection). I don't remember exactly but I think it didn't fix this bug (but it is a necessary fix anyway). Can you please split that part out? Sure, that part would need to be applied on gcc 4.9 too. I'll try to construct a testcase for that. That range_type and convert_via looks wrong and unnecessary to me, and it doesn't look like you have a testcase excercising it? See above.
[PATCH][2/2] Improve DSE
This improves DSE by not stopping to look for a killing stmt at
the first may-alias but instead continue looking for a kill
(well, until we hit a may-use or run into limitations of the walker).
This fixes some embarrassing missing dead store eliminations
(it's also more expensive, but the walking is limited thus
it's O(1) anyway (haha)).
Bootstrapped on x86_64-unknown-linux-gnu, testing in progress.
Richard.
2014-06-04 Richard Biener rguent...@suse.de
PR tree-optimization/60098
* tree-ssa-dse.c (dse_possible_dead_store_p): Walk until
we hit a kill.
(dse_optimize_stmt): Simplify, now that we found a kill
earlier.
* gcc.dg/tree-ssa/ssa-dse-15.c: New testcase.
Index: gcc/tree-ssa-dse.c
===
*** gcc/tree-ssa-dse.c.orig 2014-05-06 16:15:43.030962178 +0200
--- gcc/tree-ssa-dse.c 2014-06-04 10:12:33.584954818 +0200
*** dse_possible_dead_store_p (gimple stmt,
*** 198,207
break;
}
}
! /* We deliberately stop on clobbering statements and not only on
! killing ones to make walking cheaper. Otherwise we can just
! continue walking until both stores have equal reference trees. */
! while (!stmt_may_clobber_ref_p (temp, gimple_assign_lhs (stmt)));
*use_stmt = temp;
--- 198,205
break;
}
}
! /* Continue walking until we reach a kill. */
! while (!stmt_kills_ref_p (temp, gimple_assign_lhs (stmt)));
*use_stmt = temp;
*** dse_optimize_stmt (gimple_stmt_iterator
*** 248,304
if (!dse_possible_dead_store_p (stmt, use_stmt))
return;
/* But only remove *this_2(D) ={v} {CLOBBER} if killed by
another clobber stmt. */
if (gimple_clobber_p (stmt)
!gimple_clobber_p (use_stmt))
return;
! /* If we have precisely one immediate use at this point and the
!stores are to the same memory location or there is a chain of
!virtual uses from stmt and the stmt which stores to that same
!memory location, then we may have found redundant store. */
! if ((gimple_has_lhs (use_stmt)
! (operand_equal_p (gimple_assign_lhs (stmt),
! gimple_get_lhs (use_stmt), 0)))
! || stmt_kills_ref_p (use_stmt, gimple_assign_lhs (stmt)))
! {
! basic_block bb;
! /* If use_stmt is or might be a nop assignment, e.g. for
!struct { ... } S a, b, *p; ...
!b = a; b = b;
!or
!b = a; b = *p; where p might be b,
!or
!*p = a; *p = b; where p might be b,
!or
!*p = *u; *p = *v; where p might be v, then USE_STMT
!acts as a use as well as definition, so store in STMT
!is not dead. */
! if (stmt != use_stmt
! ref_maybe_used_by_stmt_p (use_stmt, gimple_assign_lhs (stmt)))
! return;
!
! if (dump_file (dump_flags TDF_DETAILS))
! {
! fprintf (dump_file, Deleted dead store ');
! print_gimple_stmt (dump_file, gsi_stmt (*gsi), dump_flags, 0);
! fprintf (dump_file, '\n);
! }
!
! /* Then we need to fix the operand of the consuming stmt. */
! unlink_stmt_vdef (stmt);
!
! /* Remove the dead store. */
! bb = gimple_bb (stmt);
! if (gsi_remove (gsi, true))
! bitmap_set_bit (need_eh_cleanup, bb-index);
!
! /* And release any SSA_NAMEs set in this statement back to the
!SSA_NAME manager. */
! release_defs (stmt);
}
}
}
--- 246,294
if (!dse_possible_dead_store_p (stmt, use_stmt))
return;
+ /* Now we know that use_stmt kills the LHS of stmt. */
+
/* But only remove *this_2(D) ={v} {CLOBBER} if killed by
another clobber stmt. */
if (gimple_clobber_p (stmt)
!gimple_clobber_p (use_stmt))
return;
! basic_block bb;
!
! /* If use_stmt is or might be a nop assignment, e.g. for
! struct { ... } S a, b, *p; ...
! b = a; b = b;
!or
! b = a; b = *p; where p might be b,
!or
!*p = a; *p = b; where p might be b,
!or
!*p = *u; *p = *v; where p might be v, then USE_STMT
! acts as a use as well as definition, so store in STMT
! is not dead. */
! if (stmt != use_stmt
! ref_maybe_used_by_stmt_p (use_stmt, gimple_assign_lhs (stmt)))
! return;
! if (dump_file (dump_flags TDF_DETAILS))
! {
! fprintf (dump_file, Deleted dead store ');
! print_gimple_stmt (dump_file, gsi_stmt (*gsi), dump_flags, 0);
! fprintf (dump_file, '\n);
}
+
+ /* Then we need to fix the operand of the consuming stmt. */
+
Re: [PATCH] Fix PR61306: improve handling of sign and cast in bswap
On Wed, Jun 4, 2014 at 12:42 PM, Thomas Preud'homme thomas.preudho...@arm.com wrote: From: Richard Biener [mailto:richard.guent...@gmail.com] I'd rather change the comparisons - if (n-size (int)sizeof (int64_t)) -n-n = ((uint64_t)1 (n-size * BITS_PER_UNIT)) - 1; + if (bitsize / BITS_PER_UNIT (int)sizeof (int64_t)) +n-n = ((uint64_t)1 bitsize) - 1; to work in bits, thus bitsize 8 * sizeof (int64_t) (note that using BITS_PER_UNIT is bogus here - you are dealing with 8-bit bytes on the host, not whatever the target uses). Otherwise it smells like truncation may lose bits (probably not in practice). Ah yes, right. + /* Sign extension: result is dependent on the value. */ + if (!TYPE_UNSIGNED (type) !TYPE_UNSIGNED (n-type) +type_size TYPE_PRECISION (n-type)) + return NULL_TREE; whether it's sign-extended depends solely on the sign of the converted entity, so I'm not sure why you are restricting this to signed n-type. Consider signed char *p; ((unsigned int)p[0]) 8 | ((unsigned int)p[1]) 16 the loads are still sign-extended but n-type is unsigned. Indeed, I understood it for convert_via (the requirement to be unsigned) but got it wrong here. I'm failing to get why you possibly need two casts ... you should only need one, from the bswap/load result to the final type (zero-extended as you say - so the load type should simply be unsigned which it is already). Because of the type of the shift constant, the bitwise expression is usually of type int. However, if you write a function that does a 32 bit load in host endianness (like a 32 bit little endian load on x86_64) with a result of size 64 bits, then you need to sign extend, since the source type is signed. This is a situation I encountered in bfd_getl32 in binutils I think. Now if you consider bfd_getl16 instead a direct sign extension is out of the question. Suppose bfd_getl16 is called to read from a memory address that contains 0xff 0xfe. The bitwise expression would thus be equivalent to the value 0xfeff since it's of type int. Then after the sign extension to 64bits you'd have 0xfeff. But after replacing the bitwise expression you end up with a 16bit load into a 16bit SSA variable. If you sign extend that directly to 64 bits you'll end up with 0xfeff which is wrong. But if you zero extend to an int value (the type of the bitwise OR expression) and then sign extend to the target type you'll have the correct result. Err, but if you zero-extend directly to the target type you have the correct result, too. But you're right, we can do simpler by sign extending if load size == size of bitwise expression and zero extend else. The change of load_type to range_type would still be needed because in case of a load + bswap it's better to load in the same type as the type of the parameter of bswap. After bswap you'd need to convert to a signed or unsigned value according to the logic above (load size == size of bitwise expression) In the original statement, the bitwise OR expression would have the 2 bytes of higher weight be 0 while the 2 bytes of lower weight would be the value read from memory. The result of the sign extension would be So I think that the testcase in the patch is fixed already by doing the n-type change (and a proper sign-extension detection). I don't remember exactly but I think it didn't fix this bug (but it is a necessary fix anyway). Can you please split that part out? Sure, that part would need to be applied on gcc 4.9 too. I'll try to construct a testcase for that. That range_type and convert_via looks wrong and unnecessary to me, and it doesn't look like you have a testcase excercising it? See above. But nothing for the testsuite? The testcase you add fails foul of sign-extending the loads. Richard.
Re: [build, driver] RFC: Support compressed debug sections
On Jun 4, 2014, at 1:54 AM, Rainer Orth r...@cebitec.uni-bielefeld.de wrote: Mike Stump mikest...@comcast.net writes: On Jun 3, 2014, at 3:40 AM, Rainer Orth r...@cebitec.uni-bielefeld.de wrote: It's been another week, and I still need approval for the build, doc, and Darwin changes: So, the darwin bits look trivial enough, if the entire scheme is what people want to do. The darwin bits are Ok.
RE: [PATCH] Fix PR61306: improve handling of sign and cast in bswap
From: Richard Biener [mailto:richard.guent...@gmail.com] Err, but if you zero-extend directly to the target type you have the correct result, too. Yep but in some case we need sign extend (32 bit bitwise OR stored into 64 bit result). As I said, the logic could be simplified by sign extending if load_size == bitwise expression size and zero extending if not true. I'll rework the patch in this direction. But nothing for the testsuite? The testcase you add fails foul of sign-extending the loads. Ack, I'll add a test for zero extension and one for sign extension. Cheers, Thomas
RE: [Patch, PR 60158] gcc/varasm.c : Pass actual alignment value to output_constant_pool_2
Ping.
I have changed the subject line accordingly.
Regards,
Rohit
-Original Message-
From: David Edelsohn [mailto:dje@gmail.com]
Sent: Thursday, May 08, 2014 9:28 PM
To: Dharmakan Rohit-B30502; Jakub Jelinek; Richard Biener
Cc: Alan Modra; gcc-patches@gcc.gnu.org; Wienskoski Edmar-RA8797
Subject: Re: [Patch, PR 60158] Generate .fixup sections for
.data.rel.ro.local entries.
Rohit,
The subject line and thread may confuse people that this is a PowerPC-
specific issue. You need approval from a reviewer with authority over
varasm.c.
Thanks, David
On Thu, May 8, 2014 at 9:54 AM, rohitarul...@freescale.com
rohitarul...@freescale.com wrote:
-Original Message-
From: Alan Modra [mailto:amo...@gmail.com]
Sent: Saturday, April 26, 2014 11:52 AM
To: Dharmakan Rohit-B30502
Cc: gcc-patches@gcc.gnu.org; dje@gmail.com; Wienskoski
Edmar-RA8797
Subject: Re: [Patch, PR 60158] Generate .fixup sections for
.data.rel.ro.local entries.
On Fri, Apr 25, 2014 at 02:57:38PM +, rohitarul...@freescale.com
wrote:
Source file: gcc-4.8.2/gcc/varasm.c @@ -7120,7 +7120,7 @@
if (CONSTANT_POOL_ADDRESS_P (symbol))
{
desc = SYMBOL_REF_CONSTANT (symbol);
output_constant_pool_1 (desc, 1);
- (A)
offset += GET_MODE_SIZE (desc-mode);
I think the reason 1 is passed here for align is that with -fsection-
anchors, in output_object_block we've already laid out everything in
the block, assigning offsets from the start of the block. Aligning
shouldn't be necessary, because we've already done that.. OTOH, it
shouldn't hurt to align again.
Thanks. I have tested for both the cases on e500v2, e500mc, e5500,
ppc64 (GCC v4.8.2 branch) with no regressions.
Patch1 [gcc.fix_pr60158_fixup_table-fsf]: Pass actual alignment value
to output_constant_pool_2.
Patch2 [gcc.fix_pr60158_fixup_table-fsf-2]: Use the alignment data
available in the first argument (constant_descriptor_rtx) of
output_constant_pool_1.
(Note: this generates .align directive twice).
Is it ok to commit? Any comments?
Regards,
Rohit
Re: [PATCH] Fix PR61306: improve handling of sign and cast in bswap
On Wed, Jun 4, 2014 at 1:15 PM, Thomas Preud'homme thomas.preudho...@arm.com wrote: From: Richard Biener [mailto:richard.guent...@gmail.com] Err, but if you zero-extend directly to the target type you have the correct result, too. Yep but in some case we need sign extend (32 bit bitwise OR stored into 64 bit result). But you'd convert the 32bit bitwise OR to the original type anyway (no extension, but sign-change). The extension to the stored type happens with the original IL. As I said, the logic could be simplified by sign extending if load_size == bitwise expression size and zero extending if not true. I'll rework the patch in this direction. But nothing for the testsuite? The testcase you add fails foul of sign-extending the loads. Ack, I'll add a test for zero extension and one for sign extension. Thanks. Richard. Cheers, Thomas
Re: [PATCH][AARCH64]Support full addressing modes for ldr/str in vectorization scenarios
On 4 June 2014 10:06, Charles Baylis charles.bay...@linaro.org wrote: On 4 June 2014 03:11, Bin.Cheng amker.ch...@gmail.com wrote: Yes, If there is a PR, I can evaluate how this can help and ask release maintainer for approval. I'll reduce the test case and create one shortly I have created PR61411 with a reduced test case.
Re: PR61385: phiopt drops some PHIs
On Wed, 4 Jun 2014, Richard Biener wrote:
So I'd say we should instead simply bail out if the middle-bb has a PHI node.
Sounds good to me, so I am testing the mini-patch I had originally posted
to bugzilla:
2014-06-04 Marc Glisse marc.gli...@inria.fr
PR tree-optimization/61385
gcc/
* tree-ssa-phiopt.c (value_replacement): Punt if there are PHI nodes.
gcc/testsuite/
* gcc.dg/tree-ssa/pr61385.c: New file.
--
Marc GlisseIndex: gcc/testsuite/gcc.dg/tree-ssa/pr61385.c
===
--- gcc/testsuite/gcc.dg/tree-ssa/pr61385.c (revision 0)
+++ gcc/testsuite/gcc.dg/tree-ssa/pr61385.c (working copy)
@@ -0,0 +1,43 @@
+/* { dg-do compile } */
+/* { dg-options -O2 } */
+
+#define assert(x) if (!(x)) __builtin_abort ()
+
+int a, b, c, d, e, f, g;
+
+int
+fn1 ()
+{
+ int *h = c;
+ for (; c 1; c++)
+{
+ int *i = a, *k = a;
+ f = 0;
+ if (b)
+ return 0;
+ if (*h)
+ {
+ int **j = i;
+ *j = 0;
+ d = 0;
+ }
+ else
+ g = e = 0;
+ if (*h)
+ {
+ int **l = k;
+ *l = g;
+ }
+ d = *h;
+ assert (k == a || k);
+ assert (i);
+}
+ return 0;
+}
+
+int
+main ()
+{
+ fn1 ();
+ return 0;
+}
Index: gcc/tree-ssa-phiopt.c
===
--- gcc/tree-ssa-phiopt.c (revision 211221)
+++ gcc/tree-ssa-phiopt.c (working copy)
@@ -842,20 +842,24 @@ value_replacement (basic_block cond_bb,
/* Now optimize (x != 0) ? x + y : y to just y.
The following condition is too restrictive, there can easily be another
stmt in middle_bb, for instance a CONVERT_EXPR for the second argument.
*/
gimple assign = last_and_only_stmt (middle_bb);
if (!assign || gimple_code (assign) != GIMPLE_ASSIGN
|| gimple_assign_rhs_class (assign) != GIMPLE_BINARY_RHS
|| (!INTEGRAL_TYPE_P (TREE_TYPE (arg0))
!POINTER_TYPE_P (TREE_TYPE (arg0
return 0;
+ /* Punt if there are (degenerate) PHIs in middle_bb, there should not be. */
+ if (!gimple_seq_empty_p (phi_nodes (middle_bb)))
+return 0;
+
/* Only transform if it removes the condition. */
if (!single_non_singleton_phi_for_edges (phi_nodes (gimple_bb (phi)), e0,
e1))
return 0;
/* Size-wise, this is always profitable. */
if (optimize_bb_for_speed_p (cond_bb)
/* The special case is useless if it has a low probability. */
profile_status_for_fn (cfun) != PROFILE_ABSENT
EDGE_PRED (middle_bb, 0)-probability PROB_EVEN
/* If assign is cheap, there is no point avoiding it. */
Re: PR61385: phiopt drops some PHIs
On Wed, Jun 4, 2014 at 1:20 PM, Marc Glisse marc.gli...@inria.fr wrote:
On Wed, 4 Jun 2014, Richard Biener wrote:
So I'd say we should instead simply bail out if the middle-bb has a PHI
node.
Sounds good to me, so I am testing the mini-patch I had originally posted to
bugzilla:
Ok.
Thanks,
Richard.
2014-06-04 Marc Glisse marc.gli...@inria.fr
PR tree-optimization/61385
gcc/
* tree-ssa-phiopt.c (value_replacement): Punt if there are PHI
nodes.
gcc/testsuite/
* gcc.dg/tree-ssa/pr61385.c: New file.
--
Marc Glisse
Index: gcc/testsuite/gcc.dg/tree-ssa/pr61385.c
===
--- gcc/testsuite/gcc.dg/tree-ssa/pr61385.c (revision 0)
+++ gcc/testsuite/gcc.dg/tree-ssa/pr61385.c (working copy)
@@ -0,0 +1,43 @@
+/* { dg-do compile } */
+/* { dg-options -O2 } */
+
+#define assert(x) if (!(x)) __builtin_abort ()
+
+int a, b, c, d, e, f, g;
+
+int
+fn1 ()
+{
+ int *h = c;
+ for (; c 1; c++)
+{
+ int *i = a, *k = a;
+ f = 0;
+ if (b)
+ return 0;
+ if (*h)
+ {
+ int **j = i;
+ *j = 0;
+ d = 0;
+ }
+ else
+ g = e = 0;
+ if (*h)
+ {
+ int **l = k;
+ *l = g;
+ }
+ d = *h;
+ assert (k == a || k);
+ assert (i);
+}
+ return 0;
+}
+
+int
+main ()
+{
+ fn1 ();
+ return 0;
+}
Index: gcc/tree-ssa-phiopt.c
===
--- gcc/tree-ssa-phiopt.c (revision 211221)
+++ gcc/tree-ssa-phiopt.c (working copy)
@@ -842,20 +842,24 @@ value_replacement (basic_block cond_bb,
/* Now optimize (x != 0) ? x + y : y to just y.
The following condition is too restrictive, there can easily be
another
stmt in middle_bb, for instance a CONVERT_EXPR for the second
argument. */
gimple assign = last_and_only_stmt (middle_bb);
if (!assign || gimple_code (assign) != GIMPLE_ASSIGN
|| gimple_assign_rhs_class (assign) != GIMPLE_BINARY_RHS
|| (!INTEGRAL_TYPE_P (TREE_TYPE (arg0))
!POINTER_TYPE_P (TREE_TYPE (arg0
return 0;
+ /* Punt if there are (degenerate) PHIs in middle_bb, there should not be.
*/
+ if (!gimple_seq_empty_p (phi_nodes (middle_bb)))
+return 0;
+
/* Only transform if it removes the condition. */
if (!single_non_singleton_phi_for_edges (phi_nodes (gimple_bb (phi)), e0,
e1))
return 0;
/* Size-wise, this is always profitable. */
if (optimize_bb_for_speed_p (cond_bb)
/* The special case is useless if it has a low probability. */
profile_status_for_fn (cfun) != PROFILE_ABSENT
EDGE_PRED (middle_bb, 0)-probability PROB_EVEN
/* If assign is cheap, there is no point avoiding it. */
Re: Fix a function decl in gfortran
On 06/04/2014 09:40 AM, Tobias Burnus wrote: Still untested patch, but I cannot resist pointing out stupid typos by myself. I intent to tests the build and test the patch - and then to commit it as obvious. If you see problems with this approach please scream now. I have no idea about the approach, but I'll be testing it on our nvptx compiler (results probably late today or tomorrow). Thanks! Bernd
Re: [C++ Patch] PR 43453
Hi, On 05/26/2014 05:41 PM, Paolo Carlini wrote: ... the below should be better, handles correctly cv-qualifiers. Pinging this... Thanks! Paolo.
[PATCH][match-and-simplify] Restore bootstrap somewhat
The following patche is necessary to get to stage2 target library
building.
Bootstrapped until that point, applied.
Richard.
2014-06-04 Richard Biener rguent...@suse.de
* genmatch.c (error_cb, fatal_at): Annotate with printf
format attribute to silence warning.
* gimple-match-head.c (gimple_resimplify1): Check for
builtin availability and properly zero out excess operands.
(gimple_resimplify2): Likewise.
(gimple_resimplify3): Likewise.
(maybe_push_res_to_seq): Check for builtin availability.
(gimple_match_and_simplify): Likewise.
Index: gcc/genmatch.c
===
--- gcc/genmatch.c (revision 211221)
+++ gcc/genmatch.c (working copy)
@@ -549,6 +549,9 @@ write_gimple (FILE *f, vecsimplify *
static struct line_maps *line_table;
static bool
+#if GCC_VERSION = 4001
+__attribute__((format (printf, 6, 0)))
+#endif
error_cb (cpp_reader *, int, int, source_location location,
unsigned int, const char *msg, va_list *ap)
{
@@ -562,6 +565,9 @@ error_cb (cpp_reader *, int, int, source
}
static void
+#if GCC_VERSION = 4001
+__attribute__((format (printf, 2, 3)))
+#endif
fatal_at (const cpp_token *tk, const char *msg, ...)
{
va_list ap;
Index: gcc/gimple-match-head.c
===
--- gcc/gimple-match-head.c (revision 211221)
+++ gcc/gimple-match-head.c (working copy)
@@ -105,18 +105,23 @@ gimple_resimplify1 (gimple_seq *seq,
else
{
tree decl = builtin_decl_implicit (*res_code);
- tem = fold_builtin_n (UNKNOWN_LOCATION, decl, res_ops, 1, false);
- if (tem)
+ if (decl)
{
- /* fold_builtin_n wraps the result inside a NOP_EXPR. */
- STRIP_NOPS (tem);
- tem = fold_convert (type, tem);
+ tem = fold_builtin_n (UNKNOWN_LOCATION, decl, res_ops, 1, false);
+ if (tem)
+ {
+ /* fold_builtin_n wraps the result inside a NOP_EXPR. */
+ STRIP_NOPS (tem);
+ tem = fold_convert (type, tem);
+ }
}
}
if (tem != NULL_TREE
CONSTANT_CLASS_P (tem))
{
res_ops[0] = tem;
+ res_ops[1] = NULL_TREE;
+ res_ops[2] = NULL_TREE;
*res_code = TREE_CODE (res_ops[0]);
return true;
}
@@ -157,17 +162,22 @@ gimple_resimplify2 (gimple_seq *seq,
else
{
tree decl = builtin_decl_implicit (*res_code);
- tem = fold_builtin_n (UNKNOWN_LOCATION, decl, res_ops, 2, false);
- if (tem)
+ if (decl)
{
- /* fold_builtin_n wraps the result inside a NOP_EXPR. */
- STRIP_NOPS (tem);
- tem = fold_convert (type, tem);
+ tem = fold_builtin_n (UNKNOWN_LOCATION, decl, res_ops, 2, false);
+ if (tem)
+ {
+ /* fold_builtin_n wraps the result inside a NOP_EXPR. */
+ STRIP_NOPS (tem);
+ tem = fold_convert (type, tem);
+ }
}
}
if (tem != NULL_TREE)
{
res_ops[0] = tem;
+ res_ops[1] = NULL_TREE;
+ res_ops[2] = NULL_TREE;
*res_code = TREE_CODE (res_ops[0]);
return true;
}
@@ -221,18 +231,23 @@ gimple_resimplify3 (gimple_seq *seq,
else
{
tree decl = builtin_decl_implicit (*res_code);
- tem = fold_builtin_n (UNKNOWN_LOCATION, decl, res_ops, 3, false);
- if (tem)
+ if (decl)
{
- /* fold_builtin_n wraps the result inside a NOP_EXPR. */
- STRIP_NOPS (tem);
- tem = fold_convert (type, tem);
+ tem = fold_builtin_n (UNKNOWN_LOCATION, decl, res_ops, 3, false);
+ if (tem)
+ {
+ /* fold_builtin_n wraps the result inside a NOP_EXPR. */
+ STRIP_NOPS (tem);
+ tem = fold_convert (type, tem);
+ }
}
}
if (tem != NULL_TREE
CONSTANT_CLASS_P (tem))
{
res_ops[0] = tem;
+ res_ops[1] = NULL_TREE;
+ res_ops[2] = NULL_TREE;
*res_code = TREE_CODE (res_ops[0]);
return true;
}
@@ -297,9 +312,11 @@ maybe_push_res_to_seq (code_helper rcode
{
if (!seq)
return NULL_TREE;
+ tree decl = builtin_decl_implicit (rcode);
+ if (!decl)
+ return NULL_TREE;
if (!res)
res = make_ssa_name (type, NULL);
- tree decl = builtin_decl_implicit (rcode);
unsigned nargs = type_num_arguments (TREE_TYPE (decl));
gcc_assert (nargs = 3);
gimple new_stmt = gimple_build_call (decl, nargs, ops[0], ops[1],
ops[2]);
@@ -401,14 +418,17 @@ gimple_match_and_simplify (enum built_in
if
[PATCH][match-and-simplify] Fix cutpaste error
Committed. Hopefully that restores bootstrap ...
Richard.
2014-06-04 Richard Biener rguent...@suse.de
* gimple-match-head.c (gimple_match_and_simplify): Fix cutpaste
error.
Index: gcc/gimple-match-head.c
===
--- gcc/gimple-match-head.c (revision 211226)
+++ gcc/gimple-match-head.c (working copy)
@@ -604,7 +604,7 @@ gimple_match_and_simplify (gimple stmt,
if (!arg1)
return false;
}
- tree arg2 = gimple_call_arg (stmt, 0);
+ tree arg2 = gimple_call_arg (stmt, 1);
if (valueize TREE_CODE (arg2) == SSA_NAME)
{
arg2 = valueize (arg2);
Re: [patch, mips, tree] align microMIPS functions to 16 bits with -Os
On Tue, 3 Jun 2014, Richard Sandiford wrote:
Sandra Loosemore san...@codesourcery.com writes:
Catherine included an earlier version of this patch with the microMIPS
submission a couple years ago:
https://gcc.gnu.org/ml/gcc-patches/2012-07/msg00972.html
Richard's response was:
Looks like the wrong place to do this. Please treat this as a separate
patch and get a tree expert to comment.
So, here is the separate patch, finally. :-) Can a tree expert
comment? I dug around and didn't see another obvious hook to set
function alignment in a target-specific way depending on attributes of
the function.
Besides the new test cases, I regression-tested this on mips-sde-elf
using Mentor's usual assortment of multilibs, specifically including one
for microMIPS.
OK, I asked Richi on IRC today. To recap, one of the things I was
worried about was a test against the address, like (foo 2) == 0,
being optimised away before we set the alignment. Richi pointed
out that my idea downthread about cgraph_create_node would also be
too late to avoid that. Also, looking at it now, I see that we don't
trust DECL_ALIGN on functions anyway. From get_object_alignment_2:
/* Extract alignment information from the innermost object and
possibly adjust bitpos and offset. */
if (TREE_CODE (exp) == FUNCTION_DECL)
{
/* Function addresses can encode extra information besides their
alignment. However, if TARGET_PTRMEMFUNC_VBIT_LOCATION
allows the low bit to be used as a virtual bit, we know
that the address itself must be at least 2-byte aligned. */
if (TARGET_PTRMEMFUNC_VBIT_LOCATION == ptrmemfunc_vbit_in_pfn)
align = 2 * BITS_PER_UNIT;
}
And since we use the low bit to encode the ISA mode on MIPS, the upshot
is that we never assume any alignment for functions. So there's nothing
to worry about after all.
Richi suggested just changing the alignment at output time. I assume
that would be a case of replacing the DECL_ALIGN in:
/* Tell assembler to move to target machine's alignment for functions. */
align = floor_log2 (DECL_ALIGN (decl) / BITS_PER_UNIT);
if (align 0)
{
ASM_OUTPUT_ALIGN (asm_out_file, align);
}
with a hook. (Is that right?)
Yeah, kind of. Of course if DECL_ALIGN on function-decls is unused
then we may as well initialize it to 1 in tree.c and at an appropriate
stage adjust it to the result of a target hook invocation.
Appropriate stage would be the above place (which means DECL_ALIGN
is essentially unused for FUNCTION_DECLs).
So ... can you massage the DECL_ALIGN macro to ICE on FUNCTION_DECLs
and see where we access it? (generic code will possibly trip on it,
but the question is is there any user that cares?)
Richard.
Re: [patch i386]: Fix PR/46219 Generate indirect jump instruction
So after SIBLING_CALL_P flags gets now preserved in peep2_attempt, the patch simplifies. Regression tested for x86_64-unknown-linux-gnu, and i686-pc-cygwin. Ok for apply (with ChangeLog plus testcase adjustment as done by initial patch)? Index: config/i386/i386.md === --- config/i386/i386.md(Revision 211212) +++ config/i386/i386.md(Arbeitskopie) @@ -111,6 +111,7 @@ UNSPEC_LEA_ADDR UNSPEC_XBEGIN_ABORT UNSPEC_STOS + UNSPEC_PEEPSIB ;; For SSE/MMX support: UNSPEC_FIX_NOTRUNC @@ -11382,6 +11383,61 @@ * return ix86_output_call_insn (insn, operands[0]); [(set_attr type call)]) +(define_insn *sibcall_intern + [(call (unspec [(mem:QI (match_operand:W 0 memory_operand))] UNSPEC_PEEPSIB) + (match_operand 1))] + + * return ix86_output_call_insn (insn, operands[0]); + [(set_attr type call)]) + +(define_peephole2 + [(set (match_operand:DI 0 register_operand) +(match_operand:DI 1 memory_operand)) + (call (mem:QI (match_operand:DI 2 register_operand)) + (match_operand 3))] + TARGET_64BIT REG_P (operands[0]) + REG_P (operands[2]) + SIBLING_CALL_P (peep2_next_insn (1)) + REGNO (operands[0]) == REGNO (operands[2]) + [(call (unspec [(mem:QI (match_dup 1))] UNSPEC_PEEPSIB) (match_dup 3))]) + +(define_peephole2 + [(set (match_operand:DI 0 register_operand) +(match_operand:DI 1 memory_operand)) + (unspec_volatile [(const_int 0)] UNSPECV_BLOCKAGE) + (call (mem:QI (match_operand:DI 2 register_operand)) + (match_operand 3))] + TARGET_64BIT REG_P (operands[0]) + REG_P (operands[2]) + SIBLING_CALL_P (peep2_next_insn (2)) + REGNO (operands[0]) == REGNO (operands[2]) + [(unspec_volatile [(const_int 0)] UNSPECV_BLOCKAGE) + (call (unspec [(mem:QI (match_dup 1))] UNSPEC_PEEPSIB) (match_dup 3))]) + +(define_peephole2 + [(set (match_operand:SI 0 register_operand) +(match_operand:SI 1 memory_operand)) + (call (mem:QI (match_operand:SI 2 register_operand)) + (match_operand 3))] + !TARGET_64BIT REG_P (operands[0]) + REG_P (operands[2]) + SIBLING_CALL_P (peep2_next_insn (1)) + REGNO (operands[0]) == REGNO (operands[2]) + [(call (unspec [(mem:QI (match_dup 1))] UNSPEC_PEEPSIB) (match_dup 3))]) + +(define_peephole2 + [(set (match_operand:SI 0 register_operand) +(match_operand:SI 1 memory_operand)) + (unspec_volatile [(const_int 0)] UNSPECV_BLOCKAGE) + (call (mem:QI (match_operand:SI 2 register_operand)) + (match_operand 3))] + !TARGET_64BIT REG_P (operands[0]) + REG_P (operands[2]) + SIBLING_CALL_P (peep2_next_insn (2)) + REGNO (operands[0]) == REGNO (operands[2]) + [(unspec_volatile [(const_int 0)] UNSPECV_BLOCKAGE) + (call (unspec [(mem:QI (match_dup 1))] UNSPEC_PEEPSIB) (match_dup 3))]) + (define_expand call_pop [(parallel [(call (match_operand:QI 0) (match_operand:SI 1)) @@ -11415,6 +11471,16 @@ * return ix86_output_call_insn (insn, operands[0]); [(set_attr type call)]) +(define_insn *sibcall_pop_intern + [(call (unspec [(mem:QI (match_operand:SI 0 memory_operand))] UNSPEC_PEEPSIB) + (match_operand 1)) + (set (reg:SI SP_REG) +(plus:SI (reg:SI SP_REG) + (match_operand:SI 2 immediate_operand i)))] + !TARGET_64BIT + * return ix86_output_call_insn (insn, operands[0]); + [(set_attr type call)]) + ;; Call subroutine, returning value in operand 0 (define_expand call_value @@ -11457,6 +11523,14 @@ * return ix86_output_call_insn (insn, operands[1]); [(set_attr type callv)]) +(define_insn *sibcall_value_intern + [(set (match_operand 0) +(call (unspec [(mem:QI (match_operand:W 1 memory_operand))] UNSPEC_PEEPSIB) + (match_operand 2)))] + + * return ix86_output_call_insn (insn, operands[1]); + [(set_attr type callv)]) + (define_insn *call_value_rex64_ms_sysv [(match_parallel 3 call_rex64_ms_sysv_operation [(set (match_operand 0) @@ -11503,6 +11577,17 @@ * return ix86_output_call_insn (insn, operands[1]); [(set_attr type callv)]) +(define_insn *sibcall_value_pop_intern + [(set (match_operand 0) +(call (unspec [(mem:QI (match_operand:SI 1 memory_operand))] UNSPEC_PEEPSIB) + (match_operand 2))) + (set (reg:SI SP_REG) +(plus:SI (reg:SI SP_REG) + (match_operand:SI 3 immediate_operand i)))] + !TARGET_64BIT + * return ix86_output_call_insn (insn, operands[1]); + [(set_attr type callv)]) + ;; Call subroutine returning any type. (define_expand untyped_call
[PATCH] Spurious .s with -fdump-ada-spec
Hello,
Currently, GCC outputs .s assembly files when running on C/C++ header
files with -fdump-ada-spec. This is unexpected, since -fdump-ada-spec is
used only to output an Ada binding.
This patch fixes the spec-strings for C/C++ header files in order to
pass a -o %g.s flag to cc1/cc1plus, so that output assembly files are
cleaned at exit. It also fixes the processing of the -fsyntax-only flag
for C header file so that the -o ... argument is passed only once, as
the spec-string for C++ header files does.
I could bootstrap the patch and it triggered no regression on
x86_64-linux. I've written a simple test that checks that no assembly
file/object file is produced when compiling C/C++ header files with
-fdump-ada-spec. In order to do this, I had to create a new *.exp file
so that the testsuite processes my C header files (the regular dg.exp
file skips *.h files).
I also noticed that using -S and -o options at the same time currently
doesn't work on header files (even without -fdump-ada-spec): cc1 is
spawned with two -o arguments. That's why in the dump-ada-spec-2.h
file I've specified dg-do assemble instead of dg-do compile.
Many thanks in advance for your comments!
2014-06-02 Eric Botcazou botca...@adacore.com
Pierre-Marie de Rodat dero...@adacore.com
gcc/
* gcc.dg/dump-ada-spec/dump-ada-spec.exp: New.
* gcc.dg/dump-ada-spec-1.c: Move to dump-ada-spec.
* gcc.dg/dump-ada-spec/dump-ada-spec-2.h: New.
* g++.dg/dump-ada-spec: New.
gcc/testsuite/
* lib/gcc-dg.exp (file-exists, file-exists-not): New.
* gcc.c (default_compilers): Prevent creation of .s files with
-fdump-ada-spec and -fdump-xref.
* cp/lang-specs.h: Likewise.
--
Pierre-Marie de Rodat
diff --git gcc/ChangeLog gcc/ChangeLog
index bc9b036..e74d282 100644
--- gcc/ChangeLog
+++ gcc/ChangeLog
@@ -1,3 +1,10 @@
+2014-06-02 Eric Botcazou botca...@adacore.com
+Pierre-Marie de Rodat dero...@adacore.com
+
+ * gcc.c (default_compilers): Prevent creation of .s files with
+ -fdump-ada-spec and -fdump-xref.
+ * cp/lang-specs.h: Likewise.
+
2014-06-02 Uros Bizjak ubiz...@gmail.com
PR target/61239
diff --git gcc/cp/lang-specs.h gcc/cp/lang-specs.h
index 79c9222..35c3954 100644
--- gcc/cp/lang-specs.h
+++ gcc/cp/lang-specs.h
@@ -47,7 +47,7 @@ along with GCC; see the file COPYING3. If not see
cc1plus %{save-temps*|no-integrated-cpp:-fpreprocessed
%{save-temps*:%b.ii} %{!save-temps*:%g.ii}}\
%{!save-temps*:%{!no-integrated-cpp:%(cpp_unique_options)}}\
%(cc1_options) %2\
- %{!fsyntax-only:%{!fdump-ada-spec*:-o %g.s %{!o*:--output-pch=%i.gch}\
+ %{!fsyntax-only:-o %g.s %{!fdump-ada-spec*:%{!o*:--output-pch=%i.gch}\
%W{o*:--output-pch=%*}}%V,
CPLUSPLUS_CPP_SPEC, 0, 0},
{@c++,
diff --git gcc/gcc.c gcc/gcc.c
index 9ac18e6..699753a 100644
--- gcc/gcc.c
+++ gcc/gcc.c
@@ -1057,12 +1057,12 @@ static const struct compiler default_compilers[] =
%(cpp_options) -o %{save-temps*:%b.i} %{!save-temps*:%g.i} \n\
cc1 -fpreprocessed %{save-temps*:%b.i} %{!save-temps*:%g.i}
\
%(cc1_options)\
-%{!fdump-ada-spec*:-o %g.s %{!o*:--output-pch=%i.gch}\
-%W{o*:--output-pch=%*}}%V}\
+%{!fsyntax-only:-o %g.s
%{!fdump-ada-spec*:%{!o*:--output-pch=%i.gch}\
+%W{o*:--output-pch=%*}}%V}}\
%{!save-temps*:%{!traditional-cpp:%{!no-integrated-cpp:\
cc1 %(cpp_unique_options) %(cc1_options)\
-%{!fdump-ada-spec*:-o %g.s %{!o*:--output-pch=%i.gch}\
-%W{o*:--output-pch=%*}}%V}}, 0, 0, 0},
+%{!fsyntax-only:-o %g.s
%{!fdump-ada-spec*:%{!o*:--output-pch=%i.gch}\
+%W{o*:--output-pch=%*}}%V}}}, 0, 0, 0},
{.i, @cpp-output, 0, 0, 0},
{@cpp-output,
%{!M:%{!MM:%{!E:cc1 -fpreprocessed %i %(cc1_options)
%{!fsyntax-only:%(invoke_as), 0, 0, 0},
diff --git gcc/testsuite/ChangeLog gcc/testsuite/ChangeLog
index a52af2b..ce1ca39 100644
--- gcc/testsuite/ChangeLog
+++ gcc/testsuite/ChangeLog
@@ -1,3 +1,12 @@
+2014-06-02 Eric Botcazou botca...@adacore.com
+Pierre-Marie de Rodat dero...@adacore.com
+
+ * lib/gcc-dg.exp (file-exists, file-exists-not): New.
+ * gcc.dg/dump-ada-spec/dump-ada-spec.exp: New.
+ * gcc.dg/dump-ada-spec-1.c: Move to dump-ada-spec.
+ * gcc.dg/dump-ada-spec/dump-ada-spec-2.h: New.
+ * g++.dg/dump-ada-spec: New.
+
2014-06-02 Uros Bizjak ubiz...@gmail.com
* gcc.target/i386/xop-rotate1-vector.c (dg-options): Add -mno-avx2.
diff --git gcc/testsuite/g++.dg/dump-ada-spec/dump-ada-spec-1.c
gcc/testsuite/g++.dg/dump-ada-spec/dump-ada-spec-1.c
new file mode 100644
index 000..eb249e7
--- /dev/null
+++ gcc/testsuite/g++.dg/dump-ada-spec/dump-ada-spec-1.c
@@
Re: [PATCH, Pointer Bounds Checker 6/x] New static constructor types
2014-06-04 13:58 GMT+04:00 Richard Biener richard.guent...@gmail.com:
On Wed, Apr 16, 2014 at 2:33 PM, Ilya Enkovich enkovich@gmail.com wrote:
Hi,
This patch add new static constructor types used by Pointer Bounds Checker.
It was approved earlier for 4.9 and I'll assume patch is OK for trunk if no
objections arise.
Patch was bootstrapped and tested for linux-x86_64.
Thanks,
Ilya
--
gcc/
2014-04-16 Ilya Enkovich ilya.enkov...@intel.com
* ipa.c (cgraph_build_static_cdtor_1): Support contructors
with chkp ctor and bnd_legacy attributes.
* gimplify.c (gimplify_init_constructor): Avoid infinite
loop during gimplification of bounds initializer.
diff --git a/gcc/gimplify.c b/gcc/gimplify.c
index 7441784..67ab515 100644
--- a/gcc/gimplify.c
+++ b/gcc/gimplify.c
@@ -3803,10 +3803,19 @@ gimplify_init_constructor (tree *expr_p, gimple_seq
*pre_p, gimple_seq *post_p,
individual element initialization. Also don't do this for small
all-zero initializers (which aren't big enough to merit
clearing), and don't try to make bitwise copies of
- TREE_ADDRESSABLE types. */
+ TREE_ADDRESSABLE types.
+
+ We cannot apply such transformation when compiling chkp static
+ initializer because creation of initializer image in the memory
+ will require static initialization of bounds for it. It should
+ result in another gimplification of similar initializer and we
+ may fall into infinite loop. */
if (valid_const_initializer
!(cleared || num_nonzero_elements == 0)
-!TREE_ADDRESSABLE (type))
+!TREE_ADDRESSABLE (type)
+(!current_function_decl
+ || !lookup_attribute (chkp ctor,
+ DECL_ATTRIBUTES
(current_function_decl
Simply make the type TREE_ADDRESSABLE?
Wouldn't it be a hack to mark it addressable just to not hit this
condition? It would also require to have an unshared copy of the type
to not affect other statements with that type.
{
HOST_WIDE_INT size = int_size_in_bytes (type);
unsigned int align;
diff --git a/gcc/ipa.c b/gcc/ipa.c
index 26e9b03..5ab3aed 100644
--- a/gcc/ipa.c
+++ b/gcc/ipa.c
@@ -1345,9 +1345,11 @@ make_pass_ipa_whole_program_visibility (gcc::context
*ctxt)
}
/* Generate and emit a static constructor or destructor. WHICH must
- be one of 'I' (for a constructor) or 'D' (for a destructor). BODY
- is a STATEMENT_LIST containing GENERIC statements. PRIORITY is the
- initialization priority for this constructor or destructor.
+ be one of 'I' (for a constructor), 'D' (for a destructor), 'P'
+ (for chp static vars constructor) or 'B' (for chkp static bounds
+ constructor). BODY is a STATEMENT_LIST containing GENERIC
+ statements. PRIORITY is the initialization priority for this
+ constructor or destructor.
FINAL specify whether the externally visible name for collect2 should
be produced. */
@@ -1406,6 +1408,20 @@ cgraph_build_static_cdtor_1 (char which, tree body,
int priority, bool final)
DECL_STATIC_CONSTRUCTOR (decl) = 1;
decl_init_priority_insert (decl, priority);
break;
+case 'P':
+ DECL_STATIC_CONSTRUCTOR (decl) = 1;
+ DECL_ATTRIBUTES (decl) = tree_cons (get_identifier (chkp ctor),
+ NULL,
+ NULL_TREE);
Ick. Please try to avoid using attributes for this. Rather adjust the
caller of this function to set a flag in the cgraph node.
It is too late because all early local passes are executed by that
time and I need this attribute to be set before them.
Ilya
So I don't like this patch at all.
Richard.
+ decl_init_priority_insert (decl, priority);
+ break;
+case 'B':
+ DECL_STATIC_CONSTRUCTOR (decl) = 1;
+ DECL_ATTRIBUTES (decl) = tree_cons (get_identifier (bnd_legacy),
+ NULL,
+ NULL_TREE);
+ decl_init_priority_insert (decl, priority);
+ break;
case 'D':
DECL_STATIC_DESTRUCTOR (decl) = 1;
decl_fini_priority_insert (decl, priority);
@@ -1423,9 +1439,11 @@ cgraph_build_static_cdtor_1 (char which, tree body,
int priority, bool final)
}
/* Generate and emit a static constructor or destructor. WHICH must
- be one of 'I' (for a constructor) or 'D' (for a destructor). BODY
- is a STATEMENT_LIST containing GENERIC statements. PRIORITY is the
- initialization priority for this constructor or destructor. */
+ be one of 'I' (for a constructor), 'D' (for a destructor), 'P'
+ (for chkp static vars constructor) or 'B' (for chkp static bounds
+ constructor). BODY is a STATEMENT_LIST containing GENERIC
+ statements. PRIORITY is the
Re: patch to fix PR61325
On 4 June 2014 00:02, James Greenhalgh james.greenha...@arm.com wrote: On Thu, May 29, 2014 at 06:38:22PM +0100, Vladimir Makarov wrote: The following patch PR61325. The details can be found on https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61325 The patch was bootstrapped and tested on x86/x86-64. Committed as rev. 211060 to gcc-4.9 branch and as rev.211061 to trunk. 2014-05-29 Vladimir Makarov vmaka...@redhat.com PR rtl-optimization/61325 * lra-constraints.c (process_address): Rename to process_address_1. (process_address): New function. 2014-05-29 Vladimir Makarov vmaka...@redhat.com PR rtl-optimization/61325 * gcc.target/aarch64/pr61325.c: New. Hi Vlad, This patch appears to cause issues where the ARM backend can get stuck in a seemingly infinite loop. Compiling: ./gcc.c-torture/compile/unalign-1.c Directly With cc1 using command line: cc1 gcc-clean/gcc/testsuite/gcc.c-torture/compile/unalign-1.c -O3 -mcpu=cortex-a15 -mthumb -mfloat-abi=hard -mfpu=neon-vfpv4 Triggers the issue reproducibly for me across a few ARM configurations. Hello, On my side, I can see a huge increase in memory use (up to at least 350GB in make check) by GCC since this commit. (and the Compute Farm admins have started to kill my jobs because they consume too much memory ;-) I can see this on several ARM and AArch64 configurations, but not on armeb, aarch64-bare and arm* when restricting to armv5t. Christophe.
Re: [PATCH] Fix logic for detection of zero initializer (PR c/53119)
On Mon, Jun 02, 2014 at 12:58:53AM -0400, S. Gilles wrote:
Thanks for tackling this.
@@ -6858,6 +6858,9 @@
/* 1 if this constructor is erroneous so far. */
static int constructor_erroneous;
+/* 1 if this constructor is the universal zero initializer { 0 } */
. */ at the end of the comment, please.
@@ -7317,12 +7321,8 @@
set_nonincremental_init (braced_init_obstack);
}
- if (implicit == 1 warn_missing_braces !missing_braces_mentioned)
-{
- missing_braces_mentioned = 1;
- warning_init (input_location, OPT_Wmissing_braces,
- missing braces around initializer);
-}
+ if (implicit == 1)
+ found_missing_braces = 1;
Wrong indentation, there should be only two spaces.
+ /* Warn when some structs are initialized with direct aggregation. */
+ if (!implicit found_missing_braces warn_missing_braces
+ !constructor_zeroinit)
+{
+ warning_init (input_location, OPT_Wmissing_braces,
+ missing braces around initializer);
+}
+
Please use loc instead of input_location.
@@ -8594,6 +8598,11 @@
designator_depth = 0;
designator_erroneous = 0;
+ if (!implicit value.value !integer_zerop (value.value))
+{
+ constructor_zeroinit = 0;
+}
+
You can drop the braces here.
But the problem I see is that this patch treats even { 0, 0 } as
a universal zero initializer, thus we wouldn't warn on say:
/* -Wmissing-braces -Wmissing-field-initializers */
struct T { int a, b; };
struct U { int a, b; struct T t; };
void
f (void)
{
int a[2][2][2] = { 0, 0, 0 };
struct U u = { 0, 0 };
}
I'm not sure whether that is desirable.
Marek
[PATCH, PR 61391]
Hi All, Here is a simple fix for 61391 - missed a check that statement basic block is inside loop. With this fix test-case from bug is compiled successfully. Bootstrap and regression testing did not show any new failures. Is it OK for trunk? ChangeLog: 2014-06-04 Yuri Rumyantsev ysrum...@gmail.com PR tree-optimization/61319 * tree-if-conv.c (is_cond_scalar_reduction): Add missed check that stmt belongs to loop. patch Description: Binary data
Re: [PATCH, Pointer Bounds Checker 6/x] New static constructor types
On Wed, Jun 4, 2014 at 3:13 PM, Ilya Enkovich enkovich@gmail.com wrote:
2014-06-04 13:58 GMT+04:00 Richard Biener richard.guent...@gmail.com:
On Wed, Apr 16, 2014 at 2:33 PM, Ilya Enkovich enkovich@gmail.com
wrote:
Hi,
This patch add new static constructor types used by Pointer Bounds Checker.
It was approved earlier for 4.9 and I'll assume patch is OK for trunk if
no objections arise.
Patch was bootstrapped and tested for linux-x86_64.
Thanks,
Ilya
--
gcc/
2014-04-16 Ilya Enkovich ilya.enkov...@intel.com
* ipa.c (cgraph_build_static_cdtor_1): Support contructors
with chkp ctor and bnd_legacy attributes.
* gimplify.c (gimplify_init_constructor): Avoid infinite
loop during gimplification of bounds initializer.
diff --git a/gcc/gimplify.c b/gcc/gimplify.c
index 7441784..67ab515 100644
--- a/gcc/gimplify.c
+++ b/gcc/gimplify.c
@@ -3803,10 +3803,19 @@ gimplify_init_constructor (tree *expr_p, gimple_seq
*pre_p, gimple_seq *post_p,
individual element initialization. Also don't do this for small
all-zero initializers (which aren't big enough to merit
clearing), and don't try to make bitwise copies of
- TREE_ADDRESSABLE types. */
+ TREE_ADDRESSABLE types.
+
+ We cannot apply such transformation when compiling chkp static
+ initializer because creation of initializer image in the memory
+ will require static initialization of bounds for it. It should
+ result in another gimplification of similar initializer and we
+ may fall into infinite loop. */
if (valid_const_initializer
!(cleared || num_nonzero_elements == 0)
-!TREE_ADDRESSABLE (type))
+!TREE_ADDRESSABLE (type)
+(!current_function_decl
+ || !lookup_attribute (chkp ctor,
+ DECL_ATTRIBUTES
(current_function_decl
Simply make the type TREE_ADDRESSABLE?
Wouldn't it be a hack to mark it addressable just to not hit this
condition? It would also require to have an unshared copy of the type
to not affect other statements with that type.
{
HOST_WIDE_INT size = int_size_in_bytes (type);
unsigned int align;
diff --git a/gcc/ipa.c b/gcc/ipa.c
index 26e9b03..5ab3aed 100644
--- a/gcc/ipa.c
+++ b/gcc/ipa.c
@@ -1345,9 +1345,11 @@ make_pass_ipa_whole_program_visibility (gcc::context
*ctxt)
}
/* Generate and emit a static constructor or destructor. WHICH must
- be one of 'I' (for a constructor) or 'D' (for a destructor). BODY
- is a STATEMENT_LIST containing GENERIC statements. PRIORITY is the
- initialization priority for this constructor or destructor.
+ be one of 'I' (for a constructor), 'D' (for a destructor), 'P'
+ (for chp static vars constructor) or 'B' (for chkp static bounds
+ constructor). BODY is a STATEMENT_LIST containing GENERIC
+ statements. PRIORITY is the initialization priority for this
+ constructor or destructor.
FINAL specify whether the externally visible name for collect2 should
be produced. */
@@ -1406,6 +1408,20 @@ cgraph_build_static_cdtor_1 (char which, tree body,
int priority, bool final)
DECL_STATIC_CONSTRUCTOR (decl) = 1;
decl_init_priority_insert (decl, priority);
break;
+case 'P':
+ DECL_STATIC_CONSTRUCTOR (decl) = 1;
+ DECL_ATTRIBUTES (decl) = tree_cons (get_identifier (chkp ctor),
+ NULL,
+ NULL_TREE);
Ick. Please try to avoid using attributes for this. Rather adjust the
caller of this function to set a flag in the cgraph node.
It is too late because all early local passes are executed by that
time and I need this attribute to be set before them.
Ok, so where do you call this from? It should be possible to create
the function in GIMPLE form directly, avoiding the need to go through
gimplification.
Richard.
Ilya
So I don't like this patch at all.
Richard.
+ decl_init_priority_insert (decl, priority);
+ break;
+case 'B':
+ DECL_STATIC_CONSTRUCTOR (decl) = 1;
+ DECL_ATTRIBUTES (decl) = tree_cons (get_identifier (bnd_legacy),
+ NULL,
+ NULL_TREE);
+ decl_init_priority_insert (decl, priority);
+ break;
case 'D':
DECL_STATIC_DESTRUCTOR (decl) = 1;
decl_fini_priority_insert (decl, priority);
@@ -1423,9 +1439,11 @@ cgraph_build_static_cdtor_1 (char which, tree body,
int priority, bool final)
}
/* Generate and emit a static constructor or destructor. WHICH must
- be one of 'I' (for a constructor) or 'D' (for a destructor). BODY
- is a STATEMENT_LIST containing GENERIC statements. PRIORITY is the
- initialization priority for this constructor or
Re: [PATCH, PR 61391]
On Wed, Jun 4, 2014 at 3:17 PM, Yuri Rumyantsev ysrum...@gmail.com wrote: Hi All, Here is a simple fix for 61391 - missed a check that statement basic block is inside loop. With this fix test-case from bug is compiled successfully. Bootstrap and regression testing did not show any new failures. Is it OK for trunk? Ok. Thanks, Richard. ChangeLog: 2014-06-04 Yuri Rumyantsev ysrum...@gmail.com PR tree-optimization/61319 * tree-if-conv.c (is_cond_scalar_reduction): Add missed check that stmt belongs to loop.
Re: [PATCH, PR 61391]
On Wed, Jun 4, 2014 at 3:37 PM, Richard Biener richard.guent...@gmail.com wrote: On Wed, Jun 4, 2014 at 3:17 PM, Yuri Rumyantsev ysrum...@gmail.com wrote: Hi All, Here is a simple fix for 61391 - missed a check that statement basic block is inside loop. With this fix test-case from bug is compiled successfully. Bootstrap and regression testing did not show any new failures. Is it OK for trunk? Ok. Please also add the testcase. Thanks, Richard. ChangeLog: 2014-06-04 Yuri Rumyantsev ysrum...@gmail.com PR tree-optimization/61319 * tree-if-conv.c (is_cond_scalar_reduction): Add missed check that stmt belongs to loop.
[PATCH][match-and-simplify] Remove remaining stmt expression uses
This removes the code-generation ones by simply passing the destination
to the generators. It also fixes the last remaining warning (just
introduced) and removes the -Wno-error -fpermissive flags from building
gimple-match.o.
Bootstrapped on x86_64-unknown-linux-gnu, applied.
Richard.
2014-06-04 Richard Biener rguent...@suse.de
* Makefile.in (gimple-match.o-warn): Drop.
* genmatch.c (*::gen_gimple_transform): Add destination argument
and use that to store the result to. Drop stmt expression use.
(write_nary_simplifiers): Adjust.
* gimple-match-head.c (gimple_resimplify1): Initialize tem.
(gimple_resimplify2): Likewise.
(gimple_resimplify3): Likewise.
Index: gcc/Makefile.in
===
--- gcc/Makefile.in (revision 211221)
+++ gcc/Makefile.in (working copy)
@@ -196,7 +196,6 @@ GCC_WARN_CXXFLAGS = $(LOOSE_WARN) $($(@D
# flex output may yield harmless no previous prototype warnings
build/gengtype-lex.o-warn = -Wno-error
gengtype-lex.o-warn = -Wno-error
-gimple-match.o-warn = -Wno-error -fpermissive
# All warnings have to be shut off in stage1 if the compiler used then
# isn't gcc; configure determines that. WARN_CFLAGS will be either
Index: gcc/genmatch.c
===
--- gcc/genmatch.c (revision 211229)
+++ gcc/genmatch.c (working copy)
@@ -160,7 +160,7 @@ struct operand {
operand (enum op_type type_) : type (type_) {}
enum op_type type;
virtual void gen_gimple_match (FILE *f, const char *, const char * = NULL) =
0;
- virtual void gen_gimple_transform (FILE *f, const char * = NULL) = 0;
+ virtual void gen_gimple_transform (FILE *f, const char *, const char *) = 0;
};
struct predicate : public operand
@@ -168,7 +168,7 @@ struct predicate : public operand
predicate (const char *ident_) : operand (OP_PREDICATE), ident (ident_) {}
const char *ident;
virtual void gen_gimple_match (FILE *f, const char *, const char *);
- virtual void gen_gimple_transform (FILE *, const char *) { gcc_unreachable
(); }
+ virtual void gen_gimple_transform (FILE *, const char *, const char *) {
gcc_unreachable (); }
};
struct e_operation {
@@ -185,7 +185,7 @@ struct expr : public operand
e_operation *operation;
vecoperand * ops;
virtual void gen_gimple_match (FILE *f, const char *, const char *);
- virtual void gen_gimple_transform (FILE *f, const char *);
+ virtual void gen_gimple_transform (FILE *f, const char *, const char *);
};
struct c_expr : public operand
@@ -198,7 +198,7 @@ struct c_expr : public operand
unsigned nr_stmts;
char *fname;
virtual void gen_gimple_match (FILE *, const char *, const char *) {
gcc_unreachable (); }
- virtual void gen_gimple_transform (FILE *f, const char *);
+ virtual void gen_gimple_transform (FILE *f, const char *, const char *);
};
struct capture : public operand
@@ -208,7 +208,7 @@ struct capture : public operand
const char *where;
operand *what;
virtual void gen_gimple_match (FILE *f, const char *, const char *);
- virtual void gen_gimple_transform (FILE *f, const char *);
+ virtual void gen_gimple_transform (FILE *f, const char *, const char *);
};
@@ -409,15 +409,15 @@ expr::gen_gimple_match (FILE *f, const c
}
void
-expr::gen_gimple_transform (FILE *f, const char *label)
+expr::gen_gimple_transform (FILE *f, const char *label, const char *dest)
{
- fprintf (f, ({\n);
+ fprintf (f, {\n);
fprintf (f, tree ops[%d], res;\n, ops.length ());
for (unsigned i = 0; i ops.length (); ++i)
{
- fprintf (f, ops[%u] = , i);
- ops[i]-gen_gimple_transform (f, label);
- fprintf (f, ;\n);
+ char dest[32];
+ snprintf (dest, 32, ops[%u], i);
+ ops[i]-gen_gimple_transform (f, label, dest);
}
/* ??? Have another helper that is like gimple_build but may
fail if seq == NULL. */
@@ -437,23 +437,26 @@ expr::gen_gimple_transform (FILE *f, con
for (unsigned i = 0; i ops.length (); ++i)
fprintf (f, , ops[%u], i);
fprintf (f, , valueize);\n);
- fprintf (f, res;\n);
- fprintf (f, }));
+ fprintf (f, %s = res;\n, dest);
+ fprintf (f, });
}
void
-c_expr::gen_gimple_transform (FILE *f, const char *)
+c_expr::gen_gimple_transform (FILE *f, const char *, const char *dest)
{
/* If this expression has an outlined function variant, call it. */
if (fname)
{
- fprintf (f, %s (type, captures), fname);
+ fprintf (f, %s = %s (type, captures);\n, dest, fname);
return;
}
/* All multi-stmt expressions should have been outlined. */
gcc_assert (nr_stmts = 1);
+ if (nr_stmts == 1)
+fprintf (f, %s = , dest);
+
for (unsigned i = 0; i code.length (); ++i)
{
const cpp_token *token = code[i];
@@ -473,11 +476,6 @@ c_expr::gen_gimple_transform (FILE *f, c
}
}
- /* Skip a single stmt
[PATCH] Trust TREE_ADDRESSABLE
This makes may_be_aliased trust TREE_ADDRESSABLE setting for
exported decls. This should make it possible for LTO to
compute more optimistic aliasing and removes the pessimization
with respective to aliasing that currently LTO brought global
(but hidden visibility) statics cause.
Bootstrapped and tested on x86_64-unknown-linux-gnu for all
languages including obj-c++, ada and go (yay), applied.
Richard.
2014-06-04 Richard Biener rguent...@suse.de
* tree.h (may_be_aliased): Trust TREE_ADDRESSABLE from
TREE_PUBLIC and DECL_EXTERNAL decls.
Index: gcc/tree.h
===
--- gcc/tree.h (revision 211215)
+++ gcc/tree.h (working copy)
@@ -4506,9 +4506,7 @@ static inline bool
may_be_aliased (const_tree var)
{
return (TREE_CODE (var) != CONST_DECL
- (TREE_PUBLIC (var)
- || DECL_EXTERNAL (var)
- || TREE_ADDRESSABLE (var))
+ TREE_ADDRESSABLE (var)
!((TREE_STATIC (var) || TREE_PUBLIC (var) || DECL_EXTERNAL (var))
((TREE_READONLY (var)
!TYPE_NEEDS_CONSTRUCTING (TREE_TYPE (var)))
Re: [patch][gomp4] openacc loops
Hi Cesar!
This patch, which is derived from Ilmir Usmanov's work posted here
https://gcc.gnu.org/ml/gcc-patches/2014-04/msg00027.html, implements
the loop directive in openacc. The original patch is mostly intact,
Thank you!
I looked through the patch and found that you also added middle-end
part. I don't know a lot about middle-end, so, probably, Thomas could
review the part. However, there is a regression in middle-end.
About front-ends, especially fortran front-end:
I did disable support for do concurrent loops since openacc
2.0a supports fortran up to fortran 2003.
As I can see, you didn't remove helper code for DO CONCURRENT loops
transformation (see below).
@@ -12217,8 +12221,8 @@ c_parser_omp_for_loop (location_t loc, c_parser
*parser, enum tree_code code,
for (cl = clauses; cl; cl = OMP_CLAUSE_CHAIN (cl))
if (OMP_CLAUSE_CODE (cl) == OMP_CLAUSE_COLLAPSE)
{
- gcc_assert (code != OACC_LOOP);
- collapse = tree_to_shwi (OMP_CLAUSE_COLLAPSE_EXPR (cl));
+ //gcc_assert (code != OACC_LOOP);
I suppose you forgot to remove this comment.
+++ b/gcc/testsuite/gfortran.dg/goacc/loop-4.f95
@@ -0,0 +1,16 @@
+! { dg-do compile }
+! { dg-additional-options -fdump-tree-original -std=f2008 }
+
+PROGRAM test
+ IMPLICIT NONE
+ INTEGER :: a(64), b(64), c(64), i, j, k
+ ! Must be replaced by three loops.
+ !$acc loop
+ DO CONCURRENT (i=1:64, j=1:64, k=1:64, i==j .and. j==k)
This test is obsolete. I think you should remove this testcase since you
are not supporting DO CONCURRENT loops.
+++ b/gcc/testsuite/gfortran.dg/goacc/loop-tree.f95
For this test you should update tree-pretty-print.c (I forgot this):
@@ -675,13 +675,13 @@ dump_omp_clause (pretty_printer *buffer, tree
clause, int spc, int flags)
case OMP_CLAUSE_WORKER:
pp_string (buffer, worker();
- dump_generic_node (buffer, OMP_CLAUSE_DECL (clause), spc, flags,
false);
+ dump_generic_node (buffer, OMP_CLAUSE_WORKER_EXPR (clause), spc,
flags, false);
pp_character(buffer, ')');
break;
case OMP_CLAUSE_VECTOR:
pp_string (buffer, vector();
- dump_generic_node (buffer, OMP_CLAUSE_DECL (clause), spc, flags,
false);
+ dump_generic_node (buffer, OMP_CLAUSE_VECTOR_EXPR (clause), spc,
flags, false);
pp_character(buffer, ')');
break;
+/* Recursively generate conditional expressions. */
+static tree
+gfc_trans_oacc_loop_generate_mask_conds (gfc_code *code, int collapse)
+{
+ if (collapse 1)
+return gfc_trans_oacc_loop_generate_mask_conds (code-block-next,
+ collapse - 1);
+ else
+return gfc_trans_omp_code (code-block-next, true);
+}
+static tree
+gfc_trans_oacc_loop (gfc_code *code, stmtblock_t *pblock,
+gfc_omp_clauses *loop_clauses)
+{
+ /* DO CONCURRENT specific vars. */
+ int nforloops = 0;
+ int current_for = 0;
+
+ if (collapse = 0)
+collapse = 1;
+
+ code = code-block-next;
+
+ if (code-op == EXEC_DO_CONCURRENT)
+gfc_error (!$ACC LOOP directive is unsupported on DO CONCURRENT %L,
+ code-loc);
+
+ gcc_assert (code-op == EXEC_DO);
+
+ if (pblock == NULL)
+{
+ gfc_start_block (block);
+ pblock = block;
+}
+
+ /* Calculate number of required for loops. */
+ old_code = code;
+ for (i = 0; i collapse; i++)
+{
+ if (code-op == EXEC_DO)
+ nforloops++;
+ else
+ gcc_unreachable ();
+ code = code-block-next;
+}
+ code = old_code;
+
+ /* Set the number of required for loops for collapse. */
+ /* FIXME: this is probably correct, but OMP_CLAUSE_COLLAPSE isn't supported
+ yet. */
+ loop_clauses-collapse = nforloops;
+
+ omp_clauses = gfc_trans_omp_clauses (pblock, loop_clauses, code-loc);
+
+ init = make_tree_vec (nforloops);
+ cond = make_tree_vec (nforloops);
+ incr = make_tree_vec (nforloops);
+
+ for (i = 0; i collapse; i++)
+{
+ if (code-op == EXEC_DO)
+ gfc_trans_oacc_loop_generate_for (pblock, se, code-ext.iterator-var,
+ code-ext.iterator-start,
+ code-ext.iterator-end,
+ code-ext.iterator-step,
+ current_for++, init, cond, incr,
+ inits);
+ else
+ gcc_unreachable ();
+ if (i + 1 collapse)
+ code = code-block-next;
+}
+
+ if (pblock != block)
+{
+ pushlevel ();
+ gfc_start_block (block);
+}
This is complicated for simple DO loops. I think the following will be
enough (see gfc_trans_omp_do).
code = code-block-next;
+ if (code-op == EXEC_DO_CONCURRENT)
+gfc_error (!$ACC LOOP directive is unsupported on DO CONCURRENT %L,
+ code-loc);
gcc_assert (code-op == EXEC_DO);
init = make_tree_vec (collapse);
cond = make_tree_vec (collapse);
incr = make_tree_vec (collapse);
if
[Patch,testsuite] Fix bind_pic_locally
Hello,
This is to follow up the patch I had posted to fix bind_pic_locally some time
ago (sorry, this went in to my back log for a while).
To summarize, multilib_flags when it contains -fpic or -fPIC, overrides -fpie
or -fPIE that is added by bind_pic_locally. The fix that was finally agreed on
was to store the flags to a variable at bind_pic_locally and append it to
multilib_flags just before invoking target_compile and remove it immediately
after that (Refer [1]).
This patch implements the same. Since this is an issue not only for gcc but
also for g++ and gfortran tests, I have fixed this in g++.exp and gfortran.exp
along with gcc.exp.
This was tested and works fine on:
aarch64-none-elf
aarch64-none-linux-gnu
arm-none-linux-gnueabihf
x86_64-unknown-linux-gnu
OK for trunk?
Cheers
VP.
[1] http://gcc.gnu.org/ml/gcc-patches/2014-01/msg00365.html
~~~
gcc/testsuite/ChangeLog:
2014-06-04 Vidya Praveen vidyaprav...@arm.com
* lib/target-support.exp (bind_pic_locally): Save the flags to
'flags_to_postpone' instead of appending to 'flags'.
* lib/gcc.exp (gcc_target_compile): Append board_info's multilib_flags
with flags_to_postpone and revert after target_compile.
* lib/g++.exp (g++_target_compile): Ditto.
* lib/gfortran.exp (gfortran_target_compile): Ditto.diff --git a/gcc/testsuite/lib/g++.exp b/gcc/testsuite/lib/g++.exp
index 751e27b..6658c58 100644
--- a/gcc/testsuite/lib/g++.exp
+++ b/gcc/testsuite/lib/g++.exp
@@ -288,6 +288,8 @@ proc g++_target_compile { source dest type options } {
global gluefile wrap_flags
global ALWAYS_CXXFLAGS
global GXX_UNDER_TEST
+global flags_to_postpone
+global board_info
if { [target_info needs_status_wrapper] != [info exists gluefile] } {
lappend options libs=${gluefile}
@@ -313,10 +315,25 @@ proc g++_target_compile { source dest type options } {
exec rm -f $rponame
}
+# bind_pic_locally adds -fpie/-fPIE flags to flags_to_postpone and it is
+# appended here to multilib_flags as it can be overridden by the latter
+# if it was added earlier. After the target_compile, multilib_flags is
+# restored to its orignal content.
+set tboard [target_info name]
+if {[board_info $tboard exists multilib_flags]} {
+set orig_multilib_flags [board_info [target_info name] multilib_flags]
+append board_info($tboard,multilib_flags) $flags_to_postpone
+}
+
set options [dg-additional-files-options $options $source]
set result [target_compile $source $dest $type $options]
+if {[board_info $tboard exists multilib_flags]} {
+set board_info($tboard,multilib_flags) $orig_multilib_flags
+set flags_to_postpone
+}
+
return $result
}
diff --git a/gcc/testsuite/lib/gcc.exp b/gcc/testsuite/lib/gcc.exp
index 49394b0..f937064 100644
--- a/gcc/testsuite/lib/gcc.exp
+++ b/gcc/testsuite/lib/gcc.exp
@@ -126,7 +126,9 @@ proc gcc_target_compile { source dest type options } {
global GCC_UNDER_TEST
global TOOL_OPTIONS
global TEST_ALWAYS_FLAGS
-
+global flags_to_postpone
+global board_info
+
if {[target_info needs_status_wrapper] != \
[target_info needs_status_wrapper] != 0 \
[info exists gluefile] } {
@@ -162,8 +164,26 @@ proc gcc_target_compile { source dest type options } {
set options [concat {additional_flags=$TOOL_OPTIONS} $options]
}
+# bind_pic_locally adds -fpie/-fPIE flags to flags_to_postpone and it is
+# appended here to multilib_flags as it can be overridden by the latter
+# if it was added earlier. After the target_compile, multilib_flags is
+# restored to its orignal content.
+set tboard [target_info name]
+if {[board_info $tboard exists multilib_flags]} {
+set orig_multilib_flags [board_info [target_info name] multilib_flags]
+append board_info($tboard,multilib_flags) $flags_to_postpone
+}
+
lappend options timeout=[timeout_value]
lappend options compiler=$GCC_UNDER_TEST
set options [dg-additional-files-options $options $source]
-return [target_compile $source $dest $type $options]
+set return_val [target_compile $source $dest $type $options]
+
+if {[board_info $tboard exists multilib_flags]} {
+set board_info($tboard,multilib_flags) $orig_multilib_flags
+set flags_to_postpone
+}
+
+return $return_val
}
+
diff --git a/gcc/testsuite/lib/gfortran.exp b/gcc/testsuite/lib/gfortran.exp
index c9b5d64..9d174bb 100644
--- a/gcc/testsuite/lib/gfortran.exp
+++ b/gcc/testsuite/lib/gfortran.exp
@@ -234,16 +234,35 @@ proc gfortran_target_compile { source dest type options } {
global gluefile wrap_flags
global ALWAYS_GFORTRANFLAGS
global GFORTRAN_UNDER_TEST
+global flags_to_postpone
+global board_info
if { [target_info needs_status_wrapper] != [info exists gluefile] } {
lappend options libs=${gluefile}
lappend options
[Patch,testsuite] Fix tests that fail due to symbol visibility when -fPIC
Hello,
The following test cases fail when -fPIC is passed as dejagnu multilib flag
since -fPIC causes the 'availability' of the functions to be overwritable. I
have fixed this by adding bind_pic_locally to these cases.
gcc.dg/fail_always_inline.c
gcc.dg/inline-22.c
gcc.dg/inline_4.c
g++.dg/ipa/devirt-25.C
Tested on:
aarch64-none-elf
aarch64-none-linux-gnu
arm-none-linux-gnueabihf
x86_64-unknown-linux-gnu
OK for trunk?
Cheers
VP.
~~~
gcc/testsuite/ChangeLog:
2014-06-04 Vidya Praveen vidyaprav...@arm.com
* gcc.dg/inline-22.c: Add bind_pic_locally.
* gcc.dg/inline_4.c: Ditto.
* gcc.dg/fail_always_inline.c: Ditto.
* g++.dg/ipa/devirt-25.C: Ditto.
diff --git a/gcc/testsuite/g++.dg/ipa/devirt-25.C b/gcc/testsuite/g++.dg/ipa/devirt-25.C
index 7516479..387d529 100644
--- a/gcc/testsuite/g++.dg/ipa/devirt-25.C
+++ b/gcc/testsuite/g++.dg/ipa/devirt-25.C
@@ -1,5 +1,6 @@
/* { dg-do compile } */
/* { dg-options -O3 -fdump-ipa-cp } */
+/* { dg-add-options bind_pic_locally } */
class ert_RefCounter {
protected:
diff --git a/gcc/testsuite/gcc.dg/fail_always_inline.c b/gcc/testsuite/gcc.dg/fail_always_inline.c
index 4b196ac..86645b8 100644
--- a/gcc/testsuite/gcc.dg/fail_always_inline.c
+++ b/gcc/testsuite/gcc.dg/fail_always_inline.c
@@ -1,4 +1,5 @@
/* { dg-do compile } */
+/* { dg-add-options bind_pic_locally } */
extern __attribute__ ((always_inline)) void
bar() { } /* { dg-warning function might not be inlinable } */
diff --git a/gcc/testsuite/gcc.dg/inline-22.c b/gcc/testsuite/gcc.dg/inline-22.c
index 1785e1c..6795c5f 100644
--- a/gcc/testsuite/gcc.dg/inline-22.c
+++ b/gcc/testsuite/gcc.dg/inline-22.c
@@ -1,5 +1,6 @@
/* { dg-do compile } */
/* { dg-options -funit-at-a-time -Wno-attributes } */
+/* { dg-add-options bind_pic_locally } */
/* Verify we can inline without a complete prototype and with promoted
arguments. See also PR32492. */
__attribute__((always_inline)) void f1() {}
diff --git a/gcc/testsuite/gcc.dg/inline_4.c b/gcc/testsuite/gcc.dg/inline_4.c
index dd4fadb..ebd57e9 100644
--- a/gcc/testsuite/gcc.dg/inline_4.c
+++ b/gcc/testsuite/gcc.dg/inline_4.c
@@ -1,5 +1,6 @@
/* { dg-do compile } */
/* { dg-options -O2 -fdump-tree-optimized -fdisable-tree-einline=foo2 -fdisable-ipa-inline -Wno-attributes } */
+/* { dg-add-options bind_pic_locally } */
int g;
__attribute__((always_inline)) void bar (void)
{
Re: [C++ Patch] PR 43453
On 05/26/2014 11:41 AM, Paolo Carlini wrote:
+ /* C++11 8.5/17: If the destination type is an array of characters,
+an array of char16_t, an array of char32_t, or an array of wchar_t,
+and the initializer is a string literal */
+ else if (TREE_CODE (newrhs) == STRING_CST
+ char_type_p (TREE_TYPE (TYPE_MAIN_VARIANT (lhstype)))
+ modifycode == INIT_EXPR)
+ {
+ newrhs = digest_init (lhstype, newrhs, complain);
+ if (newrhs == error_mark_node)
+ return error_mark_node;
+ }
This hunk is OK.
if (TREE_CODE (init) == TREE_LIST
- TREE_CODE (TREE_TYPE (decl)) == ARRAY_TYPE)
+ TREE_CODE (type) == ARRAY_TYPE
+ /* C++11 8.5/17: If the destination type is an array of characters,
+an array of char16_t, an array of char32_t, or an array of wchar_t,
+and the initializer is a string literal */
+ !(char_type_p (TREE_TYPE (TYPE_MAIN_VARIANT (type)))
+ TREE_CODE (TREE_VALUE (init)) == STRING_CST))
{
error (cannot initialize arrays using this syntax);
return NULL_TREE;
Can we just remove this error rather than add a condition? I think it's
covered in other places.
The patch doesn't seem to fix
char s[] (foo);
which I think also ought to be well-formed.
Jason
[PATCH] Fix non-register vector temporaries created from SAVE_EXPRs
This removes an odd restriction in lookup_tmp_vars helper
create_tmp_from_val. That conditionalizes whether vector and complex
variables get to have a register based on whether the tmp var
is a formal one or not. But that can't matter for correctness,
either all temporaries can get a register or none.
The only case is prepare_gimple_addressable which is used to
avoid marking a register as addressable by creating a new
temporary (otherwise previous other gimplifications could need
revisiting). But here we can safely un-set DECL_GIMPLE_REG_P
before the new temporary gets further uses.
This improves initial code-gen for gcc.dg/tree-ssa/forwprop-19.c
for example when gimplfying
vec z = VEC_PERM_EXPR SAVE_EXPR y , SAVE_EXPR y , m ;
creating
y.0 = y;
z = VEC_PERM_EXPR y.0, y.0, m;
instead of two extra loads from y.0 for the uses in the VEC_PERM_EXPR.
Thus the following, bootstrapped on x86_64-unknown-linux-gnu,
testing in progress.
Richard.
2014-06-04 Richard Biener rguent...@suse.de
* gimplify.c (create_tmp_from_val): Remove is_formal parameter
and set DECL_GIMPLE_REG_P unconditionally if appropriate.
(lookup_tmp_var): Adjust.
(prepare_gimple_addressable): Unset DECL_GIMPLE_REG_P here.
Index: gcc/gimplify.c
===
--- gcc/gimplify.c (revision 211227)
+++ gcc/gimplify.c (working copy)
@@ -451,14 +451,13 @@ is_gimple_mem_rhs_or_call (tree t)
lookup_tmp_var; nobody else should call this function. */
static inline tree
-create_tmp_from_val (tree val, bool is_formal)
+create_tmp_from_val (tree val)
{
/* Drop all qualifiers and address-space information from the value type. */
tree type = TYPE_MAIN_VARIANT (TREE_TYPE (val));
tree var = create_tmp_var (type, get_name (val));
- if (is_formal
- (TREE_CODE (TREE_TYPE (var)) == COMPLEX_TYPE
- || TREE_CODE (TREE_TYPE (var)) == VECTOR_TYPE))
+ if (TREE_CODE (TREE_TYPE (var)) == COMPLEX_TYPE
+ || TREE_CODE (TREE_TYPE (var)) == VECTOR_TYPE)
DECL_GIMPLE_REG_P (var) = 1;
return var;
}
@@ -477,7 +476,7 @@ lookup_tmp_var (tree val, bool is_formal
work in reload and final and poorer code generation, outweighing
the extra memory allocation here. */
if (!optimize || !is_formal || TREE_SIDE_EFFECTS (val))
-ret = create_tmp_from_val (val, is_formal);
+ret = create_tmp_from_val (val);
else
{
elt_t elt, *elt_p;
@@ -491,7 +490,7 @@ lookup_tmp_var (tree val, bool is_formal
{
elt_p = XNEW (elt_t);
elt_p-val = val;
- elt_p-temp = ret = create_tmp_from_val (val, is_formal);
+ elt_p-temp = ret = create_tmp_from_val (val);
*slot = elt_p;
}
else
@@ -3154,7 +3153,11 @@ prepare_gimple_addressable (tree *expr_p
while (handled_component_p (*expr_p))
expr_p = TREE_OPERAND (*expr_p, 0);
if (is_gimple_reg (*expr_p))
-*expr_p = get_initialized_tmp_var (*expr_p, seq_p, NULL);
+{
+ tree var = get_initialized_tmp_var (*expr_p, seq_p, NULL);
+ DECL_GIMPLE_REG_P (var) = 0;
+ *expr_p = var;
+}
}
/* A subroutine of gimplify_modify_expr. Replace a MODIFY_EXPR with
Re: [PATCH, Pointer Bounds Checker 19/x] Support bounds in expand
Hi, On Mon, 2 Jun 2014, Ilya Enkovich wrote: There is exactly one place (except for the self-recursive ones) where you call the new store_expr with a non-null argument for bounds target, and it seems to be only necessary for when some sub-expression of the RHS is a call. Can you somehow arrange to move that handling to the single place in expand_assignment() so that you don't need to change the signature of store_expr? I see the only nice way to do it - store_expr should return bounds of expanded exp. Currently it always return NULL_RTX. Does it look better than a new argument? IMHO it does. That or introducing a new store_expr_with_bounds (with the new argument) and letting store_expr be a wrapper for that, passing the NULL. Basically anything that avoids adding a new parameter for most of the existing calls to store_expr. Ciao, Michael.
Re: [PATCH, PR 61391]
Richard. Here is update patch with test-case and new ChangeLog. gcc/ChangeLog 2014-06-04 Yuri Rumyantsev ysrum...@gmail.com PR tree-optimization/61319 * tree-if-conv.c (is_cond_scalar_reduction): Add missed check that stmt belongs to loop. gcc/testsuite/ChangeLog * gcc.dg/torture/pr61319.c: New test. 2014-06-04 17:37 GMT+04:00 Richard Biener richard.guent...@gmail.com: On Wed, Jun 4, 2014 at 3:37 PM, Richard Biener richard.guent...@gmail.com wrote: On Wed, Jun 4, 2014 at 3:17 PM, Yuri Rumyantsev ysrum...@gmail.com wrote: Hi All, Here is a simple fix for 61391 - missed a check that statement basic block is inside loop. With this fix test-case from bug is compiled successfully. Bootstrap and regression testing did not show any new failures. Is it OK for trunk? Ok. Please also add the testcase. Thanks, Richard. ChangeLog: 2014-06-04 Yuri Rumyantsev ysrum...@gmail.com PR tree-optimization/61319 * tree-if-conv.c (is_cond_scalar_reduction): Add missed check that stmt belongs to loop. patch1 Description: Binary data
Re: [PATCH, PR 61391]
On Wed, Jun 04, 2014 at 07:11:26PM +0400, Yuri Rumyantsev wrote: Here is update patch with test-case and new ChangeLog. If approved, please avoid the DOS style line endings in the testcase. Jakub
Re: [PATCH, PR 61391]
Yuri Rumyantsev ysrum...@gmail.com writes: Property changes on: testsuite/gcc.dg/torture/pr61391.c ___ Added: svn:executable Don't make the file executable. Andreas. -- Andreas Schwab, SUSE Labs, sch...@suse.de GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE 1748 E4D4 88E3 0EEA B9D7 And now for something completely different.
Re: [PATCH, PR 61391]
I converted test-case to Unix format and new patch is attached. 2014-06-04 19:14 GMT+04:00 Jakub Jelinek ja...@redhat.com: On Wed, Jun 04, 2014 at 07:11:26PM +0400, Yuri Rumyantsev wrote: Here is update patch with test-case and new ChangeLog. If approved, please avoid the DOS style line endings in the testcase. Jakub patch2 Description: Binary data
Re: [PATCH, PR 61391]
Sorry, I sent you 'bad' patch, resend it. 2014-06-04 19:19 GMT+04:00 Yuri Rumyantsev ysrum...@gmail.com: I converted test-case to Unix format and new patch is attached. 2014-06-04 19:14 GMT+04:00 Jakub Jelinek ja...@redhat.com: On Wed, Jun 04, 2014 at 07:11:26PM +0400, Yuri Rumyantsev wrote: Here is update patch with test-case and new ChangeLog. If approved, please avoid the DOS style line endings in the testcase. Jakub patch2 Description: Binary data
Re: [patch, mips, tree] align microMIPS functions to 16 bits with -Os
On 06/04/2014 06:20 AM, Richard Biener wrote:
On Tue, 3 Jun 2014, Richard Sandiford wrote:
Richi suggested just changing the alignment at output time. I assume
that would be a case of replacing the DECL_ALIGN in:
/* Tell assembler to move to target machine's alignment for functions. */
align = floor_log2 (DECL_ALIGN (decl) / BITS_PER_UNIT);
if (align 0)
{
ASM_OUTPUT_ALIGN (asm_out_file, align);
}
with a hook. (Is that right?)
Yeah, kind of. Of course if DECL_ALIGN on function-decls is unused
then we may as well initialize it to 1 in tree.c and at an appropriate
stage adjust it to the result of a target hook invocation.
Appropriate stage would be the above place (which means DECL_ALIGN
is essentially unused for FUNCTION_DECLs).
So ... can you massage the DECL_ALIGN macro to ICE on FUNCTION_DECLs
and see where we access it? (generic code will possibly trip on it,
but the question is is there any user that cares?)
Well, offhand, I know that one of the places is in
handle_aligned_attribute, in c-common/c-common.c, where we handle
user-specified alignment attributes. Here we are potentially both
reading and writing the DECL_ALIGN field of a FUNCTION_DECL.
-Sandra
C++ PATCH for c++/61382 (init-list evaluation order)
My patch for 51253 didn't handle all cases of this; I was forgetting
that when we direct-initialize a variable, we never build an
AGGR_INIT_EXPR. So let's handle this in cp_gimplify_expr.
Tested x86_64-pc-linux-gnu, applying to trunk.
commit 2cddc650aac580698d4ff47c31670f168aa3ce31
Author: Jason Merrill ja...@redhat.com
Date: Wed Jun 4 10:46:15 2014 -0400
PR c++/51253
PR c++/61382
gcc/
* gimplify.c (gimplify_arg): Non-static.
* gimplify.h: Declare it.
gcc/cp/
* cp-gimplify.c (cp_gimplify_expr): Handle CALL_EXPR_LIST_INIT_P here.
* semantics.c (simplify_aggr_init_expr): Not here, just copy it.
diff --git a/gcc/cp/cp-gimplify.c b/gcc/cp/cp-gimplify.c
index ef4b043..18142bf 100644
--- a/gcc/cp/cp-gimplify.c
+++ b/gcc/cp/cp-gimplify.c
@@ -723,6 +723,27 @@ cp_gimplify_expr (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p)
!seen_error ())
return (enum gimplify_status) gimplify_cilk_spawn (expr_p);
+ /* DR 1030 says that we need to evaluate the elements of an
+ initializer-list in forward order even when it's used as arguments to
+ a constructor. So if the target wants to evaluate them in reverse
+ order and there's more than one argument other than 'this', gimplify
+ them in order. */
+ ret = GS_OK;
+ if (PUSH_ARGS_REVERSED CALL_EXPR_LIST_INIT_P (*expr_p)
+ call_expr_nargs (*expr_p) 2)
+ {
+ int nargs = call_expr_nargs (*expr_p);
+ location_t loc = EXPR_LOC_OR_LOC (*expr_p, input_location);
+ for (int i = 1; i nargs; ++i)
+ {
+ enum gimplify_status t
+ = gimplify_arg (CALL_EXPR_ARG (*expr_p, i), pre_p, loc);
+ if (t == GS_ERROR)
+ ret = GS_ERROR;
+ }
+ }
+ break;
+
default:
ret = (enum gimplify_status) c_gimplify_expr (expr_p, pre_p, post_p);
break;
diff --git a/gcc/cp/semantics.c b/gcc/cp/semantics.c
index c1c16f4..ca0c34b 100644
--- a/gcc/cp/semantics.c
+++ b/gcc/cp/semantics.c
@@ -3869,6 +3869,7 @@ simplify_aggr_init_expr (tree *tp)
aggr_init_expr_nargs (aggr_init_expr),
AGGR_INIT_EXPR_ARGP (aggr_init_expr));
TREE_NOTHROW (call_expr) = TREE_NOTHROW (aggr_init_expr);
+ CALL_EXPR_LIST_INIT_P (call_expr) = CALL_EXPR_LIST_INIT_P (aggr_init_expr);
tree ret = call_expr;
if (style == ctor)
@@ -3900,20 +3901,6 @@ simplify_aggr_init_expr (tree *tp)
ret = build2 (COMPOUND_EXPR, TREE_TYPE (slot), ret, slot);
}
- /* DR 1030 says that we need to evaluate the elements of an
- initializer-list in forward order even when it's used as arguments to
- a constructor. So if the target wants to evaluate them in reverse
- order and there's more than one argument other than 'this', force
- pre-evaluation. */
- if (PUSH_ARGS_REVERSED CALL_EXPR_LIST_INIT_P (aggr_init_expr)
- aggr_init_expr_nargs (aggr_init_expr) 2)
-{
- tree preinit;
- stabilize_call (call_expr, preinit);
- if (preinit)
- ret = build2 (COMPOUND_EXPR, TREE_TYPE (ret), preinit, ret);
-}
-
if (AGGR_INIT_ZERO_FIRST (aggr_init_expr))
{
tree init = build_zero_init (type, NULL_TREE,
diff --git a/gcc/gimplify.c b/gcc/gimplify.c
index 92714b5..89ae41f 100644
--- a/gcc/gimplify.c
+++ b/gcc/gimplify.c
@@ -2170,7 +2170,7 @@ maybe_with_size_expr (tree *expr_p)
Store any side-effects in PRE_P. CALL_LOCATION is the location of
the CALL_EXPR. */
-static enum gimplify_status
+enum gimplify_status
gimplify_arg (tree *arg_p, gimple_seq *pre_p, location_t call_location)
{
bool (*test) (tree);
diff --git a/gcc/gimplify.h b/gcc/gimplify.h
index 47e7213..5085ccf 100644
--- a/gcc/gimplify.h
+++ b/gcc/gimplify.h
@@ -77,6 +77,7 @@ extern enum gimplify_status gimplify_expr (tree *, gimple_seq *, gimple_seq *,
extern void gimplify_type_sizes (tree, gimple_seq *);
extern void gimplify_one_sizepos (tree *, gimple_seq *);
extern gimple gimplify_body (tree, bool);
+extern enum gimplify_status gimplify_arg (tree *, gimple_seq *, location_t);
extern void gimplify_function_tree (tree);
extern enum gimplify_status gimplify_va_arg_expr (tree *, gimple_seq *,
gimple_seq *);
diff --git a/gcc/testsuite/g++.dg/cpp0x/initlist86.C b/gcc/testsuite/g++.dg/cpp0x/initlist86.C
new file mode 100644
index 000..16af476
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/initlist86.C
@@ -0,0 +1,18 @@
+// PR c++/61382
+// { dg-do run { target c++11 } }
+
+struct A
+{
+ int i,j;
+ A(int i,int j):i(i),j(j){}
+};
+
+extern C int printf (const char *, ...);
+
+int main()
+{
+ int i;
+ A a{i++,i++};
+ if (a.i != 0 || a.j != 1)
+__builtin_abort();
+}
[PATCH] Don't call init_caller_save if LRA enabled
LRA generate caller-save register store/restore during split register, not generate by caller-save.c:save_call_clobbered_regs, so initialize this module by init_caller_save is meaningless if LRA enabled 2014-06-05 Kito Cheng k...@0xlab.org * ira.c (ira): Don't call init_caller_save if LRA enabled since LRA use its own infrastructure to handle that. diff --git a/gcc/ira.c b/gcc/ira.c index eebd410..5400df4 100644 --- a/gcc/ira.c +++ b/gcc/ira.c @@ -5167,7 +5167,8 @@ ira (FILE *f) #endif bitmap_obstack_initialize (ira_bitmap_obstack); - if (flag_caller_saves) + /* LRA use its own infrastructure to handle caller save register. */ + if (flag_caller_saves !ira_use_lra_p) init_caller_save (); if (flag_ira_verbose 10)
Re: [PATCH] Fix logic for detection of zero initializer (PR c/53119)
On Wed, Jun 04, 2014 at 03:15:02PM +0200, Marek Polacek wrote:
On Mon, Jun 02, 2014 at 12:58:53AM -0400, S. Gilles wrote:
Thanks for tackling this.
@@ -6858,6 +6858,9 @@
/* 1 if this constructor is erroneous so far. */
static int constructor_erroneous;
+/* 1 if this constructor is the universal zero initializer { 0 } */
. */ at the end of the comment, please.
Applied.
@@ -7317,12 +7321,8 @@
set_nonincremental_init (braced_init_obstack);
}
- if (implicit == 1 warn_missing_braces !missing_braces_mentioned)
-{
- missing_braces_mentioned = 1;
- warning_init (input_location, OPT_Wmissing_braces,
- missing braces around initializer);
-}
+ if (implicit == 1)
+ found_missing_braces = 1;
Wrong indentation, there should be only two spaces.
Applied.
+ /* Warn when some structs are initialized with direct aggregation. */
+ if (!implicit found_missing_braces warn_missing_braces
+ !constructor_zeroinit)
+{
+ warning_init (input_location, OPT_Wmissing_braces,
+ missing braces around initializer);
+}
+
Please use loc instead of input_location.
Applied.
@@ -8594,6 +8598,11 @@
designator_depth = 0;
designator_erroneous = 0;
+ if (!implicit value.value !integer_zerop (value.value))
+{
+ constructor_zeroinit = 0;
+}
+
You can drop the braces here.
Applied - thanks for catching these.
But the problem I see is that this patch treats even { 0, 0 } as
a universal zero initializer, thus we wouldn't warn on say:
/* -Wmissing-braces -Wmissing-field-initializers */
struct T { int a, b; };
struct U { int a, b; struct T t; };
void
f (void)
{
int a[2][2][2] = { 0, 0, 0 };
struct U u = { 0, 0 };
}
I'm not sure whether that is desirable.
That is/was partially intentional. I probably should have mentioned that
originally, especially with the { 0, 0 } in the test case.
I believe an argument can be made that this behavior isn't undesirable:
the standard (unless I am mistaken) makes initialization to any number
of zeros equivalent (as long as there are not more zeros than elements),
so anyone who initializes a struct to { 0, 0 } is probably trying to
zero it out. If they don't use braces when they should, or if they use
six zeros instead of seven, the error will result in the structure being
initialized as they wanted anyway. If they use nonzero values for part
of their initialization, then there's a chance that the definition has
changed, that they aren't intentionally applying aggregation, etc.
As I write it, though, that argument seems far less compelling than when
I thought of it, especially since, with this patch, an assignment of
something like { { 0, 0 }, 0 } would cause warnings, and the `it still
probably works as they intended' argument would still apply. It makes
much more sense to have { 0 } as the only exception to properly filling
out an assignment.
Therefore, I have also amended the patch to add the length-checking from
the previous logic. The test case is also updated to make sure that
holds. The amended patch is attached (along with the ChangeLog entries
from the first version).
Marek
--
S. Gilles
Index: gcc/c/c-typeck.c
===
--- gcc/c/c-typeck.c (revision 211081)
+++ gcc/c/c-typeck.c (working copy)
@@ -74,9 +74,9 @@
if expr.original_code == SIZEOF_EXPR. */
tree c_last_sizeof_arg;
-/* Nonzero if we've already printed a missing braces around initializer
- message within this initializer. */
-static int missing_braces_mentioned;
+/* Nonzero if we might need to print a missing braces around
+ initializer message within this initializer. */
+static int found_missing_braces;
static int require_constant_value;
static int require_constant_elements;
@@ -6858,6 +6858,9 @@
/* 1 if this constructor is erroneous so far. */
static int constructor_erroneous;
+/* 1 if this constructor is the universal zero initializer { 0 }. */
+static int constructor_zeroinit;
+
/* Structure for managing pending initializer elements, organized as an
AVL tree. */
@@ -7019,7 +7022,7 @@
constructor_stack = 0;
constructor_range_stack = 0;
- missing_braces_mentioned = 0;
+ found_missing_braces = 0;
spelling_base = 0;
spelling_size = 0;
@@ -7114,6 +7117,7 @@
constructor_type = type;
constructor_incremental = 1;
constructor_designated = 0;
+ constructor_zeroinit = 1;
designator_depth = 0;
designator_erroneous = 0;
@@ -7317,12 +7321,8 @@
set_nonincremental_init (braced_init_obstack);
}
- if (implicit == 1 warn_missing_braces !missing_braces_mentioned)
-{
- missing_braces_mentioned = 1;
- warning_init (input_location, OPT_Wmissing_braces,
- missing braces around initializer);
-}
+ if (implicit == 1)
+found_missing_braces = 1;
if
Re: emit __float128 typeinfo
How about, in emit_support_tinfos, using type_for_mode to check for a TF-mode floating point type different from long_double_type_node? Jason
Re: [PATCH 2/2, x86] Add palignr support for AVX2.
Is it ok to use the following pattern?
patch passed bootstrap and make check, but one test failed:
gcc/testsuite/gcc.target/i386/vect-rebuild.c
It failed on /* { dg-final { scan-assembler-times \tv?permilpd\[ \t\] 1 } } */
which is now palignr. However, both palignr and permilpd costs 1 tick
and take 6 bytes in the opcode.
I vote for modifying the test to scan for palignr:
/* { dg-final { scan-assembler-times \tv?palignr\[ \t\] 1 } } */
2014-06-04 Evgeny Stupachenko evstu...@gmail.com
* config/i386/sse.md (*ssse3_palignrmode_perm): New.
* config/i386/predicates.md (palignr_operand): New.
Indicates if permutation is suitable for palignr instruction.
diff --git a/gcc/config/i386/predicates.md b/gcc/config/i386/predicates.md
index 2ef1384..8266f3e 100644
--- a/gcc/config/i386/predicates.md
+++ b/gcc/config/i386/predicates.md
@@ -1417,6 +1417,22 @@
return true;
})
+;; Return true if OP is a parallel for a palignr permute.
+(define_predicate palignr_operand
+ (and (match_code parallel)
+ (match_code const_int a))
+{
+ int elt = INTVAL (XVECEXP (op, 0, 0));
+ int i, nelt = XVECLEN (op, 0);
+
+ /* Check that an order in the permutation is suitable for palignr.
+ For example, {5 6 7 0 1 2 3 4} is palignr 5, xmm, xmm. */
+ for (i = 1; i nelt; ++i)
+if (INTVAL (XVECEXP (op, 0, i)) != ((elt + i) % nelt))
+ return false;
+ return true;
+})
+
;; Return true if OP is a proper third operand to vpblendw256.
(define_predicate avx2_pblendw_operand
(match_code const_int)
diff --git a/gcc/config/i386/sse.md b/gcc/config/i386/sse.md
index c91626b..5e8fd65 100644
--- a/gcc/config/i386/sse.md
+++ b/gcc/config/i386/sse.md
@@ -11454,6 +11454,36 @@
}
})
+(define_insn *ssse3_palignrmode_perm
+ [(set (match_operand:V_128 0 register_operand =x,x)
+ (vec_select:V_128
+ (match_operand:V_128 1 register_operand 0,x)
+ (match_parallel 2 palignr_operand
+ [(match_operand 3 const_int_operand n, n)])))]
+ TARGET_SSSE3
+{
+ enum machine_mode imode = GET_MODE_INNER (GET_MODE (operands[0]));
+ operands[2] = GEN_INT (INTVAL (operands[3]) * GET_MODE_SIZE (imode));
+
+ switch (which_alternative)
+{
+case 0:
+ return palignr\t{%2, %1, %0|%0, %1, %2};
+case 1:
+ return vpalignr\t{%2, %1, %1, %0|%0, %1, %1, %2};
+default:
+ gcc_unreachable ();
+}
+}
+ [(set_attr isa noavx,avx)
+ (set_attr type sseishft)
+ (set_attr atom_unit sishuf)
+ (set_attr prefix_data16 1,*)
+ (set_attr prefix_extra 1)
+ (set_attr length_immediate 1)
+ (set_attr prefix orig,vex)])
+
+
(define_insn absmode2
[(set (match_operand:MMXMODEI 0 register_operand =y)
(abs:MMXMODEI
On Mon, May 19, 2014 at 8:00 PM, Richard Henderson r...@redhat.com wrote:
On 05/05/2014 09:54 AM, Evgeny Stupachenko wrote:
@@ -42943,6 +42944,10 @@ expand_vec_perm_1 (struct expand_vec_perm_d *d)
if (expand_vec_perm_vpermil (d))
return true;
+ /* Try palignr on one operand. */
+ if (d-one_operand_p expand_vec_perm_palignr (d))
+return true;
No, because unless in_order and SSSE3, expand_vec_perm_palignr generates at
least 2 insns, and by contract expand_vec_perm_1 must generate only one.
I think what might help you out is to have the rotate permutation matched
directly, rather than have to have it converted to a shift.
Thus I think you'd do well to start this series with a patch that adds a
pattern of the form
(define_insn *ssse3_palignrmode_perm
[(set (match_operand:V_128 0 register_operand =x,x)
(vec_select:V_128
(match_operand:V_128 1 register_operand 0,x)
(match_operand:V_128 2 nonimmediate_operand xm,xm)
(match_parallel 3 palign_operand
[(match_operand 4 const_int_operand )]
TARGET_SSSE3
{
enum machine_mode imode = GET_INNER_MODE (GET_MODE (operands[0]));
operands[3] = GEN_INT (INTVAL (operands[4]) * GET_MODE_SIZE (imode));
switch (which_alternative)
{
case 0:
return palignr\t{%3, %2, %0|%0, %2, %3};
case 1:
return vpalignr\t{%3, %2, %1, %0|%0, %1, %2, %3};
default:
gcc_unreachable ();
}
}
[(set_attr isa noavx,avx)
(set_attr type sseishft)
(set_attr atom_unit sishuf)
(set_attr prefix_data16 1,*)
(set_attr prefix_extra 1)
(set_attr length_immediate 1)
(set_attr prefix orig,vex)])
where the palign_operand function verifies that the constants are all in
order.
This is very similar to the way we define the broadcast type patterns.
You'll need a similar pattern with a different predicate for the avx2 palignr,
since it's not a simple increment, but also verifying the cross-lane
constraint.
With that as patch 1/1, I believe that will significantly tidy up what else
you're attempting to change with this series.
r~
[gomp4] Missing -fopenacc handling (was: Dumping gimple for offload.)
Hi!
On Tue, 19 Nov 2013 13:58:29 +0400, Ilya Tocar tocarip.in...@gmail.com wrote:
* cgraph.h (symtab_node): Add need_dump.
* cgraphunit.c (ipa_passes): Run ipa_write_summaries for omp.
(compile): Intialize streamer for omp.
* ipa-inline-analysis.c (inline_generate_summary): Add flag_openmp.
* lto-cgraph.c (lto_set_symtab_encoder_in_partition): Respect
need_dump flag.
(select_what_to_dump): New.
* lto-streamer.c (section_name_prefix): New.
(lto_get_section_name): Use section_name_prefix.
(lto_streamer_init): Add flag_openmp.
* lto-streamer.h (OMP_SECTION_NAME_PREFIX): New.
(section_name_prefix): Ditto.
(select_what_to_dump): Ditto.
* lto/lto-partition.c (add_symbol_to_partition_1): Set need_dump.
(lto_promote_cross_file_statics): Dump everyhtinh.
* passes.c (ipa_write_summaries): Add parameter,
call select_what_to_dump.
* tree-pass.h (void ipa_write_summaries): Add parameter.
In several places, this needs to consider flag_openacc, too; r211236:
commit 02da7b3beb9f8e9d3dbb8214c0a6ebaccf407deb
Author: tschwinge tschwinge@138bc75d-0d04-0410-961f-82ee72b054a4
AuthorDate: Wed Jun 4 17:11:30 2014 +
Commit: tschwinge tschwinge@138bc75d-0d04-0410-961f-82ee72b054a4
CommitDate: Wed Jun 4 17:11:30 2014 +
Missing -fopenacc handling.
gcc/
* cgraphunit.c (ipa_passes, compile): Handle flag_openacc next to
flag_openmp.
* ipa-inline-analysis.c (inline_generate_summary): Likewise.
* lto-streamer.c (lto_streamer_init, gate_lto_out): Likewise.
* passes.c (ipa_write_summaries): Likewise.
git-svn-id: svn+ssh://gcc.gnu.org/svn/gcc/branches/gomp-4_0-branch@211236
138bc75d-0d04-0410-961f-82ee72b054a4
---
gcc/ChangeLog.gomp| 8
gcc/cgraphunit.c | 4 ++--
gcc/ipa-inline-analysis.c | 2 +-
gcc/lto-streamer.c| 2 +-
gcc/passes.c | 2 +-
5 files changed, 13 insertions(+), 5 deletions(-)
diff --git gcc/ChangeLog.gomp gcc/ChangeLog.gomp
index b543bbc..011fe77 100644
--- gcc/ChangeLog.gomp
+++ gcc/ChangeLog.gomp
@@ -1,3 +1,11 @@
+2014-06-04 Thomas Schwinge tho...@codesourcery.com
+
+ * cgraphunit.c (ipa_passes, compile): Handle flag_openacc next to
+ flag_openmp.
+ * ipa-inline-analysis.c (inline_generate_summary): Likewise.
+ * lto-streamer.c (lto_streamer_init, gate_lto_out): Likewise.
+ * passes.c (ipa_write_summaries): Likewise.
+
2014-05-12 Bernd Schmidt ber...@codesourcery.com
* lto-wrapper.c (ompbegin): New static variable.
diff --git gcc/cgraphunit.c gcc/cgraphunit.c
index c524120..fb34c2d 100644
--- gcc/cgraphunit.c
+++ gcc/cgraphunit.c
@@ -2086,7 +2086,7 @@ ipa_passes (void)
if (!in_lto_p)
{
- if (flag_openmp)
+ if (flag_openacc || flag_openmp)
{
section_name_prefix = OMP_SECTION_NAME_PREFIX;
ipa_write_summaries (true);
@@ -2187,7 +2187,7 @@ compile (void)
cgraph_state = CGRAPH_STATE_IPA;
/* If LTO is enabled, initialize the streamer hooks needed by GIMPLE. */
- if (flag_lto || flag_openmp)
+ if (flag_lto || flag_openacc || flag_openmp)
lto_streamer_hooks_init ();
/* Don't run the IPA passes if there was any error or sorry messages. */
diff --git gcc/ipa-inline-analysis.c gcc/ipa-inline-analysis.c
index 176954a..340f26a 100644
--- gcc/ipa-inline-analysis.c
+++ gcc/ipa-inline-analysis.c
@@ -4010,7 +4010,7 @@ inline_generate_summary (void)
/* When not optimizing, do not bother to analyze. Inlining is still done
because edge redirection needs to happen there. */
- if (!optimize !flag_lto !flag_wpa !flag_openmp)
+ if (!optimize !flag_lto !flag_wpa !flag_openacc !flag_openmp)
return;
function_insertion_hook_holder =
diff --git gcc/lto-streamer.c gcc/lto-streamer.c
index 6153b0a..e9fc6e1 100644
--- gcc/lto-streamer.c
+++ gcc/lto-streamer.c
@@ -316,7 +316,7 @@ lto_streamer_init (void)
bool
gate_lto_out (void)
{
- return ((flag_generate_lto || in_lto_p || flag_openmp)
+ return ((flag_generate_lto || in_lto_p || flag_openacc || flag_openmp)
/* Don't bother doing anything if the program has errors. */
!seen_error ());
}
diff --git gcc/passes.c gcc/passes.c
index 620376e..e4c2336 100644
--- gcc/passes.c
+++ gcc/passes.c
@@ -2335,7 +2335,7 @@ ipa_write_summaries (bool is_omp)
struct cgraph_node *node;
struct cgraph_node **order;
- if (!(flag_generate_lto || flag_openmp) || seen_error () )
+ if (!(flag_generate_lto || flag_openacc || flag_openmp) || seen_error () )
return;
select_what_to_dump (is_omp);
Grüße,
Thomas
pgpVGbV8lqs8d.pgp
Description: PGP signature
Re: Fix a function decl in gfortran
On 06/04/2014 09:40 AM, Tobias Burnus wrote: Still untested patch, but I cannot resist pointing out stupid typos by myself. I intent to tests the build and test the patch - and then to commit it as obvious. If you see problems with this approach please scream now. Even with this applied, I'm still seeing similar failures. I'm now looking at gfortran.dg/realloc_on_assign_1.f03 and related testcases. (gdb) p gfc_debug_expr(gfc_expr*) (expr) _gfortran_reshape_4[[((MAIN__:src(FULL)) ((/ , /)) ((arg not-present)) ((arg not-present)))]] [later:] (gdb) p exp $17 = call_expr 0x7064ac60 (gdb) pge _gfortran_reshape_4 (_631, _630, _629, 0B, 0B) (gdb) p fndecl $18 = function_decl 0x707e5b00 _gfortran_reshape_4 (gdb) pt function_decl 0x707e5b00 _gfortran_reshape_4 type function_type type void_type void VOID align 8 symtab 0 alias set -1 canonical type pointer_to_this pointer_type SI size integer_cst constant 32 unit size integer_cst constant 4 align 32 symtab 0 alias set -1 canonical type attributes tree_list purpose identifier_node fn spec value tree_list 0x707fddc0 value string_cst 0x707f8f60 constant .wrr arg-types tree_list value reference_type chain tree_list value reference_type chain tree_list value reference_type chain tree_list value void_type void pointer_to_this pointer_type and in libgfortran: void reshape_4 (gfc_array_i4 * const restrict ret, gfc_array_i4 * const restrict source, shape_type * const restrict shape, gfc_array_i4 * const restrict pad, shape_type * const restrict order) So we're seeing more arguments in the call than on the decl (oddly the call only has four arguments at the assembly level, but that might a problem with my backend - I'll investigate). Bernd
Re: [PATCH 3/5] Make recog_op_alt consumers check the enabled attribute
Thanks for the reviews, now committed Jeff Law l...@redhat.com writes: On 05/31/14 03:15, Richard Sandiford wrote: As described in the covering note, it seems better to put the onus of checking the enabled attribute on the passes that are walking each alternative, like LRA does for its internal subpasses. That leads to a nicer interface in patch 4 and would make it easier to precompute the information at build time. (The only thing preventing that now is the subunion class.) Thanks, Richard gcc/ * recog.c (preprocess_constraints): Don't skip disabled alternatives. * ira-lives.c (check_and_make_def_conflict): Check for disabled alternatives. (make_early_clobber_and_input_conflicts): Likewise. * config/i386/i386.c (ix86_legitimate_combined_insn): Likewise. Did you spot check the other ports which utilized the enabled attribute to see if they need tweaking too? I see aarch64, alpha, arc, arm, avr, c6x m68k, mips, mn10300, nds32, s390, sh sparc. I didn't check to see if any of them walk the alternatives in the backend. Yeah, the only port besides i386 to use preprocess_constraints is arm, but it only looks at alternative which_alternative. Richard
Re: [PATCH 0/5] Cache recog_op_alt by insn code, take 2
Jeff Law l...@redhat.com writes: On 05/31/14 03:02, Richard Sandiford wrote: A second difference was that preprocess_constraints skips disabled alternatives while LRA's setup_operand_alternative doesn't; LRA just checks for disabled alternatives when walking the array instead. That should make no difference in practice, but since the LRA approach would also make it easier to precompute the array at build time, I thought it would be a better way to go. It also gives a cleaner interface: we can have a preprocess_insn_constraints function that takes a (define_insn-based) insn and returns the operand_alternative information without any global state. Sounds good. Do you think prebuilding those arrays once at build time is likely to have a measurable impact? I realize you probably haven't done measurements, just looking for your gut instinct here. I have a local patch that does it, but unfortunately it doesn't seem to make a measurable difference. I'll try measuring it again once other lower-hanging fruit are out of the way. Also, I hadn't realised that a define_insn with no constraints at all has 0 alternatives rather than 1. Some passes nevertheless unconditionally access the recog_op_alt information for alternative which_alternative. I could have fixed that by making n_alternatives always be = 1 in the static insn table. Several places do have fast paths for n_alternatives == 0 though, so I thought it would be better to handle the 0 alternative case in preprocess_constraints as well. All it needs is a MIN (..., 1). Funny thing is I suspect the n_alternatives == 0 case is rare in practice though. Yeah, probably. :-) Thanks, Richard
Re: RFA: Make LRA temporarily eliminate addresses before testing constraints
Vladimir Makarov vmaka...@redhat.com writes: On 06/02/2014 03:36 PM, Richard Sandiford wrote: Ping. Imagination's copyright assignment has now gone through and so in principle we're ready for the MIPS LRA switch to go in. We need this LRA patch as a prequisite though. Robert: you also had an LRA change, but is it still needed after this one? If so, could you repost it and explain the case it handles? Thanks, Richard Richard Sandiford rdsandif...@googlemail.com writes: Richard Sandiford rdsandif...@googlemail.com writes: I think a cleaner way of doing it would be to have helper functions that switch in and out of the eliminated form, storing the old form in fields of a new structure (either separate from address_info, or a local inheritance of it). We probably also want to have arrays of address_infos, one for each operand, so that we don't analyse the same address too many times during the same insn. In the end maintaining the array of address_infos seemed like too much work. It was hard to keep it up-to-date with various other changes that can be made, including swapping commutative operands, to the point where it wasn't obvious whether it was really an optimisation or not. Here's a patch that does the first. Tested on x86_64-linux-gnu. This time I also compared the assembly output for gcc.dg, g++.dg and gcc.c-torture at -O2 on: arch64-linux-gnu arm-eabi mipsisa64-sde-elf s390x-linux-gnu powerpc64-linux-gnu x86_64-linux-gnu s390x in particular is very good at exposing problems with this code. (It caught bugs in the aborted attempt to keep an array of address_infos.) OK to install? Yes, Richard. Thanks for the wide testing. Thanks Vlad, committed as r211242. Given the problems with my first version, I suppose it'd make sense to wait a few days to see whether there's any fallout before applying the MIPS backend patch. Richard
Re: [build, driver] RFC: Support compressed debug sections
If it is just to reach compatibility with the debugger, then I’d rather either just mandate a certain debugger or autoconf for what the current debugger supports. As of late people seem to just break the debugging experience with non-updated gdbs and assume that a newer gdb is used. You cannot do that: unlike the assembler and linker used, which are often hardcoded into gcc, the debugger can easily be changed below the compiler's feet, so to speak. Besides, on several platforms, you have more than one debugger available (like gdb and dbx, or others), so this isn't an option. Apart from that, the debugging experience when e.g. emitting very recent DWARF extensions and trying to use them with a gdb that doesn't understand them usually leads to some debug info missing. In this case, emitting compressed debug with a debugger that cannot read it leads to the debugger claiming (correctly, from its point of view) that there's no debugging info present. I don't want to tell users who come complaining `I compiled with -g, but my debugger tells me there's no debug info present': `look, your debugger lies, it is present, but it cannot read it'. That's a lot worse than the DWARF extensions scenario above. Agreed :) FWIW it's already a gas/assembler option, I'm curious about wanting to expose it via the compiler? On top of all that, compressed debug is a tradeoff: in some cases it may be worth it to save space on debug info if disk space is at a premium for some reason (e.g. for release builds), but in others you want to compile as fast as possible, but assembling and linking compressed debug takes more CPU time. Otherwise we could just as well default to -Os, telling our users it's better for them since it generates faster and smaller code, not minding the compile time cost and worse debugging experience. FWIW I've found in some limited timing that compression is nearly always worth it here at Google - even for compile time given the cost of writing files versus cpu time. Might be worth making it a default at some point in the future and making sure the option is invertible. -eric
Re: [C++ Patch] PR 43453
Hi,
On 06/04/2014 04:16 PM, Jason Merrill wrote:
if (TREE_CODE (init) == TREE_LIST
-TREE_CODE (TREE_TYPE (decl)) == ARRAY_TYPE)
+ TREE_CODE (type) == ARRAY_TYPE
+ /* C++11 8.5/17: If the destination type is an array of
characters,
+ an array of char16_t, an array of char32_t, or an array of
wchar_t,
+ and the initializer is a string literal */
+ !(char_type_p (TREE_TYPE (TYPE_MAIN_VARIANT (type)))
+TREE_CODE (TREE_VALUE (init)) == STRING_CST))
{
error (cannot initialize arrays using this syntax);
return NULL_TREE;
Can we just remove this error rather than add a condition? I think
it's covered in other places.
Removing it certainly passes testing.
The patch doesn't seem to fix
char s[] (foo);
which I think also ought to be well-formed.
Right. Thus I reworked a bit the code in cp_complete_array_type to
handle specially a single-element TREE_LIST together a single-element
CONSTRUCTOR. Tested x86_64-linux.
Thanks!
Paolo.
//
/cp
2014-06-04 Paolo Carlini paolo.carl...@oracle.com
PR c++/43453
* typeck.c (cp_build_modify_expr): Handle array of characters
initialized by a string literal.
* typeck2.c (store_init_value): Remove redundant check.
* decl.c (cp_complete_array_type): Rework code handling initialization
from a string literal to handle single element TREE_LIST too.
/testsuite
2014-06-04 Paolo Carlini paolo.carl...@oracle.com
PR c++/43453
* g++.dg/init/pr43453.C: New.
Index: cp/decl.c
===
--- cp/decl.c (revision 211225)
+++ cp/decl.c (working copy)
@@ -7176,20 +7176,21 @@ cp_complete_array_type (tree *ptype, tree initial_
tree value;
/* An array of character type can be initialized from a
-brace-enclosed string constant.
-
-FIXME: this code is duplicated from reshape_init. Probably
-we should just call reshape_init here? */
- if (char_type_p (TYPE_MAIN_VARIANT (TREE_TYPE (*ptype)))
- TREE_CODE (initial_value) == CONSTRUCTOR
- !vec_safe_is_empty (CONSTRUCTOR_ELTS (initial_value)))
+brace-enclosed string constant. */
+ if (char_type_p (TYPE_MAIN_VARIANT (TREE_TYPE (*ptype
{
- vecconstructor_elt, va_gc *v = CONSTRUCTOR_ELTS (initial_value);
- tree value = (*v)[0].value;
+ tree svalue = NULL_TREE;
- if (TREE_CODE (value) == STRING_CST
- v-length () == 1)
- initial_value = value;
+ if (TREE_CODE (initial_value) == CONSTRUCTOR
+ vec_safe_length (CONSTRUCTOR_ELTS (initial_value)) == 1)
+ svalue = (*CONSTRUCTOR_ELTS (initial_value))[0].value;
+ else if (TREE_CODE (initial_value) == TREE_LIST
+ list_length (initial_value) == 1)
+ /* We get here with code like `char s[] (abc);' */
+ svalue = TREE_VALUE (initial_value);
+
+ if (svalue TREE_CODE (svalue) == STRING_CST)
+ initial_value = svalue;
}
/* If any of the elements are parameter packs, we can't actually
Index: cp/typeck.c
===
--- cp/typeck.c (revision 211225)
+++ cp/typeck.c (working copy)
@@ -7511,6 +7511,18 @@ cp_build_modify_expr (tree lhs, enum tree_code mod
return error_mark_node;
}
+ /* C++11 8.5/17: If the destination type is an array of characters,
+an array of char16_t, an array of char32_t, or an array of wchar_t,
+and the initializer is a string literal */
+ else if (TREE_CODE (newrhs) == STRING_CST
+ char_type_p (TREE_TYPE (TYPE_MAIN_VARIANT (lhstype)))
+ modifycode == INIT_EXPR)
+ {
+ newrhs = digest_init (lhstype, newrhs, complain);
+ if (newrhs == error_mark_node)
+ return error_mark_node;
+ }
+
else if (!same_or_base_type_p (TYPE_MAIN_VARIANT (lhstype),
TYPE_MAIN_VARIANT (TREE_TYPE (newrhs
{
Index: cp/typeck2.c
===
--- cp/typeck2.c(revision 211225)
+++ cp/typeck2.c(working copy)
@@ -785,16 +785,9 @@ store_init_value (tree decl, tree init, vectree,
{
gcc_assert (TREE_CODE (decl) != RESULT_DECL);
- if (TREE_CODE (init) == TREE_LIST
- TREE_CODE (TREE_TYPE (decl)) == ARRAY_TYPE)
- {
- error (cannot initialize arrays using this syntax);
- return NULL_TREE;
- }
- else
- /* We get here with code like `int a (2);' */
- init = build_x_compound_expr_from_list (init, ELK_INIT,
- tf_warning_or_error);
+ /* We get here with code like `int a (2);' */
+ init = build_x_compound_expr_from_list (init, ELK_INIT,
Re: [patch] Shorten Windows path
Hi,
On 04/01/2014 05:17 AM, Joey Ye wrote:
diff --git a/libcpp/files.c b/libcpp/files.c
index 7e88778..ad68682 100644
--- a/libcpp/files.c
+++ b/libcpp/files.c
@@ -387,8 +387,14 @@ find_file_in_dir (cpp_reader *pfile, _cpp_file *file,
bool *invalid_pch)
char *copy;
void **pp;
- /* We try to canonicalize system headers. */
- if (CPP_OPTION (pfile, canonical_system_headers) file-dir-sysp)
+ /* We try to canonicalize system headers. For DOS based file
+ * system, we always try to shorten non-system headers, as DOS
+ * has a tighter constraint on max path length. */
+ if (CPP_OPTION (pfile, canonical_system_headers) file-dir-sysp
+#ifdef HAVE_DOS_BASED_FILE_SYSTEM
+ || !file-dir-sysp
+#endif
+ )
{
char * canonical_path = maybe_shorter_path (path);
Recently I have been working with a Windows GCC user that is running into the
minuscule Windows path limits discussed here. I backported this patch to see
if it helped their case, but it didn't.
The problem we ran into is that 'lrealpath' is used in 'maybe_shorter_path' to
compute the canonical file path that has the relative portions removed. On
Windows the implementation uses 'GetFullPathName'. Unfortunately,
'GetFullPathName' suffers from the same MAX_PATH limit thus the
canonicalization fails (and from what I can tell the relative path that is
passed to 'GetFullPathName' is below the limit, but the joining of the current
working directory with the relative path name passed is not).
Did y'all run into anything like this? Were other options to produce a
canonical path name discussed? Nothing obvious jumped out at me. Despite the
limits, using 'GetFullPathName' seems like the natural way to handle it because
it knows about all the various Windows file system quirks.
--
Meador Inge
CodeSourcery / Mentor Embedded
Re: patch to fix PR61325
On 2014-06-03, 6:02 PM, James Greenhalgh wrote:
On Thu, May 29, 2014 at 06:38:22PM +0100, Vladimir Makarov wrote:
The following patch PR61325. The details can be found on
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61325
The patch was bootstrapped and tested on x86/x86-64.
Committed as rev. 211060 to gcc-4.9 branch and as rev.211061 to trunk.
2014-05-29 Vladimir Makarov vmaka...@redhat.com
PR rtl-optimization/61325
* lra-constraints.c (process_address): Rename to
process_address_1.
(process_address): New function.
2014-05-29 Vladimir Makarov vmaka...@redhat.com
PR rtl-optimization/61325
* gcc.target/aarch64/pr61325.c: New.
Hi Vlad,
This patch appears to cause issues where the ARM backend can get stuck in a
seemingly infinite loop.
Sorry for inconvenience.
Could you test the following patch
Index: lra-constraints.c
===
--- lra-constraints.c (revision 211061)
+++ lra-constraints.c (working copy)
@@ -2974,6 +2974,13 @@ process_address_1 (int nop, rtx *before,
*ad.inner = simplify_gen_binary (PLUS, GET_MODE (new_reg),
new_reg, *ad.index);
}
+ else if (get_index_scale (ad) == 1)
+{
+ /* The last transformation to one reg will be made in
+curr_insn_transform function. */
+ end_sequence ();
+ return false;
+}
else
{
/* base + scale * index = base + new_reg,
If it is ok, I'll commit it as soon as possible.
I checked the patch on x86/x86-64. The patch is ok for this platform.
Re: patch to fix PR61325
On 2014-06-03, 6:02 PM, James Greenhalgh wrote:
On Thu, May 29, 2014 at 06:38:22PM +0100, Vladimir Makarov wrote:
The following patch PR61325. The details can be found on
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=61325
The patch was bootstrapped and tested on x86/x86-64.
Committed as rev. 211060 to gcc-4.9 branch and as rev.211061 to trunk.
2014-05-29 Vladimir Makarov vmaka...@redhat.com
PR rtl-optimization/61325
* lra-constraints.c (process_address): Rename to
process_address_1.
(process_address): New function.
2014-05-29 Vladimir Makarov vmaka...@redhat.com
PR rtl-optimization/61325
* gcc.target/aarch64/pr61325.c: New.
Hi Vlad,
This patch appears to cause issues where the ARM backend can get stuck in a
seemingly infinite loop.
Compiling:
./gcc.c-torture/compile/unalign-1.c
Sorry for inconvenience.
Could you test the following patch
Index: lra-constraints.c
===
--- lra-constraints.c (revision 211061)
+++ lra-constraints.c (working copy)
@@ -2974,6 +2974,13 @@ process_address_1 (int nop, rtx *before,
*ad.inner = simplify_gen_binary (PLUS, GET_MODE (new_reg),
new_reg, *ad.index);
}
+ else if (get_index_scale (ad) == 1)
+{
+ /* The last transformation to one reg will be made in
+curr_insn_transform function. */
+ end_sequence ();
+ return false;
+}
else
{
/* base + scale * index = base + new_reg,
If it is ok, I'll commit it as soon as possible.
I checked the patch on x86/x86-64. The patch is ok for this platform.
Re: [PATCH 2/2, x86] Add palignr support for AVX2.
On 06/04/2014 10:06 AM, Evgeny Stupachenko wrote:
Is it ok to use the following pattern?
patch passed bootstrap and make check, but one test failed:
gcc/testsuite/gcc.target/i386/vect-rebuild.c
It failed on /* { dg-final { scan-assembler-times \tv?permilpd\[ \t\] 1 } }
*/
which is now palignr. However, both palignr and permilpd costs 1 tick
and take 6 bytes in the opcode.
I vote for modifying the test to scan for palignr:
/* { dg-final { scan-assembler-times \tv?palignr\[ \t\] 1 } } */
2014-06-04 Evgeny Stupachenko evstu...@gmail.com
* config/i386/sse.md (*ssse3_palignrmode_perm): New.
* config/i386/predicates.md (palignr_operand): New.
Indicates if permutation is suitable for palignr instruction.
Surely permilpd avoids some sort of reformatting penalty when actually using
doubles.
If you move this pattern down below the other vec_select patterns, we'll prefer
the others for matching masks.
r~
Re: [C++ Patch] PR 43453
On 06/04/2014 02:08 PM, Paolo Carlini wrote: /* An array of character type can be initialized from a -brace-enclosed string constant. - -FIXME: this code is duplicated from reshape_init. Probably -we should just call reshape_init here? */ It strikes me as odd that we get here before we strip the parens. What if we moved the paren code up out of store_init_value to check_initializer before the call to maybe_deduce_size_from_array_init? Jason
Re: emit __float128 typeinfo
On Wed, 4 Jun 2014, Jason Merrill wrote: How about, in emit_support_tinfos, using type_for_mode to check for a TF-mode floating point type different from long_double_type_node? What should I pass as the mode argument? I can't just write TFmode, that will fail to compile on platforms that don't define it, and powerpc seems likely to call it JFmode instead quite soon. MAX_MODE_FLOAT maybe? But then if we configure with long double = __float128, we will miss __float80. Ah, we walk from GET_CLASS_NARROWEST_MODE (MODE_FLOAT) with GET_MODE_WIDER_MODE steps and test if the associated type is not in the list 0/float/double/long double. I think it should be ok with arm (it would be good if they removed their unused XFmode, but I don't even think it is necessary). Is that what you were suggesting? I'll try to write a patch, thanks. -- Marc Glisse
[Patch] PR55189 enable -Wreturn-type by default
Hello, Finally, I have been able to update all tests with -Wreturn-type enabled by default. AFAIK, under GNU/Linux Debian Jessie 64 bits, there is no PASS-FAIL tests. Now, I would like to know if I can commit that into the repository. Who can review that? As attachment, you will find the actual (tiny) patch. I split the tests update by languages. As they are big ( 1260 files changed, 1638 insertions(+), 903 deletions(-) ), I uploaded the patches on my server: http://sylvestre.ledru.info/bordel/patch/0002-Update-Objective-c-tests-with-warning-return-type-en.patch http://sylvestre.ledru.info/bordel/patch/0003-Update-Fortran-tests-with-warning-return-type-enable.patch http://sylvestre.ledru.info/bordel/patch/0004-Update-gcc-tests-with-warning-return-type-enabled-by.patch http://sylvestre.ledru.info/bordel/patch/0005-Update-C-tests-with-warning-return-type-enabled-by-d.patch http://sylvestre.ledru.info/bordel/patch/0006-Update-OpenMP-tests-with-warning-return-type-enabled.patch Thanks, Sylvestre gcc/c-family/ChangeLog: 2014-06-04 Sylvestre Ledru sylves...@debian.org * c.opt: -Wreturn-type enabled by default From 650edb9943ba8b2afb4995e70f671d8fdc26e10a Mon Sep 17 00:00:00 2001 From: Sylvestre Ledru sylves...@debian.org Date: Wed, 4 Jun 2014 13:33:40 +0200 Subject: [PATCH 1/6] Enable warning return-type by default --- gcc/c-family/c.opt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt index 5d36a80..8e78a9d 100644 --- a/gcc/c-family/c.opt +++ b/gcc/c-family/c.opt @@ -686,7 +686,7 @@ C ObjC C++ ObjC++ Var(warn_return_local_addr) Init(1) Warning Warn about returning a pointer/reference to a local or temporary variable. Wreturn-type -C ObjC C++ ObjC++ Var(warn_return_type) Warning LangEnabledBy(C ObjC C++ ObjC++,Wall) +C ObjC C++ ObjC++ Var(warn_return_type) Init(1) Warning Warn whenever a function's return type defaults to \int\ (C), or about inconsistent return types (C++) Wselector -- 2.0.0
Re: [patch][gomp4] openacc loops
Hi Cesar!
On Tue, 3 Jun 2014 16:00:30 -0700, Cesar Philippidis ce...@codesourcery.com
wrote:
in order to make
the patch yield more interesting results, I've also enabled the private
clause. Is this patch ok for the gomp-4_0-branch?
gcc/
* c/c-parser.c (c_parser_oacc_all_clauses): Update handling for
Note that gcc/c/ as well as gcc/fortran/ have separate ChangeLog* files.
OMP_CLAUSE_COLLAPSE and OMP_CLAUSE_PRIVATE.
Only for OMP_CLAUSE_COLLAPSE, not OMP_CLAUSE_PRIVATE.
(c_parser_oacc_kernels): Likewise.
OACC_LOOP_CLAUSE_MASK, not c_parser_oacc_kernels.
--- a/gcc/c/c-parser.c
+++ b/gcc/c/c-parser.c
@@ -11228,6 +11228,10 @@ c_parser_oacc_all_clauses (c_parser *parser,
omp_clause_mask mask,
switch (c_kind)
{
+ case PRAGMA_OMP_CLAUSE_COLLAPSE:
Won't this need additional work? It seems that for combined directives
(kernels loop, parallel loop), we currently don't (or, don't correctly)
parse the clauses, and support in clause splitting
(c-family/c-omp:c_omp_split_clauses) is also (generally) missing, I
think? Anyway, this is a separate change from your Fortran loop support,
so should (ideally) be a separate patch/commit. (Also, I'm not sure to
which extent we're at all currently handling combined directives in
gimplification and lowering?)
+ clauses = c_parser_omp_clause_collapse (parser, clauses);
+ c_name = collapse;
+ break;
Update the comment on c_parser_omp_clause_collapse to state that it's for
OpenACC, too.
@@ -12217,8 +12221,8 @@ c_parser_omp_for_loop (location_t loc, c_parser
*parser, enum tree_code code,
for (cl = clauses; cl; cl = OMP_CLAUSE_CHAIN (cl))
if (OMP_CLAUSE_CODE (cl) == OMP_CLAUSE_COLLAPSE)
{
- gcc_assert (code != OACC_LOOP);
- collapse = tree_to_shwi (OMP_CLAUSE_COLLAPSE_EXPR (cl));
+ //gcc_assert (code != OACC_LOOP);
+ collapse = tree_to_shwi (OMP_CLAUSE_COLLAPSE_EXPR (cl));
}
As Ilmir noted, remove the gcc_assert -- assuming you have some
confidence that the following code (including gimplification and
lowering) matches the OpenACC semantics for collapse != 1.
--- a/gcc/gimple.h
+++ b/gcc/gimple.h
@@ -5809,15 +5809,25 @@ is_gimple_omp (const_gimple stmt)
need any special handling for OpenACC. */
static inline bool
-is_gimple_omp_oacc_specifically (const_gimple stmt)
+is_gimple_omp_oacc_specifically (const_gimple stmt,
+ enum omp_clause_code code = OMP_CLAUSE_ERROR)
{
gcc_assert (is_gimple_omp (stmt));
switch (gimple_code (stmt))
{
case GIMPLE_OACC_KERNELS:
case GIMPLE_OACC_PARALLEL:
- return true;
+ switch (code)
+ {
+ case OMP_CLAUSE_COLLAPSE:
+ case OMP_CLAUSE_PRIVATE:
+ return false;
+ default:
+ return true;
+ }
case GIMPLE_OMP_FOR:
+ if (code == OMP_CLAUSE_COLLAPSE || code == OMP_CLAUSE_PRIVATE)
+ return false;
switch (gimple_omp_for_kind (stmt))
{
case GF_OMP_FOR_KIND_OACC_LOOP:
Hmm, why do we need this?
--- a/gcc/omp-low.c
+++ b/gcc/omp-low.c
@@ -1534,7 +1534,8 @@ scan_sharing_clauses (tree clauses, omp_context *ctx)
switch (OMP_CLAUSE_CODE (c))
{
case OMP_CLAUSE_PRIVATE:
- gcc_assert (!is_gimple_omp_oacc_specifically (ctx-stmt));
+ gcc_assert (!is_gimple_omp_oacc_specifically (ctx-stmt,
+ OMP_CLAUSE_CODE (c)));
I'd say, in these guarded code paths, if you have confidence that
they're now correct for OpenACC, that is, a clause such as
OMP_CLAUSE_PRIVATE is interpreted correctly for OpenACC (it has the
same semantics as as for OpenMP), then you should simply remove the
assert completely (or, if applicable, move the case OMP_CLAUSE_PRIVATE or
the surrounding cases so that OMP_CLAUSE_PRIVATE is no longer covered by
the assert). For example, do it like this:
case OMP_CLAUSE_NOWAIT:
case OMP_CLAUSE_ORDERED:
- case OMP_CLAUSE_COLLAPSE:
case OMP_CLAUSE_UNTIED:
case OMP_CLAUSE_MERGEABLE:
case OMP_CLAUSE_PROC_BIND:
case OMP_CLAUSE_SAFELEN:
gcc_assert (!is_gimple_omp_oacc_specifically (ctx-stmt));
+ /* FALLTHRU */
+ case OMP_CLAUSE_COLLAPSE:
break;
With these things addressed/verified, the OMP_CLAUSE_COLLAPSE changes are
good to commit, thanks!
@@ -1762,7 +1763,6 @@ scan_sharing_clauses (tree clauses, omp_context *ctx)
}
}
break;
-
case OMP_CLAUSE_NOWAIT:
case OMP_CLAUSE_ORDERED:
case OMP_CLAUSE_COLLAPSE:
To ease my life ;-) as a branch maintainer, please don't introduce such
divergence from the GCC trunk code.
@@ -1817,13 +1818,9 @@ scan_sharing_clauses (tree clauses, omp_context *ctx)
case OMP_CLAUSE_PRIVATE:
case OMP_CLAUSE_FIRSTPRIVATE:
case OMP_CLAUSE_REDUCTION:
- if (is_gimple_omp_oacc_specifically
Re: [patch i386]: Fix PR/46219 Generate indirect jump instruction
On 06/04/2014 05:37 AM, Kai Tietz wrote: +(define_peephole2 + [(set (match_operand:DI 0 register_operand) +(match_operand:DI 1 memory_operand)) + (call (mem:QI (match_operand:DI 2 register_operand)) + (match_operand 3))] + TARGET_64BIT REG_P (operands[0]) + REG_P (operands[2]) + SIBLING_CALL_P (peep2_next_insn (1)) + REGNO (operands[0]) == REGNO (operands[2]) + [(call (unspec [(mem:QI (match_dup 1))] UNSPEC_PEEPSIB) (match_dup 3))]) You can use (match_dup 0) instead of that (match_operand 2); that allows you to remove + REG_P (operands[2]) + REGNO (operands[0]) == REGNO (operands[2]) You don't need the + REG_P (operands[2]) because of the register_operand constraint; all subregs will have been resolved at this point. You need a !TARGET_X32 check for your TARGET_64BIT patches, since the jmpq insn always reads 64-bits. +(define_insn *sibcall_pop_intern + [(call (unspec [(mem:QI (match_operand:SI 0 memory_operand))] UNSPEC_PEEPSIB) Watch your line length (and line-wrapping when posting patches). Align the UNSPEC_PEEPSIB with the [. This, and the other following sibcall intern patterns, including +(define_insn *sibcall_value_intern +(define_insn *sibcall_value_pop_intern have no matching peephole2 patterns, and so will never be generated. You'll need more peep2's. r~
Fix address space computation in expand_debug_expr
For the AVR target, assertions in convert_debug_memory_address cause a couple of ICEs (see PR 52472). Jakub suggested returning a NULL rtx, which works, but on debugging further, I found that expand_debug_expr appears to incorrectly compute the address space for ADDR_EXPR and MEM_REFs. For ADDR_EXPR, TREE_TYPE(exp) is a POINTER_TYPE (always?), but in the generic address space, even if the object whose address is taken is in a different address space. expand_debug_expr takes TYPE_ADDR_SPACE(TREE_TYPE(exp)) and therefore computes the address space as generic. convert_debug_memory_address then asserts that the mode is a valid pointer mode in the address space and fails. Similarly, for MEM_REFs, TREE_TYPE(exp) is the type of the dereferenced value, and therefore checking if it is a POINTER_TYPE doesn't help for a single pointer dereference. The address space gets computed as generic even if the pointer points to a different address space. The address mode for the generic address space is passed to convert_debug_memory_address, and the assertion that that mode must match the mode of the rtx then fails. The below patch attempts to fix this by picking the right TREE_TYPE to pass to TYPE_ADDR_SPACE for MEM_REF (use type of arg 0) and ADDR_EXPR (check for pointer type and look at nested addr space). Does this look reasonable or did I get it all wrong? Regards Senthil diff --git gcc/cfgexpand.c gcc/cfgexpand.c index 8b0e466..ca78953 100644 --- gcc/cfgexpand.c +++ gcc/cfgexpand.c @@ -3941,8 +3941,8 @@ expand_debug_expr (tree exp) op0 = plus_constant (inner_mode, op0, INTVAL (op1)); } - if (POINTER_TYPE_P (TREE_TYPE (exp))) - as = TYPE_ADDR_SPACE (TREE_TYPE (TREE_TYPE (exp))); + if (POINTER_TYPE_P (TREE_TYPE (TREE_OPERAND (exp, 0 + as = TYPE_ADDR_SPACE (TREE_TYPE (TREE_TYPE (TREE_OPERAND (exp, 0; else as = ADDR_SPACE_GENERIC; @@ -4467,7 +4467,11 @@ expand_debug_expr (tree exp) return NULL; } - as = TYPE_ADDR_SPACE (TREE_TYPE (exp)); + if (POINTER_TYPE_P (TREE_TYPE (exp))) +as = TYPE_ADDR_SPACE (TREE_TYPE (TREE_TYPE (exp))); + else +as = TYPE_ADDR_SPACE (TREE_TYPE (exp)); + op0 = convert_debug_memory_address (mode, XEXP (op0, 0), as); return op0;
Re: [C++ Patch] PR 43453
Hi,
On 06/04/2014 09:32 PM, Jason Merrill wrote:
On 06/04/2014 02:08 PM, Paolo Carlini wrote:
/* An array of character type can be initialized from a
- brace-enclosed string constant.
-
- FIXME: this code is duplicated from reshape_init. Probably
- we should just call reshape_init here? */
It strikes me as odd that we get here before we strip the parens. What
if we moved the paren code up out of store_init_value to
check_initializer before the call to maybe_deduce_size_from_array_init?
Ok... At first I was worried by the other
maybe_deduce_size_from_array_init call, but I don't think the
initializer of a char array can be dependent, right? Then I'm finishing
testing the below.
Thanks!
Paolo.
///
Index: cp/decl.c
===
--- cp/decl.c (revision 211242)
+++ cp/decl.c (working copy)
@@ -5758,6 +5758,13 @@ check_initializer (tree decl, tree init, int flags
check_narrowing (type, init);
}
}
+ else if (TREE_CODE (type) == ARRAY_TYPE
+ TREE_CODE (init) == TREE_LIST
+ char_type_p (TYPE_MAIN_VARIANT (TREE_TYPE (type)))
+ list_length (init) == 1
+ TREE_CODE (TREE_VALUE (init)) == STRING_CST)
+ /* We get here with code like `char s[] (abc);' */
+ init = TREE_VALUE (init);
/* If DECL has an array type without a specific bound, deduce the
array size from the initializer. */
@@ -7176,20 +7183,21 @@ cp_complete_array_type (tree *ptype, tree initial_
tree value;
/* An array of character type can be initialized from a
-brace-enclosed string constant.
-
-FIXME: this code is duplicated from reshape_init. Probably
-we should just call reshape_init here? */
- if (char_type_p (TYPE_MAIN_VARIANT (TREE_TYPE (*ptype)))
- TREE_CODE (initial_value) == CONSTRUCTOR
- !vec_safe_is_empty (CONSTRUCTOR_ELTS (initial_value)))
+brace-enclosed string constant. */
+ if (char_type_p (TYPE_MAIN_VARIANT (TREE_TYPE (*ptype
{
- vecconstructor_elt, va_gc *v = CONSTRUCTOR_ELTS (initial_value);
- tree value = (*v)[0].value;
+ tree svalue = initial_value;
- if (TREE_CODE (value) == STRING_CST
- v-length () == 1)
- initial_value = value;
+ if (TREE_CODE (svalue) == CONSTRUCTOR
+ vec_safe_length (CONSTRUCTOR_ELTS (svalue)) == 1)
+ svalue = (*CONSTRUCTOR_ELTS (svalue))[0].value;
+ else if (TREE_CODE (svalue) == TREE_LIST
+ list_length (svalue) == 1)
+ // We get here with code like `char s[] (abc);'
+ svalue = TREE_VALUE (svalue);
+
+ if (TREE_CODE (svalue) == STRING_CST)
+ initial_value = svalue;
}
/* If any of the elements are parameter packs, we can't actually
Index: cp/typeck.c
===
--- cp/typeck.c (revision 211242)
+++ cp/typeck.c (working copy)
@@ -7511,6 +7511,18 @@ cp_build_modify_expr (tree lhs, enum tree_code mod
return error_mark_node;
}
+ /* C++11 8.5/17: If the destination type is an array of characters,
+an array of char16_t, an array of char32_t, or an array of wchar_t,
+and the initializer is a string literal */
+ else if (TREE_CODE (newrhs) == STRING_CST
+ char_type_p (TREE_TYPE (TYPE_MAIN_VARIANT (lhstype)))
+ modifycode == INIT_EXPR)
+ {
+ newrhs = digest_init (lhstype, newrhs, complain);
+ if (newrhs == error_mark_node)
+ return error_mark_node;
+ }
+
else if (!same_or_base_type_p (TYPE_MAIN_VARIANT (lhstype),
TYPE_MAIN_VARIANT (TREE_TYPE (newrhs
{
Index: cp/typeck2.c
===
--- cp/typeck2.c(revision 211242)
+++ cp/typeck2.c(working copy)
@@ -785,16 +785,9 @@ store_init_value (tree decl, tree init, vectree,
{
gcc_assert (TREE_CODE (decl) != RESULT_DECL);
- if (TREE_CODE (init) == TREE_LIST
- TREE_CODE (TREE_TYPE (decl)) == ARRAY_TYPE)
- {
- error (cannot initialize arrays using this syntax);
- return NULL_TREE;
- }
- else
- /* We get here with code like `int a (2);' */
- init = build_x_compound_expr_from_list (init, ELK_INIT,
- tf_warning_or_error);
+ /* We get here with code like `int a (2);' */
+ init = build_x_compound_expr_from_list (init, ELK_INIT,
+ tf_warning_or_error);
}
/* End of special C++ code. */
Index: testsuite/g++.dg/init/pr43453.C
===
---
Re: [C++ Patch] PR 43453
... sorry attached a wrong diff.
Paolo.
/
Index: cp/decl.c
===
--- cp/decl.c (revision 211242)
+++ cp/decl.c (working copy)
@@ -5758,6 +5758,13 @@ check_initializer (tree decl, tree init, int flags
check_narrowing (type, init);
}
}
+ else if (TREE_CODE (type) == ARRAY_TYPE
+ TREE_CODE (init) == TREE_LIST
+ char_type_p (TYPE_MAIN_VARIANT (TREE_TYPE (type)))
+ list_length (init) == 1
+ TREE_CODE (TREE_VALUE (init)) == STRING_CST)
+ /* We get here with code like `char s[] (abc);' */
+ init = TREE_VALUE (init);
/* If DECL has an array type without a specific bound, deduce the
array size from the initializer. */
Index: cp/typeck.c
===
--- cp/typeck.c (revision 211242)
+++ cp/typeck.c (working copy)
@@ -7511,6 +7511,18 @@ cp_build_modify_expr (tree lhs, enum tree_code mod
return error_mark_node;
}
+ /* C++11 8.5/17: If the destination type is an array of characters,
+an array of char16_t, an array of char32_t, or an array of wchar_t,
+and the initializer is a string literal */
+ else if (TREE_CODE (newrhs) == STRING_CST
+ char_type_p (TREE_TYPE (TYPE_MAIN_VARIANT (lhstype)))
+ modifycode == INIT_EXPR)
+ {
+ newrhs = digest_init (lhstype, newrhs, complain);
+ if (newrhs == error_mark_node)
+ return error_mark_node;
+ }
+
else if (!same_or_base_type_p (TYPE_MAIN_VARIANT (lhstype),
TYPE_MAIN_VARIANT (TREE_TYPE (newrhs
{
Index: cp/typeck2.c
===
--- cp/typeck2.c(revision 211242)
+++ cp/typeck2.c(working copy)
@@ -785,16 +785,9 @@ store_init_value (tree decl, tree init, vectree,
{
gcc_assert (TREE_CODE (decl) != RESULT_DECL);
- if (TREE_CODE (init) == TREE_LIST
- TREE_CODE (TREE_TYPE (decl)) == ARRAY_TYPE)
- {
- error (cannot initialize arrays using this syntax);
- return NULL_TREE;
- }
- else
- /* We get here with code like `int a (2);' */
- init = build_x_compound_expr_from_list (init, ELK_INIT,
- tf_warning_or_error);
+ /* We get here with code like `int a (2);' */
+ init = build_x_compound_expr_from_list (init, ELK_INIT,
+ tf_warning_or_error);
}
/* End of special C++ code. */
Index: testsuite/g++.dg/init/pr43453.C
===
--- testsuite/g++.dg/init/pr43453.C (revision 0)
+++ testsuite/g++.dg/init/pr43453.C (working copy)
@@ -0,0 +1,33 @@
+// PR c++/43453
+
+struct A {
+ char x[4];
+ A() : x(bug) { };
+};
+
+char x [4] (bug);
+
+struct CA {
+ const char cx[4];
+ CA() : cx(bug) { };
+};
+
+const char cx [4] (bug);
+
+struct B {
+ char y[4];
+ B() : y(bu) { };
+};
+
+char y [4] (bu);
+
+struct C {
+ char z[4];
+ C() : z(bugs) { }; // { dg-error too long }
+};
+
+char z [4] (bugs);// { dg-error too long }
+
+char k [] (bug);
+
+const char ck [] (bug);
[s390, Committed] Use INTVAL only on CONST_INT in addptrdi3 and addptrsi3
Andreas,
This patch ensures that INTVAL is only used on a CONST_INT in define_expands
addptrdi3 and addptrsi3. I ran into this issue when building gcc for target s390
with --enable-checking=yes,rtl.
Finished s390 build with patch, and committed as trivial.
Thanks,
- Tom
2014-06-04 Tom de Vries t...@codesourcery.com
* config/s390/s390.md (addptrdi3, addptrsi3): Use INTVAL only on
CONST_INT.
diff --git a/gcc/config/s390/s390.md b/gcc/config/s390/s390.md
index b17c1fa..5fdbdbe 100644
--- a/gcc/config/s390/s390.md
+++ b/gcc/config/s390/s390.md
@@ -5045,10 +5045,10 @@
(match_operand:DI 2 nonmemory_operand )))]
TARGET_64BIT
{
- HOST_WIDE_INT c = INTVAL (operands[2]);
-
if (GET_CODE (operands[2]) == CONST_INT)
{
+ HOST_WIDE_INT c = INTVAL (operands[2]);
+
if (!CONST_OK_FOR_CONSTRAINT_P (c, 'K', K)
!CONST_OK_FOR_CONSTRAINT_P (c, 'O', Os))
{
@@ -5071,10 +5071,10 @@
(use (const_int 0))])]
!TARGET_64BIT
{
- HOST_WIDE_INT c = INTVAL (operands[2]);
-
if (GET_CODE (operands[2]) == CONST_INT)
{
+ HOST_WIDE_INT c = INTVAL (operands[2]);
+
if (!CONST_OK_FOR_CONSTRAINT_P (c, 'K', K)
!CONST_OK_FOR_CONSTRAINT_P (c, 'O', Os))
{
--
1.9.1
Re: Fix a function decl in gfortran
Hi Bernd,
Bernd Schmidt wrote:
On 06/04/2014 09:40 AM, Tobias Burnus wrote:
I intent to tests the build and test the patch - and then to
commit it as obvious. If you see problems with this approach
please scream now.
Even with this applied, I'm still seeing similar failures.
I didn't claim that the patch would fix everything – nor that it was
well tested.
Can you try the attached version? The change is that I now properly use
se-ignore_optional to test whether absent optional arguments should
be skipped - rather than using this mornings ad-hoc solution of doing so
unconditionally. Additionally, the patch has now survived stage2
building – which is more testing than I could do this morning.
Tobias
2014-06-04 Tobias Burnus bur...@net-b.de
* gfortran.h (gfc_copy_formal_args_intr): Update prototype.
* symbol.c (gfc_copy_formal_args_intr): Handle the case
that absent optional arguments should be ignored.
* trans-intrinsic.c (gfc_get_symbol_for_expr): Ditto.
(gfc_conv_intrinsic_funcall,
conv_generic_with_optional_char_arg): Update call.
* resolve.c (gfc_resolve_intrinsic): Ditto.
diff --git a/gcc/fortran/gfortran.h b/gcc/fortran/gfortran.h
index 3e5cdbd..1df79fd 100644
--- a/gcc/fortran/gfortran.h
+++ b/gcc/fortran/gfortran.h
@@ -2748,7 +2785,8 @@ gfc_symbol* gfc_get_ultimate_derived_super_type (gfc_symbol*);
bool gfc_type_is_extension_of (gfc_symbol *, gfc_symbol *);
bool gfc_type_compatible (gfc_typespec *, gfc_typespec *);
-void gfc_copy_formal_args_intr (gfc_symbol *, gfc_intrinsic_sym *);
+void gfc_copy_formal_args_intr (gfc_symbol *, gfc_intrinsic_sym *,
+gfc_actual_arglist *);
void gfc_free_finalizer (gfc_finalizer *el); /* Needed in resolve.c, too */
diff --git a/gcc/fortran/resolve.c b/gcc/fortran/resolve.c
index 7579573..d224d6e 100644
--- a/gcc/fortran/resolve.c
+++ b/gcc/fortran/resolve.c
@@ -1674,7 +1674,7 @@ gfc_resolve_intrinsic (gfc_symbol *sym, locus *loc)
return false;
}
- gfc_copy_formal_args_intr (sym, isym);
+ gfc_copy_formal_args_intr (sym, isym, NULL);
sym-attr.pure = isym-pure;
sym-attr.elemental = isym-elemental;
diff --git a/gcc/fortran/symbol.c b/gcc/fortran/symbol.c
index 3785c2e..922b421 100644
--- a/gcc/fortran/symbol.c
+++ b/gcc/fortran/symbol.c
@@ -4015,16 +4042,21 @@ add_proc_interface (gfc_symbol *sym, ifsrc source, gfc_formal_arglist *formal)
each arg is set according to the existing ones. This function is
used when creating procedure declaration variables from a procedure
declaration statement (see match_proc_decl()) to create the formal
- args based on the args of a given named interface. */
+ args based on the args of a given named interface.
+
+ When an actual argument list is provided, skip the absent arguments.
+ To be used together with gfc_se-ignore_optional. */
void
-gfc_copy_formal_args_intr (gfc_symbol *dest, gfc_intrinsic_sym *src)
+gfc_copy_formal_args_intr (gfc_symbol *dest, gfc_intrinsic_sym *src,
+ gfc_actual_arglist *actual)
{
gfc_formal_arglist *head = NULL;
gfc_formal_arglist *tail = NULL;
gfc_formal_arglist *formal_arg = NULL;
gfc_intrinsic_arg *curr_arg = NULL;
gfc_formal_arglist *formal_prev = NULL;
+ gfc_actual_arglist *act_arg = actual;
/* Save current namespace so we can change it for formal args. */
gfc_namespace *parent_ns = gfc_current_ns;
@@ -4035,6 +4067,17 @@ gfc_copy_formal_args_intr (gfc_symbol *dest, gfc_intrinsic_sym *src)
for (curr_arg = src-formal; curr_arg; curr_arg = curr_arg-next)
{
+ /* Skip absent arguments. */
+ if (actual)
+ {
+ gcc_assert (act_arg != NULL);
+ if (act_arg-expr == NULL)
+ {
+ act_arg = act_arg-next;
+ continue;
+ }
+ act_arg = act_arg-next;
+ }
formal_arg = gfc_get_formal_arglist ();
gfc_get_symbol (curr_arg-name, gfc_current_ns, (formal_arg-sym));
diff --git a/gcc/fortran/trans-intrinsic.c b/gcc/fortran/trans-intrinsic.c
index a76d0f7..b6b4546 100644
--- a/gcc/fortran/trans-intrinsic.c
+++ b/gcc/fortran/trans-intrinsic.c
@@ -2371,7 +2744,7 @@ gfc_conv_intrinsic_minmax_char (gfc_se * se, gfc_expr * expr, int op)
has the generic name. */
static gfc_symbol *
-gfc_get_symbol_for_expr (gfc_expr * expr)
+gfc_get_symbol_for_expr (gfc_expr * expr, bool ignore_optional)
{
gfc_symbol *sym;
@@ -2394,7 +2767,9 @@ gfc_get_symbol_for_expr (gfc_expr * expr)
sym-as-rank = expr-rank;
}
- gfc_copy_formal_args_intr (sym, expr-value.function.isym);
+ gfc_copy_formal_args_intr (sym, expr-value.function.isym,
+ ignore_optional ? expr-value.function.actual
+ : NULL);
return sym;
}
@@ -2413,7 +2788,7 @@ gfc_conv_intrinsic_funcall (gfc_se * se, gfc_expr * expr)
else
gcc_assert (expr-rank == 0);
- sym = gfc_get_symbol_for_expr (expr);
+ sym = gfc_get_symbol_for_expr (expr, se-ignore_optional);
/* Calls to libgfortran_matmul need to be appended special arguments,
to be able to call the
