Re: [6 Regession] Usage of unitialized pointer io/list_read.c (
On 15 February 2016 at 23:16, Janne Blomqvist wrote: > On Mon, Feb 15, 2016 at 11:45 PM, Jerry DeLisle wrote: >> The title of the PR should be "Mishandling of namelist comments" or >> "Interpreting '!' as a comment in non-namelist reads". >> >> The attached patch fixes the regression by reverting the previous attempt at >> namelist comments that used only CASE_SEPARATOR to enable comments in >> namelists. >> The approach now is to test specifically for '!' in each type of read >> various >> functions. If in namelist mode the respective case falls through to the >> handling >> of separators which eats the line when a '!' is found. Otherwise, the read >> is >> determined to be bad and an error is issued. >> >> Since the reporter of this PR noticed something screwy with the 'new' >> pointer in >> push_char4, I took a close look at the code and deleted it. I also heavily >> instrumented and tested this mechanism to grow the buffer and deleted the >> use of >> memset which was commented to not be needed. The 'new' was not being >> initialized >> and I think was a leftover from a previous edit and just missed. >> >> I added two new test cases in the patch. These test all the new error >> conditions. Also, read_bang4.f90 uses a large kind=4 string to exercise the >> buffer mechanism. Verification is through making sure what we read in matches >> what we wrote out to the test scratch file >> >> Regression tested on x86_64-Linux. OK for trunk? any thoughts on back >> porting >> to 5 since it fixes a potentially bad pointer problem in push_char4? > > Ok for both trunk and 5. > > Hi, Although OK in trunk, I've noticed regressions in the gcc-5 branch since you commtted r233442: - PASS now FAIL [PASS => FAIL]: gfortran.dg/namelist_38.f90 -O0 execution test gfortran.dg/namelist_38.f90 -O1 execution test gfortran.dg/namelist_38.f90 -O2 execution test gfortran.dg/namelist_38.f90 -O3 -fomit-frame-pointer execution test gfortran.dg/namelist_38.f90 -O3 -fomit-frame-pointer -funroll-all-loops -finline-functions execution test gfortran.dg/namelist_38.f90 -O3 -fomit-frame-pointer -funroll-loops execution test gfortran.dg/namelist_38.f90 -O3 -g execution test gfortran.dg/namelist_38.f90 -Os execution test gfortran.dg/namelist_84.f90 -O0 execution test gfortran.dg/namelist_84.f90 -O1 execution test gfortran.dg/namelist_84.f90 -O2 execution test gfortran.dg/namelist_84.f90 -O3 -fomit-frame-pointer execution test gfortran.dg/namelist_84.f90 -O3 -fomit-frame-pointer -funroll-all-loops -finline-functions execution test gfortran.dg/namelist_84.f90 -O3 -fomit-frame-pointer -funroll-loops execution test gfortran.dg/namelist_84.f90 -O3 -g execution test gfortran.dg/namelist_84.f90 -Os execution test Looking at the logs, I can see: At line 33 of file /aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/testsuite/gfortran.dg/namelist_38.f90 (unit = 10, file = '/gfortrantmpzLPCW0') Fortran runtime error: Cannot match namelist object name a'a FAIL: gfortran.dg/namelist_38.f90 -O0 execution test At line 20 of file /aci-gcc-fsf/sources/gcc-fsf/gccsrc/gcc/testsuite/gfortran.dg/namelist_84.f90 (unit = 10, file = '/gfortrantmpVWEbdK') Fortran runtime error: Cannot match namelist object name mon FAIL: gfortran.dg/namelist_84.f90 -O0 execution test I'm running the tests on arm* and aarch64* targets. Am I missing something? Thanks, Christophe. > -- > Janne Blomqvist
Re: RFC: [Patch, PR Bug 60818] - ICE in validate_condition_mode on powerpc*-linux-gnu* ]
On Tue, Feb 16, 2016 at 05:02:30AM +, Rohit Arul Raj D wrote:
> ii) combiner pass converts "gtu" to "ne" and converts the mode to "CC" (from
> CCUNS).
> set (reg/i:SI 3 3)
> (if_then_else:SI (ne (reg:CC 166) -> operator and mode
> changed
> (const_int 0 [0]))
> (reg:SI 168)
> (const_int 0 [0])))
>
> (gdb) p debug_rtx (other_insn)
> (insn 11 10 16 2 (set (reg:SI 165 [ D.2339+-3 ])
> (if_then_else:SI (ne (reg:CC 166)
> (const_int 0 [0]))
> (reg:SI 168)
> (reg:SI 167))) test.c:7 317 {isel_unsigned_si}
> (expr_list:REG_DEAD (reg:SI 168)
> (expr_list:REG_DEAD (reg:SI 167)
> (expr_list:REG_DEAD (reg:CC 166)
> (expr_list:REG_EQUAL (gtu:SI (reg:CC 166)
> (const_int 0 [0]))
> (nil))
>
> iii) once the instruction match fails, it tries to link back to the pattern
> stored in the REG_EQUAL note to the SRC. But while doing so, it doesn't
> change the conditional mode based on the operator which leads to the ICE.
I don't think combine is doing anything wrong here. Combine is quite
reasonable in making the substitutions.
What's wrong is the rs6000 backend asserting that (gtu (reg:CC)) can't
happen, because obviously it does. Rather than trying to fix combine,
(where the ICE happens on attempting to validate the insn!), I think
the rs6000 backend should change. Like so. Not yet bootstrapped,
but I'm about to fire one off.
PR target/60818
* config/rs6000/rs6000.c (validate_condition_mode): Return a
bool rather than asserting modes as expected. Update all uses
to assert.
* config/rs6000/rs6000-protos.h (validate_condition_mode):
Update prototype.
* config/rs6000/predicates.md (branch_comparison_operator):
Use result of validate_condition_mode.
diff --git a/gcc/config/rs6000/predicates.md b/gcc/config/rs6000/predicates.md
index a74a6a0..f037b0d 100644
--- a/gcc/config/rs6000/predicates.md
+++ b/gcc/config/rs6000/predicates.md
@@ -1089,13 +1089,11 @@
;; Return 1 if OP is a comparison operation that is valid for a branch
;; instruction. We check the opcode against the mode of the CC value.
-;; validate_condition_mode is an assertion.
(define_predicate "branch_comparison_operator"
(and (match_operand 0 "comparison_operator")
(and (match_test "GET_MODE_CLASS (GET_MODE (XEXP (op, 0))) == MODE_CC")
(match_test "validate_condition_mode (GET_CODE (op),
- GET_MODE (XEXP (op, 0))),
- 1"
+ GET_MODE (XEXP (op, 0)))"
;; Return 1 if OP is a valid comparison operator for "cbranch" instructions.
;; If we're assuming that FP operations cannot generate user-visible traps,
diff --git a/gcc/config/rs6000/rs6000-protos.h
b/gcc/config/rs6000/rs6000-protos.h
index d9a6b1f..91822e8 100644
--- a/gcc/config/rs6000/rs6000-protos.h
+++ b/gcc/config/rs6000/rs6000-protos.h
@@ -40,7 +40,7 @@ extern int small_data_operand (rtx, machine_mode);
extern bool mem_operand_gpr (rtx, machine_mode);
extern bool toc_relative_expr_p (const_rtx, bool);
extern bool invalid_e500_subreg (rtx, machine_mode);
-extern void validate_condition_mode (enum rtx_code, machine_mode);
+extern bool validate_condition_mode (enum rtx_code, machine_mode);
extern bool legitimate_constant_pool_address_p (const_rtx, machine_mode,
bool);
extern bool legitimate_indirect_address_p (rtx, int);
diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
index ed94c53..3fff9ab 100644
--- a/gcc/config/rs6000/rs6000.c
+++ b/gcc/config/rs6000/rs6000.c
@@ -17162,7 +17162,7 @@ rs6000_output_load_multiple (rtx operands[3])
match. The other alternatives either don't make sense or should
never be generated. */
-void
+bool
validate_condition_mode (enum rtx_code code, machine_mode mode)
{
gcc_assert ((GET_RTX_CLASS (code) == RTX_COMPARE
@@ -17170,28 +17170,36 @@ validate_condition_mode (enum rtx_code code,
machine_mode mode)
&& GET_MODE_CLASS (mode) == MODE_CC);
/* These don't make sense. */
- gcc_assert ((code != GT && code != LT && code != GE && code != LE)
- || mode != CCUNSmode);
+ if (mode == CCUNSmode
+ && (code == GT || code == LT || code == GE || code == LE))
+return false;
- gcc_assert ((code != GTU && code != LTU && code != GEU && code != LEU)
- || mode == CCUNSmode);
+ if (mode != CCUNSmode
+ && (code == GTU || code == LTU || code == GEU || code == LEU))
+return false;
- gcc_assert (mode == CCFPmode
- || (code != ORDERED && code != UNORDERED
- && code != UNEQ && code != LTGT
- && code != UNGT && code != UNLT
- && code != UNGE && code != UNLE));
+ if (mode != C
[PATCH] Fix PR69801
Bootstrapped and tested on x86_64-unknown-linux-gnu, applied.
Richard.
2016-02-16 Richard Biener
PR middle-end/69801
* fold-const.c (operand_equal_p): For COND_EXPR zero operand
mask OEP_ADDRESS_OF.
* gcc.dg/pr69801.c: New testcase.
Index: gcc/fold-const.c
===
*** gcc/fold-const.c(revision 233418)
--- gcc/fold-const.c(working copy)
*** operand_equal_p (const_tree arg0, const_
*** 3118,3123
--- 3118,3128
TREE_OPERAND (arg1, 0), flags));
case COND_EXPR:
+ if (! OP_SAME (1) || ! OP_SAME (2))
+ return 0;
+ flags &= ~OEP_ADDRESS_OF;
+ return OP_SAME (0);
+
case VEC_COND_EXPR:
case DOT_PROD_EXPR:
return OP_SAME (0) && OP_SAME (1) && OP_SAME (2);
Index: gcc/testsuite/gcc.dg/pr69801.c
===
*** gcc/testsuite/gcc.dg/pr69801.c (revision 0)
--- gcc/testsuite/gcc.dg/pr69801.c (working copy)
***
*** 0
--- 1,7
+ /* { dg-do compile } */
+
+ struct {
+ char c[1];
+ } b, c;
+ int d, e;
+ void fn1() { e ? (d ? b : c).c : (d ? b : c).c; }
Re: [PATCH] Fix PR69291, RTL if-conversion bug
On Mon, 15 Feb 2016, Richard Sandiford wrote: > Richard Biener writes: > > On Wed, 10 Feb 2016, Bernd Schmidt wrote: > > > >> On 02/10/2016 02:50 PM, Richard Biener wrote: > >> > On Wed, 10 Feb 2016, Bernd Schmidt wrote: > >> > > >> > > On 02/10/2016 02:35 PM, Richard Biener wrote: > >> > > > >> > > > Index: gcc/ifcvt.c > >> > > > === > >> > > > --- gcc/ifcvt.c (revision 233262) > >> > > > +++ gcc/ifcvt.c (working copy) > >> > > > @@ -1274,7 +1274,8 @@ noce_try_store_flag_constants (struct no > >> > > > && CONST_INT_P (XEXP (a, 1)) > >> > > > && CONST_INT_P (XEXP (b, 1)) > >> > > > && rtx_equal_p (XEXP (a, 0), XEXP (b, 0)) > >> > > > - && noce_operand_ok (XEXP (a, 0)) > >> > > > + && (REG_P (XEXP (a, 0)) > >> > > > + || ! reg_mentioned_p (if_info->x, XEXP (a, 0))) > >> > > > >> > > I guess that would also work. Could maybe use a brief comment. > >> > > >> > Ok. I'm testing that. I wonder if we need to use > >> > reg_overlap_mentioned_p > >> > here (hard-reg pairs?) or if reg_mentioned_p is safe. > >> > >> Let's go with reg_overlap_mentioned_p. I kind of forgot about that once I > >> thought of possible issues with emitting a move :-( > > > > Ok, the following is in testing now. > > > > Ok? > > > > Thanks, > > Richard. > > > > 2016-02-10 Richard Biener > > > > PR rtl-optimization/69291 > > * ifcvt.c (noce_try_store_flag_constants): Do not allow > > subexpressions affected by changing the result. > > > > Index: gcc/ifcvt.c > > === > > --- gcc/ifcvt.c (revision 233262) > > +++ gcc/ifcvt.c (working copy) > > @@ -1274,7 +1274,10 @@ noce_try_store_flag_constants (struct no > >&& CONST_INT_P (XEXP (a, 1)) > >&& CONST_INT_P (XEXP (b, 1)) > >&& rtx_equal_p (XEXP (a, 0), XEXP (b, 0)) > > - && noce_operand_ok (XEXP (a, 0)) > > + /* Allow expressions that are not using the result or plain > > + registers where we handle overlap below. */ > > + && (REG_P (XEXP (a, 0)) > > + || ! reg_overlap_mentioned_p (if_info->x, XEXP (a, 0))) > >&& if_info->branch_cost >= 2) > > Sorry if this has already been covered, but shouldn't we be adding > to the noce_operand_ok check rather than replacing it? I think we > still want to check side_effects_p and may_trap_p. Whoops, yes. Testing fix. Richard.
Re: [Patch AArch64] Use software sqrt expansion always for -mlow-precision-recip-sqrt
On 11 January 2016 at 11:53, James Greenhalgh wrote: > > --- > 2015-12-10 James Greenhalgh > > * config/aarch64/aarch64.c (use_rsqrt_p): Always use software > reciprocal sqrt for -mlow-precision-recip-sqrt. > OK /Marcus
Re: [Patch AArch64] GCC 6 regression in vector performance. - Fix vector initialization to happen with lane load instructions.
On 20 January 2016 at 15:22, James Greenhalgh wrote: > gcc/ > > 2016-01-20 James Greenhalgh > Ramana Radhakrishnan > > * config/aarch64/aarch64.c (aarch64_expand_vector_init): Refactor, > always use lane loads to construct non-constant vectors. > > gcc/testsuite/ > > 2016-01-20 James Greenhalgh > Ramana Radhakrishnan > > * gcc.target/aarch64/vector_initialization_nostack.c: New. > OK /Marcus
Re: [Patch AArch64] Restrict 16-bit sqrdml{sa}h instructions to FP_LO_REGS
On 26 January 2016 at 16:04, James Greenhalgh wrote: > 2016-01-25 James Greenhalgh > > * config/aarch64/aarch64.md > (arch64_sqrdmlh_lane): Fix register > constraints for operand 3. > (aarch64_sqrdmlh_laneq): Likewise. > OK /Marcus
Re: [AArch64] Remove AARCH64_EXTRA_TUNE_RECIP_SQRT from Cortex-A57 tuning
On 11 January 2016 at 12:04, James Greenhalgh wrote: > 2015-12-11 James Greenhalgh > > * config/aarch64/aarch64.c (cortexa57_tunings): Remove > AARCH64_EXTRA_TUNE_RECIP_SQRT. > OK /Marcus
Re: [PATCH] Fix up vectorization of multiplication with bool cast to integer (PR tree-optimization/69820)
On Mon, 15 Feb 2016, Jakub Jelinek wrote:
> Hi!
>
> The following testcase is miscompiled, because we first
> create a pattern stmt for _5 = (int) _3; where _3 is bool,
> but then recognize the following multiply as widening multiply, ignore
> there the previous pattern stmt and thus instead of expanding the
> cast as cond ? 1 : 0 we actually end up expanding it as cond (i.e.
> cond ? -1 : 0). In the widen mult pattern recognizer it perhaps would be
> possible to handle that case, unless both arguments are cast from
> bool, but there are lots of other places which call type_conversion_p and
> most of them would need to either give up in those cases or add special
> handling for it. So, it seems like the easiest fix at least for GCC6 is
> to punt in type_conversion_p on casts from bool/unsigned :1 (or should
> I instead check STMT_VINFO_IN_PATTERN_P on the cast stmt and give up
> if true?).
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
Hmm, most places calling type_conversion_p already check for the
pattern case and only accept a very specific or fail.
Only widen_mul/sum don't do that.
I believe type_conversion_p should simply look at the detected pattern,
thus
Index: tree-vect-patterns.c
===
--- tree-vect-patterns.c(revision 233418)
+++ tree-vect-patterns.c(working copy)
@@ -171,6 +171,13 @@ type_conversion_p (tree name, gimple *us
if (!*def_stmt)
return false;
+ if (dt == vect_internal_def)
+{
+ stmt_vec_info def_vinfo = vinfo_for_stmt (*def_stmt);
+ if (STMT_VINFO_IN_PATTERN_P (def_vinfo))
+ *def_stmt = STMT_VINFO_RELATED_STMT (def_vinfo);
+}
+
if (!is_gimple_assign (*def_stmt))
return false;
does that fix it?
Thanks,
Richard.
> 2016-02-15 Jakub Jelinek
>
> PR tree-optimization/69820
> * tree-vect-patterns.c (type_conversion_p): Return false if
> *orig_type is unsigned single precision or boolean.
> (vect_recog_dot_prod_pattern, vect_recog_widen_mult_pattern):
> Formatting fix.
>
> * gcc.dg/vect/pr69820.c: New test.
>
> --- gcc/tree-vect-patterns.c.jj 2016-01-12 14:14:56.0 +0100
> +++ gcc/tree-vect-patterns.c 2016-02-15 18:52:52.695249972 +0100
> @@ -184,6 +184,13 @@ type_conversion_p (tree name, gimple *us
>|| ((TYPE_UNSIGNED (type) != TYPE_UNSIGNED (*orig_type)) &&
> check_sign))
> return false;
>
> + /* Conversion from bool or unsigned single bit precision bitfields
> + should have been recognized by vect_recog_bool_pattern, callers
> + of this function are generally unprepared to handle those. */
> + if ((TYPE_PRECISION (*orig_type) == 1 && TYPE_UNSIGNED (*orig_type))
> + || TREE_CODE (*orig_type) == BOOLEAN_TYPE)
> +return false;
> +
>if (TYPE_PRECISION (type) >= (TYPE_PRECISION (*orig_type) * 2))
> *promotion = true;
>else
> @@ -334,8 +341,8 @@ vect_recog_dot_prod_pattern (vecstmt = last_stmt;
>
>if (type_conversion_p (oprnd0, stmt, true, &half_type, &def_stmt,
> - &promotion)
> - && promotion)
> + &promotion)
> + && promotion)
> {
>stmt = def_stmt;
>oprnd0 = gimple_assign_rhs1 (stmt);
> @@ -395,13 +402,13 @@ vect_recog_dot_prod_pattern (vec|| !types_compatible_p (TREE_TYPE (oprnd1), prod_type))
> return NULL;
>if (!type_conversion_p (oprnd0, stmt, true, &half_type0, &def_stmt,
> -&promotion)
> - || !promotion)
> + &promotion)
> + || !promotion)
> return NULL;
>oprnd00 = gimple_assign_rhs1 (def_stmt);
>if (!type_conversion_p (oprnd1, stmt, true, &half_type1, &def_stmt,
> -&promotion)
> - || !promotion)
> + &promotion)
> + || !promotion)
> return NULL;
>oprnd01 = gimple_assign_rhs1 (def_stmt);
>if (!types_compatible_p (half_type0, half_type1))
> @@ -891,10 +898,10 @@ vect_recog_widen_mult_pattern (vec oprnd = &oprnd1;
> }
>
> -tree old_oprnd = gimple_assign_rhs1 (def_stmt);
> - tree new_oprnd = make_ssa_name (half_type0);
> - new_stmt = gimple_build_assign (new_oprnd, NOP_EXPR, old_oprnd);
> -*oprnd = new_oprnd;
> + tree old_oprnd = gimple_assign_rhs1 (def_stmt);
> + tree new_oprnd = make_ssa_name (half_type0);
> + new_stmt = gimple_build_assign (new_oprnd, NOP_EXPR, old_oprnd);
> + *oprnd = new_oprnd;
> }
>
>/* Handle unsigned case. Look for
> --- gcc/testsuite/gcc.dg/vect/pr69820.c.jj2016-02-15 18:54:15.333122283
> +0100
> +++ gcc/testsuite/gcc.dg/vect/pr69820.c 2016-02-15 18:55:45.529891445
> +0100
> @@ -0,0 +1,35 @@
> +/* PR tree-optimization/69820 */
> +
> +#include "tree-vect.h"
> +
> +unsigned int a[100];
>
[PATCH] Fix half of PR69586
While investigating PR69586 I noticed we fail to register asserts for the seen condition. This is fixed by the below patch. I'm still thinking on how to improve BIT_AND_EXPR handling to catch the missed jump-threading. Boootstrapped and tested on x86_64-unknown-linux-gnu, applied. Richard. 2016-02-16 Richard Biener PR tree-optimization/69586 * tree-vrp.c (register_edge_assert_for_2): Handle all integral types for conversion sources. Index: gcc/tree-vrp.c === --- gcc/tree-vrp.c (revision 233418) +++ gcc/tree-vrp.c (working copy) @@ -5449,7 +5466,7 @@ register_edge_assert_for_2 (tree name, e cst2 = NULL_TREE; if (rhs_code == BIT_AND_EXPR || (CONVERT_EXPR_CODE_P (rhs_code) - && TREE_CODE (TREE_TYPE (val)) == INTEGER_TYPE + && INTEGRAL_TYPE_P (TREE_TYPE (val)) && TYPE_UNSIGNED (TREE_TYPE (val)) && TYPE_PRECISION (TREE_TYPE (gimple_assign_rhs1 (def_stmt))) > prec))
Re: [PATCH] Fix PR69771, bogus CONST_INT during shift expansion
On Tue, 16 Feb 2016, Jakub Jelinek wrote:
> On Mon, Feb 15, 2016 at 08:43:22PM +0100, Richard Biener wrote:
> > On February 15, 2016 7:15:35 PM GMT+01:00, Jakub Jelinek
> > wrote:
> > >On Mon, Feb 15, 2016 at 06:58:45PM +0100, Richard Biener wrote:
> > >> We could also force_reg those at expansion or apply
> > >SHIFT_COUNT_TRUNCATED to those invalid constants there.
> > >
> > >Sure, but for force_reg we'd still need the gen_int_mode anyway.
> > >As for SHIFT_COUNT_TRUNCATED, it should have been applied already from
> > >the
> > >caller - expand_shift_1.
> >
> > But then no out of bound values should remain.
> > Until we get 256bit ints where your workaround wouldn't work either?
>
> Of course it would work, because in that case mode would be OImode, not
> QImode, and thus the code would ensure the shift count is valid OImode
> constant.
Ah, yes - here we're still using op0 mode. For OImode wouldn't we
get a CONST_DOUBLE or CONST_WIDE_INT though?
> Anyway, the patch I've posted has been broken for vector shifts,
> the last argument to gen_int_mode should have been GET_MODE_INNER (mode).
>
> Here is a variant of that patch with force_reg, seems to work on
> aarch64 and x86_64.
Works for me.
Thanks,
Richard.
> 2016-02-16 Jakub Jelinek
>
> PR rtl-optimization/69764
> PR rtl-optimization/69771
> * optabs.c (expand_binop): Ensure for shift optabs invalid CONST_INT
> op1 is valid for GET_MODE_INNER (mode) and force it into a reg.
>
> --- gcc/optabs.c.jj 2016-02-15 22:22:46.161674598 +0100
> +++ gcc/optabs.c 2016-02-16 08:20:01.206889067 +0100
> @@ -1125,6 +1125,16 @@ expand_binop (machine_mode mode, optab b
>op1 = negate_rtx (mode, op1);
>binoptab = add_optab;
> }
> + /* For shifts, constant invalid op1 might be expanded from different
> + mode than MODE. As those are invalid, force them to a register
> + to avoid further problems during expansion. */
> + else if (CONST_INT_P (op1)
> +&& shift_optab_p (binoptab)
> +&& UINTVAL (op1) >= GET_MODE_BITSIZE (GET_MODE_INNER (mode)))
> +{
> + op1 = gen_int_mode (INTVAL (op1), GET_MODE_INNER (mode));
> + op1 = force_reg (GET_MODE_INNER (mode), op1);
> +}
>
>/* Record where to delete back to if we backtrack. */
>last = get_last_insn ();
>
>
> Jakub
>
>
--
Richard Biener
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB
21284 (AG Nuernberg)
Re: [PATCH] Fix PR69771, bogus CONST_INT during shift expansion
On Tue, Feb 16, 2016 at 10:05:55AM +0100, Richard Biener wrote: > On Tue, 16 Feb 2016, Jakub Jelinek wrote: > > > On Mon, Feb 15, 2016 at 08:43:22PM +0100, Richard Biener wrote: > > > On February 15, 2016 7:15:35 PM GMT+01:00, Jakub Jelinek > > > wrote: > > > >On Mon, Feb 15, 2016 at 06:58:45PM +0100, Richard Biener wrote: > > > >> We could also force_reg those at expansion or apply > > > >SHIFT_COUNT_TRUNCATED to those invalid constants there. > > > > > > > >Sure, but for force_reg we'd still need the gen_int_mode anyway. > > > >As for SHIFT_COUNT_TRUNCATED, it should have been applied already from > > > >the > > > >caller - expand_shift_1. > > > > > > But then no out of bound values should remain. > > > Until we get 256bit ints where your workaround wouldn't work either? > > > > Of course it would work, because in that case mode would be OImode, not > > QImode, and thus the code would ensure the shift count is valid OImode > > constant. > > Ah, yes - here we're still using op0 mode. For OImode wouldn't we > get a CONST_DOUBLE or CONST_WIDE_INT though? I think the C/C++ FEs use unsigned int for the type of the shift count in any case, so op1 will still be CONST_INT. > > Here is a variant of that patch with force_reg, seems to work on > > aarch64 and x86_64. > > Works for me. Ok, will bootstrap/regtest it today and commit if it passes testing. Jakub
Re: [PATCH] Fix reassoc ICE (PR tree-optimization/69802)
On Mon, 15 Feb 2016, Jakub Jelinek wrote:
> Hi!
>
> The following patch fixes an ICE where one of the range tests
> is SSA_NAME_DEF_STMT of a bool/_Bool or unsigned : 1 bitfield.
> In that case, we don't know where to put the adjusted range test.
> The patch for this uncommon case gives up, unless the range test
> can be the SSA_NAME_DEF_STMT itself, and in that case makes sure we
> DTRT.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
Ok.
Richard.
> 2016-02-15 Jakub Jelinek
>
> PR tree-optimization/69802
> * tree-ssa-reassoc.c (update_range_test): If op is
> SSA_NAME_IS_DEFAULT_DEF, give up unless tem is a positive
> op == 1 test of precision 1 integral op, otherwise handle
> that case as op itself. Fix up formatting.
> (optimize_range_tests_to_bit_test, optimize_range_tests): Fix
> up formatting.
>
> * gcc.dg/pr69802.c: New test.
>
> --- gcc/tree-ssa-reassoc.c.jj 2016-02-12 10:23:51.0 +0100
> +++ gcc/tree-ssa-reassoc.c2016-02-15 14:25:54.996572238 +0100
> @@ -2046,19 +2046,41 @@ update_range_test (struct range_entry *r
> {
>operand_entry *oe = (*ops)[range->idx];
>tree op = oe->op;
> - gimple *stmt = op ? SSA_NAME_DEF_STMT (op) :
> -last_stmt (BASIC_BLOCK_FOR_FN (cfun, oe->id));
> + gimple *stmt = op ? SSA_NAME_DEF_STMT (op)
> + : last_stmt (BASIC_BLOCK_FOR_FN (cfun, oe->id));
>location_t loc = gimple_location (stmt);
>tree optype = op ? TREE_TYPE (op) : boolean_type_node;
>tree tem = build_range_check (loc, optype, unshare_expr (exp),
> in_p, low, high);
>enum warn_strict_overflow_code wc = WARN_STRICT_OVERFLOW_COMPARISON;
>gimple_stmt_iterator gsi;
> - unsigned int i;
> + unsigned int i, uid;
>
>if (tem == NULL_TREE)
> return false;
>
> + /* If op is default def SSA_NAME, there is no place to insert the
> + new comparison. Give up, unless we can use OP itself as the
> + range test. */
> + if (op && SSA_NAME_IS_DEFAULT_DEF (op))
> +{
> + if (op == range->exp
> + && ((TYPE_PRECISION (optype) == 1 && TYPE_UNSIGNED (optype))
> + || TREE_CODE (optype) == BOOLEAN_TYPE)
> + && (op == tem
> + || (TREE_CODE (tem) == EQ_EXPR
> + && TREE_OPERAND (tem, 0) == op
> + && integer_onep (TREE_OPERAND (tem, 1
> + && opcode != BIT_IOR_EXPR
> + && (opcode != ERROR_MARK || oe->rank != BIT_IOR_EXPR))
> + {
> + stmt = NULL;
> + tem = op;
> + }
> + else
> + return false;
> +}
> +
>if (strict_overflow_p && issue_strict_overflow_warning (wc))
> warning_at (loc, OPT_Wstrict_overflow,
> "assuming signed overflow does not occur "
> @@ -2096,12 +2118,22 @@ update_range_test (struct range_entry *r
> tem = invert_truthvalue_loc (loc, tem);
>
>tem = fold_convert_loc (loc, optype, tem);
> - gsi = gsi_for_stmt (stmt);
> - unsigned int uid = gimple_uid (stmt);
> + if (stmt)
> +{
> + gsi = gsi_for_stmt (stmt);
> + uid = gimple_uid (stmt);
> +}
> + else
> +{
> + gsi = gsi_none ();
> + uid = 0;
> +}
> + if (stmt == NULL)
> +gcc_checking_assert (tem == op);
>/* In rare cases range->exp can be equal to lhs of stmt.
> In that case we have to insert after the stmt rather then before
> it. If stmt is a PHI, insert it at the start of the basic block. */
> - if (op != range->exp)
> + else if (op != range->exp)
> {
>gsi_insert_seq_before (&gsi, seq, GSI_SAME_STMT);
>tem = force_gimple_operand_gsi (&gsi, tem, true, NULL_TREE, true,
> @@ -2489,7 +2521,7 @@ optimize_range_tests_to_bit_test (enum t
> operand_entry *oe = (*ops)[ranges[i].idx];
> tree op = oe->op;
> gimple *stmt = op ? SSA_NAME_DEF_STMT (op)
> -: last_stmt (BASIC_BLOCK_FOR_FN (cfun, oe->id));
> + : last_stmt (BASIC_BLOCK_FOR_FN (cfun, oe->id));
> location_t loc = gimple_location (stmt);
> tree optype = op ? TREE_TYPE (op) : boolean_type_node;
>
> @@ -2553,7 +2585,7 @@ optimize_range_tests_to_bit_test (enum t
> gcc_assert (TREE_CODE (exp) == SSA_NAME);
> gimple_set_visited (SSA_NAME_DEF_STMT (exp), true);
> gimple *g = gimple_build_assign (make_ssa_name (optype),
> - BIT_IOR_EXPR, tem, exp);
> +BIT_IOR_EXPR, tem, exp);
> gimple_set_location (g, loc);
> gimple_seq_add_stmt_without_update (&seq, g);
> exp = gimple_assign_lhs (g);
> @@ -2599,8 +2631,9 @@ optimize_range_tests (enum tree_code opc
>oe = (*ops)[i];
>ranges[i].idx = i;
>init_range_entry (ranges + i, oe->op,
> - oe->op ? NULL :
> - last_stmt (BASIC_BLOCK_FOR_FN (cfun, oe->id)));
> + oe->op
> +
Re: [PATCH] Fix up vectorization of multiplication with bool cast to integer (PR tree-optimization/69820)
On Tue, Feb 16, 2016 at 09:58:37AM +0100, Richard Biener wrote:
> > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
>
> Hmm, most places calling type_conversion_p already check for the
> pattern case and only accept a very specific or fail.
>
> Only widen_mul/sum don't do that.
>
> I believe type_conversion_p should simply look at the detected pattern,
> thus
>
> Index: tree-vect-patterns.c
> ===
> --- tree-vect-patterns.c(revision 233418)
> +++ tree-vect-patterns.c(working copy)
> @@ -171,6 +171,13 @@ type_conversion_p (tree name, gimple *us
>if (!*def_stmt)
> return false;
>
> + if (dt == vect_internal_def)
> +{
> + stmt_vec_info def_vinfo = vinfo_for_stmt (*def_stmt);
> + if (STMT_VINFO_IN_PATTERN_P (def_vinfo))
> + *def_stmt = STMT_VINFO_RELATED_STMT (def_vinfo);
> +}
> +
>if (!is_gimple_assign (*def_stmt))
> return false;
>
> does that fix it?
This doesn't work.
pr69820.c: In function ‘foo’:
pr69820.c:6:1: internal compiler error: in vect_get_vec_def_for_operand, at
tree-vect-stmts.c:1424
foo (void)
^~~
0x106f8c6 vect_get_vec_def_for_operand(tree_node*, gimple*, tree_node*)
../../gcc/tree-vect-stmts.c:1424
0x1078daf vectorizable_conversion
../../gcc/tree-vect-stmts.c:4087
0x1085da5 vect_transform_stmt(gimple*, gimple_stmt_iterator*, bool*,
_slp_tree*, _slp_instance*)
../../gcc/tree-vect-stmts.c:8191
...
I think the vectorizer is not prepared to see one pattern seq referencing
SSA_NAME set by a different pattern seq.
Jakub
[PATCH][AArch64] Skip gcc.target/aarch64/assembler_arch_1.c if assembler does not support it
Hi all,
As Christophe reported at:
https://gcc.gnu.org/ml/gcc-patches/2016-02/msg00784.html
The test gcc.target/aarch64/assembler_arch_1.c fails to assemble on older
assemblers that
don't support the LSE architecture extension.
I'd really like to keep the test an assemble test rather than just a compile
one since
it is extracted from a real error when building the Linux kernel, so this patch
is
the simplest way I can think of skipping the test for older assemblers.
I've tested that the test appears UNSUPPORTED when used with a binutils that
doesn't
support LSE and PASSes normally on latest binutils.
Do we want to have such a specialised check for assembler capabilities?
Or do we want to just let this test FAIL on older assemblers and see this
failure go away
in the future as users/testers migrate to later binutils releases?
If such a check is desired, here's a patch that does it.
Thanks,
Kyrill
2016-02-16 Kyrylo Tkachov
* lib/target-supports.exp
(check_effective_target_aarch64_lse_assembler): New effective target
check.
* gcc.target/aarch64/assembler_arch_1.c: Add arch64_lse_assembler
effective target check.
diff --git a/gcc/testsuite/gcc.target/aarch64/assembler_arch_1.c b/gcc/testsuite/gcc.target/aarch64/assembler_arch_1.c
index 901e50a178d7a4a443a5ad0abe63f624688db268..ef34e76cef9c3076a3c32e76aa7a7f658774f786 100644
--- a/gcc/testsuite/gcc.target/aarch64/assembler_arch_1.c
+++ b/gcc/testsuite/gcc.target/aarch64/assembler_arch_1.c
@@ -1,4 +1,5 @@
/* { dg-do assemble } */
+/* { dg-require-effective-target aarch64_lse_assembler } */
/* { dg-options "-march=armv8-a" } */
/* Make sure that the function header in assembly doesn't override
diff --git a/gcc/testsuite/lib/target-supports.exp b/gcc/testsuite/lib/target-supports.exp
index 645981a8733b68a7c37919a53b324841d2c125aa..08b4eacc26f44506d00fda58f99c203a4199856b 100644
--- a/gcc/testsuite/lib/target-supports.exp
+++ b/gcc/testsuite/lib/target-supports.exp
@@ -6718,6 +6718,18 @@ proc check_effective_target_aarch64_tiny { } {
}
}
+# Return 1 if the target AArch64 assembler supports the LSE extensions.
+
+proc check_effective_target_aarch64_lse_assembler { } {
+if { [istarget aarch64*-*-*] } {
+ return [check_no_compiler_messages aarch64_lse_assembler object {
+ __asm__ (".arch_extension lse");
+ } "-march=armv8-a+lse"]
+} else {
+ return 0
+}
+}
+
proc check_effective_target_aarch64_small { } {
if { [istarget aarch64*-*-*] } {
return [check_no_compiler_messages aarch64_small object {
Re: [PATCH][AArch64] Skip gcc.target/aarch64/assembler_arch_1.c if assembler does not support it
On Tue, Feb 16, 2016 at 09:27:00AM +, Kyrill Tkachov wrote:
> Hi all,
>
> As Christophe reported at:
> https://gcc.gnu.org/ml/gcc-patches/2016-02/msg00784.html
>
> The test gcc.target/aarch64/assembler_arch_1.c fails to assemble on older
> assemblers that don't support the LSE architecture extension.
Actually, Christophe's report is that the ".arch_extension" directive
isn't understood by older assemblers...
> I'd really like to keep the test an assemble test rather than just a compile
> one since it is extracted from a real error when building the Linux kernel,
> so this patch is the simplest way I can think of skipping the test for older
> assemblers.
>
> I've tested that the test appears UNSUPPORTED when used with a binutils that
> doesn't support LSE and PASSes normally on latest binutils.
>
> Do we want to have such a specialised check for assembler capabilities? Or
> do we want to just let this test FAIL on older assemblers and see this
> failure go away in the future as users/testers migrate to later binutils
> releases?
...so, this test conflates two features we're asking for from the assembler,
support for LSE and support for .arch_extension. For your test, that's
exactly what you need, and for binutils it will never matter, but it feels
like a misnamed check to me.
> If such a check is desired, here's a patch that does it.
Yes, the check is probably useful (if we need to test other LSE features, we
will want it - e.g. if we wanted to bump some of the armv8.1+lse atomics
tests up to assemble tests).
Anyway, this patch is OK by me as is, as I don't see a way to disambiguate
the test name without ending up with two tiny effective-target checks. But
maybe someone who knows the testsuite better would have other opinions?
> 2016-02-16 Kyrylo Tkachov
>
> * lib/target-supports.exp
> (check_effective_target_aarch64_lse_assembler): New effective target
> check.
> * gcc.target/aarch64/assembler_arch_1.c: Add arch64_lse_assembler
> effective target check.
> diff --git a/gcc/testsuite/gcc.target/aarch64/assembler_arch_1.c
> b/gcc/testsuite/gcc.target/aarch64/assembler_arch_1.c
> index
> 901e50a178d7a4a443a5ad0abe63f624688db268..ef34e76cef9c3076a3c32e76aa7a7f658774f786
> 100644
> --- a/gcc/testsuite/gcc.target/aarch64/assembler_arch_1.c
> +++ b/gcc/testsuite/gcc.target/aarch64/assembler_arch_1.c
> @@ -1,4 +1,5 @@
> /* { dg-do assemble } */
> +/* { dg-require-effective-target aarch64_lse_assembler } */
> /* { dg-options "-march=armv8-a" } */
>
> /* Make sure that the function header in assembly doesn't override
> diff --git a/gcc/testsuite/lib/target-supports.exp
> b/gcc/testsuite/lib/target-supports.exp
> index
> 645981a8733b68a7c37919a53b324841d2c125aa..08b4eacc26f44506d00fda58f99c203a4199856b
> 100644
> --- a/gcc/testsuite/lib/target-supports.exp
> +++ b/gcc/testsuite/lib/target-supports.exp
> @@ -6718,6 +6718,18 @@ proc check_effective_target_aarch64_tiny { } {
> }
> }
>
> +# Return 1 if the target AArch64 assembler supports the LSE extensions.
I think the "extensions" here is redundant :-).
Thanks,
James
Re: [PING][PATCH, PR67709 ] Don't call call_cgraph_insertion_hooks in simd_clone_create
On 16/02/16 03:22, Jan Hubicka wrote: On 08/02/16 13:54, Jakub Jelinek wrote: On Mon, Feb 08, 2016 at 01:46:44PM +0100, Tom de Vries wrote: [ The pass before pass_omp_simd_clone is pass_dispatcher_calls. It has a function create_target_clone, similar to simd_clone_create, with a node.defition and !node.defition part. The !node.defition part does not call 'symtab->call_cgraph_insertion_hooks (new_node)'. ] I'll defer to Honza or Richi if it is ok not to call cgraph insertion hooks at this point (and since when they can be avoided), or what else should be done. The patch could be ok even for 6.0, not just stage1, if they are ok with it (or propose some other change). Ping (Given that Jakub suggested this or an alternative patch might be included in 6.0 stage4). Original submission at https://gcc.gnu.org/ml/gcc-patches/2016-02/msg00543.html . OK, so the ICE is that you intorduce new deifnition but you set in_other_partition and do not really introudce gimple body for it? In that case we should indeed not call creation hooks, because these are used only when new function is introduced to a given partition. Patch looks OK to me thus. Committed to trunk. AFAIU, it's not a release regression given that: - this has failed since 4.9.0, and - the test-case is not supported in 4.8, so we're not required to fix it in 4.9 and 5 branches. But, the test-case does fail in 5 and 4.9 branches, and the fix seems simple enough to not have the risk of introducing breakage in the release branches. OK for 4.9/5 branch? Thanks, - Tom Honza. Thanks, - Tom Don't call call_cgraph_insertion_hooks in simd_clone_create 2016-02-08 Tom de Vries PR lto/67709 * omp-low.c (simd_clone_create): Remove call to symtab->call_cgraph_insertion_hooks. * testsuite/libgomp.fortran/declare-simd-4.f90: New test. Jakub
Re: [PING][PATCH, PR67709 ] Don't call call_cgraph_insertion_hooks in simd_clone_create
On Tue, Feb 16, 2016 at 10:56:58AM +0100, Tom de Vries wrote: > AFAIU, it's not a release regression given that: > - this has failed since 4.9.0, and > - the test-case is not supported in 4.8, > so we're not required to fix it in 4.9 and 5 branches. > > But, the test-case does fail in 5 and 4.9 branches, and the fix seems simple > enough to not have the risk of introducing breakage in the release branches. > > OK for 4.9/5 branch? Okay. > 2016-02-08 Tom de Vries > > PR lto/67709 > * omp-low.c (simd_clone_create): Remove call to > symtab->call_cgraph_insertion_hooks. > > * testsuite/libgomp.fortran/declare-simd-4.f90: New test. Jakub
Re: [PATCH] Fix up vectorization of multiplication with bool cast to integer (PR tree-optimization/69820)
On Tue, 16 Feb 2016, Jakub Jelinek wrote:
> On Tue, Feb 16, 2016 at 09:58:37AM +0100, Richard Biener wrote:
> > > Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
> >
> > Hmm, most places calling type_conversion_p already check for the
> > pattern case and only accept a very specific or fail.
> >
> > Only widen_mul/sum don't do that.
> >
> > I believe type_conversion_p should simply look at the detected pattern,
> > thus
> >
> > Index: tree-vect-patterns.c
> > ===
> > --- tree-vect-patterns.c(revision 233418)
> > +++ tree-vect-patterns.c(working copy)
> > @@ -171,6 +171,13 @@ type_conversion_p (tree name, gimple *us
> >if (!*def_stmt)
> > return false;
> >
> > + if (dt == vect_internal_def)
> > +{
> > + stmt_vec_info def_vinfo = vinfo_for_stmt (*def_stmt);
> > + if (STMT_VINFO_IN_PATTERN_P (def_vinfo))
> > + *def_stmt = STMT_VINFO_RELATED_STMT (def_vinfo);
> > +}
> > +
> >if (!is_gimple_assign (*def_stmt))
> > return false;
> >
> > does that fix it?
>
> This doesn't work.
>
> pr69820.c: In function ‘foo’:
> pr69820.c:6:1: internal compiler error: in vect_get_vec_def_for_operand, at
> tree-vect-stmts.c:1424
> foo (void)
> ^~~
> 0x106f8c6 vect_get_vec_def_for_operand(tree_node*, gimple*, tree_node*)
> ../../gcc/tree-vect-stmts.c:1424
> 0x1078daf vectorizable_conversion
> ../../gcc/tree-vect-stmts.c:4087
> 0x1085da5 vect_transform_stmt(gimple*, gimple_stmt_iterator*, bool*,
> _slp_tree*, _slp_instance*)
> ../../gcc/tree-vect-stmts.c:8191
> ...
>
> I think the vectorizer is not prepared to see one pattern seq referencing
> SSA_NAME set by a different pattern seq.
Hmm, I think it works in general, but I suspect that the pattern has
to use the original def but for out analysis we have to look at the
pattern.
So another fix would be to simply fail if there was a pattern detected.
Richard.
Re: [Patch, regex, libstdc++/69794] Unify special character parsing
On 15/02/16 20:41 -0800, Tim Shen wrote: PR libstdc++/69794 * include/bits/regex_scanner.h: Add different special character sets for grep and egrep regex. * include/bits/regex_scanner.tcc: Use _M_spec_char more unifiedly. s/unifiedly/uniformly/ OK for trunk and gcc-5-branch with that change, thanks for fixing the regression.
Re: [AArch64] Remove AARCH64_EXTRA_TUNE_RECIP_SQRT from Cortex-A57 tuning
On Mon, Feb 15, 2016 at 11:24:53AM -0600, Evandro Menezes wrote: > On 02/15/16 04:50, James Greenhalgh wrote: > >On Mon, Feb 08, 2016 at 10:57:10AM +, James Greenhalgh wrote: > >>On Mon, Feb 01, 2016 at 02:00:01PM +, James Greenhalgh wrote: > >>>On Mon, Jan 25, 2016 at 11:20:46AM +, James Greenhalgh wrote: > On Mon, Jan 11, 2016 at 12:04:43PM +, James Greenhalgh wrote: > >Hi, > > > >I've seen a couple of large performance issues caused by expanding > >the high-precision reciprocal square root for Cortex-A57, so I'd like > >to turn it off by default. > > > >This is good for art (~2%) from Spec2000, bad (~3.5%) for fma3d from > >Spec2000, good (~5.5%) for gromcas from Spec2006, and very good (>10%) > >for > >some private microbenchmark kernels which stress the divide/sqrt/multiply > >units. It therefore seems to me to be the correct choice to make across > >a number of workloads. > > > >Bootstrapped and tested on aarch64-none-linux-gnu with no issues. > > > >OK? > *Ping* > >>>*pingx2* > >>*ping^3* > >*ping^4* > > > >Thanks, > >James > > > >--- > >2015-12-11 James Greenhalgh > > > > * config/aarch64/aarch64.c (cortexa57_tunings): Remove > > AARCH64_EXTRA_TUNE_RECIP_SQRT. > > > >diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c > >index 1d5d898..999c9fc 100644 > >--- a/gcc/config/aarch64/aarch64.c > >+++ b/gcc/config/aarch64/aarch64.c > >@@ -484,8 +484,7 @@ static const struct tune_params cortexa57_tunings = > >0, /* max_case_values. */ > >0, /* cache_line_size. */ > >tune_params::AUTOPREFETCHER_WEAK,/* autoprefetcher_model. */ > >- (AARCH64_EXTRA_TUNE_RENAME_FMA_REGS > >- | AARCH64_EXTRA_TUNE_RECIP_SQRT) /* tune_flags. */ > >+ (AARCH64_EXTRA_TUNE_RENAME_FMA_REGS) /* tune_flags. */ > > }; > > static const struct tune_params cortexa72_tunings = > > > > James, > > There seem to be SPEC CPU2000fp validation issues on A57 when this > flag is present too. Though I evaluated the algorithm with a huge > random set of values, always delivering accuracy around 1ulp, which > should be enough for CPU2000fp (wit x86-64), I expected the > benchmarks to pass. > > My suspicion is that the Newton series on AArch64 is probably good > only for SP. Then, DP might require an extra round, probably > exacerbating the performance penalty. > > I'd like to try to split this tuning option into one for SP and > another for DP. Thoughts? I haven't seen validation issues with the default expansion, but with -mlow-precision-recip-sqrt I do see failures. I think this is to be expected. I don't support splitting the low-precision flag to "-mlow-precision-float-recip-sqrt" and "-mlow-precision-double-recip-sqrt", I think that is pushing a particular set of Spec tuning flags over any meaningful use case. I could imagine a case for splitting the internal tuning flag to give AARCH64_EXTRA_TUNE_SF_RECIP_SQRT and AARCH64_EXTRA_TUNE_DF_RECIP_SQRT, but I'm not sure I understand the benefits of this? Certainly, I think your goals for performance (turn on for 64-bit divide/sqrt) would contradict your goals for accuracy (turn off for 64-bit divide/sqrt). I'm happy with these flags as they are, but I might be missing a subtelty in your argument? Thanks, James
Re: C++ PATCH for c++/69753 (DR141 broke member template lookup)
On 2016.02.15 at 16:13 -0500, Jason Merrill wrote:
> When we stopped finding function templates with unqualified lookup due to
> the DR141 fix, that exposed bugs in our lookup within the object expression
> scope; an object-expression of the current instantiation does not make the
> expression dependent. This patch fixes this issue specifically for implicit
> "this->", which is the case in question in this PR; addressing this issue
> more generally will take more work.
>
> Tested x86_64-pc-linux-gnu, applying to trunk.
> diff --git a/gcc/testsuite/g++.dg/lookup/member3.C
> b/gcc/testsuite/g++.dg/lookup/member3.C
> new file mode 100644
> index 000..f4e097e4
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/lookup/member3.C
> @@ -0,0 +1,17 @@
> +// PR c++/69753
> +// { dg-do compile { target c++11 } }
> +
> +class A {
> +public:
> + template void As();
> + static A *FromWebContents();
> + A *FromWebContents2();
> +};
> +template class B : A {
> + void FromWebContents() {
> +auto guest = A::FromWebContents();
> +guest ? guest->As() : nullptr;
> +auto guest2 = A::FromWebContents2();
> +guest2 ? guest2->As() : nullptr;
> + }
> +};
Please note that clang rejects the testcase in the non static case:
gcc/testsuite/g++.dg/lookup/member3.C:15:22: error: use 'template' keyword to
treat 'As' as a dependent template name
guest2 ? guest2->As() : nullptr;
^
template
1 error generated.
Here is another testcase that every compiler I've tested (clang, icc,
microsoft) accepts, but is rejected by gcc-6:
class A {
public:
template void m_fn1();
};
A *fn1(int *);
template class B : A {
static int *m_fn2() { fn1(m_fn2())->m_fn1(); }
};
--
Markus
Re: [PATCH] Fix up vectorization of multiplication with bool cast to integer (PR tree-optimization/69820)
On Tue, Feb 16, 2016 at 11:06:53AM +0100, Richard Biener wrote:
> Hmm, I think it works in general, but I suspect that the pattern has
> to use the original def but for out analysis we have to look at the
> pattern.
>
> So another fix would be to simply fail if there was a pattern detected.
That seems to work, sure. So is that ok if it passes bootstrap/regtest?
2016-02-15 Jakub Jelinek
Richard Biener
PR tree-optimization/69820
* tree-vect-patterns.c (type_conversion_p): Return false if
*orig_type is unsigned single precision or boolean.
(vect_recog_dot_prod_pattern, vect_recog_widen_mult_pattern):
Formatting fix.
* gcc.dg/vect/pr69820.c: New test.
--- gcc/tree-vect-patterns.c.jj 2016-02-15 22:22:45.958677390 +0100
+++ gcc/tree-vect-patterns.c2016-02-16 11:12:20.982147520 +0100
@@ -171,6 +171,13 @@ type_conversion_p (tree name, gimple *us
if (!*def_stmt)
return false;
+ if (dt == vect_internal_def)
+{
+ stmt_vec_info def_vinfo = vinfo_for_stmt (*def_stmt);
+ if (STMT_VINFO_IN_PATTERN_P (def_vinfo))
+ return false;
+}
+
if (!is_gimple_assign (*def_stmt))
return false;
@@ -334,8 +341,8 @@ vect_recog_dot_prod_pattern (vec
Re: [PATCH, ARM] stop changing signedness in PROMOTE_MODE
On Mon, Feb 15, 2016 at 11:32 AM, Kyrill Tkachov wrote: > > On 04/02/16 08:58, Ramana Radhakrishnan wrote: >> >> On Tue, Jun 30, 2015 at 2:15 AM, Jim Wilson wrote: >>> >>> This is my suggested fix for PR 65932, which is a linux kernel >>> miscompile with gcc-5.1. >>> >>> The problem here is caused by a chain of events. The first is that >>> the relatively new eipa_sra pass creates fake parameters that behave >>> slightly differently than normal parameters. The second is that the >>> optimizer creates phi nodes that copy local variables to fake >>> parameters and/or vice versa. The third is that the ouf-of-ssa pass >>> assumes that it can emit simple move instructions for these phi nodes. >>> And the fourth is that the ARM port has a PROMOTE_MODE macro that >>> forces QImode and HImode to unsigned, but a >>> TARGET_PROMOTE_FUNCTION_MODE hook that does not. So signed char and >>> short parameters have different in register representations than local >>> variables, and require a conversion when copying between them, a >>> conversion that the out-of-ssa pass can't easily emit. >>> >>> Ultimately, I think this is a problem in the arm backend. It should >>> not have a PROMOTE_MODE macro that is changing the sign of char and >>> short local variables. I also think that we should merge the >>> PROMOTE_MODE macro with the TARGET_PROMOTE_FUNCTION_MODE hook to >>> prevent this from happening again. >>> >>> I see four general problems with the current ARM PROMOTE_MODE definition. >>> 1) Unsigned char is only faster for armv5 and earlier, before the sxtb >>> instruction was added. It is a lose for armv6 and later. >>> 2) Unsigned short was only faster for targets that don't support >>> unaligned accesses. Support for these targets was removed a while >>> ago, and this PROMODE_MODE hunk should have been removed at the same >>> time. It was accidentally left behind. >>> 3) TARGET_PROMOTE_FUNCTION_MODE used to be a boolean hook, when it was >>> converted to a function, the PROMOTE_MODE code was copied without the >>> UNSIGNEDP changes. Thus it is only an accident that >>> TARGET_PROMOTE_FUNCTION_MODE and PROMOTE_MODE disagree. Changing >>> TARGET_PROMOTE_FUNCTION_MODE is an ABI change, so only PROMOTE_MODE >>> changes to resolve the difference are safe. >>> 4) There is a general principle that you should only change signedness >>> in PROMOTE_MODE if the hardware forces it, as otherwise this results >>> in extra conversion instructions that make code slower. The mips64 >>> hardware for instance requires that 32-bit values be sign-extended >>> regardless of type, and instructions may trap if this is not true. >>> However, it has a set of 32-bit instructions that operate on these >>> values, and hence no conversions are required. There is no similar >>> case on ARM. Thus the conversions are unnecessary and unwise. This >>> can be seen in the testcases where gcc emits both a zero-extend and a >>> sign-extend inside a loop, as the sign-extend is required for a >>> compare, and the zero-extend is required by PROMOTE_MODE. >> >> Given Kyrill's testing with the patch and the reasonably detailed >> check of the effects of code generation changes - The arm.h hunk is ok >> - I do think we should make this explicit in the documentation that >> TARGET_PROMOTE_MODE and TARGET_PROMOTE_FUNCTION_MODE should agree and >> better still maybe put in a checking assert for the same in the >> mid-end but that could be the subject of a follow-up patch. >> >> Ok to apply just the arm.h hunk as I think Kyrill has taken care of >> the testsuite fallout separately. > > Hi all, > > I'd like to backport the arm.h from this ( r233130) to the GCC 5 > branch. As the CSE patch from my series had some fallout on x86_64 > due to a deficiency in the AVX patterns that is too invasive to fix > at this stage (and presumably backport), I'd like to just backport > this arm.h fix and adjust the tests to XFAIL the fallout that comes > with not applying the CSE patch. The attached patch does that. I would hope that someone looks to fix the AVX port for GCC 7 - as the CSE patch is something that is useful on ARM for performance in real terms and could have benefits elsewhere. OK to apply if no regressions - thanks for pushing this through. Thanks, Ramana > > The code quality fallout on code outside the testsuite is not > that gread. The SPEC benchmarks are not affected by not applying > the CSE change, and only a single sequence in a popular embedded benchmark > shows some degradation for -mtune=cortex-a9 in the same way as the > wmul-1.c and wmul-2.c tests. > > I think that's a fair tradeoff for fixing the wrong code bug on that branch. > > Ok to backport r233130 and the attached testsuite patch to the GCC 5 branch? > > Thanks, > Kyrill > > 2016-02-15 Kyrylo Tkachov > > PR target/65932 > * gcc.target/arm/wmul-1.c: Add -mtune=cortex-a9 to dg-options. > xfail the scan-assembler test. > * gcc.target/arm/wmul-2.c: Likewise. > *
[PATCH] Fixup PR69291 fix
Bootstrapped and tested on x86_64-unknown-linux-gnu, applied.
Richard.
2016-02-16 Richard Biener
PR rtl-optimization/69291
* ifcvt.c (noce_try_store_flag_constants): Re-instantiate
noce_operand_ok check.
Index: gcc/ifcvt.c
===
--- gcc/ifcvt.c (revision 233443)
+++ gcc/ifcvt.c (working copy)
@@ -1277,7 +1277,8 @@ noce_try_store_flag_constants (struct no
/* Allow expressions that are not using the result or plain
registers where we handle overlap below. */
&& (REG_P (XEXP (a, 0))
- || ! reg_overlap_mentioned_p (if_info->x, XEXP (a, 0)))
+ || (noce_operand_ok (XEXP (a, 0))
+ && ! reg_overlap_mentioned_p (if_info->x, XEXP (a, 0
&& if_info->branch_cost >= 2)
{
common = XEXP (a, 0);
Re: [PATCH] Fix up vectorization of multiplication with bool cast to integer (PR tree-optimization/69820)
On Tue, 16 Feb 2016, Jakub Jelinek wrote:
> On Tue, Feb 16, 2016 at 11:06:53AM +0100, Richard Biener wrote:
> > Hmm, I think it works in general, but I suspect that the pattern has
> > to use the original def but for out analysis we have to look at the
> > pattern.
> >
> > So another fix would be to simply fail if there was a pattern detected.
>
> That seems to work, sure. So is that ok if it passes bootstrap/regtest?
Ok.
Thanks,
Richard.
> 2016-02-15 Jakub Jelinek
> Richard Biener
>
> PR tree-optimization/69820
> * tree-vect-patterns.c (type_conversion_p): Return false if
> *orig_type is unsigned single precision or boolean.
> (vect_recog_dot_prod_pattern, vect_recog_widen_mult_pattern):
> Formatting fix.
>
> * gcc.dg/vect/pr69820.c: New test.
>
> --- gcc/tree-vect-patterns.c.jj 2016-02-15 22:22:45.958677390 +0100
> +++ gcc/tree-vect-patterns.c 2016-02-16 11:12:20.982147520 +0100
> @@ -171,6 +171,13 @@ type_conversion_p (tree name, gimple *us
>if (!*def_stmt)
> return false;
>
> + if (dt == vect_internal_def)
> +{
> + stmt_vec_info def_vinfo = vinfo_for_stmt (*def_stmt);
> + if (STMT_VINFO_IN_PATTERN_P (def_vinfo))
> + return false;
> +}
> +
>if (!is_gimple_assign (*def_stmt))
> return false;
>
> @@ -334,8 +341,8 @@ vect_recog_dot_prod_pattern (vecstmt = last_stmt;
>
>if (type_conversion_p (oprnd0, stmt, true, &half_type, &def_stmt,
> - &promotion)
> - && promotion)
> + &promotion)
> + && promotion)
> {
>stmt = def_stmt;
>oprnd0 = gimple_assign_rhs1 (stmt);
> @@ -395,13 +402,13 @@ vect_recog_dot_prod_pattern (vec|| !types_compatible_p (TREE_TYPE (oprnd1), prod_type))
> return NULL;
>if (!type_conversion_p (oprnd0, stmt, true, &half_type0, &def_stmt,
> -&promotion)
> - || !promotion)
> + &promotion)
> + || !promotion)
> return NULL;
>oprnd00 = gimple_assign_rhs1 (def_stmt);
>if (!type_conversion_p (oprnd1, stmt, true, &half_type1, &def_stmt,
> -&promotion)
> - || !promotion)
> + &promotion)
> + || !promotion)
> return NULL;
>oprnd01 = gimple_assign_rhs1 (def_stmt);
>if (!types_compatible_p (half_type0, half_type1))
> @@ -891,10 +898,10 @@ vect_recog_widen_mult_pattern (vec oprnd = &oprnd1;
> }
>
> -tree old_oprnd = gimple_assign_rhs1 (def_stmt);
> - tree new_oprnd = make_ssa_name (half_type0);
> - new_stmt = gimple_build_assign (new_oprnd, NOP_EXPR, old_oprnd);
> -*oprnd = new_oprnd;
> + tree old_oprnd = gimple_assign_rhs1 (def_stmt);
> + tree new_oprnd = make_ssa_name (half_type0);
> + new_stmt = gimple_build_assign (new_oprnd, NOP_EXPR, old_oprnd);
> + *oprnd = new_oprnd;
> }
>
>/* Handle unsigned case. Look for
> --- gcc/testsuite/gcc.dg/vect/pr69820.c.jj2016-02-16 10:21:40.155236476
> +0100
> +++ gcc/testsuite/gcc.dg/vect/pr69820.c 2016-02-16 10:21:40.155236476
> +0100
> @@ -0,0 +1,35 @@
> +/* PR tree-optimization/69820 */
> +
> +#include "tree-vect.h"
> +
> +unsigned int a[100];
> +long long int b[100];
> +unsigned short c[100];
> +
> +__attribute__((noinline, noclone)) void
> +foo (void)
> +{
> + int i;
> + for (i = 0; i < 100; ++i)
> +b[i] = a[i] * (c[i] * (_Bool) c[i]);
> +}
> +
> +int
> +main ()
> +{
> + int i;
> + if (__SIZEOF_INT__ * __CHAR_BIT__ != 32)
> +return 0;
> + check_vect ();
> + for (i = 0; i < 100; ++i)
> +{
> + a[i] = 3489456818U;
> + b[i] = 0x1eadbeefbeefdeadLL;
> + c[i] = 38364;
> +}
> + foo ();
> + for (i = 0; i < 100; ++i)
> +if (b[i] != 0xed446af8U)
> + __builtin_abort ();
> + return 0;
> +}
>
> Jakub
>
>
--
Richard Biener
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB
21284 (AG Nuernberg)
Re: [PING][PATCH, PR67709 ] Don't call call_cgraph_insertion_hooks in simd_clone_create
On 16/02/16 11:04, Jakub Jelinek wrote: On Tue, Feb 16, 2016 at 10:56:58AM +0100, Tom de Vries wrote: >AFAIU, it's not a release regression given that: >- this has failed since 4.9.0, and >- the test-case is not supported in 4.8, >so we're not required to fix it in 4.9 and 5 branches. > >But, the test-case does fail in 5 and 4.9 branches, and the fix seems simple >enough to not have the risk of introducing breakage in the release branches. > >OK for 4.9/5 branch? Okay. Hmm, it doesn't apply cleanly. The trunk version of simd_clone_create also contains: 'Fix ICE for SIMD clones usage in LTO' ( https://gcc.gnu.org/ml/gcc-patches/2015-10/msg00434.html ), which adds the 'in_other_partition' propagation in simd_clone_create. That's also not a release regression (reproduces with 4.9.0 release, not supported in 4.8), and still reproduces on 4.9 and 5 branches. OK to backport 'Fix ICE for SIMD clones usage in LTO' to 4.9/5 branch? Thanks, - Tom
Re: [PING][PATCH, PR67709 ] Don't call call_cgraph_insertion_hooks in simd_clone_create
On Tue, Feb 16, 2016 at 12:10:29PM +0100, Tom de Vries wrote: > On 16/02/16 11:04, Jakub Jelinek wrote: > >On Tue, Feb 16, 2016 at 10:56:58AM +0100, Tom de Vries wrote: > >>>AFAIU, it's not a release regression given that: > >>>- this has failed since 4.9.0, and > >>>- the test-case is not supported in 4.8, > >>>so we're not required to fix it in 4.9 and 5 branches. > >>> > >>>But, the test-case does fail in 5 and 4.9 branches, and the fix seems > >>>simple > >>>enough to not have the risk of introducing breakage in the release > >>>branches. > >>> > >>>OK for 4.9/5 branch? > >Okay. > > > > Hmm, it doesn't apply cleanly. > > The trunk version of simd_clone_create also contains: 'Fix ICE for SIMD > clones usage in LTO' ( > https://gcc.gnu.org/ml/gcc-patches/2015-10/msg00434.html ), which adds the > 'in_other_partition' propagation in simd_clone_create. > > That's also not a release regression (reproduces with 4.9.0 release, not > supported in 4.8), and still reproduces on 4.9 and 5 branches. > > OK to backport 'Fix ICE for SIMD clones usage in LTO' to 4.9/5 branch? Ok. Jakub
[PATCH, PR69532]: Simple changes to effective target in two tests from *_ok -> *_hw
Hi, I have a fix for bugzilla defect 69532, which is a simple change to a couple of arm tests to check for effective target arm_v8_neon_hw instead of arm_v8_neon_ok. Tested: arm-none-eabi: No regressions in arm.exp testsuite. Good to go? David Sherwood. ChangeLog: 2016-02-16 David Sherwood gcc/testsuite: PR target/69532 * gcc.target/arm/fmaxmin.c: Change effective target to arm_v8_neon_hw. * gcc.target/arm/vect-fmaxmin.c: Likewise. pr69532.patch Description: Binary data
Re: [PATCH, PR69532]: Simple changes to effective target in two tests from *_ok -> *_hw
On Tue, Feb 16, 2016 at 11:47 AM, David Sherwood wrote: > Hi, > > I have a fix for bugzilla defect 69532, which is a simple change to > a couple of arm tests to check for effective target arm_v8_neon_hw > instead of arm_v8_neon_ok. > > Tested: > arm-none-eabi: No regressions in arm.exp testsuite. > > Good to go? > > David Sherwood. > > ChangeLog: > > 2016-02-16 David Sherwood > > gcc/testsuite: Drop the reference to gcc/testsuite. > PR target/69532 > * gcc.target/arm/fmaxmin.c: Change effective target to > arm_v8_neon_hw. > * gcc.target/arm/vect-fmaxmin.c: Likewise. > Ok with those changes. Ramana
Re: [6 Regession] Usage of unitialized pointer io/list_read.c (
Hi Jerry, > Thanks for review. Committed to trunk r233436. The test gfortran.dg/read_bang4.f90 fails on x86_64-apple-darwin15: a.out(15552,0x7fff7b2e3000) malloc: *** error for object 0x7fb472804c00: pointer being freed was not allocated *** set a breakpoint in malloc_error_break to debug Program received signal SIGABRT: Process abort signal. If I compile the test with -fsanitize=address, I get = ==15561==ERROR: AddressSanitizer: attempting double-free on 0x61a1c280 in thread T0: #0 0x106d1ac40 in wrap_free.part.0 sanitizer_malloc_mac.inc:103 #1 0x106c0078f in _gfortrani_list_formatted_read list_read.c:137 #2 0x440042 () 0x61a1c280 is located 0 bytes inside of 1200-byte region [0x61a1c280,0x61a1c730) freed by thread T0 here: #0 0x106d19e09 in wrap_realloc sanitizer_malloc_mac.inc:108 #1 0x106b38a14 in _gfortrani_xrealloc memory.c:98 previously allocated by thread T0 here: #0 0x106d19fe9 in wrap_calloc sanitizer_malloc_mac.inc:114 #1 0x106b389d2 in _gfortrani_xcalloc memory.c:84 SUMMARY: AddressSanitizer: double-free sanitizer_malloc_mac.inc:103 in wrap_free.part.0 ==15561==ABORTING Program received signal SIGABRT: Process abort signal. TIA Dominique
Re: RFC: [Patch, PR Bug 60818] - ICE in validate_condition_mode on powerpc*-linux-gnu* ]
On Tue, Feb 16, 2016 at 07:00:58PM +1030, Alan Modra wrote:
> What's wrong is the rs6000 backend asserting that (gtu (reg:CC)) can't
> happen, because obviously it does. Rather than trying to fix combine,
> (where the ICE happens on attempting to validate the insn!), I think
> the rs6000 backend should change. Like so. Not yet bootstrapped,
> but I'm about to fire one off.
>
> PR target/60818
> * config/rs6000/rs6000.c (validate_condition_mode): Return a
> bool rather than asserting modes as expected. Update all uses
> to assert.
> * config/rs6000/rs6000-protos.h (validate_condition_mode):
> Update prototype.
> * config/rs6000/predicates.md (branch_comparison_operator):
> Use result of validate_condition_mode.
Now bootstrapped and regression tested powerpc64le-linux and
powerpc64-linux biarch, mainline, gcc-5 and gcc-4.9. With this
testsuite addition. OK to apply?
* gcc.target/powerpc/pr60818.c: New.
diff --git a/gcc/testsuite/gcc.target/powerpc/pr60818.c
b/gcc/testsuite/gcc.target/powerpc/pr60818.c
new file mode 100644
index 000..773480b
--- /dev/null
+++ b/gcc/testsuite/gcc.target/powerpc/pr60818.c
@@ -0,0 +1,62 @@
+/* { dg-do compile { target powerpc*-*-* } } */
+/* { dg-options "-O1 -mno-mfcrf -misel" } */
+
+int d7;
+
+static int
+ca(int l3)
+{
+ for (d7 = 0; d7 < 1; ++d7)
+;
+ return l3;
+}
+
+int
+c9(void)
+{
+ int yj;
+ return ca(((yj != 1) & 65535U) > d7);
+}
+
+
+int
+kf(int a2, unsigned int dc)
+{
+ int t3;
+ int b1[2];
+ for (t3 = 0; t3 < 2; ++t3)
+b1[t3] = 2;
+ return ((t3 > a2) >= b1[0]) < dc;
+}
+
+
+void
+ds(void)
+{
+ unsigned int t5;
+ unsigned int re;
+ int yn;
+ int *o2;
+ int *s0 = &yn;
+ for (re = 0; re < 2; ++re)
+if (0 != t5)
+ *o2 = (*s0 ^= 1) | (re = ((t5 < yn) >= (t5 > yn)));
+}
+
+
+unsigned int ou;
+int jv (void)
+{
+ unsigned int rg;
+ return rg < ou;
+}
+
+
+unsigned int vz, tr, c, fr;
+
+void
+gi(void)
+{
+ if (vz < 1)
+vz = ((fr < tr) >= (fr > tr));
+}
--
Alan Modra
Australia Development Lab, IBM
Re: [PATCH] 69759 - document __builtin_alloca and __builtin_alloca_with_align
On Tue, 16 Feb 2016, Martin Sebor wrote: > That said, I think it's worth pointing out that max_align_t has > nothing to do with standard C types. The intent of the type is > to expose a type with the strictest alignment supported by > an implementation for an object of any type and with any storage > duration, not the alignment of the most strictly aligned basic > (or fundamental) type. The minimum alignment for max_align_t (given the proposed fix for DR#445) is defined in terms of standard types (where if various extensions in TRs and TSes are implemented, the types in those are standard types for this purpose). While an implementation may choose to use a larger alignment than the minimum, it's inefficient to do so, considering that malloc must return suitably aligned memory even when the size allocated is smaller than the alignment (and GCC chooses to use the minimum, modulo taking the alignments that long long and long double have outside structures rather than their possibly smaller alignments inside structures). -- Joseph S. Myers jos...@codesourcery.com
Re: Wonly-top-basic-asm
> @example
> -/* Note that this code will not compile with -masm=intel */
> -#define DebugBreak() asm("int $3")
> +/* Define macro at file scope with basic asm. */
> +/* Add macro parameter p to eax. */
> +asm(".macro test p\n\t"
> +"addl $\\p, %eax\n\t"
> +".endm");
> +
> +/* Use macro in function using extended asm. It needs */
> +/* the "cc" clobber since the flags are changed and uses */
> +/* the "a" constraint since it modifies eax. */
> +int DoAdd(int value)
> +@{
> + asm("test 5" : "+a" (value) : : "cc");
> + return value;
> +@}
To make this work you need the .macro appear before all uses in the asm files.
I do not think
we really promise that wihtout -fno-toplevel-reorder -fno-lto
Honza
RE: [PATCH] [ARC] Add single/double IEEE precission FPU support.
Thanks Joern, Committed: r233451 > -Original Message- > From: Joern Wolfgang Rennecke [mailto:g...@amylaar.uk] > Sent: Saturday, February 13, 2016 12:42 AM > To: Claudiu Zissulescu; gcc-patches@gcc.gnu.org > Cc: francois.bed...@synopsys.com; jeremy.benn...@embecosm.com > Subject: Re: [PATCH] [ARC] Add single/double IEEE precission FPU support. > > > > On 10/02/16 13:31, Claudiu Zissulescu wrote: > > Please find attached the amended patch for FPU instructions. > > > > Ok to apply? > +(define_insn "*cmpdf_fpu" > > I'm wondering - could you compare with +zero using a literal (adding an > alternative)? > (No need to hold up the main patch, but you can consider it for a follow-up > patch) > > (define_insn "*cmpsf_fpu_uneq" > + [(set (reg:CC_FPU_UNEQ CC_REG) > + (compare:CC_FPU_UNEQ > +(match_operand:DF 0 "even_register_operand" "r") > > Typo: probably should be *cmpdf_fpu_uneq > > +case CC_FPUmode: > + return !((code == LTGT) || (code == UNEQ)); > ` > strictly speaking, this shouldn't accept unsigned comparisons, although I > can't > think of a scenario where these would be presented in this mode, and the > failure mode would just be an abort in get_arc_condition_code. > > Otherwise, this is OK.
Re: Fix c/69522, memory management issue in c-parser
On 02/08/2016 05:30 PM, Jeff Law wrote: On 01/29/2016 04:40 AM, Bernd Schmidt wrote: c/ PR c/69522 * c-parser.c (c_parser_braced_init): New arg outer_obstack. All callers changed. If nested_p is true, use it to call finish_implicit_inits. * c-tree.h (finish_implicit_inits): Declare. * c-typeck.c (finish_implicit_inits): New function. Move code from ... (push_init_level): ... here. (set_designator, process_init_element): Call finish_implicit_inits. testsuite/ PR c/69522 gcc.dg/pr69522.c: New test. OK. Thanks for tracking this down. Ok for branches too? Bernd
Re: [PATCH][AArch64] Skip gcc.target/aarch64/assembler_arch_1.c if assembler does not support it
On 16 February 2016 at 10:53, James Greenhalgh wrote:
> On Tue, Feb 16, 2016 at 09:27:00AM +, Kyrill Tkachov wrote:
>> Hi all,
>>
>> As Christophe reported at:
>> https://gcc.gnu.org/ml/gcc-patches/2016-02/msg00784.html
>>
>> The test gcc.target/aarch64/assembler_arch_1.c fails to assemble on older
>> assemblers that don't support the LSE architecture extension.
>
> Actually, Christophe's report is that the ".arch_extension" directive
> isn't understood by older assemblers...
>
>> I'd really like to keep the test an assemble test rather than just a compile
>> one since it is extracted from a real error when building the Linux kernel,
>> so this patch is the simplest way I can think of skipping the test for older
>> assemblers.
>>
>> I've tested that the test appears UNSUPPORTED when used with a binutils that
>> doesn't support LSE and PASSes normally on latest binutils.
>>
>> Do we want to have such a specialised check for assembler capabilities? Or
>> do we want to just let this test FAIL on older assemblers and see this
>> failure go away in the future as users/testers migrate to later binutils
>> releases?
>
> ...so, this test conflates two features we're asking for from the assembler,
> support for LSE and support for .arch_extension. For your test, that's
> exactly what you need, and for binutils it will never matter, but it feels
> like a misnamed check to me.
>
>> If such a check is desired, here's a patch that does it.
>
> Yes, the check is probably useful (if we need to test other LSE features, we
> will want it - e.g. if we wanted to bump some of the armv8.1+lse atomics
> tests up to assemble tests).
>
> Anyway, this patch is OK by me as is, as I don't see a way to disambiguate
> the test name without ending up with two tiny effective-target checks. But
> maybe someone who knows the testsuite better would have other opinions?
>
I agree this is useful, and it would probably be overkill to split this further.
Thanks for taking care of that.
>> 2016-02-16 Kyrylo Tkachov
>>
>> * lib/target-supports.exp
>> (check_effective_target_aarch64_lse_assembler): New effective target
>> check.
>> * gcc.target/aarch64/assembler_arch_1.c: Add arch64_lse_assembler
>> effective target check.
>
>> diff --git a/gcc/testsuite/gcc.target/aarch64/assembler_arch_1.c
>> b/gcc/testsuite/gcc.target/aarch64/assembler_arch_1.c
>> index
>> 901e50a178d7a4a443a5ad0abe63f624688db268..ef34e76cef9c3076a3c32e76aa7a7f658774f786
>> 100644
>> --- a/gcc/testsuite/gcc.target/aarch64/assembler_arch_1.c
>> +++ b/gcc/testsuite/gcc.target/aarch64/assembler_arch_1.c
>> @@ -1,4 +1,5 @@
>> /* { dg-do assemble } */
>> +/* { dg-require-effective-target aarch64_lse_assembler } */
>> /* { dg-options "-march=armv8-a" } */
>>
>> /* Make sure that the function header in assembly doesn't override
>> diff --git a/gcc/testsuite/lib/target-supports.exp
>> b/gcc/testsuite/lib/target-supports.exp
>> index
>> 645981a8733b68a7c37919a53b324841d2c125aa..08b4eacc26f44506d00fda58f99c203a4199856b
>> 100644
>> --- a/gcc/testsuite/lib/target-supports.exp
>> +++ b/gcc/testsuite/lib/target-supports.exp
>> @@ -6718,6 +6718,18 @@ proc check_effective_target_aarch64_tiny { } {
>> }
>> }
>>
>> +# Return 1 if the target AArch64 assembler supports the LSE extensions.
>
> I think the "extensions" here is redundant :-).
>
> Thanks,
> James
>
>
Re: C++ PATCH for c++/69753 (DR141 broke member template lookup)
On Tue, Feb 16, 2016 at 5:38 AM, Markus Trippelsdorf
wrote:
> On 2016.02.15 at 16:13 -0500, Jason Merrill wrote:
>> When we stopped finding function templates with unqualified lookup due to
>> the DR141 fix, that exposed bugs in our lookup within the object expression
>> scope; an object-expression of the current instantiation does not make the
>> expression dependent. This patch fixes this issue specifically for implicit
>> "this->", which is the case in question in this PR; addressing this issue
>> more generally will take more work.
>>
>> Tested x86_64-pc-linux-gnu, applying to trunk.
>
>> diff --git a/gcc/testsuite/g++.dg/lookup/member3.C
>> b/gcc/testsuite/g++.dg/lookup/member3.C
>> new file mode 100644
>> index 000..f4e097e4
>> --- /dev/null
>> +++ b/gcc/testsuite/g++.dg/lookup/member3.C
>> @@ -0,0 +1,17 @@
>> +// PR c++/69753
>> +// { dg-do compile { target c++11 } }
>> +
>> +class A {
>> +public:
>> + template void As();
>> + static A *FromWebContents();
>> + A *FromWebContents2();
>> +};
>> +template class B : A {
>> + void FromWebContents() {
>> +auto guest = A::FromWebContents();
>> +guest ? guest->As() : nullptr;
>> +auto guest2 = A::FromWebContents2();
>> +guest2 ? guest2->As() : nullptr;
>> + }
>> +};
>
> Please note that clang rejects the testcase in the non static case:
>
> gcc/testsuite/g++.dg/lookup/member3.C:15:22: error: use 'template' keyword to
> treat 'As' as a dependent template name
> guest2 ? guest2->As() : nullptr;
> ^
> template
> 1 error generated.
>
> Here is another testcase that every compiler I've tested (clang, icc,
> microsoft) accepts, but is rejected by gcc-6:
>
> class A {
> public:
> template void m_fn1();
> };
> A *fn1(int *);
> template class B : A {
> static int *m_fn2() { fn1(m_fn2())->m_fn1(); }
> };
>
> --
> Markus
clang also rejects the case where A::FromWebContents is overloaded
with a static member function and non-static one, and gcc now accepts
this case.
class A {
public:
template void As();
static A *FromWebContents();
template
A *FromWebContents(Ts...);
};
template class B : A {
void FromWebContents() {
auto guest = A::FromWebContents();
guest->As();
}
};
[wwwdocs] Describe behavior of -flifetime-dse in class constructors
Hello. As I finally hunted issue in Firefox that was responsible for start-up segfault, I would like to describe a new behavior of the compiler that emits clobbers to class constructors (w/ -flifetime-dse). As also Richi spotted quite similar issue in openjade package, I think it worth for mentioning in porting: Ok? Thanks, Martin Index: htdocs/gcc-6/porting_to.html === RCS file: /cvs/gcc/wwwdocs/htdocs/gcc-6/porting_to.html,v retrieving revision 1.14 diff --unified -r1.14 porting_to.html --- htdocs/gcc-6/porting_to.html 14 Feb 2016 13:13:43 - 1.14 +++ htdocs/gcc-6/porting_to.html 16 Feb 2016 14:41:10 - @@ -316,7 +316,7 @@ -Finally, the type and mangling of flexible array members has changed +Furthermore, the type and mangling of flexible array members has changed from previous releases. While in GCC 5 and prior the type of a flexible array member is an array of zero elements (a GCC extension), in GCC 6 it is that of an array of an unspecified bound (i.e., T[] as opposed @@ -324,6 +324,50 @@ -fabi-version or -Wabi option to disable or warn about. + +Finally, the C++ compiler (with enabled -flifetime-dse) +has been more aggressive in dead-store elimination in situations where +a memory store to a location precedes a constructor to the +memory location. Described situation can be commonly found in programs +which zero a memory that is eventually passed to a placement new operator: + + +#include+#include +#include + +struct A +{ + A () {} + void *operator new (size_t s) + { +void *ptr = malloc (s); +memset (ptr, 0, s); +return ptr; + } + + int value; +}; + +A * +__attribute__ ((noinline)) +build (void) +{ + return new A (); +} + +int main() +{ + A *a = build (); + assert (a->value == 0); /* Use of uninitialized value */ + free (a); +} + + +If the program cannot be fixed to remove the undefined behavior then +the option -fno-lifetime-dse can be used to disable +this optimization. + -Wmisleading-indentation A new warning -Wmisleading-indentation was added
[PATCH] Fix PR69776 more
So there's the same issue left in DOM and an "optimization" in the alias machinery fails to honor the overridden alias-sets. Bootstrapped and tested on x86_64-unknown-linux-gnu, applied. Richard. 2016-02-16 Richard Biener PR tree-optimization/69776 * tree-ssa-alias.c (indirect_ref_may_alias_decl_p): Get alias sets from caller. (indirect_refs_may_alias_p): Likewise. (refs_may_alias_p_1): Pass alias sets as from ao_ref. * tree-ssa-sccvn.c (vn_reference_lookup): Also adjust vr alias-set according to tbaa_p. * tree-ssa-dom.c (lookup_avail_expr): Add tbaa_p flag. (optimize_stmt): For redundant store discovery do not allow tbaa. * gcc.dg/torture/pr69776-2.c: New testcase. Index: gcc/tree-ssa-alias.c === *** gcc/tree-ssa-alias.c(revision 233418) --- gcc/tree-ssa-alias.c(working copy) *** indirect_ref_may_alias_decl_p (tree ref1 *** 1067,1078 ptrtype1 = TREE_TYPE (TREE_OPERAND (base1, 1)); /* If the alias set for a pointer access is zero all bets are off. */ - if (base1_alias_set == -1) - base1_alias_set = get_deref_alias_set (ptrtype1); if (base1_alias_set == 0) return true; - if (base2_alias_set == -1) - base2_alias_set = get_alias_set (base2); /* When we are trying to disambiguate an access with a pointer dereference as base versus one with a decl as base we can use both the size --- 1067,1074 *** indirect_refs_may_alias_p (tree ref1 ATT *** 1239,1251 ptrtype2 = TREE_TYPE (TREE_OPERAND (base2, 1)); /* If the alias set for a pointer access is zero all bets are off. */ ! if (base1_alias_set == -1) ! base1_alias_set = get_deref_alias_set (ptrtype1); ! if (base1_alias_set == 0) ! return true; ! if (base2_alias_set == -1) ! base2_alias_set = get_deref_alias_set (ptrtype2); ! if (base2_alias_set == 0) return true; /* If both references are through the same type, they do not alias --- 1235,1242 ptrtype2 = TREE_TYPE (TREE_OPERAND (base2, 1)); /* If the alias set for a pointer access is zero all bets are off. */ ! if (base1_alias_set == 0 ! || base2_alias_set == 0) return true; /* If both references are through the same type, they do not alias *** refs_may_alias_p_1 (ao_ref *ref1, ao_ref *** 1417,1423 if (var1_p && ind2_p) return indirect_ref_may_alias_decl_p (ref2->ref, base2, offset2, max_size2, ! ao_ref_alias_set (ref2), -1, ref1->ref, base1, offset1, max_size1, ao_ref_alias_set (ref1), --- 1408,1415 if (var1_p && ind2_p) return indirect_ref_may_alias_decl_p (ref2->ref, base2, offset2, max_size2, ! ao_ref_alias_set (ref2), ! ao_ref_base_alias_set (ref2), ref1->ref, base1, offset1, max_size1, ao_ref_alias_set (ref1), *** refs_may_alias_p_1 (ao_ref *ref1, ao_ref *** 1426,1435 else if (ind1_p && ind2_p) return indirect_refs_may_alias_p (ref1->ref, base1, offset1, max_size1, ! ao_ref_alias_set (ref1), -1, ref2->ref, base2, offset2, max_size2, ! ao_ref_alias_set (ref2), -1, tbaa_p); gcc_unreachable (); --- 1418,1429 else if (ind1_p && ind2_p) return indirect_refs_may_alias_p (ref1->ref, base1, offset1, max_size1, ! ao_ref_alias_set (ref1), ! ao_ref_base_alias_set (ref1), ref2->ref, base2, offset2, max_size2, ! ao_ref_alias_set (ref2), ! ao_ref_base_alias_set (ref2), tbaa_p); gcc_unreachable (); Index: gcc/tree-ssa-sccvn.c === *** gcc/tree-ssa-sccvn.c(revision 233418) --- gcc/tree-ssa-sccvn.c(working copy) *** vn_reference_lookup (tree op, tree vuse, *** 2249,2255 vr1.operands = operands = valueize_shared_reference_ops_from_ref (op, &valuezied_anything); vr1.type = TREE_TYPE (op); ! vr1.set = get_alias_set (op); vr1.hashc
Re: [wwwdocs] Describe behavior of -flifetime-dse in class constructors
On 16/02/16 14:55, Martin Liška wrote: Hello. As I finally hunted issue in Firefox that was responsible for start-up segfault, I would like to describe a new behavior of the compiler that emits clobbers to class constructors (w/ -flifetime-dse). As also Richi spotted quite similar issue in openjade package, I think it worth for mentioning in porting: Ok? Thanks, Martin -Finally, the type and mangling of flexible array members has changed +Furthermore, the type and mangling of flexible array members has changed from previous releases. While in GCC 5 and prior the type of a flexible array member is an array of zero elements (a GCC extension), in GCC 6 it is that of an array of an unspecified bound (i.e., T[] as opposed @@ -324,6 +324,50 @@ -fabi-version or -Wabi option to disable or warn about. + +Finally, the C++ compiler (with enabled -flifetime-dse) +has been more aggressive in dead-store elimination in situations where +a memory store to a location precedes a constructor to the +memory location. Described situation can be commonly found in programs +which zero a memory that is eventually passed to a placement new operator: Minor nit, but I notice we have a different "Finally" point now ;). Perhaps we shouldn't bother saying "finally" as we keep finding new entries for this list. Thanks, Kyrill
Re: [RFC] [PATCH] Add __array_size keyword
On Mon, Feb 15, 2016 at 11:11:54PM +, Joseph Myers wrote:
> On Sat, 13 Feb 2016, Stuart Brady wrote:
>
> > So in other words, adapting all of the sizeof tests would be appropriate,
> > and sizeof tests for non-array types would change from expected passes to
> > expected failures?
>
> It's not very clear what's a sizeof test, but all that are testing sizeof
> (as opposed to incidentally using it) and applicable to this keyword, yes.
Okay. I will go through all of the gcc.dg tests that mention sizeof and
provide an opinion as to whether I think a corresponding __array_size
test would be needed, then.
> > > Is the rule for your keyword that the operand is evaluated, and the
> > > result not an integer constant, iff the operand is an array with a
> > > variable number of elements (as opposed to an array with a constant
> > > number of elements that themselves are variable-sized, for example)?
> >
> > If I've understood correctly, then yes:
> >
> >#include
> >void foo(int i) {
> > int a[i], b[__array_size(a)];
> > printf("%zi, %zi\n", __array_size(a), __array_size(b));
> >};
> >int main() { foo(42); }
>
> That test doesn't relate to my question, which is about when arguments are
> evaluated and when results are or are not integer constant expressions.
Ah, I think I understand, now.
> For whether arguments are evaluated, you need __array_size with arguments
> that have side effects, and then test whether those side effects occurred.
> For whether results are integer constant expressions, you can test e.g.
> whether __array_size (a) - __array_size (a) is accepted in a context
> requiring a pointer (whether it acts as a valid null pointer constant).
With my patch, void *p = __array_size(a) - __array_size(a) sets p to be
null. That behaviour seems desirable to me.
I will look into this in some more depth and get back to you, but my
thinking is that the requirement should be for any implementation defined
behaviour relating to sizeof to have equivalent behaviour for
__array_size(). This means that in situations where sizeof(expr) /
sizeof(*(expr)) would have side-effects, __array_size(expr) would cause
those side-effects to occur only once.
My view is that __array_size should result in an integer constant
expression iff sizeof would do so, and that __array_size should evaluate
its operand with side-effects iff sizeof would do so. This is purely in
accordance with the principle of least surprise. I would welcome any
discussion as to whether some alternative behaviour might be preferred.
> > > C11 6.5.3.4#2 (sizeof) would need testing,
> >
> > Does this section differ from the September 7th draft in any way?
>
> I don't know.
I now see that this draft really was rather old. I can now refer to
N1570 (April 12, 2011) and will use that, instead. Apologies for the
noise, there -- I will redo my list of amendments to the draft using
N1570 instead.
> > > Presumably this keyword can be applied to an array at function prototype
> > > scope whose size is explicitly or implicitly [*], though nothing useful
> > > can be done with the results, as with [*]? (Cf. gcc.dg/vla-5.c.)
> >
> > I'm not sure I quite understand the meaning of an implicit [*]. Does that
> > just mean __array_size(foo) with an int foo[*] as another parameter?
>
> Implicit [*] is e.g.
>
> void f (int a, int (*b)[a], int (*c)[__array_size (*b)]);
>
> where the VLA *b is at function prototype scope and so gets treated as [*]
> - and then __array_size (*) effectively means "an indeterminate value of
> type size_t" (but since that value only ever gets used in ways that end up
> with it being discarded, possibly through another implicit conversion to
> [*] as here, manipulating such indeterminate values is never a problem).
Yes, I can see no reason for this to differ in this regard to sizeof,
and I will make sure that I include this in my tests.
Unfortunately, I do find that TYPE_MAX_VALUE (TYPE_DOMAIN (type)) is
unset in that scope so your example results in a compilation failure.
I imagine this is possibly as it would get treated as [*] as you say.
TYPE_SIZE_UNIT is set, which is why this works when using sizeof.
However, I will follow your advice, and try not to focus too much on the
implementation just yet, but will instead assume we can deal with this at
a later stage.
--
Thanks,
Stuart Brady
[PATCH] Fix driver handling of multiple -ftree-parallelize-loops= options (PR driver/69805)
Hi!
As mentioned in the PR, %{ftree-parallelize-loops=*} expands to
all -ftree-parallelize-loops= options, not just the last one.
So greater_than_spec_func is actually called say for
-ftree-parallelize-loops=0 -ftree-parallelize-loops=2 with
- ftree-parallelize-loops=0 - ftree-parallelize-loops=2 1
(each whitespace separated sequence separate arg), but it asserts
it sees just 3 arguments.
Passing the - and ftree-parallelize-loops= stuff looks weird,
and we have %* that substitutes just the variable part of the option,
so in addition to fixing the case of multiple options I've also changed
%:gt() behaviour, so that it now gets just the numbers and compares the
last two of them. So for the above options it would be called with
0 2 1
and would compare
2 > 1
and return "", or for
-ftree-parallelize-loops=2 -ftree-parallelize-loops=0 -ftree-parallelize-loops=1
would be
2 0 1 1
and compare
1 > 1
and return NULL. %:gt() is not used anywhere else, and has been introduced
only in GCC 6.
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
2016-02-16 Jakub Jelinek
PR driver/69805
* gcc.c (LINK_COMMAND_SPEC, GOMP_SELF_SPECS): Use
:%* in %:gt() argument.
(greater_than_spec_func): Adjust for expecting only numbers,
if there are more than two numbers, compare the last two.
* testsuite/libgomp.c/pr69805.c: New test.
--- gcc/gcc.c.jj2016-02-15 22:22:51.0 +0100
+++ gcc/gcc.c 2016-02-16 09:35:03.579849080 +0100
@@ -1019,7 +1019,7 @@ proper position among the other output f
%{s} %{t} %{u*} %{z} %{Z} %{!nostdlib:%{!nostartfiles:%S}} \
%{static:} %{L*} %(mfwrap) %(link_libgcc) " \
VTABLE_VERIFICATION_SPEC " " SANITIZER_EARLY_SPEC " %o " CHKP_SPEC " \
-%{fopenacc|fopenmp|%:gt(%{ftree-parallelize-loops=*} 1):\
+%{fopenacc|fopenmp|%:gt(%{ftree-parallelize-loops=*:%*} 1):\
%:include(libgomp.spec)%(link_gomp)}\
%{fcilkplus:%:include(libcilkrts.spec)%(link_cilkrts)}\
%{fgnu-tm:%:include(libitm.spec)%(link_itm)}\
@@ -1183,7 +1183,7 @@ static const char *const multilib_defaul
for targets that use different start files and suchlike. */
#ifndef GOMP_SELF_SPECS
#define GOMP_SELF_SPECS \
- "%{fopenacc|fopenmp|%:gt(%{ftree-parallelize-loops=*} 1): " \
+ "%{fopenacc|fopenmp|%:gt(%{ftree-parallelize-loops=*:%*} 1): " \
"-pthread}"
#endif
@@ -9764,7 +9764,7 @@ replace_extension_spec_func (int argc, c
return result;
}
-/* Returns "" if the n in ARGV[1] == -opt= is greater than ARGV[2].
+/* Returns "" if ARGV[ARGC - 2] is greater than ARGV[ARGC-1].
Otherwise, return NULL. */
static const char *
@@ -9775,29 +9775,13 @@ greater_than_spec_func (int argc, const
if (argc == 1)
return NULL;
- gcc_assert (argc == 3);
- gcc_assert (argv[0][0] == '-');
- gcc_assert (argv[0][1] == '\0');
-
- /* Point p to in in -opt=. */
- const char *p = argv[1];
- while (true)
-{
- char c = *p;
- if (c == '\0')
- gcc_unreachable ();
+ gcc_assert (argc >= 2);
- ++p;
-
- if (c == '=')
- break;
-}
-
- long arg = strtol (p, &converted, 10);
- gcc_assert (converted != p);
+ long arg = strtol (argv[argc - 2], &converted, 10);
+ gcc_assert (converted != argv[argc - 2]);
- long lim = strtol (argv[2], &converted, 10);
- gcc_assert (converted != argv[2]);
+ long lim = strtol (argv[argc - 1], &converted, 10);
+ gcc_assert (converted != argv[argc - 1]);
if (arg > lim)
return "";
--- libgomp/testsuite/libgomp.c/pr69805.c.jj2016-02-16 09:54:46.928527601
+0100
+++ libgomp/testsuite/libgomp.c/pr69805.c 2016-02-16 09:55:29.453941023
+0100
@@ -0,0 +1,9 @@
+/* PR driver/69805 */
+/* { dg-do link } */
+/* { dg-options "-ftree-parallelize-loops=1 -O2 -ftree-parallelize-loops=2" }
*/
+
+int
+main ()
+{
+ return 0;
+}
Jakub
Re: [RFC] [PATCH] Add __array_size keyword
On Mon, Feb 15, 2016 at 03:51:43PM +, Stuart Brady wrote: > On Mon, Feb 15, 2016 at 03:05:36PM +0100, Marek Polacek wrote: > > On Sat, Feb 13, 2016 at 03:16:49AM +, Stuart Brady wrote: > > > For a hypothetical change to the C standard itself, I think one might use > > > the name "_ArraySize", but for a non-standard extension this would not be > > > appropriate. I think "__array_size" is fine, though, and is consistent > > > with "__alignof". > > > > Please no CamelCase, that's ugly ;). I also think that "__array_size" is > > fine. [...] > Hypothetically, I suppose _Array_Size, _Array_size or _Arraysize would > all be better than the CamelCase verison. On further consideration, referring to N1570, I see now that _Array_size would be consistent in style with _Static_assert and _Thread_local, if such a keyword were ever to be added as part of the standard. -- Cheers, Stuart Brady
[PATCH] Move the -Wnonnull compare against NULL warning from FEs to early uninit under -Wnonnull-compare (PR c/69835)
Hi!
As has been reported today on gcc@, the new -Wnonnull warning addition
warns even about nonnull parameters that were changed (or could be changed)
in the function. IMHO the nonnull attribute only talks about the value of
the parameter upon entry to the function, if you assign something else
to the argument afterwards and test that for NULL or non-NULL, it is like
any other variable.
But, we don't have the infrastructure to detect this in the FE, so this
patch moves the warning soon after we go into SSA form.
As -Wnonnull is a C/C++/ObjC/ObjC++ only warning, I've added a new
-Wnonnull-compare switch for it too (and enable it for C/C++/ObjC/ObjC++
from -Wall).
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
2016-02-16 Jakub Jelinek
PR c/69835
* common.opt (Wnonnull-compare): New warning.
* doc/invoke.texi (-Wnonnull): Remove text about comparison
of arguments against NULL.
(-Wnonnull-compare): Document.
* tree-ssa-uninit.c (warn_uninitialized_vars): Add warn_nonnull_cmp
argument, if true, warn about SSA_NAME (D) of nonnull_arg_p
comparisons against NULL pointers.
(pass_late_warn_uninitialized::execute): Adjust caller.
(execute_early_warn_uninitialized): Likewise.
(gate_early_warn_uninitialized): New function.
(pass_early_warn_uninitialized::gate): Call it instead of
gate_warn_uninitialized.
c-family/
* c.opt (Wnonnull-compare): Enable for -Wall.
c/
* c-typeck.c (build_binary_op): Revert 2015-09-09 change.
cp/
* typeck.c (cp_build_binary_op): Revert 2015-09-09 change.
testsuite/
* c-c++-common/nonnull-1.c: Use -Wnonnull-compare instead of
-Wnonnull in dg-options.
* c-c++-common/nonnull-2.c: New test.
--- gcc/common.opt.jj 2016-01-27 19:47:35.0 +0100
+++ gcc/common.opt 2016-02-16 11:52:42.641623690 +0100
@@ -616,6 +616,10 @@ Wlarger-than=
Common RejectNegative Joined UInteger Warning
-Wlarger-than= Warn if an object is larger than bytes.
+Wnonnull-compare
+Var(warn_nonnull_compare) Warning
+Warn if comparing pointer parameter with nonnull attribute with NULL.
+
Wnull-dereference
Common Var(warn_null_dereference) Warning
Warn if dereferencing a NULL pointer may lead to erroneous or undefined
behavior.
--- gcc/doc/invoke.texi.jj 2016-02-08 18:39:16.0 +0100
+++ gcc/doc/invoke.texi 2016-02-16 12:31:42.037232875 +0100
@@ -276,7 +276,8 @@ Objective-C and Objective-C++ Dialects}.
-Wmain -Wmaybe-uninitialized -Wmemset-transposed-args @gol
-Wmisleading-indentation -Wmissing-braces @gol
-Wmissing-field-initializers -Wmissing-include-dirs @gol
--Wno-multichar -Wnonnull -Wnormalized=@r{[}none@r{|}id@r{|}nfc@r{|}nfkc@r{]}
@gol
+-Wno-multichar -Wnonnull -Wnonnull-compare @gol
+-Wnormalized=@r{[}none@r{|}id@r{|}nfc@r{|}nfkc@r{]} @gol
-Wnull-dereference -Wodr -Wno-overflow -Wopenmp-simd @gol
-Woverride-init-side-effects -Woverlength-strings @gol
-Wpacked -Wpacked-bitfield-compat -Wpadded @gol
@@ -3537,6 +3538,7 @@ Options} and @ref{Objective-C and Object
-Wmissing-braces @r{(only for C/ObjC)} @gol
-Wnarrowing @r{(only for C++)} @gol
-Wnonnull @gol
+-Wnonnull-compare @gol
-Wopenmp-simd @gol
-Wparentheses @gol
-Wpointer-sign @gol
@@ -3795,12 +3797,18 @@ formats that may yield only a two-digit
Warn about passing a null pointer for arguments marked as
requiring a non-null value by the @code{nonnull} function attribute.
-Also warns when comparing an argument marked with the @code{nonnull}
-function attribute against null inside the function.
-
@option{-Wnonnull} is included in @option{-Wall} and @option{-Wformat}. It
can be disabled with the @option{-Wno-nonnull} option.
+@item -Wnonnull-compare
+@opindex Wnonnull-compare
+@opindex Wno-nonnull-compare
+Warn when comparing an argument marked with the @code{nonnull}
+function attribute against null inside the function.
+
+@option{-Wnonnull-compare} is included in @option{-Wall}. It
+can be disabled with the @option{-Wno-nonnull-compare} option.
+
@item -Wnull-dereference
@opindex Wnull-dereference
@opindex Wno-null-dereference
--- gcc/tree-ssa-uninit.c.jj2016-01-13 13:28:41.0 +0100
+++ gcc/tree-ssa-uninit.c 2016-02-16 12:50:30.390619823 +0100
@@ -171,7 +171,8 @@ warn_uninit (enum opt_code wc, tree t, t
}
static unsigned int
-warn_uninitialized_vars (bool warn_possibly_uninitialized)
+warn_uninitialized_vars (bool warn_possibly_uninitialized,
+bool warn_nonnull_cmp)
{
gimple_stmt_iterator gsi;
basic_block bb;
@@ -190,6 +191,60 @@ warn_uninitialized_vars (bool warn_possi
if (is_gimple_debug (stmt))
continue;
+ if (warn_nonnull_cmp)
+ {
+ tree op0 = NULL_TREE, op1 = NULL_TREE;
+ location_t loc = gimple_location (stmt);
+ if (gimple_code (stmt) == GIMPLE_COND)
+ switch (gimple_cond_code (
Re: [PATCH] Move the -Wnonnull compare against NULL warning from FEs to early uninit under -Wnonnull-compare (PR c/69835)
On Tue, 16 Feb 2016, Jakub Jelinek wrote:
> Hi!
>
> As has been reported today on gcc@, the new -Wnonnull warning addition
> warns even about nonnull parameters that were changed (or could be changed)
> in the function. IMHO the nonnull attribute only talks about the value of
> the parameter upon entry to the function, if you assign something else
> to the argument afterwards and test that for NULL or non-NULL, it is like
> any other variable.
> But, we don't have the infrastructure to detect this in the FE, so this
> patch moves the warning soon after we go into SSA form.
> As -Wnonnull is a C/C++/ObjC/ObjC++ only warning, I've added a new
> -Wnonnull-compare switch for it too (and enable it for C/C++/ObjC/ObjC++
> from -Wall).
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
Not sure if it matters but wouldn't walking over function parameters
and their default def SSA names immediate uses be cheaper than
matching all conditions? Esp. as nonnull_arg_p walks over all
DECL_ARGUMENTS (up to the searched index) over and over again?
[we should simply set a flag on the PARM_DECL!]
Richard.
> 2016-02-16 Jakub Jelinek
>
> PR c/69835
> * common.opt (Wnonnull-compare): New warning.
> * doc/invoke.texi (-Wnonnull): Remove text about comparison
> of arguments against NULL.
> (-Wnonnull-compare): Document.
> * tree-ssa-uninit.c (warn_uninitialized_vars): Add warn_nonnull_cmp
> argument, if true, warn about SSA_NAME (D) of nonnull_arg_p
> comparisons against NULL pointers.
> (pass_late_warn_uninitialized::execute): Adjust caller.
> (execute_early_warn_uninitialized): Likewise.
> (gate_early_warn_uninitialized): New function.
> (pass_early_warn_uninitialized::gate): Call it instead of
> gate_warn_uninitialized.
> c-family/
> * c.opt (Wnonnull-compare): Enable for -Wall.
> c/
> * c-typeck.c (build_binary_op): Revert 2015-09-09 change.
> cp/
> * typeck.c (cp_build_binary_op): Revert 2015-09-09 change.
> testsuite/
> * c-c++-common/nonnull-1.c: Use -Wnonnull-compare instead of
> -Wnonnull in dg-options.
> * c-c++-common/nonnull-2.c: New test.
>
> --- gcc/common.opt.jj 2016-01-27 19:47:35.0 +0100
> +++ gcc/common.opt2016-02-16 11:52:42.641623690 +0100
> @@ -616,6 +616,10 @@ Wlarger-than=
> Common RejectNegative Joined UInteger Warning
> -Wlarger-than= Warn if an object is larger than bytes.
>
> +Wnonnull-compare
> +Var(warn_nonnull_compare) Warning
> +Warn if comparing pointer parameter with nonnull attribute with NULL.
> +
> Wnull-dereference
> Common Var(warn_null_dereference) Warning
> Warn if dereferencing a NULL pointer may lead to erroneous or undefined
> behavior.
> --- gcc/doc/invoke.texi.jj2016-02-08 18:39:16.0 +0100
> +++ gcc/doc/invoke.texi 2016-02-16 12:31:42.037232875 +0100
> @@ -276,7 +276,8 @@ Objective-C and Objective-C++ Dialects}.
> -Wmain -Wmaybe-uninitialized -Wmemset-transposed-args @gol
> -Wmisleading-indentation -Wmissing-braces @gol
> -Wmissing-field-initializers -Wmissing-include-dirs @gol
> --Wno-multichar -Wnonnull
> -Wnormalized=@r{[}none@r{|}id@r{|}nfc@r{|}nfkc@r{]} @gol
> +-Wno-multichar -Wnonnull -Wnonnull-compare @gol
> +-Wnormalized=@r{[}none@r{|}id@r{|}nfc@r{|}nfkc@r{]} @gol
> -Wnull-dereference -Wodr -Wno-overflow -Wopenmp-simd @gol
> -Woverride-init-side-effects -Woverlength-strings @gol
> -Wpacked -Wpacked-bitfield-compat -Wpadded @gol
> @@ -3537,6 +3538,7 @@ Options} and @ref{Objective-C and Object
> -Wmissing-braces @r{(only for C/ObjC)} @gol
> -Wnarrowing @r{(only for C++)} @gol
> -Wnonnull @gol
> +-Wnonnull-compare @gol
> -Wopenmp-simd @gol
> -Wparentheses @gol
> -Wpointer-sign @gol
> @@ -3795,12 +3797,18 @@ formats that may yield only a two-digit
> Warn about passing a null pointer for arguments marked as
> requiring a non-null value by the @code{nonnull} function attribute.
>
> -Also warns when comparing an argument marked with the @code{nonnull}
> -function attribute against null inside the function.
> -
> @option{-Wnonnull} is included in @option{-Wall} and @option{-Wformat}. It
> can be disabled with the @option{-Wno-nonnull} option.
>
> +@item -Wnonnull-compare
> +@opindex Wnonnull-compare
> +@opindex Wno-nonnull-compare
> +Warn when comparing an argument marked with the @code{nonnull}
> +function attribute against null inside the function.
> +
> +@option{-Wnonnull-compare} is included in @option{-Wall}. It
> +can be disabled with the @option{-Wno-nonnull-compare} option.
> +
> @item -Wnull-dereference
> @opindex Wnull-dereference
> @opindex Wno-null-dereference
> --- gcc/tree-ssa-uninit.c.jj 2016-01-13 13:28:41.0 +0100
> +++ gcc/tree-ssa-uninit.c 2016-02-16 12:50:30.390619823 +0100
> @@ -171,7 +171,8 @@ warn_uninit (enum opt_code wc, tree t, t
> }
>
> static unsigned int
> -warn_uninitialized_vars (bool warn_possibly_uninitialized
Re: [PATCH] Fix PR69801
On Tue, Feb 16, 2016 at 09:35:25AM +0100, Richard Biener wrote:
>
> Bootstrapped and tested on x86_64-unknown-linux-gnu, applied.
>
> Richard.
>
> 2016-02-16 Richard Biener
>
> PR middle-end/69801
> * fold-const.c (operand_equal_p): For COND_EXPR zero operand
> mask OEP_ADDRESS_OF.
>
> * gcc.dg/pr69801.c: New testcase.
The testcase fails, because default options include -ansi -pedantic-errors
and the testcase isn't valid C89.
Fixed thusly, regtested on x86_64-linux, committed to trunk as obvious.
2016-02-16 Jakub Jelinek
PR middle-end/69801
* gcc.dg/pr69801.c: Add empty dg-options.
--- gcc/testsuite/gcc.dg/pr69801.c.jj 2016-02-16 10:11:14.0 +0100
+++ gcc/testsuite/gcc.dg/pr69801.c 2016-02-16 16:32:30.955386927 +0100
@@ -1,4 +1,6 @@
+/* PR middle-end/69801 */
/* { dg-do compile } */
+/* { dg-options "" } */
struct {
char c[1];
Jakub
Re: Fix c/69522, memory management issue in c-parser
On 02/16/2016 07:21 AM, Bernd Schmidt wrote: On 02/08/2016 05:30 PM, Jeff Law wrote: On 01/29/2016 04:40 AM, Bernd Schmidt wrote: c/ PR c/69522 * c-parser.c (c_parser_braced_init): New arg outer_obstack. All callers changed. If nested_p is true, use it to call finish_implicit_inits. * c-tree.h (finish_implicit_inits): Declare. * c-typeck.c (finish_implicit_inits): New function. Move code from ... (push_init_level): ... here. (set_designator, process_init_element): Call finish_implicit_inits. testsuite/ PR c/69522 gcc.dg/pr69522.c: New test. OK. Thanks for tracking this down. Ok for branches too? Yes. Definitely given it's a memory management issue. jeff
Re: [PATCH] 69759 - document __builtin_alloca and __builtin_alloca_with_align
On 02/16/2016 06:44 AM, Joseph Myers wrote: On Tue, 16 Feb 2016, Martin Sebor wrote: That said, I think it's worth pointing out that max_align_t has nothing to do with standard C types. The intent of the type is to expose a type with the strictest alignment supported by an implementation for an object of any type and with any storage duration, not the alignment of the most strictly aligned basic (or fundamental) type. The minimum alignment for max_align_t (given the proposed fix for DR#445) is defined in terms of standard types (where if various extensions in TRs and TSes are implemented, the types in those are standard types for this purpose). While an implementation may choose to use a larger alignment than the minimum, it's inefficient to do so, considering that malloc must return suitably aligned memory even when the size allocated is smaller than the alignment (and GCC chooses to use the minimum, modulo taking the alignments that long long and long double have outside structures rather than their possibly smaller alignments inside structures). Max_align_t need not impact efficiency. In C, the malloc alignment also need not have an impact on the type since aligned_alloc returns storage that is (or can be) more strictly aligned. The original intent of max_align_t was to make it possible to query the strictest alignment that an object can be defined with, regardless of where it was defined (with what storage duration). In C++, this alignment is constrained by dynamic allocation because there's no way (yet) to request overaligned heap memory (C++ doesn't specify aligned_alloc()). Since C does provide aligned_alloc() to allocate overaligned memory, max_align_t is more likely to be constrained by the other storage durations. So for example on a C implementation like Linux with GLIBC, while conforming, to define max_align_t to _Alignof(long double) is redunant and less than useful. Programs can trivially query the alignment of basic types. What they cannot do (but may need to do) is query the greatest supported alignment in all contexts/storage durations. Martin
Re: [RFC] [P2] [PR tree-optimization/33562] Lowering more complex assignments.
On 02/14/2016 11:38 AM, Richard Biener wrote: On February 14, 2016 5:35:13 PM GMT+01:00, Jeff Law wrote: On 02/12/2016 10:21 AM, Jeff Law wrote: But really we simply need a better DSE algorithm. So the way to fix DSE is to keep the existing algorithm and track the hunks of Complex/aggregates that have been set a second time. My first thought was to implement this as an inverted bitmap. ie, set it to 1 for every byte in the complex/aggregate that is set by the first store. Clear bits for each byte in subsequent stores to the pieces. If the bitmap reaches an empty state, then the initial store is dead. Adjusting *ref could work too, but would probably be painful if the subsequent stores don't happen in a convenient order. So that was scary easy. We should have done this a long time ago. Essentially I call ao_get_ref_base to get the offset/size/max_size filled in for the first statement. Those are used to initialize the live bytes bitfield, as long as max_size != -1. Then when we have a possible killing statement, we use the ao in a similar manner to determine which bytes to clear (taking care that the base is the same between the two references and that in the killing statement that the size/max_size are the same.). When all the live bytes are zero then we've killed the original statement. It's ~20 lines of code. I need to pull together some additional tests, but it looks likely we'll be able to wrap this up easily for gcc-6. BTW, we had sth like this before but it had both correctness and more importantly scalability issues. Really? Do you have a pointer to any kind of discussion? I certainly don't want to re-introduce something that was ultimately abandoned due to unfixable problems. I don't offhand see any scalability issues -- unless ao_ref_init/ao_ref_base don't scale well. This change doesn't cause any additional looping in dse_possible_dead_store_p (it can only cause us to exit the loop earlier) and the bitmap stuff is cheap because we're going to hit the cache as we clear consecutive ranges of bits. jeff
Re: [PATCH] 69759 - document __builtin_alloca and __builtin_alloca_with_align
On Tue, 16 Feb 2016, Martin Sebor wrote: > Max_align_t need not impact efficiency. In C, the malloc alignment > also need not have an impact on the type since aligned_alloc returns > storage that is (or can be) more strictly aligned. As per the DR#445 resolution, malloc must return memory "whose alignment is the greatest fundamental alignment". It's obviously necessary for malloc to return memory sufficiently aligned for any object that can be declared within the standard without using over-aligned types. As per DR#075, this applies even to malloc (1). If max_align_t is 64-byte-aligned, malloc (1) must return 64-byte-aligned memory. As a practical matter of ABI compatibility, standard typedefs such as max_align_t should not depend on options for e.g. vector ISA extensions that might affect what alignments are useful (and if a change is needed, it should be a one-off change, only increasing the alignment and not depending on such ISA options). They should also avoid depending on details of implementation limits (e.g. if GCC supports 2^63 byte alignment for ELF in future, that shouldn't need to change max_align_t). > the other storage durations. So for example on a C implementation > like Linux with GLIBC, while conforming, to define max_align_t to > _Alignof(long double) is redunant and less than useful. Programs > can trivially query the alignment of basic types. What they cannot > do (but may need to do) is query the greatest supported alignment in > all contexts/storage durations. If you disregard malloc, that is effectively unbounded (2^28 bytes at least), with current GCC on ELF targets. This is not a useful thing to query, and not useful for malloc to be required to return memory aligned to. -- Joseph S. Myers jos...@codesourcery.com
[gomp4] Fix PR64748
Hi,
The attached patch is a backport of the fix for PR64748.
Thanks,
Jim
ChangeLog entries
Backport from trunk:
PR c/64748
gcc/cp/
* parser.c (cp_parser_oacc_data_clause_deviceptr): Remove checking.
* semantics.c (finish_omp_clauses): Add deviceptr checking.
gcc/testsuite/
* c-c++-common/goacc/deviceptr-1.c: Add tests.
* g++.dg/goacc/deviceptr-1.c: New file.
Index: gcc/cp/ChangeLog.gomp
===
--- gcc/cp/ChangeLog.gomp (revision 233463)
+++ gcc/cp/ChangeLog.gomp (working copy)
@@ -1,3 +1,10 @@
+2016-02-15 James Norris
+
+ Backport from trunk:
+ PR c/64748
+ * parser.c (cp_parser_oacc_data_clause_deviceptr): Remove checking.
+ * semantics.c (finish_omp_clauses): Add deviceptr checking.
+
2016-02-03 James Norris
* semantics.c (finish_id_expression): Remove usage check.
Index: gcc/cp/parser.c
===
--- gcc/cp/parser.c (revision 233463)
+++ gcc/cp/parser.c (working copy)
@@ -30093,20 +30093,6 @@
for (t = vars; t; t = TREE_CHAIN (t))
{
tree v = TREE_PURPOSE (t);
-
- /* FIXME diagnostics: Ideally we should keep individual
- locations for all the variables in the var list to make the
- following errors more precise. Perhaps
- c_parser_omp_var_list_parens should construct a list of
- locations to go along with the var list. */
-
- if (!VAR_P (v) && TREE_CODE (v) != PARM_DECL)
- error_at (loc, "%qD is not a variable", v);
- else if (TREE_TYPE (v) == error_mark_node)
- ;
- else if (!POINTER_TYPE_P (TREE_TYPE (v)))
- error_at (loc, "%qD is not a pointer variable", v);
-
tree u = build_omp_clause (loc, OMP_CLAUSE_MAP);
OMP_CLAUSE_SET_MAP_KIND (u, GOMP_MAP_FORCE_DEVICEPTR);
OMP_CLAUSE_DECL (u) = v;
Index: gcc/cp/semantics.c
===
--- gcc/cp/semantics.c (revision 233463)
+++ gcc/cp/semantics.c (working copy)
@@ -6729,6 +6729,14 @@
remove = true;
}
else if (OMP_CLAUSE_CODE (c) == OMP_CLAUSE_MAP
+ && OMP_CLAUSE_MAP_KIND (c) == GOMP_MAP_FORCE_DEVICEPTR
+ && !type_dependent_expression_p (t)
+ && !POINTER_TYPE_P (TREE_TYPE (t)))
+ {
+ error ("%qD is not a pointer variable", t);
+ remove = true;
+ }
+ else if (OMP_CLAUSE_CODE (c) == OMP_CLAUSE_MAP
&& OMP_CLAUSE_MAP_KIND (c) == GOMP_MAP_FIRSTPRIVATE_POINTER)
{
if (bitmap_bit_p (&generic_head, DECL_UID (t))
Index: gcc/testsuite/ChangeLog.gomp
===
--- gcc/testsuite/ChangeLog.gomp (revision 233463)
+++ gcc/testsuite/ChangeLog.gomp (working copy)
@@ -1,3 +1,10 @@
+2016-02-15 James Norris
+
+ Backport from trunk:
+ PR c/64748
+ * c-c++-common/goacc/deviceptr-1.c: Add tests.
+ * g++.dg/goacc/deviceptr-1.c: New file.
+
2016-02-03 James Norris
Backport from trunk:
Index: gcc/testsuite/c-c++-common/goacc/deviceptr-1.c
===
--- gcc/testsuite/c-c++-common/goacc/deviceptr-1.c (revision 233463)
+++ gcc/testsuite/c-c++-common/goacc/deviceptr-1.c (working copy)
@@ -84,3 +84,17 @@
#pragma acc parallel deviceptr(s2_p)
s2_p = 0;
}
+
+void
+func5 (float *fp)
+{
+#pragma acc data deviceptr (fp)
+ ;
+}
+
+void
+func6 (float fp)
+{
+#pragma acc data deviceptr (fp) /* { dg-error "is not a pointer variable" } */
+ ;
+}
Index: gcc/testsuite/g++.dg/goacc/deviceptr-1.C
===
--- gcc/testsuite/g++.dg/goacc/deviceptr-1.C (revision 0)
+++ gcc/testsuite/g++.dg/goacc/deviceptr-1.C (working copy)
@@ -0,0 +1,38 @@
+// { dg-do compile }
+
+template
+void
+func1 (P p)
+{
+#pragma acc data deviceptr (p)// { dg-bogus "is not a pointer" }
+ ;
+}
+
+void
+func2 (int *p)
+{
+ func1 (p);
+}
+
+template
+void
+func3 (P p)
+{
+#pragma acc data deviceptr (p)// { dg-error "is not a pointer" }
+ ;
+}
+void
+func4 (int p)
+{
+ func3 (p);
+}
+
+template
+void
+func5 (int *p, int q)
+{
+#pragma acc data deviceptr (p)// { dg-bogus "is not a pointer" }
+ ;
+#pragma acc data deviceptr (q)// { dg-error "is not a pointer" }
+ ;
+}
Re: [PATCH] 69759 - document __builtin_alloca and __builtin_alloca_with_align
On 02/16/2016 08:54 AM, Joseph Myers wrote:
On Tue, 16 Feb 2016, Martin Sebor wrote:
Max_align_t need not impact efficiency. In C, the malloc alignment
also need not have an impact on the type since aligned_alloc returns
storage that is (or can be) more strictly aligned.
As per the DR#445 resolution, malloc must return memory "whose alignment
is the greatest fundamental alignment". It's obviously necessary for
malloc to return memory sufficiently aligned for any object that can be
declared within the standard without using over-aligned types.
The DR #445 resolution goes against the intent of the type. It's
also incompatible with C++. Unless there's a C++ defect to change
it there too (I haven't found it), the C DR should be revisited to
make sure the languages remain compatible in this area.
If you disregard malloc, that is effectively unbounded (2^28 bytes at
least), with current GCC on ELF targets. This is not a useful thing to
query, and not useful for malloc to be required to return memory aligned
to.
Malloc is not (need not be) subject to the (original, pre-DR 445)
max_align_t alignment. What to define max_align_to when there
effectively is maximum alignment is a separate question.
But this discussion is tangential to the proposed documentation
update. After thinking about the context some more I've decided
that since code that uses __builtin_alloca or even alloca isn't
portable to begin with, mentioning max_align_t won't help it
become any more portable.
Attached is an updated patch with the mention of max_align_t
removed.
Martin
PR c/69759 - __builtin_alloca and __builtin_alloca_with_align undocumented
gcc/ChangeLog:
2016-02-16 Martin Sebor
PR c/69759
* doc/extend.texi (Other Builtins): Document __builtin_alloca and
__builtin_alloca_with_align.
Index: gcc/doc/extend.texi
===
--- gcc/doc/extend.texi (revision 233367)
+++ gcc/doc/extend.texi (working copy)
@@ -10144,6 +10144,8 @@ in the Cilk Plus language manual which c
@node Other Builtins
@section Other Built-in Functions Provided by GCC
@cindex built-in functions
+@findex __builtin_alloca
+@findex __builtin_alloca_with_align
@findex __builtin_call_with_static_chain
@findex __builtin_fpclassify
@findex __builtin_isfinite
@@ -10690,6 +10692,89 @@ In the same fashion, GCC provides @code{
@code{__builtin_} prefixed. The @code{isinf} and @code{isnan}
built-in functions appear both with and without the @code{__builtin_} prefix.
+@deftypefn {Built-in Function} void* __builtin_alloca (size_t size)
+The @code{__builtin_alloca} function must be called at block scope.
+The function allocates an object @var{size} bytes large on the stack
+of the calling function. The object is aligned at the default stack
+alignment boundary for the target determined by the
+@code{__BIGGEST_ALIGNMENT__} macro. @code{__builtin_alloca} returns
+a pointer to the first byte of the allocated object. The lifetime of
+the allocated object ends just before the calling function returns to
+its caller. This is so even when @code{__builtin_alloca_with_align}
+is called within a nested block.
+
+For example, the following function allocates eight objects of @code{n}
+bytes each on the stack, storing a pointer to each in consecutive elements
+of the array @code{a}. It then passes the array to function @code{g()}
+which can safely use the storage pointed to by each of the array elements.
+
+@smallexample
+void f (unsigned n)
+@{
+ void *a [8];
+ for (int i = 0; i != 8; ++i)
+a [i] = __builtin_alloca (n);
+
+ g (a, n); // safe
+@}
+@end smallexample
+
+Since the @code{__builtin_alloca} function doesn't validate its arguments
+it is the responsibility of its caller to make sure the argument doesn't
+cause it doesn't exceed the stack size limit.
+The @code{__builtin_alloca} function is provided to make it possible to
+allocate arrays with a runtime bound on the stack. Since C99 variable
+length arrays offer similar functionality under a portable, more convenient,
+and safer interface they are recommended instead, in both C99 and C++
+programs where GCC provides them as an extension.
+
+@end deftypefn
+
+@deftypefn {Built-in Function} void* __builtin_alloca_with_align (size_t size, size_t align)
+The @code{__builtin_alloca_with_align} function must be called at block
+scope. The function allocates an object @var{align} bytes large on
+the stack of the calling function. The allocated object is aligned on
+the boundary specified by the argument @var{align} whose unit is given
+in bits (not bytes). @var{size} must be positive and not exceed the stack
+size limit. @var{align} must be a constant integer expression that
+evaluates to a power of 2 greater than or equal to @code{__CHAR_BIT__}.
+Invocations with other values are rejected with an error. The function
+returns a pointer to the first byte of the allocated object. The lifetime
+of the allocated object ends at the en
Re: [PING][PATCH, PR67709 ] Don't call call_cgraph_insertion_hooks in simd_clone_create
On 16/02/16 12:11, Jakub Jelinek wrote: On Tue, Feb 16, 2016 at 12:10:29PM +0100, Tom de Vries wrote: >On 16/02/16 11:04, Jakub Jelinek wrote: > >On Tue, Feb 16, 2016 at 10:56:58AM +0100, Tom de Vries wrote: > >>>AFAIU, it's not a release regression given that: > >>>- this has failed since 4.9.0, and > >>>- the test-case is not supported in 4.8, > >>>so we're not required to fix it in 4.9 and 5 branches. > >>> > >>>But, the test-case does fail in 5 and 4.9 branches, and the fix seems simple > >>>enough to not have the risk of introducing breakage in the release branches. > >>> > >>>OK for 4.9/5 branch? > >Okay. > > > >Hmm, it doesn't apply cleanly. > >The trunk version of simd_clone_create also contains: 'Fix ICE for SIMD >clones usage in LTO' ( >https://gcc.gnu.org/ml/gcc-patches/2015-10/msg00434.html ), which adds the >'in_other_partition' propagation in simd_clone_create. > >That's also not a release regression (reproduces with 4.9.0 release, not >supported in 4.8), and still reproduces on 4.9 and 5 branches. > >OK to backport 'Fix ICE for SIMD clones usage in LTO' to 4.9/5 branch? Ok. Committed both patches to 4.9 and 5 branches. In order to run testsuite/libgomp.fortran/declare-simd-4.f90 with the 4.9 branch build, I needed in addition: - r212268 https://gcc.gnu.org/viewcvs/gcc?view=revision&revision=212268 https://gcc.gnu.org/ml/gcc-patches/2014-07/msg00218.html - r219606 https://gcc.gnu.org/viewcvs/gcc?view=revision&revision=219606 https://gcc.gnu.org/ml/gcc-patches/2015-01/msg00615.html Should these also be back-ported to 4.9 branch (and in case of r219606, also follow-up fix r219722, https://gcc.gnu.org/viewcvs?rev=219722&root=gcc&view=rev , fix for https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64605 )? Thanks, - Tom
Re: [PING][PATCH, PR67709 ] Don't call call_cgraph_insertion_hooks in simd_clone_create
On Tue, Feb 16, 2016 at 05:52:55PM +0100, Tom de Vries wrote: > Committed both patches to 4.9 and 5 branches. > > In order to run testsuite/libgomp.fortran/declare-simd-4.f90 with the 4.9 > branch build, I needed in addition: > - r212268 > https://gcc.gnu.org/viewcvs/gcc?view=revision&revision=212268 > https://gcc.gnu.org/ml/gcc-patches/2014-07/msg00218.html > - r219606 > https://gcc.gnu.org/viewcvs/gcc?view=revision&revision=219606 > https://gcc.gnu.org/ml/gcc-patches/2015-01/msg00615.html > > Should these also be back-ported to 4.9 branch (and in case of r219606, also > follow-up fix r219722, > https://gcc.gnu.org/viewcvs?rev=219722&root=gcc&view=rev , fix for > https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64605 )? IMHO just don't backport that test, test it by hand only. Jakub
Re: [wwwdocs] Describe behavior of -flifetime-dse in class constructors
On 02/16/2016 07:55 AM, Martin Liška wrote: Hello. As I finally hunted issue in Firefox that was responsible for start-up segfault, I would like to describe a new behavior of the compiler that emits clobbers to class constructors (w/ -flifetime-dse). As also Richi spotted quite similar issue in openjade package, I think it worth for mentioning in porting: I think the new text deserves a new heading of its own rather than being added under the existing "Stricter flexible array member rules." (The "Finally..." part changed by the patch still applies to the flexible array members.) Martin
Re: [PATCH] 69759 - document __builtin_alloca and __builtin_alloca_with_align
On Tue, 16 Feb 2016, Martin Sebor wrote: > On 02/16/2016 08:54 AM, Joseph Myers wrote: > > On Tue, 16 Feb 2016, Martin Sebor wrote: > > > > > Max_align_t need not impact efficiency. In C, the malloc alignment > > > also need not have an impact on the type since aligned_alloc returns > > > storage that is (or can be) more strictly aligned. > > > > As per the DR#445 resolution, malloc must return memory "whose alignment > > is the greatest fundamental alignment". It's obviously necessary for > > malloc to return memory sufficiently aligned for any object that can be > > declared within the standard without using over-aligned types. > > The DR #445 resolution goes against the intent of the type. It's I see no reason to suppose that, given the extensive discussion of the issues in WG14. max_align_t is of use as-is in writing allocators and allocator-like code (cf how glibc uses it). Something that represents the possibility of aligning things to anything up to the entire address space would not be useful. > also incompatible with C++. Unless there's a C++ defect to change Presumably the C++ liaison people considered compatibility as part of the WG14 discussions. In any case, I see no sign that (beyond the "fundamental type" terminology issue) the wording in C++ is any better thought out than the pre-DR#445 C wording. > > If you disregard malloc, that is effectively unbounded (2^28 bytes at > > least), with current GCC on ELF targets. This is not a useful thing to > > query, and not useful for malloc to be required to return memory aligned > > to. > > Malloc is not (need not be) subject to the (original, pre-DR 445) > max_align_t alignment. What to define max_align_to when there C90 and C99 compatibility requires malloc to return memory suitably aligned for all types definable within C90 and C99, and it would be very confusing to say in C11 that certain header typedefs are within such a rule but others (such as max_align_t) aren't because they are new in C11 (a rule saying that all standard header types can be stored in memory returned by malloc is much better for the C11 user). -- Joseph S. Myers jos...@codesourcery.com
RFC: Fix ARMv3 support
Hi Richard, Hi Ramana,
The ARM backend has some problems compiling for the old ARMv3
architecture. See:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=62254
for an example of this. v3 is very old now, and I am not sure how
much interest there is in continuing to support it, but I am trying to
help reduce the gcc priority bug list, so here goes...
The attached patch fixes the problem, albeit not in a very subtle way.
The problem is that arm_reload_[out|in]_hi is being called for a
register->register move because the v3 architecture does not support
16-bit register moves. Rather than trace the problem back to the real
source and fix it, I chose to just allow the reload functions to
generate an SImode register move instead. Probably not the best
solution, but it appears to work.
The attached patch also includes the test cases derived from PR 62254
and PR 69610 (which is a duplicate of PR 62254). Including all three
tests might be overkill, but it seemed like a good idea to be a little
bit paranoid, just in case.
Whilst testing the patch I also discovered that interworking is
enabled by default, which is a problem for v3 code generation, so I
added a fix to (silently) disable interworking if the target
architecture does not support Thumb instructions.
Any comments or criticisms before I apply the patch ?
Cheers
Nick
gcc/ChangeLog
2016-02-16 Nick Clifton
PR target/62554
PR target/69610
* config/arm/arm.c (arm_option_override_internal): Disable
interworking if the target does not support thumb instructions.
(arm_reload_in_hi): Handle the case where a register to register
move needs reloading because there is no simple pattern to handle
it.
(arm_reload_out_hi): Likewise.
gcc/testsuite/ChangeLog
2016-02-16 Nick Clifton
PR target/62554
PR target/69610
* gcc.target/arm/pr62554.c: New test.
* gcc.target/arm/pr69610-1.c: New test.
* gcc.target/arm/pr69610-2.c: New test.
Index: gcc/config/arm/arm.c
===
--- gcc/config/arm/arm.c (revision 233443)
+++ gcc/config/arm/arm.c (working copy)
@@ -2874,6 +2874,14 @@
{
arm_override_options_after_change_1 (opts);
+ if (TARGET_INTERWORK && !ARM_FSET_HAS_CPU1 (insn_flags, FL_THUMB))
+{
+ /* The default is to enable interworking, so this warning message would
+ be confusing to users who have just compiled with, eg, -march=armv3. */
+ /* warning (0, "ignoring -minterwork because target CPU does not support THUMB"); */
+ opts->x_target_flags &= ~MASK_INTERWORK;
+}
+
if (TARGET_THUMB_P (opts->x_target_flags)
&& !(ARM_FSET_HAS_CPU1 (insn_flags, FL_THUMB)))
{
@@ -15440,6 +15448,17 @@
else
/* The slot is out of range, or was dressed up in a SUBREG. */
base = reg_equiv_address (REGNO (ref));
+
+ /* PR 62554: If there is no equivalent memory location then just move
+ the value as an SImode register move. This happens when the target
+ architecure variant does not have an HImode register move. */
+ if (base == NULL)
+ {
+ gcc_assert (REG_P (operands[0]));
+ emit_insn (gen_movsi (gen_rtx_SUBREG (SImode, operands[0], 0),
+gen_rtx_SUBREG (SImode, ref, 0)));
+ return;
+ }
}
else
base = find_replacement (&XEXP (ref, 0));
@@ -15557,6 +15576,17 @@
else
/* The slot is out of range, or was dressed up in a SUBREG. */
base = reg_equiv_address (REGNO (ref));
+
+ /* PR 62554: If there is no equivalent memory location then just move
+ the value as an SImode register move. This happens when the target
+ architecure variant does not have an HImode register move. */
+ if (base == NULL)
+ {
+ gcc_assert (REG_P (outval));
+ emit_insn (gen_movsi (gen_rtx_SUBREG (SImode, ref, 0),
+gen_rtx_SUBREG (SImode, outval, 0)));
+ return;
+ }
}
else
base = find_replacement (&XEXP (ref, 0));
@@ -19619,6 +19649,7 @@
break;
case ARM_FT_INTERWORKED:
+ gcc_assert (arm_arch5 || arm_arch4t);
sprintf (instr, "bx%s\t%%|lr", conditional);
break;
--- /dev/null 2016-02-16 08:27:18.513962320 +
+++ gcc/testsuite/gcc.target/arm/pr62254.c 2016-02-16 16:47:30.479378118 +
@@ -0,0 +1,50 @@
+/* Check that pre ARMv4 compilation still works. */
+/* { dg-do compile } */
+/* { dg-options "-marm -march=armv3 -O" } */
+
+typedef struct
+{
+ char bits;
+ short val;
+} code;
+
+union uu
+{
+ short us;
+ char b[2];
+};
+
+int a, b, c, f, g, h;
+code *d;
+
+code e;
+
+int
+fn1 (void)
+{
+ char i;
+ do
+if (e.bits)
+ {
+ dodist:
+f = c;
+if (e.bits & 6)
+ {
+++i;
+if (g)
+ do
+{
+ union uu j;
+ j.b[1] = a;
+ h = j.us;
+}
+ while (fn1);
+ }
+else
+ {
Re: [PATCH] Fix PR69801
On Tue, Feb 16, 2016 at 12:35 AM, Richard Biener wrote:
>
> Bootstrapped and tested on x86_64-unknown-linux-gnu, applied.
>
> Richard.
>
> 2016-02-16 Richard Biener
>
> PR middle-end/69801
> * fold-const.c (operand_equal_p): For COND_EXPR zero operand
> mask OEP_ADDRESS_OF.
>
> * gcc.dg/pr69801.c: New testcase.
>
> Index: gcc/fold-const.c
> ===
> *** gcc/fold-const.c(revision 233418)
> --- gcc/fold-const.c(working copy)
> *** operand_equal_p (const_tree arg0, const_
> *** 3118,3123
> --- 3118,3128
> TREE_OPERAND (arg1, 0), flags));
>
> case COND_EXPR:
> + if (! OP_SAME (1) || ! OP_SAME (2))
> + return 0;
> + flags &= ~OEP_ADDRESS_OF;
> + return OP_SAME (0);
> +
> case VEC_COND_EXPR:
> case DOT_PROD_EXPR:
> return OP_SAME (0) && OP_SAME (1) && OP_SAME (2);
> Index: gcc/testsuite/gcc.dg/pr69801.c
> ===
> *** gcc/testsuite/gcc.dg/pr69801.c (revision 0)
> --- gcc/testsuite/gcc.dg/pr69801.c (working copy)
> ***
> *** 0
> --- 1,7
> + /* { dg-do compile } */
> +
> + struct {
> + char c[1];
> + } b, c;
> + int d, e;
> + void fn1() { e ? (d ? b : c).c : (d ? b : c).c; }
I got
output is:
/export/gnu/import/git/gcc-test-ia32corei7/src-trunk/gcc/testsuite/gcc.dg/pr69801.c:
In function 'fn1':^M
/export/gnu/import/git/gcc-test-ia32corei7/src-trunk/gcc/testsuite/gcc.dg/pr69801.c:7:32:
error: non-lvalue array in conditional expression^M
FAIL: gcc.dg/pr69801.c (test for excess errors)
--
H.J.
Re: [PATCH] Fix PR69801
On Tue, Feb 16, 2016 at 09:39:10AM -0800, H.J. Lu wrote:
> Index: gcc/testsuite/gcc.dg/pr69801.c
> > ===
> > *** gcc/testsuite/gcc.dg/pr69801.c (revision 0)
> > --- gcc/testsuite/gcc.dg/pr69801.c (working copy)
> > ***
> > *** 0
> > --- 1,7
> > + /* { dg-do compile } */
> > +
> > + struct {
> > + char c[1];
> > + } b, c;
> > + int d, e;
> > + void fn1() { e ? (d ? b : c).c : (d ? b : c).c; }
>
> I got
>
> output is:
> /export/gnu/import/git/gcc-test-ia32corei7/src-trunk/gcc/testsuite/gcc.dg/pr69801.c:
> In function 'fn1':^M
> /export/gnu/import/git/gcc-test-ia32corei7/src-trunk/gcc/testsuite/gcc.dg/pr69801.c:7:32:
> error: non-lvalue array in conditional expression^M
>
> FAIL: gcc.dg/pr69801.c (test for excess errors)
Should be already fixed:
https://gcc.gnu.org/ml/gcc-patches/2016-02/msg01084.html
Marek
Re: [6 Regession] Usage of unitialized pointer io/list_read.c (
On 02/16/2016 12:06 AM, Christophe Lyon wrote: > On 15 February 2016 at 23:16, Janne Blomqvist > wrote: >> On Mon, Feb 15, 2016 at 11:45 PM, Jerry DeLisle >> wrote: >>> The title of the PR should be "Mishandling of namelist comments" or >>> "Interpreting '!' as a comment in non-namelist reads". --- snip --- > > Although OK in trunk, I've noticed regressions in the gcc-5 branch > since you commtted > r233442: > There is a subsequent commit that updates those two failing cases, namelist_38.f90 and namelist_84.f90. Please check that you have those updates and let me know if they still fail. Regards, Jerry
[Ada] Define LANG_HOOKS_ENUM_UNDERLYING_BASE_TYPE
The fix for PR debug/16063 slightly disturbed the debug info in Ada by adding
bogus base types to enumeration types; the latter are base types in Ada.
Tested on x86_64-suse-linux, applied on the mainline and 5 branch.
2016-02-16 Eric Botcazou
* gcc-interface/misc.c (gnat_enum_underlying_base_type): New function.
(LANG_HOOKS_ENUM_UNDERLYING_BASE_TYPE): Define to above.
--
Eric BotcazouIndex: gcc-interface/misc.c
===
--- gcc-interface/misc.c (revision 233448)
+++ gcc-interface/misc.c (working copy)
@@ -575,6 +575,15 @@ gnat_descriptive_type (const_tree type)
return NULL_TREE;
}
+/* Return the underlying base type of an enumeration type. */
+
+static tree
+gnat_enum_underlying_base_type (const_tree)
+{
+ /* Enumeration types are base types in Ada. */
+ return void_type_node;
+}
+
/* Return the type to be used for debugging information instead of TYPE or
NULL_TREE if TYPE is fine. */
@@ -1390,6 +1399,8 @@ get_lang_specific (tree node)
#define LANG_HOOKS_GET_TYPE_BIAS gnat_get_type_bias
#undef LANG_HOOKS_DESCRIPTIVE_TYPE
#define LANG_HOOKS_DESCRIPTIVE_TYPE gnat_descriptive_type
+#undef LANG_HOOKS_ENUM_UNDERLYING_BASE_TYPE
+#define LANG_HOOKS_ENUM_UNDERLYING_BASE_TYPE gnat_enum_underlying_base_type
#undef LANG_HOOKS_GET_DEBUG_TYPE
#define LANG_HOOKS_GET_DEBUG_TYPE gnat_get_debug_type
#undef LANG_HOOKS_GET_FIXED_POINT_TYPE_INFO
Re: [6 Regession] Usage of unitialized pointer io/list_read.c (
On 02/16/2016 05:17 AM, Dominique d'Humières wrote: > Hi Jerry, > >> Thanks for review. Committed to trunk r233436. > > The test gfortran.dg/read_bang4.f90 fails on x86_64-apple-darwin15: > > a.out(15552,0x7fff7b2e3000) malloc: *** error for object 0x7fb472804c00: > pointer being freed was not allocated > *** set a breakpoint in malloc_error_break to debug > > Program received signal SIGABRT: Process abort signal. > > If I compile the test with -fsanitize=address, I get > > = > ==15561==ERROR: AddressSanitizer: attempting double-free on 0x61a1c280 in > thread T0: > #0 0x106d1ac40 in wrap_free.part.0 sanitizer_malloc_mac.inc:103 > #1 0x106c0078f in _gfortrani_list_formatted_read list_read.c:137 > #2 0x440042 () > > 0x61a1c280 is located 0 bytes inside of 1200-byte region > [0x61a1c280,0x61a1c730) > freed by thread T0 here: > #0 0x106d19e09 in wrap_realloc sanitizer_malloc_mac.inc:108 > #1 0x106b38a14 in _gfortrani_xrealloc memory.c:98 > > previously allocated by thread T0 here: > #0 0x106d19fe9 in wrap_calloc sanitizer_malloc_mac.inc:114 > #1 0x106b389d2 in _gfortrani_xcalloc memory.c:84 > > SUMMARY: AddressSanitizer: double-free sanitizer_malloc_mac.inc:103 in > wrap_free.part.0 > ==15561==ABORTING > > Program received signal SIGABRT: Process abort signal. > > TIA > > Dominique > > Up until now we have not had a test case like this for kind=4 testing. I did try -fsanitize=address here and do see the same thing. I will work on this some more. Thanks, Jerry
[Ada] Fix debug info glitches for character indexes
This is another regression in the debug info, this time introduced on the
mainline by my patch toggling the signedness of Character in Ada.
Tested on x86_64-suse-linux, applied on the mainline.
2016-02-16 Eric Botcazou
* gcc-interface/gigi.h (maybe_debug_type): New inline function.
* gcc-interface/misc.c (gnat_get_array_descr_info): Use it.
Call maybe_character_value on the array bounds. Get to the base type
of the index type and call maybe_debug_type on it.
* gcc-interface/utils.c (finish_character_type): Add special treatment
for char_type_node.
--
Eric BotcazouIndex: gcc-interface/gigi.h
===
--- gcc-interface/gigi.h (revision 233448)
+++ gcc-interface/gigi.h (working copy)
@@ -1164,3 +1164,14 @@ maybe_character_value (tree expr)
return expr;
}
+
+/* Return the debug type of TYPE if it exists, otherwise TYPE itself. */
+
+static inline tree
+maybe_debug_type (tree type)
+{
+ if (TYPE_CAN_HAVE_DEBUG_TYPE_P (type) && TYPE_DEBUG_TYPE (type))
+type = TYPE_DEBUG_TYPE (type);
+
+ return type;
+}
Index: gcc-interface/misc.c
===
--- gcc-interface/misc.c (revision 233467)
+++ gcc-interface/misc.c (working copy)
@@ -786,8 +786,7 @@ gnat_get_array_descr_info (const_tree co
tree thinptr_bound_field = NULL_TREE;
/* ??? See gnat_get_debug_type. */
- if (TYPE_CAN_HAVE_DEBUG_TYPE_P (type) && TYPE_DEBUG_TYPE (type))
-type = TYPE_DEBUG_TYPE (type);
+ type = maybe_debug_type (type);
/* If we have an implementation type for a packed array, get the orignial
array type. */
@@ -944,8 +943,10 @@ gnat_get_array_descr_info (const_tree co
}
else
{
- info->dimen[i].lower_bound = TYPE_MIN_VALUE (index_type);
- info->dimen[i].upper_bound = TYPE_MAX_VALUE (index_type);
+ info->dimen[i].lower_bound
+ = maybe_character_value (TYPE_MIN_VALUE (index_type));
+ info->dimen[i].upper_bound
+ = maybe_character_value (TYPE_MAX_VALUE (index_type));
}
}
@@ -963,13 +964,12 @@ gnat_get_array_descr_info (const_tree co
thinptr_bound_field = DECL_CHAIN (thinptr_bound_field);
}
- /* The DWARF back-end will output exactly INDEX_TYPE as the array index'
- "root" type, so pell subtypes when possible. */
- while (TREE_TYPE (index_type)
- && !subrange_type_for_debug_p (index_type, NULL, NULL))
+ /* The DWARF back-end will output BOUNDS_TYPE as the base type of
+ the array index, so get to the base type of INDEX_TYPE. */
+ while (TREE_TYPE (index_type))
index_type = TREE_TYPE (index_type);
- info->dimen[i].bounds_type = index_type;
+ info->dimen[i].bounds_type = maybe_debug_type (index_type);
info->dimen[i].stride = NULL_TREE;
}
Index: gcc-interface/utils.c
===
--- gcc-interface/utils.c (revision 233448)
+++ gcc-interface/utils.c (working copy)
@@ -1625,8 +1625,11 @@ finish_character_type (tree char_type)
if (TYPE_UNSIGNED (char_type))
return;
- /* Make a copy of the unsigned version since we'll modify it below. */
- tree unsigned_char_type = copy_type (gnat_unsigned_type_for (char_type));
+ /* Make a copy of a generic unsigned version since we'll modify it. */
+ tree unsigned_char_type
+= (char_type == char_type_node
+ ? unsigned_char_type_node
+ : copy_type (gnat_unsigned_type_for (char_type)));
TYPE_NAME (unsigned_char_type) = TYPE_NAME (char_type);
TYPE_STRING_FLAG (unsigned_char_type) = TYPE_STRING_FLAG (char_type);
Re: [PATCH] Move the -Wnonnull compare against NULL warning from FEs to early uninit under -Wnonnull-compare (PR c/69835)
On 02/16/2016 08:36 AM, Richard Biener wrote: On Tue, 16 Feb 2016, Jakub Jelinek wrote: Hi! As has been reported today on gcc@, the new -Wnonnull warning addition warns even about nonnull parameters that were changed (or could be changed) in the function. IMHO the nonnull attribute only talks about the value of the parameter upon entry to the function, if you assign something else to the argument afterwards and test that for NULL or non-NULL, it is like any other variable. But, we don't have the infrastructure to detect this in the FE, so this patch moves the warning soon after we go into SSA form. As -Wnonnull is a C/C++/ObjC/ObjC++ only warning, I've added a new -Wnonnull-compare switch for it too (and enable it for C/C++/ObjC/ObjC++ from -Wall). Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? Not sure if it matters but wouldn't walking over function parameters and their default def SSA names immediate uses be cheaper than matching all conditions? Esp. as nonnull_arg_p walks over all DECL_ARGUMENTS (up to the searched index) over and over again? [we should simply set a flag on the PARM_DECL!] Seems like a waste to burn a flag bit for something like this. We're already walking all the statements in this code so it's really just the cost of testing if the existing statement is "interesting" for this warning. I guess you're suggesting jakub walk the arguments and if one is a marked as non-null, then walk the immediate uses. Yea, that's probably faster than doing the test for every statement. Swapping the nonnull_arg_p and the test for whether or not op1 is 0 would be a slight help as well, particularly if there was machine generated code with oodles of arguments. I'm not sure why we're testing OFFSET_TYPE against -1. warn_uninitialized_vars seems like a poor name if we're warning for non-null too. So I'd suggest tweaking its name appropriately as well. Jeff
Re: [PATCH] Move the -Wnonnull compare against NULL warning from FEs to early uninit under -Wnonnull-compare (PR c/69835)
On Tue, Feb 16, 2016 at 11:12:27AM -0700, Jeff Law wrote:
> >Not sure if it matters but wouldn't walking over function parameters
> >and their default def SSA names immediate uses be cheaper than
> >matching all conditions? Esp. as nonnull_arg_p walks over all
> >DECL_ARGUMENTS (up to the searched index) over and over again?
> >[we should simply set a flag on the PARM_DECL!]
> Seems like a waste to burn a flag bit for something like this.
>
> We're already walking all the statements in this code so it's really just
> the cost of testing if the existing statement is "interesting" for this
> warning. I guess you're suggesting jakub walk the arguments and if one is a
> marked as non-null, then walk the immediate uses. Yea, that's probably
> faster than doing the test for every statement.
I'm already bootstrapping/regtesting following variant.
> Swapping the nonnull_arg_p and the test for whether or not op1 is 0 would be
> a slight help as well, particularly if there was machine generated code with
> oodles of arguments.
>
> I'm not sure why we're testing OFFSET_TYPE against -1.
That is because NULL pointers to member are represented as -1 in the middle
end.
2016-02-16 Jakub Jelinek
PR c/69835
* common.opt (Wnonnull-compare): New warning.
* doc/invoke.texi (-Wnonnull): Remove text about comparison
of arguments against NULL.
(-Wnonnull-compare): Document.
* Makefile.in (OBJS): Add gimple-ssa-nonnull-compare.o.
* tree-pass.h (make_pass_warn_nonnull_compare): Declare.
* passes.def (pass_warn_nonnull_compare): Add.
* gimple-ssa-nonnull-compare.c: New file.
c-family/
* c.opt (Wnonnull-compare): Enable for -Wall.
c/
* c-typeck.c (build_binary_op): Revert 2015-09-09 change.
cp/
* typeck.c (cp_build_binary_op): Revert 2015-09-09 change.
testsuite/
* c-c++-common/nonnull-1.c: Use -Wnonnull-compare instead of
-Wnonnull in dg-options.
* c-c++-common/nonnull-2.c: New test.
--- gcc/common.opt.jj 2016-02-16 16:14:40.787071893 +0100
+++ gcc/common.opt 2016-02-16 17:34:12.850648209 +0100
@@ -616,6 +616,10 @@ Wlarger-than=
Common RejectNegative Joined UInteger Warning
-Wlarger-than= Warn if an object is larger than bytes.
+Wnonnull-compare
+Var(warn_nonnull_compare) Warning
+Warn if comparing pointer parameter with nonnull attribute with NULL.
+
Wnull-dereference
Common Var(warn_null_dereference) Warning
Warn if dereferencing a NULL pointer may lead to erroneous or undefined
behavior.
--- gcc/doc/invoke.texi.jj 2016-02-16 16:14:41.692059415 +0100
+++ gcc/doc/invoke.texi 2016-02-16 17:26:55.346587296 +0100
@@ -276,7 +276,8 @@ Objective-C and Objective-C++ Dialects}.
-Wmain -Wmaybe-uninitialized -Wmemset-transposed-args @gol
-Wmisleading-indentation -Wmissing-braces @gol
-Wmissing-field-initializers -Wmissing-include-dirs @gol
--Wno-multichar -Wnonnull -Wnormalized=@r{[}none@r{|}id@r{|}nfc@r{|}nfkc@r{]}
@gol
+-Wno-multichar -Wnonnull -Wnonnull-compare @gol
+-Wnormalized=@r{[}none@r{|}id@r{|}nfc@r{|}nfkc@r{]} @gol
-Wnull-dereference -Wodr -Wno-overflow -Wopenmp-simd @gol
-Woverride-init-side-effects -Woverlength-strings @gol
-Wpacked -Wpacked-bitfield-compat -Wpadded @gol
@@ -3537,6 +3538,7 @@ Options} and @ref{Objective-C and Object
-Wmissing-braces @r{(only for C/ObjC)} @gol
-Wnarrowing @r{(only for C++)} @gol
-Wnonnull @gol
+-Wnonnull-compare @gol
-Wopenmp-simd @gol
-Wparentheses @gol
-Wpointer-sign @gol
@@ -3795,12 +3797,18 @@ formats that may yield only a two-digit
Warn about passing a null pointer for arguments marked as
requiring a non-null value by the @code{nonnull} function attribute.
-Also warns when comparing an argument marked with the @code{nonnull}
-function attribute against null inside the function.
-
@option{-Wnonnull} is included in @option{-Wall} and @option{-Wformat}. It
can be disabled with the @option{-Wno-nonnull} option.
+@item -Wnonnull-compare
+@opindex Wnonnull-compare
+@opindex Wno-nonnull-compare
+Warn when comparing an argument marked with the @code{nonnull}
+function attribute against null inside the function.
+
+@option{-Wnonnull-compare} is included in @option{-Wall}. It
+can be disabled with the @option{-Wno-nonnull-compare} option.
+
@item -Wnull-dereference
@opindex Wnull-dereference
@opindex Wno-null-dereference
--- gcc/Makefile.in.jj 2016-02-11 20:28:54.0 +0100
+++ gcc/Makefile.in 2016-02-16 17:32:26.919086223 +0100
@@ -1278,6 +1278,7 @@ OBJS = \
gimple-pretty-print.o \
gimple-ssa-backprop.o \
gimple-ssa-isolate-paths.o \
+ gimple-ssa-nonnull-compare.o \
gimple-ssa-split-paths.o \
gimple-ssa-strength-reduction.o \
gimple-streamer-in.o \
--- gcc/tree-pass.h.jj 2016-01-19 13:31:07.0 +0100
+++ gcc/tree-pass.h 2016-02-16 17:28:39.155178102 +0100
@@ -472,6 +472,7 @@ extern gimple_opt_pass *make_pass_oacc_k
extern simple_ipa_opt
Re: C++ PATCH for c++/69753 (DR141 broke member template lookup)
On 02/16/2016 09:32 AM, Patrick Palka wrote: clang also rejects the case where A::FromWebContents is overloaded with a static member function and non-static one, and gcc now accepts this case. This is a clang bug. Jason
Re: [PATCH] Move the -Wnonnull compare against NULL warning from FEs to early uninit under -Wnonnull-compare (PR c/69835)
On 02/16/2016 11:21 AM, Jakub Jelinek wrote: On Tue, Feb 16, 2016 at 11:12:27AM -0700, Jeff Law wrote: Not sure if it matters but wouldn't walking over function parameters and their default def SSA names immediate uses be cheaper than matching all conditions? Esp. as nonnull_arg_p walks over all DECL_ARGUMENTS (up to the searched index) over and over again? [we should simply set a flag on the PARM_DECL!] Seems like a waste to burn a flag bit for something like this. We're already walking all the statements in this code so it's really just the cost of testing if the existing statement is "interesting" for this warning. I guess you're suggesting jakub walk the arguments and if one is a marked as non-null, then walk the immediate uses. Yea, that's probably faster than doing the test for every statement. I'm already bootstrapping/regtesting following variant. Swapping the nonnull_arg_p and the test for whether or not op1 is 0 would be a slight help as well, particularly if there was machine generated code with oodles of arguments. I'm not sure why we're testing OFFSET_TYPE against -1. That is because NULL pointers to member are represented as -1 in the middle end. 2016-02-16 Jakub Jelinek PR c/69835 * common.opt (Wnonnull-compare): New warning. * doc/invoke.texi (-Wnonnull): Remove text about comparison of arguments against NULL. (-Wnonnull-compare): Document. * Makefile.in (OBJS): Add gimple-ssa-nonnull-compare.o. * tree-pass.h (make_pass_warn_nonnull_compare): Declare. * passes.def (pass_warn_nonnull_compare): Add. * gimple-ssa-nonnull-compare.c: New file. c-family/ * c.opt (Wnonnull-compare): Enable for -Wall. c/ * c-typeck.c (build_binary_op): Revert 2015-09-09 change. cp/ * typeck.c (cp_build_binary_op): Revert 2015-09-09 change. testsuite/ * c-c++-common/nonnull-1.c: Use -Wnonnull-compare instead of -Wnonnull in dg-options. * c-c++-common/nonnull-2.c: New test. That works for me. FWIW I'm certainly not a fan of doing this stuff on the FEs; too many false positives and too easy to miss valid warnings. So I'm in full support to pushing this a bit further back in the pipeline. Jeff
Re: [PATCH PR69052]Check if loop inv can be propagated into mem ref with additional addr expr canonicalization
From: Jeff Law
Sent: 11 February 2016 23:26
To: Bin.Cheng
Cc: Bin Cheng; gcc-patches@gcc.gnu.org; nd
Subject: Re: [PATCH PR69052]Check if loop inv can be propagated into mem ref
with additional addr expr canonicalization
>> On 02/11/2016 10:59 AM, Bin.Cheng wrote:
>> Hi Jeff,
>> Thanks for detailed review. I also think a generic canonical
>> interface for RTL is much better. I will give it a try. But with
>> high chance it's a next stage1 stuff.
> That is, of course, fine. However, if you do get something ready, I'd
> support using it within LICM for gcc-6, then using it in other places
> for gcc-7.
Hi,
This is the updated version patch. It fixes the problem by introducing a
generic address canonicalization interface. This new interface canonicalizes
address expression in following steps:
1) Rewrite ASHIFT into MULT recursively.
2) Divide address into sub expressions with PLUS as the separator.
3) Sort sub expressions according to precedence defined for communative
operations.
4) Simplify CONST_INT_P sub expressions.
5) Create new canonicalized address and return.
According to review comments, this interface is now restricted in LCIM, and
will probably be expanded to other passes like fwprop and combine after
entering GCC7.
Bootstrap and test on x86_64 and AArch64. Is it OK?
Thanks,
bin
2016-02-15 Bin Cheng
PR tree-optimization/69052
* loop-invariant.c (canonicalize_address_mult): New function.
(MAX_CANON_ADDR_PARTS): New macro.
(collect_address_parts): New function.
(compare_address_parts, canonicalize_address): New functions.
(inv_can_prop_to_addr_use): Check validity of address expression
which is canonicalized by above canonicalize_address.
gcc/testsuite/ChangeLog
2016-02-15 Bin Cheng
PR tree-optimization/69052
* gcc.target/i386/pr69052.c: New test.
>
> Jeff
diff --git a/gcc/loop-invariant.c b/gcc/loop-invariant.c
index 707f044..af528d0 100644
--- a/gcc/loop-invariant.c
+++ b/gcc/loop-invariant.c
@@ -754,6 +754,147 @@ create_new_invariant (struct def *def, rtx_insn *insn,
bitmap depends_on,
return inv;
}
+/* Return a canonical version of X for the address, from the point of view,
+ that all multiplications are represented as MULT instead of the multiply
+ by a power of 2 being represented as ASHIFT.
+
+ This function is a duplication of canonicalize_address from fwprop.c.
+ Callers should prepare a copy of X because this function may modify it
+ in place. */
+
+static void
+canonicalize_address_mult (rtx x)
+{
+ for (;;)
+switch (GET_CODE (x))
+ {
+ case ASHIFT:
+ if (CONST_INT_P (XEXP (x, 1))
+ && INTVAL (XEXP (x, 1)) < GET_MODE_BITSIZE (GET_MODE (x))
+ && INTVAL (XEXP (x, 1)) >= 0)
+ {
+ HOST_WIDE_INT shift = INTVAL (XEXP (x, 1));
+ PUT_CODE (x, MULT);
+ XEXP (x, 1) = gen_int_mode ((HOST_WIDE_INT) 1 << shift,
+ GET_MODE (x));
+ }
+
+ x = XEXP (x, 0);
+ break;
+
+ case PLUS:
+ if (GET_CODE (XEXP (x, 0)) == PLUS
+ || GET_CODE (XEXP (x, 0)) == ASHIFT
+ || GET_CODE (XEXP (x, 0)) == CONST)
+ canonicalize_address_mult (XEXP (x, 0));
+
+ x = XEXP (x, 1);
+ break;
+
+ case CONST:
+ x = XEXP (x, 0);
+ break;
+
+ default:
+ return;
+ }
+}
+
+/* Maximum number of sub expressions in address. We set it to
+ a small integer since it's unlikely to have a complicated
+ address expression. */
+
+#define MAX_CANON_ADDR_PARTS (5)
+
+/* Collect sub expressions in address X with PLUS as the seperator.
+ Sub expressions are stored in vector ADDR_PARTS. */
+
+static void
+collect_address_parts (rtx x, vec *addr_parts)
+{
+ for (;;)
+if (GET_CODE (x) == PLUS)
+ {
+ collect_address_parts (XEXP (x, 0), addr_parts);
+ x = XEXP (x, 1);
+ }
+else
+ {
+ addr_parts->safe_push (x);
+ break;
+ }
+}
+
+/* Compare function for sorting sub expressions X and Y based on
+ precedence defined for communitive operations. */
+
+static int
+compare_address_parts (const void *x, const void *y)
+{
+ const rtx *rx = (const rtx *)x;
+ const rtx *ry = (const rtx *)y;
+ int px = commutative_operand_precedence (*rx);
+ int py = commutative_operand_precedence (*ry);
+
+ return (py - px);
+}
+
+/* Return a canonical version address for X by following steps:
+ 1) Rewrite ASHIFT into MULT recursively.
+ 2) Divide address into sub expressions with PLUS as the
+ separator.
+ 3) Sort sub expressions according to precedence defined
+ for communative operations.
+ 4) Simplify CONST_INT_P sub expressions.
+ 5) Create new canonicalized address and return.
+ Callers should prepare a copy of X because this function may
+ modify it in place. */
+
+static rtx
Re: [PATCH] 69759 - document __builtin_alloca and __builtin_alloca_with_align
Presumably the C++ liaison people considered compatibility as part of the WG14 discussions. In any case, I see no sign that (beyond the "fundamental type" terminology issue) the wording in C++ is any better thought out than the pre-DR#445 C wording. The C++ wording matches the pre-DR 445 C wording and says that max_align_t is the most strictly aligned type supported. DR 445 changes max_align_t to be the most strictly aligned basic type. This change has potential ABI compatibility implications (in C now, in C++ in the future). The C change isn't a problem for C++ [yet] because there "the most strictly aligned type in all contexts (storage durations)" is constrained by C++ allocated storage which, unlike in C, is limited to malloc() and operator new (tere's no aligned_alloc() in C++ yet). But unless the next C++ adopts an analogous wording change as C, since the next revision of C+++ will very likely adopt aligned_alloc(), C++ implementations will be able to (even encouraged to) define max_align_t to reflect a more restrictive alignment than DR 445 allows C. Implementations that do will be incompatible with C. C90 and C99 compatibility requires malloc to return memory suitably aligned for all types definable within C90 and C99, and it would be very confusing to say in C11 that certain header typedefs are within such a rule but others (such as max_align_t) aren't because they are new in C11 (a rule saying that all standard header types can be stored in memory returned by malloc is much better for the C11 user). I disagree. I don't think there's any risk of confusion, and I also don't think that having max_align_t reflect the alignment of the most strictly aligned basic type (which is easy to obtain via _Alignof) without providing a way to determine the greatest supported alignment is helpful. But this is a discussion that would be best continued on the WG14 list. Here I'd like to get my updated patch reviewed so that I can move on to my other GCC 6 tasks. Thanks Martin
Re: C++ PATCH for c++/69657 (abs not inlined)
On 02/10/2016 10:31 AM, Jason Merrill wrote: On 02/09/2016 03:29 PM, Rainer Orth wrote: This patch broke Solaris bootstrap (seen on i386-pc-solaris2.12): Fixed by pruning hidden names from the lookup result in more places. ...and this broke some dubious code in LLVM. Fixed thus. Tested x86_64-pc-linux-gnu, applying to trunk.
Re: C++ PATCH for c++/69657 (abs not inlined)
On Tue, Feb 16, 2016 at 02:00:45PM -0500, Jason Merrill wrote: > On 02/10/2016 10:31 AM, Jason Merrill wrote: > >On 02/09/2016 03:29 PM, Rainer Orth wrote: > >>This patch broke Solaris bootstrap (seen on i386-pc-solaris2.12): > > > >Fixed by pruning hidden names from the lookup result in more places. > > ...and this broke some dubious code in LLVM. Fixed thus. > > Tested x86_64-pc-linux-gnu, applying to trunk. ENOPATCH. Jakub
[PATCH, rs6000] Fix pasto resulting in wrong instruction from builtins for lvxl
Hi, During the little-endian vector modification work in 2014, I accidentally introduced an error that Uli Weigand noticed this week. This results in wrong code being generated for the __builtin_altivec_lvxl and vec_lvxl interfaces; an "lvx" instruction is generated instead of an "lvxl" instruction. Now, this is only a performance issue, since the error just means a cache hint is not being generated. However, it needs to be corrected, as below. This brings up a point that, though we have many test cases for the altivec built-ins, they only test that well-formed built-ins are accepted. None of them test the actual code generation. I don't intend to fix that here, but at some point we should do better with this. We should definitely do better with future built-ins that we add. Bootstrapped and tested on powerpc64le-unknown-linux-gnu with no regressions. I hand-checked some of the existing test cases that invoke __builtin_altivec_lvxl and vec_lvxl to verify correct code gen. Is this okay for trunk? I would also like to backport this to GCC 5 and 4.9 if that's acceptable. Thanks, Bill 2016-02-16 Bill Schmidt * config/rs6000/altivec.md (*altivec_lvxl__internal): Output correct instruction. Index: gcc/config/rs6000/altivec.md === --- gcc/config/rs6000/altivec.md(revision 233466) +++ gcc/config/rs6000/altivec.md(working copy) @@ -2511,7 +2511,7 @@ (match_operand:VM2 1 "memory_operand" "Z")) (unspec [(const_int 0)] UNSPEC_SET_VSCR)])] "TARGET_ALTIVEC" - "lvx %0,%y1" + "lvxl %0,%y1" [(set_attr "type" "vecload")]) (define_expand "altivec_lvx_"
Re: [6 Regession] Usage of unitialized pointer io/list_read.c (
With the following reduced test program test implicit none integer :: i, j, k, ios integer, parameter :: big = 600 character(kind=4,len=big) :: str1, str2 do i=1,big, 10 do j = 0, 9 k = i + j str2(k:k) = char(65+j) end do end do open(15, status='scratch', encoding="utf-8") write(15,*) " 'abcdefgh!'", " ", str2 rewind(15) read(15,*,iostat=ios) str1, str2 close(15) end program valgrind shows a lot of errors before and after the commit. Dominique > Le 16 févr. 2016 à 18:58, Jerry DeLisle a écrit : > > Up until now we have not had a test case like this for kind=4 testing. I did > try -fsanitize=address here and do see the same thing. I will work on this > some > more. > > Thanks, > > Jerry
Re: [PATCH] Move the -Wnonnull compare against NULL warning from FEs to early uninit under -Wnonnull-compare (PR c/69835)
On Tue, Feb 16, 2016 at 07:21:49PM +0100, Jakub Jelinek wrote: > I'm already bootstrapping/regtesting following variant. > > 2016-02-16 Jakub Jelinek > > PR c/69835 > * common.opt (Wnonnull-compare): New warning. > * doc/invoke.texi (-Wnonnull): Remove text about comparison > of arguments against NULL. > (-Wnonnull-compare): Document. > * Makefile.in (OBJS): Add gimple-ssa-nonnull-compare.o. > * tree-pass.h (make_pass_warn_nonnull_compare): Declare. > * passes.def (pass_warn_nonnull_compare): Add. > * gimple-ssa-nonnull-compare.c: New file. > c-family/ > * c.opt (Wnonnull-compare): Enable for -Wall. > c/ > * c-typeck.c (build_binary_op): Revert 2015-09-09 change. > cp/ > * typeck.c (cp_build_binary_op): Revert 2015-09-09 change. > testsuite/ > * c-c++-common/nonnull-1.c: Use -Wnonnull-compare instead of > -Wnonnull in dg-options. > * c-c++-common/nonnull-2.c: New test. Now successfully bootstrapped/regtested on both x86_64-linux and i686-linux. Ok for trunk? Jakub
Re: Wonly-top-basic-asm
On 16.02.2016 15:03, Jan Hubicka wrote:
>> @example
>> -/* Note that this code will not compile with -masm=intel */
>> -#define DebugBreak() asm("int $3")
>> +/* Define macro at file scope with basic asm. */
>> +/* Add macro parameter p to eax. */
>> +asm(".macro test p\n\t"
>> +"addl $\\p, %eax\n\t"
>> +".endm");
>> +
>> +/* Use macro in function using extended asm. It needs */
>> +/* the "cc" clobber since the flags are changed and uses */
>> +/* the "a" constraint since it modifies eax. */
>> +int DoAdd(int value)
>> +@{
>> + asm("test 5" : "+a" (value) : : "cc");
>> + return value;
>> +@}
>
> To make this work you need the .macro appear before all uses in the asm
> files. I do not think
> we really promise that wihtout -fno-toplevel-reorder -fno-lto
>
> Honza
>
And, isn't "test" an assembler instruction name?
What if the compiler emits something like test %eax,1
somewhere in the same translation unit?
Bernd.
Re: [PATCH] Move the -Wnonnull compare against NULL warning from FEs to early uninit under -Wnonnull-compare (PR c/69835)
On 02/16/2016 01:01 PM, Jakub Jelinek wrote: On Tue, Feb 16, 2016 at 07:21:49PM +0100, Jakub Jelinek wrote: I'm already bootstrapping/regtesting following variant. 2016-02-16 Jakub Jelinek PR c/69835 * common.opt (Wnonnull-compare): New warning. * doc/invoke.texi (-Wnonnull): Remove text about comparison of arguments against NULL. (-Wnonnull-compare): Document. * Makefile.in (OBJS): Add gimple-ssa-nonnull-compare.o. * tree-pass.h (make_pass_warn_nonnull_compare): Declare. * passes.def (pass_warn_nonnull_compare): Add. * gimple-ssa-nonnull-compare.c: New file. c-family/ * c.opt (Wnonnull-compare): Enable for -Wall. c/ * c-typeck.c (build_binary_op): Revert 2015-09-09 change. cp/ * typeck.c (cp_build_binary_op): Revert 2015-09-09 change. testsuite/ * c-c++-common/nonnull-1.c: Use -Wnonnull-compare instead of -Wnonnull in dg-options. * c-c++-common/nonnull-2.c: New test. Now successfully bootstrapped/regtested on both x86_64-linux and i686-linux. Ok for trunk? Yes. Sorry I wasn't explicit. jeff
Re: [AArch64] Remove AARCH64_EXTRA_TUNE_RECIP_SQRT from Cortex-A57 tuning
On 02/16/16 04:28, James Greenhalgh wrote: On Mon, Feb 15, 2016 at 11:24:53AM -0600, Evandro Menezes wrote: James, There seem to be SPEC CPU2000fp validation issues on A57 when this flag is present too. Though I evaluated the algorithm with a huge random set of values, always delivering accuracy around 1ulp, which should be enough for CPU2000fp (wit x86-64), I expected the benchmarks to pass. My suspicion is that the Newton series on AArch64 is probably good only for SP. Then, DP might require an extra round, probably exacerbating the performance penalty. I'd like to try to split this tuning option into one for SP and another for DP. Thoughts? I haven't seen validation issues with the default expansion, but with -mlow-precision-recip-sqrt I do see failures. I think this is to be expected. I don't support splitting the low-precision flag to "-mlow-precision-float-recip-sqrt" and "-mlow-precision-double-recip-sqrt", I think that is pushing a particular set of Spec tuning flags over any meaningful use case. I could imagine a case for splitting the internal tuning flag to give AARCH64_EXTRA_TUNE_SF_RECIP_SQRT and AARCH64_EXTRA_TUNE_DF_RECIP_SQRT, but I'm not sure I understand the benefits of this? Certainly, I think your goals for performance (turn on for 64-bit divide/sqrt) would contradict your goals for accuracy (turn off for 64-bit divide/sqrt). I'm happy with these flags as they are, but I might be missing a subtelty in your argument? James, I'm still in sorting out the data, but, indeed, I see no validation issues with the approximate reciprocal square root in CPU2000. However, working on a patch to use the Newton series for square root based on the approximate reciprocal square root (d^1/2 = d * d^-1/2), I stumbled at validation errors. I'll take the discussion to that thread. Stand by... -- Evandro Menezes
Re: [PATCH, rs6000] Fix pasto resulting in wrong instruction from builtins for lvxl
On Tue, 2016-02-16 at 11:40 -0800, David Edelsohn wrote: > This is okay, but how about starting with a testcase for this? That's fine. I'll make it generic enough that we can add to it later, then. Bill > > Thanks David > > On Feb 16, 2016 11:37 AM, "Bill Schmidt" > wrote: > Hi, > > During the little-endian vector modification work in 2014, I > accidentally introduced an error that Uli Weigand noticed this > week. > This results in wrong code being generated for the > __builtin_altivec_lvxl and vec_lvxl interfaces; an "lvx" > instruction is > generated instead of an "lvxl" instruction. Now, this is only > a > performance issue, since the error just means a cache hint is > not being > generated. However, it needs to be corrected, as below. > > This brings up a point that, though we have many test cases > for the > altivec built-ins, they only test that well-formed built-ins > are > accepted. None of them test the actual code generation. I > don't intend > to fix that here, but at some point we should do better with > this. We > should definitely do better with future built-ins that we add. > > Bootstrapped and tested on powerpc64le-unknown-linux-gnu with > no > regressions. I hand-checked some of the existing test cases > that invoke > __builtin_altivec_lvxl and vec_lvxl to verify correct code > gen. Is this > okay for trunk? I would also like to backport this to GCC 5 > and 4.9 if > that's acceptable. > > Thanks, > Bill > > > 2016-02-16 Bill Schmidt > > * config/rs6000/altivec.md > (*altivec_lvxl__internal): Output > correct instruction. > > > Index: gcc/config/rs6000/altivec.md > === > --- gcc/config/rs6000/altivec.md(revision 233466) > +++ gcc/config/rs6000/altivec.md(working copy) > @@ -2511,7 +2511,7 @@ > (match_operand:VM2 1 "memory_operand" "Z")) > (unspec [(const_int 0)] UNSPEC_SET_VSCR)])] >"TARGET_ALTIVEC" > - "lvx %0,%y1" > + "lvxl %0,%y1" >[(set_attr "type" "vecload")]) > > (define_expand "altivec_lvx_" > >
Re: [PING][PATCH, PR67709 ] Don't call call_cgraph_insertion_hooks in simd_clone_create
On 16/02/16 17:54, Jakub Jelinek wrote: On Tue, Feb 16, 2016 at 05:52:55PM +0100, Tom de Vries wrote: Committed both patches to 4.9 and 5 branches. In order to run testsuite/libgomp.fortran/declare-simd-4.f90 with the 4.9 branch build, I needed in addition: - r212268 https://gcc.gnu.org/viewcvs/gcc?view=revision&revision=212268 https://gcc.gnu.org/ml/gcc-patches/2014-07/msg00218.html - r219606 https://gcc.gnu.org/viewcvs/gcc?view=revision&revision=219606 https://gcc.gnu.org/ml/gcc-patches/2015-01/msg00615.html Should these also be back-ported to 4.9 branch (and in case of r219606, also follow-up fix r219722, https://gcc.gnu.org/viewcvs?rev=219722&root=gcc&view=rev , fix for https://gcc.gnu.org/bugzilla/show_bug.cgi?id=64605 )? IMHO just don't backport that test, test it by hand only. I've already committed the testcase, and without these two patches the test shows up as unsupported, so I'll leave it at that. As for the lto bit (r219606) I can see your point, it's the only libgomp test that uses lto. But there are 16 test cases which are currently unsupported in the 4.9 branch, and would be supported with the fortran patch (r212268): ... libgomp.fortran/allocatable11.f90 libgomp.fortran/allocatable2.f90 libgomp.fortran/allocatable8.f90 libgomp.fortran/appendix-a/a.22.7.f90 libgomp.fortran/appendix-a/a.22.8.f90 libgomp.fortran/crayptr2.f90 libgomp.fortran/declare-simd-1.f90 libgomp.fortran/declare-simd-2.f90 libgomp.fortran/omp_parse3.f90 libgomp.fortran/pointer2.f90 libgomp.fortran/pr25162.f libgomp.fortran/pr32550.f90 libgomp.fortran/threadprivate1.f90 libgomp.fortran/threadprivate2.f90 libgomp.fortran/threadprivate3.f90 libgomp.fortran/threadprivate4.f90 ... Perhaps that makes it worthwhile to back-port the patch. Thanks, - Tom
Re: [AArch64] Emit square root using the Newton series
On 12/08/15 15:35, Evandro Menezes wrote: Emit square root using the Newton series 2015-12-03 Evandro Menezes gcc/ * config/aarch64/aarch64-protos.h (aarch64_emit_swsqrt): Declare new function. * config/aarch64/aarch64-simd.md (sqrt2): New expansion and insn definitions. * config/aarch64/aarch64-tuning-flags.def (AARCH64_EXTRA_TUNE_FAST_SQRT): New tuning macro. * config/aarch64/aarch64.c (aarch64_emit_swsqrt): Define new function. * config/aarch64/aarch64.md (sqrt2): New expansion and insn definitions. * config/aarch64/aarch64.opt (mlow-precision-recip-sqrt): Expand option description. * doc/invoke.texi (mlow-precision-recip-sqrt): Likewise. This patch extends the patch that added support for implementing x^-1/2 using the Newton series by adding support for x^1/2 as well. Is it OK at this point of stage 3? Thank you, James, As I was saying, this patch results in some validation errors in CPU2000 benchmarks using DF. Although proving the algorithm to be pretty solid with a vast set of random values, I'm confused why some benchmarks fail to validate with this implementation of the Newton series for square root too, when they pass with the Newton series for reciprocal square root. Since I had no problems with the same algorithm on x86-64, I wonder if the initial estimate on AArch64, which offers just 8 bits, whereas x86-64 offers 11 bits, has to do with it. Then again, the algorithm iterated 1 less time on x86-64 than on AArch64. Since it seems that the initial estimate is sufficient for CPU2000 to validate when using SF, I'm leaning towards restricting the Newton series for square root only for SF. Your thoughts on the matter are appreciated, -- Evandro Menezes
[patch, Fortran, committed] Fix PR 69742, ICE in assoc lists
Hello world,
the attached patch fixes PR 69742 (a regression) by simply not
attempting to do function elimination in an assoc list.
Committed as obvious and simple to trunk, the other affected
branches will follow shortly.
Regards
Thomas
2015-02-16 Thomas Koenig
PR fortran/69742
* frontend-passes.c (cfe-expr_0): Don't register functions
from within an ASSOCIATE statement.
2015-02-16 Thomas Koenig
PR fortran/69742
* gfortran.dg/associate_21.f90: New test.
@@ -734,9 +733,9 @@ cfe_expr_0 (gfc_expr **e, int *walk_subtrees,
gfc_expr *newvar;
gfc_expr **ei, **ej;
- /* Don't do this optimization within OMP workshare. */
+ /* Don't do this optimization within OMP workshare or ASSOC lists. */
- if (in_omp_workshare)
+ if (in_omp_workshare || in_assoc_list)
{
*walk_subtrees = 0;
return 0;
! { dg-do compile }
! { dg-options "-ffrontend-optimize" }
! PR 69742 - this used to ICE with front-end optimizatoin
! Original test case by Marco Restelli.
program p
implicit none
integer, allocatable :: i(:), j
allocate( i(5) )
i = (/( j , j=1,5 )/)
! The ICE appears when "size(i)" is used twice in associate
associate( i5 => i(size(i):size(i)) ) ! this gives ICE
!associate( i5 => i(size(2*i):size(i)) ) ! this works
i5 = 2
end associate
write(*,*) i
end program p
Re: lra-remat issues (PR68730)
On 02/15/2016 02:13 PM, Bernd Schmidt wrote: On 02/04/2016 09:27 PM, Vladimir Makarov wrote: After a few false starts, I came up with the patch below, which keeps track of not just the candidate insn, but also an activation insn, and chooses candidates only if they are both available and active. Besides passing a new arg to create_cand, the changes in create_cands are mostly cosmetic to make the function less confusing. This was bootstrapped and tested on x86_64-linux. Ok? The patch looks ok for me. Thanks for working on the PR, Bernd. I should get in the habit of asking "ok everywhere?" Can I put this on gcc-5-branch as well? ... and I managed to confuse myself about which LRA issue I had approval for on gcc-5-branch, and checked it in. Sorry. If I don't hear back until tomorrow I'll revert it. Bernd
Re: [RFC] [P2] [PR tree-optimization/33562] Lowering more complex assignments.
On 02/14/2016 11:38 AM, Richard Biener wrote: On February 14, 2016 5:35:13 PM GMT+01:00, Jeff Law wrote: On 02/12/2016 10:21 AM, Jeff Law wrote: But really we simply need a better DSE algorithm. So the way to fix DSE is to keep the existing algorithm and track the hunks of Complex/aggregates that have been set a second time. My first thought was to implement this as an inverted bitmap. ie, set it to 1 for every byte in the complex/aggregate that is set by the first store. Clear bits for each byte in subsequent stores to the pieces. If the bitmap reaches an empty state, then the initial store is dead. Adjusting *ref could work too, but would probably be painful if the subsequent stores don't happen in a convenient order. So that was scary easy. We should have done this a long time ago. Essentially I call ao_get_ref_base to get the offset/size/max_size filled in for the first statement. Those are used to initialize the live bytes bitfield, as long as max_size != -1. Then when we have a possible killing statement, we use the ao in a similar manner to determine which bytes to clear (taking care that the base is the same between the two references and that in the killing statement that the size/max_size are the same.). When all the live bytes are zero then we've killed the original statement. It's ~20 lines of code. I need to pull together some additional tests, but it looks likely we'll be able to wrap this up easily for gcc-6. BTW, we had sth like this before but it had both correctness and more importantly scalability issues. If you're referring to 48141, that was a completely different issue and I don't think there's any significant parallels between how that affected RTL DSE and tree DSE. The issue in 48141 is that RTL DSE keeps a list of active stores. Those lists were getting long, often with things that just weren't interesting. Tree DSE doesn't maintain a list of that nature (it used to, but you fixed all that a few years back). These days it does a walk of the IL. When it finds a memory store, it walks immediate uses of the virtual operands to see if an immediate use happens to write the same memory location. If it does, then the first write is dead. That's (of course) over-simplified, but captures the essence of how tree DSE works. As long as the calls to ao_ref_init/ao_ref are cheap, my code shouldn't change the overall performance of tree DSE. Jeff
Re: [PATCH, rs6000] Fix pasto resulting in wrong instruction from builtins for lvxl
On Tue, 2016-02-16 at 11:40 -0800, David Edelsohn wrote:
> This is okay, but how about starting with a testcase for this?
Fair enough. Here's the revised patch with a test, which I've verified
on powerpc64-unknown-linux-gnu. Ok to proceed?
Thanks!
Bill
[gcc]
2016-02-16 Bill Schmidt
* config/rs6000/altivec.md (*altivec_lvxl__internal): Output
correct instruction.
[gcc/testsuite]
2012-02-16 Bill Schmidt
* gcc.target/powerpc/vec-cg.c: New test.
Index: gcc/config/rs6000/altivec.md
===
--- gcc/config/rs6000/altivec.md(revision 233466)
+++ gcc/config/rs6000/altivec.md(working copy)
@@ -2511,7 +2511,7 @@
(match_operand:VM2 1 "memory_operand" "Z"))
(unspec [(const_int 0)] UNSPEC_SET_VSCR)])]
"TARGET_ALTIVEC"
- "lvx %0,%y1"
+ "lvxl %0,%y1"
[(set_attr "type" "vecload")])
(define_expand "altivec_lvx_"
Index: gcc/testsuite/gcc.target/powerpc/vec-cg.c
===
--- gcc/testsuite/gcc.target/powerpc/vec-cg.c (revision 0)
+++ gcc/testsuite/gcc.target/powerpc/vec-cg.c (working copy)
@@ -0,0 +1,22 @@
+/* Test code generation of vector built-ins. We don't have this for
+ most of ours today. As new built-ins are added, please add to this
+ test case. Update as necessary to add VSX, P8-vector, P9-vector,
+ etc. */
+
+/* { dg-do compile { target powerpc*-*-* } } */
+/* { dg-require-effective-target powerpc_altivec_ok } */
+/* { dg-options "-maltivec -O0" } */
+
+#include
+
+static vector signed int i, *pi;
+static int int1;
+
+void
+b()
+{
+ i = __builtin_altivec_lvxl (int1, pi);
+ i = vec_lvxl (int1, pi);
+}
+
+/* { dg-final { scan-assembler-times "lvxl" 2 } } */
Re: [PATCH] 69759 - document __builtin_alloca and __builtin_alloca_with_align
On Tue, 16 Feb 2016, Martin Sebor wrote: > > Presumably the C++ liaison people considered compatibility as part of the > > WG14 discussions. In any case, I see no sign that (beyond the > > "fundamental type" terminology issue) the wording in C++ is any better > > thought out than the pre-DR#445 C wording. > > The C++ wording matches the pre-DR 445 C wording and says that The ill-thought-out wording. Exactly. > max_align_t is the most strictly aligned type supported. DR 445 And, in particular, max_align_t (both pre-DR#445 and post-DR#445) cannot be more strictly aligned than malloc guarantees to return (because malloc returns memory "suitably aligned so that it may be assigned to a pointer to any type of object with a fundamental alignment requirement and then used to access such an object or an array of such objects in the space allocated", and the alignment of max_align_t is unambiguously a fundamental alignment both before and after the DR, by virtue of how fundamental alignments are defined). > changes max_align_t to be the most strictly aligned basic type. No. The most strictly aligned of an infinite set of types including those derived in certain ways from basic types and header typedefs. > I disagree. I don't think there's any risk of confusion, and > I also don't think that having max_align_t reflect the alignment > of the most strictly aligned basic type (which is easy to obtain > via _Alignof) without providing a way to determine the greatest Because max_align_t reflects an infinite set of derived types, based on both basic types and header types, it's not possible to determine the maximum alignment requirement of those using _Alignof. -- Joseph S. Myers jos...@codesourcery.com
Re: lra-remat issues (PR68730)
On 02/16/2016 04:20 PM, Bernd Schmidt wrote: On 02/15/2016 02:13 PM, Bernd Schmidt wrote: On 02/04/2016 09:27 PM, Vladimir Makarov wrote: After a few false starts, I came up with the patch below, which keeps track of not just the candidate insn, but also an activation insn, and chooses candidates only if they are both available and active. Besides passing a new arg to create_cand, the changes in create_cands are mostly cosmetic to make the function less confusing. This was bootstrapped and tested on x86_64-linux. Ok? The patch looks ok for me. Thanks for working on the PR, Bernd. I should get in the habit of asking "ok everywhere?" Can I put this on gcc-5-branch as well? ... and I managed to confuse myself about which LRA issue I had approval for on gcc-5-branch, and checked it in. Sorry. If I don't hear back until tomorrow I'll revert it. Sorry, Bernd. I should be more clear too. The patch is ok for any GCC version with lra-remat.c (it includes gcc-5 and coming gcc-6).
Re: lra-remat issues (PR68730)
... and I managed to confuse myself about which LRA issue I had approval for on gcc-5-branch, and checked it in. Sorry. If I don't hear back until tomorrow I'll revert it. Sorry, Bernd. I should be more clear too. The patch is ok for any GCC version with lra-remat.c (it includes gcc-5 and coming gcc-6). Thanks, Vlad. I'll leave it in then :) Bernd
Re: [RFC] [PATCH] Add __array_size keyword
On Tue, 16 Feb 2016, Stuart Brady wrote: > > For whether arguments are evaluated, you need __array_size with arguments > > that have side effects, and then test whether those side effects occurred. > > For whether results are integer constant expressions, you can test e.g. > > whether __array_size (a) - __array_size (a) is accepted in a context > > requiring a pointer (whether it acts as a valid null pointer constant). > > With my patch, void *p = __array_size(a) - __array_size(a) sets p to be > null. That behaviour seems desirable to me. It sets it to be null - but does it diagnose conversion from integer to pointer without a cast (it should do so if __array_size is not evaluating to an integer constant expression, but not if it is evaluating to an integer constant expression - hence this being one way to test whether the result is an integer constant expression)? -- Joseph S. Myers jos...@codesourcery.com
Re: [RFC] [PATCH] Add __array_size keyword
On Tue, Feb 16, 2016 at 11:10:08PM +, Joseph Myers wrote: > It sets it to be null - but does it diagnose conversion from integer to > pointer without a cast (it should do so if __array_size is not evaluating > to an integer constant expression, but not if it is evaluating to an > integer constant expression - hence this being one way to test whether > the result is an integer constant expression)? With -Wall -Wextra, I do not see any such warning. IIUC, the warning would be given by -Wint-conversion, included in -Wall. FWIW, if I use "int a=1; void *p=a-a;" I do get the expected warning, so I think that covers this case. Is this right? If I use __array_size in an enumeration, this works fine, rather than giving me "error: enumerator value for ‘foo’ is not an integer constant". I am not sure if this is not also a valid test. -- Many thanks, Stuart Brady
Re: [RFC] [PATCH] Add __array_size keyword
On Tue, 16 Feb 2016, Stuart Brady wrote: > On Tue, Feb 16, 2016 at 11:10:08PM +, Joseph Myers wrote: > > It sets it to be null - but does it diagnose conversion from integer to > > pointer without a cast (it should do so if __array_size is not evaluating > > to an integer constant expression, but not if it is evaluating to an > > integer constant expression - hence this being one way to test whether > > the result is an integer constant expression)? > > With -Wall -Wextra, I do not see any such warning. IIUC, the warning > would be given by -Wint-conversion, included in -Wall. FWIW, if I use > "int a=1; void *p=a-a;" I do get the expected warning, so I think that > covers this case. Is this right? What's right is: * In cases where it should return an integer constant (you've said that's when the argument is not a VLA, as for sizeof), there should be no diagnostic. * In cases where it should not return an integer constant, there should be such a diagnostic (enabled by default). The case I raised in my original message was e.g. int a; int b[2][a]; - should __array_size (b) be an integer constant (size_t)2, or should it be non-constant (size_t)2 because the argument is a VLA (albeit a VLA whose top-level dimension is an integer constant expression)? > If I use __array_size in an enumeration, this works fine, rather than > giving me "error: enumerator value for ‘foo’ is not an integer constant". > I am not sure if this is not also a valid test. It's a valid test, *but* in various contexts, such as in an enumeration, you need -pedantic to enable diagnostics for some cases of invalid integer constant expressions (but for the null pointer constant case, you don't). -- Joseph S. Myers jos...@codesourcery.com
Re: [RFC] [PATCH] Add __array_size keyword
On Tue, Feb 16, 2016 at 11:35:24PM +, Joseph Myers wrote: > What's right is: > > * In cases where it should return an integer constant (you've said that's > when the argument is not a VLA, as for sizeof), there should be no > diagnostic. Right. > * In cases where it should not return an integer constant, there should be > such a diagnostic (enabled by default). Good point. Indeed, for a VLA, I do get such a message. > The case I raised in my original message was e.g. > > int a; > int b[2][a]; > > - should __array_size (b) be an integer constant (size_t)2, or should it > be non-constant (size_t)2 because the argument is a VLA (albeit a VLA > whose top-level dimension is an integer constant expression)? Ouch. I would say it should be an integer constant (size_t)2, simply as that seems to me to be a reasonable expectation. Unfortunately, this is not what happens with my patch, as I get a -Wint-conversion warning. :-( Oddly with "int b[a][2];" I find that __array_size(*b) *is* an integer constant expression. This leads me to wonder *why* I am not getting an integer constant expression, and whether this might be a problem for other reasons. I'm concerned GCC may not be able to give warnings pertaining to the outer subscript for VLAs, as these may also be dependent on the same underlying domain max value expression as __array_size. The fact that __array_size(b) would be an integer constant expression in this case, whereas sizeof(b) / sizeof(*b) would not seems to be an advantage to this keyword that I had not originally considered. Personally, I could tolerate this not returning an integer constant expression, but I would regard that as a bug. As extra confusion would be introduced when fixing this, I would rather just get it right. > On Tue, 16 Feb 2016, Stuart Brady wrote: > > If I use __array_size in an enumeration, this works fine, rather than > > giving me "error: enumerator value for ‘foo’ is not an integer constant". > > I am not sure if this is not also a valid test. > > It's a valid test, *but* in various contexts, such as in an enumeration, > you need -pedantic to enable diagnostics for some cases of invalid integer > constant expressions (but for the null pointer constant case, you don't). Thanks, that's helpful to know. -- Kind regards, Stuart Brady
[PATCH] xtensa: fix libgcc build with --text-section-literals
Functions __muldf3_aux, __divdf3_aux, __mulsf3_aux and __divsf3_aux don't start with leaf_entry, so they need explicit .literal_position, otherwise libgcc build fails in the presence of --text-section-literals. 2016-02-17 Max Filippov libgcc/ * config/xtensa/ieee754-df.S (__muldf3_aux, __divdf3_aux): Add .literal_position before the function. * config/xtensa/ieee754-sf.S (__mulsf3_aux, __divsf3_aux): Likewise. --- libgcc/config/xtensa/ieee754-df.S | 2 ++ libgcc/config/xtensa/ieee754-sf.S | 2 ++ 2 files changed, 4 insertions(+) diff --git a/libgcc/config/xtensa/ieee754-df.S b/libgcc/config/xtensa/ieee754-df.S index 743dbf2..1d9ef46 100644 --- a/libgcc/config/xtensa/ieee754-df.S +++ b/libgcc/config/xtensa/ieee754-df.S @@ -606,6 +606,7 @@ __subdf3: #define XCHAL_NO_MUL 1 #endif + .literal_position __muldf3_aux: /* Handle unusual cases (zeros, subnormals, NaNs and Infinities). @@ -1216,6 +1217,7 @@ __muldf3: #ifdef L_divdf3 + .literal_position /* Division */ __divdf3_aux: diff --git a/libgcc/config/xtensa/ieee754-sf.S b/libgcc/config/xtensa/ieee754-sf.S index 1843749..7864a74 100644 --- a/libgcc/config/xtensa/ieee754-sf.S +++ b/libgcc/config/xtensa/ieee754-sf.S @@ -487,6 +487,7 @@ __subsf3: #define XCHAL_NO_MUL 1 #endif + .literal_position __mulsf3_aux: /* Handle unusual cases (zeros, subnormals, NaNs and Infinities). @@ -884,6 +885,7 @@ __mulsf3: #ifdef L_divsf3 + .literal_position /* Division */ __divsf3_aux: -- 2.1.4
New C++ PATCH for c++/10200 et al
Clearly the DR 141 change is requiring much larger adjustments in the
rest of the compiler than I'm comfortable making at this point in the
GCC 6 schedule, so I'm backing out my earlier changes for 10200 and
69753 and replacing them with a more modest fix for 10200: Now we will
still find member function templates by unqualified lookup, we just
won't find namespace-scope function templates. The earlier approach
will return in GCC 7 stage 1.
Tested x86_64-pc-linux-gnu, applying to trunk.
commit 862c6a750fc12cb41d29aa8f91c013f7705836c1
Author: Jason Merrill
Date: Tue Feb 16 19:11:35 2016 -0500
PR c++/10200
PR c++/69753
* call.c, cp-tree.h, name-lookup.c, pt.c, search.c, semantics.c,
tree.c, typeck2.c: Revert earlier changes.
* parser.c (cp_parser_lookup_name): Ignore namespace-scope
non-type templates after -> or .
diff --git a/gcc/cp/call.c b/gcc/cp/call.c
index db40654..cb71176 100644
--- a/gcc/cp/call.c
+++ b/gcc/cp/call.c
@@ -8160,7 +8160,7 @@ build_new_method_call_1 (tree instance, tree fns, vec **args,
if (permerror (input_location,
"cannot call constructor %<%T::%D%> directly",
- BINFO_TYPE (access_binfo), name))
+ basetype, name))
inform (input_location, "for a function-style cast, remove the "
"redundant %<::%D%>", name);
call = build_functional_cast (basetype, build_tree_list_vec (user_args),
@@ -8377,9 +8377,6 @@ build_new_method_call_1 (tree instance, tree fns, vec **args,
we know we really need it. */
cand->first_arg = instance;
}
- else if (any_dependent_bases_p ())
- /* We can't tell until instantiation time whether we can use
- *this as the implicit object argument. */;
else
{
if (complain & tf_error)
diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index b7d7bc6..3b91089 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -6252,7 +6252,6 @@ extern tree adjust_result_of_qualified_name_lookup
extern tree copied_binfo (tree, tree);
extern tree original_binfo (tree, tree);
extern int shared_member_p (tree);
-extern bool any_dependent_bases_p (tree = current_nonlambda_class_type ());
/* The representation of a deferred access check. */
@@ -6543,6 +6542,7 @@ extern tree get_first_fn (tree);
extern tree ovl_cons(tree, tree);
extern tree build_overload (tree, tree);
extern tree ovl_scope(tree);
+extern bool non_static_member_function_p(tree);
extern const char *cxx_printable_name (tree, int);
extern const char *cxx_printable_name_translate (tree, int);
extern tree build_exception_variant (tree, tree);
diff --git a/gcc/cp/name-lookup.c b/gcc/cp/name-lookup.c
index b73f3f7..89d84d7 100644
--- a/gcc/cp/name-lookup.c
+++ b/gcc/cp/name-lookup.c
@@ -,6 +,8 @@ do_class_using_decl (tree scope, tree name)
/* True if any of the bases of CURRENT_CLASS_TYPE are dependent. */
bool bases_dependent_p;
tree binfo;
+ tree base_binfo;
+ int i;
if (name == error_mark_node)
return NULL_TREE;
@@ -3369,7 +3371,16 @@ do_class_using_decl (tree scope, tree name)
|| (IDENTIFIER_TYPENAME_P (name)
&& dependent_type_p (TREE_TYPE (name;
- bases_dependent_p = any_dependent_bases_p (current_class_type);
+ bases_dependent_p = false;
+ if (processing_template_decl)
+for (binfo = TYPE_BINFO (current_class_type), i = 0;
+ BINFO_BASE_ITERATE (binfo, i, base_binfo);
+ i++)
+ if (dependent_type_p (TREE_TYPE (base_binfo)))
+ {
+ bases_dependent_p = true;
+ break;
+ }
decl = NULL_TREE;
diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index b8d8237..18f3902 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -7184,16 +7184,8 @@ cp_parser_postfix_dot_deref_expression (cp_parser *parser,
if (token_type == CPP_DEREF)
postfix_expression = build_x_arrow (location, postfix_expression,
tf_warning_or_error);
- /* According to the standard, no expression should ever have
- reference type. Unfortunately, we do not currently match
- the standard in this respect in that our internal representation
- of an expression may have reference type even when the standard
- says it does not. Therefore, we have to manually obtain the
- underlying type here. */
- scope = non_reference (TREE_TYPE (postfix_expression));
- /* Check to see whether or not the expression is type-dependent and
- not the current instantiation. */
- dependent_p = !scope || dependent_scope_p (scope);
+ /* Check to see whether or not the expression is type-dependent. */
+ dependent_p = type_dependent_expression_p (postfix_expression);
/* The identifier following the `->' or `.' is not qualified. */
parser->scope = NULL_TREE;
parser->qualifying_scope = NULL_TREE;
@@ -7202,8 +7194,16 @@ cp_parser_postfix_dot_deref_expression (cp_parser *parser,
/* Enter the scope corresponding to the type of the object
given by the POSTFIX_EXPRESSION. */
- if (!dependent_p)
+ if (!dependent
Re: [PATCH] 69780 - [4.9/5/6 Regression] ICE on __builtin_alloca_with_align, with small alignment
On 02/15/2016 01:29 AM, Jakub Jelinek wrote:
On Sun, Feb 14, 2016 at 07:16:13PM -0700, Martin Sebor wrote:
+case BUILT_IN_ALLOCA_WITH_ALIGN:
+ {
+ /* Get the requested alignment (in bits) if it's a constant
+ integer expression. */
+ HOST_WIDE_INT align =
+ TREE_CODE (args [1]) == INTEGER_CST ? tree_to_uhwi (args [1]) : 0;
Thank you for your comments. Attached is an updated patch that
corrects most of the problems you pointed out. Also included
in this patch is the update to the manual documenting both this
intrinsic and __builtin_alloca. I originally submitted that
part for review separately but since they are closely related
I think they are best reviewed and ultimately committed together.
(In the spirit of full disclosure, the review of the documentation
change got side-tracked by a discussion of an unrelated topic and
would need to be restarted anyway.)
Formatting. = needs to be on the next line.
There are literally dozens of examples of this style in this file
alone. In one of the two instances of this style in this patch,
moving the equals sign to the next line would force me to split
the initializer expression over the next two lines to avoid
exceeding the 80 character per line limit and make the code
harder to read. I also don't see the style you suggest mentioned
in the GNU coding standard or in the GCC coding conventions.
I would prefer to leave this detail to the discretion of the
author.
Martin
PR middle-end/69780 - [4.9/5/6 Regression] ICE on __builtin_alloca_with_align
with small alignment
PR c/69759 - __builtin_alloca and __builtin_alloca_with_align undocumented
gcc/c-family/ChangeLog:
2016-02-16 Martin Sebor
PR middle-end/69780
* c-common.c (check_builtin_function_arguments): Validate and reject
invalid arguments to __builtin_alloca_with_align.
gcc/ChangeLog:
2016-02-16 Martin Sebor
PR c/69759
* doc/extend.texi (Other Builtins): Document __builtin_alloca and
__builtin_alloca_with_align.
gcc/testsuite/ChangeLog:
2016-02-16 Martin Sebor
PR middle-end/69780
* g++.dg/ext/builtin_alloca.C: New test.
* gcc.dg/builtins-68.c: New test.
Index: gcc/c-family/c-common.c
===
--- gcc/c-family/c-common.c (revision 233476)
+++ gcc/c-family/c-common.c (working copy)
@@ -9816,8 +9816,41 @@ check_builtin_function_arguments (tree f
|| DECL_BUILT_IN_CLASS (fndecl) != BUILT_IN_NORMAL)
return true;
+#undef MAX_STACK_ALIGNMENT
+#define MAX_STACK_ALIGNMENT __UINT32_MAX__
+
switch (DECL_FUNCTION_CODE (fndecl))
{
+case BUILT_IN_ALLOCA_WITH_ALIGN:
+ {
+ /* Get the requested alignment (in bits) if it's a constant
+ integer expression. */
+ unsigned HOST_WIDE_INT align =
+ TREE_CODE (args[1]) == INTEGER_CST ? tree_to_uhwi (args[1]) : 0;
+
+ /* Determine if the requested alignment is a power of 2 greater
+ than CHAR_BIT. */
+ if ((align & (align - 1)) == 0)
+ align >>= LOG2_BITS_PER_UNIT;
+ else
+ align = 0;
+
+ /* Reject invalid alignments. */
+ if (align < 1 || MAX_STACK_ALIGNMENT < align)
+ {
+ /* Compute the maximum stack alignment in bits. */
+ unsigned HOST_WIDE_INT max_align_bits =
+ (unsigned HOST_WIDE_INT)MAX_STACK_ALIGNMENT << LOG2_BITS_PER_UNIT;
+
+ error_at (EXPR_LOC_OR_LOC (args[1], input_location),
+ "second argument to function %qE must be a constant "
+ "integer power of 2 between %qi and %qwu",
+ fndecl, BITS_PER_UNIT, max_align_bits);
+ return false;
+ }
+ return true;
+ }
+
case BUILT_IN_CONSTANT_P:
return builtin_function_validate_nargs (fndecl, nargs, 1);
Index: gcc/doc/extend.texi
===
--- gcc/doc/extend.texi (revision 233476)
+++ gcc/doc/extend.texi (working copy)
@@ -10144,6 +10144,8 @@ in the Cilk Plus language manual which c
@node Other Builtins
@section Other Built-in Functions Provided by GCC
@cindex built-in functions
+@findex __builtin_alloca
+@findex __builtin_alloca_with_align
@findex __builtin_call_with_static_chain
@findex __builtin_fpclassify
@findex __builtin_isfinite
@@ -10690,6 +10692,89 @@ In the same fashion, GCC provides @code{
@code{__builtin_} prefixed. The @code{isinf} and @code{isnan}
built-in functions appear both with and without the @code{__builtin_} prefix.
+@deftypefn {Built-in Function} void* __builtin_alloca (size_t size)
+The @code{__builtin_alloca} function must be called at block scope.
+The function allocates an object @var{size} bytes large on the stack
+of the calling function. The object is aligned at the default stack
+alignment boundary for the target determined by the
+@code{__BIGGEST_ALIGNMENT__} macro. @code{__builtin_alloca} returns
+a pointer to the first byte of the allocated object. The lifetime of
+the allocated object ends just before the calling function returns to
+its caller. This is so even when @code{__built
Re: [6 Regession] Usage of unitialized pointer io/list_read.c (
See patch to fix this below.
On 02/16/2016 11:38 AM, Dominique d'Humières wrote:
> With the following reduced test
>
> program test
> implicit none
> integer :: i, j, k, ios
> integer, parameter :: big = 600
> character(kind=4,len=big) :: str1, str2
>
> do i=1,big, 10
> do j = 0, 9
> k = i + j
> str2(k:k) = char(65+j)
> end do
> end do
> open(15, status='scratch', encoding="utf-8")
> write(15,*) " 'abcdefgh!'", " ", str2
> rewind(15)
> read(15,*,iostat=ios) str1, str2
> close(15)
> end program
>
> valgrind shows a lot of errors before and after the commit.
>
> Dominique
>
>> Le 16 févr. 2016 à 18:58, Jerry DeLisle a écrit :
>>
>> Up until now we have not had a test case like this for kind=4 testing. I did
>> try -fsanitize=address here and do see the same thing. I will work on this
>> some
>> more.
>>
>> Thanks,
>>
>> Jerry
>
It was all messed up. This patch on current trunk fixes it here. Please test.
diff --git a/libgfortran/io/list_read.c b/libgfortran/io/list_read.c
index fcd4b6e..bebdd8c 100644
--- a/libgfortran/io/list_read.c
+++ b/libgfortran/io/list_read.c
@@ -119,7 +119,10 @@ push_char4 (st_parameter_dt *dtp, int c)
if (dtp->u.p.saved_used >= dtp->u.p.saved_length)
{
dtp->u.p.saved_length = 2 * dtp->u.p.saved_length;
- p = xrealloc (p, dtp->u.p.saved_length * sizeof (gfc_char4_t));
+ dtp->u.p.saved_string =
+ xrealloc (dtp->u.p.saved_string,
+ dtp->u.p.saved_length * sizeof (gfc_char4_t));
+ p = (gfc_char4_t *) dtp->u.p.saved_string;
}
p[dtp->u.p.saved_used++] = c;
Re: [PATCH, rs6000] Fix pasto resulting in wrong instruction from builtins for lvxl
On Tue, Feb 16, 2016 at 4:44 PM, Bill Schmidt wrote: > On Tue, 2016-02-16 at 11:40 -0800, David Edelsohn wrote: >> This is okay, but how about starting with a testcase for this? > > Fair enough. Here's the revised patch with a test, which I've verified > on powerpc64-unknown-linux-gnu. Ok to proceed? > > Thanks! > > Bill > > > [gcc] > > 2016-02-16 Bill Schmidt > > * config/rs6000/altivec.md (*altivec_lvxl__internal): Output > correct instruction. > > [gcc/testsuite] > > 2012-02-16 Bill Schmidt > > * gcc.target/powerpc/vec-cg.c: New test. Okay. Thanks, David
Re: C++ PATCH for c++/69753 (DR141 broke member template lookup)
On 02/16/2016 05:38 AM, Markus Trippelsdorf wrote:
Here is another testcase that every compiler I've tested (clang, icc,
microsoft) accepts, but is rejected by gcc-6:
class A {
public:
template void m_fn1();
};
A *fn1(int *);
template class B : A {
static int *m_fn2() { fn1(m_fn2())->m_fn1(); }
};
Fixed.
Jason
Re: [PATCH] Fix driver handling of multiple -ftree-parallelize-loops= options (PR driver/69805)
On 16/02/16 16:24, Jakub Jelinek wrote:
Hi!
As mentioned in the PR, %{ftree-parallelize-loops=*} expands to
all -ftree-parallelize-loops= options, not just the last one.
So greater_than_spec_func is actually called say for
-ftree-parallelize-loops=0 -ftree-parallelize-loops=2 with
- ftree-parallelize-loops=0 - ftree-parallelize-loops=2 1
(each whitespace separated sequence separate arg), but it asserts
it sees just 3 arguments.
Passing the - and ftree-parallelize-loops= stuff looks weird,
and we have %* that substitutes just the variable part of the option,
so in addition to fixing the case of multiple options I've also changed
%:gt() behaviour, so that it now gets just the numbers and compares the
last two of them. So for the above options it would be called with
0 2 1
and would compare
2 > 1
and return "", or for
-ftree-parallelize-loops=2 -ftree-parallelize-loops=0 -ftree-parallelize-loops=1
would be
2 0 1 1
and compare
1 > 1
and return NULL. %:gt() is not used anywhere else, and has been introduced
only in GCC 6.
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
2016-02-16 Jakub Jelinek
PR driver/69805
* gcc.c (LINK_COMMAND_SPEC, GOMP_SELF_SPECS): Use
:%* in %:gt() argument.
(greater_than_spec_func): Adjust for expecting only numbers,
if there are more than two numbers, compare the last two.
* testsuite/libgomp.c/pr69805.c: New test.
Here's the documentation entry for the gt spec function (I forgot to add
it when introducing the function), using the new semantics.
Copy-pasting from the resulting .info viewed in emacs for a
human-readable version:
...
'gt'
The 'gt' (greater than) function takes one or more arguments.
It returns either NULL or the empty string. If it has one
argument, it returns NULL. If it has two arguments, it
compares them: it returns the empty string if the first
argument is greater than the second argument, otherwise it
returns NULL. If it has more than two arguments, it behaves
as if only the last two arguments were passed. It can be used
f.i. as 'S' in a spec directive %{'S':'X'}: if 'S' is NULL,
the empty string is substituted, and if 'S' is the empty
string, 'X' is substituted.
%:gt(%{fsome-option-value=*:%*} 1)
...
OK for stage4 trunk?
Thanks,
- Tom
Add documentation for spec function gt in invoke.texi
2016-02-16 Tom de Vries
* doc/invoke.texi (@node Spec Files): Document spec function gt.
---
gcc/doc/invoke.texi | 14 ++
1 file changed, 14 insertions(+)
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 666d976..2da7a72 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -24783,6 +24783,20 @@ Use "-Wa,OPTION" to pass "OPTION" to the assembler.
It is used to separate compiler options from assembler options
in the @option{--target-help} output.
+
+@item @code{gt}
+The @code{gt} (greater than) function takes one or more arguments. It returns
+either NULL or the empty string. If it has one argument, it returns NULL.@ If
+it has two arguments, it compares them: it returns the empty string if the first
+argument is greater than the second argument, otherwise it returns NULL.@ If it
+has more than two arguments, it behaves as if only the last two arguments were
+passed. It can be used f.i.@ as @code{S} in a spec directive
+%@{@code{S}:@code{X}@}: if @code{S} is NULL, the empty string is substituted,
+and if @code{S} is the empty string, @code{X} is substituted.
+
+@smallexample
+%:gt(%@{fsome-option-value=*:%*@} 1)
+@end smallexample
@end table
@item %@{@code{S}@}
Re: [RFC] [P2] [PR tree-optimization/33562] Lowering more complex assignments.
On 02/14/2016 11:38 AM, Richard Biener wrote: On February 14, 2016 5:35:13 PM GMT+01:00, Jeff Law wrote: On 02/12/2016 10:21 AM, Jeff Law wrote: But really we simply need a better DSE algorithm. So the way to fix DSE is to keep the existing algorithm and track the hunks of Complex/aggregates that have been set a second time. My first thought was to implement this as an inverted bitmap. ie, set it to 1 for every byte in the complex/aggregate that is set by the first store. Clear bits for each byte in subsequent stores to the pieces. If the bitmap reaches an empty state, then the initial store is dead. Adjusting *ref could work too, but would probably be painful if the subsequent stores don't happen in a convenient order. So that was scary easy. We should have done this a long time ago. Essentially I call ao_get_ref_base to get the offset/size/max_size filled in for the first statement. Those are used to initialize the live bytes bitfield, as long as max_size != -1. Then when we have a possible killing statement, we use the ao in a similar manner to determine which bytes to clear (taking care that the base is the same between the two references and that in the killing statement that the size/max_size are the same.). When all the live bytes are zero then we've killed the original statement. It's ~20 lines of code. I need to pull together some additional tests, but it looks likely we'll be able to wrap this up easily for gcc-6. BTW, we had sth like this before but it had both correctness and more importantly scalability issues. Just a couple more tibits. I instrumented a bootstrap -- the improved DSE finds ~20k additional DSE opportunities during a GCC bootstrap that could not be found by the current DSE. Yes, 20k additional statements deleted by tree DSE. Yow! Of those additional opportunities, none require bit level tracking. So we can just punt when the size/offset is not byte sized/aligned. Of those additional opportunities > 99% are for sizes of 64 bytes or smaller. Thus we can pack those into 1 or 2 bitmap elements, depending on the starting offset. So the bitmap side will be efficient with no real searching if we choose our PARAM value wisely. The outliers are, well, strange. There were cases where we found new DSE opportunities for objects of size 2k bytes or larger. There weren't many of these, but I was surprised at the size. Most likely it's a clobber or mem* thing that's participating in DSE. But I haven't looked closely at those cases yet. There's a ton of statements that are clobbering zero-sized objects. My code can determine when those clobbers are redundant (with some later clobber), but I haven't looked closely to see if that's actually a good thing to do or not. Anyway, I still don't see anything which makes me think this can't wrap-up in the immediate future. jeff
