Re: [patch, fortran] Fix PR 84394
On Fri, Mar 15, 2019 at 11:33:06PM +0100, Thomas Koenig wrote: > Am 15.03.19 um 21:31 schrieb Steve Kargl: > > Patch is missing? > > Entirely correct. Here it is. > Patch looks ok to me. -- Steve
Re: [patch, fortran] Fix PR 84394
Am 15.03.19 um 21:31 schrieb Steve Kargl:
Patch is missing?
Entirely correct. Here it is.
Regards
Thomas
Index: symbol.c
===
--- symbol.c (Revision 269635)
+++ symbol.c (Arbeitskopie)
@@ -1689,7 +1689,15 @@ gfc_add_subroutine (symbol_attribute *attr, const
return false;
attr->subroutine = 1;
- return check_conflict (attr, name, where);
+
+ /* If we are looking at a BLOCK DATA statement and we encounter a
+ name with a leading underscore (which must be
+ compiler-generated), do not check. See PR 84394. */
+
+ if (name && *name != '_' && gfc_current_state () != COMP_BLOCK_DATA)
+return check_conflict (attr, name, where);
+ else
+return true;
}
! { dg-do run }
! PR 84394 - this used to complain about private procedures in
! BLOCK data.
module mod1
implicit none
type :: type1
integer :: i1
end type type1
end module
module mod2
implicit none
contains
subroutine sub1
integer vals
common /block1/ vals(5)
if (any(vals /= [1, 2, 3, 4, 5])) stop 1
end subroutine
end module
block data blkdat
use mod1
integer vals
common /block1/ vals(5)
data vals/1, 2, 3, 4, 5/
end block data blkdat
program main
use mod2, only: sub1
implicit none
call sub1
end program
Re: [PR fortran/60091, patch] - Misleading error messages in rank-2 pointer assignment to rank-1 target
I've committed a slightly rewritten version of the error messages
to trunk as rev.269717, see attached.
Thanks for the review and the comments.
Harald
On 03/12/19 23:19, Thomas Koenig wrote:
> Hi Harald,
>
>> how about the attached version? It is quite verbose and produces
>> messages like
>>
>> Error: Expected list of 'lower-bound-expr:' or list of
>> 'lower-bound-expr:upper-bound-expr' at (1)
>
> I think this way of specifying error messages
>
> +#define BOUNDS_SPEC_LIST "list of %"
>
> ...
>
> + gfc_error ("Rank remapping requires a "
> + BOUNDS_SPEC_LIST " at %L",
> >where);
>
> will cause trouble in translation of the error messages.
>
> Could you maybe use something like
>
> + gfc_error ("Rank remapping requires "
> + lower and upper bounds at %L",
> >where);
>
> and possibly, instead of
>
> - gfc_error ("Either all or none of the upper bounds"
> - " must be specified at %L", >where);
> + gfc_error ("Rank remapping requires a "
> + BOUNDS_SPEC_LIST " at %L",
> + >where);
>return false;
>
> use
>
> " Rank remapping requires that all lower and upper bounds be specified"
>
> ?
>
> (And I am fairly certain that my versions are not the best possible
> ones...)
>
> So, either something like what you propsed (but without the #defines)
> or something like what I wrote above would be OK.
>
> Regards
>
> Thomas
Index: gcc/fortran/expr.c
===
--- gcc/fortran/expr.c (revision 269715)
+++ gcc/fortran/expr.c (working copy)
@@ -3703,6 +3703,7 @@
gfc_ref *ref;
bool is_pure, is_implicit_pure, rank_remap;
int proc_pointer;
+ bool same_rank;
lhs_attr = gfc_expr_attr (lvalue);
if (lvalue->ts.type == BT_UNKNOWN && !lhs_attr.proc_pointer)
@@ -3724,6 +3725,7 @@
proc_pointer = lvalue->symtree->n.sym->attr.proc_pointer;
rank_remap = false;
+ same_rank = lvalue->rank == rvalue->rank;
for (ref = lvalue->ref; ref; ref = ref->next)
{
if (ref->type == REF_COMPONENT)
@@ -3748,36 +3750,68 @@
lvalue->symtree->n.sym->name, >where))
return false;
- /* When bounds are given, all lbounds are necessary and either all
-or none of the upper bounds; no strides are allowed. If the
-upper bounds are present, we may do rank remapping. */
+ /* Fortran standard (e.g. F2018, 10.2.2 Pointer assignment):
+ *
+ * (C1017) If bounds-spec-list is specified, the number of
+ * bounds-specs shall equal the rank of data-pointer-object.
+ *
+ * If bounds-spec-list appears, it specifies the lower bounds.
+ *
+ * (C1018) If bounds-remapping-list is specified, the number of
+ * bounds-remappings shall equal the rank of data-pointer-object.
+ *
+ * If bounds-remapping-list appears, it specifies the upper and
+ * lower bounds of each dimension of the pointer; the pointer target
+ * shall be simply contiguous or of rank one.
+ *
+ * (C1019) If bounds-remapping-list is not specified, the ranks of
+ * data-pointer-object and data-target shall be the same.
+ *
+ * Thus when bounds are given, all lbounds are necessary and either
+ * all or none of the upper bounds; no strides are allowed. If the
+ * upper bounds are present, we may do rank remapping. */
for (dim = 0; dim < ref->u.ar.dimen; ++dim)
{
- if (!ref->u.ar.start[dim]
- || ref->u.ar.dimen_type[dim] != DIMEN_RANGE)
+ if (ref->u.ar.stride[dim])
{
- gfc_error ("Lower bound has to be present at %L",
+ gfc_error ("Stride must not be present at %L",
>where);
return false;
}
- if (ref->u.ar.stride[dim])
+ if (!same_rank && (!ref->u.ar.start[dim] ||!ref->u.ar.end[dim]))
{
- gfc_error ("Stride must not be present at %L",
->where);
+ gfc_error ("Rank remapping requires a "
+"list of % "
+"specifications at %L", >where);
return false;
}
+ if (!ref->u.ar.start[dim]
+ || ref->u.ar.dimen_type[dim] != DIMEN_RANGE)
+ {
+ gfc_error ("Expected list of % or "
+"list of % "
+"specifications at %L", >where);
+ return false;
+ }
if (dim == 0)
rank_remap = (ref->u.ar.end[dim] != NULL);
else
{
- if
Re: [PATCH] LRA: side_effects_p stmts' output is not invariant (PR89721)
Hi! On Fri, Mar 15, 2019 at 04:25:01PM -0400, Vladimir Makarov wrote: > On 2019-03-15 2:30 p.m., Segher Boessenkool wrote: > >PR89721 shows LRA treating an unspec_volatile's result as invariant, > >which of course isn't correct. This patch fixes it. > Segher, thank you for fixing this. The patch is ok to commit. Thanks, done. Is this okay for backports to 8 and 7 as well? After a while of course. > >Question. Is side_effects_p the correct check? Or do we want to > >allow PRE_INC etc. here? What about CALL? > > > No, we don't want INC/DEC. Inheritance helps to reuse some reloaded > values which are expensive for calculation. So if we have two INCs, we > will have only one, which is wrong. The same is for calls. And we could handle it, but that requires code that isn't there yet, and is it worth it anyway. Okay :-) Segher
Re: [PATCH] Make hashtab/hash-table elt removal not fail on removing non-existent entry
On Thu, 14 Mar 2019 10:19:17 +0100
Jakub Jelinek wrote:
> On Thu, Mar 14, 2019 at 09:57:27AM +0100, Richard Biener wrote:
> > I'd say make it work, thus your patch below is OK.
>
> Ok, here is an updated patch with some self-tests coverage as well that I'll
> bootstrap/regtest.
I had the simple slot == NULL || is_empty (*slot) hack in
remove_elt_with_hash in my aldot/fortran-fe-stringpool stash since a
long time now and hence support this change, fwiw.
Many thanks!
>
> 2019-03-14 Jason Merrill
> Jakub Jelinek
>
> * hash-table.h (remove_elt_with_hash): Return if slot is NULL rather
> than if is_empty (*slot).
> * hash-set-tests.c (test_set_of_strings): Add tests for addition of
> existing elt and for elt removal.
> * hash-map-tests.c (test_map_of_strings_to_int): Add test for removal
> of already removed elt.
>
> * hashtab.c (htab_remove_elt_with_hash): Return if slot is NULL rather
> than if *slot is HTAB_EMPTY_ENTRY.
>
> --- gcc/hash-table.h.jj 2019-01-01 12:37:17.446970209 +0100
> +++ gcc/hash-table.h 2019-03-14 08:48:18.861131498 +0100
> @@ -940,7 +940,7 @@ hash_table
> ::remove_elt_with_hash (const compare_type , hashval_t hash)
> {
>value_type *slot = find_slot_with_hash (comparable, hash, NO_INSERT);
> - if (is_empty (*slot))
> + if (slot == NULL)
> return;
>
>Descriptor::remove (*slot);
> --- gcc/hash-set-tests.c.jj 2019-01-01 12:37:21.648901265 +0100
> +++ gcc/hash-set-tests.c 2019-03-14 10:06:04.688950764 +0100
> @@ -48,11 +48,26 @@ test_set_of_strings ()
>ASSERT_EQ (false, s.add (red));
>ASSERT_EQ (false, s.add (green));
>ASSERT_EQ (false, s.add (blue));
> + ASSERT_EQ (true, s.add (green));
>
>/* Verify that the values are now within the set. */
>ASSERT_EQ (true, s.contains (red));
>ASSERT_EQ (true, s.contains (green));
>ASSERT_EQ (true, s.contains (blue));
> + ASSERT_EQ (3, s.elements ());
> +
> + /* Test removal. */
> + s.remove (red);
> + ASSERT_EQ (false, s.contains (red));
> + ASSERT_EQ (true, s.contains (green));
> + ASSERT_EQ (true, s.contains (blue));
> + ASSERT_EQ (2, s.elements ());
> +
> + s.remove (red);
> + ASSERT_EQ (false, s.contains (red));
> + ASSERT_EQ (true, s.contains (green));
> + ASSERT_EQ (true, s.contains (blue));
> + ASSERT_EQ (2, s.elements ());
> }
>
> /* Run all of the selftests within this file. */
> --- gcc/hash-map-tests.c.jj 2019-01-21 20:46:21.963092085 +0100
> +++ gcc/hash-map-tests.c 2019-03-14 10:06:56.994105872 +0100
> @@ -78,6 +78,10 @@ test_map_of_strings_to_int ()
>ASSERT_EQ (5, m.elements ());
>ASSERT_EQ (NULL, m.get (eric));
>
> + m.remove (eric);
> + ASSERT_EQ (5, m.elements ());
> + ASSERT_EQ (NULL, m.get (eric));
> +
>/* A plain char * key is hashed based on its value (address), rather
> than the string it points to. */
>char *another_ant = static_cast (xcalloc (4, 1));
> --- libiberty/hashtab.c.jj2019-01-01 12:38:46.868503025 +0100
> +++ libiberty/hashtab.c 2019-03-14 08:47:49.172612001 +0100
> @@ -725,7 +725,7 @@ htab_remove_elt_with_hash (htab_t htab,
>PTR *slot;
>
>slot = htab_find_slot_with_hash (htab, element, hash, NO_INSERT);
> - if (*slot == HTAB_EMPTY_ENTRY)
> + if (slot == NULL)
> return;
>
>if (htab->del_f)
>
>
> Jakub
[C++ PATCH] Fix thread_local initialization (PR c++/60702)
Hi!
As the testcase shows, we replace TLS vars that need initialization
in finish_id_expression_1 and in tsubst_copy_and_build of a VAR_DECL
with a _ZTW* call, but miss it in other cases where finish_id_expression
is not actually called. In particular build_class_member_access_expr
when we do object.static_tls_data_member access doesn't wrap it, and
when using some_class::static_tls_data_member in a teplate, we go
through finish_qualified_id_expr and not finish_id_expression either
(and tsubst_copy in that case, so it doesn't go through
tsubst_copy_and_build).
The following patch fixes that. While it is not a regression, it is a
serious wrong-code issue that just gained 6th reporter of the same issue.
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
2019-03-15 Jakub Jelinek
PR c++/60702
* semantics.c (finish_qualified_id_expr): Handle accesses to TLS
variables.
* typeck.c (build_class_member_access_expr): Likewise.
* g++.dg/tls/thread_local11.C: New test.
* g++.dg/tls/thread_local11.h: New test.
* g++.dg/tls/thread_local12a.C: New test.
* g++.dg/tls/thread_local12b.C: New test.
* g++.dg/tls/thread_local12c.C: New test.
* g++.dg/tls/thread_local12d.C: New test.
* g++.dg/tls/thread_local12e.C: New test.
* g++.dg/tls/thread_local12f.C: New test.
* g++.dg/tls/thread_local12g.C: New test.
* g++.dg/tls/thread_local12h.C: New test.
* g++.dg/tls/thread_local12i.C: New test.
* g++.dg/tls/thread_local12j.C: New test.
* g++.dg/tls/thread_local12k.C: New test.
* g++.dg/tls/thread_local12l.C: New test.
--- gcc/cp/semantics.c.jj 2019-03-14 09:14:16.718012031 +0100
+++ gcc/cp/semantics.c 2019-03-15 16:53:14.270384477 +0100
@@ -2135,6 +2135,17 @@ finish_qualified_id_expr (tree qualifyin
expr = build_qualified_name (TREE_TYPE (expr),
qualifying_class, expr,
template_p);
+ else if (VAR_P (expr)
+ && !processing_template_decl
+ && !cp_unevaluated_operand
+ && (TREE_STATIC (expr) || DECL_EXTERNAL (expr))
+ && CP_DECL_THREAD_LOCAL_P (expr))
+ {
+ if (tree wrap = get_tls_wrapper_fn (expr))
+ /* Replace an evaluated use of the thread_local variable with
+ a call to its wrapper. */
+ expr = build_cxx_call (wrap, 0, NULL, tf_warning_or_error);
+ }
expr = convert_from_reference (expr);
}
--- gcc/cp/typeck.c.jj 2019-03-13 21:21:27.0 +0100
+++ gcc/cp/typeck.c 2019-03-15 16:23:27.582046214 +0100
@@ -2443,6 +2443,16 @@ build_class_member_access_expr (cp_expr
/* A static data member. */
result = member;
mark_exp_read (object);
+
+ tree wrap;
+ if (!cp_unevaluated_operand
+ && !processing_template_decl
+ && CP_DECL_THREAD_LOCAL_P (result)
+ && (wrap = get_tls_wrapper_fn (result)))
+ /* Replace an evaluated use of the thread_local variable with
+ a call to its wrapper. */
+ result = build_cxx_call (wrap, 0, NULL, tf_warning_or_error);
+
/* If OBJECT has side-effects, they are supposed to occur. */
if (TREE_SIDE_EFFECTS (object))
result = build2 (COMPOUND_EXPR, TREE_TYPE (result), object, result);
--- gcc/testsuite/g++.dg/tls/thread_local11.C.jj2019-03-15
17:02:44.752275408 +0100
+++ gcc/testsuite/g++.dg/tls/thread_local11.C 2019-03-15 17:31:19.199665422
+0100
@@ -0,0 +1,48 @@
+// PR c++/60702
+// { dg-do compile { target c++11 } }
+// { dg-add-options tls }
+// { dg-require-effective-target tls_runtime }
+// { dg-additional-options "-fdump-tree-gimple" }
+// { dg-final { scan-tree-dump-times "_ZTW2s1" 2 "gimple" } }
+// { dg-final { scan-tree-dump-times "_ZTW2s2" 2 "gimple" } }
+// { dg-final { scan-tree-dump-times "_ZTW2s3" 2 "gimple" } }
+// { dg-final { scan-tree-dump-times "_ZTW2s4" 2 "gimple" } }
+// { dg-final { scan-tree-dump-times "_ZTWN1T2u1E" 2 "gimple" } }
+// { dg-final { scan-tree-dump-times "_ZTWN1T2u2E" 2 "gimple" } }
+// { dg-final { scan-tree-dump-times "_ZTWN1T2u3E" 2 "gimple" } }
+// { dg-final { scan-tree-dump-times "_ZTWN1T2u4E" 2 "gimple" } }
+// { dg-final { scan-tree-dump-times "_ZTWN1T2u5E" 2 "gimple" } }
+// { dg-final { scan-tree-dump-times "_ZTWN1T2u6E" 2 "gimple" } }
+// { dg-final { scan-tree-dump-times "_ZTWN1T2u7E" 2 "gimple" } }
+// { dg-final { scan-tree-dump-times "_ZTWN1T2u8E" 2 "gimple" } }
+// { dg-final { scan-tree-dump-times "_ZTH2s1" 1 "gimple" } }
+// { dg-final { scan-tree-dump-times "_ZTH2s2" 1 "gimple" } }
+// { dg-final { scan-tree-dump-times "_ZTH2s3" 1 "gimple" } }
+// { dg-final { scan-tree-dump-times "_ZTH2s4" 1 "gimple" } }
+// { dg-final { scan-tree-dump-times "_ZTHN1T2u1E" 1 "gimple" } }
+// { dg-final { scan-tree-dump-times "_ZTHN1T2u2E" 1 "gimple" } }
+// { dg-final {
[PATCH] Fix i?86 -mfpmath=sse ceil inline expansion (PR target/89726)
Hi!
As the enhanced testcase shows, ix86_expand_floorceildf_32 doesn't emit
correct ceil when honoring signed zeros, because it performs copysign,
then comparison and then based on the comparison an optional addition of 1.
The comment claims that it is essential to use x2 -= -1.0 instead of
x2 += 1.0, but in reality we emit x2 += 1.0 anyway and it makes no
difference on the sign of result if it is zero, as (unless rounding to
negative infinity) both -1.0 - (-1.0) and -1.0 + 1.0 evaluate to +0.0
rather than -0.0 we need in this case.
I have no better ideas than to use another copysign, but it should be just
one extra instruction, as we've already previously done copysign from the
same source and thus hve already computed the x & signmask and so this patch
just adds a {,v}por of that previously computed x & signmask into the result
again. In this case we aren't really copying it always to positive, but
all we want is handle the single problematic case when we compute positive 0
and need negative 0, for all others the oring won't really change the sign.
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
Or does anybody have some smarter idea?
Note, not sure if floor isn't incorrect in the round to negative infinity
case (but that would be preexisting issue).
2019-03-15 Jakub Jelinek
PR target/89726
* config/i386/i386.c (ix86_expand_floorceildf_32): In ceil
compensation use x2 += 1 instead of x2 -= -1 and when honoring
signed zeros, do another copysign after the compensation.
* gcc.target/i386/fpprec-1.c (x): Add 6 new constants.
(expect_round, expect_rint, expect_floor, expect_ceil, expect_trunc):
Add expected results for them.
--- gcc/config/i386/i386.c.jj 2019-03-14 23:44:27.862560139 +0100
+++ gcc/config/i386/i386.c 2019-03-15 15:07:54.607453430 +0100
@@ -45563,8 +45563,10 @@ ix86_expand_floorceildf_32 (rtx operand0
x2 -= 1;
Compensate. Ceil:
if (x2 < x)
- x2 -= -1;
-return x2;
+ x2 += 1;
+ if (HONOR_SIGNED_ZEROS (mode))
+ x2 = copysign (x2, x);
+ return x2;
*/
machine_mode mode = GET_MODE (operand0);
rtx xa, TWO52, tmp, one, res, mask;
@@ -45590,17 +45592,16 @@ ix86_expand_floorceildf_32 (rtx operand0
/* xa = copysign (xa, operand1) */
ix86_sse_copysign_to_positive (xa, xa, res, mask);
- /* generate 1.0 or -1.0 */
- one = force_reg (mode,
- const_double_from_real_value (do_floor
-? dconst1 : dconstm1, mode));
+ /* generate 1.0 */
+ one = force_reg (mode, const_double_from_real_value (dconst1, mode));
/* Compensate: xa = xa - (xa > operand1 ? 1 : 0) */
tmp = ix86_expand_sse_compare_mask (UNGT, xa, res, !do_floor);
emit_insn (gen_rtx_SET (tmp, gen_rtx_AND (mode, one, tmp)));
- /* We always need to subtract here to preserve signed zero. */
- tmp = expand_simple_binop (mode, MINUS,
+ tmp = expand_simple_binop (mode, do_floor ? MINUS : PLUS,
xa, tmp, NULL_RTX, 0, OPTAB_DIRECT);
+ if (!do_floor && HONOR_SIGNED_ZEROS (mode))
+ix86_sse_copysign_to_positive (tmp, tmp, res, mask);
emit_move_insn (res, tmp);
emit_label (label);
--- gcc/testsuite/gcc.target/i386/fpprec-1.c.jj 2010-05-21 11:46:22.0
+0200
+++ gcc/testsuite/gcc.target/i386/fpprec-1.c2019-03-15 15:01:51.430345113
+0100
@@ -11,6 +11,9 @@ double x[] = { __builtin_nan(""), __buil
0x1.1p-1, 0x1.fp-2,
0x1.1p+0, 0x1.fp-1,
0x1.80001p+0, 0x1.7p+0,
+ -0x1.1p-1, -0x1.fp-2,
+ -0x1.1p+0, -0x1.fp-1,
+ -0x1.80001p+0, -0x1.7p+0,
-0.0, 0.0, -0.5, 0.5, -1.0, 1.0, -1.5, 1.5, -2.0, 2.0,
-2.5, 2.5 };
#define NUM (sizeof(x)/sizeof(double))
@@ -19,6 +22,7 @@ double expect_round[] = { __builtin_nan(
-0x1.fp+1023, 0x1.fp+1023,
-0.0, 0.0,
1.0, 0.0, 1.0, 1.0, 2.0, 1.0,
+ -1.0, -0.0, -1.0, -1.0, -2.0, -1.0,
-0.0, 0.0, -1.0, 1.0, -1.0, 1.0, -2.0, 2.0, -2.0, 2.0,
-3.0, 3.0 };
@@ -26,6 +30,7 @@ double expect_rint[] = { __builtin_nan("
-0x1.fp+1023, 0x1.fp+1023,
-0.0, 0.0,
1.0, 0.0, 1.0, 1.0, 2.0, 1.0,
+-1.0, -0.0, -1.0, -1.0, -2.0, -1.0,
-0.0, 0.0, -0.0, 0.0, -1.0, 1.0, -2.0, 2.0, -2.0, 2.0,
-2.0, 2.0 };
@@ -33,6 +38,7 @@ double expect_floor[] = { __builtin_nan(
-0x1.fp+1023, 0x1.fp+1023,
-1.0, 0.0,
0.0, 0.0, 1.0, 0.0, 1.0, 1.0,
+-1.0, -1.0, -2.0, -1.0, -2.0, -2.0,
-0.0, 0.0, -1.0, 0.0, -1.0, 1.0, -2.0, 1.0, -2.0, 2.0,
-3.0, 2.0 };
@@ -40,6 +46,7 @@ double expect_ceil[] = { __builtin_nan("
-0x1.fp+1023, 0x1.fp+1023,
Re: [patch, fortran] Fix PR 84394
On Fri, Mar 15, 2019 at 07:55:51PM +0100, Thomas Koenig wrote: > Hello world, > > this patch fixes a rejects-valid 7/8/9 regression where subroutines like > _deallocate are added to a derived type in a block data, and because > they were marked PRIVATE, an error occurred. > > This solution is look for this particular case by checking for an > underscore as the first letter of the subroutine name. This can only > occur for compiler-generated subroutines, so this should be safe. > I have also restricted this to the particular case of a BLOCK DATA > as not to mask other potential errors. > > This is not elegant, but effecive... > > Regression-tested. > > Ok for trunk and backport? > Patch is missing? -- Steve
Re: [PATCH] LRA: side_effects_p stmts' output is not invariant (PR89721)
On 2019-03-15 2:30 p.m., Segher Boessenkool wrote: PR89721 shows LRA treating an unspec_volatile's result as invariant, which of course isn't correct. This patch fixes it. Segher, thank you for fixing this. The patch is ok to commit. Question. Is side_effects_p the correct check? Or do we want to allow PRE_INC etc. here? What about CALL? No, we don't want INC/DEC. Inheritance helps to reuse some reloaded values which are expensive for calculation. So if we have two INCs, we will have only one, which is wrong. The same is for calls. I believe we can ignore calls here because LRA does not reload calls (call result reg). Actually I believe the same is true for INC/DEC (we can reload INC/DEC regs but not INC/DEC themselves). Segher 2019-04-15 Segher Boessenkool PR rtl-optimization/89721 * lra-constraints (invariant_p): Return false if side_effects_p holds. --- gcc/lra-constraints.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/gcc/lra-constraints.c b/gcc/lra-constraints.c index afbd5d0..24f11ed 100644 --- a/gcc/lra-constraints.c +++ b/gcc/lra-constraints.c @@ -5839,6 +5839,9 @@ invariant_p (const_rtx x) enum rtx_code code; int i, j; + if (side_effects_p (x)) +return false; + code = GET_CODE (x); mode = GET_MODE (x); if (code == SUBREG)
[PATCH] Fix up diagnostics emitted by fortran FE load_line (PR fortran/89724)
Hi!
As reported in the PR, when glibc installs math-vector-fortran.h header,
e.g. continuation_9.f90 testcase breaks.
The problem is that load_line does its own current line counting and does
that as a rolling count of how many lines were seen already, ignoring
preprocessor line numbers, or includes from other files etc.
Unfortunately, to use %C we'd need gfc_locus which is only usable after
load_file returns and the caller updates it; it would be nice to emit at
least filename:line at the start of diagnostic, but it is unclear how to
achieve that easily.
So, this patch keeps using just the line numbers, just picks them up from
current_file->line where it stands for the current line in the current file.
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
2019-03-15 Jakub Jelinek
PR fortran/89724
* scanner.c (load_line): Remove linenum and current_line static
variables, add warned_tabs automatic variable. Use current_file->line
instead of current_line and warned_tabs boolean to avoid diagnosing
tabs multiple times on the same line.
* gfortran.dg/continuation_15.f90: New test.
* gfortran.dg/continuation_16.f90: New test.
--- gcc/fortran/scanner.c.jj2019-03-13 09:23:46.667383435 +0100
+++ gcc/fortran/scanner.c 2019-03-15 10:28:33.467897756 +0100
@@ -1738,12 +1738,12 @@ gfc_gobble_whitespace (void)
static int
load_line (FILE *input, gfc_char_t **pbuf, int *pbuflen, const int *first_char)
{
- static int linenum = 0, current_line = 1;
int c, maxlen, i, preprocessor_flag, buflen = *pbuflen;
int trunc_flag = 0, seen_comment = 0;
int seen_printable = 0, seen_ampersand = 0, quoted = ' ';
gfc_char_t *buffer;
bool found_tab = false;
+ bool warned_tabs = false;
/* Determine the maximum allowed line length. */
if (gfc_current_form == FORM_FREE)
@@ -1793,10 +1793,10 @@ load_line (FILE *input, gfc_char_t **pbu
{
if (pedantic)
gfc_error_now ("%<&%> not allowed by itself in line %d",
- current_line);
+ current_file->line);
else
gfc_warning_now (0, "%<&%> not allowed by itself in line %d",
-current_line);
+current_file->line);
}
break;
}
@@ -1850,12 +1850,12 @@ load_line (FILE *input, gfc_char_t **pbu
{
found_tab = true;
- if (warn_tabs && seen_comment == 0 && current_line != linenum)
+ if (warn_tabs && seen_comment == 0 && !warned_tabs)
{
- linenum = current_line;
+ warned_tabs = true;
gfc_warning_now (OPT_Wtabs,
"Nonconforming tab character in column %d "
- "of line %d", i+1, linenum);
+ "of line %d", i + 1, current_file->line);
}
while (i < 6)
@@ -1934,7 +1934,6 @@ next_char:
*buffer = '\0';
*pbuflen = buflen;
- current_line++;
return trunc_flag;
}
--- gcc/testsuite/gfortran.dg/continuation_15.f90.jj2019-03-15
10:37:48.338823839 +0100
+++ gcc/testsuite/gfortran.dg/continuation_15.f90 2019-03-15
10:37:42.924912371 +0100
@@ -0,0 +1,9 @@
+! PR fortran/89724
+! { dg-do compile }
+! { dg-options "-std=f95" }
+
+include 'continuation_9.f90'
+
+! { dg-warning "not allowed by itself in line 3" "" { target *-*-* } 0 }
+! { dg-warning "not allowed by itself in line 4" "" { target *-*-* } 0 }
+! { dg-warning "not allowed by itself in line 5" "" { target *-*-* } 0 }
--- gcc/testsuite/gfortran.dg/continuation_16.f90.jj2019-03-15
10:39:21.750296254 +0100
+++ gcc/testsuite/gfortran.dg/continuation_16.f90 2019-03-15
10:40:49.036868837 +0100
@@ -0,0 +1,10 @@
+! PR fortran/89724
+! { dg-do compile }
+! { dg-options "-std=f95 -nostdinc -fpre-include=simd-builtins-1.h" }
+ &
+&
+ &
+end
+! { dg-warning "not allowed by itself in line 4" "" { target *-*-* } 0 }
+! { dg-warning "not allowed by itself in line 5" "" { target *-*-* } 0 }
+! { dg-warning "not allowed by itself in line 6" "" { target *-*-* } 0 }
Jakub
[C++ PATCH] Add -fconstexpr-ops-limit= option (PR c++/87481)
On Fri, Mar 08, 2019 at 02:51:42PM +0100, Richard Biener wrote:
> Well, I think having only a single limit is desirable which means we have to
> count something resembling the overall work done. We don't have to document
> that it matches "statements" (whatever that exactly would be).
>
> I don't see multi-megabyte constexpr arrays as an issue - do you think we'd
> reject them already when parsing them? We could always decide to elide
> counting things we didn't do any real work on (like a literal '1'
> initializer).
Ok, here is an updated patch that counts cxx_eval_constant_expression calls
when:
1) not in stmt skipping mode
2) not CONSTANT_CLASS_P
3) not location_wrapper_p
so that if there are huge constexpr array initializers that just contain
constants, they aren't counted towards the limit if there are just constants
wrapped in location wrappers.
The default limit is chosen such that on the new testcase it bails out after
about 15 seconds of computation on a fast machine (for a single
cxx_eval_outermost_constant_expression). Wanted to find something so that
it doesn't trigger too often.
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
2019-03-15 Jakub Jelinek
PR c++/87481
* doc/invoke.texi (-fconstexpr-ops-limit=): Document.
* c.opt (-fconstexpr-ops-limit=): New option.
* constexpr.c (constexpr_ops_count): New variable.
(cxx_eval_constant_expression): When not skipping, not constant class
or location wrapper, increment constexpr_ops_count and if it is above
constexpr_loop_nest_limit, diagnose failure.
(cxx_eval_outermost_constant_expr): Clear constexpr_ops_count.
* g++.dg/cpp1y/constexpr-87481.C: New test.
--- gcc/doc/invoke.texi.jj 2019-03-14 23:44:26.121588028 +0100
+++ gcc/doc/invoke.texi 2019-03-15 19:24:37.428486959 +0100
@@ -210,7 +210,7 @@ in the following sections.
@gccoptlist{-fabi-version=@var{n} -fno-access-control @gol
-faligned-new=@var{n} -fargs-in-order=@var{n} -fchar8_t -fcheck-new @gol
-fconstexpr-depth=@var{n} -fconstexpr-loop-limit=@var{n} @gol
--fno-elide-constructors @gol
+-fconstexpr-ops-limit=@var{n} -fno-elide-constructors @gol
-fno-enforce-eh-specs @gol
-fno-gnu-keywords @gol
-fno-implicit-templates @gol
@@ -2525,6 +2525,16 @@ Set the maximum number of iterations for
to @var{n}. A limit is needed to detect infinite loops during
constant expression evaluation. The default is 262144 (1<<18).
+@item -fconstexpr-ops-limit=@var{n}
+@opindex fconstexpr-ops-limit
+Set the maximum number of operations during a single constexpr evaluation.
+Even when number of iterations of a single loop is limited with the above
limit,
+if there are several nested loops and each of them has many iterations but
still
+smaller than the above limit, or if in a body of some loop or even outside
+of a loop too many expressions need to be evaluated, the resulting constexpr
+evaluation might take too long.
+The default is 33554432 (1<<25).
+
@item -fdeduce-init-list
@opindex fdeduce-init-list
Enable deduction of a template type parameter as
--- gcc/c-family/c.opt.jj 2019-03-07 20:07:14.890098884 +0100
+++ gcc/c-family/c.opt 2019-03-15 19:22:06.298922976 +0100
@@ -1416,6 +1416,10 @@ fconstexpr-loop-limit=
C++ ObjC++ Joined RejectNegative UInteger Var(constexpr_loop_limit)
Init(262144)
-fconstexpr-loop-limit=Specify maximum constexpr loop
iteration count.
+fconstexpr-ops-limit=
+C++ ObjC++ Joined RejectNegative Host_Wide_Int Var(constexpr_ops_limit)
Init(33554432)
+-fconstexpr-ops-limit= Specify maximum number of constexpr operations
during a single constexpr evaluation.
+
fdebug-cpp
C ObjC C++ ObjC++
Emit debug annotations during preprocessing.
--- gcc/cp/constexpr.c.jj 2019-03-14 09:12:31.667710331 +0100
+++ gcc/cp/constexpr.c 2019-03-15 19:18:29.559416561 +0100
@@ -4335,6 +4335,11 @@ lookup_placeholder (const constexpr_ctx
return ob;
}
+/* Number of cxx_eval_constant_expression calls (except skipped ones,
+ on simple constants or location wrappers) encountered during current
+ cxx_eval_outermost_constant_expr call. */
+static HOST_WIDE_INT constexpr_ops_count;
+
/* Attempt to reduce the expression T to a constant value.
On failure, issue diagnostic and return error_mark_node. */
/* FIXME unify with c_fully_fold */
@@ -4402,6 +4407,20 @@ cxx_eval_constant_expression (const cons
return t;
}
+ /* Avoid excessively long constexpr evaluations. */
+ if (!location_wrapper_p (t)
+ && ++constexpr_ops_count >= constexpr_ops_limit)
+{
+ if (!ctx->quiet)
+ error_at (cp_expr_loc_or_loc (t, input_location),
+ "% evaluation operation count exceeds limit of "
+ "%wd (use -fconstexpr-ops-limit= to increase the limit)",
+ constexpr_ops_limit);
+ constexpr_ops_count = INTTYPE_MINIMUM (HOST_WIDE_INT);
+ *non_constant_p = true;
+ return t;
+
[patch, fortran] Fix PR 84394
Hello world, this patch fixes a rejects-valid 7/8/9 regression where subroutines like _deallocate are added to a derived type in a block data, and because they were marked PRIVATE, an error occurred. This solution is look for this particular case by checking for an underscore as the first letter of the subroutine name. This can only occur for compiler-generated subroutines, so this should be safe. I have also restricted this to the particular case of a BLOCK DATA as not to mask other potential errors. This is not elegant, but effecive... Regression-tested. Ok for trunk and backport? Regards Thomas
[PATCH] LRA: side_effects_p stmts' output is not invariant (PR89721)
PR89721 shows LRA treating an unspec_volatile's result as invariant, which of course isn't correct. This patch fixes it. Question. Is side_effects_p the correct check? Or do we want to allow PRE_INC etc. here? What about CALL? Segher 2019-04-15 Segher Boessenkool PR rtl-optimization/89721 * lra-constraints (invariant_p): Return false if side_effects_p holds. --- gcc/lra-constraints.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/gcc/lra-constraints.c b/gcc/lra-constraints.c index afbd5d0..24f11ed 100644 --- a/gcc/lra-constraints.c +++ b/gcc/lra-constraints.c @@ -5839,6 +5839,9 @@ invariant_p (const_rtx x) enum rtx_code code; int i, j; + if (side_effects_p (x)) +return false; + code = GET_CODE (x); mode = GET_MODE (x); if (code == SUBREG) -- 1.8.3.1
Re: [C++ Patch] PR 85014 ("[7/8/9 Regression] internal compiler error: in lookup_base, at cp/search.c:185")
On 3/15/19 9:28 AM, Paolo Carlini wrote: Hi, another - rather long standing - error-recovery regression, where, in some rather special circumstances, we end up passing the FUNCTION_DECL representing the operator () of the lambda to maybe_dummy_object and obviously we almost immediately crash. Not sure how much we want to dig - but simply checking that context_for_name_lookup is actually returning a type appears to work fine, the error_mark_node then propagates back to cp_parser_late_parse_one_default_arg and so on. In the special circumstances of the testcase, context_for_name_lookup finds ANON_AGGR_TYPE_P set for the DECL_CONTEXT of 'b' and iterates to its TYPE_CONTEXT which is the FUNCTION_DECL representing the operator () of the lambda. This is because we just called check_tag_decl on that anonymous type as part of calling shadow_tag on the abstract declaration. Tested x86_64-linux. OK. Jason
Re: [PATCH] Fix PR71598, aliasing between enums and compatible types
On 3/15/19 9:33 AM, Richard Biener wrote: The following is an attempt to fix PR71598 where C (and C++?) have an implementation-defined compatible integer type for each enum and the TBAA rules mandate that accesses using a compatible type are allowed. This does not apply to C++; an enum does not alias its underlying type. Jason
Go patch committed: Preserve nointerface property when inlining methods
This patch by Than McIntosh fixes the Go frontend to preserve the
nointerface property when inlining method bodies. This fixes
https://golang.org/issue/30862. Bootstrapped and ran Go tests on
x86_64-pc-linux-gnu. Committed to mainline.
Ian
Index: gcc/go/gofrontend/MERGE
===
--- gcc/go/gofrontend/MERGE (revision 269710)
+++ gcc/go/gofrontend/MERGE (working copy)
@@ -1,4 +1,4 @@
-a99959e6a4a899cfcc4d46e6b54da15d23c58a14
+cc70be24502faeffefb66fd0abeb7f20a6c7792a
The first line of this file holds the git revision number of the last
merge done from the gofrontend repository.
Index: gcc/go/gofrontend/gogo.cc
===
--- gcc/go/gofrontend/gogo.cc (revision 269619)
+++ gcc/go/gofrontend/gogo.cc (working copy)
@@ -6899,6 +6899,8 @@ Function_declaration::import_function_bo
if (fntype->is_method())
{
+ if (this->nointerface())
+fn->set_nointerface();
const Typed_identifier* receiver = fntype->receiver();
Variable* recv_param = new Variable(receiver->type(), NULL, false,
true, true, start_loc);
[committed] list myself as selective scheduler maintainer
Hi, I moved my entry in the MAINTAINERS file per https://gcc.gnu.org/ml/gcc/2011-11/msg00264.html and https://gcc.gnu.org/ml/gcc-patches/2019-03/msg00761.html . Alexander * MAINTAINERS (Reviewers): Add myself as selective scheduling reviewer. (Write After Approval): Remove myself. --- MAINTAINERS (revision 269710) +++ MAINTAINERS (working copy) @@ -293,6 +293,7 @@ register allocationSeongbae Park RTL optimizers Steven Bosscher selective scheduling Andrey Belevantsev +selective scheduling Alexander Monakov wide-int Kenneth Zadeck wide-int Mike Stump wide-int Richard Sandiford @@ -508,7 +509,6 @@ Martin Michlmayr Lee Millward Alan Modra -Alexander Monakov Catherine Moore James A. Morrison Brooks Moses
Re: [PATCH] Fix PR71598, aliasing between enums and compatible types
Hi, On Fri, 15 Mar 2019, Michael Matz wrote: > I.e. what you touched is the naming of sets (giving them identifiers), > whereas you should have touched where the relations between the sets are > established. I _think_ instead of giving enum and basetypes the same > alias set you should have rather called record_alias_subset(intset, > enumset) (or the other way around, I'm always confused there :) ). Or, because an enum with these properties could be modelled as a struct containing one member of basetype you could also change record_component_aliases(), though that doesn't allow language specific behaviour differences. Ciao, Michael.
Re: [PATCH] Fix PR71598, aliasing between enums and compatible types
Hi, On Fri, 15 Mar 2019, Richard Biener wrote: > >But different enums aren't compatible with each other (or are they?), > >while your fix simply gives enums the aliasing set of the underlying > >type, i.e. makes all of them alias with each other. That's quite > >conservative. > > But that follows from the need to be able to access an enum as int and > the other way around. So both alias sets need to be subsets of each > other which means they need to be equal. No. Suppose you have enum E1, E2 and int, you have to arrange it such that aliasset(E1) \subset aliasset(int) && aliasset(E2) \subset aliasset(int). That doesn't (and shouldn't) imply any relationship between aliassets of E1 and E2. Of course you can only express something else than equality when you aren't making all three of them the same set-identifiers. I.e. what you touched is the naming of sets (giving them identifiers), whereas you should have touched where the relations between the sets are established. I _think_ instead of giving enum and basetypes the same alias set you should have rather called record_alias_subset(intset, enumset) (or the other way around, I'm always confused there :) ). Ciao, Michael.
Re: [wwwdocs] Update snapshots.html to refer to sha512sum
On Thu, 14 Mar 2019, Jonathan Wakely wrote: > The checksum file uses SHA512 not MD5SUM, so the instructions should be > updated accordingly. > > SInce there's only one tarfile these days (not separate ones for > gcc-base, gcc-g++, gcc-fortran, etc) it doesn't seem necessary to > filter out the "OK" lines with grep, but I could add --quiet to the > command if we want that. I added --ignore-missing because I doubt most > people also download the README and index.html files, so the option > avoids gettings warnings about those files. Thank you! Gerald
Re: bootstrap error due to constexpr in go/gofrontend/ast-dump.cc
On Wed, Mar 13, 2019 at 10:01 AM Martin Sebor wrote: > > A recent change (r269633 AFAICS) introduced the constexpr keyword > into go which breaks bootstrap with a C++ 98 compiler. I fixed > it like this in my tree but haven't fully tested it. I just > thought I'd send a heads up before others run into it. > > Martin > > === > --- go/gofrontend/ast-dump.cc (revision 269652) > +++ go/gofrontend/ast-dump.cc (working copy) > @@ -610,7 +610,7 @@ class Type_dumper >const char *tag); > std::pair lookup(const Type*); > > - static constexpr unsigned notag = 0x; > + static const unsigned notag = 0x; > >private: > const Type* top_; Thanks. Committed to mainline. Ian
[RFC] D support for S/390
Hi,
during the last few days I tried to get D running on s390x (apparently
the first Big Endian platform to try it?). I did not yet go through the
code systematically and add a version(SystemZ) in every place where it
might be needed but rather tried to fix test failures as they arose.
After enabling the architecture in the configure files and adding TLS
support (see initial.diff) the test suite showed 200 something failures.
Including big endian handling in some test cases (tests.diff), the
number of failures went down to ~130.
Some more involved cases are left: dmd/constfold.c handles
'a' ~ "abc"
but fails because 'a' is treated as int64 and only the first bytes
are memcpy'd into the result buffer. When using a similar logic as
used for
"abc" ~ a.
This works but seems a rather hacky approach (cat.diff).
An even more hacky fix I applied for libdruntime/rt/aaA.d (align.diff)
where algn = 0 is passed to the talign function. I suppose it shouldn't
ever be called with algn = 0 but the alignment should be set somewhere
else initially?
Another problem is printing of characters. std.uni.isGraphical returns
false for standard ASCII characters because of the trie traversal or
rather the final lookup in memory via PackedPtr
cast(inout(T))(cast(U*) origin)[idx]
This gets the first byte but should get the last byte on Big Endian. A
simple
+ ubyte[] buf = nativeToLittleEndian (origin[idx]);
+ auto val = cast(inout(T))(buf.peek!U());
helps here, but has two problems:
- peek!U() apparently does not work in CTFE and subsequently all
compile-time unit tests fail.
- simpleIndex() is called in other places and also does the wrong thing
return cast(T)((origin[q] >> bits*r) & mask)
Refraining from peek!... I tried working around it by extracting bytes
and reversing the order but this seems to hacky to create a diff :)
I got it to work for test28.d:test39() but the unit tests still fail.
Is there a way to debug the compile-time unit tests easily? What's the
preferred method to do "the right thing" even at compile time? Any other
things that should be looked at? Any comments to the diffs so far?
Regards
Robin
diff --git a/libphobos/libdruntime/rt/aaA.d b/libphobos/libdruntime/rt/aaA.d
index 631847e4c2b..92f2c70f569 100644
--- a/libphobos/libdruntime/rt/aaA.d
+++ b/libphobos/libdruntime/rt/aaA.d
@@ -291,6 +291,8 @@ TypeInfo_Struct fakeEntryTI(const TypeInfo keyti, const TypeInfo valti)
private size_t talign(size_t tsize, size_t algn) @safe pure nothrow @nogc
{
+if (algn == 0)
+return tsize;
immutable mask = algn - 1;
assert(!(mask & algn));
return (tsize + mask) & ~mask;
diff --git a/gcc/d/dmd/constfold.c b/gcc/d/dmd/constfold.c
index ddd356bb966..4138563d599 100644
--- a/gcc/d/dmd/constfold.c
+++ b/gcc/d/dmd/constfold.c
@@ -1752,14 +1752,17 @@ UnionExp Cat(Type *type, Expression *e1, Expression *e2)
}
else if (e1->op == TOKint64 && e2->op == TOKstring)
{
-// Concatenate the strings
+// [w|d]?char ~ string --> string
+ // We assume that we only ever prepend one char of the same type
+ // (wchar,dchar) as the string's characters.
+
StringExp *es2 = (StringExp *)e2;
size_t len = 1 + es2->len;
unsigned char sz = es2->sz;
dinteger_t v = e1->toInteger();
void *s = mem.xmalloc((len + 1) * sz);
-memcpy((char *)s, , sz);
+ Port::valcpy((char *)s, v, sz);
memcpy((char *)s + sz, es2->string, es2->len * sz);
// Add terminating 0
diff --git a/libphobos/configure.tgt b/libphobos/configure.tgt
index 0471bfd816b..4ea91c949d7 100644
--- a/libphobos/configure.tgt
+++ b/libphobos/configure.tgt
@@ -32,6 +32,8 @@ case "${target}" in
;;
x86_64-*-netbsd* | i?86-*-netbsd*)
;;
+ s390*-linux*)
+ ;;
*)
UNSUPPORTED=1
;;
diff --git a/libphobos/libdruntime/rt/sections_elf_shared.d b/libphobos/libdruntime/rt/sections_elf_shared.d
index d4e1ff07699..85e2b768a87 100644
--- a/libphobos/libdruntime/rt/sections_elf_shared.d
+++ b/libphobos/libdruntime/rt/sections_elf_shared.d
@@ -978,7 +978,10 @@ struct tls_index
}
}
+import gcc.builtins;
+
extern(C) void* __tls_get_addr(tls_index* ti) nothrow @nogc;
+extern(C) void* __tls_get_addr_internal(tls_index* ti) nothrow @nogc;
/* The dynamic thread vector (DTV) pointers may point 0x8000 past the start of
* each TLS block. This is at least true for PowerPC and Mips platforms.
@@ -1012,6 +1015,8 @@ else version (MIPS32)
enum TLS_DTV_OFFSET = 0x8000;
else version (MIPS64)
enum TLS_DTV_OFFSET = 0x8000;
+else version (SystemZ)
+enum TLS_DTV_OFFSET = 0x0;
else
static assert( false, "Platform not supported." );
@@ -1022,5 +1027,13 @@ void[] getTLSRange(size_t mod, size_t sz) nothrow @nogc
// base offset
auto ti = tls_index(mod, 0);
-return (__tls_get_addr()-TLS_DTV_OFFSET)[0 .. sz];
+
+version (SystemZ)
+ {
+ auto idx = cast(void *)__tls_get_addr_internal()
+ +
Re: [PATCH] Fix PR71598, aliasing between enums and compatible types
On March 15, 2019 3:39:16 PM GMT+01:00, Michael Matz wrote:
>Hi,
>
>On Fri, 15 Mar 2019, Richard Biener wrote:
>
>> The following is an attempt to fix PR71598 where C (and C++?) have an
>
>> implementation-defined compatible integer type for each enum and the
>> TBAA rules mandate that accesses using a compatible type are allowed.
>
>But different enums aren't compatible with each other (or are they?),
>while your fix simply gives enums the aliasing set of the underlying
>type,
>i.e. makes all of them alias with each other. That's quite
>conservative.
But that follows from the need to be able to access an enum as int and the
other way around. So both alias sets need to be subsets of each other which
means they need to be equal.
Richard.
>
>Ciao,
>Michael.
>
>>
>> The fix is applied to all C family frontends and the LTO frontend
>> but not Fortran, Ada or other languages.
>>
>> Bootstrap & regtest running on x86_64-unknown-linux-gnu.
>>
>> I've tried to cover most cases even those with -fshort-enums.
>>
>> OK for trunk?
>>
>> It's probably a regression to some ancient GCC that didn't
>> perform TBAA and given it's wrong-code a backport is probably
>> mandated - do you agree? (after a while with no reported issues,
>> of course)
>>
>> Thanks,
>> Richard.
>>
>> 2019-03-15 Richard Biener
>>
>> PR c/71598
>> * gimple.c: Include langhooks.h.
>> (gimple_get_alias_set): Treat enumeral types as the underlying
>> integer type.
>>
>> c-family/
>> * c-common.c (c_common_get_alias_set): Treat enumeral types
>> as the underlying integer type.
>>
>> * c-c++-common/torture/pr71598-1.c: New testcase.
>> * c-c++-common/torture/pr71598-2.c: Likewise.
>>
>> Index: gcc/gimple.c
>> ===
>> --- gcc/gimple.c (revision 269704)
>> +++ gcc/gimple.c (working copy)
>> @@ -44,6 +44,7 @@ along with GCC; see the file COPYING3.
>> #include "stringpool.h"
>> #include "attribs.h"
>> #include "asan.h"
>> +#include "langhooks.h"
>>
>>
>> /* All the tuples have their operand vector (if present) at the very
>bottom
>> @@ -2587,6 +2588,16 @@ gimple_get_alias_set (tree t)
>> return get_alias_set (t1);
>> }
>>
>> + /* Allow aliasing between enumeral types and the underlying
>> + integer type. This is required for C since those are
>> + compatible types. */
>> + else if (TREE_CODE (t) == ENUMERAL_TYPE)
>> +{
>> + tree t1 = lang_hooks.types.type_for_size (tree_to_uhwi
>(TYPE_SIZE (t)),
>> +false /* short-cut above */);
>> + return get_alias_set (t1);
>> +}
>> +
>>return -1;
>> }
>>
>> Index: gcc/c-family/c-common.c
>> ===
>> --- gcc/c-family/c-common.c (revision 269704)
>> +++ gcc/c-family/c-common.c (working copy)
>> @@ -3681,6 +3681,15 @@ c_common_get_alias_set (tree t)
>> return get_alias_set (t1);
>> }
>>
>> + /* Allow aliasing between enumeral types and the underlying
>> + integer type. This is required since those are compatible
>types. */
>> + else if (TREE_CODE (t) == ENUMERAL_TYPE)
>> +{
>> + tree t1 = c_common_type_for_size (tree_to_uhwi (TYPE_SIZE
>(t)),
>> +false /* short-cut above */);
>> + return get_alias_set (t1);
>> +}
>> +
>>return -1;
>> }
>> >
>> Index: gcc/testsuite/c-c++-common/torture/pr71598-1.c
>> ===
>> --- gcc/testsuite/c-c++-common/torture/pr71598-1.c (nonexistent)
>> +++ gcc/testsuite/c-c++-common/torture/pr71598-1.c (working copy)
>> @@ -0,0 +1,21 @@
>> +/* { dg-do run } */
>> +/* { dg-additional-options "-fno-short-enums" } */
>> +
>> +enum e1 { c1 };
>> +
>> +__attribute__((noinline,noclone))
>> +int f(enum e1 *p, unsigned *q)
>> +{
>> + *p = c1;
>> + *q = 2;
>> + return *p;
>> +}
>> +
>> +int main()
>> +{
>> + unsigned x;
>> +
>> + if (f((enum e1 *), ) != 2)
>> +__builtin_abort();
>> + return 0;
>> +}
>> Index: gcc/testsuite/c-c++-common/torture/pr71598-2.c
>> ===
>> --- gcc/testsuite/c-c++-common/torture/pr71598-2.c (nonexistent)
>> +++ gcc/testsuite/c-c++-common/torture/pr71598-2.c (working copy)
>> @@ -0,0 +1,47 @@
>> +/* { dg-do run } */
>> +/* { dg-additional-options "-fshort-enums" } */
>> +
>> +enum e1 { c1 = -__INT_MAX__ };
>> +
>> +__attribute__((noinline,noclone))
>> +int f(enum e1 *p, signed int *q)
>> +{
>> + *p = c1;
>> + *q = 2;
>> + return *p;
>> +}
>> +
>> +enum e2 { c2 = __SHRT_MAX__ + 1};
>> +
>> +__attribute__((noinline,noclone))
>> +int g(enum e2 *p, unsigned short *q)
>> +{
>> + *p = c2;
>> + *q = 2;
>> + return *p;
>> +}
>> +
>> +enum e3 { c3 = __SCHAR_MAX__ };
>> +
>> +__attribute__((noinline,noclone))
>> +int h(enum e3 *p, unsigned char *q)
>> +{
>> + *p =
Re: [PATCH] Fix PR71598, aliasing between enums and compatible types
Hi,
On Fri, 15 Mar 2019, Richard Biener wrote:
> The following is an attempt to fix PR71598 where C (and C++?) have an
> implementation-defined compatible integer type for each enum and the
> TBAA rules mandate that accesses using a compatible type are allowed.
But different enums aren't compatible with each other (or are they?),
while your fix simply gives enums the aliasing set of the underlying type,
i.e. makes all of them alias with each other. That's quite conservative.
Ciao,
Michael.
>
> The fix is applied to all C family frontends and the LTO frontend
> but not Fortran, Ada or other languages.
>
> Bootstrap & regtest running on x86_64-unknown-linux-gnu.
>
> I've tried to cover most cases even those with -fshort-enums.
>
> OK for trunk?
>
> It's probably a regression to some ancient GCC that didn't
> perform TBAA and given it's wrong-code a backport is probably
> mandated - do you agree? (after a while with no reported issues,
> of course)
>
> Thanks,
> Richard.
>
> 2019-03-15 Richard Biener
>
> PR c/71598
> * gimple.c: Include langhooks.h.
> (gimple_get_alias_set): Treat enumeral types as the underlying
> integer type.
>
> c-family/
> * c-common.c (c_common_get_alias_set): Treat enumeral types
> as the underlying integer type.
>
> * c-c++-common/torture/pr71598-1.c: New testcase.
> * c-c++-common/torture/pr71598-2.c: Likewise.
>
> Index: gcc/gimple.c
> ===
> --- gcc/gimple.c (revision 269704)
> +++ gcc/gimple.c (working copy)
> @@ -44,6 +44,7 @@ along with GCC; see the file COPYING3.
> #include "stringpool.h"
> #include "attribs.h"
> #include "asan.h"
> +#include "langhooks.h"
>
>
> /* All the tuples have their operand vector (if present) at the very bottom
> @@ -2587,6 +2588,16 @@ gimple_get_alias_set (tree t)
> return get_alias_set (t1);
> }
>
> + /* Allow aliasing between enumeral types and the underlying
> + integer type. This is required for C since those are
> + compatible types. */
> + else if (TREE_CODE (t) == ENUMERAL_TYPE)
> +{
> + tree t1 = lang_hooks.types.type_for_size (tree_to_uhwi (TYPE_SIZE (t)),
> + false /* short-cut above */);
> + return get_alias_set (t1);
> +}
> +
>return -1;
> }
>
> Index: gcc/c-family/c-common.c
> ===
> --- gcc/c-family/c-common.c (revision 269704)
> +++ gcc/c-family/c-common.c (working copy)
> @@ -3681,6 +3681,15 @@ c_common_get_alias_set (tree t)
> return get_alias_set (t1);
> }
>
> + /* Allow aliasing between enumeral types and the underlying
> + integer type. This is required since those are compatible types. */
> + else if (TREE_CODE (t) == ENUMERAL_TYPE)
> +{
> + tree t1 = c_common_type_for_size (tree_to_uhwi (TYPE_SIZE (t)),
> + false /* short-cut above */);
> + return get_alias_set (t1);
> +}
> +
>return -1;
> }
>
> Index: gcc/testsuite/c-c++-common/torture/pr71598-1.c
> ===
> --- gcc/testsuite/c-c++-common/torture/pr71598-1.c(nonexistent)
> +++ gcc/testsuite/c-c++-common/torture/pr71598-1.c(working copy)
> @@ -0,0 +1,21 @@
> +/* { dg-do run } */
> +/* { dg-additional-options "-fno-short-enums" } */
> +
> +enum e1 { c1 };
> +
> +__attribute__((noinline,noclone))
> +int f(enum e1 *p, unsigned *q)
> +{
> + *p = c1;
> + *q = 2;
> + return *p;
> +}
> +
> +int main()
> +{
> + unsigned x;
> +
> + if (f((enum e1 *), ) != 2)
> +__builtin_abort();
> + return 0;
> +}
> Index: gcc/testsuite/c-c++-common/torture/pr71598-2.c
> ===
> --- gcc/testsuite/c-c++-common/torture/pr71598-2.c(nonexistent)
> +++ gcc/testsuite/c-c++-common/torture/pr71598-2.c(working copy)
> @@ -0,0 +1,47 @@
> +/* { dg-do run } */
> +/* { dg-additional-options "-fshort-enums" } */
> +
> +enum e1 { c1 = -__INT_MAX__ };
> +
> +__attribute__((noinline,noclone))
> +int f(enum e1 *p, signed int *q)
> +{
> + *p = c1;
> + *q = 2;
> + return *p;
> +}
> +
> +enum e2 { c2 = __SHRT_MAX__ + 1};
> +
> +__attribute__((noinline,noclone))
> +int g(enum e2 *p, unsigned short *q)
> +{
> + *p = c2;
> + *q = 2;
> + return *p;
> +}
> +
> +enum e3 { c3 = __SCHAR_MAX__ };
> +
> +__attribute__((noinline,noclone))
> +int h(enum e3 *p, unsigned char *q)
> +{
> + *p = c3;
> + *q = 2;
> + return *p;
> +}
> +
> +int main()
> +{
> + signed x;
> + unsigned short y;
> + unsigned char z;
> +
> + if (f((enum e1 *), ) != 2)
> +__builtin_abort();
> + if (g((enum e2 *), ) != 2)
> +__builtin_abort();
> + if (h((enum e3 *), ) != 2)
> +__builtin_abort();
> + return 0;
> +}
>
Re : add tsv110 pipeline scheduling
Hi , James:
Thank you very much for your meticulous review work. The explanation of
the two questions as follows:
The first problem is caused by my negligence and should be changed to "
crypto_sha256_fast" .
The second question I have verified with the hardware engineer. Only ALU2/ALU3
could support PSTATE register update so any instruction intends to update NZCV
will be issued to ALU2/ALU3. MDU could provide a better pipeline efficiency
for multi cycle ALU instruction so we issue 2 cycles ALU w/o PSTATE update to
MDU unit. the current pipeline processing is ok , except the pipeline "
tsv110_alu2" should replace with " tsv110_alu2| tsv110_alu3".
The detailed patches are as follows:
* config/aarch64/aarch64-cores.def (tsv1100): Change scheduling model.
* config/aarch64/aarch64.md : Add "tsv110.md"
* config/aarch64/tsv110.md: New file.
diff --git a/gcc/config/aarch64/aarch64-cores.def
b/gcc/config/aarch64/aarch64-cores.def
index ed56e5e..82d91d6
--- a/gcc/config/aarch64/aarch64-cores.def
+++ b/gcc/config/aarch64/aarch64-cores.def
@@ -105,7 +105,7 @@ AARCH64_CORE("neoverse-n1", neoversen1, cortexa57, 8_2A,
AARCH64_FL_FOR_ARCH8_
AARCH64_CORE("neoverse-e1", neoversee1, cortexa53, 8_2A,
AARCH64_FL_FOR_ARCH8_2 | AARCH64_FL_F16 | AARCH64_FL_RCPC | AARCH64_FL_DOTPROD
| AARCH64_FL_SSBS, cortexa53, 0x41, 0xd4a, -1)
/* HiSilicon ('H') cores. */
-AARCH64_CORE("tsv110", tsv110, cortexa57, 8_2A, AARCH64_FL_FOR_ARCH8_2 |
AARCH64_FL_CRYPTO | AARCH64_FL_F16 | AARCH64_FL_AES | AARCH64_FL_SHA2, tsv110,
0x48, 0xd01, -1)
+AARCH64_CORE("tsv110", tsv110, tsv110, 8_2A, AARCH64_FL_FOR_ARCH8_2 |
AARCH64_FL_CRYPTO | AARCH64_FL_F16 | AARCH64_FL_AES | AARCH64_FL_SHA2, tsv110,
0x48, 0xd01, -1)
/* ARMv8.4-A Architecture Processors. */
diff --git a/gcc/config/aarch64/aarch64.md b/gcc/config/aarch64/aarch64.md
index b7cd9fc..861f059 100644
--- a/gcc/config/aarch64/aarch64.md
+++ b/gcc/config/aarch64/aarch64.md
@@ -361,6 +361,7 @@
(include "thunderx.md")
(include "../arm/xgene1.md")
(include "thunderx2t99.md")
+(include "tsv110.md")
;; ---
;; Jumps and other miscellaneous insns
diff --git a/gcc/config/aarch64/tsv110.md b/gcc/config/aarch64/tsv110.md
new file mode 100644
index 000..9d12839
--- /dev/null
+++ b/gcc/config/aarch64/tsv110.md
@@ -0,0 +1,708 @@
+;; tsv110 pipeline description
+;; Copyright (C) 2018 Free Software Foundation, Inc.
+;;
+;; This file is part of GCC.
+;;
+;; GCC is free software; you can redistribute it and/or modify it
+;; under the terms of the GNU General Public License as published by
+;; the Free Software Foundation; either version 3, or (at your option)
+;; any later version.
+;;
+;; GCC is distributed in the hope that it will be useful, but
+;; WITHOUT ANY WARRANTY; without even the implied warranty of
+;; MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
+;; General Public License for more details.
+;;
+;; You should have received a copy of the GNU General Public License
+;; along with GCC; see the file COPYING3. If not see
+;;
Re: [C++ debug PATCH] [PR88534] accept VAR_DECL in class literal template parms
On Mar 14, 2019, Jason Merrill wrote:
>> You can use VAR_P for this.
> OK with that change.
Thanks, I went ahead and also added a test before dereferencing it,
since there was evidence shortly thereafter that it could possibly be
NULL.
Here's what I'm installing.
P0732R2 / C++ 2a introduce class literals as template parameters. The
front-end uses VAR_DECLs constructed from such literals to bind the
template PARM_DECLs, but dwarf2out.c used to reject such VAR_DECLs.
Taking DECL_INITIAL from such VAR_DECLs enables the generation of
DW_AT_const_value for them, at least when the class literal can
actually be represented as such.
for gcc/ChangeLog
PR c++/88534
PR c++/88537
* dwarf2out.c (generic_parameter_die): Follow DECL_INITIAL of
VAR_DECL args.
for gcc/ChangeLog
PR c++/88534
PR c++/88537
* g++.dg/cpp2a/pr88534.C: New.
* g++.dg/cpp2a/pr88537.C: New.
---
gcc/dwarf2out.c |7
gcc/testsuite/g++.dg/cpp2a/pr88534.C | 65 ++
gcc/testsuite/g++.dg/cpp2a/pr88537.C | 16
3 files changed, 88 insertions(+)
create mode 100644 gcc/testsuite/g++.dg/cpp2a/pr88534.C
create mode 100644 gcc/testsuite/g++.dg/cpp2a/pr88537.C
diff --git a/gcc/dwarf2out.c b/gcc/dwarf2out.c
index d9cefd3e1d3cf..251fff7b9ae96 100644
--- a/gcc/dwarf2out.c
+++ b/gcc/dwarf2out.c
@@ -13603,6 +13603,13 @@ generic_parameter_die (tree parm, tree arg,
dw_die_ref tmpl_die = NULL;
const char *name = NULL;
+ /* C++2a accepts class literals as template parameters, and var
+ decls with initializers represent them. The VAR_DECLs would be
+ rejected, but we can take the DECL_INITIAL constructor and
+ attempt to expand it. */
+ if (arg && VAR_P (arg))
+arg = DECL_INITIAL (arg);
+
if (!parm || !DECL_NAME (parm) || !arg)
return NULL;
diff --git a/gcc/testsuite/g++.dg/cpp2a/pr88534.C
b/gcc/testsuite/g++.dg/cpp2a/pr88534.C
new file mode 100644
index 0..54faf385f11aa
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp2a/pr88534.C
@@ -0,0 +1,65 @@
+// { dg-do compile { target c++2a } }
+// { dg-options "-g" }
+
+typedef __SIZE_TYPE__ size_t;
+
+namespace std
+{
+
+template
+struct integer_sequence
+{
+ typedef T value_type;
+ static constexpr size_t size () noexcept { return sizeof...(I); }
+};
+
+template
+using make_integer_sequence = integer_sequence;
+
+template
+using index_sequence = integer_sequence;
+
+template
+using make_index_sequence = make_integer_sequence;
+}
+
+template struct S
+{
+ T content[N];
+ using char_type = T;
+ template
+ constexpr S (const T ()[N], std::index_sequence) noexcept :
content{input[I]...} { }
+ constexpr S (const T ()[N]) noexcept : S (input,
std::make_index_sequence ()) { }
+ constexpr size_t size () const noexcept
+ {
+if (content[N - 1] == '\0')
+ return N - 1;
+else
+ return N;
+ }
+ constexpr T operator[] (size_t i) const noexcept
+ {
+return content[i];
+ }
+ constexpr const T *begin () const noexcept
+ {
+return content;
+ }
+ constexpr const T *end () const noexcept
+ {
+return content + size ();
+ }
+};
+
+template S (const T (&)[N]) -> S;
+
+template
+struct F
+{
+};
+
+auto
+foo ()
+{
+ F<"test"> f;
+}
diff --git a/gcc/testsuite/g++.dg/cpp2a/pr88537.C
b/gcc/testsuite/g++.dg/cpp2a/pr88537.C
new file mode 100644
index 0..d558d45f57830
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp2a/pr88537.C
@@ -0,0 +1,16 @@
+// { dg-do compile { target c++2a } }
+// { dg-options "-g" }
+
+struct pair {
+ unsigned a;
+ unsigned b;
+ constexpr pair(unsigned _a, unsigned _b) noexcept: a{_a}, b{_b} { }
+};
+
+template void fnc() {
+
+}
+
+void f() {
+fnc();
+}
--
Alexandre Oliva, freedom fighter https://FSFLA.org/blogs/lxo
Be the change, be Free! FSF Latin America board member
GNU Toolchain EngineerFree Software Evangelist
Hay que enGNUrecerse, pero sin perder la terGNUra jamás-GNUChe
[PATCH, PR d/88990] Committed fix for ICE in get_symbol_decl
Hi,
The patch merges the D front-end implementation with dmd upstream 8d4c876c6.
Backports fix where the extern storage class flag was wrongly
propagated to function scope when starting the semantic pass on the
body.
Bootstrapped and regression tested on x86_64-linux-gnu.
Committed to trunk as r269708.
--
Iain
---
diff --git a/gcc/d/dmd/MERGE b/gcc/d/dmd/MERGE
index 5e4abe6f33f..230fd12db2b 100644
--- a/gcc/d/dmd/MERGE
+++ b/gcc/d/dmd/MERGE
@@ -1,4 +1,4 @@
-19b1454b5ca7b1036ea5fde197d91d4a7d05c0a5
+8d4c876c658608e8f6e653803c534a9e15618f57
The first line of this file holds the git revision number of the last
merge done from the dlang/dmd repository.
diff --git a/gcc/d/dmd/declaration.c b/gcc/d/dmd/declaration.c
index 6372e39f3f6..835c6aef831 100644
--- a/gcc/d/dmd/declaration.c
+++ b/gcc/d/dmd/declaration.c
@@ -2008,6 +2008,7 @@ bool VarDeclaration::isDataseg()
else if (storage_class & (STCstatic | STCextern | STCtls | STCgshared) ||
parent->isModule() || parent->isTemplateInstance() || parent->isNspace())
{
+assert(!isParameter() && !isResult());
isdataseg = 1; // It is in the DataSegment
}
}
diff --git a/gcc/d/dmd/func.c b/gcc/d/dmd/func.c
index 4b7c2233955..afba82aac7d 100644
--- a/gcc/d/dmd/func.c
+++ b/gcc/d/dmd/func.c
@@ -1437,7 +1437,7 @@ void FuncDeclaration::semantic3(Scope *sc)
sc2->sw = NULL;
sc2->fes = fes;
sc2->linkage = LINKd;
-sc2->stc &= ~(STCauto | STCscope | STCstatic | STCabstract |
+sc2->stc &= ~(STCauto | STCscope | STCstatic | STCextern | STCabstract |
STCdeprecated | STCoverride |
STC_TYPECTOR | STCfinal | STCtls | STCgshared | STCref | STCreturn |
STCproperty | STCnothrow | STCpure | STCsafe | STCtrusted | STCsystem);
diff --git a/gcc/testsuite/gdc.test/runnable/test19734.d b/gcc/testsuite/gdc.test/runnable/test19734.d
new file mode 100644
index 000..efa7da3b019
--- /dev/null
+++ b/gcc/testsuite/gdc.test/runnable/test19734.d
@@ -0,0 +1,38 @@
+// https://issues.dlang.org/show_bug.cgi?id=19734
+// REQUIRED_ARGS: -main
+
+class C19734
+{
+import core.stdc.stdarg;
+
+extern
+{
+// Invalid 'this' parameter because of applied 'extern' storage class.
+void testin(typeof(this) p)
+in { assert(this is p); }
+body
+{
+}
+
+// Undefined reference to __result.
+int testout()
+out { assert(__result == 2); }
+body
+{
+return 2;
+}
+
+// Undefined reference to var.
+int testlocal()
+{
+int var;
+return var + 2;
+}
+
+// Variable _argptr cannot have initializer.
+int testvarargs(...)
+{
+return 0;
+}
+}
+}
diff --git a/gcc/testsuite/gdc.test/runnable/test19735.d b/gcc/testsuite/gdc.test/runnable/test19735.d
new file mode 100644
index 000..8a1a5e7d871
--- /dev/null
+++ b/gcc/testsuite/gdc.test/runnable/test19735.d
@@ -0,0 +1,22 @@
+// https://issues.dlang.org/show_bug.cgi?id=19735
+
+extern int test1(int par)
+{
+int var = 42;
+return var + par;
+}
+
+extern
+{
+int test2(int par)
+{
+int var = 42;
+return var + par;
+}
+}
+
+void main()
+{
+assert(test1(1) == test2(1));
+}
+
[PATCH] Fix PR71598, aliasing between enums and compatible types
The following is an attempt to fix PR71598 where C (and C++?) have
an implementation-defined compatible integer type for each enum
and the TBAA rules mandate that accesses using a compatible type
are allowed.
The fix is applied to all C family frontends and the LTO frontend
but not Fortran, Ada or other languages.
Bootstrap & regtest running on x86_64-unknown-linux-gnu.
I've tried to cover most cases even those with -fshort-enums.
OK for trunk?
It's probably a regression to some ancient GCC that didn't
perform TBAA and given it's wrong-code a backport is probably
mandated - do you agree? (after a while with no reported issues,
of course)
Thanks,
Richard.
2019-03-15 Richard Biener
PR c/71598
* gimple.c: Include langhooks.h.
(gimple_get_alias_set): Treat enumeral types as the underlying
integer type.
c-family/
* c-common.c (c_common_get_alias_set): Treat enumeral types
as the underlying integer type.
* c-c++-common/torture/pr71598-1.c: New testcase.
* c-c++-common/torture/pr71598-2.c: Likewise.
Index: gcc/gimple.c
===
--- gcc/gimple.c(revision 269704)
+++ gcc/gimple.c(working copy)
@@ -44,6 +44,7 @@ along with GCC; see the file COPYING3.
#include "stringpool.h"
#include "attribs.h"
#include "asan.h"
+#include "langhooks.h"
/* All the tuples have their operand vector (if present) at the very bottom
@@ -2587,6 +2588,16 @@ gimple_get_alias_set (tree t)
return get_alias_set (t1);
}
+ /* Allow aliasing between enumeral types and the underlying
+ integer type. This is required for C since those are
+ compatible types. */
+ else if (TREE_CODE (t) == ENUMERAL_TYPE)
+{
+ tree t1 = lang_hooks.types.type_for_size (tree_to_uhwi (TYPE_SIZE (t)),
+ false /* short-cut above */);
+ return get_alias_set (t1);
+}
+
return -1;
}
Index: gcc/c-family/c-common.c
===
--- gcc/c-family/c-common.c (revision 269704)
+++ gcc/c-family/c-common.c (working copy)
@@ -3681,6 +3681,15 @@ c_common_get_alias_set (tree t)
return get_alias_set (t1);
}
+ /* Allow aliasing between enumeral types and the underlying
+ integer type. This is required since those are compatible types. */
+ else if (TREE_CODE (t) == ENUMERAL_TYPE)
+{
+ tree t1 = c_common_type_for_size (tree_to_uhwi (TYPE_SIZE (t)),
+ false /* short-cut above */);
+ return get_alias_set (t1);
+}
+
return -1;
}
Index: gcc/testsuite/c-c++-common/torture/pr71598-1.c
===
--- gcc/testsuite/c-c++-common/torture/pr71598-1.c (nonexistent)
+++ gcc/testsuite/c-c++-common/torture/pr71598-1.c (working copy)
@@ -0,0 +1,21 @@
+/* { dg-do run } */
+/* { dg-additional-options "-fno-short-enums" } */
+
+enum e1 { c1 };
+
+__attribute__((noinline,noclone))
+int f(enum e1 *p, unsigned *q)
+{
+ *p = c1;
+ *q = 2;
+ return *p;
+}
+
+int main()
+{
+ unsigned x;
+
+ if (f((enum e1 *), ) != 2)
+__builtin_abort();
+ return 0;
+}
Index: gcc/testsuite/c-c++-common/torture/pr71598-2.c
===
--- gcc/testsuite/c-c++-common/torture/pr71598-2.c (nonexistent)
+++ gcc/testsuite/c-c++-common/torture/pr71598-2.c (working copy)
@@ -0,0 +1,47 @@
+/* { dg-do run } */
+/* { dg-additional-options "-fshort-enums" } */
+
+enum e1 { c1 = -__INT_MAX__ };
+
+__attribute__((noinline,noclone))
+int f(enum e1 *p, signed int *q)
+{
+ *p = c1;
+ *q = 2;
+ return *p;
+}
+
+enum e2 { c2 = __SHRT_MAX__ + 1};
+
+__attribute__((noinline,noclone))
+int g(enum e2 *p, unsigned short *q)
+{
+ *p = c2;
+ *q = 2;
+ return *p;
+}
+
+enum e3 { c3 = __SCHAR_MAX__ };
+
+__attribute__((noinline,noclone))
+int h(enum e3 *p, unsigned char *q)
+{
+ *p = c3;
+ *q = 2;
+ return *p;
+}
+
+int main()
+{
+ signed x;
+ unsigned short y;
+ unsigned char z;
+
+ if (f((enum e1 *), ) != 2)
+__builtin_abort();
+ if (g((enum e2 *), ) != 2)
+__builtin_abort();
+ if (h((enum e3 *), ) != 2)
+__builtin_abort();
+ return 0;
+}
[C++ Patch] PR 85014 ("[7/8/9 Regression] internal compiler error: in lookup_base, at cp/search.c:185")
Hi,
another - rather long standing - error-recovery regression, where, in
some rather special circumstances, we end up passing the FUNCTION_DECL
representing the operator () of the lambda to maybe_dummy_object and
obviously we almost immediately crash. Not sure how much we want to dig
- but simply checking that context_for_name_lookup is actually returning
a type appears to work fine, the error_mark_node then propagates back to
cp_parser_late_parse_one_default_arg and so on. In the special
circumstances of the testcase, context_for_name_lookup finds
ANON_AGGR_TYPE_P set for the DECL_CONTEXT of 'b' and iterates to its
TYPE_CONTEXT which is the FUNCTION_DECL representing the operator () of
the lambda. This is because we just called check_tag_decl on that
anonymous type as part of calling shadow_tag on the abstract
declaration. Tested x86_64-linux.
Thanks, Paolo.
/cp
2019-03-15 Paolo Carlini
PR c++/85014
* semantics.c (finish_non_static_data_member): Check return value
of context_for_name_lookup and immediately return error_mark_node
if isn't a type.
/testsuite
2019-03-15 Paolo Carlini
PR c++/85014
* g++.dg/cpp0x/pr85014.C: New.
Index: cp/semantics.c
===
--- cp/semantics.c (revision 269693)
+++ cp/semantics.c (working copy)
@@ -1828,7 +1828,15 @@ finish_non_static_data_member (tree decl, tree obj
{
tree scope = qualifying_scope;
if (scope == NULL_TREE)
- scope = context_for_name_lookup (decl);
+ {
+ scope = context_for_name_lookup (decl);
+ if (!TYPE_P (scope))
+ {
+ /* Can happen during error recovery (c++/85014). */
+ gcc_assert (seen_error ());
+ return error_mark_node;
+ }
+ }
object = maybe_dummy_object (scope, NULL);
}
Index: testsuite/g++.dg/cpp0x/pr85014.C
===
--- testsuite/g++.dg/cpp0x/pr85014.C(nonexistent)
+++ testsuite/g++.dg/cpp0x/pr85014.C(working copy)
@@ -0,0 +1,10 @@
+// { dg-do compile { target c++11 } }
+// { dg-options "" }
+
+struct {
+ short a[__builtin_constant_p([] {
+struct {
+ int b = b;
+ }; // { dg-error "abstract declarator" }
+ })];
+}; // { dg-error "abstract declarator" }
Re: [PATCH] Fix PR87561, 416.gamess slowdown
> > A previous patch of mine correcting the vectorizer target cost model > to properly cost scalar FP ops vs. scalar INT ops regressed > 416.gamess by ~10% on all modern x86 archs. > > The following mitigates this in the cost modeling by noticing > the vectorized loop in question has all loads and stores performed > strided (built up from scalar loads/stores) and building upon > the pessimization of strided loads added last year. > > The first half is treating strided stores the same as strided > loads which may make sense (but the latency and dependence > arguments do not count here). Unfortunately that alone > doesn't make 416.gamess vectorization fail because we end up > with TYPE_VECTOR_SUBPARTS == 2 (AVX256 vectorization is rejected > due to cost reasons already). Now comes the second half > which is to push it over the edge, adjusting the previous > pessimization by multiplying with TYPE_VECTOR_SUBPARTS + 1 > instead of just TYPE_VECTOR_SUBPARTS which makes the biggest > difference for smaller vectors. > > I've benchmarked this on a Haswell machine with SPEC 2006 > confirming the regression is fixed and re-benchmarked > appearant regressions with 3 runs confirming that was noise > and we end up with maybe even a progression there > (see the bugzilla audit-trail for details). > > Bootstrapped and tested on x86_64-unknown-linux-gnu. > > OK for trunk? > > Note I'm going to apply as two revisions to allow bisection > between the two changes, first pushing pessimizing strided > stores and then adjusting the factor. > > Thanks, > Richard. > > 2019-03-15 Richard Biener > > PR target/87561 > * config/i386/i386.c (ix86_add_stmt_cost): Apply strided > load pessimization to stores as well. > * config/i386/i386.c (ix86_add_stmt_cost): Pessimize strided > loads and stores a bit more. Looks good to me. Store costs are even more iffy than other costs because they are not part of dependency chain,so I guess whatever seems to work best in practice is good. Honza
[PATCH] Fix PR87561, 416.gamess slowdown
A previous patch of mine correcting the vectorizer target cost model
to properly cost scalar FP ops vs. scalar INT ops regressed
416.gamess by ~10% on all modern x86 archs.
The following mitigates this in the cost modeling by noticing
the vectorized loop in question has all loads and stores performed
strided (built up from scalar loads/stores) and building upon
the pessimization of strided loads added last year.
The first half is treating strided stores the same as strided
loads which may make sense (but the latency and dependence
arguments do not count here). Unfortunately that alone
doesn't make 416.gamess vectorization fail because we end up
with TYPE_VECTOR_SUBPARTS == 2 (AVX256 vectorization is rejected
due to cost reasons already). Now comes the second half
which is to push it over the edge, adjusting the previous
pessimization by multiplying with TYPE_VECTOR_SUBPARTS + 1
instead of just TYPE_VECTOR_SUBPARTS which makes the biggest
difference for smaller vectors.
I've benchmarked this on a Haswell machine with SPEC 2006
confirming the regression is fixed and re-benchmarked
appearant regressions with 3 runs confirming that was noise
and we end up with maybe even a progression there
(see the bugzilla audit-trail for details).
Bootstrapped and tested on x86_64-unknown-linux-gnu.
OK for trunk?
Note I'm going to apply as two revisions to allow bisection
between the two changes, first pushing pessimizing strided
stores and then adjusting the factor.
Thanks,
Richard.
2019-03-15 Richard Biener
PR target/87561
* config/i386/i386.c (ix86_add_stmt_cost): Apply strided
load pessimization to stores as well.
* config/i386/i386.c (ix86_add_stmt_cost): Pessimize strided
loads and stores a bit more.
Index: gcc/config/i386/i386.c
===
--- gcc/config/i386/i386.c (revision 269683)
+++ gcc/config/i386/i386.c (working copy)
@@ -50534,14 +50534,15 @@ ix86_add_stmt_cost (void *data, int coun
latency and execution resources for the many scalar loads
(AGU and load ports). Try to account for this by scaling the
construction cost by the number of elements involved. */
- if (kind == vec_construct
+ if ((kind == vec_construct || kind == vec_to_scalar)
&& stmt_info
- && STMT_VINFO_TYPE (stmt_info) == load_vec_info_type
+ && (STMT_VINFO_TYPE (stmt_info) == load_vec_info_type
+ || STMT_VINFO_TYPE (stmt_info) == store_vec_info_type)
&& STMT_VINFO_MEMORY_ACCESS_TYPE (stmt_info) == VMAT_ELEMENTWISE
&& TREE_CODE (DR_STEP (STMT_VINFO_DATA_REF (stmt_info))) != INTEGER_CST)
{
stmt_cost = ix86_builtin_vectorization_cost (kind, vectype, misalign);
- stmt_cost *= TYPE_VECTOR_SUBPARTS (vectype);
+ stmt_cost *= (TYPE_VECTOR_SUBPARTS (vectype) + 1);
}
if (stmt_cost == -1)
stmt_cost = ix86_builtin_vectorization_cost (kind, vectype, misalign);
Re: [PATCH] S/390: Fix tests that expect unquoted option names
On 15.03.19 13:19, Robin Dapp wrote: > Hi, > > r269586 puts single quotes around option names. This patch fixes tests > that expect the old format. > > Regards > Robin > > --- > > gcc/testsuite/ChangeLog: > > 2019-03-15 Robin Dapp > > * gcc.target/s390/target-attribute/tattr-1.c (htm0): > -mhtm -> '-mhtm'. > * gcc.target/s390/target-attribute/tattr-2.c: Likewise. > * gcc.target/s390/target-attribute/tattr-3.c (vx0): > -mvx -> '-mvx'. > * gcc.target/s390/target-attribute/tattr-4.c: Likewise. > Ok. Thanks! Andreas
[PATCH] S/390: Fix tests that expect unquoted option names
Hi,
r269586 puts single quotes around option names. This patch fixes tests
that expect the old format.
Regards
Robin
---
gcc/testsuite/ChangeLog:
2019-03-15 Robin Dapp
* gcc.target/s390/target-attribute/tattr-1.c (htm0):
-mhtm -> '-mhtm'.
* gcc.target/s390/target-attribute/tattr-2.c: Likewise.
* gcc.target/s390/target-attribute/tattr-3.c (vx0):
-mvx -> '-mvx'.
* gcc.target/s390/target-attribute/tattr-4.c: Likewise.
diff --git a/gcc/testsuite/gcc.target/s390/target-attribute/tattr-1.c b/gcc/testsuite/gcc.target/s390/target-attribute/tattr-1.c
index 31643490540..ff573443d04 100644
--- a/gcc/testsuite/gcc.target/s390/target-attribute/tattr-1.c
+++ b/gcc/testsuite/gcc.target/s390/target-attribute/tattr-1.c
@@ -13,7 +13,7 @@ void htm1(void)
__attribute__ ((target("arch=z10")))
void htm0(void)
{
- __builtin_tend(); /* { dg-error "is not supported without -mhtm" } */
+ __builtin_tend(); /* { dg-error "is not supported without '-mhtm'" } */
}
void htmd(void)
diff --git a/gcc/testsuite/gcc.target/s390/target-attribute/tattr-2.c b/gcc/testsuite/gcc.target/s390/target-attribute/tattr-2.c
index f0d282f6a66..739c2eabc83 100644
--- a/gcc/testsuite/gcc.target/s390/target-attribute/tattr-2.c
+++ b/gcc/testsuite/gcc.target/s390/target-attribute/tattr-2.c
@@ -20,7 +20,7 @@ void p0(void)
#ifdef __HTM__
#error __HTM__ is defined
#endif
- __builtin_tend (); /* { dg-error "is not supported without -mhtm" } */
+ __builtin_tend (); /* { dg-error "is not supported without '-mhtm'" } */
}
#pragma GCC reset_options
@@ -39,7 +39,7 @@ void a0(void)
#ifdef __HTM__
#error __HTM__ is defined
#endif
- __builtin_tend (); /* { dg-error "is not supported without -mhtm" } */
+ __builtin_tend (); /* { dg-error "is not supported without '-mhtm'" } */
}
void htmd(void)
@@ -47,5 +47,5 @@ void htmd(void)
#ifdef __HTM__
#error __HTM__ is defined
#endif
- __builtin_tend (); /* { dg-error "is not supported without -mhtm" } */
+ __builtin_tend (); /* { dg-error "is not supported without '-mhtm'" } */
}
diff --git a/gcc/testsuite/gcc.target/s390/target-attribute/tattr-3.c b/gcc/testsuite/gcc.target/s390/target-attribute/tattr-3.c
index 1af2274120d..769690e942c 100644
--- a/gcc/testsuite/gcc.target/s390/target-attribute/tattr-3.c
+++ b/gcc/testsuite/gcc.target/s390/target-attribute/tattr-3.c
@@ -16,7 +16,7 @@ void vx1(void)
__attribute__ ((target("arch=z10")))
void vx0(void)
{
- __builtin_s390_vll ((unsigned int)0, (const void *)8); /* { dg-error "requires -mvx" } */
+ __builtin_s390_vll ((unsigned int)0, (const void *)8); /* { dg-error "requires '-mvx'" } */
}
void vxd(void)
diff --git a/gcc/testsuite/gcc.target/s390/target-attribute/tattr-4.c b/gcc/testsuite/gcc.target/s390/target-attribute/tattr-4.c
index c501eca2ca6..cd813d9a59b 100644
--- a/gcc/testsuite/gcc.target/s390/target-attribute/tattr-4.c
+++ b/gcc/testsuite/gcc.target/s390/target-attribute/tattr-4.c
@@ -24,7 +24,7 @@ void a0(void)
#ifdef __VEC__
#error __VEC__ is defined
#endif
- __builtin_s390_vll ((unsigned int)0, (const void *)8); /* { dg-error "requires -mvx" } */
+ __builtin_s390_vll ((unsigned int)0, (const void *)8); /* { dg-error "requires '-mvx'" } */
}
void d(void)
@@ -32,5 +32,5 @@ void d(void)
#ifdef __VEC__
#error __VEC__ is defined
#endif
- __builtin_s390_vll ((unsigned int)0, (const void *)8); /* { dg-error "requires -mvx" } */
+ __builtin_s390_vll ((unsigned int)0, (const void *)8); /* { dg-error "requires '-mvx'" } */
}
Re: Fix PR 86979
On Fri, Mar 15, 2019 at 02:27:35PM +0300, Alexander Monakov wrote: > On Fri, 15 Mar 2019, Jakub Jelinek wrote: > > > On Fri, Mar 15, 2019 at 01:25:57PM +0300, Andrey Belevantsev wrote: > > > As explained in the PR trail, we incorrectly update the availability sets > > > in the rare case of several successors and one of them having another > > > fence. Fixed as follows. Ok for trunk? > > > > > > Best, > > > Andrey > > > > > > 2019-03-15 Andrey Belevantsev > > > > > > PR middle-end/89676 > > > * sel-sched.c (compute_av_set_at_bb_end): When we have an ineligible > > > successor, > > > use NULL as its av set. > > > > Just formatting nits, will defer actual review to some scheduler maintainer. > > I can do this, it falls under sel-sched maintenance and Andrey showed to me > in person the algorithmic corner-case here, so: OK for trunk with formatting > fixed. Can you please update MAINTAINERS for you then (and Dmitry too)? In https://gcc.gnu.org/ml/gcc/2011-11/msg00264.html you are listed all 3, but only Abel has added himself to selective scheduling in MAINTAINERS in r181284. Jakub
Re: Fix PR 86979
On Fri, 15 Mar 2019, Jakub Jelinek wrote: > On Fri, Mar 15, 2019 at 01:25:57PM +0300, Andrey Belevantsev wrote: > > As explained in the PR trail, we incorrectly update the availability sets > > in the rare case of several successors and one of them having another > > fence. Fixed as follows. Ok for trunk? > > > > Best, > > Andrey > > > > 2019-03-15 Andrey Belevantsev > > > > PR middle-end/89676 > > * sel-sched.c (compute_av_set_at_bb_end): When we have an ineligible > > successor, > > use NULL as its av set. > > Just formatting nits, will defer actual review to some scheduler maintainer. I can do this, it falls under sel-sched maintenance and Andrey showed to me in person the algorithmic corner-case here, so: OK for trunk with formatting fixed. Thanks! Alexander
Re: Fix PR 86979
On Fri, Mar 15, 2019 at 01:25:57PM +0300, Andrey Belevantsev wrote:
> As explained in the PR trail, we incorrectly update the availability sets
> in the rare case of several successors and one of them having another
> fence. Fixed as follows. Ok for trunk?
>
> Best,
> Andrey
>
> 2019-03-15 Andrey Belevantsev
>
> PR middle-end/89676
> * sel-sched.c (compute_av_set_at_bb_end): When we have an ineligible
> successor,
> use NULL as its av set.
Just formatting nits, will defer actual review to some scheduler maintainer.
Too long line in ChangeLog above, "successor," should be on the next line
already together with the rest of the description.
> diff --git a/gcc/sel-sched.c b/gcc/sel-sched.c
> index 315f2c0c0ab..2053694b196 100644
> --- a/gcc/sel-sched.c
> +++ b/gcc/sel-sched.c
> @@ -2820,10 +2820,12 @@ compute_av_set_at_bb_end (insn_t insn, ilist_t p, int
> ws)
> FOR_EACH_VEC_ELT (sinfo->succs_ok, is, succ)
>{
> basic_block succ_bb = BLOCK_FOR_INSN (succ);
> + av_set_t av_succ = (is_ineligible_successor (succ, p)
> + ? NULL
> + : BB_AV_SET (succ_bb));
>
> gcc_assert (BB_LV_SET_VALID_P (succ_bb));
> -mark_unavailable_targets (av1, BB_AV_SET (succ_bb),
> - BB_LV_SET (succ_bb));
> +mark_unavailable_targets (av1, av_succ, BB_LV_SET (succ_bb));
The above line should use tab instead of 8 spaces.
Jakub
Fix PR 86979
Hello,
As explained in the PR trail, we incorrectly update the availability sets
in the rare case of several successors and one of them having another
fence. Fixed as follows. Ok for trunk?
Best,
Andrey
2019-03-15 Andrey Belevantsev
PR middle-end/89676
* sel-sched.c (compute_av_set_at_bb_end): When we have an ineligible
successor,
use NULL as its av set.
diff --git a/gcc/sel-sched.c b/gcc/sel-sched.c
index 315f2c0c0ab..2053694b196 100644
--- a/gcc/sel-sched.c
+++ b/gcc/sel-sched.c
@@ -2820,10 +2820,12 @@ compute_av_set_at_bb_end (insn_t insn, ilist_t p, int
ws)
FOR_EACH_VEC_ELT (sinfo->succs_ok, is, succ)
{
basic_block succ_bb = BLOCK_FOR_INSN (succ);
+ av_set_t av_succ = (is_ineligible_successor (succ, p)
+ ? NULL
+ : BB_AV_SET (succ_bb));
gcc_assert (BB_LV_SET_VALID_P (succ_bb));
-mark_unavailable_targets (av1, BB_AV_SET (succ_bb),
- BB_LV_SET (succ_bb));
+mark_unavailable_targets (av1, av_succ, BB_LV_SET (succ_bb));
}
/* Finally, check liveness restrictions on paths leaving the region. */
[PATCH][AArch64] PR target/89719 Adjust gcc.target/aarch64/spellcheck*.c tests
Hi all,
As of recently the -march,-mcpu,-mtune strings in the error messages are
now quoted.
This patch adjusts the testcases in gcc.target/aarch64/ that had started
failing due to that change.
Committing to trunk as obvious.
Thanks,
Kyrill
2019-03-15 Kyrylo Tkachov
PR target/89719
* gcc.target/aarch64/spellcheck_4.c: Adjust dg-error string.
* gcc.target/aarch64/spellcheck_5.c: Likewise.
* gcc.target/aarch64/spellcheck_6.c: Likewise.
diff --git a/gcc/testsuite/gcc.target/aarch64/spellcheck_4.c b/gcc/testsuite/gcc.target/aarch64/spellcheck_4.c
index 37c9d3c4d61607803641a42d24a135d28bf375bb..4e77e56219420310981e337f1ffe2b9e3c662bcf 100644
--- a/gcc/testsuite/gcc.target/aarch64/spellcheck_4.c
+++ b/gcc/testsuite/gcc.target/aarch64/spellcheck_4.c
@@ -7,5 +7,5 @@ foo ()
{
}
-/* { dg-error "unknown value 'armv8-a-typo' for -march" "" { target *-*-* } 0 } */
+/* { dg-error "unknown value 'armv8-a-typo' for '-march'" "" { target *-*-* } 0 } */
/* { dg-message "valid arguments are: \[^\n\r]*(; did you mean 'armv*'?)?" "" { target *-*-* } 0 } */
diff --git a/gcc/testsuite/gcc.target/aarch64/spellcheck_5.c b/gcc/testsuite/gcc.target/aarch64/spellcheck_5.c
index 8ec581f12822380f3c01200225ecf8339cec5161..bf93adb905acf11023e319db1b358f3117d5a123 100644
--- a/gcc/testsuite/gcc.target/aarch64/spellcheck_5.c
+++ b/gcc/testsuite/gcc.target/aarch64/spellcheck_5.c
@@ -7,5 +7,5 @@ foo ()
{
}
-/* { dg-error "unknown value 'cortex-a17' for -mcpu" "" { target *-*-* } 0 } */
+/* { dg-error "unknown value 'cortex-a17' for '-mcpu'" "" { target *-*-* } 0 } */
/* { dg-message "valid arguments are: \[^\n\r]*; did you mean 'cortex-a57'?" "" { target *-*-* } 0 } */
diff --git a/gcc/testsuite/gcc.target/aarch64/spellcheck_6.c b/gcc/testsuite/gcc.target/aarch64/spellcheck_6.c
index 8fa491b0ca4beae6c13aa90de84235495d8a9c33..2ab2bee535735ebf6b95a8542c2fcaed17d95772 100644
--- a/gcc/testsuite/gcc.target/aarch64/spellcheck_6.c
+++ b/gcc/testsuite/gcc.target/aarch64/spellcheck_6.c
@@ -7,5 +7,5 @@ foo ()
{
}
-/* { dg-error "unknown value 'cortex-a72-typo' for -mtune" "" { target *-*-* } 0 } */
+/* { dg-error "unknown value 'cortex-a72-typo' for '-mtune'" "" { target *-*-* } 0 } */
/* { dg-message "valid arguments are: \[^\n\r]*; did you mean 'cortex-a72'?" "" { target *-*-* } 0 } */
[PATCH] Fix gcc.dg/uninit-pred-8_b.c fallout on GCC 8 branch (PR89551)
The following backports support for --param logical-op-non-short-circuit
to the GCC 8 branch (but not all the testsuite adjustments) and adjusts
the gcc.dg/uninit-pred-8_b.c as was done on trunk for the PR89497 fix.
Boostrap / regtest running on x86_64-unknown-linux-gnu.
Richard.
2019-03-15 Richard Biener
Backport from mainline
2019-03-06 Richard Biener
PR testsuite/89551
* gcc.dg/uninit-pred-8_b.c: Force logical-op-non-short-circuit
the way that makes the testcase PASS.
2018-11-30 Jakub Jelinek
PR testsuite/85368
* params.def (PARAM_LOGICAL_OP_NON_SHORT_CIRCUIT): New param.
* tree-ssa-ifcombine.c (ifcombine_ifandif): If
--param logical-op-non-short-circuit is present, override
LOGICAL_OP_NON_SHORT_CIRCUIT value from the param.
* fold-const.c (fold_range_test, fold_truth_andor): Likewise.
Index: gcc/fold-const.c
===
--- gcc/fold-const.c(revision 269701)
+++ gcc/fold-const.c(working copy)
@@ -5515,12 +5515,15 @@ fold_range_test (location_t loc, enum tr
/* On machines where the branch cost is expensive, if this is a
short-circuited branch and the underlying object on both sides
is the same, make a non-short-circuit operation. */
- else if (LOGICAL_OP_NON_SHORT_CIRCUIT
- && !flag_sanitize_coverage
- && lhs != 0 && rhs != 0
- && (code == TRUTH_ANDIF_EXPR
- || code == TRUTH_ORIF_EXPR)
- && operand_equal_p (lhs, rhs, 0))
+ bool logical_op_non_short_circuit = LOGICAL_OP_NON_SHORT_CIRCUIT;
+ if (PARAM_VALUE (PARAM_LOGICAL_OP_NON_SHORT_CIRCUIT) != -1)
+logical_op_non_short_circuit
+ = PARAM_VALUE (PARAM_LOGICAL_OP_NON_SHORT_CIRCUIT);
+ if (logical_op_non_short_circuit
+ && !flag_sanitize_coverage
+ && lhs != 0 && rhs != 0
+ && (code == TRUTH_ANDIF_EXPR || code == TRUTH_ORIF_EXPR)
+ && operand_equal_p (lhs, rhs, 0))
{
/* If simple enough, just rewrite. Otherwise, make a SAVE_EXPR
unless we are at top level or LHS contains a PLACEHOLDER_EXPR, in
@@ -8165,7 +8168,11 @@ fold_truth_andor (location_t loc, enum t
if ((tem = fold_truth_andor_1 (loc, code, type, arg0, arg1)) != 0)
return tem;
- if (LOGICAL_OP_NON_SHORT_CIRCUIT
+ bool logical_op_non_short_circuit = LOGICAL_OP_NON_SHORT_CIRCUIT;
+ if (PARAM_VALUE (PARAM_LOGICAL_OP_NON_SHORT_CIRCUIT) != -1)
+logical_op_non_short_circuit
+ = PARAM_VALUE (PARAM_LOGICAL_OP_NON_SHORT_CIRCUIT);
+ if (logical_op_non_short_circuit
&& !flag_sanitize_coverage
&& (code == TRUTH_AND_EXPR
|| code == TRUTH_ANDIF_EXPR
Index: gcc/params.def
===
--- gcc/params.def (revision 269701)
+++ gcc/params.def (working copy)
@@ -1331,6 +1331,11 @@ DEFPARAM(PARAM_AVOID_FMA_MAX_BITS,
"Maximum number of bits for which we avoid creating FMAs.",
0, 0, 512)
+DEFPARAM(PARAM_LOGICAL_OP_NON_SHORT_CIRCUIT,
+"logical-op-non-short-circuit",
+"True if a non-short-circuit operation is optimal.",
+-1, -1, 1)
+
/*
Local variables:
Index: gcc/testsuite/gcc.dg/uninit-pred-8_b.c
===
--- gcc/testsuite/gcc.dg/uninit-pred-8_b.c (revision 269701)
+++ gcc/testsuite/gcc.dg/uninit-pred-8_b.c (working copy)
@@ -1,6 +1,7 @@
-
/* { dg-do compile } */
-/* { dg-options "-Wuninitialized -O2" } */
+/* ??? Jump threading makes a mess of the logical-op-non-short-circuit=0 case
+ so force it our way. */
+/* { dg-options "-Wuninitialized -O2 --param logical-op-non-short-circuit=1" }
*/
int g;
void bar();
Index: gcc/tree-ssa-ifcombine.c
===
--- gcc/tree-ssa-ifcombine.c(revision 269701)
+++ gcc/tree-ssa-ifcombine.c(working copy)
@@ -40,6 +40,7 @@ along with GCC; see the file COPYING3.
#include "gimplify-me.h"
#include "tree-cfg.h"
#include "tree-ssa.h"
+#include "params.h"
#ifndef LOGICAL_OP_NON_SHORT_CIRCUIT
#define LOGICAL_OP_NON_SHORT_CIRCUIT \
@@ -556,7 +557,11 @@ ifcombine_ifandif (basic_block inner_con
{
tree t1, t2;
gimple_stmt_iterator gsi;
- if (!LOGICAL_OP_NON_SHORT_CIRCUIT || flag_sanitize_coverage)
+ bool logical_op_non_short_circuit = LOGICAL_OP_NON_SHORT_CIRCUIT;
+ if (PARAM_VALUE (PARAM_LOGICAL_OP_NON_SHORT_CIRCUIT) != -1)
+ logical_op_non_short_circuit
+ = PARAM_VALUE (PARAM_LOGICAL_OP_NON_SHORT_CIRCUIT);
+ if (!logical_op_non_short_circuit || flag_sanitize_coverage)
return false;
/* Only do this optimization if the inner bb contains only the
conditional. */
if (!gsi_one_before_end_p (gsi_start_nondebug_after_labels_bb
(inner_cond_bb)))
Re: [PATCH] Fix PR89710
On Fri, 15 Mar 2019, Jakub Jelinek wrote: > On Fri, Mar 15, 2019 at 09:46:33AM +0100, Richard Biener wrote: > > On Fri, 15 Mar 2019, Jakub Jelinek wrote: > > > > > On Fri, Mar 15, 2019 at 09:22:26AM +0100, Richard Biener wrote: > > > > That said, I think we can go with my patch for GCC 9 and defer a more > > > > complete and elaborate solution to GCC 10 (where I'd still prefer > > > > sth simple). > > > > > > > > What do you think? > > > > > > Ok. gimple_purge_dead_abnormal_call_edges after all isn't that expensive, > > > it just walks over all successor edges of a bb. > > > Right now gimple_purge_all_dead_abnormal_call_edges is only called by > > > sccvn on specific bbs that do need ab cleanup, tree-inline.c calls > > > gimple_purge_dead_abnormal_call_edges only if it is inlining a call at the > > > end of a bb and tree-cfg.c calls it for const/pure calls. > > > > > > In that last case, I wonder if we actually shouldn't do following, because > > > it makes no sense to call it for each constant/pure call in a bb when all > > > we > > > care about is whether it is the last stmt that is a pure/const call. > > > > Sure. Though I think with us now having gimple_call_set_ctrl_altering > > execute_fixup_cfg can be stripped down considerably - possibly simply > > leaving most parts to CFG cleanup. The noreturn fixup is still > > required if a call becomes known to not return (CFG cleanup only > > looks at the last stmt), but gimple_purge_dead_abnormal_call_edges can > > be completely elided IMHO (just schedule CFG cleanup). > > The CFG cleanup never calls gimple_purge_dead_abnormal_call_edges though. Indeed. I guess it could do that where it calls gimple_purge_dead_eh_edges (cleanup_control_flow_bb). > Do we ever update gimple_call_ctrl_altering_p flag if something changes? Yes, CFG cleanup does this and if fixup_cfg needs to live it could do that as well (it calls fixup_noreturn_call which updates it for example). Richard. > Jakub > -- Richard Biener SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nuernberg)
Re: [PATCH] Strip location wrappers inside of inchash::add_expr (PR c++/89709)
On Thu, 14 Mar 2019, Jakub Jelinek wrote:
> Hi!
>
> As mentioned in the PR, r267272 added STRIP_ANY_LOCATION_WRAPPERS
> at the start of operand_equal_p, but has not updated inchash::add_expr
> which needs to follow what operand_equal_p does, so that if two trees
> compare equal, they get the same hash value.
> For location wrappers at the outermost level we wouldn't even try to
> verify it, as they would be stripped before checking the hashes,
> and in the usual case it wouldn't make a difference because if not
> OEP_ADDRESS_OF STRIP_NOPS strips also the location wrappers.
> But if OEP_ADDRESS_OF is set and we have e.g. a location wrapper in one
> of the operands and not in the other one (the ICE is on
> >, field>>>
> vs.
> , field>>>
> which compares equal but has different hashes), then we need to make sure
> they have the same hash.
>
> The following patch fixes that and also moves the operand_equal_p location
> wrapper stripping after the hash value computation code to check the hashes
> in all cases.
>
> Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
OK.
Richard.
> 2019-03-14 Jakub Jelinek
>
> PR c++/89709
> * tree.c (inchash::add_expr): Strip any location wrappers.
> * fold-const.c (operand_equal_p): Move stripping of location wrapper
> after hash verification.
>
> * g++.dg/cpp0x/constexpr-89709.C: New test.
>
> --- gcc/tree.c.jj 2019-03-11 22:56:45.511835239 +0100
> +++ gcc/tree.c2019-03-14 11:39:55.898838708 +0100
> @@ -7743,6 +7743,8 @@ add_expr (const_tree t, inchash::hash
>return;
> }
>
> + STRIP_ANY_LOCATION_WRAPPER (t);
> +
>if (!(flags & OEP_ADDRESS_OF))
> STRIP_NOPS (t);
>
> --- gcc/fold-const.c.jj 2019-03-05 10:03:09.818780210 +0100
> +++ gcc/fold-const.c 2019-03-14 11:47:12.117791365 +0100
> @@ -2942,9 +2942,6 @@ combine_comparisons (location_t loc,
> int
> operand_equal_p (const_tree arg0, const_tree arg1, unsigned int flags)
> {
> - STRIP_ANY_LOCATION_WRAPPER (arg0);
> - STRIP_ANY_LOCATION_WRAPPER (arg1);
> -
>/* When checking, verify at the outermost operand_equal_p call that
> if operand_equal_p returns non-zero then ARG0 and ARG1 has the same
> hash value. */
> @@ -2967,6 +2964,9 @@ operand_equal_p (const_tree arg0, const_
> return 0;
> }
>
> + STRIP_ANY_LOCATION_WRAPPER (arg0);
> + STRIP_ANY_LOCATION_WRAPPER (arg1);
> +
>/* If either is ERROR_MARK, they aren't equal. */
>if (TREE_CODE (arg0) == ERROR_MARK || TREE_CODE (arg1) == ERROR_MARK
>|| TREE_TYPE (arg0) == error_mark_node
> --- gcc/testsuite/g++.dg/cpp0x/constexpr-89709.C.jj 2019-03-14
> 11:41:28.270346813 +0100
> +++ gcc/testsuite/g++.dg/cpp0x/constexpr-89709.C 2019-03-14
> 11:40:59.161816938 +0100
> @@ -0,0 +1,18 @@
> +// PR c++/89709
> +// { dg-do compile { target c++11 } }
> +// { dg-options "-O" }
> +
> +struct A { int i; };
> +A a;
> +
> +constexpr int *
> +foo ()
> +{
> + return
> +}
> +
> +bool
> +bar ()
> +{
> + return foo () ==
> +}
>
>
> Jakub
>
--
Richard Biener
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB
21284 (AG Nuernberg)
Re: [PATCH] Fix PR89710
On Fri, Mar 15, 2019 at 09:46:33AM +0100, Richard Biener wrote: > On Fri, 15 Mar 2019, Jakub Jelinek wrote: > > > On Fri, Mar 15, 2019 at 09:22:26AM +0100, Richard Biener wrote: > > > That said, I think we can go with my patch for GCC 9 and defer a more > > > complete and elaborate solution to GCC 10 (where I'd still prefer > > > sth simple). > > > > > > What do you think? > > > > Ok. gimple_purge_dead_abnormal_call_edges after all isn't that expensive, > > it just walks over all successor edges of a bb. > > Right now gimple_purge_all_dead_abnormal_call_edges is only called by > > sccvn on specific bbs that do need ab cleanup, tree-inline.c calls > > gimple_purge_dead_abnormal_call_edges only if it is inlining a call at the > > end of a bb and tree-cfg.c calls it for const/pure calls. > > > > In that last case, I wonder if we actually shouldn't do following, because > > it makes no sense to call it for each constant/pure call in a bb when all we > > care about is whether it is the last stmt that is a pure/const call. > > Sure. Though I think with us now having gimple_call_set_ctrl_altering > execute_fixup_cfg can be stripped down considerably - possibly simply > leaving most parts to CFG cleanup. The noreturn fixup is still > required if a call becomes known to not return (CFG cleanup only > looks at the last stmt), but gimple_purge_dead_abnormal_call_edges can > be completely elided IMHO (just schedule CFG cleanup). The CFG cleanup never calls gimple_purge_dead_abnormal_call_edges though. Do we ever update gimple_call_ctrl_altering_p flag if something changes? Jakub
Re: [PATCH] Fix PR89710
On Fri, 15 Mar 2019, Jakub Jelinek wrote:
> On Fri, Mar 15, 2019 at 09:22:26AM +0100, Richard Biener wrote:
> > That said, I think we can go with my patch for GCC 9 and defer a more
> > complete and elaborate solution to GCC 10 (where I'd still prefer
> > sth simple).
> >
> > What do you think?
>
> Ok. gimple_purge_dead_abnormal_call_edges after all isn't that expensive,
> it just walks over all successor edges of a bb.
> Right now gimple_purge_all_dead_abnormal_call_edges is only called by
> sccvn on specific bbs that do need ab cleanup, tree-inline.c calls
> gimple_purge_dead_abnormal_call_edges only if it is inlining a call at the
> end of a bb and tree-cfg.c calls it for const/pure calls.
>
> In that last case, I wonder if we actually shouldn't do following, because
> it makes no sense to call it for each constant/pure call in a bb when all we
> care about is whether it is the last stmt that is a pure/const call.
Sure. Though I think with us now having gimple_call_set_ctrl_altering
execute_fixup_cfg can be stripped down considerably - possibly simply
leaving most parts to CFG cleanup. The noreturn fixup is still
required if a call becomes known to not return (CFG cleanup only
looks at the last stmt), but gimple_purge_dead_abnormal_call_edges can
be completely elided IMHO (just schedule CFG cleanup).
Richard.
> --- gcc/tree-cfg.c.jj 2019-03-14 23:44:27.861560155 +0100
> +++ gcc/tree-cfg.c2019-03-15 09:37:15.667785016 +0100
> @@ -9483,7 +9483,8 @@ execute_fixup_cfg (void)
> int flags = gimple_call_flags (stmt);
> if (flags & (ECF_CONST | ECF_PURE | ECF_LOOPING_CONST_OR_PURE))
> {
> - if (gimple_purge_dead_abnormal_call_edges (bb))
> + if (gsi_one_before_end_p (gsi)
> + && gimple_purge_dead_abnormal_call_edges (bb))
> todo |= TODO_cleanup_cfg;
>
> if (gimple_in_ssa_p (cfun))
>
>
> Jakub
>
--
Richard Biener
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB
21284 (AG Nuernberg)
Re: [PATCH] Fix PR89710
On Fri, Mar 15, 2019 at 09:22:26AM +0100, Richard Biener wrote:
> That said, I think we can go with my patch for GCC 9 and defer a more
> complete and elaborate solution to GCC 10 (where I'd still prefer
> sth simple).
>
> What do you think?
Ok. gimple_purge_dead_abnormal_call_edges after all isn't that expensive,
it just walks over all successor edges of a bb.
Right now gimple_purge_all_dead_abnormal_call_edges is only called by
sccvn on specific bbs that do need ab cleanup, tree-inline.c calls
gimple_purge_dead_abnormal_call_edges only if it is inlining a call at the
end of a bb and tree-cfg.c calls it for const/pure calls.
In that last case, I wonder if we actually shouldn't do following, because
it makes no sense to call it for each constant/pure call in a bb when all we
care about is whether it is the last stmt that is a pure/const call.
--- gcc/tree-cfg.c.jj 2019-03-14 23:44:27.861560155 +0100
+++ gcc/tree-cfg.c 2019-03-15 09:37:15.667785016 +0100
@@ -9483,7 +9483,8 @@ execute_fixup_cfg (void)
int flags = gimple_call_flags (stmt);
if (flags & (ECF_CONST | ECF_PURE | ECF_LOOPING_CONST_OR_PURE))
{
- if (gimple_purge_dead_abnormal_call_edges (bb))
+ if (gsi_one_before_end_p (gsi)
+ && gimple_purge_dead_abnormal_call_edges (bb))
todo |= TODO_cleanup_cfg;
if (gimple_in_ssa_p (cfun))
Jakub
Re: GCC 8 backports
Hi.
One more documentation patch.
Martin
>From 12a1a2bf98075d79d60189d3d3b4d6c3de79a877 Mon Sep 17 00:00:00 2001
From: marxin
Date: Thu, 14 Mar 2019 14:19:33 +
Subject: Backport r269684
gcc/ChangeLog:
2019-03-14 Martin Liska
PR other/89712
* doc/invoke.texi: Remove -fdump-class-hierarchy option.
---
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index df0883f2fc9..0a941519dbc 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -583,7 +583,6 @@ Objective-C and Objective-C++ Dialects}.
-fdisable-tree-@var{pass-name}=@var{range-list} @gol
-fdump-debug -fdump-earlydebug @gol
-fdump-noaddr -fdump-unnumbered -fdump-unnumbered-links @gol
--fdump-class-hierarchy@r{[}-@var{n}@r{]} @gol
-fdump-final-insns@r{[}=@var{file}@r{]} @gol
-fdump-ipa-all -fdump-ipa-cgraph -fdump-ipa-inline @gol
-fdump-lang-all @gol
--
2.21.0
Re: [PATCH] Fix PR89710
On Thu, 14 Mar 2019, Jakub Jelinek wrote:
> On Thu, Mar 14, 2019 at 03:10:59PM +0100, Richard Biener wrote:
> > I've added a testcase.
> >
> > Bootstrapped and tested on x86_64-unknown-linux-gnu, OK for trunk
> > or should it wait for GCC10?
>
> I meant something like following where we'd clean it up everything right
> away after we DCE some returns_twice calls.
>
> Except that doesn't work well, because the temporary
> cfun->calls_setjmp = true;
> hack not only allows gimple_purge_dead_abnormal_call_edges through the
> condition your patch was removing, but also makes then
> call_can_make_abnormal_goto
> and stmt_can_make_abnormal_goto to return true. So, maybe on top of this
> patch add a bool force argument to gimple_purge_dead_abnormal_call_edges
> and gimple_purge_all_dead_abnormal_call_edges and instead of the temporary
> cfun->calls_setjmp = true;
Well, the check in gimple_purge_dead_abnormal_call_edges is simply
a (IMHO) premature optimization. Passes shouldn't call the function
w/o a good reason anyways. So my patch removing the guard there
solves this particular issue, no?
> in this patch pass true as force. Plus, if we lost the last returns_twice
> call and cfun->has_nonlocal_decl isn't set either, instead of calling
> gimple_purge_all_dead_abnormal_call_edges just on the selected bbs call
> gimple_purge_dead_abnormal_call_edges with force on on all the bbs
> because if there are no nonlocal labels and no cfun->calls_setjmp used to be
> set and is no longer set, then we don't need any AB edges from any calls?
Yes.
> --- gcc/tree-ssa-dce.c.jj 2019-01-10 11:43:17.044334047 +0100
> +++ gcc/tree-ssa-dce.c2019-03-14 17:35:47.158038514 +0100
> @@ -1201,6 +1201,8 @@ eliminate_unnecessary_stmts (void)
>gimple *stmt;
>tree call;
>vec h;
> + bool calls_setjmp = cfun->calls_setjmp;
> + bitmap need_ab_cleanup = NULL;
>
>if (dump_file && (dump_flags & TDF_DETAILS))
> fprintf (dump_file, "\nEliminating unnecessary statements:\n");
> @@ -1287,6 +1289,14 @@ eliminate_unnecessary_stmts (void)
> }
> if (!is_gimple_debug (stmt))
> something_changed = true;
> + if (calls_setjmp
> + && is_gimple_call (stmt)
> + && (gimple_call_flags (stmt) & ECF_RETURNS_TWICE))
> + {
> + if (need_ab_cleanup == NULL)
> + need_ab_cleanup = BITMAP_ALLOC (NULL);
> + bitmap_set_bit (need_ab_cleanup, bb->index);
> + }
This is equivalent to checking whether cfun->calls_setjmp is set
after this loop (and cheaper).
> remove_dead_stmt (, bb);
> }
> else if (is_gimple_call (stmt))
> @@ -1358,6 +1368,17 @@ eliminate_unnecessary_stmts (void)
>
>h.release ();
>
> + if (need_ab_cleanup)
> +{
> + bool saved_calls_setjmp = cfun->calls_setjmp;
> + cfun->calls_setjmp = true;
> + if (gimple_purge_all_dead_abnormal_call_edges (need_ab_cleanup))
> + cfg_altered = true;
also as you said it's incomplete since also functions !returns_twice
may no longer have the need for abnormal edges.
Note other passes have the very same issue (they just don't re-compute
->calls_setjmp). Eventually we can make cfg-cleanup re-compute that
for us given it looks at all BBs last stmt anyways (and only those
are relevant).
That said, I think we can go with my patch for GCC 9 and defer a more
complete and elaborate solution to GCC 10 (where I'd still prefer
sth simple).
What do you think?
Richard.
> + cfun->calls_setjmp = saved_calls_setjmp;
> +
> + BITMAP_FREE (need_ab_cleanup);
> +}
> +
>/* Since we don't track liveness of virtual PHI nodes, it is possible that
> we
> rendered some PHI nodes unreachable while they are still in use.
> Mark them for renaming. */
>
>
> Jakub
>
--
Richard Biener
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Graham Norton, HRB
21284 (AG Nuernberg)
