regression test issue
Hello Everyone, The following two Cilk Plus tests is timing out at -O1 in my x86_64 box (-O2, -O3 and -O0 works fine). These tests were working fine till revision r207047. Can someone please look at this? It looks like a middle-end/back-end issue. WARNING: program timed out. FAIL: g++.dg/cilk-plus/CK/catch_exc.cc -O1 -fcilkplus execution test WARNING: program timed out. FAIL: c-c++-common/cilk-plus/CK/spawner_inline.c -O1 execution test Thanks, Balaji V. Iyer.
Re: [PATCH][PING^2] Fix for PR59600 (prohibit inlining if no_sanitize_address)
Richard wrote: What about updated patch? Ok if it passes bootstrap and regtesting. It does, commited in r207497. -Y
[PATCH, rs6000] Handle -maltivec=be for vec_sum2s builtin (last -maltivec=be patch)
Hi,
One final patch in the series, this one for vec_sum2s. This builtin
requires some additional code generation for the case of little endian
without -maltivec=be. Here's an example:
va = {-10,1,2,3};0x 0003 0002 0001 fff6
vb = {100,101,102,-103}; 0x ff99 0066 0065 0064
vc = vec_sum2s (va, vb); 0x ff9e 005c
= {0,92,0,-98};
We need to add -10 + 1 + 101 = 92 and place it in vc[1], and add 2 + 3 +
-103 and place the result in vc[3], with zeroes in the other two
elements. To do this, we first use "vsldoi vs,vb,vb,12" to rotate 101
and -103 into big-endian elements 1 and 3, as required by the vsum2sws
instruction:
0x ff99 0066 0065 0064 ff99 0066 0065 0064
vs = 0064 ff99 0066 0065
Executing "vsum2sws vs,va,vs" then gives
vs = 0x ff9e 005c
which then must be shifted into position with "vsldoi vc,vs,vs,4"
0x ff9e 005c ff9e 005c
vc = ff9e 005c
which is the desired result.
In addition to this change, I noticed a redundant test from one of my
previous patches and simplified it. (BYTES_BIG_ENDIAN implies
VECTOR_ELT_ORDER_BIG, so we don't need to test BYTES_BIG_ENDIAN.)
As usual, new test cases are added to cover the possible cases. These
are simpler this time since only vector signed integer is a legal type
for vec_sum2s.
Bootstrapped and tested on powerpc64{,le}-unknown-linux-gnu with no
regressions. Is this ok for trunk?
Thanks,
Bill
gcc:
2014-02-04 Bill Schmidt
* config/rs6000/altivec.md (altivec_vsum2sws): Adjust code
generation for -maltivec=be.
(altivec_vsumsws): Simplify redundant test.
gcc/testsuite:
2014-02-04 Bill Schmidt
* gcc.dg/vmx/sum2s.c: New.
* gcc.dg/vmx/sum2s-be-order.c: New.
Index: gcc/testsuite/gcc.dg/vmx/sum2s.c
===
--- gcc/testsuite/gcc.dg/vmx/sum2s.c(revision 0)
+++ gcc/testsuite/gcc.dg/vmx/sum2s.c(revision 0)
@@ -0,0 +1,13 @@
+#include "harness.h"
+
+static void test()
+{
+ vector signed int vsia = {-10,1,2,3};
+ vector signed int vsib = {100,101,102,-103};
+ vector signed int vsir;
+ vector signed int vsier = {0,92,0,-98};
+
+ vsir = vec_sum2s (vsia, vsib);
+
+ check (vec_all_eq (vsir, vsier), "vsir");
+}
Index: gcc/testsuite/gcc.dg/vmx/sum2s-be-order.c
===
--- gcc/testsuite/gcc.dg/vmx/sum2s-be-order.c (revision 0)
+++ gcc/testsuite/gcc.dg/vmx/sum2s-be-order.c (revision 0)
@@ -0,0 +1,19 @@
+/* { dg-options "-maltivec=be -mabi=altivec -std=gnu99 -mno-vsx" } */
+
+#include "harness.h"
+
+static void test()
+{
+ vector signed int vsia = {-10,1,2,3};
+ vector signed int vsib = {100,101,102,-103};
+ vector signed int vsir;
+#if __BYTE_ORDER__ == __ORDER_LITTLE_ENDIAN__
+ vector signed int vsier = {91,0,107,0};
+#else
+ vector signed int vsier = {0,92,0,-98};
+#endif
+
+ vsir = vec_sum2s (vsia, vsib);
+
+ check (vec_all_eq (vsir, vsier), "vsir");
+}
Index: gcc/config/rs6000/altivec.md
===
--- gcc/config/rs6000/altivec.md(revision 207479)
+++ gcc/config/rs6000/altivec.md(working copy)
@@ -1592,10 +1610,21 @@
(unspec:V4SI [(match_operand:V4SI 1 "register_operand" "v")
(match_operand:V4SI 2 "register_operand" "v")]
UNSPEC_VSUM2SWS))
- (set (reg:SI 110) (unspec:SI [(const_int 0)] UNSPEC_SET_VSCR))]
+ (set (reg:SI 110) (unspec:SI [(const_int 0)] UNSPEC_SET_VSCR))
+ (clobber (match_scratch:V4SI 3 "=v"))]
"TARGET_ALTIVEC"
- "vsum2sws %0,%1,%2"
- [(set_attr "type" "veccomplex")])
+{
+ if (VECTOR_ELT_ORDER_BIG)
+return "vsum2sws %0,%1,%2";
+ else
+return "vsldoi %3,%2,%2,12\n\tvsum2sws %3,%1,%3\n\tvsldoi %0,%3,%3,4";
+}
+ [(set_attr "type" "veccomplex")
+ (set (attr "length")
+ (if_then_else
+ (match_test "VECTOR_ELT_ORDER_BIG")
+ (const_string "4")
+ (const_string "12")))])
(define_insn "altivec_vsumsws"
[(set (match_operand:V4SI 0 "register_operand" "=v")
@@ -1606,7 +1635,7 @@
(clobber (match_scratch:V4SI 3 "=v"))]
"TARGET_ALTIVEC"
{
- if (BYTES_BIG_ENDIAN || VECTOR_ELT_ORDER_BIG)
+ if (VECTOR_ELT_ORDER_BIG)
return "vsumsws %0,%1,%2";
else
return "vspltw %3,%2,0\n\tvsumsws %3,%1,%3\n\tvspltw %0,%3,3";
@@ -1614,7 +1643,7 @@
[(set_attr "type" "veccomplex")
(set (attr "length")
(if_then_else
- (match_test "(BYTES_BIG_ENDIAN || VECTOR_ELT_ORDER_BIG)")
+ (match_test "(VECTOR_ELT_ORDER_BIG)")
(const_string "4")
(const_string "12")))])
Re: [patch] Fix array overflow in gcc.dg/vect/no-vfa-vect-depend-2.c
+cc jakub
On Tue, Feb 4, 2014 at 4:59 PM, Paul Pluzhnikov wrote:
> Greetings,
>
> The gcc.dg/vect/no-vfa-vect-depend-2.c failed for us, when linked with
> gold, but not when linked with BFD ld.
>
> The problem appears to be off-by-one error causing array out of bounds
> access, fixed by attached patch.
Alternate fix (used in no-vfa-vect-depend-3.c):
--- gcc/testsuite/gcc.dg/vect/no-vfa-vect-depend-2.c(revision 207487)
+++ gcc/testsuite/gcc.dg/vect/no-vfa-vect-depend-2.c(working copy)
@@ -5,8 +5,8 @@
#define N 17
-int ia[N] = {48,45,42,39,36,33,30,27,24,21,18,15,12,9,6,3,0};
-int ib[N] = {48,45,42,39,36,33,30,27,24,21,18,15,12,9,6,3,0};
+int ia[N + 1] = {48,45,42,39,36,33,30,27,24,21,18,15,12,9,6,3,0};
+int ib[N + 1] = {48,45,42,39,36,33,30,27,24,21,18,15,12,9,6,3,0};
int res[N] = {48,192,180,168,156,144,132,120,108,96,84,72,60,48,36,24,12};
__attribute__ ((noinline))
>
> OK for trunk?
>
> Thanks,
>
>
> gcc/testsuite/ChangeLog:
>
> 2014-02-04 Paul Pluzhnikov
>
> * gcc.dg/vect/no-vfa-vect-depend-2.c (main1): Fix buffer
> overflow.
>
>
> Index: gcc/testsuite/gcc.dg/vect/no-vfa-vect-depend-2.c
> ===
> --- gcc/testsuite/gcc.dg/vect/no-vfa-vect-depend-2.c(revision 207487)
> +++ gcc/testsuite/gcc.dg/vect/no-vfa-vect-depend-2.c(working copy)
> @@ -15,7 +15,7 @@
>int i;
>
>/* Not vectorizable due to data dependence: dependence distance 1. */
> - for (i = N - 1; i >= 0; i--)
> + for (i = N - 2; i >= 0; i--)
> {
>ia[i] = ia[i+1] * 4;
> }
> @@ -28,7 +28,7 @@
> }
>
>/* Vectorizable. Dependence distance -1. */
> - for (i = N - 1; i >= 0; i--)
> + for (i = N - 2; i >= 0; i--)
> {
>ib[i+1] = ib[i] * 4;
> }
--
Paul Pluzhnikov
[committed] Fix simplification and argument expansion of TLS references on PA
The attached change fixes a problem found in the libcap-ng package.
TLS call arguments
were not being pre expanded. As a result, the register arguments were
being clobbered.
After fixing pa_legitimate_constant_p() to return false for all TLS
references, I found that the
code in pa_emit_move_sequence to simplify TLS references in the form
CONST (PLUS SYMBOL_REF
CONST_INT) was not being executed. I fixed that and adjusted
CONSTANT_ADDRESS_P
to reject this form.
Tested on hppa-unknown-linux-gnu, hppa2.0w-hp-hpux11.11 and hppa64-hp-
hpux11.11.
Committed to trunk and 4.8.
Dave
--
John David Anglin dave.ang...@bell.net
2014-02-04 John David Anglin
PR target/59777
* config/pa/pa.c (legitimize_tls_address): Return original address
if not passed a SYMBOL_REF rtx.
(hppa_legitimize_address): Call legitimize_tls_address for all TLS
addresses.
(pa_emit_move_sequence): Simplify TLS source operands.
(pa_legitimate_constant_p): Reject all TLS constants.
* config/pa/pa.h (PA_SYMBOL_REF_TLS_P): Correct comment.
(CONSTANT_ADDRESS_P): Reject TLS CONST addresses.
Index: config/pa/pa.c
===
--- config/pa/pa.c (revision 207393)
+++ config/pa/pa.c (working copy)
@@ -916,9 +910,12 @@
legitimize_tls_address (rtx addr)
{
rtx ret, insn, tmp, t1, t2, tp;
- enum tls_model model = SYMBOL_REF_TLS_MODEL (addr);
- switch (model)
+ /* Currently, we can't handle anything but a SYMBOL_REF. */
+ if (GET_CODE (addr) != SYMBOL_REF)
+return addr;
+
+ switch (SYMBOL_REF_TLS_MODEL (addr))
{
case TLS_MODEL_GLOBAL_DYNAMIC:
tmp = gen_reg_rtx (Pmode);
@@ -1039,7 +1036,7 @@
&& !REG_POINTER (XEXP (x, 1)))
return gen_rtx_PLUS (Pmode, XEXP (x, 1), XEXP (x, 0));
- if (PA_SYMBOL_REF_TLS_P (x))
+ if (pa_tls_referenced_p (x))
return legitimize_tls_address (x);
else if (flag_pic)
return legitimize_pic_address (x, mode, gen_reg_rtx (Pmode));
@@ -1920,9 +1917,10 @@
not consider them legitimate constants. Loop optimizations can
call the emit_move_xxx with one as a source. */
if ((GET_CODE (operand1) != HIGH && immediate_operand (operand1, mode))
+ || (GET_CODE (operand1) == HIGH
+ && symbolic_operand (XEXP (operand1, 0), mode))
|| function_label_operand (operand1, VOIDmode)
- || (GET_CODE (operand1) == HIGH
- && symbolic_operand (XEXP (operand1, 0), mode)))
+ || pa_tls_referenced_p (operand1))
{
int ishighonly = 0;
@@ -10300,7 +10298,7 @@
/* TLS_MODEL_GLOBAL_DYNAMIC and TLS_MODEL_LOCAL_DYNAMIC are not
legitimate constants. The other variants can't be handled by
the move patterns after reload starts. */
- if (PA_SYMBOL_REF_TLS_P (x))
+ if (pa_tls_referenced_p (x))
return false;
if (TARGET_64BIT && GET_CODE (x) == CONST_DOUBLE)
Index: config/pa/pa.h
===
--- config/pa/pa.h (revision 207393)
+++ config/pa/pa.h (working copy)
@@ -784,9 +784,9 @@
#define MAX_REGS_PER_ADDRESS 2
-/* Non-TLS symbolic references. */
-#define PA_SYMBOL_REF_TLS_P(RTX) \
- (GET_CODE (RTX) == SYMBOL_REF && SYMBOL_REF_TLS_MODEL (RTX) != 0)
+/* TLS symbolic reference. */
+#define PA_SYMBOL_REF_TLS_P(X) \
+ (GET_CODE (X) == SYMBOL_REF && SYMBOL_REF_TLS_MODEL (X) != 0)
/* Recognize any constant value that is a valid address except
for symbolic addresses. We get better CSE by rejecting them
@@ -796,7 +796,8 @@
#define CONSTANT_ADDRESS_P(X) \
((GET_CODE (X) == LABEL_REF \
|| (GET_CODE (X) == SYMBOL_REF && !SYMBOL_REF_TLS_MODEL (X))
\
- || GET_CODE (X) == CONST_INT || GET_CODE (X) == CONST \
+ || GET_CODE (X) == CONST_INT
\
+ || (GET_CODE (X) == CONST && !pa_tls_referenced_p (X)) \
|| GET_CODE (X) == HIGH)\
&& (reload_in_progress || reload_completed \
|| ! pa_symbolic_expression_p (X)))
[patch] Fix array overflow in gcc.dg/vect/no-vfa-vect-depend-2.c
Greetings,
The gcc.dg/vect/no-vfa-vect-depend-2.c failed for us, when linked with
gold, but not when linked with BFD ld.
The problem appears to be off-by-one error causing array out of bounds
access, fixed by attached patch.
OK for trunk?
Thanks,
gcc/testsuite/ChangeLog:
2014-02-04 Paul Pluzhnikov
* gcc.dg/vect/no-vfa-vect-depend-2.c (main1): Fix buffer
overflow.
Index: gcc/testsuite/gcc.dg/vect/no-vfa-vect-depend-2.c
===
--- gcc/testsuite/gcc.dg/vect/no-vfa-vect-depend-2.c(revision 207487)
+++ gcc/testsuite/gcc.dg/vect/no-vfa-vect-depend-2.c(working copy)
@@ -15,7 +15,7 @@
int i;
/* Not vectorizable due to data dependence: dependence distance 1. */
- for (i = N - 1; i >= 0; i--)
+ for (i = N - 2; i >= 0; i--)
{
ia[i] = ia[i+1] * 4;
}
@@ -28,7 +28,7 @@
}
/* Vectorizable. Dependence distance -1. */
- for (i = N - 1; i >= 0; i--)
+ for (i = N - 2; i >= 0; i--)
{
ib[i+1] = ib[i] * 4;
}
Re: PR ipa/59831 (ipa-cp devirt issues)
> Hi,
>
> On Mon, Feb 03, 2014 at 12:52:49AM +0100, Jan Hubicka wrote:
> > Hi,
> > this patch fixes the bug in extr_type_from_vtbl_ptr_store that made it to
> > consider store of construction virtual table or virtual table of virtual
> > base
> > as store of type's virtual table.
> >
> > In the testcase we have after early inlining:
> > virtual C::~C() (struct C * const this)
> > {
> > unsigned int i;
> > struct MultiTermDocs * _4;
> > struct A * _7;
> > unsigned int _10;
> >
> > :
> > this_2(D)->D.2980._vptr.MultiTermDocs = &MEM[(void *)&_ZTV1C + 24B];
> > _4 = &this_2(D)->D.2980;
> > MEM[(struct MultiTermDocs *)this_2(D)]._vptr.MultiTermDocs = &MEM[(void
> > *)&_ZTC1C0_13MultiTermDocs + 24B];
> > MultiTermDocs::wrap (_4);
> >
> > &_ZTC1C0_13MultiTermDocs is the construction vtable, while its context is
> > structure C and we thus assume that it is initialized to &_ZTV1C + 16B in
> > the
> > rest of code.
> >
> > This leads to wrong jump function:
> > Jump functions of caller virtual C::~C()/29:
> > callsite virtual C::~C()/29 -> MultiTermDocs::~MultiTermDocs()/10 :
> >param 0: KNOWN TYPE: base struct C, offset 0, component struct
> > MultiTermDocs
> >param 1: CONST: &MEM[(void *)&_ZTT1C + 8B]
> >
> > This is wrong, since type of _4 at call of WRAP is really construction of C.
> > With the patch we get:
> >
> > Jump functions of caller virtual C::~C()/29:
> > callsite virtual C::~C()/29 -> void MultiTermDocs::wrap()/8 :
>
> This jump function describes a different call than the one above, I
> assume the problem was that we had KNOWN_TYPE here too.
Yes, sorry, seems I copied dumps from different compilation.
The jump function from mainline before patch on testcase atteched is:
Jump functions of caller virtual C::~C()/29:
callsite virtual C::~C()/29 -> void MultiTermDocs::wrap()/8 :
param 0: KNOWN TYPE: base struct C, offset 0, component struct
MultiTermDocs
>
> >param 0: ANCESTOR: 0, offset 0, struct MultiTermDocs
> >
> > It looks bit confusing, but the ANCESTOR has no type_preserved flag,
>
> ANCESTOR jump functions certainly have type_preserved flag and as long
> as they are created with ipa_set_ancestor_jf it is impossible to
Breakpoint 5, ipa_set_ancestor_jf (jfunc=0x769050b0, offset=0,
type=0x76cd2f18, formal_id=0, agg_preserved=false, type_preserved=false) at
../../gcc/ipa-prop.c:480
480 jfunc->type = IPA_JF_ANCESTOR;
(gdb) bt
#0 ipa_set_ancestor_jf (jfunc=0x769050b0, offset=0, type=0x76cd2f18,
formal_id=0, agg_preserved=false, type_preserved=false) at
../../gcc/ipa-prop.c:480
#1 0x00b6f89f in compute_complex_assign_jump_func (info=0x1f0af58,
parms_ainfo=0x7fffe640, jfunc=0x769050b0, call=0x76923098,
stmt=0x768f3c30, name=0x76921000,
param_type=0x76cd52a0) at ../../gcc/ipa-prop.c:1108
#2 0x00b714a1 in ipa_compute_jump_functions_for_edge
(parms_ainfo=0x7fffe640, cs=0x76904750) at ../../gcc/ipa-prop.c:1650
#3 0x00b71742 in ipa_compute_jump_functions (node=0x768efe18,
parms_ainfo=0x7fffe640) at ../../gcc/ipa-prop.c:1699
#4 0x00b72f42 in ipa_analyze_node (node=0x768efe18) at
../../gcc/ipa-prop.c:2236
(gdb) up
#1 0x00b6f89f in compute_complex_assign_jump_func (info=0x1f0af58,
parms_ainfo=0x7fffe640, jfunc=0x769050b0, call=0x76923098,
stmt=0x768f3c30, name=0x76921000,
param_type=0x76cd52a0) at ../../gcc/ipa-prop.c:1108
1108 call, ssa),
type_p);
(gdb) l
1103 bool type_p = !detect_type_change (op1, base, TREE_TYPE
(param_type),
1104 call, jfunc, offset);
1105 if (type_p || jfunc->type == IPA_JF_UNKNOWN)
1106ipa_set_ancestor_jf (jfunc, offset, TREE_TYPE (op1), index,
1107 parm_ref_data_pass_through_p
(&parms_ainfo[index],
1108 call, ssa),
type_p);
so here we have type_p false but we still make ancestor jf - that is correct
(kind of): we really call destructor on ancestor and we change the dynamic type
(by storing construction vtable) before it.
> forget to set/clear it. I have even verified we update and honor the
> flag during inlining.
>
> > so it is basically just PASS_THROUGH in a funny representation.
>
> Well, I have always known we produce ancestors with zero offset given
> exactly this kind of input but I have never seen any real need to
> convert them to simple pass-throughs.
Yep, no problem with that - just made me bit confused as I originaly read
ANCESTOR in a sense of implicit TYPE_PRESERVED flag :)
Honza
>
> Anyway, the ipa-prop.c part is of course fine assuming that
> ipa-devirt.c parts work :-) But I'm too tired to attempt to understand
> it now and will probably need to read the whole file again anyway
> beca
Re: PR ipa/59831 (ipa-cp devirt issues)
Hi,
On Mon, Feb 03, 2014 at 12:52:49AM +0100, Jan Hubicka wrote:
> Hi,
> this patch fixes the bug in extr_type_from_vtbl_ptr_store that made it to
> consider store of construction virtual table or virtual table of virtual base
> as store of type's virtual table.
>
> In the testcase we have after early inlining:
> virtual C::~C() (struct C * const this)
> {
> unsigned int i;
> struct MultiTermDocs * _4;
> struct A * _7;
> unsigned int _10;
>
> :
> this_2(D)->D.2980._vptr.MultiTermDocs = &MEM[(void *)&_ZTV1C + 24B];
> _4 = &this_2(D)->D.2980;
> MEM[(struct MultiTermDocs *)this_2(D)]._vptr.MultiTermDocs = &MEM[(void
> *)&_ZTC1C0_13MultiTermDocs + 24B];
> MultiTermDocs::wrap (_4);
>
> &_ZTC1C0_13MultiTermDocs is the construction vtable, while its context is
> structure C and we thus assume that it is initialized to &_ZTV1C + 16B in the
> rest of code.
>
> This leads to wrong jump function:
> Jump functions of caller virtual C::~C()/29:
> callsite virtual C::~C()/29 -> MultiTermDocs::~MultiTermDocs()/10 :
>param 0: KNOWN TYPE: base struct C, offset 0, component struct
> MultiTermDocs
>param 1: CONST: &MEM[(void *)&_ZTT1C + 8B]
>
> This is wrong, since type of _4 at call of WRAP is really construction of C.
> With the patch we get:
>
> Jump functions of caller virtual C::~C()/29:
> callsite virtual C::~C()/29 -> void MultiTermDocs::wrap()/8 :
This jump function describes a different call than the one above, I
assume the problem was that we had KNOWN_TYPE here too.
>param 0: ANCESTOR: 0, offset 0, struct MultiTermDocs
>
> It looks bit confusing, but the ANCESTOR has no type_preserved flag,
ANCESTOR jump functions certainly have type_preserved flag and as long
as they are created with ipa_set_ancestor_jf it is impossible to
forget to set/clear it. I have even verified we update and honor the
flag during inlining.
> so it is basically just PASS_THROUGH in a funny representation.
Well, I have always known we produce ancestors with zero offset given
exactly this kind of input but I have never seen any real need to
convert them to simple pass-throughs.
Anyway, the ipa-prop.c part is of course fine assuming that
ipa-devirt.c parts work :-) But I'm too tired to attempt to understand
it now and will probably need to read the whole file again anyway
because I rememer little and am getting increasingly lost in this
discussion.
Thanks,
Martin
[google gcc-4_8] unify int and fp scaling in gcov-tool
Hi,
The attached patch uses a callback function to unify the integer and
floating-point scaling in gcov-tool. (Also fix a bug in fp scaling of
ic and dc counters in earlier code).
Tested with spec2006 profiles.
OK for checking in?
Thanks,
-Rong
2014-02-04 Rong Xu
* gcc/gcov-tool.c (profile_rewrite2): Remove.
(profile_rewrite): Merge int and fp scaling.
(do_rewrite): Ditto.
* libgcc/libgcov-util.c (typedef gcov_type) New.
(__gcov_add_counter_op): Merge int and fp scaling.
(__gcov_ior_counter_op): Ditto.
(__gcov_delta_counter_op): Ditto.
(__gcov_single_counter_op): Ditto.
(__gcov_icall_topn_op): Ditto.
(__gcov_dc_op): Ditto.
(fp_scale): Ditto.
(int_scale): Ditto.
(gcov_profile_scale): Ditto.
(gcov_profile_normalize): Ditto.
(__gcov_scale2_add): Remove.
(__gcov_scale2_ior): Remove.
(__gcov_scale2_delta): Remove.
(__gcov_scale2_single): Remove.
(__gcov_scale2_icall_topn): Remove.
(__gcov_scale2_dc): Remove.
(gcov_profile_scale2): Remove.
Index: gcc/gcov-tool.c
===
--- gcc/gcov-tool.c (revision 207488)
+++ gcc/gcov-tool.c (working copy)
@@ -43,8 +43,7 @@ see the files COPYING3 and COPYING.RUNTIME respect
extern int gcov_profile_merge (struct gcov_info*, struct gcov_info*, int, int);
extern int gcov_profile_normalize (struct gcov_info*, gcov_type);
-extern int gcov_profile_scale (struct gcov_info*, float);
-extern int gcov_profile_scale2 (struct gcov_info*, int, int);
+extern int gcov_profile_scale (struct gcov_info*, float, int, int);
extern struct gcov_info* gcov_read_profile_dir (const char*, int);
extern void gcov_exit (void);
extern void set_gcov_list (struct gcov_info *);
@@ -227,45 +226,13 @@ do_merge (int argc, char **argv)
return ret;
}
-/* Scale the counters in profile DIR by a factor of N/D.
- Result is written to dir OUT. Return 0 on success. */
-
-static int
-profile_rewrite2 (const char *dir, const char *out, int n, int d)
-{
- char *pwd;
- int ret;
- struct gcov_info * profile;
-
-
- profile = gcov_read_profile_dir (dir, 0);
- if (!profile)
-return 1;
-
- /* Output new profile. */
- unlink_profile_dir (out);
- mkdir (out, 0755);
- pwd = getcwd (NULL, 0);
- gcc_assert (pwd);
- ret = chdir (out);
- gcc_assert (ret == 0);
-
- gcov_profile_scale2 (profile, n, d);
-
- set_gcov_list (profile);
- gcov_exit ();
-
- ret = chdir (pwd);
- free (pwd);
- return 0;
-}
-
/* If N_VAL is no-zero, normalize the profile by setting the largest counter
counter value to N_VAL and scale others counters proportionally.
Otherwise, multiply the all counters by SCALE. */
static int
-profile_rewrite (const char *d1, const char *out, long long n_val, float scale)
+profile_rewrite (const char *d1, const char *out, long long n_val,
+ float scale, int n, int d)
{
char *pwd;
int ret;
@@ -287,7 +254,7 @@ static int
if (n_val)
gcov_profile_normalize (d1_profile, (gcov_type) n_val);
else
-gcov_profile_scale (d1_profile, scale);
+gcov_profile_scale (d1_profile, scale, n, d);
set_gcov_list (d1_profile);
gcov_exit ();
@@ -604,9 +571,9 @@ do_rewrite (int argc, char **argv)
if (argc - optind == 1)
{
if (denominator > 0)
-ret = profile_rewrite2 (argv[optind], output_dir, numerator,
denominator);
+ret = profile_rewrite (argv[optind], output_dir, 0, 0.0, numerator,
denominator);
else
-ret = profile_rewrite (argv[optind], output_dir, normalize_val,
scale);
+ret = profile_rewrite (argv[optind], output_dir, normalize_val,
scale, 0, 0);
}
else
rewrite_usage ();
Index: libgcc/libgcov-util.c
===
--- libgcc/libgcov-util.c (revision 207488)
+++ libgcc/libgcov-util.c (working copy)
@@ -873,39 +873,38 @@ gcov_profile_merge (struct gcov_info *tgt_profile,
return 0;
}
-/* This part of code is to scale profile counters. */
+typedef gcov_type (*counter_op_fn) (gcov_type, void*, void*);
-/* Type of function used to normalize counters. */
-typedef void (*gcov_scale_fn) (gcov_type *, gcov_unsigned_t, double);
+/* Performing FN upon arc counters. */
-/* Scale arc counters. N_COUNTERS of counter value in COUNTERS array are
- multiplied by a factor F. */
-
static void
-__gcov_scale_add (gcov_type *counters, unsigned n_counters, double f)
+__gcov_add_counter_op (gcov_type *counters, unsigned n_counters,
+ counter_op_fn fn, void *data1, void *data2)
{
for (; n_counters; counters++, n_counters--)
{
gcov_type val = *counters;
- *counters = val * f;
+ *counters = fn(val, data1, data2);
}
}
-/* Scale ior counters. */
+/* Performing FN upon ior counters. */
static void
-__gcov_scale_ior (gcov_type
Re: PR ipa/59831 (ipa-cp devirt issues)
> > - if (TREE_CODE (t) != TREE_BINFO)
> > + /* Try to work out BINFO from virtual table pointer value in
> > replacements. */
> > + if (!t && agg_reps && !ie->indirect_info->by_ref)
>
> At this point you know that !ie->indirect_info->polymorphic is set and
> thus ie->indirect_info->by_ref is always false because it really has
> no meaning (it is only meaningful when agg_contents is set which is
> mutually exclusive with the polymorphic flag).
I was worried here about case where in future we may want to represent call of
virtual methods from pointers passed by reference (i.e. in some other object).
We don't do that at the moment, but for that would really need better jump
functions.
If you preffer, I can remove that check.
Honza
>
> > {
> > - tree binfo;
> > - binfo = gimple_extract_devirt_binfo_from_cst
> > -(t, ie->indirect_info->otr_type);
> > - if (!binfo)
> > + while (agg_reps)
> > + {
> > + if (agg_reps->index == param_index
> > + && agg_reps->offset == ie->indirect_info->offset
> > + && agg_reps->by_ref)
> > + {
> > + debug_tree (t);
> > + t = agg_reps->value;
> > + t = vtable_pointer_value_to_binfo (t);
> > + break;
> > + }
> > + agg_reps = agg_reps->next;
> > + }
> > +}
> > +
> > + /* Try to work out BINFO from virtual table pointer value in known
> > + known aggregate values. */
> > + if (!t && known_aggs.length () > (unsigned int) param_index
> > + && !ie->indirect_info->by_ref)
>
> The same here.
>
> Thanks,
>
> Martin
Fix handling of DECL_ONE_ONLY with LTO partitioning
Hi,
I tried to rebuild firefox without visibilities that leads to about 10fold
increase
in LTO time, mostly by ltrans. What happens is that by the old rule that
DECL_ONE_ONLY is duplicated into each partition that needs it we end up
duplicating
things everywhere. Moreover we mark tem all as used_from_ohter_partition and
that
makes us to not optimize them out from any of the partitions even if they are
dead.
I recall my analysis when I was adding this condition - I assumed that every
unit that uses COMDAT must also define it. This is wrong for keyed classes (as
was later corrected, but the not updated for forced_by_abi) and in fact when we
know that the COMDAT is going to be output to final file - i.e. linker plugin
tells us there is no one defining the symbol, we can decompose COMDAT group
early.
I also noticed bug in varasm where we ignore the fact that symbol is known to
be output
in other partition that causes us to use GOT/PLT references with this fix and
hidden
visibilities re-instantiated.
I believe this is the problem causing random jumps in Martin Liska's systetap
graphs.
Martin, is there any chance to generated updated graphs for firefox or other C++
monster, idealy both with -freorder-blocks-and-partition and with
-fno-reorder-blocks-and-partition?
Bootstrapped/regtested x86_64-linux, tested on firefox and comitted.
Honza
* lto/lto-partition.c (get_symbol_class): Only unforced DECL_ONE_ONLY
needs duplicating, not generic COMDAT.
* ipa.c (function_and_variable_visibility): Decompose DECL_ONE_ONLY
groups when we know they are controlled by LTO.
* varasm.c (default_binds_local_p_1): If object is in other partition,
it will be resolved locally.
Index: lto/lto-partition.c
===
--- lto/lto-partition.c (revision 207479)
+++ lto/lto-partition.c (working copy)
@@ -94,10 +94,12 @@ get_symbol_class (symtab_node *node)
else if (!cgraph (node)->definition)
return SYMBOL_EXTERNAL;
- /* Comdats are duplicated to every use unless they are keyed.
- Those do not need duplication. */
- if (DECL_COMDAT (node->decl)
+ /* Linker discardable symbols are duplicated to every use unless they are
+ keyed.
+ Keyed symbols or those. */
+ if (DECL_ONE_ONLY (node->decl)
&& !node->force_output
+ && !node->forced_by_abi
&& !symtab_used_from_object_file_p (node))
return SYMBOL_DUPLICATE;
Index: ipa.c
===
--- ipa.c (revision 207451)
+++ ipa.c (working copy)
@@ -1002,6 +1002,36 @@ function_and_variable_visibility (bool w
if (DECL_EXTERNAL (decl_node->decl))
DECL_EXTERNAL (node->decl) = 1;
}
+
+ /* If whole comdat group is used only within LTO code, we can dissolve
it,
+we handle the unification ourselves.
+We keep COMDAT and weak so visibility out of DSO does not change.
+Later we may bring the symbols static if they are not exported. */
+ if (DECL_ONE_ONLY (node->decl)
+ && (node->resolution == LDPR_PREVAILING_DEF_IRONLY
+ || node->resolution == LDPR_PREVAILING_DEF_IRONLY_EXP))
+ {
+ symtab_node *next = node;
+
+ if (node->same_comdat_group)
+ for (next = node->same_comdat_group;
+next != node;
+next = next->same_comdat_group)
+ if (next->externally_visible
+ && (next->resolution != LDPR_PREVAILING_DEF_IRONLY
+ && next->resolution != LDPR_PREVAILING_DEF_IRONLY_EXP))
+ break;
+ if (node == next)
+ {
+ if (node->same_comdat_group)
+ for (next = node->same_comdat_group;
+next != node;
+next = next->same_comdat_group)
+ DECL_COMDAT_GROUP (next->decl) = NULL;
+ DECL_COMDAT_GROUP (node->decl) = NULL;
+ symtab_dissolve_same_comdat_group_list (node);
+ }
+ }
}
FOR_EACH_DEFINED_FUNCTION (node)
{
Index: varasm.c
===
--- varasm.c(revision 207451)
+++ varasm.c(working copy)
@@ -6739,7 +6739,7 @@ default_binds_local_p_1 (const_tree exp,
&& (TREE_STATIC (exp) || DECL_EXTERNAL (exp)))
{
varpool_node *vnode = varpool_get_node (exp);
- if (vnode && resolution_local_p (vnode->resolution))
+ if (vnode && (resolution_local_p (vnode->resolution) ||
vnode->in_other_partition))
resolved_locally = true;
if (vnode
&& resolution_to_local_definition_p (vnode->resolution))
@@ -6749,7 +6749,7 @@ default_binds_local_p_1 (const_tree exp,
{
struct cgraph_node *node = cgraph_get_node (exp);
if (node
- && resolution_local_p (node->resolution))
+ && (resolution_local_p (node->resolution) ||
n
Re: PR ipa/59831 (ipa-cp devirt issues)
Hi,
On Fri, Jan 31, 2014 at 07:22:55AM +0100, Jan Hubicka wrote:
...
> PR ipa/59831
> * gimple-fold.c (gimple_extract_devirt_binfo_from_cst): Remove.
> * ipa-devirt.c (get_poymorphic_call_info_for_decl): Break out from ...
> (get_polymorphic_call_info): ... here.
> (get_polymorphic_call_info_from_invariant): New function based on
> gimple_extract_devirt_binfo_from_cst.
> (try_make_edge_direct_virtual_call): Update to use ipa-devirt.
> (ipa_get_indirect_edge_target_1): Likewise.
> (get_polymorphic_call_info_from_invariant): New function.
> (vtable_pointer_value_to_binfo): New function.
> * ipa-utils.h (get_polymorphic_call_info_from_invariant): Declare.
> (vtable_pointer_value_to_binfo): Declare.
> * ipa-prop.c (extr_type_from_vtbl_ptr_store): Use it.
> (try_make_edge_direct_virtual_call): Use aggregate propagation;
> rewrite handling of constants.
> * ipa-cp.c (ipa_get_indirect_edge_target_1): Likewise.
>
> * testsuite/g++.dg/ipa/devirt-21.C: New testcase.
> * testsuite/g++.dg/ipa/devirt-20.C: Fix template.
...
> Index: ipa-cp.c
> ===
> --- ipa-cp.c (revision 207287)
> +++ ipa-cp.c (working copy)
> @@ -117,6 +117,7 @@ along with GCC; see the file COPYING3.
> #include "params.h"
> #include "ipa-inline.h"
> #include "ipa-utils.h"
> +#include "print-tree.h"
>
> struct ipcp_value;
>
> @@ -1479,9 +1480,9 @@ ipa_get_indirect_edge_target_1 (struct c
>HOST_WIDE_INT token, anc_offset;
>tree otr_type;
>tree t;
> - tree target;
> + tree target = NULL;
>
> - if (param_index == -1
> + if (!flag_devirtualize || param_index == -1
>|| known_vals.length () <= (unsigned int) param_index)
> return NULL_TREE;
>
> @@ -1532,25 +1533,76 @@ ipa_get_indirect_edge_target_1 (struct c
>anc_offset = ie->indirect_info->offset;
>otr_type = ie->indirect_info->otr_type;
>
> - t = known_vals[param_index];
> + t = NULL;
> +
> + /* Do we know BINFO? */
>if (!t && known_binfos.length () > (unsigned int) param_index)
> t = known_binfos[param_index];
> - if (!t)
> -return NULL_TREE;
> + /* FIXME: ipcp_discover_new_direct_edges makes no difference between
> + constants and binfos. This is because ipa-cp currently assumes that
> known
> + value is always better than binfo. Hopefully this will be fixed when
> + ipa-cp is turned to propagate polymorphic call contextes. */
> + if (known_vals[param_index] && TREE_CODE (known_vals[param_index]) ==
> TREE_BINFO)
> +t = known_vals[param_index];
>
> - if (TREE_CODE (t) != TREE_BINFO)
> + /* Try to work out BINFO from virtual table pointer value in replacements.
> */
> + if (!t && agg_reps && !ie->indirect_info->by_ref)
At this point you know that !ie->indirect_info->polymorphic is set and
thus ie->indirect_info->by_ref is always false because it really has
no meaning (it is only meaningful when agg_contents is set which is
mutually exclusive with the polymorphic flag).
> {
> - tree binfo;
> - binfo = gimple_extract_devirt_binfo_from_cst
> - (t, ie->indirect_info->otr_type);
> - if (!binfo)
> + while (agg_reps)
> + {
> + if (agg_reps->index == param_index
> + && agg_reps->offset == ie->indirect_info->offset
> + && agg_reps->by_ref)
> + {
> + debug_tree (t);
> + t = agg_reps->value;
> + t = vtable_pointer_value_to_binfo (t);
> + break;
> + }
> + agg_reps = agg_reps->next;
> + }
> +}
> +
> + /* Try to work out BINFO from virtual table pointer value in known
> + known aggregate values. */
> + if (!t && known_aggs.length () > (unsigned int) param_index
> + && !ie->indirect_info->by_ref)
The same here.
Thanks,
Martin
Re: [MIPS] Use soft-fp for libgcc floating-point routines
On Tue, 4 Feb 2014, Richard Sandiford wrote:
> So here I just provide simple C functions for each operation that has
> hard-float support. We can then restrict the softfp stuff to functions
> that really do need softfp support, meaning that softfp_exclude_libgcc2 := n
> should always be OK. As an added bonus it reduces the footprint of
> libgcc_s.so
> on hard-float targets.
Simple C functions are indeed the best thing for cases where the functions
are only part of the ABI at all because of historical accident of it being
hard to build fp-bit only for soft-float multilibs. But they're also
useful for more than just MIPS - 32-bit powerpc hard float could use them
as well, for example (it seems likely there are other cases, but powerpc
is one I know about). So I think the implementation and a makefile
fragment for them should be outside config/mips, although targets may need
to configure exactly which functions to build from this set. (For
example, e500v1 would want simple C functions for SFmode but soft-fp for
DFmode, and for __unordsf2; e500v2 would want soft-fp for __unordsf2 and
__unorddf2 but otherwise simple C functions. I don't think the initial
patch needs to build out the full configurability, as long as the files
are in the right place, outside config/mips.)
A patch putting the files in an architecture-independent place should
probably then be reviewed by Ian as libgcc maintainer.
> +#elif defined (OP_eq2)
> +int FUNC (TYPE x, TYPE y) { return x == y; }
> +#elif defined (OP_ne2)
> +int FUNC (TYPE x, TYPE y) { return x != y; }
> +#elif defined (OP_ge2)
> +int FUNC (TYPE x, TYPE y) { return x >= y; }
> +#elif defined (OP_gt2)
> +int FUNC (TYPE x, TYPE y) { return x > y; }
> +#elif defined (OP_le2)
> +int FUNC (TYPE x, TYPE y) { return x <= y; }
> +#elif defined (OP_lt2)
> +int FUNC (TYPE x, TYPE y) { return x < y; }
That's not the semantics of the comparison functions. They return an
integer value that can be compared to 0 with the original operation to get
the final result, rather than returning the result of the comparison
directly. See libgcc.texi.
--
Joseph S. Myers
jos...@codesourcery.com
New Dutch PO file for 'gcc' (version 4.9-b20140202)
Hello, gentle maintainer. This is a message from the Translation Project robot. A revised PO file for textual domain 'gcc' has been submitted by the Dutch team of translators. The file is available at: http://translationproject.org/latest/gcc/nl.po (This file, 'gcc-4.9-b20140202.nl.po', has just now been sent to you in a separate email.) All other PO files for your package are available in: http://translationproject.org/latest/gcc/ Please consider including all of these in your next release, whether official or a pretest. Whenever you have a new distribution with a new version number ready, containing a newer POT file, please send the URL of that distribution tarball to the address below. The tarball may be just a pretest or a snapshot, it does not even have to compile. It is just used by the translators when they need some extra translation context. The following HTML page has been updated: http://translationproject.org/domain/gcc.html If any question arises, please contact the translation coordinator. Thank you for all your work, The Translation Project robot, in the name of your translation coordinator.
patch to fix PR59913
The following patch fixes
http://gcc.gnu.org/bugzilla/show_bug.cgi?id=59913
The patch was successfully tested and bootstrapped on x86/x86-64 and arm.
Committed as rev. 207485.
2014-02-04 Vladimir Makarov
PR bootstrap/59913
* lra-constraints.c (need_for_split_p): Use more 3 reloads as
threshold for pseudo splitting.
(update_ebb_live_info): Process call argument hard registers and
hard registers from insn definition too.
(max_small_class_regs_num): New constant.
(inherit_in_ebb): Update live hard regs through EBBs. Update
reloads_num only for small register classes. Don't split for
outputs of jumps.
Index: lra-constraints.c
===
--- lra-constraints.c (revision 207428)
+++ lra-constraints.c (working copy)
@@ -4583,7 +4583,7 @@ need_for_split_p (HARD_REG_SET potential
impossibility to find hard register for reload pseudo of
small register class. */
&& (usage_insns[regno].reloads_num
- + (regno < FIRST_PSEUDO_REGISTER ? 0 : 2) < reloads_num)
+ + (regno < FIRST_PSEUDO_REGISTER ? 0 : 3) < reloads_num)
&& (regno < FIRST_PSEUDO_REGISTER
/* For short living pseudos, spilling + inheritance can
be considered a substitution for splitting.
@@ -4840,7 +4840,7 @@ static void
update_ebb_live_info (rtx head, rtx tail)
{
unsigned int j;
- int regno;
+ int i, regno;
bool live_p;
rtx prev_insn, set;
bool remove_p;
@@ -4898,6 +4898,7 @@ update_ebb_live_info (rtx head, rtx tail
if (! NONDEBUG_INSN_P (curr_insn))
continue;
curr_id = lra_get_insn_recog_data (curr_insn);
+ curr_static_id = curr_id->insn_static_data;
remove_p = false;
if ((set = single_set (curr_insn)) != NULL_RTX && REG_P (SET_DEST (set))
&& (regno = REGNO (SET_DEST (set))) >= FIRST_PSEUDO_REGISTER
@@ -4908,11 +4909,23 @@ update_ebb_live_info (rtx head, rtx tail
for (reg = curr_id->regs; reg != NULL; reg = reg->next)
if (reg->type == OP_OUT && ! reg->subreg_p)
bitmap_clear_bit (&live_regs, reg->regno);
+ for (reg = curr_static_id->hard_regs; reg != NULL; reg = reg->next)
+ if (reg->type == OP_OUT && ! reg->subreg_p)
+ bitmap_clear_bit (&live_regs, reg->regno);
/* Mark each used value as live. */
for (reg = curr_id->regs; reg != NULL; reg = reg->next)
if (reg->type != OP_OUT
&& bitmap_bit_p (&check_only_regs, reg->regno))
bitmap_set_bit (&live_regs, reg->regno);
+ for (reg = curr_static_id->hard_regs; reg != NULL; reg = reg->next)
+ if (reg->type != OP_OUT
+ && bitmap_bit_p (&check_only_regs, reg->regno))
+ bitmap_set_bit (&live_regs, reg->regno);
+ if (curr_id->arg_hard_regs != NULL)
+ /* Make argument hard registers live. */
+ for (i = 0; (regno = curr_id->arg_hard_regs[i]) >= 0; i++)
+ if (bitmap_bit_p (&check_only_regs, regno))
+ bitmap_set_bit (&live_regs, regno);
/* It is quite important to remove dead move insns because it
means removing dead store. We don't need to process them for
constraints. */
@@ -5002,6 +5015,14 @@ get_live_on_other_edges (basic_block fro
/* Used as a temporary results of some bitmap calculations. */
static bitmap_head temp_bitmap;
+/* We split for reloads of small class of hard regs. The following
+ defines how many hard regs the class should have to be qualified as
+ small. The code is mostly oriented to x86/x86-64 architecture
+ where some insns need to use only specific register or pair of
+ registers and these register can live in RTL explicitly, e.g. for
+ parameter passing. */
+static const int max_small_class_regs_num = 2;
+
/* Do inheritance/split transformations in EBB starting with HEAD and
finishing on TAIL. We process EBB insns in the reverse order.
Return true if we did any inheritance/split transformation in the
@@ -5036,7 +5057,8 @@ inherit_in_ebb (rtx head, rtx tail)
bitmap_clear (&check_only_regs);
last_processed_bb = NULL;
CLEAR_HARD_REG_SET (potential_reload_hard_regs);
- CLEAR_HARD_REG_SET (live_hard_regs);
+ COPY_HARD_REG_SET (live_hard_regs, eliminable_regset);
+ IOR_HARD_REG_SET (live_hard_regs, lra_no_alloc_regs);
/* We don't process new insns generated in the loop. */
for (curr_insn = tail; curr_insn != PREV_INSN (head); curr_insn = prev_insn)
{
@@ -5062,9 +5084,6 @@ inherit_in_ebb (rtx head, rtx tail)
|| (find_reg_note (last_insn,
REG_NORETURN, NULL_RTX) == NULL_RTX
&& ! SIBLING_CALL_P (last_insn;
- REG_SET_TO_HARD_REG_SET (live_hard_regs, df_get_live_out (curr_bb));
- IOR_HARD_REG_SET (live_hard_regs, eliminable_regset);
- IOR_HARD_REG_SET (live_hard_regs, lra_no_alloc_regs);
CLEAR_HARD_REG_SET (potential_reload_hard_regs);
EXECUTE_IF_SET_IN_BITMAP (to_process, 0, j, bi)
{
@@ -5098,7 +5117,8 @@ inherit_in_ebb (rtx head, rtx tail)
&& (cl = lra_get_allocno_class (dst_reg
Re: [google gcc-4_8] fix profiledbootstrap
ok. David On Tue, Feb 4, 2014 at 11:28 AM, Rong Xu wrote: > Hi, > > The attached patch fixes the duplicated definition error in > gcov-tool.c in profiledbootstrap. > > Google branch only and tested with profiledbootstrap. > > Ok for checking in? > > -Rong
Re: RFA: MN10300: Fix building libstdc++-v3
On 02/04/14 10:45, nick clifton wrote: Hi Jeff, But how/why did an extended register get selected to begin with? It happens in reload.c:find_valid_class(). HARD_REGNO_MODE_OK and other macros/hooks ought to be preventing this relatively early. Ah - and that is the problem. mn10300_hard_regno_mode_ok is allowing extended registers to match some classes even in MN10300. So how about the patch below ? This also allows the libstdc++-v3 library to be built. Cheers Nick gcc/ChangeLog 2014-02-04 Nick Clifton * config/mn10300/mn10300.c (mn10300_hard_regno_mode_ok): Do not accept extended registers in any mode when compiling for the MN10300. This is fine. I'm assuming that insn predicate checks prevent similar situations from occurring with FP_REGS & FP_ACC_REGS. ie, patterns with constraints which allow those registers are conditional on TARGET_AM33. Jeff
Re: [google gcc-4_8] gcov-tool: some new LIPO supports.
Ok. Thanks, Teresa On Tue, Feb 4, 2014 at 10:59 AM, Rong Xu wrote: > Here is the revised patch that integrates Teresa' comments. > Ok for checking in? > > -Rong > > On Thu, Jan 30, 2014 at 1:20 PM, Rong Xu wrote: >> Comments are inlined. New patch attached to this email. >> >> -Rong >> >> On Thu, Jan 30, 2014 at 12:39 PM, Xinliang David Li >> wrote: >>> On Wed, Jan 29, 2014 at 4:24 PM, Rong Xu wrote: Hi, The attached patch adds some new features to gcov-tool. It aims to rewrite LIPO profile for reuse, debug or performance tuning purpose. -l, --modu_list Only use the modules in this file >>> >>> I think in verbose mode, the tool should emit warnings when a module >>> is trimmed due to this option. This can be used in regression test. >>> >> >> In previous patch, this warning is emitted unconventionally (not >> guarded by verbose flag). >> I changed it to under verbose mode in this patch. >> -r, --string Replace string in path >>> >>> --path_substr_replace or something in that line. >>> >> >> Done. >> -u, --use_imports_file Use the grouping in import files. >>> >>> Is there a path in code to auto test this? >> >> As we discussed offline. This can be verified by a separated script. >> >>> -l uses only the modules in specified file to compute module grouping (and subsequent dumping). -r replaces the pattern specified in the argument. The format is: old_str1:new_str1[,old_str2:new_str2]*, only the first occurrence is replaced. -u skips the run-time module grouping computation and reuses the one comes with the profiles (which is user editable). Tested with profiles from google internal benchmarks. >>> >>> Also use strcasestr for case insenstive operation. >> >> Done. >> >>> >>> David >>> -Rong -- Teresa Johnson | Software Engineer | tejohn...@google.com | 408-460-2413
[google gcc-4_8] fix profiledbootstrap
Hi, The attached patch fixes the duplicated definition error in gcov-tool.c in profiledbootstrap. Google branch only and tested with profiledbootstrap. Ok for checking in? -Rong Index: gcov-tool.c === --- gcov-tool.c (revision 207435) +++ gcov-tool.c (working copy) @@ -50,16 +50,23 @@ static int verbose; -gcov_unsigned_t __gcov_lipo_grouping_algorithm; -gcov_unsigned_t __gcov_lipo_merge_modu_edges; -gcov_unsigned_t __gcov_lipo_weak_inclusion; -gcov_unsigned_t __gcov_lipo_max_mem; -gcov_unsigned_t __gcov_lipo_random_group_size; -gcov_unsigned_t __gcov_lipo_cutoff; -gcov_unsigned_t __gcov_lipo_random_seed; -gcov_unsigned_t __gcov_lipo_dump_cgraph; -gcov_unsigned_t __gcov_lipo_propagate_scale; +/* The following defines are needed by dyn-ipa.c. + They will also be emitted by the compiler with -fprofile-generate, + which means this file cannot be compiled with -fprofile-generate + -- otherwise we get duplicated defintions. + Make the defines weak to link with other objects/libraries + that potentially compiled with -fprofile-generate. */ +__attribute__ ((weak)) gcov_unsigned_t __gcov_lipo_grouping_algorithm; +__attribute__ ((weak)) gcov_unsigned_t __gcov_lipo_merge_modu_edges; +__attribute__ ((weak)) gcov_unsigned_t __gcov_lipo_weak_inclusion; +__attribute__ ((weak)) gcov_unsigned_t __gcov_lipo_max_mem; +__attribute__ ((weak)) gcov_unsigned_t __gcov_lipo_random_group_size; +__attribute__ ((weak)) gcov_unsigned_t __gcov_lipo_cutoff; +__attribute__ ((weak)) gcov_unsigned_t __gcov_lipo_random_seed; +__attribute__ ((weak)) gcov_unsigned_t __gcov_lipo_dump_cgraph; +__attribute__ ((weak)) gcov_unsigned_t __gcov_lipo_propagate_scale; + /* Remove file NAME if it has a gcda suffix. */ static int Index: Makefile.in === --- Makefile.in (revision 207435) +++ Makefile.in (working copy) @@ -4090,6 +4090,9 @@ GCOV_TOOL_OBJS = gcov-tool.o libgcov-util.o dyn-ipa.o gcov-params.o gcov-tool.o: gcov-tool.c $(GCOV_IO_H) intl.h $(SYSTEM_H) coretypes.h $(TM_H) \ $(CONFIG_H) version.h $(DIAGNOSTIC_H) + +$(COMPILER) -c $(ALL_CPPFLAGS) \ + $(filter-out -fprofile-generate -fprofile-use,$(ALL_COMPILERFLAGS)) \ + $(INCLUDES) -o $@ $< gcov-params.o: params.c $(PARAMS_H) +$(COMPILER) -DIN_GCOV_TOOL -c $(ALL_COMPILERFLAGS) $(ALL_CPPFLAGS) \ $(INCLUDES) -o $@ $<
[PATCH, rs6000] Implement -maltivec=be for vec_pack, vec_unpackh, vec_unpackl Altivec builtins
Hi,
Yet another -maltivec=be patch, this one for the vector pack/unpack
builtins. For the pack operations, we need to reverse the order of the
inputs for little endian without -maltivec=be. For the unpack
operations, we need to replace unpackh with unpackl, and vice versa, for
little endian without -maltivec=be.
For both pack and unpack, there are some internal uses that should not
have their semantics changed from the existing implementation. For
these, "_direct" forms of the insns are provided, as has been done in
previous patches in this series.
Four new test cases are added to test these builtins for all applicable
vector types, with and without -maltivec=be.
Bootstrapped and tested on powerpc64{,le}-unknown-linux-gnu with no
regressions. Is this ok for trunk?
Thanks,
Bill
gcc:
2014-02-04 Bill Schmidt
* altivec.md (UNSPEC_VPACK_UNS_UNS_MOD_DIRECT): New unspec.
(UNSPEC_VUNPACK_HI_SIGN_DIRECT): Likewise.
(UNSPEC_VUNPACK_LO_SIGN_DIRECT): Likewise.
(mulv8hi3): Use gen_altivec_vpkuwum_direct instead of
gen_altivec_vpkuwum.
(altivec_vpkpx): Test for VECTOR_ELT_ORDER_BIG instead of for
BYTES_BIG_ENDIAN.
(altivec_vpksss): Likewise.
(altivec_vpksus): Likewise.
(altivec_vpkuus): Likewise.
(altivec_vpkuum): Likewise.
(altivec_vpkuum_direct): New (copy of
altivec_vpkuum that still relies on BYTES_BIG_ENDIAN, for
internal use).
(altivec_vupkhs): Emit vupkls* instead of vupkhs* when
target is little endian and -maltivec=be is not specified.
(*altivec_vupkhs_direct): New (copy of
altivec_vupkhs that always emits vupkhs*, for internal
use).
(altivec_vupkls): Emit vupkhs* instead of vupkls* when
target is little endian and -maltivec=be is not specified.
(*altivec_vupkls_direct): New (copy of
altivec_vupkls that always emits vupkls*, for internal
use).
(altivec_vupkhpx): Emit vupklpx instead of vupkhpx when target is
little endian and -maltivec=be is not specified.
(altivec_vupklpx): Emit vupkhpx instead of vupklpx when target is
little endian and -maltivec=be is not specified.
gcc/testsuite:
2014-02-04 Bill Schmidt
* gcc.dg/vmx/pack.c: New.
* gcc.dg/vmx/pack-be-order.c: New.
* gcc.dg/vmx/unpack.c: New.
* gcc.dg/vmx/unpack-be-order.c: New.
Index: gcc/testsuite/gcc.dg/vmx/unpack.c
===
--- gcc/testsuite/gcc.dg/vmx/unpack.c (revision 0)
+++ gcc/testsuite/gcc.dg/vmx/unpack.c (revision 0)
@@ -0,0 +1,67 @@
+#include "harness.h"
+
+#define BIG 4294967295
+
+static void test()
+{
+ /* Input vectors. */
+ vector signed char vsc = {-8,-7,-6,-5,-4,-3,-2,-1,0,1,2,3,4,5,6,7};
+ vector bool char vbc = {0,255,255,0,0,0,255,0,255,0,0,255,255,255,0,255};
+ vector pixel vp = {(0<<15) + (1<<10) + (2<<5) + 3,
+(1<<15) + (4<<10) + (5<<5) + 6,
+(0<<15) + (7<<10) + (8<<5) + 9,
+(1<<15) + (10<<10) + (11<<5) + 12,
+(1<<15) + (13<<10) + (14<<5) + 15,
+(0<<15) + (16<<10) + (17<<5) + 18,
+(1<<15) + (19<<10) + (20<<5) + 21,
+(0<<15) + (22<<10) + (23<<5) + 24};
+ vector signed short vss = {-4,-3,-2,-1,0,1,2,3};
+ vector bool short vbs = {0,65535,65535,0,0,0,65535,0};
+
+ /* Result vectors. */
+ vector signed short vsch, vscl;
+ vector bool short vbsh, vbsl;
+ vector unsigned int vuih, vuil;
+ vector signed int vsih, vsil;
+ vector bool int vbih, vbil;
+
+ /* Expected result vectors. */
+ vector signed short vschr = {-8,-7,-6,-5,-4,-3,-2,-1};
+ vector signed short vsclr = {0,1,2,3,4,5,6,7};
+ vector bool short vbshr = {0,65535,65535,0,0,0,65535,0};
+ vector bool short vbslr = {65535,0,0,65535,65535,65535,0,65535};
+ vector unsigned int vuihr = {(0<<24) + (1<<16) + (2<<8) + 3,
+ (65535<<24) + (4<<16) + (5<<8) + 6,
+ (0<<24) + (7<<16) + (8<<8) + 9,
+ (65535<<24) + (10<<16) + (11<<8) + 12};
+ vector unsigned int vuilr = {(65535<<24) + (13<<16) + (14<<8) + 15,
+ (0<<24) + (16<<16) + (17<<8) + 18,
+ (65535<<24) + (19<<16) + (20<<8) + 21,
+ (0<<24) + (22<<16) + (23<<8) + 24};
+ vector signed int vsihr = {-4,-3,-2,-1};
+ vector signed int vsilr = {0,1,2,3};
+ vector bool int vbihr = {0,BIG,BIG,0};
+ vector bool int vbilr = {0,0,BIG,0};
+
+ vsch = vec_unpackh (vsc);
+ vscl = vec_unpackl (vsc);
+ vbsh = vec_unpackh (vbc);
+ vbsl = vec_unpackl (vbc);
+ vuih = vec_unpackh (vp);
+ vuil = vec_unpackl (vp);
+ vsih = vec_unpackh (vss);
+ vsil = vec_unpackl (vss);
+ vbih = vec_unpackh (vbs);
+ vbil = vec_unpackl (vbs);
+
+ check (vec_all_eq (vsch, vschr
Re: [C,C++] integer constants in attribute arguments
On Tue, 4 Feb 2014, Andreas Schwab wrote:
Marc Glisse writes:
Index: gcc/testsuite/g++.dg/cpp0x/constexpr-attribute2.C
===
--- gcc/testsuite/g++.dg/cpp0x/constexpr-attribute2.C (revision 0)
+++ gcc/testsuite/g++.dg/cpp0x/constexpr-attribute2.C (working copy)
@@ -0,0 +1,32 @@
+// { dg-options -std=gnu++11 }
+
+struct t { t(); };
+
+constexpr int prio = 123;
+constexpr int size = 8;
+constexpr int pos = 1;
+enum A { zero = 0, one, two };
+__attribute__((init_priority(prio))) t a;
+
+enum class E1 : int {
+first = 101,
+second,
+third,
+};
+__attribute__((init_priority(E1::second))) t b; // Should not compile?
+
+enum E2 {
+E2_first = 141,
+E2_second,
+E2_third,
+};
+__attribute__((init_priority(E2_second))) t c;
+
+void* my_calloc(unsigned, unsigned) __attribute__((alloc_size(pos,two)));
+void* my_realloc(void*, unsigned) __attribute__((alloc_size(two)));
+
+typedef char vec __attribute__((vector_size(size)));
+
+void f(char*) __attribute__((nonnull(pos)));
+
+void g() __attribute__((aligned(size)));
/usr/local/gcc/gcc-20140204/gcc/testsuite/g++.dg/cpp0x/constexpr-attribute2.C:32:6:
error: alignment for 'void g()' must be at least 16
(I don't know why we error out for this, align specifies a minimal
alignment, if it ends up more aligned, that's fine)
FAIL: g++.dg/cpp0x/constexpr-attribute2.C (test for excess errors)
It would be nice to at least mention the platform in this kind of email.
Feel free to replace 8 with 16 in the initialization of size (you can
commit it as obvious if it works for you).
Thanks,
--
Marc Glisse
Re: [google gcc-4_8] gcov-tool: some new LIPO supports.
Here is the revised patch that integrates Teresa' comments.
Ok for checking in?
-Rong
On Thu, Jan 30, 2014 at 1:20 PM, Rong Xu wrote:
> Comments are inlined. New patch attached to this email.
>
> -Rong
>
> On Thu, Jan 30, 2014 at 12:39 PM, Xinliang David Li
> wrote:
>> On Wed, Jan 29, 2014 at 4:24 PM, Rong Xu wrote:
>>> Hi,
>>>
>>> The attached patch adds some new features to gcov-tool. It aims to
>>> rewrite LIPO profile for reuse, debug or performance tuning purpose.
>>>
>>> -l, --modu_list Only use the modules in this file
>>
>> I think in verbose mode, the tool should emit warnings when a module
>> is trimmed due to this option. This can be used in regression test.
>>
>
> In previous patch, this warning is emitted unconventionally (not
> guarded by verbose flag).
> I changed it to under verbose mode in this patch.
>
>>> -r, --string Replace string in path
>>
>> --path_substr_replace or something in that line.
>>
>
> Done.
>
>>> -u, --use_imports_file Use the grouping in import files.
>>>
>>
>> Is there a path in code to auto test this?
>
> As we discussed offline. This can be verified by a separated script.
>
>>
>>> -l uses only the modules in specified file to compute module grouping
>>> (and subsequent dumping).
>>> -r replaces the pattern specified in the argument. The format is:
>>> old_str1:new_str1[,old_str2:new_str2]*, only the first occurrence is
>>> replaced.
>>> -u skips the run-time module grouping computation and reuses the one
>>> comes with the profiles (which is user editable).
>>>
>>> Tested with profiles from google internal benchmarks.
>>>
>>
>> Also use strcasestr for case insenstive operation.
>
> Done.
>
>>
>> David
>>
>>> -Rong
Index: gcc/gcov-tool.c
===
--- gcc/gcov-tool.c (revision 207435)
+++ gcc/gcov-tool.c (working copy)
@@ -29,6 +29,7 @@ see the files COPYING3 and COPYING.RUNTIME respect
#include "coretypes.h"
#include "tm.h"
#include "intl.h"
+#include "hashtab.h"
#include "diagnostic.h"
#include "version.h"
#include "gcov-io.h"
@@ -38,6 +39,7 @@ see the files COPYING3 and COPYING.RUNTIME respect
#include
#include
#include "params.h"
+#include
extern int gcov_profile_merge (struct gcov_info*, struct gcov_info*, int, int);
extern int gcov_profile_normalize (struct gcov_info*, gcov_type);
@@ -47,9 +49,11 @@ extern struct gcov_info* gcov_read_profile_dir (co
extern void gcov_exit (void);
extern void set_gcov_list (struct gcov_info *);
extern void gcov_set_verbose (void);
+extern void set_use_existing_grouping (void);
+extern void set_use_modu_list (void);
+extern void lipo_set_substitute_string (const char *);
-static int verbose;
-
+/* The following defines are needed by dyn-ipa.c. */
gcov_unsigned_t __gcov_lipo_grouping_algorithm;
gcov_unsigned_t __gcov_lipo_merge_modu_edges;
gcov_unsigned_t __gcov_lipo_weak_inclusion;
@@ -60,6 +64,8 @@ gcov_unsigned_t __gcov_lipo_random_seed;
gcov_unsigned_t __gcov_lipo_dump_cgraph;
gcov_unsigned_t __gcov_lipo_propagate_scale;
+static int verbose;
+
/* Remove file NAME if it has a gcda suffix. */
static int
@@ -285,6 +291,189 @@ profile_rewrite (const char *d1, const char *out,
return 0;
}
+/* This is the hashtab entry to store a name and mod_id pair. */
+typedef struct {
+ const char *name;
+ unsigned id;
+} mod_name_id;
+
+/* Hash and comparison functions for strings. */
+
+static unsigned
+mod_name_id_htab_hash (const void *s_p)
+{
+ const char *s = ((const mod_name_id *) s_p)->name;
+ return (*htab_hash_string) (s);
+}
+
+static int
+mod_name_id_hash_eq (const void *s1_p, const void *s2_p)
+{
+ return strcmp (((const mod_name_id *) s1_p)->name,
+ ((const mod_name_id *) s2_p)->name) == 0;
+}
+
+static htab_t mod_name_id_hash_table;
+
+/* Look up an entry in the hash table. STRING is the module name.
+ CREATE controls to insert to htab or not.
+ If (*ID_P != 0), we write (*ID_P) to htab.
+ If (*ID_P == 0), we write module_id to (*ID_P).
+ return 1 if an entry is found and otherwise 0. */
+
+static int
+module_name_hash_lookup (const char *string, unsigned *id_p, int create)
+{
+ void **e;
+ mod_name_id t;
+
+ t.name = string;
+ e = htab_find_slot (mod_name_id_hash_table, &t,
+ create ? INSERT : NO_INSERT);
+ if (e == NULL)
+return 0;
+ if (*e == NULL)
+{
+ *e = XNEW (mod_name_id *);
+ (*(mod_name_id **)e)->name = xstrdup (string);
+}
+ if (id_p)
+{
+ if (*id_p != 0)
+(*(mod_name_id **)e)->id = *id_p;
+ else
+*id_p = (*(mod_name_id **)e)->id;
+}
+ return 1;
+}
+
+/* Return 1 if NAME is of a source type that LIPO targets.
+ Return 0 otherwise. */
+
+static int
+is_lipo_source_type (char *name)
+{
+ char *p;
+
+ if (strcasestr (name, ".c") ||
+ strcasestr (name, ".cc") ||
+ strcasestr (name, ".cpp") ||
+ strcasestr (name, ".c++"))
+retu
Re: [MIPS] Use soft-fp for libgcc floating-point routines
"Joseph S. Myers" writes:
> I note that you're not using t-softfp-excl. Logically, it's best to use
> the libgcc2.c functions in the cases where hardware floating point is
> involved (when they are providing conversions to/from DImode/TImode, and
> to/from unsigned, based on conversions to/from a narrower type or to/from
> signed, and the floating-point type involved is a hardware type), because
> that will be more efficient than a fully software implementation. But
> when building for a soft-float multilib, and in any case when TFmode is
> involved, it's best to use the soft-fp functions rather than the libgcc2.c
> versions.
Thanks, I'd missed that.
> Thus:
>
> * For soft-float, what you have seems optimal.
>
> * For hard-float o32, using t-softfp-excl would be best.
>
> * For hard-float n32 and n64, what's optimal would be a hybrid the
> makefile code doesn't yet support, so would need custom handling in MIPS
> fragments; see above, and the t-softfp comment "This list is taken from
> mklibgcc.in and doesn't presently allow for 64-bit targets where si should
> become di and di should become ti.". How much this matters (and indeed
> how much it matters for hard-float o32) depends on which of the
> conversions involving SFmode / DFmode do actually end up using a libgcc
> function (for which the libgcc2.c version would be better than the soft-fp
> version).
I agree that would work but in the end I went for a slightly different
approach. The main reason was that although libgcc2.c provides those
good di and ti conversion routines you mentioned, it doesn't AFAICT
provide any si routines; they used to come from fpbit.c instead.
So if we're going to provide fast di and ti routines we should probably
try to do the same for si. TBH, I also didn't fancy trying to change
anything as sensitive as that list of exclusions in t-softfp.
As you say, most of the functions with hard-float support should be used
rarely if at all. I think the only reason we ended up providing a full
set of routines for hard-float was because, back in the bad old days when
libgcc was in gcc/, it was very difficult to tailor it to individual
multilibs. That includes the set of exported symbols.
So here I just provide simple C functions for each operation that has
hard-float support. We can then restrict the softfp stuff to functions
that really do need softfp support, meaning that softfp_exclude_libgcc2 := n
should always be OK. As an added bonus it reduces the footprint of libgcc_s.so
on hard-float targets.
Tested on mips64-linux-gnu and mipsisa64-sde-elf. I checked all the
new o32, n32 and n64 functions to make sure that they produced the
expected asm and (in particular) didn't have any recursive calls.
I also rechecked the list of exported symbols in each case.
Does this look OK?
Thanks,
Richard
libgcc/
* configure.ac (libgcc_cv_mips_hard_float): New.
* configure: Regenerate.
* config.host (mips*-*-*): Use t-hardfp-sfdf rather than
t-softfp-sfdf for hard-float targets.
* config/mips/t-mips (LIB2_SIDITI_CONV_FUNCS): Reinstate.
(softfp_float_modes, softfp_int_modes, softfp_extensions)
(softfp_truncations, softfp_exclude_libgcc2): New.
* config/mips/t-hardfp-sfdf: New file.
* config/mips/hardfp.c: Likewise.
Index: libgcc/configure.ac
===
--- libgcc/configure.ac 2014-02-04 18:11:37.078922650 +
+++ libgcc/configure.ac 2014-02-04 18:11:41.534950708 +
@@ -292,6 +292,18 @@ EOF
eval `${CC-cc} -E conftest.c | grep host_address=`
rm -f conftest.c
+case ${host} in
+mips*-*-*)
+ AC_CACHE_CHECK([whether the target is hard-float],
+[libgcc_cv_mips_hard_float],
+[AC_COMPILE_IFELSE(
+[#ifndef __mips_hard_float
+ #error FOO
+ #endif],
+[libgcc_cv_mips_hard_float=yes],
+[libgcc_cv_mips_hard_float=no])])
+esac
+
# Collect host-machine-specific information.
. ${srcdir}/config.host
Index: libgcc/config.host
===
--- libgcc/config.host 2014-02-04 18:11:37.077922644 +
+++ libgcc/config.host 2014-02-04 18:11:41.434950078 +
@@ -142,7 +142,12 @@ microblaze*-*-*)
mips*-*-*)
# All MIPS targets provide a full set of FP routines.
cpu_type=mips
- tmake_file="mips/t-mips t-softfp-sfdf"
+ tmake_file="mips/t-mips"
+ if test "${libgcc_cv_mips_hard_float}" = yes; then
+ tmake_file="${tmake_file} mips/t-hardfp-sfdf"
+ else
+ tmake_file="${tmake_file} t-softfp-sfdf"
+ fi
if test "${ac_cv_sizeof_long_double}" = 16; then
tmake_file="${tmake_file} mips/t-softfp-tf"
fi
Index: libgcc/config/mips/t-mips
===
--- libgcc/config/mips/t-mips 2014-02-04 18:11:37.077922644 +
+++ libgcc/config/mips/t-mips 20
Re: var-tracking and s390's LM(G)
Jakub Jelinek writes: > On Tue, Feb 04, 2014 at 04:59:54PM +, Richard Sandiford wrote: >> Right, that was my point. So by relying on unwind info in var-tracking.c >> we're getting the wrong answer for the stack offset after the LM(G) >> instruction. It sounds like we're going to pretend it's right anyway >> for expediency. But in that case why not also allow the exit block >> offsets to vary on the same basis? It's not like the current test is >> an assert -- it's just silently refusing to do any var-tracking on the >> function if the epilogue isn't fully understood. >> >> If these offsets must match on exit for correctness then we should >> assert rather than silently bailing out. But if they're allowed to vary >> then we should do our best to carry on. And since the exit block offset >> is never used, it seems expedient to allow varying offsets in that case. > > Ok then, but please do follow-up on the changes to not save/restore > r15 in the unwind info, use REG_CFA_ADJUST_CFA where possible and teach > var-tracking about that note. Thanks, will do. I know Uli was worried about the CFI size too. richard
Contents of PO file 'cpplib-4.9-b20140202.eo.po'
cpplib-4.9-b20140202.eo.po.gz Description: Binary data The Translation Project robot, in the name of your translation coordinator.
New Esperanto PO file for 'cpplib' (version 4.9-b20140202)
Hello, gentle maintainer. This is a message from the Translation Project robot. A revised PO file for textual domain 'cpplib' has been submitted by the Esperanto team of translators. The file is available at: http://translationproject.org/latest/cpplib/eo.po (This file, 'cpplib-4.9-b20140202.eo.po', has just now been sent to you in a separate email.) All other PO files for your package are available in: http://translationproject.org/latest/cpplib/ Please consider including all of these in your next release, whether official or a pretest. Whenever you have a new distribution with a new version number ready, containing a newer POT file, please send the URL of that distribution tarball to the address below. The tarball may be just a pretest or a snapshot, it does not even have to compile. It is just used by the translators when they need some extra translation context. The following HTML page has been updated: http://translationproject.org/domain/cpplib.html If any question arises, please contact the translation coordinator. Thank you for all your work, The Translation Project robot, in the name of your translation coordinator.
Re: RFA: MN10300: Fix building libstdc++-v3
Hi Jeff, But how/why did an extended register get selected to begin with? It happens in reload.c:find_valid_class(). HARD_REGNO_MODE_OK and other macros/hooks ought to be preventing this relatively early. Ah - and that is the problem. mn10300_hard_regno_mode_ok is allowing extended registers to match some classes even in MN10300. So how about the patch below ? This also allows the libstdc++-v3 library to be built. Cheers Nick gcc/ChangeLog 2014-02-04 Nick Clifton * config/mn10300/mn10300.c (mn10300_hard_regno_mode_ok): Do not accept extended registers in any mode when compiling for the MN10300. Index: gcc/config/mn10300/mn10300.c === --- gcc/config/mn10300/mn10300.c(revision 207452) +++ gcc/config/mn10300/mn10300.c(working copy) @@ -2614,7 +2612,10 @@ || REGNO_REG_CLASS (regno) == FP_ACC_REGS) /* Do not store integer values in FP registers. */ return GET_MODE_CLASS (mode) == MODE_FLOAT && ((regno & 1) == 0); - + + if (! TARGET_AM33 && REGNO_REG_CLASS (regno) == EXTENDED_REGS) +return false; + if (((regno) & 1) == 0 || GET_MODE_SIZE (mode) == 4) return true;
Re: Fix bootstrap with -mno-accumulate-outgoing-args
On 02/04/2014 09:35 AM, Jan Hubicka wrote: > I am not terribly familiar with the code, will you look into it or shall I > give it a try? I'm looking at it. Was there a PR for the Go bootstrap failure? r~
Re: Fix bootstrap with -mno-accumulate-outgoing-args
> On 02/04/2014 08:48 AM, Jan Hubicka wrote: > > How things are supposed to work in this case? So perhaps we need scheduler > > to > > understand and move around the ARGS_SIZE note? > > I believe we do need to have the scheduler move the notes around. If we need notes on non-stack adjusting insns as you seem to show in your testcase, I guess it is the only way around. Still combine stack adjust may be smart enough to not produce redundant note in the case of go's ICE. I am not terribly familiar with the code, will you look into it or shall I give it a try? Thanks! Honza > > > r~
Re: var-tracking and s390's LM(G)
On Tue, Feb 04, 2014 at 04:59:54PM +, Richard Sandiford wrote: > Right, that was my point. So by relying on unwind info in var-tracking.c > we're getting the wrong answer for the stack offset after the LM(G) > instruction. It sounds like we're going to pretend it's right anyway > for expediency. But in that case why not also allow the exit block > offsets to vary on the same basis? It's not like the current test is > an assert -- it's just silently refusing to do any var-tracking on the > function if the epilogue isn't fully understood. > > If these offsets must match on exit for correctness then we should > assert rather than silently bailing out. But if they're allowed to vary > then we should do our best to carry on. And since the exit block offset > is never used, it seems expedient to allow varying offsets in that case. Ok then, but please do follow-up on the changes to not save/restore r15 in the unwind info, use REG_CFA_ADJUST_CFA where possible and teach var-tracking about that note. Jakub
Re: [PATCH] Don't ICE on non-initialized odr_hash during gimple folding (PR ipa/59947)
> Hi! > > On this testcase the TU is -O0, but a particular function is -O2. > The -O0 means odr_hash isn't created, but during fold_gimple_call > we call possible_polymorphic_call_targets because -fdevirtualize > is true in the current function (as it is -O2) and ICE because > we assume odr_hash can be used safely. > > Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux, > ok for trunk? OK, thank you! Honza
Re: PR ipa/59831 (ipa-cp devirt issues)
Hi,
I went ahead and comitted Markus' patch. I updated the testcase to use hidden
visibility.
With default visibility the gimple-fold change will enable devirtualization.
Honza
Index: ChangeLog
===
--- ChangeLog (revision 207477)
+++ ChangeLog (working copy)
@@ -1,3 +1,9 @@
+2014-02-04 Markus Trippelsdorf
+
+ PR ipa/60058
+ * ipa-cp.c (ipa_get_indirect_edge_target_1): Check that target
+ is non-null.
+
2014-02-04 Jan Hubicka
* gimple-fold.c (can_refer_decl_in_current_unit_p): Default visibility
is safe.
Index: ipa-cp.c
===
--- ipa-cp.c(revision 207451)
+++ ipa-cp.c(working copy)
@@ -1573,20 +1573,23 @@ ipa_get_indirect_edge_target_1 (struct c
{
target = gimple_get_virt_method_for_vtable
(ie->indirect_info->otr_token,
vtable, offset);
- if ((TREE_CODE (TREE_TYPE (target)) == FUNCTION_TYPE
- && DECL_FUNCTION_CODE (target) == BUILT_IN_UNREACHABLE)
- || !possible_polymorphic_call_target_p
- (ie, cgraph_get_node (target)))
+ if (target)
{
- if (dump_file)
- fprintf (dump_file,
-"Type inconsident devirtualization: %s/%i->%s\n",
-ie->caller->name (), ie->caller->order,
-IDENTIFIER_POINTER (DECL_ASSEMBLER_NAME (target)));
- target = builtin_decl_implicit (BUILT_IN_UNREACHABLE);
- cgraph_get_create_node (target);
+ if ((TREE_CODE (TREE_TYPE (target)) == FUNCTION_TYPE
+ && DECL_FUNCTION_CODE (target) == BUILT_IN_UNREACHABLE)
+ || !possible_polymorphic_call_target_p
+ (ie, cgraph_get_node (target)))
+ {
+ if (dump_file)
+ fprintf (dump_file,
+"Type inconsident devirtualization: %s/%i->%s\n",
+ie->caller->name (), ie->caller->order,
+IDENTIFIER_POINTER (DECL_ASSEMBLER_NAME (target)));
+ target = builtin_decl_implicit (BUILT_IN_UNREACHABLE);
+ cgraph_get_create_node (target);
+ }
+ return target;
}
- return target;
}
}
Index: testsuite/ChangeLog
===
--- testsuite/ChangeLog (revision 207451)
+++ testsuite/ChangeLog (working copy)
@@ -1,3 +1,8 @@
+2014-02-04 Markus Trippelsdorf
+
+ PR ipa/60058
+ * g++.dg/torture/pr60058.C: New testcase.
+
2014-02-03 Jan Hubicka
PR ipa/59882
Index: testsuite/g++.dg/torture/pr60058.C
===
--- testsuite/g++.dg/torture/pr60058.C (revision 0)
+++ testsuite/g++.dg/torture/pr60058.C (revision 0)
@@ -0,0 +1,30 @@
+/* { dg-do compile } */
+/* { dg-require-visibility "" } */
+
+typedef enum {} UErrorCode;
+class A {
+public:
+ virtual A &m_fn1(A &, const A &, UErrorCode &) const;
+ void m_fn2();
+ A();
+ A(int);
+};
+class __attribute__((visibility("hidden"))) B : public A {
+public:
+ B(A &p1) : norm2(p1), set(0) {}
+ A &m_fn1(A &, const A &, UErrorCode &) const;
+ A &norm2;
+ const int &set;
+};
+
+UErrorCode a;
+A c;
+void fn1(A *p1) {
+ A b;
+ p1->m_fn1(b, 0, a).m_fn2();
+}
+
+void fn2() {
+ B d(c);
+ fn1(&d);
+}
Re: [C PATCH] Get rid of spurious warning (PR c/60036)
On Tue, 4 Feb 2014, Marek Polacek wrote: > Starting with revision 206620, if we have LHS op= RHS and RHS has > side effects, we preevaluate RHS first, which means that we wrap it > in C_MAYBE_CONST_EXPR_EXPR + SAVE_EXPR. This regressed -Wconversion, > since we now issue bogus warnings. The reason is that conversion_warning > doesn't grok C_MAYBE_CONST_EXPR_EXPR/SAVE_EXPR, meaning that it > just calls unsafe_conversion_p on it that warns. But for e.g. relational > and equality operators that's useless as those always yield 0 or 1. > This patch fixes this by unwrapping the expr first. > > Regtested/bootstrapped on x86_64-linux, ok for trunk? OK. -- Joseph S. Myers jos...@codesourcery.com
Re: PR ipa/59831 (ipa-cp devirt issues)
> Hi, > > On 02/04/2014 06:43 AM, Jan Hubicka wrote: > >Hi, > >this patch solves the actual ICE in PR59831 by using ipa-devirt instead of > >gimple_extract_devirt_binfo_from_cst as discussed in the first post. > > > >Honza > > > > PR ipa/59831 > > * ipa-cp.c (ipa_get_indirect_edge_target_1): Use ipa-devirt > > to figure out targets of polymorphic calls with known decl. > > * ipa-prop.c (try_make_edge_direct_virtual_call): Likewise. > > * ipa-utils.h (get_polymorphic_call_info_from_invariant): Declare. > > * ipa-devirt.c (get_polymorphic_call_info_for_decl): Break out from ... > > (get_polymorphic_call_info): ... here. > > (get_polymorphic_call_info_from_invariant): New function. > > > > * g++.dg/ipa/devirt-22.C: New testcase. > Today I'm seeing regressions in the libstdc++-v3 testsuite in the > form of ICEs exactly in ipa_get_indirect_edge_target_1: Ah, sorry, I had the can_refer_to_current_unit_p patch in my tree too and at last minute I decided to split those two changes. I will add check to ipa_get_indirect_edge_target_1 to expect target to be NULLnow and als commit the change now approved by Richard. Honza > > FAIL: 27_io/basic_stringbuf/sputbackc/char/9425.cc (test for excess errors) > WARNING: 27_io/basic_stringbuf/sputbackc/char/9425.cc compilation > failed to produce executable > FAIL: 27_io/basic_stringbuf/sputbackc/wchar_t/9425.cc (test for > excess errors) > WARNING: 27_io/basic_stringbuf/sputbackc/wchar_t/9425.cc compilation > failed to produce executable > FAIL: 27_io/basic_stringbuf/str/char/2.cc (test for excess errors) > WARNING: 27_io/basic_stringbuf/str/char/2.cc compilation failed to > produce executable > FAIL: 27_io/basic_stringbuf/str/wchar_t/2.cc (test for excess errors) > WARNING: 27_io/basic_stringbuf/str/wchar_t/2.cc compilation failed > to produce executable > > Can you please have a look? > > Thanks, > Paolo.
Re: Fix bootstrap with -mno-accumulate-outgoing-args
On 02/04/2014 08:48 AM, Jan Hubicka wrote: > How things are supposed to work in this case? So perhaps we need scheduler to > understand and move around the ARGS_SIZE note? I believe we do need to have the scheduler move the notes around. r~
Re: [PATCH] Clear DECL_LANG_SPECIFIC after changing arguments (PR tree-optimization/60002)
> Hi! > > As described in the PR, while build_function_decl_skip_args > takes care of changing the type from METHOD_TYPE to FUNCTION_TYPE > if removing first argument (this), the FE may have other special arguments > that it is aware of (such as in-charge etc.) and if the this argument > is not removed, but the other arguments are, then the FE can crash > upon seeing such methods. > > Fixed by clearing DECL_LANG_SPECIFIC if we remove any arguments. > > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? > > 2014-02-04 Jakub Jelinek > > PR tree-optimization/60002 > * cgraphclones.c (build_function_decl_skip_args): Clear > DECL_LANG_SPECIFIC. > > * g++.dg/opt/pr60002.C: New test. Seems OK to me. So we basically care to not crash from langhooks, right? Naturally, those should go away ;) Honza
[C PATCH] Get rid of spurious warning (PR c/60036)
Starting with revision 206620, if we have LHS op= RHS and RHS has
side effects, we preevaluate RHS first, which means that we wrap it
in C_MAYBE_CONST_EXPR_EXPR + SAVE_EXPR. This regressed -Wconversion,
since we now issue bogus warnings. The reason is that conversion_warning
doesn't grok C_MAYBE_CONST_EXPR_EXPR/SAVE_EXPR, meaning that it
just calls unsafe_conversion_p on it that warns. But for e.g. relational
and equality operators that's useless as those always yield 0 or 1.
This patch fixes this by unwrapping the expr first.
Regtested/bootstrapped on x86_64-linux, ok for trunk?
2014-02-04 Marek Polacek
PR c/60036
c-family/
* c-common.c (conversion_warning): Unwrap C_MAYBE_CONST_EXPR and/or
SAVE_EXPR.
testsuite/
* gcc.dg/pr60036.c: New test.
diff --git gcc/c-family/c-common.c gcc/c-family/c-common.c
index fc12788..007e727 100644
--- gcc/c-family/c-common.c
+++ gcc/c-family/c-common.c
@@ -2714,6 +2714,14 @@ conversion_warning (location_t loc, tree type, tree expr)
if (!warn_conversion && !warn_sign_conversion && !warn_float_conversion)
return;
+ /* This may happen, because for LHS op= RHS we preevaluate
+ RHS and create C_MAYBE_CONST_EXPR >, which
+ means we could no longer see the code of the EXPR. */
+ if (TREE_CODE (expr) == C_MAYBE_CONST_EXPR)
+expr = C_MAYBE_CONST_EXPR_EXPR (expr);
+ if (TREE_CODE (expr) == SAVE_EXPR)
+expr = TREE_OPERAND (expr, 0);
+
switch (TREE_CODE (expr))
{
case EQ_EXPR:
diff --git gcc/testsuite/gcc.dg/pr60036.c gcc/testsuite/gcc.dg/pr60036.c
index e69de29..07eb6ac 100644
--- gcc/testsuite/gcc.dg/pr60036.c
+++ gcc/testsuite/gcc.dg/pr60036.c
@@ -0,0 +1,28 @@
+/* PR c/60036 */
+/* { dg-do compile } */
+/* { dg-options "-Wconversion" } */
+
+extern int fn (void);
+
+void
+foo (int i)
+{
+ unsigned int f = 9;
+
+ /* Don't warn on these. */
+ f += fn () || i;
+ f += fn () && i;
+ f += ! fn ();
+ f -= fn () == i;
+ f |= fn () != i;
+ f &= fn () < i;
+ f ^= fn () > i;
+ f &= fn () <= i;
+ f ^= fn () >= i;
+
+ /* But warn on the following. */
+ f += fn (); /* { dg-warning "conversion" } */
+ f += fn () | i; /* { dg-warning "conversion" } */
+ f += fn () & i; /* { dg-warning "conversion" } */
+ f += fn () ^ i; /* { dg-warning "conversion" } */
+}
Marek
Re: var-tracking and s390's LM(G)
Jakub Jelinek writes: > On Tue, Feb 04, 2014 at 03:38:51PM +, Richard Sandiford wrote: >> What do you want var-tracking to do about the __builtin_eh_return case >> though? It isn't correct to say that the instruction adds the frame size >> to the stack pointer then, since the loaded value includes the eh_return >> adjustment. Pretending it is a constant addition in order to satisfy the >> sanity check seems a bit hackish, but there again it'd be a shame to lose >> tracking for the whole function just because of that. > > For __builtin_eh_return the unwind info starting with restoring the stack > pointer to the landing pad's stack pointer until you actually return/jump to > the > landing pad is wrong in either case, not just on s390, but on various other > targets. Saying REG_CFA_ADJUST_CFA by the frame size will result in CFA > from that point being stack_pointer + 160, which is exactly what you are > doing anyway, the DW_CFA_offset 15, ... and DW_CFA_restore aren't helpful to > that at all and don't change anything on it. > > Unless you have the old value of the stack pointer saved in some hard > register, providing correct unwind info there is not possible. Right, that was my point. So by relying on unwind info in var-tracking.c we're getting the wrong answer for the stack offset after the LM(G) instruction. It sounds like we're going to pretend it's right anyway for expediency. But in that case why not also allow the exit block offsets to vary on the same basis? It's not like the current test is an assert -- it's just silently refusing to do any var-tracking on the function if the epilogue isn't fully understood. If these offsets must match on exit for correctness then we should assert rather than silently bailing out. But if they're allowed to vary then we should do our best to carry on. And since the exit block offset is never used, it seems expedient to allow varying offsets in that case. Thanks, Richard
PATCH: Compile g++.dg/opt/pr52727.C with -march=i686 for ia32
Hi,
Revision 206943 caused a regression on g++.dg/opt/pr52727.C, which wasn't
caught by my automated tester since SSE2 and SSE math are enabled by
default. This patch adds -march=i686 for ia32 when compiling
g++.dg/opt/pr52727.C, which is required to reproduce the bug. OK to
install?
Thanks.
H.J.
---
2014-02-04 H.J. Lu
* g++.dg/opt/pr52727.C: Compile with -march=i686 for ia32.
diff --git a/gcc/testsuite/g++.dg/opt/pr52727.C
b/gcc/testsuite/g++.dg/opt/pr52727.C
index 4dd3853..ed8b973 100644
--- a/gcc/testsuite/g++.dg/opt/pr52727.C
+++ b/gcc/testsuite/g++.dg/opt/pr52727.C
@@ -1,5 +1,6 @@
// { dg-do compile }
// { dg-options "-g -Os" }
+// { dg-additional-options "-march=i686" { target ia32 } }
int grow (int);
void fn (int);
Re: Fix bootstrap with -mno-accumulate-outgoing-args
> On 01/01/2014 06:23 AM, Jan Hubicka wrote: > > last_sp_adjust = 0; > > + /* We no longer adjust stack size. Whoever adjusted it earlier > > +hopefully got the note right. */ > > + note = find_reg_note (insn, REG_ARGS_SIZE, NULL_RTX); > > + if (note) > > + remove_note (insn, note); > > continue; > > This is a totally unfounded hope, by the way. I'm certain that I tried this Sorry to hear that - that assumption seemed right in the testcase I loked into. > the first time around, with similarly poor results. > > See the g++.dg/opt/pr52727.C test case that fails with this patch. I can only > assume you didn't examine i686 test results when you committed this? > > The problem is that the last adjustment _did_ apply the correct note > adjustment, for the stack at that time. But neither moving the note nor > dropping the note is correct. > > Consider > > // args_size 4 > add $4, %esp// args_size 0 > push%eax// no note; fpu, not call related > > transformed by csa to > > mov %eax, (%esp) > > If we move the note from the add to the mov, as we did prior to your patch, we > retain the essence of the pop, that we're no longer queueing arguments. But > if > we drop the note, as we do with your patch, then we still believe that we have > 4 bytes of arguments on the stack. > > You say a bootstrap with Go was the testcase that sent you in this direction? > I'll have a look... Yes, if you remove the patch you should get ICE during go x86_64 bootstrap with generic tuning. I tried to describe the situation in original mail and the PR - the sequence was same as you describe here, but comming from two consecutive calls, so args_size was 8, then reduced to 0 by add and then increased to 8 by push. I see in the testcase the psh comes from splitter - that is something I did not consider indded. I am sorry for that. How things are supposed to work in this case? So perhaps we need scheduler to understand and move around the ARGS_SIZE note? Honza
[PATCH, PR 55260] Use correct info in cgraph_edge_brings_all_agg_vals_for_node
Hi,
PR 55260 has been reopened because there is still one use of wrong
info, this time in cgraph_edge_brings_all_agg_vals_for_node when
checking whether aggregate lattices are set to bottom which can lead
to index out of bounds assert failures or even potentially ignoring
bottom lattices.
Fixed thusly. Bootstrapped and tested on trunk on x86_64-linux,
bootstrap is still underway on the 4.8 branch where this exact fix is
also necessary. OK for both?
Thanks,
Martin
2014-02-03 Martin Jambor
PR ipa/55260
* ipa-cp.c (cgraph_edge_brings_all_agg_vals_for_node): Uce correct
info when checking whether lattices are bottom.
testsuite/
* gcc.dg/ipa/pr55260.c: New test.
diff --git a/gcc/ipa-cp.c b/gcc/ipa-cp.c
index 10fa4b6..70bb33f 100644
--- a/gcc/ipa-cp.c
+++ b/gcc/ipa-cp.c
@@ -3178,6 +3178,7 @@ cgraph_edge_brings_all_agg_vals_for_node (struct
cgraph_edge *cs,
struct cgraph_node *node)
{
struct ipa_node_params *orig_caller_info = IPA_NODE_REF (cs->caller);
+ struct ipa_node_params *orig_node_info;
struct ipa_agg_replacement_value *aggval;
int i, ec, count;
@@ -3192,6 +3193,7 @@ cgraph_edge_brings_all_agg_vals_for_node (struct
cgraph_edge *cs,
if (aggval->index >= ec)
return false;
+ orig_node_info = IPA_NODE_REF (IPA_NODE_REF (node)->ipcp_orig_node);
if (orig_caller_info->ipcp_orig_node)
orig_caller_info = IPA_NODE_REF (orig_caller_info->ipcp_orig_node);
@@ -3209,7 +3211,7 @@ cgraph_edge_brings_all_agg_vals_for_node (struct
cgraph_edge *cs,
if (!interesting)
continue;
- plats = ipa_get_parm_lattices (orig_caller_info, aggval->index);
+ plats = ipa_get_parm_lattices (orig_node_info, aggval->index);
if (plats->aggs_bottom)
return false;
diff --git a/gcc/testsuite/gcc.dg/ipa/pr55260.c
b/gcc/testsuite/gcc.dg/ipa/pr55260.c
new file mode 100644
index 000..ef151b0
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/ipa/pr55260.c
@@ -0,0 +1,38 @@
+/* { dg-do compile } */
+/* { dg-options "-O2 -fno-inline -fipa-cp-clone" } */
+
+typedef struct {
+ int *ptr;
+ int len;
+} string;
+typedef struct {
+ string nantstr;
+ int *nant;
+} malv;
+typedef struct {
+ int *nor;
+} list_heads;
+int b;
+list_heads *fn1(string, int *, unsigned);
+void fn2(malv *p1, list_heads *p2, unsigned p3) {
+ string a = p1->nantstr;
+ fn1(a, p1->nant, p3);
+}
+
+void fn3(unsigned p1) { fn2(0, 0, p1); }
+
+list_heads *fn1(string p1, int *p2, unsigned p3) {
+ while (1) {
+if (p3)
+ fn3(1);
+if (b)
+ return 0;
+fn3(1);
+ }
+}
+
+void fn5() {
+ list_heads c;
+ c.nor = 0;
+ fn2(0, &c, 1);
+}
[jit] Improvements to array access (API change and additions)
Committed to branch dmalcolm/jit:
The following commit adds a way to create an array type from an existing
type:
/* Given type "T", get type "T[N]" (for a constant N). */
extern gcc_jit_type *
gcc_jit_context_new_array_type (gcc_jit_context *ctxt,
gcc_jit_location *loc,
gcc_jit_type *element_type,
int num_elements);
and renames the array access API from:
gcc_jit_context_new_array_lookup
to:
gcc_jit_context_new_array_access
changing it to return an lvalue rather than an rvalue.
It also adds a helper function:
extern gcc_jit_loop *
gcc_jit_function_new_loop_over_range (gcc_jit_function *func,
gcc_jit_location *loc,
gcc_jit_lvalue *iteration_var,
gcc_jit_rvalue *start_of_range,
gcc_jit_rvalue *upper_bound_of_range,
gcc_jit_rvalue *step);
for creating loops of this form more easily:
for (i = start_of_range; i < upper_bound_of_range; i += step)
{
BODY;
}
"start_of_range" can be NULL, in which case 0 is used.
"upper_bound_of_range" must be non-NULL.
"step" can be NULL, in which case 1 is used.
gcc/jit/
* libgccjit.h (gcc_jit_context_new_array_type): New.
(gcc_jit_context_new_array_lookup): Rename to...
(gcc_jit_context_new_array_access): ...this, and change return
type from rvalue to lvalue.
(gcc_jit_function_new_loop_over_range): New.
* libgccjit.map (gcc_jit_context_new_array_lookup): Rename to...
(gcc_jit_context_new_array_access): ..this.
(gcc_jit_context_new_array_type): New.
(gcc_jit_function_new_loop_over_range): New.
* libgccjit++.h (gccjit::context::new_array_type): New methods.
(gccjit::context::new_array_access): New methods.
* libgccjit.c (gcc_jit_context_new_array_type): New.
(gcc_jit_context_new_array_lookup): Rename to...
(gcc_jit_context_new_array_access): ...this, and change return
type from rvalue to lvalue.
(gcc_jit_function_new_loop): Provide NULL for the added arguments
for the underlying API.
(gcc_jit_function_new_loop_over_range): New.
* internal-api.h (gcc::jit::recording::context::
new_array_type): New.
(gcc::jit::recording::context::new_array_lookup): Rename to...
(gcc::jit::recording::context::new_array_access): ...this, and
change return type from an rvalue to an lvalue.
(gcc::jit::recording::array_type): New subclass of type.
(gcc::jit::recording::function::new_loop): Add iteration_var and
step params, so that we can add a += when the loop ends.
(gcc::jit::recording::array_lookup): Rename to...
(gcc::jit::recording::array_access): ...this, and subclass from
lvalue rathern than rvalue.
(gcc::jit::recording::loop): Add optional iteration_var and
step fields, for use by gcc_jit_function_new_loop_over_range so
that, if present, we can add a += when the loop ends.
(gcc::jit::playback::context::new_array_type): New.
(gcc::jit::playback::context::new_array_lookup): Rename to...
(gcc::jit::playback::context::new_array_access): ...this, and
change return type from an rvalue to an lvalue.
* internal-api.c (gcc::jit::recording::context::new_array_type): New.
(gcc::jit::recording::context::new_array_lookup): Rename to...
(gcc::jit::recording::context::new_array_access): ...this, and
change return type from an rvalue to an lvalue.
(gcc::jit::recording::array_type::dereference): New.
(gcc::jit::recording::array_type::replay_into): New.
(gcc::jit::recording::array_type::make_debug_string): New.
(gcc::jit::recording::function::new_loop): Add iteration_var and
step params.
(gcc::jit::recording::array_lookup): Rename to...
(gcc::jit::recording::array_access): ...this.
(gcc::jit::recording::loop::end): If we have an iteration_var, and
thus a step, inject a += operation immediately before the loop
end.
(gcc::jit::playback::context::new_array_type): New.
(gcc::jit::playback::context::new_array_lookup): Rename to...
(gcc::jit::playback::context::new_array_access): ...this, and
change return type from an rvalue to an lvalue.
gcc/testsuite/
* jit.dg/test-arrays.c: New test case, verifying that array types
work as expected (and using the the new
gcc_jit_function_new_loop_over_range API call).
* jit.dg/test-dot-product.c (create_code): Update for API changes to
array access.
---
gcc/jit/internal-api.c | 90 +---
gcc/jit/internal-api.h
Re: Properly merge forced_by_abi when merging nodes
> On Tue, Feb 4, 2014 at 6:52 AM, Jan Hubicka wrote:
> > Hi,
> > while merging nodes in lto-symtab, we need to copy the force_output and
> > forced_by_abi flags.
> > Thanks to Markus who noticed the issue.
>
> Don't we need to choose the forced_by_abi variant if there are variants
> without that flag? Do we maybe even need to warn about such mismatches?
I think it is ok to choose any variant - just as linked would do. In the
testcase these are all COMDAT. One is explicitely instantiated and the bug is
that we privatized the comdat to the DSO assuming that all users will make
their own comdats.
LLVM has two variants of the header - one with implementation in it and one
without and it assumes that because one of DSOs explicitely instantiate it,
the others may or may not use the other variant.
I am not 100% sure it is valid C++, but this way it should work.
Honza```
>
> Richard.
>
> > Bootstrapped/regtested x86_64-linux, comitted.
> >
> > Honza
> >
> > Index: ChangeLog
> > ===
> > --- ChangeLog (revision 207438)
> > +++ ChangeLog (working copy)
> > @@ -1,3 +1,9 @@
> > +2014-02-04 Jan Hubicka
> > + Markus Trippelsdorf
> > +
> > + * lto-symtab.c (lto_cgraph_replace_node, lto_varpool_replace_node):
> > + merge force_output and forced_by_abi flags.
> > +
> > 2014-01-24 Balaji V. Iyer
> >
> > * lto-lang.c (lto_init): Replaced flag_enable_cilkplus with
> > Index: lto-symtab.c
> > ===
> > --- lto-symtab.c(revision 207438)
> > +++ lto-symtab.c(working copy)
> > @@ -59,6 +59,8 @@ lto_cgraph_replace_node (struct cgraph_n
> >/* Merge node flags. */
> >if (node->force_output)
> > cgraph_mark_force_output_node (prevailing_node);
> > + if (node->forced_by_abi)
> > +prevailing_node->forced_by_abi = true;
> >if (node->address_taken)
> > {
> >gcc_assert (!prevailing_node->global.inlined_to);
> > @@ -110,6 +112,10 @@ lto_varpool_replace_node (varpool_node *
> >gcc_assert (!vnode->analyzed || prevailing_node->analyzed);
> >
> >ipa_clone_referring (prevailing_node, &vnode->ref_list);
> > + if (vnode->force_output)
> > +prevailing_node->force_output = true;
> > + if (vnode->forced_by_abi)
> > +prevailing_node->forced_by_abi = true;
> >
> >/* Be sure we can garbage collect the initializer. */
> >if (DECL_INITIAL (vnode->decl)
Re: [C++ PATCH] Restore declarator_obstack in cp_parser_omp_declare_reduction (PR c++/58703)
How about putting the fail label at the end of the function so the cleanup code is only in one place? OK either way. Jason
Re: [RFC] [PATCH, AARCH64] : Using standard patterns for stack protection.
Hi Venkat,
On 22 January 2014 16:57, Venkataramanan Kumar
wrote:
> Hi Marcus,
>
> After we changed the frame growing direction (downwards) in Aarch64,
> the back-end now generates stack smashing set and test based on
> generic code available in GCC.
>
> But most of the ports (i386, spu, rs6000, s390, sh, sparc, tilepro and
> tilegx) define machine descriptions using standard pattern names
> 'stack_protect_set' and 'stack_protect_test'. This is done for both
> TLS model as well as global variable based stack guard model.
+ ""
+ "ldr\\t%x2, %1\;str\\t%x2, %0\;mov\t%x2,0"
+ [(set_attr "length" "12")])
This pattern emits an opaque sequence of instructions that cannot be
scheduled, is that necessary? Can we not expand individual
instructions or at least split ?
+ "ldr\t%x3, %x1\;ldr\t%x0, %x2\;eor\t%x0, %x3, %x0"
+ [(set_attr "length" "12")])
Likewise.
-/* { dg-do compile { target i?86-*-* x86_64-*-* rs6000-*-* s390x-*-* } } */
+/* { dg-do compile { target stack_protection } } */
/* { dg-options "-O2 -fstack-protector-strong" } */
Do we need a new effective target test, why is the existing
"fstack_protector" not appropriate?
Cheers
/Marcus
Re: Fix bootstrap with -mno-accumulate-outgoing-args
On 01/01/2014 06:23 AM, Jan Hubicka wrote: > last_sp_adjust = 0; > + /* We no longer adjust stack size. Whoever adjusted it earlier > + hopefully got the note right. */ > + note = find_reg_note (insn, REG_ARGS_SIZE, NULL_RTX); > + if (note) > + remove_note (insn, note); > continue; This is a totally unfounded hope, by the way. I'm certain that I tried this the first time around, with similarly poor results. See the g++.dg/opt/pr52727.C test case that fails with this patch. I can only assume you didn't examine i686 test results when you committed this? The problem is that the last adjustment _did_ apply the correct note adjustment, for the stack at that time. But neither moving the note nor dropping the note is correct. Consider // args_size 4 add $4, %esp// args_size 0 push%eax// no note; fpu, not call related transformed by csa to mov %eax, (%esp) If we move the note from the add to the mov, as we did prior to your patch, we retain the essence of the pop, that we're no longer queueing arguments. But if we drop the note, as we do with your patch, then we still believe that we have 4 bytes of arguments on the stack. You say a bootstrap with Go was the testcase that sent you in this direction? I'll have a look... r~
Re: var-tracking and s390's LM(G)
On Tue, Feb 04, 2014 at 03:38:51PM +, Richard Sandiford wrote: > What do you want var-tracking to do about the __builtin_eh_return case > though? It isn't correct to say that the instruction adds the frame size > to the stack pointer then, since the loaded value includes the eh_return > adjustment. Pretending it is a constant addition in order to satisfy the > sanity check seems a bit hackish, but there again it'd be a shame to lose > tracking for the whole function just because of that. For __builtin_eh_return the unwind info starting with restoring the stack pointer to the landing pad's stack pointer until you actually return/jump to the landing pad is wrong in either case, not just on s390, but on various other targets. Saying REG_CFA_ADJUST_CFA by the frame size will result in CFA from that point being stack_pointer + 160, which is exactly what you are doing anyway, the DW_CFA_offset 15, ... and DW_CFA_restore aren't helpful to that at all and don't change anything on it. Unless you have the old value of the stack pointer saved in some hard register, providing correct unwind info there is not possible. Jakub
[PING] [PATCH] longlong.h: Add prototype for udiv_w_sdiv
On 29/01/14 11:19, Andreas Krebbel wrote:
> Hi,
>
> the attached patch adds a prototype for __udiv_w_sdiv to longlong.h if
> needed. This fixes tons of build warnings on s390 32 bit in glibc.
>
> Ok?
>
> Bye,
>
> -Andreas-
>
>
> 2014-01-29 Andreas Krebbel
>
> * longlong.h: Add __udiv_w_sdiv prototype.
>
> diff --git a/include/longlong.h b/include/longlong.h
> index 5f00e54..053a81d 100644
> --- a/include/longlong.h
> +++ b/include/longlong.h
> @@ -1671,7 +1671,8 @@ extern UHItype __stormy16_count_leading_zeros (UHItype);
> #if !defined (udiv_qrnnd) && defined (sdiv_qrnnd)
> #define udiv_qrnnd(q, r, nh, nl, d) \
>do {
> \
> -USItype __r; \
> +extern UWtype __udiv_w_sdiv (UWtype *, UWtype, UWtype, UWtype); \
> +UWtype __r;
> \
> (q) = __udiv_w_sdiv (&__r, nh, nl, d); \
> (r) = __r;
> \
>} while (0)
>
Re: [ARM][PATCH] Vectorizer generates unaligned access when -mno-unaligned-access is enabled
On 02/03/14 05:41, Kugan wrote: With the 2013-09-21 version of trunk, attached test case results in bus error when simultaneously enabling both -mno-unaligned-access and -ftree-vectorize. The error is caused by unaligned vector load via two vldr instructions: vldr d16, [r3, #-16] vldr d17, [r3, #-8] So, does this mean that we get unaligned vector loads when munaligned-access is on, using vldr instructions ? This worries me because we shouldn't be putting out vldr's. I was hoping to have dug a bit further today but don't think I will make much more progress. Can you dig a bit further to make sure that this is only code path that triggers this ? Cause for this error is, even when -mno-unaligned-access is enabled, backend will inform vectorizer that it supports misaligned accesses via TARGET_VECTORIZE_SUPPORT_VECTOR_MISALIGNMENT. Attached patch fixes this. Is this OK for trunk? How has it been tested ? Ramana Thanks, Kugan +2014-02-03 Yury Gribov +Kugan Vivekanandarajah + + * config/arm/arm.c (arm_vector_alignment_reachable): Check + unaligned_access. + * config/arm/arm.c (arm_builtin_support_vector_misalignment): Likewise. + +2014-02-03 Yury Gribov +Kugan Vivekanandarajah + + * gcc.target/arm/vect-noalign.c: New file. + -- Ramana Radhakrishnan Principal Engineer ARM Ltd.
Re: [build, libgcc] Ensure libgcc_s unwinder is always used on 64-bit Solaris 10+/x86 (PR target/59788)
On Tue, 4 Feb 2014, Rainer Orth wrote: > > The patch to libgo/ltmain.sh is now committed to mainline. > > I've now committed the rest, thanks. > > Btw., what's the procedure for syncing the toplevel ltmain.sh these > days? AFAIK there's both the old CVS src repo as well as the new > binutils-gdb git repo. Does the patch need to go to both? Yes, shared files and directories should be kept in sync between all three repositories (the src repository is still used for newlib). It looks like some (e.g. configure.ac) have been getting out of sync lately. -- Joseph S. Myers jos...@codesourcery.com
Re: var-tracking and s390's LM(G)
Jakub Jelinek writes:
> On Tue, Feb 04, 2014 at 12:21:14PM +, Richard Sandiford wrote:
>> Jakub Jelinek writes:
>> >> But then we wouldn't be able to use var-tracking when __builtin_eh_return
>> >> is used, since in that case replacing the (set (reg ...) (mem ...))
>> >> with a (plus ...) would be incorrect -- the value we're loading from the
>> >> stack will have had a variable adjustment applied. And I know from
>> >> painful
>> >> experience that being able to debug the unwind code is very useful. :-)
>> >
>> > Aren't functions using EH_RETURN typically using frame pointer?
>>
>> Sorry, forgot to respond to this bit. On s390, _Unwind_RaiseException and
>> _Unwind_ForcedUnwind don't use a frame pointer, at least not on trunk.
>> %r11 is used as a general scratch register instead. E.g.:
>>
>> 2ba8 <_Unwind_ForcedUnwind>:
>> 2ba8: 90 6f f0 18 stm %r6,%r15,24(%r15)
>> 2bac: 0d d0 basr%r13,%r0
>> 2bae: 60 40 f0 50 std %f4,80(%r15)
>> 2bb2: 60 60 f0 58 std %f6,88(%r15)
>> 2bb6: a7 fa fd f8 ahi %r15,-520
>> 2bba: 58 c0 d0 9e l %r12,158(%r13)
>> 2bbe: 58 10 d0 9a l %r1,154(%r13)
>> 2bc2: 18 b2 lr %r11,%r2
>> ...
>> 2c10: 98 6f f2 20 lm %r6,%r15,544(%r15)
>> 2c14: 07 f4 br %r4
>
> Looking at pr54693-2.c -O2 -g -m64 -march=z196 (was that the one you meant)
> with your patches, I see:
> (insn/f:TI 122 30 31 4 (parallel [
> (set/f (mem:DI (plus:DI (reg/f:DI 15 %r15)
> (const_int 80 [0x50])) [3 S8 A8])
> (reg:DI 10 %r10))
> (set/f (mem:DI (plus:DI (reg/f:DI 15 %r15)
> (const_int 88 [0x58])) [3 S8 A8])
> (reg:DI 11 %r11))
> (set/f (mem:DI (plus:DI (reg/f:DI 15 %r15)
> (const_int 96 [0x60])) [3 S8 A8])
> (reg:DI 12 %r12))
> (set/f (mem:DI (plus:DI (reg/f:DI 15 %r15)
> (const_int 104 [0x68])) [3 S8 A8])
> (reg:DI 13 %r13))
> (set/f (mem:DI (plus:DI (reg/f:DI 15 %r15)
> (const_int 112 [0x70])) [3 S8 A8])
> (reg:DI 14 %r14))
> (set/f (mem:DI (plus:DI (reg/f:DI 15 %r15)
> (const_int 120 [0x78])) [3 S8 A8])
> (reg/f:DI 15 %r15))
> ]) pr54693-2.c:16 94 {*store_multiple_di}
> (expr_list:REG_DEAD (reg:DI 14 %r14)
> (expr_list:REG_DEAD (reg:DI 13 %r13)
> (expr_list:REG_DEAD (reg:DI 12 %r12)
> (expr_list:REG_DEAD (reg:DI 11 %r11)
> (expr_list:REG_DEAD (reg:DI 10 %r10)
> (nil)))
> ...
> (insn/f 123 31 124 4 (set (reg/f:DI 15 %r15)
> (plus:DI (reg/f:DI 15 %r15)
> (const_int -160 [0xff60]))) pr54693-2.c:16 65 {*la_64}
> (expr_list:REG_FRAME_RELATED_EXPR (set (reg/f:DI 15 %r15)
> (plus:DI (reg/f:DI 15 %r15)
> (const_int -160 [0xff60])))
> (nil)))
> ...
> (insn/f:TI 135 134 136 7 (parallel [
> (set (reg:DI 10 %r10)
> (mem:DI (plus:DI (reg/f:DI 15 %r15)
> (const_int 240 [0xf0])) [3 S8 A8]))
> (set (reg:DI 11 %r11)
> (mem:DI (plus:DI (reg/f:DI 15 %r15)
> (const_int 248 [0xf8])) [3 S8 A8]))
> (set (reg:DI 12 %r12)
> (mem:DI (plus:DI (reg/f:DI 15 %r15)
> (const_int 256 [0x100])) [3 S8 A8]))
> (set (reg:DI 13 %r13)
> (mem:DI (plus:DI (reg/f:DI 15 %r15)
> (const_int 264 [0x108])) [3 S8 A8]))
> (set (reg:DI 14 %r14)
> (mem:DI (plus:DI (reg/f:DI 15 %r15)
> (const_int 272 [0x110])) [3 S8 A8]))
> (set (reg/f:DI 15 %r15)
> (mem:DI (plus:DI (reg/f:DI 15 %r15)
> (const_int 280 [0x118])) [3 S8 A8]))
> ]) pr54693-2.c:25 92 {*load_multiple_di}
> (expr_list:REG_CFA_DEF_CFA (plus:DI (reg/f:DI 15 %r15)
> (const_int 160 [0xa0]))
> (expr_list:REG_CFA_RESTORE (reg/f:DI 15 %r15)
> (expr_list:REG_CFA_RESTORE (reg:DI 14 %r14)
> (expr_list:REG_CFA_RESTORE (reg:DI 13 %r13)
> (expr_list:REG_CFA_RESTORE (reg:DI 12 %r12)
> (expr_list:REG_CFA_RESTORE (reg:DI 11 %r11)
> (expr_list:REG_CFA_RESTORE (reg:DI 10 %r10)
> (nil)
> In a function that doesn't need frame pointer, I'd say this is a serious
> bloat of the unwind info, you are saving/restoring %r15 not because you have
> to, but just that i
Re: [PATCH][PING^2] Fix for PR59600 (prohibit inlining if no_sanitize_address)
On Tue, Feb 4, 2014 at 3:32 PM, Yury Gribov wrote: > Richard wrote: >> >> I think you can't rely on pointer equivalence of the lookup_attribute >> result so you want instead of > > > Thanks, makes sense. > > >> Please also name CIF_OPTION_MISMATCH as CIF_ATTRIBUTE_MISMATCH >> and say "function attribute mismatch" in the description. > > > Done. > > What about updated patch? Ok if it passes bootstrap and regtesting. Thanks, Richard. > -Y
Re: libsanitizer merge from upstream r196090
Commited in 207472. -Maxim
Re: libsanitizer merge from upstream r196090
On Tue, Feb 04, 2014 at 06:44:22PM +0400, Maxim Ostapenko wrote:
> Hi,
>
> > FAIL: g++.dg/tsan/default_options.C -O2 execution test
> >
> > Both these tests don't report anything (well, default_options.C
> > prints DONE),
> > simply succeed (with dg-shouldfail that is a failure)
>
> I've finally fixed g+.dg/tsan/default_options.C test (the check
> dg-shouldfail should be removed).
>
> OK to commit?
Sure.
> 2014-02-04 Max Ostapenko
>
> * g++.dg/tsan/default_options.C: Invert check.
>
> diff --git a/gcc/testsuite/g++.dg/tsan/default_options.C
> b/gcc/testsuite/g++.dg/tsan/default_options.C
> index f0c0ece..13e1984 100644
> --- a/gcc/testsuite/g++.dg/tsan/default_options.C
> +++ b/gcc/testsuite/g++.dg/tsan/default_options.C
> @@ -1,5 +1,3 @@
> -/* { dg-shouldfail "tsan" } */
> -
> #include
> #include
Jakub
[Ada] Mark intrinsic subprograms as Pure
Intrinsic subprograms in Pure packages were not marked as Pure, which was actually unintended. The spec of einfo was also not up-to-date, which is now fixed. Tested on x86_64-pc-linux-gnu, committed on trunk 2014-02-04 Arnaud Charlet * g-souinf.ads: Subprograms in this unit are actually not pure. * freeze.adb (Freeze_Subprogram): Do not reset Is_Pure for Intrinsics. * einfo.ads (Is_Pure): Update doc to match implementation. Index: g-souinf.ads === --- g-souinf.ads(revision 207465) +++ g-souinf.ads(working copy) @@ -37,7 +37,14 @@ -- the name of the source file in which the exception is handled. package GNAT.Source_Info is - pragma Pure; + pragma Preelaborate; + -- Note that this unit is Preelaborate, but not Pure, that's because the + -- functions here such as Line are clearly not pure functions, and normally + -- we mark intrinsic functions in a Pure unit as Pure, even though they are + -- imported. + -- + -- Historical note: this used to be Pure, but that was when we marked all + -- intrinsics as not Pure, even in Pure units, so no problems arose. function File return String; -- Return the name of the current file, not including the path information. Index: einfo.ads === --- einfo.ads (revision 207465) +++ einfo.ads (working copy) @@ -2775,13 +2775,13 @@ --Is_Pure (Flag44) -- Defined in all entities. Set in all entities of a unit to which a --- pragma Pure is applied, and also set for the entity of the unit --- itself. In addition, this flag may be set for any other functions --- or procedures that are known to be side effect free, so in the case --- of subprograms, the Is_Pure flag may be used by the optimizer to --- imply that it can assume freedom from side effects (other than those --- resulting from assignment to out parameters, or to objects designated --- by access parameters). +-- pragma Pure is applied except for non intrinsic imported subprogram, +-- and also set for the entity of the unit itself. In addition, this +-- flag may be set for any other functions or procedures that are known +-- to be side effect free, so in the case of subprograms, the Is_Pure +-- flag may be used by the optimizer to imply that it can assume freedom +-- from side effects (other than those resulting from assignment to out +-- parameters, or to objects designated by access parameters). --Is_Pure_Unit_Access_Type (Flag189) -- Defined in access type and subtype entities. Set if the type or Index: freeze.adb === --- freeze.adb (revision 207466) +++ freeze.adb (working copy) @@ -6514,15 +6514,17 @@ end if; -- Reset the Pure indication on an imported subprogram unless an - -- explicit Pure_Function pragma was present. We do this because - -- otherwise it is an insidious error to call a non-pure function from - -- pure unit and have calls mysteriously optimized away. What happens - -- here is that the Import can bypass the normal check to ensure that - -- pure units call only pure subprograms. + -- explicit Pure_Function pragma was present or the subprogram is an + -- intrinsic. We do this because otherwise it is an insidious error + -- to call a non-pure function from pure unit and have calls + -- mysteriously optimized away. What happens here is that the Import + -- can bypass the normal check to ensure that pure units call only pure + -- subprograms. if Is_Imported (E) and then Is_Pure (E) and then not Has_Pragma_Pure_Function (E) +and then not Is_Intrinsic_Subprogram (E) then Set_Is_Pure (E, False); end if;
Re: PR59723: fix LTO + fortran namelist ICEs
On Tue, 4 Feb 2014, Dominique Dhumieres wrote:
> With the petches (an a minor fix in gcc/lto/lto.c) the remaining failures
> are
>
> FAIL: gfortran.dg/debug/pr35154-dwarf2.f -gdwarf-2 scan-assembler
> (DW_AT_name: "label"|"label[^\n]*"[^\n]*DW_AT_name)
> FAIL: gfortran.dg/debug/pr35154-dwarf2.f -gdwarf-2 -g3 scan-assembler
> (DW_AT_name: "label"|"label[^\n]*"[^\n]*DW_AT_name)
> FAIL: gfortran.dg/bind_c_array_params_2.f90 -O scan-assembler-times
> myBindC 1
> FAIL: gfortran.dg/bind_c_vars.f90 -O0 (test for excess errors)
> FAIL: gfortran.dg/bind_c_vars.f90 -O1 (test for excess errors)
> FAIL: gfortran.dg/bind_c_vars.f90 -O2 (test for excess errors)
> FAIL: gfortran.dg/bind_c_vars.f90 -O3 -fomit-frame-pointer (test for excess
> errors)
> FAIL: gfortran.dg/bind_c_vars.f90 -O3 -fomit-frame-pointer -funroll-loops
> (test for excess errors)
> FAIL: gfortran.dg/bind_c_vars.f90 -O3 -fomit-frame-pointer
> -funroll-all-loops -finline-functions (test for excess errors)
> FAIL: gfortran.dg/bind_c_vars.f90 -O3 -g (test for excess errors)
> FAIL: gfortran.dg/bind_c_vars.f90 -Os (test for excess errors)
> FAIL: gfortran.dg/namelist_14.f90 -O3 -g (internal compiler error)
> FAIL: gfortran.dg/namelist_14.f90 -O3 -g (test for excess errors)
> FAIL: gfortran.dg/namelist_51.f90 -O3 -g (test for excess errors)
> FAIL: gfortran.dg/namelist_69.f90 -O3 -g (internal compiler error)
> FAIL: gfortran.dg/namelist_69.f90 -O3 -g (test for excess errors)
> FAIL: gfortran.dg/namelist_70.f90 -O3 -g (internal compiler error)
> FAIL: gfortran.dg/namelist_70.f90 -O3 -g (test for excess errors)
> FAIL: gfortran.dg/pr48636-2.f90 -O scan-ipa-dump cp "Creating a
> specialized node of bar/[0-9]*\\."
> FAIL: gfortran.dg/pr52835.f90 -O scan-tree-dump optimized "bar "
> FAIL: gfortran.dg/pr53787.f90 -O scan-ipa-dump cp "Creating a specialized
> node of init"
> FAIL: gfortran.dg/pr53787.f90 -O scan-ipa-dump-times cp "Aggregate
> replacements" 2
>
> The ICEs are
>
> /opt/gcc/work/gcc/testsuite/gfortran.dg/namelist_14.f90:96:0: internal
> compiler error: in force_decl_die, at dwarf2out.c:20211
>
> The excess error for gfortran.dg/namelist_51.f90 is
>
> warning: invalid DWARF generated by the compiler: DIE 0x01f0 has multiple
> AT_calling_convention attributes in
> '/var/folders/8q/sh_swgz96r7f5vnn08f7fxr0gn/T//ccub2YMQ.ltrans0.ltrans.o'.
>
> Note that if the test is compiled with -g only, I get an ICE
>
> lto1: internal compiler error: in add_AT_specification, at dwarf2out.c:4096
>
> I have looked to the scan errors for bind_c_array_params_2.f90:
>
> myBindC appears twice
>
> call_myBindC
> .ascii
> "_main\0\0\0\0\0\0\0\0\0\0\0\0\274\0\0\0_myBindC\0\0\2\0\0\0\0\0\0\0\0\0\356\0\0\0__gfortran_set_options
> ...
>
> and for pr48636-2.f90 where "Creating a specialized node of bar"
> is replaced with "Creating a specialized node of __foo_MOD_bar".
>
> Such failures are quite easy to fix, but I wonder if they worth the effort.
The dwarf2out.c ICEs are fixed with the tree_is_indexable hunk.
Bootstrapped and tested on x86_64-unknown-linux-gnu, SPEC 2k6 -flto -g
built and gfortran.dg tested with -flto, applied to trunk.
Richard.
2014-02-04 Richard Biener
PR lto/59723
* lto-streamer-out.c (tree_is_indexable): Force NAMELIST_DECLs
in function context local.
(lto_output_tree_ref): Do not write trees from lto_output_tree_ref.
* lto-streamer-in.c (lto_input_tree_ref): Handle LTO_namelist_decl_ref
similar to LTO_imported_decl_ref.
lto/
* lto.c (mentions_vars_p): Handle NAMELIST_DECL.
(lto_fixup_prevailing_decls): Handle fixing up CONSTRUCTOR values.
Index: gcc/lto-streamer-in.c
===
*** gcc/lto-streamer-in.c (revision 207455)
--- gcc/lto-streamer-in.c (working copy)
*** lto_input_tree_ref (struct lto_input_blo
*** 244,275
case LTO_imported_decl_ref:
case LTO_label_decl_ref:
case LTO_translation_unit_decl_ref:
ix_u = streamer_read_uhwi (ib);
result = lto_file_decl_data_get_var_decl (data_in->file_data, ix_u);
break;
- case LTO_namelist_decl_ref:
- {
- tree tmp;
- vec *nml_decls = NULL;
- unsigned i, n;
-
- result = make_node (NAMELIST_DECL);
- TREE_TYPE (result) = void_type_node;
- DECL_NAME (result) = stream_read_tree (ib, data_in);
- n = streamer_read_uhwi (ib);
- for (i = 0; i < n; i++)
- {
- ix_u = streamer_read_uhwi (ib);
- tmp = lto_file_decl_data_get_var_decl (data_in->file_data, ix_u);
- gcc_assert (tmp != NULL_TREE);
- CONSTRUCTOR_APPEND_ELT (nml_decls, NULL_TREE, tmp);
- }
- NAMELIST_DECL_ASSOCIATED_DECL (result) = build_constructor (NULL_TREE,
- nml_decls);
-
[Ada] Loops over containers without variable indexing
This patch adds a legality check that in a loop over a container that only has a constant indexing aspect, elements are constant within the loop, and attempts to modify them are appripriately rejected. Compiling p.adb must yield: p.adb:15:07: left hand side of assignment must be a variable --- with Ada.Containers.Hashed_Sets; use Ada.Containers; procedure P is function Hash (N : Natural) return Hash_Type is begin return Hash_Type (N); end; package Sets is new Ada.Containers.Hashed_Sets (Natural, Hash, "="); S : Sets.Set; begin S.Insert (1); for NConst of S loop NConst := 0; -- Should not be allowed end loop; end; Tested on x86_64-pc-linux-gnu, committed on trunk 2014-02-04 Ed Schonberg * exp_ch5.adb (Expand_Iterator_Loop): For a container element iterator, indicate that the element is a constant if the container type does not have a variable indexing aspect. * sem_ch8.adb (Analyze_Object_Renaming): If the entity is already marked as constant, do not reset its Ekind, to ensure that container elements in an element loop are not modified if the container (e.g. a hashed set) only has a constant indexing aspect. Index: exp_ch5.adb === --- exp_ch5.adb (revision 207465) +++ exp_ch5.adb (working copy) @@ -3128,6 +3128,15 @@ Set_Debug_Info_Needed (Id); + -- If the container does not have a variable indexing aspect, + -- the element is a constant in the loop. + + if No (Find_Value_Of_Aspect + (Container_Typ, Aspect_Variable_Indexing)) + then + Set_Ekind (Id, E_Constant); + end if; + -- If the container holds controlled objects, wrap the loop -- statements and element renaming declaration with a block. -- This ensures that the result of Element (Cusor) is Index: sem_ch8.adb === --- sem_ch8.adb (revision 207465) +++ sem_ch8.adb (working copy) @@ -1193,8 +1193,14 @@ end; end if; - Set_Ekind (Id, E_Variable); + -- Set the Ekind of the entity, unless it has been set already, as is + -- the case for the iteration object over a container with no variable + -- indexing. + if Ekind (Id) /= E_Constant then + Set_Ekind (Id, E_Variable); + end if; + -- Initialize the object size and alignment. Note that we used to call -- Init_Size_Align here, but that's wrong for objects which have only -- an Esize, not an RM_Size field.
Re: libsanitizer merge from upstream r196090
Hi,
> FAIL: g++.dg/tsan/default_options.C -O2 execution test
>
> Both these tests don't report anything (well, default_options.C
> prints DONE),
> simply succeed (with dg-shouldfail that is a failure)
I've finally fixed g+.dg/tsan/default_options.C test (the check
dg-shouldfail should be removed).
OK to commit?
-Maxim
2014-02-04 Max Ostapenko
* g++.dg/tsan/default_options.C: Invert check.
diff --git a/gcc/testsuite/g++.dg/tsan/default_options.C b/gcc/testsuite/g++.dg/tsan/default_options.C
index f0c0ece..13e1984 100644
--- a/gcc/testsuite/g++.dg/tsan/default_options.C
+++ b/gcc/testsuite/g++.dg/tsan/default_options.C
@@ -1,5 +1,3 @@
-/* { dg-shouldfail "tsan" } */
-
#include
#include
Re: [PATCH][PING^2] Fix for PR59600 (prohibit inlining if no_sanitize_address)
Richard wrote:
I think you can't rely on pointer equivalence of the lookup_attribute
result so you want instead of
Thanks, makes sense.
Please also name CIF_OPTION_MISMATCH as CIF_ATTRIBUTE_MISMATCH
and say "function attribute mismatch" in the description.
Done.
What about updated patch?
-Y
PR sanitizer/59600
gcc/ChangeLog:
2014-01-21 ygribov
* cif-code.def (ATTRIBUTE_MISMATCH): New CIF code.
* ipa-inline.c (report_inline_failed_reason): Handle mismatched
sanitization attributes.
(sanitize_attrs_match_for_inline_p): New function.
(can_inline_edge_p): Likewise.
gcc/testsuite/ChangeLog:
2014-01-21 ygribov
* gcc.dg/asan/nosanitize-and-inline.c: : New test.
diff --git a/gcc/cif-code.def b/gcc/cif-code.def
index 5591f9a..71f3e39 100644
--- a/gcc/cif-code.def
+++ b/gcc/cif-code.def
@@ -123,3 +123,7 @@ DEFCIFCODE(OPTIMIZATION_MISMATCH, CIF_FINAL_ERROR,
/* We can't inline because the callee refers to comdat-local symbols. */
DEFCIFCODE(USES_COMDAT_LOCAL, CIF_FINAL_NORMAL,
N_("callee refers to comdat-local symbols"))
+
+/* We can't inline because of mismatched caller/callee attributes. */
+DEFCIFCODE(ATTRIBUTE_MISMATCH, CIF_FINAL_NORMAL,
+ N_("function attribute mismatch"))
diff --git a/gcc/ipa-inline.c b/gcc/ipa-inline.c
index 5f47e0b..ce24ea5 100644
--- a/gcc/ipa-inline.c
+++ b/gcc/ipa-inline.c
@@ -234,7 +234,25 @@ report_inline_failed_reason (struct cgraph_edge *e)
}
}
-/* Decide if we can inline the edge and possibly update
+ /* Decide whether sanitizer-related attributes allow inlining. */
+
+static bool
+sanitize_attrs_match_for_inline_p (const_tree caller, const_tree callee)
+{
+ /* Don't care if sanitizer is disabled */
+ if (!(flag_sanitize & SANITIZE_ADDRESS))
+return true;
+
+ if (!caller || !callee)
+return true;
+
+ return !!lookup_attribute ("no_sanitize_address",
+ DECL_ATTRIBUTES (caller)) ==
+ !!lookup_attribute ("no_sanitize_address",
+ DECL_ATTRIBUTES (callee));
+}
+
+ /* Decide if we can inline the edge and possibly update
inline_failed reason.
We check whether inlining is possible at all and whether
caller growth limits allow doing so.
@@ -327,6 +345,12 @@ can_inline_edge_p (struct cgraph_edge *e, bool report,
e->inline_failed = CIF_TARGET_OPTION_MISMATCH;
inlinable = false;
}
+ /* Don't inline a function with mismatched sanitization attributes. */
+ else if (!sanitize_attrs_match_for_inline_p (e->caller->decl, callee->decl))
+{
+ e->inline_failed = CIF_ATTRIBUTE_MISMATCH;
+ inlinable = false;
+}
/* Check if caller growth allows the inlining. */
else if (!DECL_DISREGARD_INLINE_LIMITS (callee->decl)
&& !disregard_limits
diff --git a/gcc/testsuite/gcc.dg/asan/nosanitize-and-inline.c b/gcc/testsuite/gcc.dg/asan/nosanitize-and-inline.c
index e69de29..5853801 100644
--- a/gcc/testsuite/gcc.dg/asan/nosanitize-and-inline.c
+++ b/gcc/testsuite/gcc.dg/asan/nosanitize-and-inline.c
@@ -0,0 +1,57 @@
+/* { dg-do run } */
+
+/* This is a simplified version of what Emacs does internally,
+ when marking its stack. */
+
+static unsigned long sum;
+static void *stack_base;
+
+/* A simple substitute for what Emacs actually does. */
+static void
+mark_maybe_pointer (void *p)
+{
+ sum ^= (unsigned long) p;
+}
+
+static inline void __attribute__ ((no_sanitize_address))
+mark_memory (void **start, void **end)
+{
+ void **pp;
+
+ if (end < start)
+{
+ void **tem = start;
+ start = end;
+ end = tem;
+}
+
+ for (pp = start; pp < end; pp++)
+{
+ /* This is the dereference that we don't want sanitized. */
+ void *p = *pp;
+
+ mark_maybe_pointer (p);
+}
+}
+
+static void
+mark_stack (void)
+{
+ void *end;
+ mark_memory (stack_base, &end);
+}
+
+void
+garbage_collect (void)
+{
+ mark_stack ();
+}
+
+int
+main (void)
+{
+ void *dummy;
+ stack_base = &dummy;
+ garbage_collect ();
+ return 0;
+}
Re: var-tracking and s390's LM(G)
On Tue, Feb 04, 2014 at 12:21:14PM +, Richard Sandiford wrote:
> Jakub Jelinek writes:
> >> But then we wouldn't be able to use var-tracking when __builtin_eh_return
> >> is used, since in that case replacing the (set (reg ...) (mem ...))
> >> with a (plus ...) would be incorrect -- the value we're loading from the
> >> stack will have had a variable adjustment applied. And I know from painful
> >> experience that being able to debug the unwind code is very useful. :-)
> >
> > Aren't functions using EH_RETURN typically using frame pointer?
>
> Sorry, forgot to respond to this bit. On s390, _Unwind_RaiseException and
> _Unwind_ForcedUnwind don't use a frame pointer, at least not on trunk.
> %r11 is used as a general scratch register instead. E.g.:
>
> 2ba8 <_Unwind_ForcedUnwind>:
> 2ba8: 90 6f f0 18 stm %r6,%r15,24(%r15)
> 2bac: 0d d0 basr%r13,%r0
> 2bae: 60 40 f0 50 std %f4,80(%r15)
> 2bb2: 60 60 f0 58 std %f6,88(%r15)
> 2bb6: a7 fa fd f8 ahi %r15,-520
> 2bba: 58 c0 d0 9e l %r12,158(%r13)
> 2bbe: 58 10 d0 9a l %r1,154(%r13)
> 2bc2: 18 b2 lr %r11,%r2
> ...
> 2c10: 98 6f f2 20 lm %r6,%r15,544(%r15)
> 2c14: 07 f4 br %r4
Looking at pr54693-2.c -O2 -g -m64 -march=z196 (was that the one you meant)
with your patches, I see:
(insn/f:TI 122 30 31 4 (parallel [
(set/f (mem:DI (plus:DI (reg/f:DI 15 %r15)
(const_int 80 [0x50])) [3 S8 A8])
(reg:DI 10 %r10))
(set/f (mem:DI (plus:DI (reg/f:DI 15 %r15)
(const_int 88 [0x58])) [3 S8 A8])
(reg:DI 11 %r11))
(set/f (mem:DI (plus:DI (reg/f:DI 15 %r15)
(const_int 96 [0x60])) [3 S8 A8])
(reg:DI 12 %r12))
(set/f (mem:DI (plus:DI (reg/f:DI 15 %r15)
(const_int 104 [0x68])) [3 S8 A8])
(reg:DI 13 %r13))
(set/f (mem:DI (plus:DI (reg/f:DI 15 %r15)
(const_int 112 [0x70])) [3 S8 A8])
(reg:DI 14 %r14))
(set/f (mem:DI (plus:DI (reg/f:DI 15 %r15)
(const_int 120 [0x78])) [3 S8 A8])
(reg/f:DI 15 %r15))
]) pr54693-2.c:16 94 {*store_multiple_di}
(expr_list:REG_DEAD (reg:DI 14 %r14)
(expr_list:REG_DEAD (reg:DI 13 %r13)
(expr_list:REG_DEAD (reg:DI 12 %r12)
(expr_list:REG_DEAD (reg:DI 11 %r11)
(expr_list:REG_DEAD (reg:DI 10 %r10)
(nil)))
...
(insn/f 123 31 124 4 (set (reg/f:DI 15 %r15)
(plus:DI (reg/f:DI 15 %r15)
(const_int -160 [0xff60]))) pr54693-2.c:16 65 {*la_64}
(expr_list:REG_FRAME_RELATED_EXPR (set (reg/f:DI 15 %r15)
(plus:DI (reg/f:DI 15 %r15)
(const_int -160 [0xff60])))
(nil)))
...
(insn/f:TI 135 134 136 7 (parallel [
(set (reg:DI 10 %r10)
(mem:DI (plus:DI (reg/f:DI 15 %r15)
(const_int 240 [0xf0])) [3 S8 A8]))
(set (reg:DI 11 %r11)
(mem:DI (plus:DI (reg/f:DI 15 %r15)
(const_int 248 [0xf8])) [3 S8 A8]))
(set (reg:DI 12 %r12)
(mem:DI (plus:DI (reg/f:DI 15 %r15)
(const_int 256 [0x100])) [3 S8 A8]))
(set (reg:DI 13 %r13)
(mem:DI (plus:DI (reg/f:DI 15 %r15)
(const_int 264 [0x108])) [3 S8 A8]))
(set (reg:DI 14 %r14)
(mem:DI (plus:DI (reg/f:DI 15 %r15)
(const_int 272 [0x110])) [3 S8 A8]))
(set (reg/f:DI 15 %r15)
(mem:DI (plus:DI (reg/f:DI 15 %r15)
(const_int 280 [0x118])) [3 S8 A8]))
]) pr54693-2.c:25 92 {*load_multiple_di}
(expr_list:REG_CFA_DEF_CFA (plus:DI (reg/f:DI 15 %r15)
(const_int 160 [0xa0]))
(expr_list:REG_CFA_RESTORE (reg/f:DI 15 %r15)
(expr_list:REG_CFA_RESTORE (reg:DI 14 %r14)
(expr_list:REG_CFA_RESTORE (reg:DI 13 %r13)
(expr_list:REG_CFA_RESTORE (reg:DI 12 %r12)
(expr_list:REG_CFA_RESTORE (reg:DI 11 %r11)
(expr_list:REG_CFA_RESTORE (reg:DI 10 %r10)
(nil)
In a function that doesn't need frame pointer, I'd say this is a serious
bloat of the unwind info, you are saving/restoring %r15 not because you have
to, but just that it seems to be cheaper for the target. So, I'd say you
shouldn't emit DW_CFA_offset 15, 5 on the insn 122 in the prologue and
DW_CFA_restore 15 in the epilogue (twice in this case), that just wast
Re: PR59723: fix LTO + fortran namelist ICEs
With the petches (an a minor fix in gcc/lto/lto.c) the remaining failures are FAIL: gfortran.dg/debug/pr35154-dwarf2.f -gdwarf-2 scan-assembler (DW_AT_name: "label"|"label[^\n]*"[^\n]*DW_AT_name) FAIL: gfortran.dg/debug/pr35154-dwarf2.f -gdwarf-2 -g3 scan-assembler (DW_AT_name: "label"|"label[^\n]*"[^\n]*DW_AT_name) FAIL: gfortran.dg/bind_c_array_params_2.f90 -O scan-assembler-times myBindC 1 FAIL: gfortran.dg/bind_c_vars.f90 -O0 (test for excess errors) FAIL: gfortran.dg/bind_c_vars.f90 -O1 (test for excess errors) FAIL: gfortran.dg/bind_c_vars.f90 -O2 (test for excess errors) FAIL: gfortran.dg/bind_c_vars.f90 -O3 -fomit-frame-pointer (test for excess errors) FAIL: gfortran.dg/bind_c_vars.f90 -O3 -fomit-frame-pointer -funroll-loops (test for excess errors) FAIL: gfortran.dg/bind_c_vars.f90 -O3 -fomit-frame-pointer -funroll-all-loops -finline-functions (test for excess errors) FAIL: gfortran.dg/bind_c_vars.f90 -O3 -g (test for excess errors) FAIL: gfortran.dg/bind_c_vars.f90 -Os (test for excess errors) FAIL: gfortran.dg/namelist_14.f90 -O3 -g (internal compiler error) FAIL: gfortran.dg/namelist_14.f90 -O3 -g (test for excess errors) FAIL: gfortran.dg/namelist_51.f90 -O3 -g (test for excess errors) FAIL: gfortran.dg/namelist_69.f90 -O3 -g (internal compiler error) FAIL: gfortran.dg/namelist_69.f90 -O3 -g (test for excess errors) FAIL: gfortran.dg/namelist_70.f90 -O3 -g (internal compiler error) FAIL: gfortran.dg/namelist_70.f90 -O3 -g (test for excess errors) FAIL: gfortran.dg/pr48636-2.f90 -O scan-ipa-dump cp "Creating a specialized node of bar/[0-9]*\\." FAIL: gfortran.dg/pr52835.f90 -O scan-tree-dump optimized "bar " FAIL: gfortran.dg/pr53787.f90 -O scan-ipa-dump cp "Creating a specialized node of init" FAIL: gfortran.dg/pr53787.f90 -O scan-ipa-dump-times cp "Aggregate replacements" 2 The ICEs are /opt/gcc/work/gcc/testsuite/gfortran.dg/namelist_14.f90:96:0: internal compiler error: in force_decl_die, at dwarf2out.c:20211 The excess error for gfortran.dg/namelist_51.f90 is warning: invalid DWARF generated by the compiler: DIE 0x01f0 has multiple AT_calling_convention attributes in '/var/folders/8q/sh_swgz96r7f5vnn08f7fxr0gn/T//ccub2YMQ.ltrans0.ltrans.o'. Note that if the test is compiled with -g only, I get an ICE lto1: internal compiler error: in add_AT_specification, at dwarf2out.c:4096 I have looked to the scan errors for bind_c_array_params_2.f90: myBindC appears twice call_myBindC .ascii "_main\0\0\0\0\0\0\0\0\0\0\0\0\274\0\0\0_myBindC\0\0\2\0\0\0\0\0\0\0\0\0\356\0\0\0__gfortran_set_options ... and for pr48636-2.f90 where "Creating a specialized node of bar" is replaced with "Creating a specialized node of __foo_MOD_bar". Such failures are quite easy to fix, but I wonder if they worth the effort. Dominique
Re: PR ipa/59831 (ipa-cp devirt issues)
On 2014.02.04 at 14:18 +0100, Paolo Carlini wrote: > .. to wit, for 27_io/basic_stringbuf/sputbackc/char/9425.cc: > > 0xbb482f crash_signal > /scratch/Gcc/svn-dirs/trunk/gcc/toplev.c:337 > 0x10ce353 contains_struct_check > /scratch/Gcc/svn-dirs/trunk/gcc/tree.h:2822 > 0x10ce353 ipa_get_indirect_edge_target_1 > /scratch/Gcc/svn-dirs/trunk/gcc/ipa-cp.c:1576 > 0xa3901c estimate_edge_devirt_benefit > /scratch/Gcc/svn-dirs/trunk/gcc/ipa-inline-analysis.c:2945 > 0xa3901c estimate_edge_size_and_time > /scratch/Gcc/svn-dirs/trunk/gcc/ipa-inline-analysis.c:2977 > 0xa3901c estimate_calls_size_and_time > /scratch/Gcc/svn-dirs/trunk/gcc/ipa-inline-analysis.c:3030 > 0xa394cc estimate_node_size_and_time > /scratch/Gcc/svn-dirs/trunk/gcc/ipa-inline-analysis.c:3117 > 0xa3a5c4 do_estimate_edge_time(cgraph_edge*) > /scratch/Gcc/svn-dirs/trunk/gcc/ipa-inline-analysis.c:3607 > 0xa3a947 do_estimate_edge_size(cgraph_edge*) > /scratch/Gcc/svn-dirs/trunk/gcc/ipa-inline-analysis.c:3646 > 0xa3af7d estimate_edge_size > /scratch/Gcc/svn-dirs/trunk/gcc/ipa-inline.h:277 > 0xa3af7d estimate_edge_growth > /scratch/Gcc/svn-dirs/trunk/gcc/ipa-inline.h:289 > 0xa3af7d estimate_size_after_inlining(cgraph_node*, cgraph_edge*) > /scratch/Gcc/svn-dirs/trunk/gcc/ipa-inline-analysis.c:3739 > 0x10dd033 caller_growth_limits > /scratch/Gcc/svn-dirs/trunk/gcc/ipa-inline.c:184 > 0x10dd033 can_inline_edge_p > /scratch/Gcc/svn-dirs/trunk/gcc/ipa-inline.c:338 > 0x10dd71f update_callee_keys > /scratch/Gcc/svn-dirs/trunk/gcc/ipa-inline.c:1211 > 0x10dfe23 inline_small_functions > /scratch/Gcc/svn-dirs/trunk/gcc/ipa-inline.c:1791 > 0x10dfe23 ipa_inline > /scratch/Gcc/svn-dirs/trunk/gcc/ipa-inline.c:2025 > 0x10dfe23 execute > /scratch/Gcc/svn-dirs/trunk/gcc/ipa-inline.c:2381 See also: http://gcc.gnu.org/bugzilla/show_bug.cgi?id=60058 -- Markus
Re: PR ipa/59831 (ipa-cp devirt issues)
.. to wit, for 27_io/basic_stringbuf/sputbackc/char/9425.cc: 0xbb482f crash_signal /scratch/Gcc/svn-dirs/trunk/gcc/toplev.c:337 0x10ce353 contains_struct_check /scratch/Gcc/svn-dirs/trunk/gcc/tree.h:2822 0x10ce353 ipa_get_indirect_edge_target_1 /scratch/Gcc/svn-dirs/trunk/gcc/ipa-cp.c:1576 0xa3901c estimate_edge_devirt_benefit /scratch/Gcc/svn-dirs/trunk/gcc/ipa-inline-analysis.c:2945 0xa3901c estimate_edge_size_and_time /scratch/Gcc/svn-dirs/trunk/gcc/ipa-inline-analysis.c:2977 0xa3901c estimate_calls_size_and_time /scratch/Gcc/svn-dirs/trunk/gcc/ipa-inline-analysis.c:3030 0xa394cc estimate_node_size_and_time /scratch/Gcc/svn-dirs/trunk/gcc/ipa-inline-analysis.c:3117 0xa3a5c4 do_estimate_edge_time(cgraph_edge*) /scratch/Gcc/svn-dirs/trunk/gcc/ipa-inline-analysis.c:3607 0xa3a947 do_estimate_edge_size(cgraph_edge*) /scratch/Gcc/svn-dirs/trunk/gcc/ipa-inline-analysis.c:3646 0xa3af7d estimate_edge_size /scratch/Gcc/svn-dirs/trunk/gcc/ipa-inline.h:277 0xa3af7d estimate_edge_growth /scratch/Gcc/svn-dirs/trunk/gcc/ipa-inline.h:289 0xa3af7d estimate_size_after_inlining(cgraph_node*, cgraph_edge*) /scratch/Gcc/svn-dirs/trunk/gcc/ipa-inline-analysis.c:3739 0x10dd033 caller_growth_limits /scratch/Gcc/svn-dirs/trunk/gcc/ipa-inline.c:184 0x10dd033 can_inline_edge_p /scratch/Gcc/svn-dirs/trunk/gcc/ipa-inline.c:338 0x10dd71f update_callee_keys /scratch/Gcc/svn-dirs/trunk/gcc/ipa-inline.c:1211 0x10dfe23 inline_small_functions /scratch/Gcc/svn-dirs/trunk/gcc/ipa-inline.c:1791 0x10dfe23 ipa_inline /scratch/Gcc/svn-dirs/trunk/gcc/ipa-inline.c:2025 0x10dfe23 execute /scratch/Gcc/svn-dirs/trunk/gcc/ipa-inline.c:2381 Paolo.
Re: PR ipa/59831 (ipa-cp devirt issues)
Hi, On 02/04/2014 06:43 AM, Jan Hubicka wrote: Hi, this patch solves the actual ICE in PR59831 by using ipa-devirt instead of gimple_extract_devirt_binfo_from_cst as discussed in the first post. Honza PR ipa/59831 * ipa-cp.c (ipa_get_indirect_edge_target_1): Use ipa-devirt to figure out targets of polymorphic calls with known decl. * ipa-prop.c (try_make_edge_direct_virtual_call): Likewise. * ipa-utils.h (get_polymorphic_call_info_from_invariant): Declare. * ipa-devirt.c (get_polymorphic_call_info_for_decl): Break out from ... (get_polymorphic_call_info): ... here. (get_polymorphic_call_info_from_invariant): New function. * g++.dg/ipa/devirt-22.C: New testcase. Today I'm seeing regressions in the libstdc++-v3 testsuite in the form of ICEs exactly in ipa_get_indirect_edge_target_1: FAIL: 27_io/basic_stringbuf/sputbackc/char/9425.cc (test for excess errors) WARNING: 27_io/basic_stringbuf/sputbackc/char/9425.cc compilation failed to produce executable FAIL: 27_io/basic_stringbuf/sputbackc/wchar_t/9425.cc (test for excess errors) WARNING: 27_io/basic_stringbuf/sputbackc/wchar_t/9425.cc compilation failed to produce executable FAIL: 27_io/basic_stringbuf/str/char/2.cc (test for excess errors) WARNING: 27_io/basic_stringbuf/str/char/2.cc compilation failed to produce executable FAIL: 27_io/basic_stringbuf/str/wchar_t/2.cc (test for excess errors) WARNING: 27_io/basic_stringbuf/str/wchar_t/2.cc compilation failed to produce executable Can you please have a look? Thanks, Paolo.
Re: [PATCH] Fix masked loads/stores with -fnon-call-exceptions (PR tree-optimization/60023)
On Tue, 4 Feb 2014, Jakub Jelinek wrote: > On Tue, Feb 04, 2014 at 01:57:19PM +0100, Richard Biener wrote: > > > @@ -1691,6 +1691,13 @@ vect_finish_stmt_generation (gimple stmt > > > } > > > > > >gimple_set_location (vec_stmt, gimple_location (stmt)); > > > + > > > + /* While EH edges will generally prevent vectorization, stmt might > > > + e.g. be in a must-not-throw region. Ensure newly created stmts > > > + that could throw are part of the same region. */ > > > + int lp_nr = lookup_stmt_eh_lp (stmt); > > > + if (lp_nr != 0 && stmt_could_throw_p (vec_stmt)) > > > +add_stmt_to_eh_lp (vec_stmt, lp_nr); > > > > Can you instead use maybe_clean_or_replace_eh_stmt (stmt, vec_stmt)? > > That should (maybe) take care of the missed DCE as well (maybe only > > for the first scalar stmt though). > > No, because you can have several vec_stmts that could throw created > for a single scalar stmt that can throw. By doing > maybe_clean_or_replace_eh_stmt the EH will be moved on the first > vec_stmt and the rest of stmts will not be added to the region, > furthermore, the scalar stmt which still remains in the IL will > not be in the EH table anymore either (the vectorizer apparently has > code to make scalar stmts used even after vectorization, not sure > if it can happen for loads/stores). > > If/when the scalar stmt is removed (right now just stores), it > is removed from the EH tables. Hmm, I see. Patch is ok then. Thanks, Richard.
Re: [PATCH] Fix masked loads/stores with -fnon-call-exceptions (PR tree-optimization/60023)
On Tue, Feb 04, 2014 at 01:57:19PM +0100, Richard Biener wrote: > > @@ -1691,6 +1691,13 @@ vect_finish_stmt_generation (gimple stmt > > } > > > >gimple_set_location (vec_stmt, gimple_location (stmt)); > > + > > + /* While EH edges will generally prevent vectorization, stmt might > > + e.g. be in a must-not-throw region. Ensure newly created stmts > > + that could throw are part of the same region. */ > > + int lp_nr = lookup_stmt_eh_lp (stmt); > > + if (lp_nr != 0 && stmt_could_throw_p (vec_stmt)) > > +add_stmt_to_eh_lp (vec_stmt, lp_nr); > > Can you instead use maybe_clean_or_replace_eh_stmt (stmt, vec_stmt)? > That should (maybe) take care of the missed DCE as well (maybe only > for the first scalar stmt though). No, because you can have several vec_stmts that could throw created for a single scalar stmt that can throw. By doing maybe_clean_or_replace_eh_stmt the EH will be moved on the first vec_stmt and the rest of stmts will not be added to the region, furthermore, the scalar stmt which still remains in the IL will not be in the EH table anymore either (the vectorizer apparently has code to make scalar stmts used even after vectorization, not sure if it can happen for loads/stores). If/when the scalar stmt is removed (right now just stores), it is removed from the EH tables. Jakub
Re: [PATCH] Disallow cloning of __attribute__((optimize (0))) functions (PR ipa/60026)
On Tue, 4 Feb 2014, Jakub Jelinek wrote:
> Hi!
>
> As discussed in the PR, this patch disallows cloning of
> non-optimized functions, which doesn't work properly e.g. because
> -O0 functions don't have vdef/vuses, but also doesn't sound like
> something people would like to happen if they say particular function
> shouldn't be optimized. If everything is compiled with -O0, we
> don't perform the IPA optimizations at all.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
Ok.
Thanks,
Richard.
> 2014-02-04 Jakub Jelinek
>
> PR ipa/60026
> * tree-inline.c (copy_forbidden): Fail for
> __attribute__((optimize (0))) functions.
>
> * c-c++-common/torture/pr60026.c: New test.
>
> --- gcc/tree-inline.c.jj 2014-02-01 09:39:59.0 +0100
> +++ gcc/tree-inline.c 2014-02-03 19:33:52.146931674 +0100
> @@ -3315,6 +3315,18 @@ copy_forbidden (struct function *fun, tr
> goto fail;
>}
>
> + tree fs_opts;
> + fs_opts = DECL_FUNCTION_SPECIFIC_OPTIMIZATION (fun->decl);
> + if (fs_opts)
> +{
> + struct cl_optimization *os = TREE_OPTIMIZATION (fs_opts);
> + if (!os->x_optimize)
> + {
> + reason = G_("function %q+F compiled without optimizations");
> + goto fail;
> + }
> +}
> +
> fail:
>fun->cannot_be_copied_reason = reason;
>fun->cannot_be_copied_set = true;
> --- gcc/testsuite/c-c++-common/torture/pr60026.c.jj 2014-02-03
> 18:18:37.395092543 +0100
> +++ gcc/testsuite/c-c++-common/torture/pr60026.c 2014-02-03
> 18:18:26.0 +0100
> @@ -0,0 +1,28 @@
> +/* PR ipa/60026 */
> +/* { dg-do compile } */
> +
> +struct S { int f; } a;
> +
> +__attribute__((optimize (0)))
> +struct S foo (int x, struct S y)
> +{
> + int b = y.f;
> + return a;
> +}
> +
> +void
> +bar ()
> +{
> + while (a.f)
> +{
> + struct S c = {0};
> + foo (0, c);
> +}
> +}
> +
> +int
> +main ()
> +{
> + bar ();
> + return 0;
> +}
>
> Jakub
>
>
--
Richard Biener
SUSE / SUSE Labs
SUSE LINUX Products GmbH - Nuernberg - AG Nuernberg - HRB 16746
GF: Jeff Hawn, Jennifer Guild, Felix Imend"orffer
Re: [PATCH] Fix masked loads/stores with -fnon-call-exceptions (PR tree-optimization/60023)
On Tue, 4 Feb 2014, Jakub Jelinek wrote:
> Hi!
>
> If a load or store is in EH tables, but doesn't have EH edges out of it,
> e.g. for must-not-throw regions, we weren't cleaning it up and adding
> MASK_{LOAD,STORE} into EH region instead.
> Similarly, the vectorizer needs to add the vectorized stmts to the same
> EH region as the corresponding {,masked} load/store (second hunk).
> The rest is just making the compiler more robust and not crash if DCE etc.
> decides to remove the lhs of MASK_LOAD.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
>
> P.S.: the patch doesn't fix the old issue that -fnon-call-exceptions
> loads are never removed from the vectorized loops, stores aren't actually
> removed when transforming the stmt, but later on and handling scalar loads
> similarly (turning them into say assignments of zero) didn't look trivial.
>
> 2014-02-04 Jakub Jelinek
>
> PR tree-optimization/60023
> * tree-if-conv.c (predicate_mem_writes): Pass true instead of
> false to gsi_replace.
> * tree-vect-stmts.c (vect_finish_stmt_generation): If stmt
> has been in some EH region and vec_stmt could throw, add
> vec_stmt into the same EH region.
> * tree-data-ref.c (get_references_in_stmt): If IFN_MASK_LOAD
> has no lhs, ignore it.
> * internal-fn.c (expand_MASK_LOAD): Likewise.
>
> * g++.dg/vect/pr60023.cc: New test.
>
> --- gcc/tree-if-conv.c.jj 2014-01-03 11:40:57.0 +0100
> +++ gcc/tree-if-conv.c2014-02-03 16:01:40.250716523 +0100
> @@ -1723,7 +1723,7 @@ predicate_mem_writes (loop_p loop)
> new_stmt
> = gimple_build_call_internal (IFN_MASK_STORE, 4, addr, ptr,
> mask, rhs);
> - gsi_replace (&gsi, new_stmt, false);
> + gsi_replace (&gsi, new_stmt, true);
> }
> else if (gimple_vdef (stmt))
> {
> --- gcc/tree-vect-stmts.c.jj 2014-01-16 20:54:59.0 +0100
> +++ gcc/tree-vect-stmts.c 2014-02-03 16:29:00.881394627 +0100
> @@ -1691,6 +1691,13 @@ vect_finish_stmt_generation (gimple stmt
> }
>
>gimple_set_location (vec_stmt, gimple_location (stmt));
> +
> + /* While EH edges will generally prevent vectorization, stmt might
> + e.g. be in a must-not-throw region. Ensure newly created stmts
> + that could throw are part of the same region. */
> + int lp_nr = lookup_stmt_eh_lp (stmt);
> + if (lp_nr != 0 && stmt_could_throw_p (vec_stmt))
> +add_stmt_to_eh_lp (vec_stmt, lp_nr);
Can you instead use maybe_clean_or_replace_eh_stmt (stmt, vec_stmt)?
That should (maybe) take care of the missed DCE as well (maybe only
for the first scalar stmt though).
Otherwise ok.
Thanks,
Richard.
> }
>
> /* Checks if CALL can be vectorized in type VECTYPE. Returns
> --- gcc/tree-data-ref.c.jj2014-01-03 11:40:57.0 +0100
> +++ gcc/tree-data-ref.c 2014-02-03 16:05:42.111480787 +0100
> @@ -4401,6 +4401,8 @@ get_references_in_stmt (gimple stmt, vec
> switch (gimple_call_internal_fn (stmt))
> {
> case IFN_MASK_LOAD:
> + if (gimple_call_lhs (stmt) == NULL_TREE)
> + break;
> ref.is_read = true;
> case IFN_MASK_STORE:
> ref.ref = fold_build2 (MEM_REF,
> --- gcc/internal-fn.c.jj 2014-02-03 11:48:17.0 +0100
> +++ gcc/internal-fn.c 2014-02-03 16:08:55.606492962 +0100
> @@ -820,6 +820,8 @@ expand_MASK_LOAD (gimple stmt)
>
>maskt = gimple_call_arg (stmt, 2);
>lhs = gimple_call_lhs (stmt);
> + if (lhs == NULL_TREE)
> +return;
>type = TREE_TYPE (lhs);
>rhs = fold_build2 (MEM_REF, type, gimple_call_arg (stmt, 0),
>gimple_call_arg (stmt, 1));
> --- gcc/testsuite/g++.dg/vect/pr60023.cc.jj 2014-02-03 16:39:17.996271473
> +0100
> +++ gcc/testsuite/g++.dg/vect/pr60023.cc 2014-02-03 16:39:36.455176379
> +0100
> @@ -0,0 +1,80 @@
> +// PR tree-optimization/60023
> +// { dg-do compile }
> +// { dg-additional-options "-O3 -std=c++11 -fnon-call-exceptions" }
> +// { dg-additional-options "-mavx2" { target { i?86-*-* x86_64-*-* } } }
> +
> +struct A { A (); ~A (); };
> +
> +void
> +f1 (int *p, int *q, int *r) noexcept (true)
> +{
> + int i;
> + for (i = 0; i < 1024; i++)
> +if (r[i])
> + p[i] = q[i] + 1;
> +}
> +
> +void
> +f2 (int *p, int *q, int *r)
> +{
> + int i;
> + for (i = 0; i < 1024; i++)
> +if (r[i])
> + p[i] = q[i] + 1;
> +}
> +
> +void
> +f3 (int *p, int *q) noexcept (true)
> +{
> + int i;
> + for (i = 0; i < 1024; i++)
> +p[i] = q[i] + 1;
> +}
> +
> +void
> +f4 (int *p, int *q)
> +{
> + int i;
> + for (i = 0; i < 1024; i++)
> +p[i] = q[i] + 1;
> +}
> +
> +void
> +f5 (int *p, int *q, int *r) noexcept (true)
> +{
> + int i;
> + A a;
> + for (i = 0; i < 1024; i++)
> +if (r[i])
> + p[i] = q[i] + 1;
> +}
> +
> +void
> +f6 (int *p, int *q, int *r)
> +{
> + int i;
> + A a;
> + for (i = 0; i < 1024; i++)
Re: [PATCH] Clear DECL_LANG_SPECIFIC after changing arguments (PR tree-optimization/60002)
On Tue, Feb 04, 2014 at 01:43:01PM +0100, Richard Biener wrote: > On Tue, 4 Feb 2014, Jakub Jelinek wrote: > > > Hi! > > > > As described in the PR, while build_function_decl_skip_args > > takes care of changing the type from METHOD_TYPE to FUNCTION_TYPE > > if removing first argument (this), the FE may have other special arguments > > that it is aware of (such as in-charge etc.) and if the this argument > > is not removed, but the other arguments are, then the FE can crash > > upon seeing such methods. > > > > Fixed by clearing DECL_LANG_SPECIFIC if we remove any arguments. > > > > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? > > Ok, but shouldn't we do that in the copy_node case as well? Two > places look like > > /* Make a new FUNCTION_DECL tree node */ > if (!args_to_skip) > new_decl = copy_node (old_decl); > else > new_decl = build_function_decl_skip_args (old_decl, args_to_skip, > false); > > it's no longer a FE created function after all? It is not a FE created function then, sure, but if we don't remove any arguments, at least the issue the patch is fixing isn't a problem there. Jakub
[PATCH] Don't ICE on non-initialized odr_hash during gimple folding (PR ipa/59947)
Hi!
On this testcase the TU is -O0, but a particular function is -O2.
The -O0 means odr_hash isn't created, but during fold_gimple_call
we call possible_polymorphic_call_targets because -fdevirtualize
is true in the current function (as it is -O2) and ICE because
we assume odr_hash can be used safely.
Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux,
ok for trunk?
2014-02-04 Jakub Jelinek
PR ipa/59947
* ipa-devirt.c (possible_polymorphic_call_targets): Fix
a comment typo and formatting issue. If odr_hash hasn't been
created, return vNULL and set *completep to false.
* g++.dg/opt/pr59947.C: New test.
--- gcc/ipa-devirt.c.jj 2014-02-03 08:53:12.0 +0100
+++ gcc/ipa-devirt.c2014-02-03 19:23:01.130257924 +0100
@@ -1276,7 +1276,7 @@ devirt_variable_node_removal_hook (varpo
temporarily change to one of base types. INCLUDE_DERIVER_TYPES make
us to walk the inheritance graph for all derivations.
- If COMPLETEP is non-NULL, store true if the list is complette.
+ If COMPLETEP is non-NULL, store true if the list is complete.
CACHE_TOKEN (if non-NULL) will get stored to an unique ID of entry
in the target cache. If user needs to visit every target list
just once, it can memoize them.
@@ -1295,7 +1295,7 @@ possible_polymorphic_call_targets (tree
static struct cgraph_node_hook_list *node_removal_hook_holder;
pointer_set_t *inserted;
pointer_set_t *matched_vtables;
- vec nodes=vNULL;
+ vec nodes = vNULL;
odr_type type, outer_type;
polymorphic_call_target_d key;
polymorphic_call_target_d **slot;
@@ -1303,6 +1303,13 @@ possible_polymorphic_call_targets (tree
tree binfo, target;
bool final;
+ if (!odr_hash.is_created ())
+{
+ if (completep)
+ *completep = false;
+ return nodes;
+}
+
type = get_odr_type (otr_type, true);
/* Lookup the outer class type we want to walk. */
--- gcc/testsuite/g++.dg/opt/pr59947.C.jj 2014-02-03 19:26:45.041118010
+0100
+++ gcc/testsuite/g++.dg/opt/pr59947.C 2014-02-03 19:26:07.0 +0100
@@ -0,0 +1,34 @@
+// PR ipa/59947
+// { dg-do compile }
+// { dg-options "-O0 -std=c++11" }
+
+#pragma GCC optimize ("O2")
+template
+inline void
+foo (T & a) noexcept { T tmp = static_cast (a); };
+struct A
+{
+ A () noexcept : a (1), b (1) {}
+ virtual void c () noexcept = 0;
+ void d () noexcept { c (); }
+ int a;
+ int b;
+};
+struct B
+{
+ ~B () noexcept { e->d (); }
+ A *e;
+};
+template
+struct C
+{
+ B f;
+};
+struct D {};
+template
+struct E
+{
+ void bar () { foo (g); }
+ C g;
+};
+template class E ;
Jakub
[PATCH] Disallow cloning of __attribute__((optimize (0))) functions (PR ipa/60026)
Hi!
As discussed in the PR, this patch disallows cloning of
non-optimized functions, which doesn't work properly e.g. because
-O0 functions don't have vdef/vuses, but also doesn't sound like
something people would like to happen if they say particular function
shouldn't be optimized. If everything is compiled with -O0, we
don't perform the IPA optimizations at all.
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
2014-02-04 Jakub Jelinek
PR ipa/60026
* tree-inline.c (copy_forbidden): Fail for
__attribute__((optimize (0))) functions.
* c-c++-common/torture/pr60026.c: New test.
--- gcc/tree-inline.c.jj2014-02-01 09:39:59.0 +0100
+++ gcc/tree-inline.c 2014-02-03 19:33:52.146931674 +0100
@@ -3315,6 +3315,18 @@ copy_forbidden (struct function *fun, tr
goto fail;
}
+ tree fs_opts;
+ fs_opts = DECL_FUNCTION_SPECIFIC_OPTIMIZATION (fun->decl);
+ if (fs_opts)
+{
+ struct cl_optimization *os = TREE_OPTIMIZATION (fs_opts);
+ if (!os->x_optimize)
+ {
+ reason = G_("function %q+F compiled without optimizations");
+ goto fail;
+ }
+}
+
fail:
fun->cannot_be_copied_reason = reason;
fun->cannot_be_copied_set = true;
--- gcc/testsuite/c-c++-common/torture/pr60026.c.jj 2014-02-03
18:18:37.395092543 +0100
+++ gcc/testsuite/c-c++-common/torture/pr60026.c2014-02-03
18:18:26.0 +0100
@@ -0,0 +1,28 @@
+/* PR ipa/60026 */
+/* { dg-do compile } */
+
+struct S { int f; } a;
+
+__attribute__((optimize (0)))
+struct S foo (int x, struct S y)
+{
+ int b = y.f;
+ return a;
+}
+
+void
+bar ()
+{
+ while (a.f)
+{
+ struct S c = {0};
+ foo (0, c);
+}
+}
+
+int
+main ()
+{
+ bar ();
+ return 0;
+}
Jakub
[PATCH] Fix masked loads/stores with -fnon-call-exceptions (PR tree-optimization/60023)
Hi!
If a load or store is in EH tables, but doesn't have EH edges out of it,
e.g. for must-not-throw regions, we weren't cleaning it up and adding
MASK_{LOAD,STORE} into EH region instead.
Similarly, the vectorizer needs to add the vectorized stmts to the same
EH region as the corresponding {,masked} load/store (second hunk).
The rest is just making the compiler more robust and not crash if DCE etc.
decides to remove the lhs of MASK_LOAD.
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
P.S.: the patch doesn't fix the old issue that -fnon-call-exceptions
loads are never removed from the vectorized loops, stores aren't actually
removed when transforming the stmt, but later on and handling scalar loads
similarly (turning them into say assignments of zero) didn't look trivial.
2014-02-04 Jakub Jelinek
PR tree-optimization/60023
* tree-if-conv.c (predicate_mem_writes): Pass true instead of
false to gsi_replace.
* tree-vect-stmts.c (vect_finish_stmt_generation): If stmt
has been in some EH region and vec_stmt could throw, add
vec_stmt into the same EH region.
* tree-data-ref.c (get_references_in_stmt): If IFN_MASK_LOAD
has no lhs, ignore it.
* internal-fn.c (expand_MASK_LOAD): Likewise.
* g++.dg/vect/pr60023.cc: New test.
--- gcc/tree-if-conv.c.jj 2014-01-03 11:40:57.0 +0100
+++ gcc/tree-if-conv.c 2014-02-03 16:01:40.250716523 +0100
@@ -1723,7 +1723,7 @@ predicate_mem_writes (loop_p loop)
new_stmt
= gimple_build_call_internal (IFN_MASK_STORE, 4, addr, ptr,
mask, rhs);
- gsi_replace (&gsi, new_stmt, false);
+ gsi_replace (&gsi, new_stmt, true);
}
else if (gimple_vdef (stmt))
{
--- gcc/tree-vect-stmts.c.jj2014-01-16 20:54:59.0 +0100
+++ gcc/tree-vect-stmts.c 2014-02-03 16:29:00.881394627 +0100
@@ -1691,6 +1691,13 @@ vect_finish_stmt_generation (gimple stmt
}
gimple_set_location (vec_stmt, gimple_location (stmt));
+
+ /* While EH edges will generally prevent vectorization, stmt might
+ e.g. be in a must-not-throw region. Ensure newly created stmts
+ that could throw are part of the same region. */
+ int lp_nr = lookup_stmt_eh_lp (stmt);
+ if (lp_nr != 0 && stmt_could_throw_p (vec_stmt))
+add_stmt_to_eh_lp (vec_stmt, lp_nr);
}
/* Checks if CALL can be vectorized in type VECTYPE. Returns
--- gcc/tree-data-ref.c.jj 2014-01-03 11:40:57.0 +0100
+++ gcc/tree-data-ref.c 2014-02-03 16:05:42.111480787 +0100
@@ -4401,6 +4401,8 @@ get_references_in_stmt (gimple stmt, vec
switch (gimple_call_internal_fn (stmt))
{
case IFN_MASK_LOAD:
+ if (gimple_call_lhs (stmt) == NULL_TREE)
+ break;
ref.is_read = true;
case IFN_MASK_STORE:
ref.ref = fold_build2 (MEM_REF,
--- gcc/internal-fn.c.jj2014-02-03 11:48:17.0 +0100
+++ gcc/internal-fn.c 2014-02-03 16:08:55.606492962 +0100
@@ -820,6 +820,8 @@ expand_MASK_LOAD (gimple stmt)
maskt = gimple_call_arg (stmt, 2);
lhs = gimple_call_lhs (stmt);
+ if (lhs == NULL_TREE)
+return;
type = TREE_TYPE (lhs);
rhs = fold_build2 (MEM_REF, type, gimple_call_arg (stmt, 0),
gimple_call_arg (stmt, 1));
--- gcc/testsuite/g++.dg/vect/pr60023.cc.jj 2014-02-03 16:39:17.996271473
+0100
+++ gcc/testsuite/g++.dg/vect/pr60023.cc2014-02-03 16:39:36.455176379
+0100
@@ -0,0 +1,80 @@
+// PR tree-optimization/60023
+// { dg-do compile }
+// { dg-additional-options "-O3 -std=c++11 -fnon-call-exceptions" }
+// { dg-additional-options "-mavx2" { target { i?86-*-* x86_64-*-* } } }
+
+struct A { A (); ~A (); };
+
+void
+f1 (int *p, int *q, int *r) noexcept (true)
+{
+ int i;
+ for (i = 0; i < 1024; i++)
+if (r[i])
+ p[i] = q[i] + 1;
+}
+
+void
+f2 (int *p, int *q, int *r)
+{
+ int i;
+ for (i = 0; i < 1024; i++)
+if (r[i])
+ p[i] = q[i] + 1;
+}
+
+void
+f3 (int *p, int *q) noexcept (true)
+{
+ int i;
+ for (i = 0; i < 1024; i++)
+p[i] = q[i] + 1;
+}
+
+void
+f4 (int *p, int *q)
+{
+ int i;
+ for (i = 0; i < 1024; i++)
+p[i] = q[i] + 1;
+}
+
+void
+f5 (int *p, int *q, int *r) noexcept (true)
+{
+ int i;
+ A a;
+ for (i = 0; i < 1024; i++)
+if (r[i])
+ p[i] = q[i] + 1;
+}
+
+void
+f6 (int *p, int *q, int *r)
+{
+ int i;
+ A a;
+ for (i = 0; i < 1024; i++)
+if (r[i])
+ p[i] = q[i] + 1;
+}
+
+void
+f7 (int *p, int *q) noexcept (true)
+{
+ int i;
+ A a;
+ for (i = 0; i < 1024; i++)
+p[i] = q[i] + 1;
+}
+
+void
+f8 (int *p, int *q)
+{
+ int i;
+ A a;
+ for (i = 0; i < 1024; i++)
+p[i] = q[i] + 1;
+}
+
+// { dg-final { cleanup-tree-dump "vect" } }
Jakub
Re: [PATCH] Clear DECL_LANG_SPECIFIC after changing arguments (PR tree-optimization/60002)
On Tue, 4 Feb 2014, Jakub Jelinek wrote:
> Hi!
>
> As described in the PR, while build_function_decl_skip_args
> takes care of changing the type from METHOD_TYPE to FUNCTION_TYPE
> if removing first argument (this), the FE may have other special arguments
> that it is aware of (such as in-charge etc.) and if the this argument
> is not removed, but the other arguments are, then the FE can crash
> upon seeing such methods.
>
> Fixed by clearing DECL_LANG_SPECIFIC if we remove any arguments.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
Ok, but shouldn't we do that in the copy_node case as well? Two
places look like
/* Make a new FUNCTION_DECL tree node */
if (!args_to_skip)
new_decl = copy_node (old_decl);
else
new_decl = build_function_decl_skip_args (old_decl, args_to_skip,
false);
it's no longer a FE created function after all?
Richard.
> 2014-02-04 Jakub Jelinek
>
> PR tree-optimization/60002
> * cgraphclones.c (build_function_decl_skip_args): Clear
> DECL_LANG_SPECIFIC.
>
> * g++.dg/opt/pr60002.C: New test.
>
> --- gcc/cgraphclones.c.jj 2014-01-31 22:21:42.911750272 +0100
> +++ gcc/cgraphclones.c2014-02-03 19:43:30.678941701 +0100
> @@ -390,6 +390,9 @@ build_function_decl_skip_args (tree orig
>DECL_BUILT_IN_CLASS (new_decl) = NOT_BUILT_IN;
>DECL_FUNCTION_CODE (new_decl) = (enum built_in_function) 0;
> }
> + /* The FE might have information and assumptions about the other
> + arguments. */
> + DECL_LANG_SPECIFIC (new_decl) = NULL;
>return new_decl;
> }
>
> --- gcc/testsuite/g++.dg/opt/pr60002.C.jj 2014-02-03 19:43:47.671856267
> +0100
> +++ gcc/testsuite/g++.dg/opt/pr60002.C2014-02-03 19:43:00.0
> +0100
> @@ -0,0 +1,12 @@
> +// PR tree-optimization/60002
> +// { dg-do compile }
> +// { dg-options "-O2 -fcompare-debug -fdeclone-ctor-dtor -fipa-cp-clone" }
> +
> +struct A {};
> +
> +struct B : virtual A { B (); ~B (); };
> +
> +B::~B ()
> +{
> + B ();
> +}
>
> Jakub
>
>
--
Richard Biener
SUSE / SUSE Labs
SUSE LINUX Products GmbH - Nuernberg - AG Nuernberg - HRB 16746
GF: Jeff Hawn, Jennifer Guild, Felix Imend"orffer
[C++ PATCH] Restore declarator_obstack in cp_parser_omp_declare_reduction (PR c++/58703)
Hi!
Apparently at least on invalid #pragma omp declare reduction,
when parsing the type-ids, the parser may allocate some declarators
of the declarator_obstack, which results in ICE at the end of translation
unit when declarator_obstack is not in the initial empty state.
Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux,
ok for trunk?
2014-02-04 Jakub Jelinek
PR c++/58703
* parser.c (cp_parser_omp_declare_reduction): Save and free
declarator_obstack.
* c-c++-common/gomp/pr58703.c: New test.
--- gcc/cp/parser.c.jj 2014-02-03 08:53:11.0 +0100
+++ gcc/cp/parser.c 2014-02-03 14:00:58.569308322 +0100
@@ -30623,6 +30623,10 @@ cp_parser_omp_declare_reduction (cp_pars
cp_token *first_token;
cp_token_cache *cp;
int errs;
+ void *p;
+
+ /* Get the high-water mark for the DECLARATOR_OBSTACK. */
+ p = obstack_alloc (&declarator_obstack, 0);
if (!cp_parser_require (parser, CPP_OPEN_PAREN, RT_OPEN_PAREN))
goto fail;
@@ -30731,6 +30735,9 @@ cp_parser_omp_declare_reduction (cp_pars
{
fail:
cp_parser_skip_to_pragma_eol (parser, pragma_tok);
+
+ /* Free any declarators allocated. */
+ obstack_free (&declarator_obstack, p);
return;
}
@@ -30835,6 +30842,9 @@ cp_parser_omp_declare_reduction (cp_pars
}
cp_parser_require_pragma_eol (parser, pragma_tok);
+
+ /* Free any declarators allocated. */
+ obstack_free (&declarator_obstack, p);
}
/* OpenMP 4.0
--- gcc/testsuite/c-c++-common/gomp/pr58703.c.jj2014-02-03
14:02:56.735693978 +0100
+++ gcc/testsuite/c-c++-common/gomp/pr58703.c 2014-02-03 14:02:37.0
+0100
@@ -0,0 +1,6 @@
+/* PR c++/58703 */
+/* { dg-do compile } */
+/* { dg-options "-fopenmp" } */
+
+#pragma omp declare reduction (+ : char[] : omp_out += omp_in) /* { dg-error
"function or array type" } */
+#pragma omp declare reduction (+ : char() : omp_out += omp_in) /* { dg-error
"function or array type" } */
Jakub
[PATCH] Fix ubsan expansion of multiplication (PR rtl-optimization/60030)
Hi! ubsan/overflow-1.c test ICEs on ppc32, because lopart is SImode, and we attempt to do a DImode left shift on it by 32. We don't really care about the upper bits, those are all shifted away anyway, so this patch fixes it by adding a paradoxical SUBREG, which results in better generated code compared to e.g. adding there a ZERO_EXTEND. Bootstrapped/regtested on x86_64-linux and i686-linux, additionally tested with ubsan tests on x86_64 and i686 with the add/sub/mul/neg with overflow patterns disabled and tested using ppc* cross on overflow-1.c. Ok for trunk? 2014-02-04 Jakub Jelinek PR rtl-optimization/60030 * internal-fn.c (ubsan_expand_si_overflow_mul_check): Surround lopart with paradoxical subreg before shifting it up by hprec. --- gcc/internal-fn.c.jj2014-01-29 12:43:24.0 +0100 +++ gcc/internal-fn.c 2014-02-03 10:40:57.0 +0100 @@ -646,7 +646,8 @@ ubsan_expand_si_overflow_mul_check (gimp emit_cmp_and_jump_insns (hipart, const0_rtx, GE, NULL_RTX, hmode, false, after_hipart_neg, PROB_EVEN); - tem = expand_shift (LSHIFT_EXPR, mode, lopart, hprec, NULL_RTX, 1); + tem = gen_rtx_SUBREG (mode, lopart, 0); + tem = expand_shift (LSHIFT_EXPR, mode, tem, hprec, NULL_RTX, 1); tem = expand_simple_binop (mode, MINUS, loxhi, tem, NULL_RTX, 1, OPTAB_DIRECT); emit_move_insn (loxhi, tem); Jakub
[PATCH] Clear DECL_LANG_SPECIFIC after changing arguments (PR tree-optimization/60002)
Hi!
As described in the PR, while build_function_decl_skip_args
takes care of changing the type from METHOD_TYPE to FUNCTION_TYPE
if removing first argument (this), the FE may have other special arguments
that it is aware of (such as in-charge etc.) and if the this argument
is not removed, but the other arguments are, then the FE can crash
upon seeing such methods.
Fixed by clearing DECL_LANG_SPECIFIC if we remove any arguments.
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
2014-02-04 Jakub Jelinek
PR tree-optimization/60002
* cgraphclones.c (build_function_decl_skip_args): Clear
DECL_LANG_SPECIFIC.
* g++.dg/opt/pr60002.C: New test.
--- gcc/cgraphclones.c.jj 2014-01-31 22:21:42.911750272 +0100
+++ gcc/cgraphclones.c 2014-02-03 19:43:30.678941701 +0100
@@ -390,6 +390,9 @@ build_function_decl_skip_args (tree orig
DECL_BUILT_IN_CLASS (new_decl) = NOT_BUILT_IN;
DECL_FUNCTION_CODE (new_decl) = (enum built_in_function) 0;
}
+ /* The FE might have information and assumptions about the other
+ arguments. */
+ DECL_LANG_SPECIFIC (new_decl) = NULL;
return new_decl;
}
--- gcc/testsuite/g++.dg/opt/pr60002.C.jj 2014-02-03 19:43:47.671856267
+0100
+++ gcc/testsuite/g++.dg/opt/pr60002.C 2014-02-03 19:43:00.0 +0100
@@ -0,0 +1,12 @@
+// PR tree-optimization/60002
+// { dg-do compile }
+// { dg-options "-O2 -fcompare-debug -fdeclone-ctor-dtor -fipa-cp-clone" }
+
+struct A {};
+
+struct B : virtual A { B (); ~B (); };
+
+B::~B ()
+{
+ B ();
+}
Jakub
[committed] Fix --enable-checking=valgrind reported FAIL on simd-clones-2.c (PR other/58712)
Hi!
We shouldn't copy the inbranch artificial argument. Also,
if clone_info->inbranch, then inbranch_specified is necessarily true,
thus (i & 1) will be always 0 and in that case we still need room for the
extra argument.
Committed to trunk.
2014-02-04 Jakub Jelinek
PR other/58712
* omp-low.c (simd_clone_struct_copy): If from->inbranch
is set, copy one less argument.
(expand_simd_clones): Don't subtract clone_info->inbranch
from simd_clone_struct_alloc argument.
--- gcc/omp-low.c.jj2014-01-29 12:43:24.0 +0100
+++ gcc/omp-low.c 2014-02-03 17:46:40.667922617 +0100
@@ -10660,7 +10660,8 @@ simd_clone_struct_copy (struct cgraph_si
struct cgraph_simd_clone *from)
{
memcpy (to, from, (sizeof (struct cgraph_simd_clone)
-+ from->nargs * sizeof (struct cgraph_simd_clone_arg)));
++ ((from->nargs - from->inbranch)
+ * sizeof (struct cgraph_simd_clone_arg;
}
/* Return vector of parameter types of function FNDECL. This uses
@@ -11694,7 +11695,6 @@ expand_simd_clones (struct cgraph_node *
if (i != 0)
{
clone = simd_clone_struct_alloc (clone_info->nargs
- - clone_info->inbranch
+ ((i & 1) != 0));
simd_clone_struct_copy (clone, clone_info);
/* Undo changes targetm.simd_clone.compute_vecsize_and_simdlen
Jakub
Re: var-tracking and s390's LM(G)
Jakub Jelinek writes: >> But then we wouldn't be able to use var-tracking when __builtin_eh_return >> is used, since in that case replacing the (set (reg ...) (mem ...)) >> with a (plus ...) would be incorrect -- the value we're loading from the >> stack will have had a variable adjustment applied. And I know from painful >> experience that being able to debug the unwind code is very useful. :-) > > Aren't functions using EH_RETURN typically using frame pointer? Sorry, forgot to respond to this bit. On s390, _Unwind_RaiseException and _Unwind_ForcedUnwind don't use a frame pointer, at least not on trunk. %r11 is used as a general scratch register instead. E.g.: 2ba8 <_Unwind_ForcedUnwind>: 2ba8: 90 6f f0 18 stm %r6,%r15,24(%r15) 2bac: 0d d0 basr%r13,%r0 2bae: 60 40 f0 50 std %f4,80(%r15) 2bb2: 60 60 f0 58 std %f6,88(%r15) 2bb6: a7 fa fd f8 ahi %r15,-520 2bba: 58 c0 d0 9e l %r12,158(%r13) 2bbe: 58 10 d0 9a l %r1,154(%r13) 2bc2: 18 b2 lr %r11,%r2 ... 2c10: 98 6f f2 20 lm %r6,%r15,544(%r15) 2c14: 07 f4 br %r4 Thanks, Richard
Re: var-tracking and s390's LM(G)
Jakub Jelinek writes: > On Tue, Feb 04, 2014 at 11:33:57AM +, Richard Sandiford wrote: >> > So, how does the lmg insn look like in RTL dump on some problematic >> > testcase? >> > insn_stack_adjust_offset_pre_post already uses REG_FRAME_RELATED_EXPR, >> > which is also a kind of CFI note (the oldest one), so likely the issue >> > is just that it hasn't been adjusted to handle other newer REG_CFA_* notes >> > that tell how the stack pointer is adjusted. >> >> It's just a (mem ...) access: >> >> (parallel >>[... >> (set (reg %r14) (mem:[SD]I (plus (reg ...) (const_int X1 >> (set (reg %r15) (mem:[SD]I (plus (reg ...) (const_int X2]) > > I meant what reg notes it has (and why it doesn't use > REG_FRAME_RELATED_EXPR). It has a REG_CFA_RESTORE for each loaded register. It also has a REG_CFA_DEF_CFA with (plus (sp) (const_int ...)) in lieu of a REG_FRAME_RELATED_EXPR of the form (set (sp) (plus (...) (const_int ...))), which like I say wouldn't always be correct. And I think that's a valid use of REG_CFA_DEF_CFA: /* Attached to insns that are RTX_FRAME_RELATED_P, but are too complex for FRAME_RELATED_EXPR intuition. ... */ REG_NOTE (CFA_DEF_CFA) (although the assignment to %r15 isn't first in the parallel, despite the snipped part of the comment implying that it should be. I don't understand why we'd require that though.) FWIW this form comes from: 2009-06-04 Jakub Jelinek * config/s390/s390.c (global_not_special_regno_p): New static inline. (save_gprs): Don't tell unwinder when a global register is saved. (s390_emit_epilogue): Emit needed epilogue unwind info. >> >> The simplest fix seems to be to disable this check for the exit block. >> >> We never use its stack_adjust anyway, and dwarf2cfi already checks >> >> (using CFA information) that the offsets in a shrink-wrapped function >> >> are consistent. >> >> >> >> Tested on s390-linux-gnu and s390x-linux-gnu. OK to install? >> > >> > I don't like this, my strong preference is to handle REG_CFA_* notes. >> >> But then we wouldn't be able to use var-tracking when __builtin_eh_return >> is used, since in that case replacing the (set (reg ...) (mem ...)) >> with a (plus ...) would be incorrect -- the value we're loading from the >> stack will have had a variable adjustment applied. And I know from painful >> experience that being able to debug the unwind code is very useful. :-) > > Aren't functions using EH_RETURN typically using frame pointer? > And, var-tracking disabling doesn't really mean no debug info, just worse > debug info. IMHO the sanity check in var-tracking is worth much more than > var-tracking in unwind-dw2.o in the case where you wouldn't use frame > pointer. Why doesn't dwarf2cfi ICE on it then when the CFA changes can't be > described properly? Because the CFA information (as provided by REG_CFA... notes) is correct. It's just that var-tracking doesn't use those notes. FWIW, using them was one of the options I mentioned in the original mail. I don't see why you think relaxing this sanity check for the exit block is so bad though. If your argument is that var-tracking must understand every assignment to the stack pointer then I think it should bail out whenever it sees an assignment to sp that it doesn't understand, rather than silently ignoring it. Thanks, Richard
[C++ Patch/RFC] PR 60047
Hi,
thus I tried to have a look to this issue, while experiencing some weird
problems with the debugger, which slowed me down a lot. I have been able
to figure out that we don't seem to actively set constexpr_p to true
anywhere in implicitly_declare_fn (besides the unrelated case of
inheriting constructors), thus I'm wondering if it would make sense to
simply do the below?!? (and revert my recent tweak)
Otherwise, Jason, if you suspect something deeper is going on, I would
rather ask you to take over, today the debugger is still misbehaving on
me :(
Thanks!
Paolo.
//
Index: cp/method.c
===
--- cp/method.c (revision 207457)
+++ cp/method.c (working copy)
@@ -1366,7 +1366,7 @@ synthesized_method_walk (tree ctype, special_funct
}
vbases = CLASSTYPE_VBASECLASSES (ctype);
- if (vec_safe_is_empty (vbases))
+ if (vbases == NULL)
/* No virtual bases to worry about. */;
else if (!assign_p)
{
@@ -1660,7 +1660,7 @@ implicitly_declare_fn (special_function_kind kind,
else if (trivial_p && cxx_dialect >= cxx11
&& (kind == sfk_copy_constructor
|| kind == sfk_move_constructor))
-gcc_assert (constexpr_p);
+constexpr_p = true;
if (!trivial_p && type_has_trivial_fn (type, kind))
type_set_nontrivial_flag (type, kind);
Index: testsuite/g++.dg/cpp0x/pr60047.C
===
--- testsuite/g++.dg/cpp0x/pr60047.C(revision 0)
+++ testsuite/g++.dg/cpp0x/pr60047.C(working copy)
@@ -0,0 +1,14 @@
+// PR c++/60047
+// { dg-do compile { target c++11 } }
+
+struct B { };
+
+template struct A : virtual B
+{
+ A();
+ A(const A&);
+};
+
+template A::A(const A&) = default;
+
+A a = A();
Re: var-tracking and s390's LM(G)
On Tue, Feb 04, 2014 at 11:33:57AM +, Richard Sandiford wrote: > > So, how does the lmg insn look like in RTL dump on some problematic > > testcase? > > insn_stack_adjust_offset_pre_post already uses REG_FRAME_RELATED_EXPR, > > which is also a kind of CFI note (the oldest one), so likely the issue > > is just that it hasn't been adjusted to handle other newer REG_CFA_* notes > > that tell how the stack pointer is adjusted. > > It's just a (mem ...) access: > > (parallel >[... > (set (reg %r14) (mem:[SD]I (plus (reg ...) (const_int X1 > (set (reg %r15) (mem:[SD]I (plus (reg ...) (const_int X2]) I meant what reg notes it has (and why it doesn't use REG_FRAME_RELATED_EXPR). > >> The simplest fix seems to be to disable this check for the exit block. > >> We never use its stack_adjust anyway, and dwarf2cfi already checks > >> (using CFA information) that the offsets in a shrink-wrapped function > >> are consistent. > >> > >> Tested on s390-linux-gnu and s390x-linux-gnu. OK to install? > > > > I don't like this, my strong preference is to handle REG_CFA_* notes. > > But then we wouldn't be able to use var-tracking when __builtin_eh_return > is used, since in that case replacing the (set (reg ...) (mem ...)) > with a (plus ...) would be incorrect -- the value we're loading from the > stack will have had a variable adjustment applied. And I know from painful > experience that being able to debug the unwind code is very useful. :-) Aren't functions using EH_RETURN typically using frame pointer? And, var-tracking disabling doesn't really mean no debug info, just worse debug info. IMHO the sanity check in var-tracking is worth much more than var-tracking in unwind-dw2.o in the case where you wouldn't use frame pointer. Why doesn't dwarf2cfi ICE on it then when the CFA changes can't be described properly? Jakub
Re: [C,C++] integer constants in attribute arguments
Marc Glisse writes:
> Index: gcc/testsuite/g++.dg/cpp0x/constexpr-attribute2.C
> ===
> --- gcc/testsuite/g++.dg/cpp0x/constexpr-attribute2.C (revision 0)
> +++ gcc/testsuite/g++.dg/cpp0x/constexpr-attribute2.C (working copy)
> @@ -0,0 +1,32 @@
> +// { dg-options -std=gnu++11 }
> +
> +struct t { t(); };
> +
> +constexpr int prio = 123;
> +constexpr int size = 8;
> +constexpr int pos = 1;
> +enum A { zero = 0, one, two };
> +__attribute__((init_priority(prio))) t a;
> +
> +enum class E1 : int {
> +first = 101,
> +second,
> +third,
> +};
> +__attribute__((init_priority(E1::second))) t b; // Should not compile?
> +
> +enum E2 {
> +E2_first = 141,
> +E2_second,
> +E2_third,
> +};
> +__attribute__((init_priority(E2_second))) t c;
> +
> +void* my_calloc(unsigned, unsigned) __attribute__((alloc_size(pos,two)));
> +void* my_realloc(void*, unsigned) __attribute__((alloc_size(two)));
> +
> +typedef char vec __attribute__((vector_size(size)));
> +
> +void f(char*) __attribute__((nonnull(pos)));
> +
> +void g() __attribute__((aligned(size)));
/usr/local/gcc/gcc-20140204/gcc/testsuite/g++.dg/cpp0x/constexpr-attribute2.C:32:6:
error: alignment for 'void g()' must be at least 16
FAIL: g++.dg/cpp0x/constexpr-attribute2.C (test for excess errors)
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: var-tracking and s390's LM(G)
Jakub Jelinek writes: > On Tue, Feb 04, 2014 at 10:47:49AM +, Richard Sandiford wrote: >> I wondered whether we could model the load of the stack pointer as an >> addition in cases where that is safe (e.g. it would require !calls_eh_return >> at least). But that would only work in functions that don't need a frame >> pointer. In functions that do need a frame pointer we'd presumably have >> (plus (hfp) (const_int X)) instead, which stack_adjust_offset_pre_post >> would again not understand. > > But vt_stack_adjustments is only called if frame pointer isn't used. Sorry, missed that. >> Another option would be to work out the offset from REG_CFA_DEF_CFA notes, >> where present, but I'm a bit uncomfortable with the idea of mixing two >> different methods of calculating the stack offset. > > So, how does the lmg insn look like in RTL dump on some problematic > testcase? > insn_stack_adjust_offset_pre_post already uses REG_FRAME_RELATED_EXPR, > which is also a kind of CFI note (the oldest one), so likely the issue > is just that it hasn't been adjusted to handle other newer REG_CFA_* notes > that tell how the stack pointer is adjusted. It's just a (mem ...) access: (parallel [... (set (reg %r14) (mem:[SD]I (plus (reg ...) (const_int X1 (set (reg %r15) (mem:[SD]I (plus (reg ...) (const_int X2]) >> The simplest fix seems to be to disable this check for the exit block. >> We never use its stack_adjust anyway, and dwarf2cfi already checks >> (using CFA information) that the offsets in a shrink-wrapped function >> are consistent. >> >> Tested on s390-linux-gnu and s390x-linux-gnu. OK to install? > > I don't like this, my strong preference is to handle REG_CFA_* notes. But then we wouldn't be able to use var-tracking when __builtin_eh_return is used, since in that case replacing the (set (reg ...) (mem ...)) with a (plus ...) would be incorrect -- the value we're loading from the stack will have had a variable adjustment applied. And I know from painful experience that being able to debug the unwind code is very useful. :-) Thanks, Richard
[s390] Add return and simple_return patterns
This patch adds return and simple_return patterns to the s390 backend,
which eanbles shrink-wrapping and conditional returns to be used.
Perhaps the only subtle thing is the handling of call-clobbered base
registers. The idea is to emit the initialising main_pool pattern in
both early_mach -- at the very beginning of the function -- and in the
prologue. Then, if shrink-wrapping is used, the one added by early_mach
will still be the first in the function. If shrink-wrapping isn't used
then the one added by the prologue will be the first in the function.
s390_mainpool_start then deletes whichever isn't needed.
Tested in the same way as the previous patches. OK to install?
Thanks,
Richard
gcc/
* config/s390/s390-protos.h (s390_can_use_simple_return_insn)
(s390_can_use_return_insn): Declare.
* config/s390/s390.h (EPILOGUE_USES): Define.
* config/s390/s390.c (s390_mainpool_start): Allow two main_pool
instructions.
(s390_chunkify_start): Handle return JUMP_LABELs.
(s390_early_mach): Emit a main_pool instruction on the entry edge.
(s300_set_up_by_prologue, s390_can_use_simple_return_insn)
(s390_can_use_return_insn): New functions.
(s390_fix_long_loop_prediction): Handle conditional returns.
(TARGET_SET_UP_BY_PROLOGUE): Define.
* config/s390/s390.md (ANY_RETURN): New code iterator.
(*creturn, *csimple_return, return, simple_return): New patterns.
Index: gcc/config/s390/s390-protos.h
===
--- gcc/config/s390/s390-protos.h 2014-01-31 17:20:20.520231908 +
+++ gcc/config/s390/s390-protos.h 2014-01-31 17:20:36.126182141 +
@@ -36,6 +36,8 @@ extern bool s390_check_symref_alignment
extern HOST_WIDE_INT s390_initial_elimination_offset (int, int);
extern void s390_emit_prologue (void);
extern void s390_emit_epilogue (bool);
+extern bool s390_can_use_simple_return_insn (void);
+extern bool s390_can_use_return_insn (void);
extern void s390_function_profiler (FILE *, int);
extern void s390_set_has_landing_pad_p (bool);
extern bool s390_hard_regno_mode_ok (unsigned int, enum machine_mode);
Index: gcc/config/s390/s390.h
===
--- gcc/config/s390/s390.h 2014-01-31 17:20:20.520231908 +
+++ gcc/config/s390/s390.h 2014-01-31 17:20:36.129182131 +
@@ -878,6 +878,10 @@ #define ASM_OUTPUT_ADDR_DIFF_ELT(FILE, B
fputc ('\n', (FILE));
\
} while (0)
+/* Mark the return register as used by the epilogue so that we can
+ use it in unadorned (return) and (simple_return) instructions. */
+#define EPILOGUE_USES(REGNO) ((REGNO) == RETURN_REGNUM)
+
#undef ASM_OUTPUT_FUNCTION_LABEL
#define ASM_OUTPUT_FUNCTION_LABEL(FILE, NAME, DECL) \
s390_asm_output_function_label (FILE, NAME, DECL)
Index: gcc/config/s390/s390.c
===
--- gcc/config/s390/s390.c 2014-01-31 17:20:35.405184447 +
+++ gcc/config/s390/s390.c 2014-01-31 17:20:36.129182131 +
@@ -,7 +,15 @@ s390_mainpool_start (void)
&& GET_CODE (SET_SRC (PATTERN (insn))) == UNSPEC_VOLATILE
&& XINT (SET_SRC (PATTERN (insn)), 1) == UNSPECV_MAIN_POOL)
{
- gcc_assert (!pool->pool_insn);
+ /* There might be two main_pool instructions if base_reg
+is call-clobbered; one for shrink-wrapped code and one
+for the rest. We want to keep the first. */
+ if (pool->pool_insn)
+ {
+ insn = PREV_INSN (insn);
+ delete_insn (NEXT_INSN (insn));
+ continue;
+ }
pool->pool_insn = insn;
}
@@ -7103,7 +7111,7 @@ s390_chunkify_start (void)
if (GET_CODE (pat) == SET)
{
rtx label = JUMP_LABEL (insn);
- if (label)
+ if (label && !ANY_RETURN_P (label))
{
if (s390_find_pool (pool_list, label)
!= s390_find_pool (pool_list, insn))
@@ -8628,6 +8636,11 @@ s390_early_mach (void)
/* Re-compute register info. */
s390_register_info ();
+ /* If we're using a base register, ensure that it is always valid for
+ the first non-prologue instruction. */
+ if (cfun->machine->base_reg)
+emit_insn_at_entry (gen_main_pool (cfun->machine->base_reg));
+
/* Annotate all constant pool references to let the scheduler know
they implicitly use the base register. */
for (insn = get_insns (); insn; insn = NEXT_INSN (insn))
@@ -9163,6 +9176,55 @@ s390_emit_epilogue (bool sibcall)
}
}
+/* Implement TARGET_SET_UP_BY_PROLOGUE. */
+
+static void
+s300_set_up_by_prologue (hard_reg_set_container *regs)
+{
+ if (cfun->machine->base_reg
+ && !call_really_used_regs[REGNO (cfun->machine->base_reg)])
+SET_HARD_REG_BIT (regs->s
[s390] Fix some epilogue CFA notes
This patch fixes the CFA notes used when an epilogue restores a GPR from an FPR. It also makes sure that s390_optimize_prologue preserves the CFA information. Tested in the same way as the previous patch. OK to install? Thanks, Richard gcc/ * config/s390/s390.c (s390_restore_gprs_from_fprs): Add REG_CFA_RESTORE notes to each restore. Also add REG_CFA_DEF_CFA when restoring %r15. (s390_optimize_prologue): Don't clear RTX_FRAME_RELATED_P. Update the REG_CFA_RESTORE list when deciding not to restore a register. Index: gcc/config/s390/s390.c === --- gcc/config/s390/s390.c 2014-01-31 17:20:31.582196663 + +++ gcc/config/s390/s390.c 2014-02-04 11:15:41.085329442 + @@ -8603,11 +8603,11 @@ s390_restore_gprs_from_fprs (void) emit_move_insn (gen_rtx_REG (DImode, i), gen_rtx_REG (DImode, cfun_gpr_save_slot (i))); df_set_regs_ever_live (i, true); - /* The frame related flag is only required on the save -operations. We nevertheless set it also for the restore -in order to recognize these instructions in -s390_optimize_prologue. The flag will then be -deleted. */ + add_reg_note (insn, REG_CFA_RESTORE, gen_rtx_REG (DImode, i)); + if (i == STACK_POINTER_REGNUM) + add_reg_note (insn, REG_CFA_DEF_CFA, + plus_constant (Pmode, stack_pointer_rtx, +STACK_POINTER_OFFSET)); RTX_FRAME_RELATED_P (insn) = 1; } } @@ -10847,12 +10847,6 @@ s390_optimize_prologue (void) || call_really_used_regs[gpr_regno]) continue; - /* For restores we have to revert the frame related flag -since no debug info is supposed to be generated for -these. */ - if (dest_regno == gpr_regno) - RTX_FRAME_RELATED_P (insn) = 0; - /* It must not happen that what we once saved in an FPR now needs a stack slot. */ gcc_assert (cfun_gpr_save_slot (gpr_regno) != -1); @@ -10935,8 +10929,6 @@ s390_optimize_prologue (void) if (GET_CODE (base) != REG || off < 0) continue; - RTX_FRAME_RELATED_P (insn) = 0; - if (cfun_frame_layout.first_restore_gpr != -1 && (cfun_frame_layout.first_restore_gpr < first || cfun_frame_layout.last_restore_gpr > last)) @@ -10954,8 +10946,19 @@ s390_optimize_prologue (void) - first) * UNITS_PER_LONG, cfun_frame_layout.first_restore_gpr, cfun_frame_layout.last_restore_gpr); - RTX_FRAME_RELATED_P (new_insn) = 0; + + /* Remove REG_CFA_RESTOREs for registers that we no +longer need to save. */ + REG_NOTES (new_insn) = REG_NOTES (insn); + for (rtx *ptr = ®_NOTES (new_insn); *ptr; ) + if (REG_NOTE_KIND (*ptr) == REG_CFA_RESTORE + && ((int) REGNO (XEXP (*ptr, 0)) + < cfun_frame_layout.first_restore_gpr)) + *ptr = XEXP (*ptr, 1); + else + ptr = &XEXP (*ptr, 1); new_insn = emit_insn_before (new_insn, insn); + RTX_FRAME_RELATED_P (new_insn) = 1; INSN_ADDRESSES_NEW (new_insn, -1); } @@ -10977,8 +10980,6 @@ s390_optimize_prologue (void) if (GET_CODE (base) != REG || off < 0) continue; - RTX_FRAME_RELATED_P (insn) = 0; - if (REGNO (base) != STACK_POINTER_REGNUM && REGNO (base) != HARD_FRAME_POINTER_REGNUM) continue;
Re: PR59723: fix LTO + fortran namelist ICEs
On Tue, 4 Feb 2014, Richard Biener wrote:
> On Tue, 4 Feb 2014, Dominique Dhumieres wrote:
>
> > Richard,
> >
> > With your patch at http://gcc.gnu.org/ml/gcc-patches/2014-02/msg00144.html
> > I get multiple ICEs when testing with
> > check-gfortran RUNTESTFLAGS="--target_board=unix'{-m32/-flto,-m64/-flto}'"
> >
> > They are of the kind
> >
> > lto1: internal compiler error: in mentions_vars_p, at lto/lto.c:972
> >
> > Note that the Aldy's patch at
> > http://gcc.gnu.org/ml/gcc-patches/2014-01/msg01384.html
> > fixed all the ICE in the same tests.
>
> Yep, I'm collecting more fixes - see below for what I have. Aldy's
> fix is simply wrong (as is the namelist-decl-ref handling in the LTO
> code).
>
> With the patch below we can still hit an ICE in lto_fixup_prevailing_decls
> as we seem to arrive with a CONSTRUCTOR here. I'm waiting for Micha
> to show up to tell me if that's simply because CONSTRUCTOR isn't handled
> in the code (yet) or if there is a deeper reason.
That would be additionally
Index: gcc/lto/lto.c
===
*** gcc/lto/lto.c (revision 207455)
--- gcc/lto/lto.c (working copy)
*** lto_fixup_prevailing_decls (tree t)
*** 2597,2603
enum tree_code code = TREE_CODE (t);
bool fixed = false;
! gcc_checking_assert (code != CONSTRUCTOR && code != TREE_BINFO);
LTO_NO_PREVAIL (TREE_TYPE (t));
if (CODE_CONTAINS_STRUCT (code, TS_COMMON))
LTO_NO_PREVAIL (TREE_CHAIN (t));
--- 2598,2604
enum tree_code code = TREE_CODE (t);
bool fixed = false;
! gcc_checking_assert (code != TREE_BINFO);
LTO_NO_PREVAIL (TREE_TYPE (t));
if (CODE_CONTAINS_STRUCT (code, TS_COMMON))
LTO_NO_PREVAIL (TREE_CHAIN (t));
*** lto_fixup_prevailing_decls (tree t)
*** 2659,2664
--- 2660,2672
for (i = TREE_OPERAND_LENGTH (t) - 1; i >= 0; --i)
LTO_SET_PREVAIL (TREE_OPERAND (t, i));
}
+ else if (TREE_CODE (t) == CONSTRUCTOR)
+ {
+ int i;
+ tree val;
+ FOR_EACH_CONSTRUCTOR_VALUE (CONSTRUCTOR_ELTS (t), i, val)
+ LTO_SET_PREVAIL (val);
+ }
else
{
switch (code)
> Richard.
>
> 2014-02-04 Richard Biener
>
> PR lto/59723
> * lto-streamer-out.c (lto_output_tree_ref): Do not write
> trees from lto_output_tree_ref.
> * lto-streamer-in.c (lto_input_tree_ref): Handle LTO_namelist_decl_ref
> similar to LTO_imported_decl_ref.
>
> lto/
> * lto.c (mentions_vars_p) Handle NAMELIST_DECL.
>
> Index: gcc/lto-streamer-out.c
> ===
> *** gcc/lto-streamer-out.c(revision 207455)
> --- gcc/lto-streamer-out.c(working copy)
> *** lto_output_tree_ref (struct output_block
> *** 255,273
> break;
>
> case NAMELIST_DECL:
> ! {
> ! unsigned i;
> ! tree value, tmp;
> !
> ! streamer_write_record_start (ob, LTO_namelist_decl_ref);
> ! stream_write_tree (ob, DECL_NAME (expr), true);
> ! tmp = NAMELIST_DECL_ASSOCIATED_DECL (expr);
> ! gcc_assert (tmp != NULL_TREE);
> ! streamer_write_uhwi (ob, CONSTRUCTOR_ELTS (tmp)->length());
> ! FOR_EACH_CONSTRUCTOR_VALUE (CONSTRUCTOR_ELTS (tmp), i, value)
> ! lto_output_var_decl_index (ob->decl_state, ob->main_stream, value);
> ! break;
> ! }
>
> case NAMESPACE_DECL:
> streamer_write_record_start (ob, LTO_namespace_decl_ref);
> --- 261,269
> break;
>
> case NAMELIST_DECL:
> ! streamer_write_record_start (ob, LTO_namelist_decl_ref);
> ! lto_output_var_decl_index (ob->decl_state, ob->main_stream, expr);
> ! break;
>
> case NAMESPACE_DECL:
> streamer_write_record_start (ob, LTO_namespace_decl_ref);
>
> Index: gcc/lto/lto.c
> ===
> *** gcc/lto/lto.c (revision 207455)
> --- gcc/lto/lto.c (working copy)
> *** mentions_vars_p (tree t)
> *** 926,931
> --- 926,932
> case RESULT_DECL:
> case IMPORTED_DECL:
> case NAMESPACE_DECL:
> + case NAMELIST_DECL:
> return mentions_vars_p_decl_common (t);
>
> case VAR_DECL:
> Index: gcc/lto-streamer-in.c
> ===
> *** gcc/lto-streamer-in.c (revision 207455)
> --- gcc/lto-streamer-in.c (working copy)
> *** lto_input_tree_ref (struct lto_input_blo
> *** 244,275
> case LTO_imported_decl_ref:
> case LTO_label_decl_ref:
> case LTO_translation_unit_decl_ref:
> ix_u = streamer_read_uhwi (ib);
> result = lto_file_decl_data_get_var_decl (data_in->file_data, ix_u);
> break;
>
> - case LTO_namelist_decl_ref:
> - {
> - tree tmp;
> - vec *nml_decls = NULL;
> - unsigned i, n;
> -
> - result = make_node (NAMELIS
Re: PR59723: fix LTO + fortran namelist ICEs
On Tue, 4 Feb 2014, Dominique Dhumieres wrote:
> Richard,
>
> With your patch at http://gcc.gnu.org/ml/gcc-patches/2014-02/msg00144.html
> I get multiple ICEs when testing with
> check-gfortran RUNTESTFLAGS="--target_board=unix'{-m32/-flto,-m64/-flto}'"
>
> They are of the kind
>
> lto1: internal compiler error: in mentions_vars_p, at lto/lto.c:972
>
> Note that the Aldy's patch at
> http://gcc.gnu.org/ml/gcc-patches/2014-01/msg01384.html
> fixed all the ICE in the same tests.
Yep, I'm collecting more fixes - see below for what I have. Aldy's
fix is simply wrong (as is the namelist-decl-ref handling in the LTO
code).
With the patch below we can still hit an ICE in lto_fixup_prevailing_decls
as we seem to arrive with a CONSTRUCTOR here. I'm waiting for Micha
to show up to tell me if that's simply because CONSTRUCTOR isn't handled
in the code (yet) or if there is a deeper reason.
Richard.
2014-02-04 Richard Biener
PR lto/59723
* lto-streamer-out.c (lto_output_tree_ref): Do not write
trees from lto_output_tree_ref.
* lto-streamer-in.c (lto_input_tree_ref): Handle LTO_namelist_decl_ref
similar to LTO_imported_decl_ref.
lto/
* lto.c (mentions_vars_p) Handle NAMELIST_DECL.
Index: gcc/lto-streamer-out.c
===
*** gcc/lto-streamer-out.c (revision 207455)
--- gcc/lto-streamer-out.c (working copy)
*** lto_output_tree_ref (struct output_block
*** 255,273
break;
case NAMELIST_DECL:
! {
! unsigned i;
! tree value, tmp;
!
! streamer_write_record_start (ob, LTO_namelist_decl_ref);
! stream_write_tree (ob, DECL_NAME (expr), true);
! tmp = NAMELIST_DECL_ASSOCIATED_DECL (expr);
! gcc_assert (tmp != NULL_TREE);
! streamer_write_uhwi (ob, CONSTRUCTOR_ELTS (tmp)->length());
! FOR_EACH_CONSTRUCTOR_VALUE (CONSTRUCTOR_ELTS (tmp), i, value)
! lto_output_var_decl_index (ob->decl_state, ob->main_stream, value);
! break;
! }
case NAMESPACE_DECL:
streamer_write_record_start (ob, LTO_namespace_decl_ref);
--- 261,269
break;
case NAMELIST_DECL:
! streamer_write_record_start (ob, LTO_namelist_decl_ref);
! lto_output_var_decl_index (ob->decl_state, ob->main_stream, expr);
! break;
case NAMESPACE_DECL:
streamer_write_record_start (ob, LTO_namespace_decl_ref);
Index: gcc/lto/lto.c
===
*** gcc/lto/lto.c (revision 207455)
--- gcc/lto/lto.c (working copy)
*** mentions_vars_p (tree t)
*** 926,931
--- 926,932
case RESULT_DECL:
case IMPORTED_DECL:
case NAMESPACE_DECL:
+ case NAMELIST_DECL:
return mentions_vars_p_decl_common (t);
case VAR_DECL:
Index: gcc/lto-streamer-in.c
===
*** gcc/lto-streamer-in.c (revision 207455)
--- gcc/lto-streamer-in.c (working copy)
*** lto_input_tree_ref (struct lto_input_blo
*** 244,275
case LTO_imported_decl_ref:
case LTO_label_decl_ref:
case LTO_translation_unit_decl_ref:
ix_u = streamer_read_uhwi (ib);
result = lto_file_decl_data_get_var_decl (data_in->file_data, ix_u);
break;
- case LTO_namelist_decl_ref:
- {
- tree tmp;
- vec *nml_decls = NULL;
- unsigned i, n;
-
- result = make_node (NAMELIST_DECL);
- TREE_TYPE (result) = void_type_node;
- DECL_NAME (result) = stream_read_tree (ib, data_in);
- n = streamer_read_uhwi (ib);
- for (i = 0; i < n; i++)
- {
- ix_u = streamer_read_uhwi (ib);
- tmp = lto_file_decl_data_get_var_decl (data_in->file_data, ix_u);
- gcc_assert (tmp != NULL_TREE);
- CONSTRUCTOR_APPEND_ELT (nml_decls, NULL_TREE, tmp);
- }
- NAMELIST_DECL_ASSOCIATED_DECL (result) = build_constructor (NULL_TREE,
- nml_decls);
- break;
- }
-
default:
gcc_unreachable ();
}
--- 244,254
case LTO_imported_decl_ref:
case LTO_label_decl_ref:
case LTO_translation_unit_decl_ref:
+ case LTO_namelist_decl_ref:
ix_u = streamer_read_uhwi (ib);
result = lto_file_decl_data_get_var_decl (data_in->file_data, ix_u);
break;
default:
gcc_unreachable ();
}
[s390] Split out pre-prologue rewrite into separate pass
s390_emit_prologue performs some optimisations on the function before
emitting the prologue. It also rewrites constant pool accesses to make
the base register explicit.
Doing this in the prologue pattern makes the interaction with direct
returns and simple_returns less obvious, so this patch splits the code
out into a new target-specific pre-prologue pass. I've called it
"early_mach" for want of a better name.
I also moved s390_option_override to the end of the file in order
to avoid some forward declarations that would have been needed otherwise.
The only change is at the very end of the function.
Tested on s390-linux-gnu and s390x-linux-gnu, using
unix{,-m31,-march=z10/-m31,-march=z10,-march=z196,-march=zEC12}
for the latter. OK to install?
Thanks,
Richard
gcc/
* config/s390/s390.c: Include tree-pass.h and context.h.
(s390_early_mach): New function, split out from...
(s390_emit_prologue): ...here.
(pass_data_s390_early_mach): New pass structure.
(pass_s390_early_mach): New class.
(s390_option_override): Create and register early_mach pass.
Move to end of file.
Index: gcc/config/s390/s390.c
===
--- gcc/config/s390/s390.c 2014-01-31 17:13:43.803357694 +
+++ gcc/config/s390/s390.c 2014-01-31 17:20:31.582196663 +
@@ -68,6 +68,8 @@ Software Foundation; either version 3, o
#include "params.h"
#include "cfgloop.h"
#include "opts.h"
+#include "tree-pass.h"
+#include "context.h"
/* Define the specific costs for a given cpu. */
@@ -1678,220 +1680,6 @@ s390_init_machine_status (void)
return ggc_alloc_cleared_machine_function ();
}
-static void
-s390_option_override (void)
-{
- unsigned int i;
- cl_deferred_option *opt;
- vec *v =
-(vec *) s390_deferred_options;
-
- if (v)
-FOR_EACH_VEC_ELT (*v, i, opt)
- {
- switch (opt->opt_index)
- {
- case OPT_mhotpatch:
- s390_hotpatch_trampoline_halfwords = (opt->value) ?
- s390_hotpatch_trampoline_halfwords_default : -1;
- break;
- case OPT_mhotpatch_:
- {
- int val;
-
- val = integral_argument (opt->arg);
- if (val == -1)
- {
- /* argument is not a plain number */
- error ("argument to %qs should be a non-negative integer",
-"-mhotpatch=");
- break;
- }
- else if (val > s390_hotpatch_trampoline_halfwords_max)
- {
- error ("argument to %qs is too large (max. %d)",
-"-mhotpatch=", s390_hotpatch_trampoline_halfwords_max);
- break;
- }
- s390_hotpatch_trampoline_halfwords = val;
- break;
- }
- default:
- gcc_unreachable ();
- }
- }
-
- /* Set up function hooks. */
- init_machine_status = s390_init_machine_status;
-
- /* Architecture mode defaults according to ABI. */
- if (!(target_flags_explicit & MASK_ZARCH))
-{
- if (TARGET_64BIT)
- target_flags |= MASK_ZARCH;
- else
- target_flags &= ~MASK_ZARCH;
-}
-
- /* Set the march default in case it hasn't been specified on
- cmdline. */
- if (s390_arch == PROCESSOR_max)
-{
- s390_arch_string = TARGET_ZARCH? "z900" : "g5";
- s390_arch = TARGET_ZARCH ? PROCESSOR_2064_Z900 : PROCESSOR_9672_G5;
- s390_arch_flags = processor_flags_table[(int)s390_arch];
-}
-
- /* Determine processor to tune for. */
- if (s390_tune == PROCESSOR_max)
-{
- s390_tune = s390_arch;
- s390_tune_flags = s390_arch_flags;
-}
-
- /* Sanity checks. */
- if (TARGET_ZARCH && !TARGET_CPU_ZARCH)
-error ("z/Architecture mode not supported on %s", s390_arch_string);
- if (TARGET_64BIT && !TARGET_ZARCH)
-error ("64-bit ABI not supported in ESA/390 mode");
-
- /* Use hardware DFP if available and not explicitly disabled by
- user. E.g. with -m31 -march=z10 -mzarch */
- if (!(target_flags_explicit & MASK_HARD_DFP) && TARGET_DFP)
-target_flags |= MASK_HARD_DFP;
-
- /* Enable hardware transactions if available and not explicitly
- disabled by user. E.g. with -m31 -march=zEC12 -mzarch */
- if (!(target_flags_explicit & MASK_OPT_HTM) && TARGET_CPU_HTM &&
TARGET_ZARCH)
-target_flags |= MASK_OPT_HTM;
-
- if (TARGET_HARD_DFP && !TARGET_DFP)
-{
- if (target_flags_explicit & MASK_HARD_DFP)
- {
- if (!TARGET_CPU_DFP)
- error ("hardware decimal floating point instructions"
- " not available on %s", s390_arch_string);
- if (!TARGET_ZARCH)
- error ("hardware decimal floating point instructions"
- " not available in ESA/390 mode");
- }
- else
- target_flags &= ~MASK_HARD_DFP;
-}
-
- if ((target_flags_explicit & MASK_SOFT_FLOA
Re: PR59723: fix LTO + fortran namelist ICEs
Richard,
With your patch at http://gcc.gnu.org/ml/gcc-patches/2014-02/msg00144.html
I get multiple ICEs when testing with
check-gfortran RUNTESTFLAGS="--target_board=unix'{-m32/-flto,-m64/-flto}'"
They are of the kind
lto1: internal compiler error: in mentions_vars_p, at lto/lto.c:972
Note that the Aldy's patch at
http://gcc.gnu.org/ml/gcc-patches/2014-01/msg01384.html
fixed all the ICE in the same tests.
Dominique
Re: var-tracking and s390's LM(G)
On Tue, Feb 04, 2014 at 10:47:49AM +, Richard Sandiford wrote: > I wondered whether we could model the load of the stack pointer as an > addition in cases where that is safe (e.g. it would require !calls_eh_return > at least). But that would only work in functions that don't need a frame > pointer. In functions that do need a frame pointer we'd presumably have > (plus (hfp) (const_int X)) instead, which stack_adjust_offset_pre_post > would again not understand. But vt_stack_adjustments is only called if frame pointer isn't used. > Another option would be to work out the offset from REG_CFA_DEF_CFA notes, > where present, but I'm a bit uncomfortable with the idea of mixing two > different methods of calculating the stack offset. So, how does the lmg insn look like in RTL dump on some problematic testcase? insn_stack_adjust_offset_pre_post already uses REG_FRAME_RELATED_EXPR, which is also a kind of CFI note (the oldest one), so likely the issue is just that it hasn't been adjusted to handle other newer REG_CFA_* notes that tell how the stack pointer is adjusted. > The simplest fix seems to be to disable this check for the exit block. > We never use its stack_adjust anyway, and dwarf2cfi already checks > (using CFA information) that the offsets in a shrink-wrapped function > are consistent. > > Tested on s390-linux-gnu and s390x-linux-gnu. OK to install? I don't like this, my strong preference is to handle REG_CFA_* notes. Jakub
[PATCH] Backport compiler-rt r200747 into libsanitizer (PR sanitizer/60055)
Hi!
This backports upstream fix for __tsan_default_options.
Committed to trunk.
2014-02-04 Jakub Jelinek
PR sanitizer/60055
* tsan/tsan_flags.cc (__tsan_default_options): Add
SANITIZER_INTERFACE_ATTRIBUTE. Backport from upstream r200747.
* tsan/tsan_rtl.cc (__tsan::OnFinalize): Likewise.
--- libsanitizer/tsan/tsan_flags.cc (revision 200746)
+++ libsanitizer/tsan/tsan_flags.cc (revision 200747)
@@ -29,7 +29,8 @@ extern "C" const char* __tsan_default_op
void WEAK OverrideFlags(Flags *f) {
(void)f;
}
-extern "C" const char *WEAK __tsan_default_options() {
+extern "C" SANITIZER_INTERFACE_ATTRIBUTE
+const char *WEAK __tsan_default_options() {
return "";
}
#endif
--- libsanitizer/tsan/tsan_rtl.cc (revision 200746)
+++ libsanitizer/tsan/tsan_rtl.cc (revision 200747)
@@ -40,6 +40,7 @@ static char ctx_placeholder[sizeof(Conte
#ifdef TSAN_EXTERNAL_HOOKS
bool OnFinalize(bool failed);
#else
+SANITIZER_INTERFACE_ATTRIBUTE
bool WEAK OnFinalize(bool failed) {
return failed;
}
Jakub
var-tracking and s390's LM(G)
Sorry, looks like I missed the stage 3 deadline by a day, but:
we'd like to add support for shrink-wrapping and conditional returns
to s390(x) for 4.9. Doing this showed up a problem with the way that
var-tracking handles the load-multiple instruction that s390 uses in
many function epilogues.
stack_adjust_offset_pre_post punts on any assignment to the stack
pointer that it doesn't understand and assumes that no adjustment
is made. And the only kind of direct set that it understands is
(reasonably enough) one of the form (plus (sp) (const_int X)).
(There's a MINUS case too but it should never trigger.)
But if s390 needs to save and restore several registers, it will
use LM(G) to restore the stack pointer too, rather than generating
a separate addition. stack_adjust_offset_pre_post doesn't understand
the load and so the stack_adjust after the epilogue is the same as
before it, rather than the correct post-epilogue value.
When shrink-wrapping is enabled, we have other edges to the exit block
that have the correct stack_adjust and so we fail the test:
/* Check whether the adjustments on the edges are the same. */
if (VTI (dest)->in.stack_adjust != VTI (src)->out.stack_adjust)
{
free (stack);
return false;
}
No var-tracking is then performed on the function, leading to some new
guality.exp failures.
I wondered whether we could model the load of the stack pointer as an
addition in cases where that is safe (e.g. it would require !calls_eh_return
at least). But that would only work in functions that don't need a frame
pointer. In functions that do need a frame pointer we'd presumably have
(plus (hfp) (const_int X)) instead, which stack_adjust_offset_pre_post
would again not understand.
Another option would be to work out the offset from REG_CFA_DEF_CFA notes,
where present, but I'm a bit uncomfortable with the idea of mixing two
different methods of calculating the stack offset.
The simplest fix seems to be to disable this check for the exit block.
We never use its stack_adjust anyway, and dwarf2cfi already checks
(using CFA information) that the offsets in a shrink-wrapped function
are consistent.
Tested on s390-linux-gnu and s390x-linux-gnu. OK to install?
Thanks,
Richard
gcc/
* var-tracking.c (vt_stack_adjustments): Don't require stack_adjusts
to match for the exit block.
Index: gcc/var-tracking.c
===
--- gcc/var-tracking.c 2014-02-03 12:16:49.009149939 +
+++ gcc/var-tracking.c 2014-02-04 09:58:59.924381497 +
@@ -886,8 +886,25 @@ vt_stack_adjustments (void)
}
else
{
- /* Check whether the adjustments on the edges are the same. */
- if (VTI (dest)->in.stack_adjust != VTI (src)->out.stack_adjust)
+ /* We can end up with different stack adjustments for the exit block
+of a shrink-wrapped function if stack_adjust_offset_pre_post
+doesn't understand the rtx pattern used to restore the stack
+pointer in the epilogue. For example, on s390(x), the stack
+pointer is often restored via a load-multiple instruction
+and so no stack_adjust offset is recorded for it. This means
+that the stack offset at the end of the epilogue block is the
+the same as the offset before the epilogue, whereas other paths
+to the exit block will have the correct stack_adjust.
+
+It is safe to ignore these differences because (a) we never
+use the stack_adjust for the exit block in this pass and
+(b) dwarf2cfi checks whether the CFA notes in a shrink-wrapped
+function are correct.
+
+We must check whether the adjustments on other edges are
+the same though. */
+ if (dest != EXIT_BLOCK_PTR_FOR_FN (cfun)
+ && VTI (dest)->in.stack_adjust != VTI (src)->out.stack_adjust)
{
free (stack);
return false;
Re: [RFA][PR 16361] Add warnings for NULL pointer dereferences and such
On 01/27/2014 08:09 PM, Jeff Law wrote: + if (dereference) + warning_at ((gimple_location (use_stmt) +? gimple_location (use_stmt) +: gimple_phi_arg_location (phi, i)), + OPT_Wnull_dereference, + "Potential NULL pointer dereference"); I think warnings should start with a small letter (unless the first word is capitalized for other reasons). Two more instances follow. Instead of "Potential invalid NULL argument or return value", "potential NULL value used in non-NULL context" would be clearer. It would be really nice if you could add a note that points to the declaration of non-NULL-ness for the context. After the change, the third argument to infer_nonnull_range seems always true, including the call in tre-vrp.c:infer_value_range, so there's a minor cleanup opportunity. Looking at infer_nonnull_range, there's an undocumented interaction with -fdelete-null-pointer-checks. -- Florian Weimer / Red Hat Product Security Team
Re: [C++ Patch] PR 58846
Hi, On 02/04/2014 10:55 AM, Rainer Orth wrote: Paolo Carlini writes: a very minor ICE on invalid regression, but since we explicitly allow for redeclarations (also see comments in declare_global_var) we may as well avoid crashing, at least in mainline. Tested x86_64-linux. The test FAILs on Solaris which doesn't have __cxa_atexit: FAIL: g++.dg/init/dso_handle2.C -std=c++98 (test for errors, line 10) FAIL: g++.dg/init/dso_handle2.C -std=c++11 (test for errors, line 10) Fixed like dso_handle1.C does by forcing -fuse-cxa-atexit. Tested with the appropriate runtest invocations on i386-pc-solaris2.11 and x86_64-unknown-linux-gnu. Ok for mainline? I think it qualifies at obvious. Thanks! Paolo. PS: At some point I had it, when I adapted the testcase from an existing one, then I tried to minimize it ;-/
[testsuite] Don't xfail gcc.dg/binop-xor1.c
AFAICT the gcc.dg/binop-xor1.c test is XPASSing everywhere since about
20131114:
XPASS: gcc.dg/binop-xor1.c scan-tree-dump-times optimized "^" 1
To reduce testsuite noise, I'd like to apply the following patch.
Tested with the appropriate runtest invocations on i386-pc-solaris2.11
and x86_64-unknown-linux-gnu.
Ok for mainline?
Rainer
2014-02-04 Rainer Orth
* gcc.dg/binop-xor1.c: Don't xfail scan-tree-dump-times.
# HG changeset patch
# Parent 2ea5215f63132658a45a94a32e3a2c5db29ded92
Don't xfail gcc.dg/binop-xor1.c
diff --git a/gcc/testsuite/gcc.dg/binop-xor1.c b/gcc/testsuite/gcc.dg/binop-xor1.c
--- a/gcc/testsuite/gcc.dg/binop-xor1.c
+++ b/gcc/testsuite/gcc.dg/binop-xor1.c
@@ -7,5 +7,5 @@ foo (int a, int b, int c)
return ((a && !b && c) || (!a && b && c));
}
-/* { dg-final { scan-tree-dump-times "\\\^" 1 "optimized" { xfail *-*-* } } } */
+/* { dg-final { scan-tree-dump-times "\\\^" 1 "optimized" } } */
/* { dg-final { cleanup-tree-dump "optimized" } } */
--
-
Rainer Orth, Center for Biotechnology, Bielefeld University
Re: [ARM] fix a typo in ftest-armv7a-thumb.c
On 03/02/14 10:22, Ramana Radhakrishnan wrote: On 02/03/14 09:58, Renlin Li wrote: Hi all, This is a simple patch which fix a type error found in gcc/testsuite/gcc.target/arm/ftest-armv7a-thumb.c Okay for trunk? Ok. Hi all, I've committed this as r207457. Kyrill Ramana Regards, Renlin Li gcc/testsuite/ChangeLog: 2014-02-03 Renlin Li * gcc.target/arm/ftest-armv7a-thumb.c: Change aramv7-a to armv7-a.
Re: [C++ Patch] PR 58846
Paolo Carlini writes:
> a very minor ICE on invalid regression, but since we explicitly allow for
> redeclarations (also see comments in declare_global_var) we may as well
> avoid crashing, at least in mainline. Tested x86_64-linux.
The test FAILs on Solaris which doesn't have __cxa_atexit:
FAIL: g++.dg/init/dso_handle2.C -std=c++98 (test for errors, line 10)
FAIL: g++.dg/init/dso_handle2.C -std=c++11 (test for errors, line 10)
Fixed like dso_handle1.C does by forcing -fuse-cxa-atexit.
Tested with the appropriate runtest invocations on i386-pc-solaris2.11
and x86_64-unknown-linux-gnu. Ok for mainline?
Rainer
2014-02-04 Rainer Orth
* g++.dg/init/dso_handle2.C: Compile with -fuse-cxa-atexit.
# HG changeset patch
# Parent 944218077f384619dcc391f55c51e5ba477dd022
Fix g++.dg/init/dso_handle2.C on Solaris
gcc/testsuite:
* g++.dg/init/dso_handle2.C: Compile with -fuse-cxa-atexit.
diff --git a/gcc/testsuite/g++.dg/init/dso_handle2.C b/gcc/testsuite/g++.dg/init/dso_handle2.C
--- a/gcc/testsuite/g++.dg/init/dso_handle2.C
+++ b/gcc/testsuite/g++.dg/init/dso_handle2.C
@@ -1,4 +1,5 @@
// PR c++/58846
+// { dg-options "-fuse-cxa-atexit" }
extern "C" { char* __dso_handle; }
--
-
Rainer Orth, Center for Biotechnology, Bielefeld University
Re: PR59723: fix LTO + fortran namelist ICEs
On Tue, 28 Jan 2014, Jakub Jelinek wrote:
> On Tue, Jan 28, 2014 at 10:40:51AM +0100, Richard Biener wrote:
> > The patch doesn't make much sense to me. I think the problem is that
> > NAMELIST_DECL is output in a ref section (LTO_namelist_decl_ref) but
> > the output routine writes other refs (via stream_write_tree and
> > outputting the constructor). lto_output_tree_ref may not do this
> > kind of stuff. Instead the contents of a NAMELIST_DECL need to be
> > output from the generic tree writing routines.
> >
> > Where are NAMELIST_DECLs possibly refered from?
>
> I think usually from BLOCK_VARS of some BLOCK.
The following seems to fix things for me - bootstrap / regtest and
SPEC 2k6 LTO -g build in progress.
Now somebody could verify if LTO produces sensible debuginfo for
NAMELIST_DECLs.
Richard.
2014-02-04 Richard Biener
PR lto/59723
* lto-streamer-out.c (lto_output_tree_ref): Do not write
trees from lto_output_tree_ref.
Index: gcc/lto-streamer-out.c
===
*** gcc/lto-streamer-out.c (revision 207455)
--- gcc/lto-streamer-out.c (working copy)
*** lto_output_tree_ref (struct output_block
*** 255,273
break;
case NAMELIST_DECL:
! {
! unsigned i;
! tree value, tmp;
!
! streamer_write_record_start (ob, LTO_namelist_decl_ref);
! stream_write_tree (ob, DECL_NAME (expr), true);
! tmp = NAMELIST_DECL_ASSOCIATED_DECL (expr);
! gcc_assert (tmp != NULL_TREE);
! streamer_write_uhwi (ob, CONSTRUCTOR_ELTS (tmp)->length());
! FOR_EACH_CONSTRUCTOR_VALUE (CONSTRUCTOR_ELTS (tmp), i, value)
! lto_output_var_decl_index (ob->decl_state, ob->main_stream, value);
! break;
! }
case NAMESPACE_DECL:
streamer_write_record_start (ob, LTO_namespace_decl_ref);
--- 261,269
break;
case NAMELIST_DECL:
! streamer_write_record_start (ob, LTO_namelist_decl_ref);
! lto_output_var_decl_index (ob->decl_state, ob->main_stream, expr);
! break;
case NAMESPACE_DECL:
streamer_write_record_start (ob, LTO_namespace_decl_ref);
Re: [build, libgcc] Ensure libgcc_s unwinder is always used on 64-bit Solaris 10+/x86 (PR target/59788)
Ian Lance Taylor writes: >> Could you please take care of that, provided the RMs approve? The patch >> was ok two weeks ago and is completely Solaris-specific, so I'd like to >> get it into the 4.9.0 release. > > I was just waiting for you to commit it elsewhere. And I was waiting for you to commit the libgo part since doing it the other way round would have broken the amd64 libgo. > The patch to libgo/ltmain.sh is now committed to mainline. I've now committed the rest, thanks. Btw., what's the procedure for syncing the toplevel ltmain.sh these days? AFAIK there's both the old CVS src repo as well as the new binutils-gdb git repo. Does the patch need to go to both? Rainer -- - Rainer Orth, Center for Biotechnology, Bielefeld University
proposal to add -Wheader-guard option
Hi, I was wondering if it's a good idea to add -Wheader-guard option
that warns on mismatches between #ifndef and #define lines
in header guard, similar to -Wheader-guard in clang-3.4 ?
(http://llvm.org/releases/3.4/tools/clang/docs/ReleaseNotes.html)
I have implemented patch for -Wheader-guard (please find it attached).
Consider a file having the following format:
#ifndef cmacro (or #if !defined(cmacro) )
#define dmacro
// rest of the stuff
#endif
The warning is triggered if the edit distance
(http://en.wikipedia.org/wiki/Levenshtein_distance), between cmacro
and dmacro
is <= max (len(cmacro), len(dmacro)) / 2
If the edit distance is more than half, I assume that cmacro
and dmacro are "very different", and the intent
was probably not to define header guard (This is what clang does too).
Example:
#ifndef FOO_H
#define _FOO_H
#endif
foo.h:1:0: warning: FOO_H used as header guard followed by #define of
different macro [-Wheader-guard]
#ifndef FOO_H
^
foo.h:2:0: note: FOO_H is defined here, did you mean _FOO_H ?
#define _FOO_H
^
Warning is not triggered in the following cases:
1] The edit distance between #ifndef (or #!defined) macro
and #define macro is > half of maximum length between two macros
Example:
#ifndef FOO
#define BAR
#endif
2] #ifndef and #define are not on consecutive lines (blank lines/ comment lines
are not ignored).
Example:
#ifndef cmacro
/* hello world */
extern int x;
#define dmacro
#endif
3] dmacro gets undefined
Example:
#ifndef cmacro
#define dmacro
#undef dmacro
#endif
However the following warning gets generated during the build:
../../src/libcpp/directives.c: In function 'void _cpp_pop_buffer(cpp_reader*)':
../../src/libcpp/directives.c:
2720:59: warning: 'inc_type' may be used
uninitialized in this function [-Wmaybe-uninitialized]
_cpp_pop_file_buffer (pfile, inc, to_free, inc_type);
^
_cpp_pop_buffer(): http://pastebin.com/aLYLnXJa
I have defined inc_type only if inc is not null (ie buffer is file
buffer) in 1st if()
and used it (passed it to _cpp_pop_file_buffer() ), only if inc is not null
in 2nd if(). I guess this warning could be considered harmless ?
How should I should rewrite it to avoid the warning ?
Thanks and Regards,
Prathamesh
Index: libcpp/directives.c
===
--- libcpp/directives.c (revision 207451)
+++ libcpp/directives.c (working copy)
@@ -565,6 +565,27 @@ lex_macro_node (cpp_reader *pfile, bool
return NULL;
}
+// return true if top of if_stack is cmacro
+static bool
+cmacro_ifs_top_p(cpp_reader *pfile)
+{
+ struct if_stack *ifs = pfile->buffer->if_stack;
+ return ifs && (ifs->next == NULL) && (ifs->mi_cmacro != NULL);
+}
+
+static linenum_type
+linediff (struct line_maps *maps, source_location loc1, source_location loc2)
+{
+ linenum_type temp;
+
+ if (loc1 < loc2)
+temp = loc1, loc1 = loc2, loc2 = temp;
+
+ const struct line_map *m1 = linemap_lookup (maps, loc1);
+ const struct line_map *m2 = linemap_lookup (maps, loc2);
+ return SOURCE_LINE (m1, loc1) - SOURCE_LINE (m2, loc2);
+}
+
/* Process a #define directive. Most work is done in macro.c. */
static void
do_define (cpp_reader *pfile)
@@ -586,6 +607,15 @@ do_define (cpp_reader *pfile)
pfile->cb.define (pfile, pfile->directive_line, node);
node->flags &= ~NODE_USED;
+
+ // possibly #define following #ifndef in the include guard
+ if (pfile->buffer->dmacro == NULL && cmacro_ifs_top_p (pfile)
+ && linediff (pfile->line_table, pfile->directive_line, pfile->buffer->if_stack->line) == 1)
+ {
+pfile->buffer->dmacro = node;
+pfile->buffer->cmacro_loc = pfile->buffer->if_stack->line;
+pfile->buffer->dmacro_loc = pfile->directive_line;
+ }
}
}
@@ -2527,6 +2557,104 @@ cpp_get_deps (cpp_reader *pfile)
return pfile->deps;
}
+static inline unsigned
+ed_min3 (unsigned a, unsigned b, unsigned c)
+{
+ return (a < b) ? (a < c) ? a : c
+ : (b < c) ? b : c;
+}
+
+/*
+ * Levenshtein distance: http://en.wikipedia.org/wiki/Levenshtein_distance
+ * The algorithm is implemented using dynamic programming.
+ * Instead of the matrix[s_len][t_len], we only need storage
+ * for one row with length = min(s_len, t_len), since we look only at one row and it's previous row
+ * at a time (and previous row's contents are replaced in place).
+ * hstr is string laid along the row of matrix and vstr is laid along column
+ * hstr is string with lesser length between s and t.
+ */
+
+static unsigned
+edit_dist (const unsigned char *s, const unsigned char *t,
+ unsigned s_len, unsigned t_len)
+{
+ unsigned *row;
+ unsigned n_rows, n_cols, i, j, prev, temp, dist;
+ const unsigned char *hstr, *vstr;
+
+ if (s_len < t_len)
+ {
+hstr = s;
+vstr = t;
+n_cols = s_len + 1;
+n_rows = t_len + 1;
+ }
+ else
+ {
+hstr = t;
+vstr = s;
+n_cols = t_len +
