[PATCH 2/2] doc/implement-c.texi: About same-as-scalar-type volatile aggregate accesses, PR94600
We say very little about reads and writes to aggregate / compound objects, just scalar objects (i.e. assignments don't cause reads). Let's lets say something safe about aggregate objects, but only for those that are the same size as a scalar type. There's an equal-sounding section (Volatiles) in extend.texi, but this seems a more appropriate place, as specifying the behavior of a standard qualifier. gcc: PR middle-end/94600 * doc/implement-c.texi (Qualifiers implementation): Add blurb about access to the whole of a volatile aggregate object, only for same-size as scalar object. --- gcc/doc/implement-c.texi | 4 1 file changed, 4 insertions(+) diff --git a/gcc/doc/implement-c.texi b/gcc/doc/implement-c.texi index 692297b69c4..d64922b28ad 100644 --- a/gcc/doc/implement-c.texi +++ b/gcc/doc/implement-c.texi @@ -576,6 +576,10 @@ are of scalar types, the expression is interpreted by GCC as a read of the volatile object; in the other cases, the expression is only evaluated for its side effects. +When an object of aggregate type has the same size as a scalar type, GCC +handles an access to the whole of that volatile aggregate type object +equal to an access to that volatile same-sized scalar type object. + @end itemize @node Declarators implementation -- 2.11.0
Re: [PATCH 1/2] PR94600: fix volatile access to the whole of a compound object.
> From: Hans-Peter Nilsson > Date: Tue, 7 Jul 2020 06:01:37 +0200 > gcc: > PR middle-end/94600 > * expr.c (expand_constructor): Make a temporary also if we're > storing to volatile memory. Oops, I dropped attribution here, but this patch is by Richard Biener (modulo the comment, that's my fault). See the PR. brgds, H-P
[PATCH] rs6000: Refine RTL unroll adjust hook
For very small loops (< 6 insns), it would be fine to unroll 4
times to use cache line better. Like below loops:
`while (i) a[--i] = NULL; while (p < e) *d++ = *p++;`
And for very complex loops which may cause negative impacts:
branch-miss or cache-miss. Like below loop: there are calls,
early exits and branches in loop.
```
for (int i = 0; i < n; i++) {
int e = a[I];
if (function_call(e)) break;
}
```
This patch enhances RTL unroll for small loops and prevent to
unroll complex loops.
gcc/ChangeLog
2020-07-03 Jiufu Guo
* config/rs6000/rs6000.c (rs6000_loop_unroll_adjust): Refine hook.
(rs6000_complex_loop_p): New function.
(num_loop_calls): New function.
---
gcc/config/rs6000/rs6000.c | 46 +-
1 file changed, 40 insertions(+), 6 deletions(-)
diff --git a/gcc/config/rs6000/rs6000.c b/gcc/config/rs6000/rs6000.c
index 58f5d780603..a4874fa0efc 100644
--- a/gcc/config/rs6000/rs6000.c
+++ b/gcc/config/rs6000/rs6000.c
@@ -5130,22 +5130,56 @@ rs6000_destroy_cost_data (void *data)
free (data);
}
+/* Count the number of call insns in LOOP. */
+static unsigned int
+num_loop_calls (struct loop *loop)
+{
+ basic_block *bbs;
+ rtx_insn *insn;
+ unsigned int i;
+ unsigned int call_ins_num = 0;
+
+ bbs = get_loop_body (loop);
+ for (i = 0; i < loop->num_nodes; i++)
+FOR_BB_INSNS (bbs[i], insn)
+ if (CALL_P (insn))
+ call_ins_num++;
+
+ free (bbs);
+
+ return call_ins_num;
+}
+
+/* Return true if LOOP is too complex to be unrolled. */
+static bool
+rs6000_complex_loop_p (struct loop *loop)
+{
+ unsigned call_num;
+
+ return loop->ninsns > 10
+&& (call_num = num_loop_calls (loop)) > 0
+&& (call_num + num_loop_branches (loop)) * 5 > loop->ninsns
+&& !single_exit (loop);
+}
+
/* Implement targetm.loop_unroll_adjust. */
static unsigned
rs6000_loop_unroll_adjust (unsigned nunroll, struct loop *loop)
{
- if (unroll_only_small_loops)
+ if (unroll_only_small_loops)
{
- /* TODO: This is hardcoded to 10 right now. It can be refined, for
-example we may want to unroll very small loops more times (4 perhaps).
-We also should use a PARAM for this. */
+ if (loop->ninsns <= 6)
+ return MIN (4, nunroll);
if (loop->ninsns <= 10)
return MIN (2, nunroll);
- else
- return 0;
+
+ return 0;
}
+ if (rs6000_complex_loop_p (loop))
+return 0;
+
return nunroll;
}
--
2.25.1
Re: RFC: make combine do as advertised (cheaper-than)?
On Mon, Jul 6, 2020 at 4:03 AM Hans-Peter Nilsson via Gcc-patches
wrote:
>
> Most comments, including the second sentence in the head comment
> of combine_validate_cost, the main decision-maker of the combine
> pass, refer to the function as returning true if the new
> insns(s) *cheaper* than the old insns, when in fact the function
> returned true also if the cost was the same. Returning true for
> cheaper also seems more sane than as-cheap-as considering the
> need to avoid oscillation between same-cost combinations. Also,
> it makes the job of later passes harder, having combine make
> more complex combinations of the same cost.
>
> Right, you can affect this with your target TARGET_RTX_COSTS and
> TARGET_INSN_COST hooks, but only for trivial cases, and you have
> increasingly more complex combinations (many-to-many
> combinations) where you have to twist simple logic to appease
> combine (stop it from combining) or give up.
>
> Main-interest ports are unsurprisingly pretty tied to this
> effect. I'd love to install the following patch, adjusting the
> function and the two opposing comments. But...it causes
> hundreds of regressions for each of x86_64-linux and
> aarch64-linux (tens for ppc64le-linux), so I'm just not up to
> the task, at least not without previous buy-in from reviewers.
>
> It would need those targets to have their TARGET_INSN_COST
> and/or TARGET_RTX_COSTS functions adjusted.
>
> Alternatives from the top of my head, one of:
>
> - With buy-in from global reviewers, installing this patch on a
> development branch and let all target maintainers adjust their
> target test-cases and cost-functions there, for merge when
> first-class targets are done. (I'm a dreamer.)
>
> - A target combine hook for the decision (passing for inspection
> tuples of from-insns and to-insns and costs) and just falling
> back to the current addition of rtx costs.
>
> - A simpler target combine decision hook that says which one of
> "cheaper" or "as-cheap-as".
A target combine decision hook that on old == new cost
decides between both and given the actual insns? Might
lead to quite hackish target hook implementations...
> - Adjusting documentation and comments that are currently
> untruthful about the cost decision to instead say (to the effect
> of) "as cheap as" instead of "cheaper".
Well, adjusting the function comment to reflect reality is
kind-of obvious ;)
> So, WDYT?
>
> (Tested as above, causing massive pattern-match regressions.)
>
> gcc:
> * combine.c (combine_validate_cost): Reject unless the new total
> cost is cheaper than the original. Adjust the minority of
> comments that don't say "cheaper":
>
> ---
> gcc/combine.c | 8
> 1 file changed, 4 insertions(+), 4 deletions(-)
>
> diff --git a/gcc/combine.c b/gcc/combine.c
> index f69413a..7da144e 100644
> --- a/gcc/combine.c
> +++ b/gcc/combine.c
> @@ -846,8 +846,8 @@ do_SUBST_LINK (struct insn_link **into, struct insn_link
> *newval)
> than the original sequence I0, I1, I2, I3 and undobuf.other_insn. Note
> that I0, I1 and/or NEWI2PAT may be NULL_RTX. Similarly, NEWOTHERPAT and
> undobuf.other_insn may also both be NULL_RTX. Return false if the cost
> - of all the instructions can be estimated and the replacements are more
> - expensive than the original sequence. */
> + of all the instructions can be estimated and the replacements are not
> + cheaper than the original sequence. */
>
> static bool
> combine_validate_cost (rtx_insn *i0, rtx_insn *i1, rtx_insn *i2, rtx_insn
> *i3,
> @@ -938,8 +938,8 @@ combine_validate_cost (rtx_insn *i0, rtx_insn *i1,
> rtx_insn *i2, rtx_insn *i3,
> }
>
>/* Disallow this combination if both new_cost and old_cost are greater than
> - zero, and new_cost is greater than old cost. */
> - int reject = old_cost > 0 && new_cost > old_cost;
> + zero, and new_cost is greater than or equal to the old cost. */
> + int reject = old_cost > 0 && new_cost >= old_cost;
>
>if (dump_file)
> {
> --
> 2.11.0
RE: [PATCH PR94442] [AArch64] Redundant ldp/stp instructions emitted at -O3
> -Original Message-
> From: Richard Biener [mailto:richard.guent...@gmail.com]
> Sent: Thursday, July 2, 2020 10:46 PM
> To: xiezhiheng
> Cc: gcc-patches@gcc.gnu.org
> Subject: Re: [PATCH PR94442] [AArch64] Redundant ldp/stp instructions
> emitted at -O3
>
> On Thu, Jul 2, 2020 at 3:22 PM xiezhiheng wrote:
> >
> > Hi,
> >
> > This is a fix for pr94442.
> > I modify get_inner_reference to handle the case for MEM[ptr, off].
> > I extract the "off" and add it to the recorded offset, then I build a
> > MEM[ptr, 0] and return it later.
> >
> > diff --git a/gcc/expr.c b/gcc/expr.c
> > index 3c68b0d754c..8cc18449a0c 100644
> > --- a/gcc/expr.c
> > +++ b/gcc/expr.c
> > @@ -7362,7 +7362,8 @@ tree
> > get_inner_reference (tree exp, poly_int64_pod *pbitsize,
> > poly_int64_pod *pbitpos, tree *poffset,
> > machine_mode *pmode, int *punsignedp,
> > -int *preversep, int *pvolatilep)
> > +int *preversep, int *pvolatilep,
> > +bool include_memref_p)
> > {
> >tree size_tree = 0;
> >machine_mode mode = VOIDmode;
> > @@ -7509,6 +7510,21 @@ get_inner_reference (tree exp, poly_int64_pod
> *pbitsize,
> > }
> > exp = TREE_OPERAND (TREE_OPERAND (exp, 0), 0);
> > }
> > + else if (include_memref_p
> > + && TREE_CODE (TREE_OPERAND (exp, 0)) ==
> SSA_NAME)
> > + {
> > + tree off = TREE_OPERAND (exp, 1);
> > + if (!integer_zerop (off))
> > + {
> > + poly_offset_int boff = mem_ref_offset (exp);
> > + boff <<= LOG2_BITS_PER_UNIT;
> > + bit_offset += boff;
> > +
> > + exp = build2 (MEM_REF, TREE_TYPE (exp),
> > + TREE_OPERAND (exp, 0),
> > + build_int_cst (TREE_TYPE (off), 0));
> > + }
> > + }
> > goto done;
> >
> > default:
> > @@ -10786,7 +10802,7 @@ expand_expr_real_1 (tree exp, rtx target,
> machine_mode tmode,
> > int reversep, volatilep = 0, must_force_mem;
> > tree tem
> > = get_inner_reference (exp, &bitsize, &bitpos, &offset,
> &mode1,
> > -&unsignedp, &reversep, &volatilep);
> > +&unsignedp, &reversep, &volatilep,
> true);
> > rtx orig_op0, memloc;
> > bool clear_mem_expr = false;
> >
> > diff --git a/gcc/tree.h b/gcc/tree.h
> > index a74872f5f3e..7df0d15f7f9 100644
> > --- a/gcc/tree.h
> > +++ b/gcc/tree.h
> > @@ -6139,7 +6139,8 @@ extern bool complete_ctor_at_level_p
> (const_tree, HOST_WIDE_INT, const_tree);
> > look for the ultimate containing object, which is returned and specify
> > the access position and size. */
> > extern tree get_inner_reference (tree, poly_int64_pod *, poly_int64_pod
> *,
> > -tree *, machine_mode *, int *, int *,
> int *);
> > +tree *, machine_mode *, int *, int *,
> int *,
> > +bool = false);
> >
> > extern tree build_personality_function (const char *);
> >
> >
> > I add an argument "include_memref_p" to control whether to go into
> MEM_REF,
> > because without it will cause the test case "Warray-bounds-46.c" to fail in
> regression.
> >
> > It because function set_base_and_offset in gimple-ssa-warn-restrict.c
> > base = get_inner_reference (expr, &bitsize, &bitpos, &var_off,
> > &mode, &sign, &reverse, &vol);
> > ...
> > ...
> > if (TREE_CODE (base) == MEM_REF)
> > {
> > tree memrefoff = fold_convert (ptrdiff_type_node, TREE_OPERAND
> (base, 1));
> > extend_offset_range (memrefoff);
> > base = TREE_OPERAND (base, 0);
> >
> > if (refoff != HOST_WIDE_INT_MIN
> > && TREE_CODE (expr) == COMPONENT_REF)
> > {
> > /* Bump up the offset of the referenced subobject to reflect
> > the offset to the enclosing object. For example, so that
> > in
> >struct S { char a, b[3]; } s[2];
> >strcpy (s[1].b, "1234");
> > REFOFF is set to s[1].b - (char*)s. */
> > offset_int off = tree_to_shwi (memrefoff);
> > refoff += off;
> > }
> >
> > if (!integer_zerop (memrefoff)) <=
> > /* A non-zero offset into an array of struct with flexible array
> >members implies that the array is empty because there is no
> >way to initialize such a member when it belongs to an array.
> >This must be some sort of a bug. */
> > refsize = 0;
> > }
> >
> > needs MEM_REF offset to judge whether refsize should be set to zero.
> > But I fold the offset into bitpos and the offset will always be zero.
> >
> > Suggestion?
>
> The thing you want to fix is not get_i
Re: [PATCH PR94442] [AArch64] Redundant ldp/stp instructions emitted at -O3
xiezhiheng writes:
>> -Original Message-
>> From: Richard Biener [mailto:richard.guent...@gmail.com]
>> Sent: Thursday, July 2, 2020 10:46 PM
>> To: xiezhiheng
>> Cc: gcc-patches@gcc.gnu.org
>> Subject: Re: [PATCH PR94442] [AArch64] Redundant ldp/stp instructions
>> emitted at -O3
>>
>> On Thu, Jul 2, 2020 at 3:22 PM xiezhiheng wrote:
>> >
>> > Hi,
>> >
>> > This is a fix for pr94442.
>> > I modify get_inner_reference to handle the case for MEM[ptr, off].
>> > I extract the "off" and add it to the recorded offset, then I build a
>> > MEM[ptr, 0] and return it later.
>> >
>> > diff --git a/gcc/expr.c b/gcc/expr.c
>> > index 3c68b0d754c..8cc18449a0c 100644
>> > --- a/gcc/expr.c
>> > +++ b/gcc/expr.c
>> > @@ -7362,7 +7362,8 @@ tree
>> > get_inner_reference (tree exp, poly_int64_pod *pbitsize,
>> > poly_int64_pod *pbitpos, tree *poffset,
>> > machine_mode *pmode, int *punsignedp,
>> > -int *preversep, int *pvolatilep)
>> > +int *preversep, int *pvolatilep,
>> > +bool include_memref_p)
>> > {
>> >tree size_tree = 0;
>> >machine_mode mode = VOIDmode;
>> > @@ -7509,6 +7510,21 @@ get_inner_reference (tree exp, poly_int64_pod
>> *pbitsize,
>> > }
>> > exp = TREE_OPERAND (TREE_OPERAND (exp, 0), 0);
>> > }
>> > + else if (include_memref_p
>> > + && TREE_CODE (TREE_OPERAND (exp, 0)) ==
>> SSA_NAME)
>> > + {
>> > + tree off = TREE_OPERAND (exp, 1);
>> > + if (!integer_zerop (off))
>> > + {
>> > + poly_offset_int boff = mem_ref_offset (exp);
>> > + boff <<= LOG2_BITS_PER_UNIT;
>> > + bit_offset += boff;
>> > +
>> > + exp = build2 (MEM_REF, TREE_TYPE (exp),
>> > + TREE_OPERAND (exp, 0),
>> > + build_int_cst (TREE_TYPE (off), 0));
>> > + }
>> > + }
>> > goto done;
>> >
>> > default:
>> > @@ -10786,7 +10802,7 @@ expand_expr_real_1 (tree exp, rtx target,
>> machine_mode tmode,
>> > int reversep, volatilep = 0, must_force_mem;
>> > tree tem
>> > = get_inner_reference (exp, &bitsize, &bitpos, &offset,
>> &mode1,
>> > -&unsignedp, &reversep, &volatilep);
>> > +&unsignedp, &reversep, &volatilep,
>> true);
>> > rtx orig_op0, memloc;
>> > bool clear_mem_expr = false;
>> >
>> > diff --git a/gcc/tree.h b/gcc/tree.h
>> > index a74872f5f3e..7df0d15f7f9 100644
>> > --- a/gcc/tree.h
>> > +++ b/gcc/tree.h
>> > @@ -6139,7 +6139,8 @@ extern bool complete_ctor_at_level_p
>> (const_tree, HOST_WIDE_INT, const_tree);
>> > look for the ultimate containing object, which is returned and specify
>> > the access position and size. */
>> > extern tree get_inner_reference (tree, poly_int64_pod *, poly_int64_pod
>> *,
>> > -tree *, machine_mode *, int *, int *,
>> int *);
>> > +tree *, machine_mode *, int *, int *,
>> int *,
>> > +bool = false);
>> >
>> > extern tree build_personality_function (const char *);
>> >
>> >
>> > I add an argument "include_memref_p" to control whether to go into
>> MEM_REF,
>> > because without it will cause the test case "Warray-bounds-46.c" to fail in
>> regression.
>> >
>> > It because function set_base_and_offset in gimple-ssa-warn-restrict.c
>> > base = get_inner_reference (expr, &bitsize, &bitpos, &var_off,
>> > &mode, &sign, &reverse, &vol);
>> > ...
>> > ...
>> > if (TREE_CODE (base) == MEM_REF)
>> > {
>> > tree memrefoff = fold_convert (ptrdiff_type_node, TREE_OPERAND
>> (base, 1));
>> > extend_offset_range (memrefoff);
>> > base = TREE_OPERAND (base, 0);
>> >
>> > if (refoff != HOST_WIDE_INT_MIN
>> > && TREE_CODE (expr) == COMPONENT_REF)
>> > {
>> > /* Bump up the offset of the referenced subobject to reflect
>> > the offset to the enclosing object. For example, so that
>> > in
>> >struct S { char a, b[3]; } s[2];
>> >strcpy (s[1].b, "1234");
>> > REFOFF is set to s[1].b - (char*)s. */
>> > offset_int off = tree_to_shwi (memrefoff);
>> > refoff += off;
>> > }
>> >
>> > if (!integer_zerop (memrefoff)) <=
>> > /* A non-zero offset into an array of struct with flexible array
>> >members implies that the array is empty because there is no
>> >way to initialize such a member when it belongs to an array.
>> >This must be some sort of a bug. */
>> > refsize = 0;
>> > }
>> >
>> > needs MEM_REF offset to judge whether refsize should be set t
Re: RFC: make combine do as advertised (cheaper-than)?
Hans-Peter Nilsson via Gcc-patches writes: > Most comments, including the second sentence in the head comment > of combine_validate_cost, the main decision-maker of the combine > pass, refer to the function as returning true if the new > insns(s) *cheaper* than the old insns, when in fact the function > returned true also if the cost was the same. Returning true for > cheaper also seems more sane than as-cheap-as considering the > need to avoid oscillation between same-cost combinations. Also, > it makes the job of later passes harder, having combine make > more complex combinations of the same cost. > > Right, you can affect this with your target TARGET_RTX_COSTS and > TARGET_INSN_COST hooks, but only for trivial cases, and you have > increasingly more complex combinations (many-to-many > combinations) where you have to twist simple logic to appease > combine (stop it from combining) or give up. > > Main-interest ports are unsurprisingly pretty tied to this > effect. I'd love to install the following patch, adjusting the > function and the two opposing comments. But...it causes > hundreds of regressions for each of x86_64-linux and > aarch64-linux (tens for ppc64le-linux), so I'm just not up to > the task, at least not without previous buy-in from reviewers. Out of interest, how do the results change if we still allow the combination for equal costs if the new sequence is shorter than the original? I think that still counts as “cheaper than”, but maybe I'm too RISC-centric. ;-) (I'm not saying that's what we should do, I'm just curious.) Originally combine always produced shorter sequences, so by the above reasoning, combining for == was correct. These days we allow N-to-N replacements too, which are obviously a good thing when the new cost is lower, but are more of a wash when the costs are the same. But even then, the combination should have a “canonicalisation” effect. (Unfortunately that effect includes the result of expand_compound_operation/make_compound_operation.) Do you have specific examples of where things go wrong? Thanks, Richard
Re: [Patch 1/3] aarch64: New Straight Line Speculation (SLS) mitigation flags
Matthew Malcomson writes:
> + aarch64_sls_hardening = SLS_NONE;
> + if (strcmp (const_str, "none") == 0)
> +{
> + aarch64_sls_hardening = SLS_NONE;
> + return;
Gah, I totally misread the previous patch and didn't see that
you were already setting aarch64_sls_hardening to SLS_NONE above.
So this line is obviously redundant after all, sorry for the noise.
OK with the line above removed, thanks.
Richard
Re: [PATCH] analyzer: Fix -Wanalyzer-possible-null-argument warning
On 05/07/20 21:48 -0400, David Malcolm wrote: On Wed, 2020-07-01 at 18:29 +0100, Jonathan Wakely wrote: On 30/06/20 17:43 +0100, Jonathan Wakely wrote: > gcc/testsuite/ChangeLog: > >* g++.dg/analyzer/pr94028.C: Make operator new non-throwing so >that the compiler doesn't implicitly mark it as returning >non-null. > > Fixes these: > > FAIL: g++.dg/analyzer/pr94028.C -std=c++98 (test for excess > errors) > FAIL: g++.dg/analyzer/pr94028.C -std=c++14 (test for excess > errors) > FAIL: g++.dg/analyzer/pr94028.C -std=c++17 (test for excess > errors) > FAIL: g++.dg/analyzer/pr94028.C -std=c++2a (test for excess > errors) Updated to add PR 96014 to the commit log. OK for master? Sorry for not responding to this earlier. My knowledge of C++ exceptions is a little rusty; I found the addition of "throw()" to mark the decl as non-throwing to be confusing. An operator new is required to report allocation failure by throwing an exception that can be caught by `catch (const std::bad_alloc&)`, unless it is marked as non-throwing, in which case it reports allocation failure by returning a null pointer. I believe the C++ front end adds the returns_nonnull attribute to operator new unless it is marked non-throwing. The operator new in this test just calls calloc, which can return null, but it isn't marked as non-throwing so has the returns_nonnull attribute. Therefore it has undefined behaviour if calloc ever fails and returns null. Your analyzer seems to be noticing this, and so warning, which is nice. The way to fix it is to either check the return value of calloc and throw std::bad_alloc() if calloc returned null, or to simply mark the operator new as non-throwing so that returning null is OK. Looking in my copy of Stroustrup 4th edition (C++11) p367 it says this is an empty exception specification, and is equivalent to "noexcept", and Stroustrup recommends using the latter instead. Did you use this syntax for backwards compat with C++98, or is "noexcept" available in the earlier C++ dialects? noexcept is not valid in C++98/C++03. But I forgot that throw() is deprecated in C++17 and removed in C++20, so although G++ still accepts throw() to be futureproof we should use: #if __cplusplus < 201103L # define NOTHROW throw() #else # define NOTHROW noexcept #endif and then mark it NOTHROW. Although it would take fewer lines of code to just check what calloc returns and turn a null pointer into a std::bad_alloc exception.
Re: RFC: make combine do as advertised (cheaper-than)?
> From: Richard Sandiford > Date: Mon, 6 Jul 2020 11:48:25 +0200 > Out of interest, how do the results change if we still allow the > combination for equal costs if the new sequence is shorter than > the original? I think that still counts as "cheaper than", > but maybe I'm too RISC-centric. ;-) (I'm not saying that's > what we should do, I'm just curious.) Aha. I agree to the point I'd say it's a key question and we should (i.e. if the "speed" cost is the same, inspect again using size metrics, definitely reject if larger, though undecided if same). > Originally combine always produced shorter sequences, so by the > above reasoning, combining for == was correct. These days we allow > N-to-N replacements too, which are obviously a good thing when > the new cost is lower, but are more of a wash when the costs > are the same. But even then, the combination should have a > "canonicalisation" effect. (Unfortunately that effect includes > the result of expand_compound_operation/make_compound_operation.) > Do you have specific examples of where things go wrong? I initially thought I had one clear one, but it was an actual bug: https://gcc.gnu.org/pipermail/gcc-patches/2020-July/549412.html which I guess is a point in favor for keeping it same-cost, but on the other hand, that condition (!i2_was_move && !i3_was_move) (failing due to wrong value of i3_was_move) is an artificial measure that would not be needed if same-cost combinations were rejected. brgds, H-P
[Ada] AI12-0376 Relax RM 13.1(10) rule wrt primitive operations
After discussions at the ARG, the rule in RM 13.1(10) limiting
representation aspects on derived types is removed by AI12-0376.
It has also been confirmed that Default_Component_Value is a
representation aspect.
Tested on x86_64-pc-linux-gnu, committed on trunk
gcc/ada/
* aspects.ads (Is_Representation_Aspect):
Default_Component_Value is a representation aspect.
* sem_ch13.adb (Check_Aspect_Too_Late, Rep_Item_Too_Late): Relax
RM 13.1(10) rule wrt primitive operations for Ada 202x.diff --git a/gcc/ada/aspects.ads b/gcc/ada/aspects.ads
--- a/gcc/ada/aspects.ads
+++ b/gcc/ada/aspects.ads
@@ -454,7 +454,7 @@ package Aspects is
Aspect_Contract_Cases => False,
Aspect_Convention => True,
Aspect_CPU => False,
- Aspect_Default_Component_Value => False,
+ Aspect_Default_Component_Value => True,
Aspect_Default_Initial_Condition=> False,
Aspect_Default_Iterator => False,
Aspect_Default_Storage_Pool => True,
diff --git a/gcc/ada/sem_ch13.adb b/gcc/ada/sem_ch13.adb
--- a/gcc/ada/sem_ch13.adb
+++ b/gcc/ada/sem_ch13.adb
@@ -952,7 +952,6 @@ package body Sem_Ch13 is
procedure Check_Aspect_Too_Late (N : Node_Id) is
Typ : constant Entity_Id := Entity (N);
Expr : constant Node_Id := Expression (N);
- A_Id : constant Aspect_Id := Get_Aspect_Id (N);
function Find_Type_Reference
(Typ : Entity_Id; Expr : Node_Id) return Boolean;
@@ -995,42 +994,44 @@ package body Sem_Ch13 is
Parent_Type : Entity_Id;
begin
- if A_Id /= Aspect_Default_Value then
+ -- Ensure Expr is analyzed so that e.g. all types are properly
+ -- resolved for Find_Type_Reference.
--- Ensure Expr is analyzed so that e.g. all types are properly
--- resolved for Find_Type_Reference.
-
-Analyze (Expr);
+ Analyze (Expr);
--- A self-referential aspect is illegal if it forces freezing the
--- entity before the corresponding aspect has been analyzed.
+ -- A self-referential aspect is illegal if it forces freezing the
+ -- entity before the corresponding aspect has been analyzed.
-if Find_Type_Reference (Typ, Expr) then
- Error_Msg_NE
- ("aspect specification causes premature freezing of&",
- N, Typ);
-end if;
+ if Find_Type_Reference (Typ, Expr) then
+Error_Msg_NE
+ ("aspect specification causes premature freezing of&", N, Typ);
end if;
-- For representation aspects, check for case of untagged derived
- -- type whose parent either has primitive operations, or is a by
- -- reference type (RM 13.1(10)).
+ -- type whose parent either has primitive operations (pre Ada 202x),
+ -- or is a by-reference type (RM 13.1(10)).
+ -- Strictly speaking the check also applies to Ada 2012 but it is
+ -- really too constraining for existing code already, so relax it.
+ -- ??? Confirming aspects should be allowed here.
- if Is_Representation_Aspect (A_Id)
+ if Is_Representation_Aspect (Get_Aspect_Id (N))
and then Is_Derived_Type (Typ)
and then not Is_Tagged_Type (Typ)
then
Parent_Type := Etype (Base_Type (Typ));
-if Has_Primitive_Operations (Parent_Type) then
- No_Type_Rep_Item (N);
+if Ada_Version <= Ada_2012
+ and then Has_Primitive_Operations (Parent_Type)
+then
+ Error_Msg_N
+ ("|representation aspect not permitted before Ada 202x!", N);
Error_Msg_NE
("\parent type & has primitive operations!", N, Parent_Type);
elsif Is_By_Reference_Type (Parent_Type) then
No_Type_Rep_Item (N);
Error_Msg_NE
- ("\parent type & is a by reference type!", N, Parent_Type);
+ ("\parent type & is a by-reference type!", N, Parent_Type);
end if;
end if;
end Check_Aspect_Too_Late;
@@ -13868,9 +13869,11 @@ package body Sem_Ch13 is
return True;
-- Check for case of untagged derived type whose parent either has
- -- primitive operations, or is a by reference type (RM 13.1(10)). In
- -- this case we do not output a Too_Late message, since there is no
- -- earlier point where the rep item could be placed to make it legal.
+ -- primitive operations (pre Ada 202x), or is a by-reference type (RM
+ -- 13.1(10)). In this case we do not output a Too_Late message, since
+ -- there is no earlier point where the rep item could be placed to make
+ -- it legal.
+ -- ??? Confirming representation clause
[Ada] Wording problems with predicates (AI12-0099)
AI12-0099 corrects the RM wording for two issues. One is that the "not" operator is allowed in predicate-static expressions, which is already supported properly by GNAT. The other is that predicates get inherited in the case of concurrent types that extend from an interface with a predicate. For the second issue, GNAT was not handling the generation of the predicate function properly, and left out generation of the body because it wasn't inheriting a predicate from the ancestor type for a concurrent type that extends an interface. Tested on x86_64-pc-linux-gnu, committed on trunk gcc/ada/ * sem_aux.adb: Add a with clause for Nlists. (Nearest_Ancestor): Test for the case of concurrent types (testing for both Is_Concurrent_Type and Is_Concurrent_Record_Type), and return the first ancestor in the Interfaces list if present (otherwise will return Empty if no interfaces). * sem_ch13.adb (Build_Predicate_Functions): Add a ??? comment about missing handling for adding predicates when they can be inherited from multiple progenitors.diff --git a/gcc/ada/sem_aux.adb b/gcc/ada/sem_aux.adb --- a/gcc/ada/sem_aux.adb +++ b/gcc/ada/sem_aux.adb @@ -32,6 +32,7 @@ with Atree; use Atree; with Einfo; use Einfo; +with Nlists; use Nlists; with Snames; use Snames; with Stand; use Stand; with Uintp; use Uintp; @@ -1375,6 +1376,18 @@ package body Sem_Aux is end if; end; + -- If this is a concurrent declaration with a nonempty interface list, + -- get the first progenitor. Account for case of a record type created + -- for a concurrent type (which is the only case that seems to occur + -- in practice). + + elsif Nkind (D) = N_Full_Type_Declaration +and then (Is_Concurrent_Type (Defining_Identifier (D)) + or else Is_Concurrent_Record_Type (Defining_Identifier (D))) +and then Is_Non_Empty_List (Interface_List (Type_Definition (D))) + then + return Entity (First (Interface_List (Type_Definition (D; + -- If derived type and private type, get the full view to find who we -- are derived from. diff --git a/gcc/ada/sem_ch13.adb b/gcc/ada/sem_ch13.adb --- a/gcc/ada/sem_ch13.adb +++ b/gcc/ada/sem_ch13.adb @@ -9584,6 +9584,9 @@ package body Sem_Ch13 is -- Add predicates for ancestor if present. These must come before the -- ones for the current type, as required by AI12-0071-1. + -- Looks like predicates aren't added for case of inheriting from + -- multiple progenitors??? + declare Atyp : Entity_Id; begin
[Ada] Remove special case for processing null range in GNATprove
Historically both low and high range bounds were resolved with the range type, but we decided to resolve high bound with the range's base type. This appears to be necessary for having proper range checks in GNATprove mode, especially for null ranges, but it is no longer needed. This patch reverts change in Resolve_Range that was made for GNATprove. Tested on x86_64-pc-linux-gnu, committed on trunk gcc/ada/ * sem_res.adb (Resolve_Range): Resolve both low and high bounds with the range type.diff --git a/gcc/ada/sem_res.adb b/gcc/ada/sem_res.adb --- a/gcc/ada/sem_res.adb +++ b/gcc/ada/sem_res.adb @@ -10257,13 +10257,8 @@ package body Sem_Res is begin Set_Etype (N, Typ); - -- The lower bound should be in Typ. The higher bound can be in Typ's - -- base type if the range is null. It may still be invalid if it is - -- higher than the lower bound. This is checked later in the context in - -- which the range appears. - Resolve (L, Typ); - Resolve (H, Base_Type (Typ)); + Resolve (H, Typ); -- Reanalyze the lower bound after both bounds have been analyzed, so -- that the range is known to be static or not by now. This may trigger
[Ada] Remove obsolete code in Eval_Attribute
The procedure does not deal with run-time checks at all, except in a very peculiar case, 'Pos applied to an integer type, causing multiple checks to be generated for this case. No functional changes. Tested on x86_64-pc-linux-gnu, committed on trunk gcc/ada/ * sem_attr.adb (Eval_Attribute): Do not apply range checks in the case of 'Pos applied to an integer type here.diff --git a/gcc/ada/sem_attr.adb b/gcc/ada/sem_attr.adb --- a/gcc/ada/sem_attr.adb +++ b/gcc/ada/sem_attr.adb @@ -8354,16 +8354,6 @@ package body Sem_Attr is if not Compile_Time_Known_Value (E) or else not Is_Scalar_Type (Etype (E)) then - -- An odd special case, if this is a Pos attribute, this - -- is where we need to apply a range check since it does - -- not get done anywhere else. - - if Id = Attribute_Pos then - if Is_Integer_Type (Etype (E)) then - Apply_Range_Check (E, Etype (N)); - end if; - end if; - Check_Expressions; return;
[Ada] Visibility error with aggregate in Declare_Expression
The local objects declared in a Declare_Expression must be available when the expression is resolved. THis patch makes these objects visible during resolution, by reinstalling the scope created during prior analysis of the Declare_Expression. This scope exists only for visibility processing and plays no other semantic role. Tested on x86_64-pc-linux-gnu, committed on trunk gcc/ada/ * sem_res.adb (Resolve_Declare_Expression): New subprogram, to install the scope holding local declarations of the expression, before completing its resolution. (Resolve): For an Expression_With_Actions that comes from a source occurrence of a Declare_Expression, call new subprogram.diff --git a/gcc/ada/sem_res.adb b/gcc/ada/sem_res.adb --- a/gcc/ada/sem_res.adb +++ b/gcc/ada/sem_res.adb @@ -178,6 +178,7 @@ package body Sem_Res is procedure Resolve_Case_Expression (N : Node_Id; Typ : Entity_Id); procedure Resolve_Character_Literal (N : Node_Id; Typ : Entity_Id); procedure Resolve_Comparison_Op (N : Node_Id; Typ : Entity_Id); + procedure Resolve_Declare_Expression(N : Node_Id; Typ : Entity_Id); procedure Resolve_Entity_Name (N : Node_Id; Typ : Entity_Id); procedure Resolve_Equality_Op (N : Node_Id; Typ : Entity_Id); procedure Resolve_Explicit_Dereference (N : Node_Id; Typ : Entity_Id); @@ -2285,10 +2286,18 @@ package body Sem_Res is Check_Parameterless_Call (N); -- The resolution of an Expression_With_Actions is determined by - -- its Expression. + -- its Expression, but if the node comes from source it is a + -- Declare_Expression and requires scope management. if Nkind (N) = N_Expression_With_Actions then - Resolve (Expression (N), Typ); + if Comes_From_Source (N) +and then N = Original_Node (N) + then +Resolve_Declare_Expression (N, Typ); + + else +Resolve (Expression (N), Typ); + end if; Found := True; Expr_Type := Etype (Expression (N)); @@ -7399,6 +7408,49 @@ package body Sem_Res is end if; end Resolve_Comparison_Op; + + -- Resolve_Declare_Expression -- + + + procedure Resolve_Declare_Expression + (N : Node_Id; + Typ : Entity_Id) + is + Decl : Node_Id; + begin + -- Install the scope created for local declarations, if + -- any. The syntax allows a Declare_Expression with no + -- declarations, in analogy with block statements. + + Decl := First (Actions (N)); + + while Present (Decl) loop + exit when Nkind (Decl) = N_Object_Declaration; + Next (Decl); + end loop; + + if Present (Decl) then + Push_Scope (Scope (Defining_Identifier (Decl))); + + declare +E : Entity_Id := First_Entity (Current_Scope); + + begin +while Present (E) loop + Set_Current_Entity (E); + Set_Is_Immediately_Visible (E); + Next_Entity (E); +end loop; + end; + + Resolve (Expression (N), Typ); + End_Scope; + + else + Resolve (Expression (N), Typ); + end if; + end Resolve_Declare_Expression; + - -- Resolve_Discrete_Subtype_Indication -- -
[Ada] Fix evaluation of Enum_Rep applied to imported constant
Fixes an illegal call to Compile_Time_Known_Value with Empty parameter, which is now reported as a crash with switch -gnatdk. This is clearly a corner case; in preparation for a cleanup of some dubious code for Enum_Rep attribute. Tested on x86_64-pc-linux-gnu, committed on trunk gcc/ada/ * sem_attr.adb (Eval_Attribute): Check if constant has an initialization expression.diff --git a/gcc/ada/sem_attr.adb b/gcc/ada/sem_attr.adb --- a/gcc/ada/sem_attr.adb +++ b/gcc/ada/sem_attr.adb @@ -7834,6 +7834,8 @@ package body Sem_Attr is (Ekind (Entity (Enum_Expr)) = E_Constant and then Nkind (Parent (Entity (Enum_Expr))) = N_Object_Declaration + and then Present + (Expression (Parent (Entity (P and then Compile_Time_Known_Value (Expression (Parent (Entity (P)) then
[Ada] Small cleanup throughout Exp_Ch9
This mostly replaces verbose lines with calls to the Entry_Index_Type accessor functions and also changes Collect_Entry_Families to using the same method as Build_Entry_Count_Expression to determine whether an entry family is large. No functional changes. Tested on x86_64-pc-linux-gnu, committed on trunk gcc/ada/ * exp_ch9.adb (Is_Potentially_Large_Family): Add documentation. (Actual_Index_Expression): Use Entry_Index_Type. (Build_Entry_Count_Expression): Likewise. (Build_Find_Body_Index): Likewise. (Collect_Entry_Families): Likewise. Use directly the bounds of the index type to find out whether the family is large. (Entry_Index_Expression): Likewise.diff --git a/gcc/ada/exp_ch9.adb b/gcc/ada/exp_ch9.adb --- a/gcc/ada/exp_ch9.adb +++ b/gcc/ada/exp_ch9.adb @@ -434,6 +434,8 @@ package body Exp_Ch9 is Conctyp: Entity_Id; Lo : Node_Id; Hi : Node_Id) return Boolean; + -- Determine whether an entry family is potentially large because one of + -- its bounds denotes a discrminant. function Is_Private_Primitive_Subprogram (Id : Entity_Id) return Boolean; -- Determine whether Id is a function or a procedure and is marked as a @@ -585,7 +587,7 @@ package body Exp_Ch9 is -- structure. if Present (Index) then - S := Etype (Discrete_Subtype_Definition (Declaration_Node (Ent))); + S := Entry_Index_Type (Ent); Expr := Make_Op_Add (Sloc, @@ -612,8 +614,7 @@ package body Exp_Ch9 is Set_Intval (Num, Intval (Num) + 1); elsif Ekind (Prev) = E_Entry_Family then -S := - Etype (Discrete_Subtype_Definition (Declaration_Node (Prev))); +S := Entry_Index_Type (Prev); -- The need for the following full view retrieval stems from this -- complex case of nested generics and tasking: @@ -1702,7 +1703,7 @@ package body Exp_Ch9 is Next (Comp); end loop; -Typ := Etype (Discrete_Subtype_Definition (Parent (Ent))); +Typ := Entry_Index_Type (Ent); Hi := Type_High_Bound (Typ); Lo := Type_Low_Bound (Typ); Large := Is_Potentially_Large_Family @@ -2761,7 +2762,7 @@ package body Exp_Ch9 is Add_If_Clause (Make_Integer_Literal (Loc, 1)); elsif Ekind (Ent) = E_Entry_Family then - E_Typ := Etype (Discrete_Subtype_Definition (Parent (Ent))); + E_Typ := Entry_Index_Type (Ent); Hi := Convert_Discriminant_Ref (Type_High_Bound (E_Typ)); Lo := Convert_Discriminant_Ref (Type_Low_Bound (E_Typ)); Add_If_Clause (Family_Size (Loc, Hi, Lo, Typ, False)); @@ -5189,23 +5190,21 @@ package body Exp_Ch9 is Efam_Type := Make_Temporary (Loc, 'F'); declare - Bas : Entity_Id := - Base_Type - (Etype (Discrete_Subtype_Definition (Parent (Efam; - - Bas_Decl : Node_Id := Empty; - Lo, Hi : Node_Id; + Eityp : constant Entity_Id := Entry_Index_Type (Efam); + Lo: constant Node_Id := Type_Low_Bound (Eityp); + Hi: constant Node_Id := Type_High_Bound (Eityp); + Bdecl : Node_Id; + Bityp : Entity_Id; begin - Get_Index_Bounds - (Discrete_Subtype_Definition (Parent (Efam)), Lo, Hi); + Bityp := Base_Type (Eityp); - if Is_Potentially_Large_Family (Bas, Conctyp, Lo, Hi) then - Bas := Make_Temporary (Loc, 'B'); + if Is_Potentially_Large_Family (Bityp, Conctyp, Lo, Hi) then + Bityp := Make_Temporary (Loc, 'B'); - Bas_Decl := + Bdecl := Make_Subtype_Declaration (Loc, - Defining_Identifier => Bas, + Defining_Identifier => Bityp, Subtype_Indication => Make_Subtype_Indication (Loc, Subtype_Mark => @@ -5218,9 +5217,9 @@ package body Exp_Ch9 is Make_Integer_Literal (Loc, Entry_Family_Bound - 1); - Insert_After (Current_Node, Bas_Decl); - Current_Node := Bas_Decl; - Analyze (Bas_Decl); + Insert_After (Current_Node, Bdecl); + Current_Node := Bdecl; + Analyze (Bdecl); end if; Efam_Decl := @@ -5229,7 +5228,7 @@ package body Exp_Ch9 is Type_Definition => Make_Unconstrained_Array_Definition (Loc, Subtype_Marks => - (New_List (New_Occurrence_Of (Bas, L
[Ada] Predicates and the current instance of a subtype (AI12-0068)
AI12-0068 defines a reference to a current instance occurring in an
aspect_specification of a type or subtype to be a value, not an object.
This means that a Constrained attribute in such a context is always
True. It also means that attributes that take a prefix that must be an
object are illegal in that context. We determine whether we have such a
current instance reference by testing for a reference to a formal in
parameter of one of the subprograms that get created for subtype
predicates, invariants, or Default_Initial_Condition aspects, since the
current instance reference itself never gets analyzed but has been
changed into a reference to the special formal parameter by the time
Analyze_Attribute is called.
Tested on x86_64-pc-linux-gnu, committed on trunk
gcc/ada/
* sem_attr.adb (Analyze_Attribute, Attribute_Constrained): Issue
a warning if the attribute prefix is a current instance
reference within an aspect of a type or subtype.
(Address_Checks): Replace test of Is_Object (Ent) with
Is_Object_Reference (P) so that testing for current instances
will be done.
(Eval_Attribute): Add test for current instance reference, to
ensure that we still fold array attributes when current
instances are involved, since value prefixes are allowed for
array attributes, and will now be excluded by
Is_Object_Reference.
* sem_util.ads (Is_Current_Instance_Reference_In_Type_Aspect):
New exported query function.
* sem_util.adb (Is_Object_Reference): Return False for the case
where N is a current instance reference within an
aspect_specification of a type or subtype (basically if the
reference occurs within a predicate, invariant, or DIC aspect
expression).
(Is_Current_Instance_Reference_In_Type_Aspect): New function
that tests whether a node is a reference to a current instance
formal of a predicate, invariant, or
Default_Initial_Condition (DIC) subprogram.diff --git a/gcc/ada/sem_attr.adb b/gcc/ada/sem_attr.adb
--- a/gcc/ada/sem_attr.adb
+++ b/gcc/ada/sem_attr.adb
@@ -525,7 +525,7 @@ package body Sem_Attr is
-- Object or label reference
- elsif Is_Object (Ent) or else Ekind (Ent) = E_Label then
+ elsif Is_Object_Reference (P) or else Ekind (Ent) = E_Label then
Set_Address_Taken (Ent);
-- Deal with No_Implicit_Aliasing restriction
@@ -3486,11 +3486,25 @@ package body Sem_Attr is
return;
end if;
- -- Normal (non-obsolescent case) of application to object of
+ -- Normal (non-obsolescent case) of application to object or value of
-- a discriminated type.
else
-Check_Object_Reference (P);
+-- AI12-0068: In a type or subtype aspect, a prefix denoting the
+-- current instance of the (sub)type is defined to be a value,
+-- not an object, so the Constrained attribute is always True
+-- (see RM 8.6(18/5) and RM 3.7.2(3/5)). We issue a warning about
+-- this unintuitive result, to help avoid confusion.
+
+if Is_Current_Instance_Reference_In_Type_Aspect (P) then
+ Error_Msg_Name_1 := Aname;
+ Error_Msg_N
+ ("current instance attribute % in subtype aspect always " &
+ "true??", N);
+
+else
+ Check_Object_Reference (P);
+end if;
-- If N does not come from source, then we allow the
-- the attribute prefix to be of a private type whose
@@ -4169,11 +4183,13 @@ package body Sem_Attr is
if Comes_From_Source (N) then
--- A similar attribute Valid_Scalars can be prefixed with
--- references to both functions and objects, but this attribute
--- can be only prefixed with references to objects.
+-- This attribute be prefixed with references to objects or
+-- values (such as a current instance value given within a type
+-- or subtype aspect).
-if not Is_Object_Reference (P) then
+if not Is_Object_Reference (P)
+ and then not Is_Current_Instance_Reference_In_Type_Aspect (P)
+then
Error_Attr_P ("prefix of % attribute must be object");
end if;
end if;
@@ -7745,11 +7761,13 @@ package body Sem_Attr is
return;
end if;
- -- Special processing for cases where the prefix is an object. For this
- -- purpose, a string literal counts as an object (attributes of string
- -- literals can only appear in generated code).
+ -- Special processing for cases where the prefix is an object or value,
+ -- including string literals (attributes of string literals can only
+ -- appear in g
[Ada] Refine use of access types in unnesting
When we pass an unconstrained value in an activation record, we pass it an access type instead of an address so that we can get the bounds in the nested subprogram. However, the current code does this only when the uplevel reference is a parameter and uses 'Access. There are cases, as introduced in the fix for C760A02, involving a renaming, where an object is an unconstrained array type, but not a parameter. Moreover, we should use 'Unchecked_Access to avoid error messages in that case. Tested on x86_64-pc-linux-gnu, committed on trunk gcc/ada/ * exp_unst.adb (Needs_Fat_Pointer): Don't check for formal. (Unnest_Subprogram): Use 'Unchecked_Access instead of 'Access when populating activation record.diff --git a/gcc/ada/exp_unst.adb b/gcc/ada/exp_unst.adb --- a/gcc/ada/exp_unst.adb +++ b/gcc/ada/exp_unst.adb @@ -251,18 +251,14 @@ package body Exp_Unst is --- function Needs_Fat_Pointer (E : Entity_Id) return Boolean is - Typ : Entity_Id; - begin - if Is_Formal (E) then - Typ := Etype (E); - if Is_Private_Type (Typ) and then Present (Full_View (Typ)) then -Typ := Full_View (Typ); - end if; + Typ : Entity_Id := Etype (E); - return Is_Array_Type (Typ) and then not Is_Constrained (Typ); - else - return False; + begin + if Is_Private_Type (Typ) and then Present (Full_View (Typ)) then + Typ := Full_View (Typ); end if; + + return Is_Array_Type (Typ) and then not Is_Constrained (Typ); end Needs_Fat_Pointer; @@ -2084,7 +2080,7 @@ package body Exp_Unst is -- or else 'Access for unconstrained array if Needs_Fat_Pointer (Ent) then -Attr := Name_Access; +Attr := Name_Unchecked_Access; else Attr := Name_Address; end if;
[Ada] Fix bad interaction between Unchecked_Conversion and limited type
This prevents the compiler from aborting or crashing on the instantiation of Unchecked_Conversion on a limited type which is passed the result of a function call. The semantics of this operation are not very clear and the programmer is essentially on his/her own here, but the compiler should try to generate something plausible instead of dying. There is also a further complication when the type is additionally a CPP type and the function a CPP constructor, but in this case we do not try to enforce the Build-In-Place semantics of the call. Tested on x86_64-pc-linux-gnu, committed on trunk gcc/ada/ * exp_ch6.adb (Add_Simple_Call_By_Copy_Code): Rename parameter from Bit_Packed_Array to Force. (Expand_Actuals): Do not apply BIP processing if the subprogram is intrinsic. Adjust calls to Add_Simple_Call_By_Copy_Code and add one for In parameters whose actual is a CPP constructor call.diff --git a/gcc/ada/exp_ch6.adb b/gcc/ada/exp_ch6.adb --- a/gcc/ada/exp_ch6.adb +++ b/gcc/ada/exp_ch6.adb @@ -1469,11 +1469,16 @@ package body Exp_Ch6 is -- also takes care of any constraint checks required for the type -- conversion case (on both the way in and the way out). - procedure Add_Simple_Call_By_Copy_Code (Bit_Packed_Array : Boolean); + procedure Add_Simple_Call_By_Copy_Code (Force : Boolean); -- This is similar to the above, but is used in cases where we know -- that all that is needed is to simply create a temporary and copy - -- the value in and out of the temporary. If Bit_Packed_Array is True, - -- the procedure is called for a bit-packed array actual. + -- the value in and out of the temporary. If Force is True, then the + -- procedure may disregard legality considerations. + + -- ??? We need to do the copy for a bit-packed array because this is + -- where the rewriting into a mask-and-shift sequence is done. But of + -- course this may break the program if it expects bits to be really + -- passed by reference. That's what we have done historically though. procedure Add_Validation_Call_By_Copy_Code (Act : Node_Id); -- Perform copy-back for actual parameter Act which denotes a validation @@ -1851,7 +1856,7 @@ package body Exp_Ch6 is -- Add_Simple_Call_By_Copy_Code -- -- - procedure Add_Simple_Call_By_Copy_Code (Bit_Packed_Array : Boolean) is + procedure Add_Simple_Call_By_Copy_Code (Force : Boolean) is Decl : Node_Id; F_Typ : Entity_Id := Etype (Formal); Incod : Node_Id; @@ -1862,12 +1867,9 @@ package body Exp_Ch6 is Temp : Entity_Id; begin - -- ??? We need to do the copy for a bit-packed array because this is - -- where the rewriting into a mask-and-shift sequence is done. But of - -- course this may break the program if it expects bits to be really - -- passed by reference. That's what we have done historically though. + -- Unless forced not to, check the legality of the copy operation - if not Bit_Packed_Array and then not Is_Legal_Copy then + if not Force and then not Is_Legal_Copy then return; end if; @@ -2272,7 +2274,13 @@ package body Exp_Ch6 is -- functions that are treated as build-in-place to include other -- composite result types. -if Is_Build_In_Place_Function_Call (Actual) then +-- But do not do it here for intrinsic subprograms since this will +-- be done properly after the subprogram is expanded. + +if Is_Intrinsic_Subprogram (Subp) then + null; + +elsif Is_Build_In_Place_Function_Call (Actual) then Build_Activation_Chain_Entity (N); Build_Master_Entity (Etype (Actual)); Make_Build_In_Place_Call_In_Anonymous_Context (Actual); @@ -2366,7 +2374,7 @@ package body Exp_Ch6 is -- [in] out parameters. elsif Is_Ref_To_Bit_Packed_Array (Actual) then - Add_Simple_Call_By_Copy_Code (Bit_Packed_Array => True); + Add_Simple_Call_By_Copy_Code (Force => True); -- If a nonscalar actual is possibly bit-aligned, we need a copy -- because the back-end cannot cope with such objects. In other @@ -2382,7 +2390,7 @@ package body Exp_Ch6 is Component_May_Be_Bit_Aligned (Entity (Selector_Name (Actual))) and then not Represented_As_Scalar (Etype (Formal)) then - Add_Simple_Call_By_Copy_Code (Bit_Packed_Array => False); + Add_Simple_Call_By_Copy_Code (Force => False); -- References to slices of bit-packed arrays are expanded @@ -2568,14 +2576,19 @@ package body Exp_Ch6 is -- Is this really necessary in all cases??? elsif Is_
[Ada] Set range checks flag on 'Update for GNATprove in expansion
Range check flag on scalar expression in attribute Update was originally set when this attribute is expanded into a sequence of assignment. However, in GNATprove mode we have a custom expansion, which does nothing with attribute Update; in particular, it didn't set the required range check. As a workaround, we set this flag while resolving the attribute, both for GNAT and GNATprove. Yet, this didn't really work for attribute Update in subprograms "inlined for proof", where for some reason no checks at all were inserted. Also, this extra range check was unnecessary for a code like this: type T is record C : Natural; end record; X : T := (C => 1); Y : T := X'Update (C => X.C + 1); because an overflow check (which is emitted anyway) is enough to protect from a failing range check. A similar code, just without attribute Update, does not have a range check: Y.C := X.C + 1; This extra range check was hurting both compilation (by generating useless run-time checks) and proof (by triggering messages about checks that can never fail). Tested on x86_64-pc-linux-gnu, committed on trunk gcc/ada/ * exp_spark.adb (Expand_SPARK_N_Attribute_Reference): Apply scalar range checks. * sem_attr.adb (Resolve_Attribute): Do not set scalar range checks when resolving attribute Update.diff --git a/gcc/ada/exp_spark.adb b/gcc/ada/exp_spark.adb --- a/gcc/ada/exp_spark.adb +++ b/gcc/ada/exp_spark.adb @@ -251,6 +251,53 @@ package body Exp_SPARK is Analyze_And_Resolve (N, Standard_Boolean); end if; + elsif Attr_Id = Attribute_Update then + declare +Aggr : constant Node_Id := First (Expressions (N)); +-- The aggregate expression + +Assoc : Node_Id; +Comp : Node_Id; +Comp_Type : Node_Id; +Expr : Node_Id; + + begin +-- Apply scalar range checks on the updated components, if needed + +if Is_Array_Type (Typ) then + Assoc := First (Component_Associations (Aggr)); + + while Present (Assoc) loop + Expr := Expression (Assoc); + Comp_Type := Component_Type (Typ); + + if Is_Scalar_Type (Comp_Type) then + Apply_Scalar_Range_Check (Expr, Comp_Type); + end if; + + Next (Assoc); + end loop; + +else pragma Assert (Is_Record_Type (Typ)); + + Assoc := First (Component_Associations (Aggr)); + while Present (Assoc) loop + Expr := Expression (Assoc); + Comp := First (Choices (Assoc)); + Comp_Type := Etype (Entity (Comp)); + + -- Use the type of the first component from the Choices + -- list, as multiple components can only appear there if + -- they have exactly the same type. + + if Is_Scalar_Type (Comp_Type) then + Apply_Scalar_Range_Check (Expr, Comp_Type); + end if; + + Next (Assoc); + end loop; +end if; + end; end if; end Expand_SPARK_N_Attribute_Reference; diff --git a/gcc/ada/sem_attr.adb b/gcc/ada/sem_attr.adb --- a/gcc/ada/sem_attr.adb +++ b/gcc/ada/sem_attr.adb @@ -11995,26 +11995,6 @@ package body Sem_Attr is Expr := Expression (Assoc); Resolve (Expr, Component_Type (Typ)); - -- For scalar array components set Do_Range_Check when - -- needed. Constraint checking on non-scalar components - -- is done in Aggregate_Constraint_Checks, but only if - -- full analysis is enabled. These flags are not set in - -- the front-end in GnatProve mode. - - if Is_Scalar_Type (Component_Type (Typ)) -and then not Is_OK_Static_Expression (Expr) -and then not Range_Checks_Suppressed (Component_Type (Typ)) - then - if Is_Entity_Name (Expr) - and then Etype (Expr) = Component_Type (Typ) - then -null; - - else -Set_Do_Range_Check (Expr); - end if; - end if; - -- The choices in the association are static constants, -- or static aggregates each of whose components belongs -- to the proper index type. However, they must also @@ -12072,14 +12052,6 @@ package body Sem_Attr is and then not Error_Posted (Comp) then Resolve (Expr, Etype (Entity (Comp))); - - if Is_Scalar_Type (Etype (Entity (Comp))) - and then not Is_OK_Static_
[Ada] Crash on declaration with aggregate for unchecked union
This patch fixes a compiler abort on an object declaration whose expression is an aggregate for an unchecked union type. A declaration for an object of such a type is otherwise treated as a renaming of the expression (typically an object of the type), but this does work for an aggregate, which must be resolved and handled as an initial value of a regular declaration. Tested on x86_64-pc-linux-gnu, committed on trunk gcc/ada/ * sem_ch3.adb (Analyze_Object_Declaration): If the type is an Unchecked_Union, and the expression is an aggregate. complete the analysis and resolution of the aggregate, and treat like a regular object declaration, instead of as a renaming declarattion.diff --git a/gcc/ada/sem_ch3.adb b/gcc/ada/sem_ch3.adb --- a/gcc/ada/sem_ch3.adb +++ b/gcc/ada/sem_ch3.adb @@ -4564,16 +4564,26 @@ package body Sem_Ch3 is Set_Ekind (Id, E_Variable); end if; - Rewrite (N, - Make_Object_Renaming_Declaration (Loc, - Defining_Identifier => Id, - Subtype_Mark=> New_Occurrence_Of (T, Loc), - Name=> E)); - - Set_Renamed_Object (Id, E); - Freeze_Before (N, T); - Set_Is_Frozen (Id); - goto Leave; + -- If the expression is an aggregate it contains the required + -- discriminant values but it has not been resolved yet, so do + -- it now, and treat it as the initial expression of an object + -- declaration, rather than a renaming. + + if Nkind (E) = N_Aggregate then + Analyze_And_Resolve (E, T); + + else + Rewrite (N, +Make_Object_Renaming_Declaration (Loc, + Defining_Identifier => Id, + Subtype_Mark=> New_Occurrence_Of (T, Loc), + Name=> E)); + + Set_Renamed_Object (Id, E); + Freeze_Before (N, T); + Set_Is_Frozen (Id); + goto Leave; + end if; else -- Ensure that the generated subtype has a unique external name
[Ada] Crash on derived type with rep. clause and renamed discriminant
This patch fixes a compiler abort on a declaration of subtype of a derived type DT whose declaration renames a discriminant of its parent type T, when there is a record representation clause for DT. Tested on x86_64-pc-linux-gnu, committed on trunk gcc/ada/ * exp_ch5.adb (Find_Component): If the target type is a derived record type and the required component is a discriminant that is renamed in the derived type declaration, use the name of the original discriminant to locate the intended target component.diff --git a/gcc/ada/exp_ch5.adb b/gcc/ada/exp_ch5.adb --- a/gcc/ada/exp_ch5.adb +++ b/gcc/ada/exp_ch5.adb @@ -1594,6 +1594,18 @@ package body Exp_Ch5 is while Present (C) loop if Chars (C) = Chars (Comp) then return C; + + -- The component may be a renamed discriminant, in + -- which case check against the name of the original + -- discriminant of the parent type. + + elsif Is_Derived_Type (Scope (Comp)) + and then Ekind (Comp) = E_Discriminant + and then Present (Corresponding_Discriminant (Comp)) + and then + Chars (C) = Chars (Corresponding_Discriminant (Comp)) + then + return C; end if; Next_Entity (C);
[Ada] Improve run-time performance for large initialized allocators
This extends the optimization applied to large aggregates used to reset array objects to the case of allocators initialized by such aggregates. The aggregates must essentially satisfy the same conditions as in the former case, although aggregates explicitly giving the bounds of the allocation are supported in this new case. Tested on x86_64-pc-linux-gnu, committed on trunk gcc/ada/ * exp_aggr.adb (Aggr_Assignment_OK_For_Backend): Move to library level and use a new predicate Is_OK_Aggregate to recognize the aggregates suitable for direct assignment by the back-end. (Convert_Array_Aggr_In_Allocator): If neither in CodePeer mode nor generating C code, generate a direct assignment instead of further expanding if Aggr_Assignment_OK_For_Backend returns true.diff --git a/gcc/ada/exp_aggr.adb b/gcc/ada/exp_aggr.adb --- a/gcc/ada/exp_aggr.adb +++ b/gcc/ada/exp_aggr.adb @@ -246,6 +246,9 @@ package body Exp_Aggr is -- Local Subprograms for Array Aggregate Expansion -- - + function Aggr_Assignment_OK_For_Backend (N : Node_Id) return Boolean; + -- Returns true if an aggregate assignment can be done by the back end + function Aggr_Size_OK (N : Node_Id; Typ : Entity_Id) return Boolean; -- Very large static aggregates present problems to the back-end, and are -- transformed into assignments and loops. This function verifies that the @@ -343,6 +346,246 @@ package body Exp_Aggr is -- false if this transformation cannot be performed. THis is similar to, -- and reuses part of the machinery in Packed_Array_Aggregate_Handled. + + -- Aggr_Assignment_OK_For_Backend -- + + + -- Back-end processing by Gigi/gcc is possible only if all the following + -- conditions are met: + + --1. N consists of a single OTHERS choice, possibly recursively, or + -- of a single choice, possibly recursively, if it is surrounded by + -- a qualified expression whose subtype mark is unconstrained. + + --2. The array type has no null ranges (the purpose of this is to + -- avoid a bogus warning for an out-of-range value). + + --3. The array type has no atomic components + + --4. The component type is elementary + + --5. The component size is a multiple of Storage_Unit + + --6. The component size is Storage_Unit or the value is of the form + -- M * (1 + A**1 + A**2 + .. A**(K-1)) where A = 2**(Storage_Unit) + -- and M in 0 .. A-1. This can also be viewed as K occurrences of + -- the Storage_Unit value M, concatenated together. + + -- The ultimate goal is to generate a call to a fast memset routine + -- specifically optimized for the target. + + function Aggr_Assignment_OK_For_Backend (N : Node_Id) return Boolean is + Csiz : Uint := No_Uint; + Ctyp : Entity_Id; + Expr : Node_Id; + High : Node_Id; + Index : Entity_Id; + Low : Node_Id; + Nunits: Int; + Remainder : Uint; + Value : Uint; + + function Is_OK_Aggregate (Aggr : Node_Id) return Boolean; + -- Return true if Aggr is suitable for back-end assignment + + - + -- Is_OK_Aggregate -- + - + + function Is_OK_Aggregate (Aggr : Node_Id) return Boolean is + Assoc : constant List_Id := Component_Associations (Aggr); + + begin + -- An "others" aggregate is most likely OK, but see below + + if Is_Others_Aggregate (Aggr) then +null; + + -- An aggregate with a single choice requires a qualified expression + -- whose subtype mark is an unconstrained type because we need it to + -- have the semantics of an "others" aggregate. + + elsif Nkind (Parent (N)) = N_Qualified_Expression + and then not Is_Constrained (Entity (Subtype_Mark (Parent (N + and then Is_Single_Aggregate (Aggr) + then +null; + + -- The other cases are not OK + + else +return False; + end if; + + -- In any case we do not support an iterated association + + return Nkind (First (Assoc)) /= N_Iterated_Component_Association; + end Is_OK_Aggregate; + + begin + -- Back end doesn't know about <> + + if Has_Default_Init_Comps (N) then + return False; + end if; + + -- Recurse as far as possible to find the innermost component type + + Ctyp := Etype (N); + Expr := N; + while Is_Array_Type (Ctyp) loop + if Nkind (Expr) /= N_Aggregate + or else not Is_OK_Aggregate (Expr) + then +return False; + end if; + + Index := First_Index (Ctyp); + while Present (Index) loop +Get_Index_Bounds (Index, Low, High); +
[Ada] Hang on generic declaration with ambiguous formal package
This patch fixes the compiler whereby malformed code featuring a generic package declaration with a formal package may result in an infinite loop when the name of the formal package is hidden by the formal parameter itself. Tested on x86_64-pc-linux-gnu, committed on trunk gcc/ada/ * sem_ch12.adb (Analyze_Associations): Add check for errors on the generic formal before continuing with instantiation.diff --git a/gcc/ada/sem_ch12.adb b/gcc/ada/sem_ch12.adb --- a/gcc/ada/sem_ch12.adb +++ b/gcc/ada/sem_ch12.adb @@ -1948,10 +1948,19 @@ package body Sem_Ch12 is end if; when N_Formal_Package_Declaration => - Match := -Matching_Actual - (Defining_Identifier (Formal), - Defining_Identifier (Original_Node (Analyzed_Formal))); + -- The name of the formal package may be hidden by the + -- formal parameter itself. + + if Error_Posted (Analyzed_Formal) then + Abandon_Instantiation (Instantiation_Node); + + else + Match := + Matching_Actual + (Defining_Identifier (Formal), + Defining_Identifier +(Original_Node (Analyzed_Formal))); + end if; if No (Match) then if Partial_Parameterization then
[Ada] Crash when an exception handler is executed with -gnatdk
In few routines we expect exceptions and handle them by falling back to an appropriate safe value (e.g. in Compile_Time_Known_Value we fall back to False and merely skip some optimizations). This is especially important while processing code with previous errors. However, this behaviour prevents us from detecting a genuinely bad handling of a legal code. With this patch we can use -gnatdk switch to get a bugbox when exception occurs without previous errors. Tested on x86_64-pc-linux-gnu, committed on trunk gcc/ada/ * sem_ch5.adb (Analyze_Loop_Parameter_Specification): Propagate exception when switch -gnatdk is used and no previous errors are present. * sem_eval.adb (Compile_Time_Known_Value, Is_In_Range): Likewise. * sem_warn.adb (Operand_Has_Warnings_Suppressed): Likewise.diff --git a/gcc/ada/sem_ch5.adb b/gcc/ada/sem_ch5.adb --- a/gcc/ada/sem_ch5.adb +++ b/gcc/ada/sem_ch5.adb @@ -26,6 +26,7 @@ with Aspects; use Aspects; with Atree;use Atree; with Checks; use Checks; +with Debug;use Debug; with Einfo;use Einfo; with Errout; use Errout; with Expander; use Expander; @@ -3302,7 +3303,13 @@ package body Sem_Ch5 is -- the warning is perfectly acceptable. exception -when others => null; +when others => + -- With debug flag K we will get an exception unless an error + -- has already occurred (useful for debugging). + + if Debug_Flag_K then + Check_Error_Detected; + end if; end; end if; diff --git a/gcc/ada/sem_eval.adb b/gcc/ada/sem_eval.adb --- a/gcc/ada/sem_eval.adb +++ b/gcc/ada/sem_eval.adb @@ -1848,6 +1848,13 @@ package body Sem_Eval is exception when others => + -- With debug flag K we will get an exception unless an error has + -- already occurred (useful for debugging). + + if Debug_Flag_K then +Check_Error_Detected; + end if; + return False; end Compile_Time_Known_Value; @@ -4962,14 +4969,14 @@ package body Sem_Eval is exception when others => - - -- Debug flag K disables this behavior (useful for debugging) + -- With debug flag K we will get an exception unless an error has + -- already occurred (useful for debugging). if Debug_Flag_K then -raise; - else -return False; +Check_Error_Detected; end if; + + return False; end In_Subrange_Of; - diff --git a/gcc/ada/sem_warn.adb b/gcc/ada/sem_warn.adb --- a/gcc/ada/sem_warn.adb +++ b/gcc/ada/sem_warn.adb @@ -2993,6 +2993,13 @@ package body Sem_Warn is exception when others => + -- With debug flag K we will get an exception unless an error has + -- already occurred (useful for debugging). + + if Debug_Flag_K then +Check_Error_Detected; + end if; + return False; end Operand_Has_Warnings_Suppressed;
[Ada] Use Stream_Element_Arrays internally for secure hash computations
Refactor share code for secure hash computations to use Stream_Element_Array instead of String as the internal buffer type. This resolves an issue where we would overlay a String on a user provided Stream_Element_Array when that user array is larger than the largest possible String. Tested on x86_64-pc-linux-gnu, committed on trunk gcc/ada/ * libgnat/g-sechas.ads, libgnat/g-sechas.adb: Refactor to use Stream_Element_Array as the internal buffer type. * libgnat/g-shshco.adb: Adjust to use Stream_Element_Offset instead of Integer as the index in the internal state buffer.diff --git a/gcc/ada/libgnat/g-sechas.adb b/gcc/ada/libgnat/g-sechas.adb --- a/gcc/ada/libgnat/g-sechas.adb +++ b/gcc/ada/libgnat/g-sechas.adb @@ -40,25 +40,25 @@ package body GNAT.Secure_Hashes is type Fill_Buffer_Access is access procedure (M : in out Message_State; -S : String; -First : Natural; -Last : out Natural); - -- A procedure to transfer data from S, starting at First, into M's block +SEA : Stream_Element_Array; +First : Stream_Element_Offset; +Last : out Stream_Element_Offset); + -- A procedure to transfer data from SEA, starting at First, into M's block -- buffer until either the block buffer is full or all data from S has been -- consumed. procedure Fill_Buffer_Copy (M : in out Message_State; - S : String; - First : Natural; - Last : out Natural); + SEA : Stream_Element_Array; + First : Stream_Element_Offset; + Last : out Stream_Element_Offset); -- Transfer procedure which just copies data from S to M procedure Fill_Buffer_Swap (M : in out Message_State; - S : String; - First : Natural; - Last : out Natural); + SEA : Stream_Element_Array; + First : Stream_Element_Offset; + Last : out Stream_Element_Offset); -- Transfer procedure which swaps bytes from S when copying into M. S must -- have even length. Note that the swapping is performed considering pairs -- starting at S'First, even if S'First /= First (that is, if @@ -75,22 +75,23 @@ package body GNAT.Secure_Hashes is procedure Fill_Buffer_Copy (M : in out Message_State; - S : String; - First : Natural; - Last : out Natural) + SEA : Stream_Element_Array; + First : Stream_Element_Offset; + Last : out Stream_Element_Offset) is - Buf_String : String (M.Buffer'Range); - for Buf_String'Address use M.Buffer'Address; - pragma Import (Ada, Buf_String); + Buf_SEA : Stream_Element_Array (M.Buffer'Range); + for Buf_SEA'Address use M.Buffer'Address; + pragma Import (Ada, Buf_SEA); - Length : constant Natural := - Natural'Min (M.Block_Length - M.Last, S'Last - First + 1); + Length : constant Stream_Element_Offset := + Stream_Element_Offset'Min +(M.Block_Length - M.Last, SEA'Last - First + 1); begin pragma Assert (Length > 0); - Buf_String (M.Last + 1 .. M.Last + Length) := -S (First .. First + Length - 1); + Buf_SEA (M.Last + 1 .. M.Last + Length) := +SEA (First .. First + Length - 1); M.Last := M.Last + Length; Last := First + Length - 1; end Fill_Buffer_Copy; @@ -101,20 +102,21 @@ package body GNAT.Secure_Hashes is procedure Fill_Buffer_Swap (M : in out Message_State; - S : String; - First : Natural; - Last : out Natural) + SEA : Stream_Element_Array; + First : Stream_Element_Offset; + Last : out Stream_Element_Offset) is - pragma Assert (S'Length mod 2 = 0); - Length : constant Natural := - Natural'Min (M.Block_Length - M.Last, S'Last - First + 1); + pragma Assert (SEA'Length mod 2 = 0); + Length : constant Stream_Element_Offset := + Stream_Element_Offset'Min + (M.Block_Length - M.Last, SEA'Last - First + 1); begin Last := First; while Last - First < Length loop M.Buffer (M.Last + 1 + Last - First) := - (if (Last - S'First) mod 2 = 0 -then S (Last + 1) -else S (Last - 1)); + (if (Last - SEA'First) mod 2 = 0 +then SEA (Last + 1) +else SEA (Last - 1)); Last := Last + 1; end loop; M.Last := M.Last + Length; @@ -146,7 +148,7 @@ package body GNAT.Secure_Hashes is procedure Update (C : in out Context; - S : String; + SEA : Stream_Element_Array; Fill_Buffer : Fill_Buffer_Access); -- Internal common routine for all Update procedures @@ -161,8 +163,7 @@ package body GNAT.Secure_Hashes is function Digest (C : Context) return Message_Digest is - Hash_Bits : Stream_Element_Ar
[Ada] Time_IO.Image: Allow printing the time zone
This patch adds support to Image for the GNU standard format "%:::z", which prints the time zone in the format "+hh", "-hh", "+hh:mm", or "-hh:mm", as appropriate. We do not support any of the other standard GNU time-zone formats (%z, %:z, %::z, %Z). Tested on x86_64-pc-linux-gnu, committed on trunk gcc/ada/ * libgnat/g-catiio.ads: Document newly supported format. Add ISO_Time constant, for convenience. * libgnat/g-catiio.adb (Image_Helper): New helper function to do all the formatting work, called by the two exported Image functions. Add support for "%:::z" here. Add a Time_Zone parameter used by the "%:::z" processing. This parameter is not used for the actual time zone computations; local time is always used for that, for ease of implementation reasons. It would make sense to use Append throughout this function, but that's a cleanup for another day. (Image): Modify these to pass the local time zone, or the specified time zone, as appropriate.diff --git a/gcc/ada/libgnat/g-catiio.adb b/gcc/ada/libgnat/g-catiio.adb --- a/gcc/ada/libgnat/g-catiio.adb +++ b/gcc/ada/libgnat/g-catiio.adb @@ -69,6 +69,14 @@ package body GNAT.Calendar.Time_IO is -- Local Subprograms -- --- + function Image_Helper + (Date : Ada.Calendar.Time; + Picture : Picture_String; + Time_Zone : Time_Zones.Time_Offset) return String; + -- This is called by the two exported Image functions. It uses the local + -- time zone for its computations, but uses Time_Zone when interpreting the + -- "%:::z" tag. + function Am_Pm (H : Natural) return String; -- Return AM or PM depending on the hour H @@ -168,6 +176,10 @@ package body GNAT.Calendar.Time_IO is return Image (Sec_Number (N), Padding, Length); end Image; + --- + -- Image -- + --- + function Image (N : Sec_Number; Padding : Padding_Mode := Zero; @@ -213,15 +225,16 @@ package body GNAT.Calendar.Time_IO is Time_Zone : Time_Zones.Time_Offset) return String is -- We subtract off the local time zone, and add in the requested - -- Time_Zone, and then pass it on to the version of Image that uses - -- the local time zone. + -- Time_Zone, and then pass it on to Image_Helper, which uses the + -- local time zone. use Time_Zones; Local_TZ : constant Time_Offset := Local_Time_Offset (Date); Minute_Offset : constant Integer := Integer (Time_Zone - Local_TZ); Second_Offset : constant Integer := Minute_Offset * 60; begin - return Image (Date + Duration (Second_Offset), Picture); + return Image_Helper +(Date + Duration (Second_Offset), Picture, Time_Zone); end Image; --- @@ -232,6 +245,21 @@ package body GNAT.Calendar.Time_IO is (Date: Ada.Calendar.Time; Picture : Picture_String) return String is + use Time_Zones; + Local_TZ : constant Time_Offset := Local_Time_Offset (Date); + begin + return Image_Helper (Date, Picture, Local_TZ); + end Image; + + -- + -- Image_Helper -- + -- + + function Image_Helper + (Date : Ada.Calendar.Time; + Picture : Picture_String; + Time_Zone : Time_Zones.Time_Offset) return String + is Padding : Padding_Mode := Zero; -- Padding is set for one directive @@ -424,6 +452,43 @@ package body GNAT.Calendar.Time_IO is Image (Minute, Padding, Length => 2) & ':' & Image (Second, Padding, Length => 2); + -- Time zone. Append "+hh", "-hh", "+hh:mm", or "-hh:mm", as + -- appropriate. + + when ':' => + declare + use type Time_Zones.Time_Offset; + TZ_Form : constant Picture_String := "%:::z"; + TZ : constant Natural := Natural (abs Time_Zone); + begin + if P + TZ_Form'Length - 1 <= Picture'Last + and then Picture (P .. P + TZ_Form'Length - 1) = "%:::z" + then +if Time_Zone >= 0 then + Result := Result & "+"; +else + Result := Result & "-"; +end if; + +Result := Result & + Image (Integer (TZ / 60), Padding, Length => 2); + +if TZ mod 60 /= 0 then + Result := Result & ":"; + Result := Result & + Image (TZ mod 60, Padding, Length => 2); +end if; + +P := P + TZ_Form'Length - 2; -- will add 2 below + + -- We do not support any of the other standard GNU +
[Ada] Time_IO.Value: Allow subseconds and time zones together
The syntax parsed by GNAT.Calendar.Time_IO.Value allowed a fraction of a
second, or a time zone, but not both. This is a violation of the
relevant ISO standard ISO-8601. This patch allows both to be specified.
Tested on x86_64-pc-linux-gnu, committed on trunk
gcc/ada/
* libgnat/g-catiio.ads: Change the regular expression that
documents the allowed format to match what ISO-8601 allows.
* libgnat/g-catiio.adb (Scan_Subsecond): Rewrite so it doesn't
assume the subsecond comes last.
(Parse_ISO_8601): Parse an optional subsecond, followed by an
optional time zone, rather than making these alternatives to
each other.diff --git a/gcc/ada/libgnat/g-catiio.adb b/gcc/ada/libgnat/g-catiio.adb
--- a/gcc/ada/libgnat/g-catiio.adb
+++ b/gcc/ada/libgnat/g-catiio.adb
@@ -778,17 +778,10 @@ package body GNAT.Calendar.Time_IO is
begin
Advance_Digits (Num_Digits => 1);
- while Symbol in '0' .. '9'
- and then Index < Date'Length
- loop
+ while Index <= Date'Length and then Symbol in '0' .. '9' loop
Advance;
end loop;
- if Symbol not in '0' .. '9' then
-raise Wrong_Syntax;
- end if;
-
- Advance;
return Second_Duration'Value ("0." & Date (From .. Index - 1));
end Scan_Subsecond;
@@ -845,6 +838,8 @@ package body GNAT.Calendar.Time_IO is
subtype Sign_Type is Character with Predicate => Sign_Type in '+' | '-';
+ -- Start of processing for Parse_ISO_8601
+
begin
-- Parse date
@@ -869,26 +864,31 @@ package body GNAT.Calendar.Time_IO is
Second := Scan_Second;
- -- [('Z' | ('.' | ',') s{s} | ('+'|'-')hh:mm)]
+ -- [ ('.' | ',') s{s} ]
if Index <= Date'Last then
-
--- Suffix 'Z' signifies that this is UTC time (time zone 0)
-
-if Symbol = 'Z' then
- Time_Zone_Seen := True;
- Time_Zone_Offset := 0;
- Advance;
-
-- A decimal fraction shall have at least one digit, and has as
-- many digits as supported by the underlying implementation.
-- The valid decimal separators are those specified in ISO 31-0,
-- i.e. the comma [,] or full stop [.]. Of these, the comma is
-- the preferred separator of ISO-8601.
-elsif Symbol = ',' or else Symbol = '.' then
+if Symbol = ',' or else Symbol = '.' then
Advance; -- past decimal separator
Subsec := Scan_Subsecond;
+end if;
+ end if;
+
+ -- [ ('Z' | ('+'|'-')hh':'mm) ]
+
+ if Index <= Date'Last then
+Time_Zone_Seen := Symbol in 'Z' | Sign_Type;
+
+-- Suffix 'Z' signifies that this is UTC time (time zone 0)
+
+if Symbol = 'Z' then
+ Time_Zone_Offset := 0;
+ Advance;
-- Difference between local time and UTC: It shall be expressed
-- as positive (i.e. with the leading plus sign [+]) if the local
@@ -900,12 +900,10 @@ package body GNAT.Calendar.Time_IO is
elsif Symbol in Sign_Type then
declare
- Time_Zone_Sign : constant Sign_Type := Symbol;
-
+ Time_Zone_Sign : constant Sign_Type := Symbol;
Time_Zone_Hour : Hour_Number;
Time_Zone_Minute : Minute_Number;
begin
- Time_Zone_Seen := True;
Advance;
Time_Zone_Hour := Scan_Hour;
@@ -923,9 +921,10 @@ package body GNAT.Calendar.Time_IO is
Time_Zone_Offset :=
Time_Offset (Time_Zone_Hour * 60 + Time_Zone_Minute);
- if Time_Zone_Sign = '-' then
- Time_Zone_Offset := -Time_Zone_Offset;
- end if;
+ case Time_Zone_Sign is
+ when '+' => null;
+ when '-' => Time_Zone_Offset := -Time_Zone_Offset;
+ end case;
end;
else
raise Wrong_Syntax;
diff --git a/gcc/ada/libgnat/g-catiio.ads b/gcc/ada/libgnat/g-catiio.ads
--- a/gcc/ada/libgnat/g-catiio.ads
+++ b/gcc/ada/libgnat/g-catiio.ads
@@ -154,7 +154,8 @@ package GNAT.Calendar.Time_IO is
-- supported:
--
--(mmdd | '-'mm'-'dd)'T'(hhmmss | hh':'mm':'ss)
- -- [ ('Z' | ('.' | ',') s{s} | ('+'|'-')hh':'mm) ]
+ -- [ ('.' | ',') s{s} ]
+ -- [ ('Z' | ('+'|'-')hh':'mm) ]
-- Trailing characters (including spaces) are not allowed.
-- In the ISO case, the current time zone is not used; the time zone
-- is as specified in the string, defaulting to UTC.
[Ada] Implement Time_IO.Image with Time_Zone
This patch adds a new function Image in GNAT.Time_IO that takes a Time_Zone parameter rather than using local time. Tested on x86_64-pc-linux-gnu, committed on trunk gcc/ada/ * libgnat/g-catiio.ads, libgnat/g-catiio.adb (Image): New function. It might seem like the local-time Image should be implemented in terms of the Time_Zone Image, but that would be far harder to implement, so we do it the other way around.diff --git a/gcc/ada/libgnat/g-catiio.adb b/gcc/ada/libgnat/g-catiio.adb --- a/gcc/ada/libgnat/g-catiio.adb +++ b/gcc/ada/libgnat/g-catiio.adb @@ -29,8 +29,6 @@ -- -- -- -with Ada.Calendar;use Ada.Calendar; -with Ada.Calendar.Time_Zones; with Ada.Characters.Handling; with Ada.Strings.Unbounded; use Ada.Strings.Unbounded; with Ada.Text_IO; @@ -210,6 +208,27 @@ package body GNAT.Calendar.Time_IO is --- function Image + (Date : Ada.Calendar.Time; + Picture : Picture_String; + Time_Zone : Time_Zones.Time_Offset) return String + is + -- We subtract off the local time zone, and add in the requested + -- Time_Zone, and then pass it on to the version of Image that uses + -- the local time zone. + + use Time_Zones; + Local_TZ : constant Time_Offset := Local_Time_Offset (Date); + Minute_Offset : constant Integer := Integer (Time_Zone - Local_TZ); + Second_Offset : constant Integer := Minute_Offset * 60; + begin + return Image (Date + Duration (Second_Offset), Picture); + end Image; + + --- + -- Image -- + --- + + function Image (Date: Ada.Calendar.Time; Picture : Picture_String) return String is diff --git a/gcc/ada/libgnat/g-catiio.ads b/gcc/ada/libgnat/g-catiio.ads --- a/gcc/ada/libgnat/g-catiio.ads +++ b/gcc/ada/libgnat/g-catiio.ads @@ -32,6 +32,8 @@ -- This package augments standard Ada.Text_IO with facilities for input -- and output of time values in standardized format. +with Ada.Calendar.Time_Zones; use Ada.Calendar; + package GNAT.Calendar.Time_IO is Picture_Error : exception; @@ -115,6 +117,13 @@ package GNAT.Calendar.Time_IO is -- with format Picture. Raise Picture_Error if picture string is null or -- has an incorrect format. + function Image + (Date : Ada.Calendar.Time; + Picture : Picture_String; + Time_Zone : Time_Zones.Time_Offset) return String; + -- Same as previous Image, except it uses the specified time zone instead + -- of the local time zone. + function Value (Date : String) return Ada.Calendar.Time; -- Parse the string Date, interpreted as a time representation in the -- current local time zone, and return the corresponding Time value. The
[Ada] Correct time zone in GNAT.Calendar.Time_IO.Value
This patch fixes a bug in which if GNAT.Calendar.Time_IO.Value is called with an ISO date string, the time zone offset is computed incorrectly. Tested on x86_64-pc-linux-gnu, committed on trunk gcc/ada/ * libgnat/g-catiio.adb (Parse_ISO_8601): New name for Parse_ISO_8861_UTC. 8601 is the correct ISO standard number. Also, "UTC" was confusing. All Time values are represented in UTC, but the ISO 8601 date strings include a time zone. If a time zone was specified, call Ada.Calendar.Formatting.Time_Of instead of GNAT.Calendar.Time_Of, because the latter adjusts to the current time zone, whereas we want to use (just) the time zone specified in the ISO string. This allows us to pass Time_Zone instead to Time_Of, instead of adjusting by Local_Disp by hand. If no time zone was specified, call GNAT.Calendar.Time_Of as before. Use expanded names to clarify which Time_Of is being called. Remove redundant comment, and move nonredundant part of the commment to the spec. (Value): Minor: use "not in" instead of "or else". * libgnat/g-catiio.ads: Comment moved here. Correct the ISO standard number. * libgnat/g-calend.adb: Add ??? comments. * libgnat/a-calend.ads, libgnat/a-calend.adb: Update obsolete comments regarding the representation of type Time. Move the information about the epoch (year 2150) to the spec, and avoid uttering "2150" more than once. * libgnat/a-catizo.ads (Time_Offset): Add comment.diff --git a/gcc/ada/libgnat/a-calend.adb b/gcc/ada/libgnat/a-calend.adb --- a/gcc/ada/libgnat/a-calend.adb +++ b/gcc/ada/libgnat/a-calend.adb @@ -167,9 +167,9 @@ is Secs_In_Non_Leap_Year : constant := 365 * Secs_In_Day; Nanos_In_Four_Years : constant := Secs_In_Four_Years * Nano; - -- Lower and upper bound of Ada time. The zero (0) value of type Time is - -- positioned at year 2150. Note that the lower and upper bound account - -- for the non-leap centennial years. + -- Lower and upper bound of Ada time. Note that the lower and upper bound + -- account for the non-leap centennial years. See "Implementation of Time" + -- in the spec for what the zero value represents. Ada_Low : constant Time_Rep := -(61 * 366 + 188 * 365) * Nanos_In_Day; Ada_High : constant Time_Rep := (60 * 366 + 190 * 365) * Nanos_In_Day; diff --git a/gcc/ada/libgnat/a-calend.ads b/gcc/ada/libgnat/a-calend.ads --- a/gcc/ada/libgnat/a-calend.ads +++ b/gcc/ada/libgnat/a-calend.ads @@ -157,16 +157,20 @@ private -- Implementation of Time -- - -- Time is represented as a signed 64 bit integer count of nanoseconds - -- since the start of Ada time (1901-01-01 00:00:00.0 UTC). Time values - -- produced by Time_Of are internally normalized to UTC regardless of their - -- local time zone. This representation ensures correct handling of leap - -- seconds as well as performing arithmetic. In Ada 95, Split and Time_Of - -- will treat a time value as being in the local time zone, in Ada 2005, - -- Split and Time_Of will treat a time value as being in the designated - -- time zone by the formal parameter or in UTC by default. The size of the - -- type is large enough to cover the Ada 2005 range of time (1901-01-01 - -- 00:00:00.0 UTC - 2399-12-31-23:59:59.9 UTC). + -- Time is represented as a signed 64 bit signed integer count of + -- nanoseconds since the "epoch" 2150-01-01 00:00:00 UTC. Thus a value of 0 + -- represents the epoch. As of this writing, the epoch is in the future, + -- so Time values returned by Clock will be negative. + -- + -- Time values produced by Time_Of are internally normalized to UTC + -- regardless of their local time zone. This representation ensures correct + -- handling of leap seconds as well as performing arithmetic. In Ada 95, + -- Split and Time_Of will treat a time value as being in the local time + -- zone, in Ada 2005, Split and Time_Of will treat a time value as being in + -- the designated time zone by the formal parameter or in UTC by + -- default. The size of the type is large enough to cover the Ada + -- range of time (1901-01-01T00:00:00.0 UTC - 2399-12-31T23:59:59.9 + -- UTC). -- -- Leap Seconds -- @@ -234,8 +238,8 @@ private function Epoch_Offset return Time_Rep; pragma Inline (Epoch_Offset); - -- Return the difference between 2150-1-1 UTC and 1970-1-1 UTC expressed in - -- nanoseconds. Note that year 2100 is non-leap. + -- Return the difference between our epoch and 1970-1-1 UTC (the Unix + -- epoch) expressed in nanoseconds. Note that year 2100 is non-leap. Days_In_Month : constant array (Month_Number) of Day_Number := (31, 28, 31, 30, 31, 30, 31, 31, 30, 31, 30, 31); diff --git a/gcc
[Ada] Ada2020: AI12-0198 potentially unevaluated array components
This patch implements AI12-0198-1, which enforces detecting components which belong to a non-static or null range of index values of an array aggregate. Tested on x86_64-pc-linux-gnu, committed on trunk gcc/ada/ * sem_util.ads (Interval_Lists.Aggregate_Intervals): New subprogram. * sem_util.adb (Has_Null_Others_Choice, Non_Static_Or_Null_Range, Interval_Lists.Aggregate_Intervals): New subprograms. (Is_Potentially_Unevaluated): Adding support to detect potentially unevaluated components of array aggregates.diff --git a/gcc/ada/sem_util.adb b/gcc/ada/sem_util.adb --- a/gcc/ada/sem_util.adb +++ b/gcc/ada/sem_util.adb @@ -17694,9 +17694,81 @@ package body Sem_Util is function Is_Potentially_Unevaluated (N : Node_Id) return Boolean is + function Has_Null_Others_Choice (Aggr : Node_Id) return Boolean; + -- Aggr is an array aggregate with static bounds and an others clause; + -- return True if the others choice of the given array aggregate does + -- not cover any component (i.e. is null). + + function Non_Static_Or_Null_Range (N : Node_Id) return Boolean; + -- Return True if the given range is nonstatic or null + + + -- Has_Null_Others_Choice -- + + + function Has_Null_Others_Choice (Aggr : Node_Id) return Boolean is + Idx : constant Node_Id := First_Index (Etype (Aggr)); + Hiv : constant Uint := Expr_Value (Type_High_Bound (Etype (Idx))); + Lov : constant Uint := Expr_Value (Type_Low_Bound (Etype (Idx))); + + begin + declare +Intervals : constant Interval_Lists.Discrete_Interval_List := + Interval_Lists.Aggregate_Intervals (Aggr); + + begin +-- The others choice is null if, after normalization, we +-- have a single interval covering the whole aggregate. + +return Intervals'Length = 1 + and then +Intervals (Intervals'First).Low = Lov + and then +Intervals (Intervals'First).High = Hiv; + end; + + -- If the aggregate is malformed (that is, indexes are not disjoint) + -- then no action is needed at this stage; the error will be reported + -- later by the frontend. + + exception + when Interval_Lists.Intervals_Error => +return False; + end Has_Null_Others_Choice; + + -- + -- Non_Static_Or_Null_Range -- + -- + + function Non_Static_Or_Null_Range (N : Node_Id) return Boolean is + Low, High : Node_Id; + + begin + Get_Index_Bounds (N, Low, High); + + -- Check static bounds + + if not Compile_Time_Known_Value (Low) + or else not Compile_Time_Known_Value (High) + then +return True; + + -- Check null range + + elsif Expr_Value (High) < Expr_Value (Low) then +return True; + end if; + + return False; + end Non_Static_Or_Null_Range; + + -- Local variables + Par : Node_Id; Expr : Node_Id; + -- Start of processing for Is_Potentially_Unevaluated + begin Expr := N; Par := N; @@ -17732,6 +17804,8 @@ package body Sem_Util is N_Not_In, N_Or_Else, N_Quantified_Expression) +and then not (Nkind (Par) = N_Aggregate +and then Is_Array_Type (Etype (Par))) loop Expr := Par; Par := Parent (Par); @@ -17776,6 +17850,55 @@ package body Sem_Util is elsif Nkind (Par) = N_Quantified_Expression then return Expr = Condition (Par); + elsif Nkind (Par) = N_Aggregate +and then Is_Array_Type (Etype (Par)) +and then Nkind (Expr) = N_Component_Association + then + declare +Choice : Node_Id; +In_Others_Choice : Boolean := False; + + begin +-- The expression of an array_component_association is potentially +-- unevaluated if the associated choice is a subtype_indication or +-- range that defines a nonstatic or null range. + +Choice := First (Choices (Expr)); +while Present (Choice) loop + if Nkind (Choice) = N_Range + and then Non_Static_Or_Null_Range (Choice) + then + return True; + + elsif Nkind (Choice) = N_Identifier + and then Present (Scalar_Range (Etype (Choice))) + and then + Non_Static_Or_Null_Range (Scalar_Range (Etype (Choice))) + then + return True; + + elsif Nkind (Choice) = N_Others_Choice then +
[Ada] Avoid forced loading of System.Priority in CodePeer mode
The comment for routine Is_RTE explains that it is a more efficient equivalent of an equality testing with routine RTE. Tested on x86_64-pc-linux-gnu, committed on trunk gcc/ada/ * checks.adb (Apply_Scalar_Range_Check): Use Is_RTE.diff --git a/gcc/ada/checks.adb b/gcc/ada/checks.adb --- a/gcc/ada/checks.adb +++ b/gcc/ada/checks.adb @@ -3326,7 +3326,7 @@ package body Checks is -- provide a wider range. if not CodePeer_Mode - or else Target_Typ /= RTE (RE_Priority) + or else not Is_RTE (Target_Typ, RE_Priority) then Bad_Value; end if;
Re: [PATCH][GCC][Arm] PR target/95646: Do not clobber callee saved registers with CMSE
On 30/06/2020 14:50, Andre Vieira (lists) wrote: On 29/06/2020 11:15, Christophe Lyon wrote: On Mon, 29 Jun 2020 at 10:56, Andre Vieira (lists) wrote: On 23/06/2020 21:52, Christophe Lyon wrote: On Tue, 23 Jun 2020 at 15:28, Andre Vieira (lists) wrote: On 23/06/2020 13:10, Kyrylo Tkachov wrote: -Original Message- From: Andre Vieira (lists) Sent: 22 June 2020 09:52 To: gcc-patches@gcc.gnu.org Cc: Kyrylo Tkachov Subject: [PATCH][GCC][Arm] PR target/95646: Do not clobber callee saved registers with CMSE Hi, As reported in bugzilla when the -mcmse option is used while compiling for size (-Os) with a thumb-1 target the generated code will clear the registers r7-r10. These however are callee saved and should be preserved accross ABI boundaries. The reason this happens is because these registers are made "fixed" when optimising for size with Thumb-1 in a way to make sure they are not used, as pushing and popping hi-registers requires extra moves to and from LO_REGS. To fix this, this patch uses 'callee_saved_reg_p', which accounts for this optimisation, instead of 'call_used_or_fixed_reg_p'. Be aware of 'callee_saved_reg_p''s definition, as it does still take call used registers into account, which aren't callee_saved in my opinion, so it is a rather misnoemer, works in our advantage here though as it does exactly what we need. Regression tested on arm-none-eabi. Is this OK for trunk? (Will eventually backport to previous versions if stable.) Ok. Thanks, Kyrill As I was getting ready to push this I noticed I didn't add any skip-ifs to prevent this failing with specific target options. So here's a new version with those. Still OK? Hi, This is not sufficient to skip arm-linux-gnueabi* configs built with non-default cpu/fpu. For instance, with arm-linux-gnueabihf --with-cpu=cortex-a9 --with-fpu=neon-fp16 --with-float=hard I see: FAIL: gcc.target/arm/pr95646.c (test for excess errors) Excess errors: cc1: error: ARMv8-M Security Extensions incompatible with selected FPU cc1: error: target CPU does not support ARM mode and the testcase is compiled with -mcpu=cortex-m23 -mcmse -Os Resending as I don't think my earlier one made it to the lists (sorry if you are receiving this double!) I'm not following this, before I go off and try to reproduce it, what do you mean by 'the testcase is compiled with -mcpu=cortex-m23 -mcmse -Os'? These are the options you are seeing in the log file? Surely they should override the default options? Only thing I can think of is this might need an extra -mfloat-abi=soft to make sure it overrides the default float-abi. Could you give that a try? No it doesn't make a difference alone. I also had to add: -mfpu=auto (that clears the above warning) -mthumb otherwise we now get cc1: error: target CPU does not support ARM mode Looks like some effective-target machinery is needed So I had a look at this, I was pretty sure that -mfloat-abi=soft overwrote -mfpu=<>, which in large it does, as in no FP instructions will be generated but the error you see only checks for the right number of FP registers. Which doesn't check whether 'TARGET_HARD_FLOAT' is set or not. I'll fix this too and use the check-effective-target for armv8-m.base for this test as it is indeed a better approach than my bag of skip-ifs. I'm testing it locally to make sure my changes don't break anything. Cheers, Andre Hi, Sorry for the delay. So I changed the test to use the effective-target machinery as you suggested and I also made sure that you don't get the "ARMv8-M Security Extensions incompatible with selected FPU" when -mfloat-abi=soft. Further changed 'asm' to '__asm__' to avoid failures with '-std=' options. Regression tested on arm-none-eabi. @Christophe: could you test this for your configuration, shouldn't fail anymore! Is this OK for trunk? Cheers, Andre gcc/ChangeLog: 2020-07-06 Andre Vieira * config/arm/arm.c (arm_options_perform_arch_sanity_checks): Do not check +D32 for CMSE if -mfloat-abi=soft gcc/testsuite/ChangeLog: 2020-07-06 Andre Vieira * gcc.target/arm/pr95646.c: Fix testism. Christophe Cheers, Andre Christophe Cheers, Andre Cheers, Andre gcc/ChangeLog: 2020-06-22 Andre Vieira PR target/95646 * config/arm/arm.c: (cmse_nonsecure_entry_clear_before_return): Use 'callee_saved_reg_p' instead of 'calL_used_or_fixed_reg_p'. gcc/testsuite/ChangeLog: 2020-06-22 Andre Vieira PR target/95646 * gcc.target/arm/pr95646.c: New test. diff --git a/gcc/config/arm/arm.c b/gcc/config/arm/arm.c index dac9a6fb5c41ce42cd7a278b417eab25239a043c..38500220bfb2a1ddbbff15eb552451701f7256d5 100644 --- a/gcc/config/arm/arm.c +++ b/gcc/config/arm/arm.c @@ -3834,7 +3834,7 @@ arm_options_perform_arch_sanity_checks (void) /* We don't clear D16-D31 VFP registers for cmse_nonsecure_call functions and ARMv8-M Baseline and Mainline do not a
[PATCH] tree-optimization/96075 - fix bogus misalignment calculation
This fixes bogus misalignment calculation for negative steps
since an assertion a previous comment indicated no longer holds:
/* DR_STEP(dr) is the same as -TYPE_SIZE of the scalar type,
otherwise we wouldn't be here. */
Thus the following replaces DR_STEP by -TYPE_SIZE.
Bootstrapped and tested on x86_64-unknown-linux-gnu, applied to trunk
sofar.
Richard.
2020-07-06 Richard Biener
PR tree-optimization/96075
* tree-vect-data-refs.c (vect_compute_data_ref_alignment): Use
TYPE_SIZE_UNIT of the vector component type instead of DR_STEP
for the misalignment calculation for negative step.
* gcc.dg/vect/slp-46.c: New testcase.
---
gcc/testsuite/gcc.dg/vect/slp-46.c | 96 ++
gcc/tree-vect-data-refs.c | 2 +-
2 files changed, 97 insertions(+), 1 deletion(-)
create mode 100644 gcc/testsuite/gcc.dg/vect/slp-46.c
diff --git a/gcc/testsuite/gcc.dg/vect/slp-46.c
b/gcc/testsuite/gcc.dg/vect/slp-46.c
new file mode 100644
index 000..17dfa285ec1
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/vect/slp-46.c
@@ -0,0 +1,96 @@
+/* { dg-require-effective-target vect_double } */
+
+#include "tree-vect.h"
+
+double x[1024], y[1024];
+
+void __attribute__((noipa)) foo()
+{
+ for (int i = 0; i < 512; ++i)
+{
+ x[2*i] = y[i];
+ x[2*i+1] = y[i];
+}
+}
+
+void __attribute__((noipa)) bar()
+{
+ for (int i = 0; i < 512; ++i)
+{
+ x[2*i] = y[2*i];
+ x[2*i+1] = y[2*i];
+}
+}
+
+void __attribute__((noipa)) baz()
+{
+ for (int i = 0; i < 512; ++i)
+{
+ x[2*i] = y[511-i];
+ x[2*i+1] = y[511-i];
+}
+}
+
+void __attribute__((noipa)) boo()
+{
+ for (int i = 0; i < 512; ++i)
+{
+ x[2*i] = y[2*(511-i)];
+ x[2*i+1] = y[2*(511-i)];
+}
+}
+
+int
+main ()
+{
+ check_vect ();
+
+ for (int i = 0; i < 1024; ++i)
+{
+ x[i] = 0;
+ y[i] = i;
+ __asm__ volatile ("");
+}
+
+ foo ();
+ for (int i = 0; i < 1024; ++i)
+if (x[i] != y[i/2])
+ abort ();
+
+ for (int i = 0; i < 1024; ++i)
+{
+ x[i] = 0;
+ __asm__ volatile ("");
+}
+
+ bar ();
+ for (int i = 0; i < 1024; ++i)
+if (x[i] != y[2*(i/2)])
+ abort ();
+
+ for (int i = 0; i < 1024; ++i)
+{
+ x[i] = 0;
+ __asm__ volatile ("");
+}
+
+ baz ();
+ for (int i = 0; i < 1024; ++i)
+if (x[i] != y[511 - i/2])
+ abort ();
+
+ for (int i = 0; i < 1024; ++i)
+{
+ x[i] = 0;
+ __asm__ volatile ("");
+}
+
+ boo ();
+ for (int i = 0; i < 1024; ++i)
+if (x[i] != y[2*(511 - i/2)])
+ abort ();
+
+ return 0;
+}
+
+/* { dg-final { scan-tree-dump-times "vectorizing stmts using SLP" 2 "vect" }
} */
diff --git a/gcc/tree-vect-data-refs.c b/gcc/tree-vect-data-refs.c
index 2462276e7c2..959c2d3378f 100644
--- a/gcc/tree-vect-data-refs.c
+++ b/gcc/tree-vect-data-refs.c
@@ -1109,7 +1109,7 @@ vect_compute_data_ref_alignment (vec_info *vinfo,
dr_vec_info *dr_info)
if (tree_int_cst_sgn (drb->step) < 0)
/* PLUS because STEP is negative. */
misalignment += ((TYPE_VECTOR_SUBPARTS (vectype) - 1)
-* TREE_INT_CST_LOW (drb->step));
+* -TREE_INT_CST_LOW (TYPE_SIZE_UNIT (TREE_TYPE
(vectype;
unsigned int const_misalignment;
if (!known_misalignment (misalignment, vect_align_c, &const_misalignment))
--
2.26.2
[PATCH] c++: Allow empty-declaration in C++11 and later (PR 96068)
Since C++11 a semim-colon on its own at namespace scope is not invalid,
so do not give a pedantic diagnostic about it.
gcc/cp/ChangeLog:
PR c++/96068
* parser.c (cp_parser_toplevel_declaration): Only do pedwarn for
empty-declaration in C++98.
gcc/testsuite/ChangeLog:
* g++.old-deja/g++.bugs/900404_04.C: Add c++98_only selector to
dg-error for extra ';'.
* g++.old-deja/g++.law/missed-error2.C: Likewise.
Tested x86_64-linux, OK for trunk?
commit 0070906d6384341d356e7c925f99e978e748136a
Author: Jonathan Wakely
Date: Mon Jul 6 15:58:33 2020 +0100
c++: Allow empty-declaration in C++11 and later (PR 96068)
Since C++11 a semim-colon on its own at namespace scope is not invalid,
so do not give a pedantic diagnostic about it.
gcc/cp/ChangeLog:
PR c++/96068
* parser.c (cp_parser_toplevel_declaration): Only do pedwarn for
empty-declaration in C++98.
gcc/testsuite/ChangeLog:
* g++.old-deja/g++.bugs/900404_04.C: Add c++98_only selector to
dg-error for extra ';'.
* g++.old-deja/g++.law/missed-error2.C: Likewise.
diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index 815582c825e..6e7637c6016 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -13502,10 +13502,11 @@ cp_parser_toplevel_declaration (cp_parser* parser)
cp_parser_pragma (parser, pragma_external, NULL);
else if (token->type == CPP_SEMICOLON)
{
- /* A declaration consisting of a single semicolon is
-invalid. Allow it unless we're being pedantic. */
cp_lexer_consume_token (parser->lexer);
- pedwarn (input_location, OPT_Wpedantic, "extra %<;%>");
+ /* A declaration consisting of a single semicolon is invalid
+ * before C++11. Allow it unless we're being pedantic. */
+ if (cxx_dialect < cxx11)
+ pedwarn (input_location, OPT_Wpedantic, "extra %<;%>");
}
else
/* Parse the declaration itself. */
diff --git a/gcc/testsuite/g++.old-deja/g++.bugs/900404_04.C
b/gcc/testsuite/g++.old-deja/g++.bugs/900404_04.C
index 04ff6690549..86117939637 100644
--- a/gcc/testsuite/g++.old-deja/g++.bugs/900404_04.C
+++ b/gcc/testsuite/g++.old-deja/g++.bugs/900404_04.C
@@ -7,12 +7,14 @@
// elaborated-type-specifier with class key, or an enum-specifier. The
// declaration below contains neither.
-// g++ fails to flag errors for such usage.
+// Since C++11 this is allowed as an empty-declaration.
+
+// g++ fails to flag errors for such usage in C++98.
// keywords: semicolon, vacuous, file scope, declaration
int i;
-; // { dg-error "extra ';'" }
+; // { dg-error "extra ';'" "" { target c++98_only } 0 }
int main () { return 0; }
diff --git a/gcc/testsuite/g++.old-deja/g++.law/missed-error2.C
b/gcc/testsuite/g++.old-deja/g++.law/missed-error2.C
index ee69e1156cc..eaf8c01b679 100644
--- a/gcc/testsuite/g++.old-deja/g++.law/missed-error2.C
+++ b/gcc/testsuite/g++.old-deja/g++.law/missed-error2.C
@@ -15,9 +15,9 @@
#endif
inline int max(int a, int b) {return a > b ? a : b;}; // { dg-message "note" }
- // { dg-error "extra ';'" "extra ;" { target *-*-* } .-1 }
+ // { dg-error "extra ';'" "extra ;" { target c++98_only } .-1 }
inline double max(double a, double b) {return a > b ? a : b;}; // { dg-message
"note" } candidate
- // { dg-error "extra ';'" "extra ;" { target *-*-* } .-1 }
+ // { dg-error "extra ';'" "extra ;" { target c++98_only } .-1 }
int main() {
static void foo(int i, int j, double x, double y) ;// { dg-error "" } .*
Re: [PATCH] c++: Allow empty-declaration in C++11 and later (PR 96068)
On 7/6/20 11:01 AM, Jonathan Wakely wrote: Since C++11 a semim-colon on its own at namespace scope is not invalid, so do not give a pedantic diagnostic about it. gcc/cp/ChangeLog: PR c++/96068 * parser.c (cp_parser_toplevel_declaration): Only do pedwarn for empty-declaration in C++98. gcc/testsuite/ChangeLog: * g++.old-deja/g++.bugs/900404_04.C: Add c++98_only selector to dg-error for extra ';'. * g++.old-deja/g++.law/missed-error2.C: Likewise. Tested x86_64-linux, OK for trunk? ok thanks. -- Nathan Sidwell
[PATCH] aarch64: Change costs for TX2 to expose more vectorization opportunities
This patch changes some vector costs for TX2 so that
more vectorizations beneficial for TX2 chip can happen.
The new cost model makes the x264 benchmark of CPU2017
7% faster with no negative performance impact on other
benchmarks.
Bootstrapped on linux-aarch64
2020-07-06 Anton Youdkevitch
gcc/
* config/aarch64/aarch64.c (thunderx2t99_regmove_cost):
Change instruction cost
(thunderx2t99_vector_cost): Likewise
>From 3440e019c05fe5b565041cad549c6eefa2004a2b Mon Sep 17 00:00:00 2001
From: Anton Youdkevitch
Date: Tue, 26 May 2020 04:23:04 -0700
Subject: [PATCH] Change costs for TX2 to expose more vectorization opportunities
Make the costs such that they do not exaclty reflect
the actual instructions costs from the manual but make
the codegen emit the code we want it to.
---
gcc/config/aarch64/aarch64.c | 18 +-
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
index e92c7e6..18c2251 100644
--- a/gcc/config/aarch64/aarch64.c
+++ b/gcc/config/aarch64/aarch64.c
@@ -535,9 +535,9 @@ static const struct cpu_regmove_cost thunderx2t99_regmove_cost =
{
1, /* GP2GP */
/* Avoid the use of int<->fp moves for spilling. */
- 8, /* GP2FP */
- 8, /* FP2GP */
- 4 /* FP2FP */
+ 5, /* GP2FP */
+ 6, /* FP2GP */
+ 3, /* FP2FP */
};
static const struct cpu_regmove_cost thunderx3t110_regmove_cost =
@@ -704,15 +704,15 @@ static const struct cpu_vector_cost thunderx2t99_vector_cost =
6, /* scalar_fp_stmt_cost */
4, /* scalar_load_cost */
1, /* scalar_store_cost */
- 5, /* vec_int_stmt_cost */
- 6, /* vec_fp_stmt_cost */
+ 4, /* vec_int_stmt_cost */
+ 5, /* vec_fp_stmt_cost */
10, /* vec_permute_cost */
6, /* vec_to_scalar_cost */
5, /* scalar_to_vec_cost */
- 8, /* vec_align_load_cost */
- 8, /* vec_unalign_load_cost */
- 4, /* vec_unalign_store_cost */
- 4, /* vec_store_cost */
+ 4, /* vec_align_load_cost */
+ 4, /* vec_unalign_load_cost */
+ 1, /* vec_unalign_store_cost */
+ 1, /* vec_store_cost */
2, /* cond_taken_branch_cost */
1 /* cond_not_taken_branch_cost */
};
--
2.7.4
[committed] libstdc++: Fix -Wmismatched-tags warnings (PR 96063)
libstdc++-v3/ChangeLog:
* include/bits/fs_dir.h: Use consistent tag in class-head.
* include/bits/localefwd.h: Likwise.
* include/bits/regex.h: Likwise.
* include/bits/stl_map.h: Likwise.
* include/bits/stl_multimap.h: Likwise.
* include/bits/stl_multiset.h: Likwise.
* include/bits/stl_set.h: Likwise.
* include/std/complex: Likwise.
* include/std/functional: Likwise.
* include/std/future: Likwise.
* include/std/system_error: Likwise.
* include/std/thread: Likwise.
* include/std/tuple: Likwise.
* include/std/type_traits: Likwise.
* include/std/valarray: Likwise.
Tested powerpc64le-linux, committed to trunk.
commit 0e5abeb0cb06f9798a559e1238839c5d6581657e
Author: Jonathan Wakely
Date: Mon Jul 6 16:19:23 2020 +0100
libstdc++: Fix -Wmismatched-tags warnings (PR 96063)
libstdc++-v3/ChangeLog:
* include/bits/fs_dir.h: Use consistent tag in class-head.
* include/bits/localefwd.h: Likwise.
* include/bits/regex.h: Likwise.
* include/bits/stl_map.h: Likwise.
* include/bits/stl_multimap.h: Likwise.
* include/bits/stl_multiset.h: Likwise.
* include/bits/stl_set.h: Likwise.
* include/std/complex: Likwise.
* include/std/functional: Likwise.
* include/std/future: Likwise.
* include/std/system_error: Likwise.
* include/std/thread: Likwise.
* include/std/tuple: Likwise.
* include/std/type_traits: Likwise.
* include/std/valarray: Likwise.
diff --git a/libstdc++-v3/include/bits/fs_dir.h
b/libstdc++-v3/include/bits/fs_dir.h
index 686dfce6e5f..264fa783bbb 100644
--- a/libstdc++-v3/include/bits/fs_dir.h
+++ b/libstdc++-v3/include/bits/fs_dir.h
@@ -312,7 +312,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11
#endif
private:
-friend class _Dir;
+friend struct _Dir;
friend class directory_iterator;
friend class recursive_directory_iterator;
diff --git a/libstdc++-v3/include/bits/localefwd.h
b/libstdc++-v3/include/bits/localefwd.h
index 8fecc0b6525..824b07b6476 100644
--- a/libstdc++-v3/include/bits/localefwd.h
+++ b/libstdc++-v3/include/bits/localefwd.h
@@ -122,7 +122,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
tolower(_CharT, const locale&);
// 22.2.1 and 22.2.1.3 ctype
- class ctype_base;
+ struct ctype_base;
template
class ctype;
template<> class ctype;
@@ -200,7 +200,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11
_GLIBCXX_END_NAMESPACE_CXX11
// 22.2.7 message retrieval
- class messages_base;
+ struct messages_base;
_GLIBCXX_BEGIN_NAMESPACE_CXX11
template
class messages;
diff --git a/libstdc++-v3/include/bits/regex.h
b/libstdc++-v3/include/bits/regex.h
index 4032fd7559b..31ebcc1eb86 100644
--- a/libstdc++-v3/include/bits/regex.h
+++ b/libstdc++-v3/include/bits/regex.h
@@ -77,12 +77,13 @@ _GLIBCXX_BEGIN_NAMESPACE_CXX11
* satisfies the requirements of such a traits class.
*/
template
-struct regex_traits
+class regex_traits
{
public:
typedef _Ch_type char_type;
typedef std::basic_string string_type;
typedef std::locale locale_type;
+
private:
struct _RegexMask
{
diff --git a/libstdc++-v3/include/bits/stl_map.h
b/libstdc++-v3/include/bits/stl_map.h
index 2772d11462e..7120f9bcfc8 100644
--- a/libstdc++-v3/include/bits/stl_map.h
+++ b/libstdc++-v3/include/bits/stl_map.h
@@ -635,7 +635,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
{ return _M_t._M_reinsert_node_hint_unique(__hint, std::move(__nh)); }
template
- friend class std::_Rb_tree_merge_helper;
+ friend struct std::_Rb_tree_merge_helper;
template
void
diff --git a/libstdc++-v3/include/bits/stl_multimap.h
b/libstdc++-v3/include/bits/stl_multimap.h
index 65f4d0117a4..2791a3c6b3c 100644
--- a/libstdc++-v3/include/bits/stl_multimap.h
+++ b/libstdc++-v3/include/bits/stl_multimap.h
@@ -651,7 +651,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
{ return _M_t._M_reinsert_node_hint_equal(__hint, std::move(__nh)); }
template
- friend class std::_Rb_tree_merge_helper;
+ friend struct std::_Rb_tree_merge_helper;
template
void
diff --git a/libstdc++-v3/include/bits/stl_multiset.h
b/libstdc++-v3/include/bits/stl_multiset.h
index bf6ae7de095..34ba7364f15 100644
--- a/libstdc++-v3/include/bits/stl_multiset.h
+++ b/libstdc++-v3/include/bits/stl_multiset.h
@@ -589,7 +589,7 @@ _GLIBCXX_BEGIN_NAMESPACE_CONTAINER
{ return _M_t._M_reinsert_node_hint_equal(__hint, std::move(__nh)); }
template
- friend class std::_Rb_tree_merge_helper;
+ friend struct std::_Rb_tree_merge_helper;
template
void
diff --git a/libstdc++-v3/include/bits/stl_set.h
b/libstdc++-v3/include/bits/stl_set.h
index da426650815..c86312
Re: [PATCH 7/9] [OpenACC] Do not strip GOMP_MAP_TO_PSET/GOMP_MAP_POINTER for enter/exit data directives
Hi Julian!
On 2020-06-16T15:39:43-0700, Julian Brown wrote:
> When attaching pointers in Fortran, OpenACC 2.6 specifies that a
> descriptor must be copied to the target at the same time (see next
> patch). That means that stripping GOMP_MAP_TO_PSET (and lesserly,
> GOMP_MAP_POINTER), which was behaviour introduced by the manual deep-copy
> middle-end support patch, was probably wrong.
>
> That arguably answers some of the questions at the end of:
>
> https://gcc.gnu.org/pipermail/gcc-patches/2020-June/547424.html
ACK.
> It appears that the user can (but certainly should not!) map a synthesized
> array descriptor using an "enter data" operation that can go out of
> scope before that data is unmapped. It would be nice to give a warning
> for an attempt to do such a thing, though I have no idea if that's
> possible in practice.
That's a rather complex scenario. ;-)
If I'm understanding this right, what we need to show is that an object
is created as a persistent, visible device copy, with state initialized
by 'enter data', and then any 'GOMP_MAP_TO_PSET' etc. that come with each
OpenACC 'parallel' etc. are no-ops (because the object is present
already).
My attached (new) 'libgomp.oacc-fortran/dynamic-pointer-1.f90' would seem
to be a conceptually simple test case for this, using a Fortran
'pointer'. (I hope I got my Fortran right, please verify.) This test
case doesn't work in current master and releases/gcc-10 branches (because
we don't create the persistent, visible device copy), and is "enabled" by
your patch posted here. I'm intentionally not saying "regression fixed"
or something like that, because it also doesn't work before all the
"OpenACC 2.6 deep copy: middle-end parts" etc. changes... (Maybe because
of wrong handling of 'GOMP_MAP_TO_PSET' back then, too? Just mentioning
that for completeness; I don't think we need to investigate that now.)
Please include some such rationale in the commit log, or "even" as source
code comments, as makes sense. This code surely is complicated/complex
to grasp.
> gcc/
> * gimplify.c (gimplify_scan_omp_clauses): Do not strip
> GOMP_MAP_TO_PSET/GOMP_MAP_POINTER for OpenACC enter/exit data
> directives.
Should reference PR92929 here.
Please include my attached (new)
'libgomp.oacc-fortran/dynamic-pointer-1.f90' (assuming that one makes
sense to you), and then this is OK for master and releases/gcc-10
branches.
We then (later) still need to resolve other items discussed in PR92929
"OpenACC/OpenMP 'target' 'exit data'/'update' optimizations".
Grüße
Thomas
> gcc/gimplify.c | 11 ++-
> gcc/testsuite/gfortran.dg/goacc/finalize-1.f | 4 ++--
> 2 files changed, 4 insertions(+), 11 deletions(-)
>
> diff --git a/gcc/gimplify.c b/gcc/gimplify.c
> index 9851edfc4db..aa6853f0dcc 100644
> --- a/gcc/gimplify.c
> +++ b/gcc/gimplify.c
> @@ -8767,6 +8767,8 @@ gimplify_scan_omp_clauses (tree *list_p, gimple_seq
> *pre_p,
> case OMP_TARGET_DATA:
> case OMP_TARGET_ENTER_DATA:
> case OMP_TARGET_EXIT_DATA:
> + case OACC_ENTER_DATA:
> + case OACC_EXIT_DATA:
> case OACC_HOST_DATA:
> if (OMP_CLAUSE_MAP_KIND (c) == GOMP_MAP_FIRSTPRIVATE_POINTER
> || (OMP_CLAUSE_MAP_KIND (c)
> @@ -8775,15 +8777,6 @@ gimplify_scan_omp_clauses (tree *list_p, gimple_seq
> *pre_p,
> mapped, but not the pointer to it. */
> remove = true;
> break;
> - case OACC_ENTER_DATA:
> - case OACC_EXIT_DATA:
> - if (OMP_CLAUSE_MAP_KIND (c) == GOMP_MAP_POINTER
> - || OMP_CLAUSE_MAP_KIND (c) == GOMP_MAP_TO_PSET
> - || OMP_CLAUSE_MAP_KIND (c) == GOMP_MAP_FIRSTPRIVATE_POINTER
> - || (OMP_CLAUSE_MAP_KIND (c)
> - == GOMP_MAP_FIRSTPRIVATE_REFERENCE))
> - remove = true;
> - break;
> default:
> break;
> }
> diff --git a/gcc/testsuite/gfortran.dg/goacc/finalize-1.f
> b/gcc/testsuite/gfortran.dg/goacc/finalize-1.f
> index 1e2e3e94b8a..ca642156e9f 100644
> --- a/gcc/testsuite/gfortran.dg/goacc/finalize-1.f
> +++ b/gcc/testsuite/gfortran.dg/goacc/finalize-1.f
> @@ -21,7 +21,7 @@
>
> !$ACC EXIT DATA FINALIZE DELETE (del_f_p(2:5))
> ! { dg-final { scan-tree-dump-times "(?n)#pragma acc exit data
> map\\(release:\\*\\(c_char \\*\\) parm\\.0\\.data \\\[len: \[^\\\]\]+\\\]\\)
> map\\(to:del_f_p \\\[pointer set, len: \[0-9\]+\\\]\\)
> map\\(alloc:\\(integer\\(kind=1\\)\\\[0:\\\] \\* restrict\\) del_f_p\\.data
> \\\[pointer assign, bias: \\(sizetype\\) parm\\.0\\.data - \\(sizetype\\)
> del_f_p\\.data\\\]\\) finalize;$" 1 "original" } }
> -! { dg-final { scan-tree-dump-times "(?n)#pragma omp target
> oacc_enter_exit_data map\\(delete:MEM\\\[\\(c_char \\*\\)\[^\\\]\]+\\\]
> \\\[len: \[^\\\]\]+\\\]\\) finalize$" 1 "gimple" } }
> +! { dg-final { scan-tree-dump-times "(?n)#pragma omp target
>
c++: Always use pushdecl for exception library helpers
The ABI exception helpers like __throw were being created by first looking for them, and then adding if not found. Primarily because libitm wasn't declaring them with the correct exception specifiers. I fixed libitm a while back, so let's just use push_library_fn and let the symbol table machinery deal with duplicates. push_library_fn was making the assumtion there wasn't already a decl available, by always returning the new decl. Bad things would happen if there was a duplicate, because duplicate_decls explicitly gcc_frees the new decl. Fixed by having it return whatever pushdecl returns. gcc/cp/ * decl.c (push_library_fn): Return the decl pushdecl_toplevel returns. * except.c (verify_library_fn): Replace with ... (declare_library_fn_1): ... this fn. Always push the fn. (declare_library_fn): Call it. (build_throw): Call declare_library_fn_1. gcc/testsuite/ * g++.dg/eh/builtin10.C: Adjust expected errors. * g++.dg/eh/builtin11.C: Likewise. * g++.dg/eh/builtin5.C: Likewise. * g++.dg/eh/builtin6.C: Likewise. * g++.dg/eh/builtin7.C: Likewise. * g++.dg/eh/builtin9.C: Likewise. * g++.dg/parse/crash55.C: Likewise. -- Nathan Sidwell
Re: [PATCH] aarch64: Change costs for TX2 to expose more vectorization opportunities
Anton Youdkevitch writes:
> This patch changes some vector costs for TX2 so that
> more vectorizations beneficial for TX2 chip can happen.
>
> The new cost model makes the x264 benchmark of CPU2017
> 7% faster with no negative performance impact on other
> benchmarks.
>
> Bootstrapped on linux-aarch64
>
> 2020-07-06 Anton Youdkevitch
> gcc/
> * config/aarch64/aarch64.c (thunderx2t99_regmove_cost):
> Change instruction cost
> (thunderx2t99_vector_cost): Likewise
OK if Andrew agrees.
Thanks,
Richard
>
> From 3440e019c05fe5b565041cad549c6eefa2004a2b Mon Sep 17 00:00:00 2001
> From: Anton Youdkevitch
> Date: Tue, 26 May 2020 04:23:04 -0700
> Subject: [PATCH] Change costs for TX2 to expose more vectorization
> opportunities
>
> Make the costs such that they do not exaclty reflect
> the actual instructions costs from the manual but make
> the codegen emit the code we want it to.
> ---
> gcc/config/aarch64/aarch64.c | 18 +-
> 1 file changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
> index e92c7e6..18c2251 100644
> --- a/gcc/config/aarch64/aarch64.c
> +++ b/gcc/config/aarch64/aarch64.c
> @@ -535,9 +535,9 @@ static const struct cpu_regmove_cost
> thunderx2t99_regmove_cost =
> {
>1, /* GP2GP */
>/* Avoid the use of int<->fp moves for spilling. */
> - 8, /* GP2FP */
> - 8, /* FP2GP */
> - 4 /* FP2FP */
> + 5, /* GP2FP */
> + 6, /* FP2GP */
> + 3, /* FP2FP */
> };
>
> static const struct cpu_regmove_cost thunderx3t110_regmove_cost =
> @@ -704,15 +704,15 @@ static const struct cpu_vector_cost
> thunderx2t99_vector_cost =
>6, /* scalar_fp_stmt_cost */
>4, /* scalar_load_cost */
>1, /* scalar_store_cost */
> - 5, /* vec_int_stmt_cost */
> - 6, /* vec_fp_stmt_cost */
> + 4, /* vec_int_stmt_cost */
> + 5, /* vec_fp_stmt_cost */
>10, /* vec_permute_cost */
>6, /* vec_to_scalar_cost */
>5, /* scalar_to_vec_cost */
> - 8, /* vec_align_load_cost */
> - 8, /* vec_unalign_load_cost */
> - 4, /* vec_unalign_store_cost */
> - 4, /* vec_store_cost */
> + 4, /* vec_align_load_cost */
> + 4, /* vec_unalign_load_cost */
> + 1, /* vec_unalign_store_cost */
> + 1, /* vec_store_cost */
>2, /* cond_taken_branch_cost */
>1 /* cond_not_taken_branch_cost */
> };
Re: [EXT] Re: [PATCH] aarch64: Change costs for TX2 to expose more vectorization opportunities
I approve of this patch. I'm responsible for GCC for TX2 at Marvell. Andrew
Pinski should certainly chime in if he wants.
Joel
On 7/6/20, 10:48 AM, "Gcc-patches on behalf of Richard Sandiford"
wrote:
External Email
--
Anton Youdkevitch writes:
> This patch changes some vector costs for TX2 so that
> more vectorizations beneficial for TX2 chip can happen.
>
> The new cost model makes the x264 benchmark of CPU2017
> 7% faster with no negative performance impact on other
> benchmarks.
>
> Bootstrapped on linux-aarch64
>
> 2020-07-06 Anton Youdkevitch
> gcc/
> * config/aarch64/aarch64.c (thunderx2t99_regmove_cost):
> Change instruction cost
> (thunderx2t99_vector_cost): Likewise
OK if Andrew agrees.
Thanks,
Richard
>
> From 3440e019c05fe5b565041cad549c6eefa2004a2b Mon Sep 17 00:00:00 2001
> From: Anton Youdkevitch
> Date: Tue, 26 May 2020 04:23:04 -0700
> Subject: [PATCH] Change costs for TX2 to expose more vectorization
opportunities
>
> Make the costs such that they do not exaclty reflect
> the actual instructions costs from the manual but make
> the codegen emit the code we want it to.
> ---
> gcc/config/aarch64/aarch64.c | 18 +-
> 1 file changed, 9 insertions(+), 9 deletions(-)
>
> diff --git a/gcc/config/aarch64/aarch64.c b/gcc/config/aarch64/aarch64.c
> index e92c7e6..18c2251 100644
> --- a/gcc/config/aarch64/aarch64.c
> +++ b/gcc/config/aarch64/aarch64.c
> @@ -535,9 +535,9 @@ static const struct cpu_regmove_cost
thunderx2t99_regmove_cost =
> {
>1, /* GP2GP */
>/* Avoid the use of int<->fp moves for spilling. */
> - 8, /* GP2FP */
> - 8, /* FP2GP */
> - 4 /* FP2FP */
> + 5, /* GP2FP */
> + 6, /* FP2GP */
> + 3, /* FP2FP */
> };
>
> static const struct cpu_regmove_cost thunderx3t110_regmove_cost =
> @@ -704,15 +704,15 @@ static const struct cpu_vector_cost
thunderx2t99_vector_cost =
>6, /* scalar_fp_stmt_cost */
>4, /* scalar_load_cost */
>1, /* scalar_store_cost */
> - 5, /* vec_int_stmt_cost */
> - 6, /* vec_fp_stmt_cost */
> + 4, /* vec_int_stmt_cost */
> + 5, /* vec_fp_stmt_cost */
>10, /* vec_permute_cost */
>6, /* vec_to_scalar_cost */
>5, /* scalar_to_vec_cost */
> - 8, /* vec_align_load_cost */
> - 8, /* vec_unalign_load_cost */
> - 4, /* vec_unalign_store_cost */
> - 4, /* vec_store_cost */
> + 4, /* vec_align_load_cost */
> + 4, /* vec_unalign_load_cost */
> + 1, /* vec_unalign_store_cost */
> + 1, /* vec_store_cost */
>2, /* cond_taken_branch_cost */
>1 /* cond_not_taken_branch_cost */
> };
Re: [PATCH] aarch64: Change costs for TX2 to expose more vectorization opportunities
Joel Jones writes: > I approve of this patch. I'm responsible for GCC for TX2 at Marvell. Andrew > Pinski should certainly chime in if he wants. Ah, in that case, the patch is OK. Thanks, Richard
Re: [PATCH] rs6000: Add execution tests for mma builtins.
Hi!
On Tue, Jun 30, 2020 at 05:32:24PM -0500, Aaron Sawdey via Gcc-patches wrote:
> This patch adds execution tests that use the MMA builtins,
> checks for the right answer, and checks that __builtin_cpu_supports
> and __builtin_cpu_is return sane answers given that the code
> executed correctly.
> Actually the power10_hw test I think requires
> current glibc to pick up the change that lets
> __builtin_cpu_is("power10") work.
The power10_hw test checks if a "pli" insn works, it doesn't use
__builtin_cpu_is.
> 2020-06-30 Rajalakshmi Srinivasaraghavan
> Aaron Sawdey
>
> gcc/testsuite/
> * gcc.target/powerpc/mma-single-test.c: New file.
> * gcc.target/powerpc/mma-double-test.c: New file.
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/powerpc/mma-double-test.c
> @@ -0,0 +1,211 @@
> +/* { dg-do run } */
> +/* { dg-require-effective-target power10_hw } */
> +/* { dg-options "-Wno-psabi -mdejagnu-cpu=power10 -O2" } */
We probably should have separate selectors for MMA sooner rather than
later (but doesn't have to be *now*).
What is -Wno-psabi needed for?
Okay for trunk with maybe that taken out. Thanks! Also okay for 10 if
you want a backport?
Segher
Re: [PATCH] Add power10 BRD/BRW/BRH support.
Hi!
On Wed, Jul 01, 2020 at 01:04:02PM -0400, Michael Meissner wrote:
> This patch adds support for the Power10 BRD, BRW, and BRH instructions that do
> byte swapping on values in GPRs.
> I used '*' for the length attribute where it generates a single instruction.
More exactly: those that can use the default length calculation.
> I did not change the tests in the splitter as I don't see any easier way to do
> the test. For reference, there are 3 cases:
>
>1) Power10 instruction swapping a GPR which does not get split
>2) Non-power10 instruction splitting the insn into component parts; (and)
>3) Power9 instruction swapping a vector register which does not get split.
> (define_insn_and_split "bswaphi2_reg"
> + brh %0,%1
> #
> xxbrh %x0,%x1"
> + (set_attr "type" "shift,*,vecperm")
> + (set_attr "isa" "p10,*,p9v")])
I don't think "shift" will be a good type for this, but we'll see. (The
insn types are mostly for scheduling... It is often hard to come up with
something good while there is only one implementation. Power9 was quite
different from most older chips here. Hopefully it can improve now we
also have Power10.
> ;; We are always BITS_BIG_ENDIAN, so the bit positions below in
> ;; zero_extract insns do not change for -mlittle.
> (define_insn_and_split "bswapsi2_reg"
(Not new in this patch of course... There are no zero_extract insns
below, heh).
> @@ -2641,9 +2643,9 @@ (define_insn_and_split "bswapsi2_reg"
> (and:SI (match_dup 0)
> (const_int -256]
>""
> - [(set_attr "length" "12,4")
> - (set_attr "type" "*,vecperm")
> - (set_attr "isa" "*,p9v")])
> + [(set_attr "length" "4,12,4")
> + (set_attr "type" "shift,*,vecperm")
> + (set_attr "isa" "p10,*,p9v")])
"length" should use "*" here, as well.
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/powerpc/bswap-brd.c
> @@ -0,0 +1,23 @@
> +/* { dg-do compile { target { powerpc*-*-* && lp64 } } } */
Everything in gcc.target/powerpc is powerpc*-*-* already.
Why does this neede lp64? (Put that in a comment.)
> +/* This tests whether GCC generates the ISA 3.1 BRW byte swap instruction for
> + GPR data, but generates XXBRW for data in a vector register. */
This comment is wrong. This tests for "brd" (and "xxbrd"), instead.
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/powerpc/bswap-brh.c
> @@ -0,0 +1,12 @@
> +/* { dg-do compile { target { powerpc*-*-* } } } */
powerpc*-*-* is default. Don't use it here.
> +/* This tests whether GCC generates the ISA 3.1 16-bit byte swap
> + instruction BRH. */
"whether GCC generates "brh" to implement 16-bit byte swap" or such...
(the instruction swaps every pair of bytes).
Okay for trunk with those things fixed. Thanks!
Segher
Re: [PATCH 1/2] PowerPC: Rename FP min/max/cmove functions.
Hi! On Wed, Jul 01, 2020 at 06:20:22PM -0400, Michael Meissner wrote: > This patch changes the name of two functions that were added to support power9 > instructions, so that the name of these functions are no longer specific to > power9. > * config/rs6000/rs6000.c (emit_fp_min_max_insn): Rename > rs6000_emit_p9_fp_minmax. You can mention both the old and the new name on the lhs, which is easier to read (and write!) > -/* ISA 3.0 (power9) minmax subcase to emit a XSMAXCDP or XSMINCDP instruction > - for SF/DF scalars. Move TRUE_COND to DEST if OP of the operands of the > last > - comparison is nonzero/true, FALSE_COND if it is zero/false. Return 0 if > the > - hardware has no such operation. */ > +/* Min/max subcase to emit an appropriate instruction for SF/DF scalars on > ISA > + 3.0. > + > + Move TRUE_COND to DEST if OP of the operands of the last comparison is > + nonzero/true, FALSE_COND if it is zero/false. Return 0 if the hardware > has > + no such operation. */ > > static int > -rs6000_emit_p9_fp_minmax (rtx dest, rtx op, rtx true_cond, rtx false_cond) > +emit_fp_min_max_insn (rtx dest, rtx op, rtx true_cond, rtx false_cond) Please change the name (and the comment) to make it clear this function does *not* always emit code, and returns a boolean (an int, maybe change that too?) saying whether it did do the deed. Segher
[committed] wwwdocs: Editorial changes to the LTO section.
Pushed. Gerald --- htdocs/gcc-10/changes.html | 13 +++-- 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/htdocs/gcc-10/changes.html b/htdocs/gcc-10/changes.html index 0959e4c4..4e5f6139 100644 --- a/htdocs/gcc-10/changes.html +++ b/htdocs/gcc-10/changes.html @@ -143,17 +143,18 @@ You may also want to check out our A new binary https://gcc.gnu.org/onlinedocs/gcc-10.1.0/gcc/lto-dump.html";>lto-dump -has been added. The program can dump various -information about a LTO bytecode object file. +has been added. It dumps various information about LTO bytecode + object files. - Parallel phase of the LTO can automatically detect a running make's - jobserver or can fall back to number of available cores. + The parallel phase of the LTO can automatically detect a running + make's jobserver or fall back to number of available cores. -The LTO bytecode can be compressed with +The LTO bytecode can be compressed with the https://facebook.github.io/zstd/";>zstd -algorithm. Configure script can automatically detect the zstd support. +algorithm. The configure script automatically detects + zstd support. Most --param values can now be specified at translation unit granularity. This includes all parameters controlling -- 2.27.0
[PATCH] PR fortran/96086 - ICE in gfc_match_select_rank, at fortran/match.c:6645
More NULL pointer dereferences on invalid code, detected by Gerhard.
Regtested on x86_64-pc-linux-gnu.
OK for master?
Thanks,
Harald
PR fortran/96086 - ICE in gfc_match_select_rank, at fortran/match.c:6645
Handle NULL pointer dereference on SELECT RANK with an invalid
assumed-rank array declaration.
gcc/fortran/
PR fortran/96086
* match.c (gfc_match_select_rank): Catch NULL pointer
dereference.
* resolve.c (resolve_assoc_var): Catch NULL pointer dereference
that may occur after a illegal declaration.
diff --git a/gcc/fortran/match.c b/gcc/fortran/match.c
index 7d3711c55f9..cb09c5f8ec5 100644
--- a/gcc/fortran/match.c
+++ b/gcc/fortran/match.c
@@ -6647,7 +6647,8 @@ gfc_match_select_rank (void)
if (expr2->symtree)
{
sym2 = expr2->symtree->n.sym;
- as = sym2->ts.type == BT_CLASS ? CLASS_DATA (sym2)->as : sym2->as;
+ as = (sym2->ts.type == BT_CLASS
+ && CLASS_DATA (sym2)) ? CLASS_DATA (sym2)->as : sym2->as;
}
if (expr2->expr_type != EXPR_VARIABLE
@@ -6659,7 +6660,7 @@ gfc_match_select_rank (void)
goto cleanup;
}
- if (expr2->ts.type == BT_CLASS)
+ if (expr2->ts.type == BT_CLASS && CLASS_DATA (sym2))
{
copy_ts_from_selector_to_associate (expr1, expr2);
diff --git a/gcc/fortran/resolve.c b/gcc/fortran/resolve.c
index 223de91..ef251dc142d 100644
--- a/gcc/fortran/resolve.c
+++ b/gcc/fortran/resolve.c
@@ -9012,7 +9012,9 @@ resolve_assoc_var (gfc_symbol* sym, bool resolve_target)
if (as->corank != 0)
sym->attr.codimension = 1;
}
- else if (sym->ts.type == BT_CLASS && (!CLASS_DATA (sym)->as || sym->assoc->rankguessed))
+ else if (sym->ts.type == BT_CLASS
+ && CLASS_DATA (sym)
+ && (!CLASS_DATA (sym)->as || sym->assoc->rankguessed))
{
if (!CLASS_DATA (sym)->as)
CLASS_DATA (sym)->as = gfc_get_array_spec ();
diff --git a/gcc/testsuite/gfortran.dg/pr96086.f90 b/gcc/testsuite/gfortran.dg/pr96086.f90
new file mode 100644
index 000..b80967a7a07
--- /dev/null
+++ b/gcc/testsuite/gfortran.dg/pr96086.f90
@@ -0,0 +1,8 @@
+! { dg-do compile }
+! PR fortran/96086 - ICE in gfc_match_select_rank, at fortran/match.c:6645
+
+subroutine s
+ class(*) :: x(..) ! { dg-error "Assumed-rank array" }
+ select rank (y => x) ! { dg-error "CLASS variable" }
+ end select
+end
Re: [PATCH 2/2] PowerPC: Add ISA 3.1 IEEE 128-bit min, max, and cmove.
Hi!
On Wed, Jul 01, 2020 at 06:20:23PM -0400, Michael Meissner wrote:
> This patch adds support for the IEEE 128-bit floating point minimum, maximum,
> and compare instructions generating a mask. These instructions were added in
> ISA 3.1 (i.e. power10).
> (emit_fp_cmove_with_mask_xxsel): Update comment.
Your patch does not touch this function?
> + /* See if we can use the ISA 3.1 min/max/compare instructions for IEEE
> + 128-bit floating point. At present, don't worry about doing conditional
> + moves with different types for the comparison and movement (unlike
> SF/DF,
> + where you can do a conditional test between double and use float as the
> + if/then parts. */
Two spaces after a full stop.
> +;; IEEE 128-bit min/max
> +(define_insn "s3"
> + [(set (match_operand:IEEE128 0 "altivec_register_operand" "=v")
> + (fp_minmax:IEEE128
> + (match_operand:IEEE128 1 "altivec_register_operand" "v")
> + (match_operand:IEEE128 2 "altivec_register_operand" "v")))]
> + "TARGET_FLOAT128_HW && TARGET_POWER10 && FLOAT128_IEEE_P (mode)"
> + "xscqp %0,%1,%2"
> + [(set_attr "type" "fp")
> + (set_attr "size" "128")])
As with the SF/DF min/max, this is undefined if either operand is a NaN,
or if both are zero (or underdefined). Because of that, the "c" version
of the instructions can be used here. But it isn't obviously clear how
this works with the builtins :-(
> +(define_insn_and_split "*movcc_hardware"
> + [(set (match_operand:IEEE128 0 "altivec_register_operand" "=&v,v")
> + (if_then_else:IEEE128
> + (match_operator:CCFP 1 "fpmask_comparison_operator"
> + [(match_operand:IEEE128 2 "altivec_register_operand" "v,v")
> + (match_operand:IEEE128 3 "altivec_register_operand" "v,v")])
> + (match_operand:IEEE128 4 "vsx_register_operand" "wa,wa")
> + (match_operand:IEEE128 5 "vsx_register_operand" "wa,wa")))
> + (clobber (match_scratch:V2DI 6 "=0,&wa"))]
Why do you use "wa" instead of "v" here? Won't it always need "v" in the
end anyway?
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/powerpc/float128-minmax-2.c
> @@ -0,0 +1,70 @@
> +/* { dg-do compile { target { powerpc*-*-* && lp64 } } } */
> +/* { dg-require-effective-target power10_ok } */
> +/* { dg-options "-mdejagnu-cpu=power10 -O2 -ffast-math" } */
> +/* { dg-final { scan-assembler-not "xscmpuqp" } } */
> +/* { dg-final { scan-assembler "xscmpeqqp" } } */
> +/* { dg-final { scan-assembler "xscmpgtqp" } } */
> +/* { dg-final { scan-assembler "xscmpgeqp" } } */
> +/* { dg-final { scan-assembler "xsmaxcqp" } } */
> +/* { dg-final { scan-assembler "xsmincqp" } } */
> +/* { dg-final { scan-assembler "xxsel" } } */
Please use \m \M.
Segher
Re: [PATCH] PowerPC: Optimize DImode -> vector store.
Hi!
On Wed, Jul 01, 2020 at 06:43:32PM -0400, Michael Meissner wrote:
> This patch fixes a PR that I noticed several years ago during power8
> development. I noticed that the compiler would often create a two element
> vector and store the vector.
>
> Particularly for DImode on power8, this could involve two direct moves and a
> XXPERMDI to glue the two parts together. On power9, there a single direct
> move
> instruction that combines the two elements.
But on p9 it reduces path length (and actual latency) as well, while not
taking more insns.
>
> Originally I had the optimization for DFmode as well as DImode. I found if
> the
> values were already in vector registers, that generally it was faster to do
> the
> XXPERMDI and vector store.
I don't see that? How is DF different from DI in any way here?
> +(define_insn_and_split "*concatv2di_store"
> + [(set (match_operand:V2DI 0 "memory_operand" "=m,m,m,m")
Should this just require ds_form_memory, instead of having a fallback
later?
> + /* Because we are creating scalar stores, we don't have to swap the order
> + of the elements and then swap the stores to get the right order on
> + little endian systems. */
Or any other system. Yes.
> +;; Optimize creating a vector with 2 duplicate DImode elements and storing
> it.
> +(define_insn_and_split "*dupv2di_store"
Hrm, can't this just use *concatv2di_store some way?
> --- /dev/null
> +++ b/gcc/testsuite/gcc.target/powerpc/pr81594.c
> @@ -0,0 +1,61 @@
> +/* { dg-do compile { target { powerpc-*-* && ilp64 } } } */
> +/* { dg-require-effective-target powerpc_p8vector_ok } */
> +/* { dg-options "-mdejagnu-cpu=power8 -O2" } */
This hasn't been tested.
> +/* PR target/81594. Optimize creating a vector of 2 64-bit elements and then
> + storing the vector into separate stores. */
Please mention the PR # in the subject and the changelog.
> +/* { dg-final { scan-assembler-not {\mstxv\M} } } */
> +/* { dg-final { scan-assembler-not {\mstxvx\M}} } */
> +/* { dg-final { scan-assembler-not {\mmfvsrd\M} } } */
> +/* { dg-final { scan-assembler-not {\mmtvsrd\M} } } */
> +/* { dg-final { scan-assembler-not {\mmtvsrdd\M} } } */
> +/* { dg-final { scan-assembler-not {\mxxpermdi\M} } } */
There are other ways to write the stores (older insns, and prefixed
insns, for example). xxpermdi has many extenden mnemonics (and we
actually use those, they improve readability quite a bit).
It helps if you also test what insns *should* be there, the testcase
will not so easily silently pass (while the generated code is not what
it wants) that way.
Segher
[committed 1/3] libstdc++: Add noexcept to std::optional initialization (PR 96036)
libstdc++-v3/ChangeLog:
PR libstdc++/96036
* include/std/optional (optional): Add noexcept-specifier to
every constructor, assignment operator, emplace function and
dereference operator.
* testsuite/20_util/optional/assignment/noexcept.cc: New test.
* testsuite/20_util/optional/cons/noexcept.cc: New test.
Tested powerpc64le-linux, committed to trunk.
commit 8992cd1892df1adb352cf5d5b279a00686d1e88a
Author: Jonathan Wakely
Date: Mon Jul 6 21:54:12 2020 +0100
libstdc++: Add noexcept to std::optional initialization (PR 96036)
libstdc++-v3/ChangeLog:
PR libstdc++/96036
* include/std/optional (optional): Add noexcept-specifier to
every constructor, assignment operator, emplace function and
dereference operator.
* testsuite/20_util/optional/assignment/noexcept.cc: New test.
* testsuite/20_util/optional/cons/noexcept.cc: New test.
diff --git a/libstdc++-v3/include/std/optional
b/libstdc++-v3/include/std/optional
index 923d45ae0e8..785c434412d 100644
--- a/libstdc++-v3/include/std/optional
+++ b/libstdc++-v3/include/std/optional
@@ -698,6 +698,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
is_convertible<_Up&&, _Tp>> = true>
constexpr
optional(_Up&& __t)
+ noexcept(is_nothrow_constructible_v<_Tp, _Up>)
: _Base(std::in_place, std::forward<_Up>(__t)) { }
template>> = false>
explicit constexpr
optional(_Up&& __t)
+ noexcept(is_nothrow_constructible_v<_Tp, _Up>)
: _Base(std::in_place, std::forward<_Up>(__t)) { }
template>> = true>
constexpr
optional(const optional<_Up>& __t)
+ noexcept(is_nothrow_constructible_v<_Tp, const _Up&>)
{
if (__t)
emplace(*__t);
@@ -727,6 +730,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
__not_<__converts_from_optional<_Tp, _Up>>> = false>
explicit constexpr
optional(const optional<_Up>& __t)
+ noexcept(is_nothrow_constructible_v<_Tp, const _Up&>)
{
if (__t)
emplace(*__t);
@@ -739,6 +743,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
__not_<__converts_from_optional<_Tp, _Up>>> = true>
constexpr
optional(optional<_Up>&& __t)
+ noexcept(is_nothrow_constructible_v<_Tp, _Up>)
{
if (__t)
emplace(std::move(*__t));
@@ -751,6 +756,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
__not_<__converts_from_optional<_Tp, _Up>>> = false>
explicit constexpr
optional(optional<_Up>&& __t)
+ noexcept(is_nothrow_constructible_v<_Tp, _Up>)
{
if (__t)
emplace(std::move(*__t));
@@ -760,6 +766,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
_Requires> = false>
explicit constexpr
optional(in_place_t, _Args&&... __args)
+ noexcept(is_nothrow_constructible_v<_Tp, _Args...>)
: _Base(std::in_place, std::forward<_Args>(__args)...) { }
template> = false>
explicit constexpr
optional(in_place_t, initializer_list<_Up> __il, _Args&&... __args)
+ noexcept(is_nothrow_constructible_v<_Tp, initializer_list<_Up>&,
+ _Args...>)
: _Base(std::in_place, __il, std::forward<_Args>(__args)...) { }
+
// Assignment operators.
optional&
operator=(nullopt_t) noexcept
@@ -786,6 +796,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
is_assignable<_Tp&, _Up>>,
optional&>
operator=(_Up&& __u)
+ noexcept(__and_v,
+is_nothrow_assignable<_Tp&, _Up>>)
{
if (this->_M_is_engaged())
this->_M_get() = std::forward<_Up>(__u);
@@ -803,6 +815,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
__not_<__assigns_from_optional<_Tp, _Up>>>,
optional&>
operator=(const optional<_Up>& __u)
+ noexcept(__and_v,
+is_nothrow_assignable<_Tp&, const _Up&>>)
{
if (__u)
{
@@ -826,6 +840,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
__not_<__assigns_from_optional<_Tp, _Up>>>,
optional&>
operator=(optional<_Up>&& __u)
+ noexcept(__and_v,
+is_nothrow_assignable<_Tp&, _Up>>)
{
if (__u)
{
@@ -845,6 +861,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
template
enable_if_t, _Tp&>
emplace(_Args&&... __args)
+ noexcept(is_nothrow_constructible_v<_Tp, _Args...>)
{
this->_M_reset();
this->_M_construct(std::forward<_Args>(__args)...);
@@ -855,6 +872,8 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
enable_if_t&,
_Args&&...>, _Tp&>
emplace(initializer_list<_Up>
[committed 2/3] libstdc++: Constrain std::make_optional
The standard rquires that std::make_optional is constrained similarly to the std::optional constructors, which our implementation fails to do. As a conforming extension this also adds a noexcept-specifier to each std::make_optional overload. libstdc++-v3/ChangeLog: * include/std/optional (make_optional): Add enable_if constraints and noexcept-specifier to each overload. * testsuite/20_util/optional/make_optional-2.cc: New test. Tested powerpc64le-linux, committed to trunk.
[committed 3/3] libstdc++: Cleanup whitespace and type trait usage in
This makes the formatting in consistent and also removes
redundant && tokens from template arguments for traits like
is_constructible and is_convertible.
libstdc++-v3/ChangeLog:
* include/std/optional (_Optional_payload_base, _Optional_base)
(optional, __optional_hash_call_base): Adjust whitespace and
other formatting. Remove redundant && tokens on template
arguments to type traits.
Tested powerpc64le-linux, committed to trunk.
commit 6e1c9715b3142bc09e313c0d4b196694ab7ae153
Author: Jonathan Wakely
Date: Mon Jul 6 21:54:12 2020 +0100
libstdc++: Cleanup whitespace and type trait usage in
This makes the formatting in consistent and also removes
redundant && tokens from template arguments for traits like
is_constructible and is_convertible.
libstdc++-v3/ChangeLog:
* include/std/optional (_Optional_payload_base, _Optional_base)
(optional, __optional_hash_call_base): Adjust whitespace and
other formatting. Remove redundant && tokens on template
arguments to type traits.
diff --git a/libstdc++-v3/include/std/optional b/libstdc++-v3/include/std/optional
index 24821f81d0f..f9f42efe09c 100644
--- a/libstdc++-v3/include/std/optional
+++ b/libstdc++-v3/include/std/optional
@@ -163,9 +163,9 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
constexpr void
_M_copy_assign(const _Optional_payload_base& __other)
{
-if (this->_M_engaged && __other._M_engaged)
- this->_M_get() = __other._M_get();
-else
+ if (this->_M_engaged && __other._M_engaged)
+ this->_M_get() = __other._M_get();
+ else
{
if (__other._M_engaged)
this->_M_construct(__other._M_get());
@@ -211,7 +211,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
{ }
_Empty_byte _M_empty;
- _Up _M_value;
+ _Up _M_value;
};
template
@@ -235,7 +235,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
~_Storage() { }
_Empty_byte _M_empty;
- _Up _M_value;
+ _Up _M_value;
};
_Storage<_Stored_type> _M_payload;
@@ -243,14 +243,14 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
bool _M_engaged = false;
template
-void
-_M_construct(_Args&&... __args)
-noexcept(is_nothrow_constructible_v<_Stored_type, _Args...>)
-{
- ::new ((void *) std::__addressof(this->_M_payload))
-_Stored_type(std::forward<_Args>(__args)...);
- this->_M_engaged = true;
-}
+ void
+ _M_construct(_Args&&... __args)
+ noexcept(is_nothrow_constructible_v<_Stored_type, _Args...>)
+ {
+ ::new ((void *) std::__addressof(this->_M_payload))
+ _Stored_type(std::forward<_Args>(__args)...);
+ this->_M_engaged = true;
+ }
constexpr void
_M_destroy() noexcept
@@ -471,39 +471,41 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
bool = is_trivially_copy_constructible_v<_Tp>,
bool = is_trivially_move_constructible_v<_Tp>>
struct _Optional_base
- : _Optional_base_impl<_Tp, _Optional_base<_Tp>>
+: _Optional_base_impl<_Tp, _Optional_base<_Tp>>
{
// Constructors for disengaged optionals.
constexpr _Optional_base() = default;
// Constructors for engaged optionals.
template, bool> = false>
-constexpr explicit _Optional_base(in_place_t, _Args&&... __args)
-: _M_payload(in_place,
- std::forward<_Args>(__args)...) { }
+ enable_if_t, bool> = false>
+ constexpr explicit
+ _Optional_base(in_place_t, _Args&&... __args)
+ : _M_payload(in_place, std::forward<_Args>(__args)...)
+ { }
template&,
- _Args&&...>, bool> = false>
-constexpr explicit _Optional_base(in_place_t,
- initializer_list<_Up> __il,
- _Args&&... __args)
-: _M_payload(in_place,
- __il, std::forward<_Args>(__args)...)
-{ }
+ _Args...>, bool> = false>
+ constexpr explicit
+ _Optional_base(in_place_t,
+ initializer_list<_Up> __il,
+ _Args&&... __args)
+ : _M_payload(in_place, __il, std::forward<_Args>(__args)...)
+ { }
// Copy and move constructors.
- constexpr _Optional_base(const _Optional_base& __other)
- : _M_payload(__other._M_payload._M_engaged,
- __other._M_payload)
+ constexpr
+ _Optional_base(const _Optional_base& __other)
+ : _M_payload(__other._M_payload._M_engaged, __other._M_payload)
{ }
- constexpr _Optional_base(_Optional_base&& __other)
+ constexpr
+ _Optional_base(_Optional_base&& __other)
noexcept(is_nothrow_move_constructible_v<_Tp>)
- : _M_payload(__other._M_payload._M_engaged,
- std::move(__other._M_payload))
+ : _M_payload(__other._M_payload._M_engaged,
+ std::move(__other._M_payload))
{ }
// Assignment operators.
@@ -515,33 +517,33 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
template
struct _Optional_base<_Tp, false, tr
Re: [committed 2/3] libstdc++: Constrain std::make_optional
On 06/07/20 22:06 +0100, Jonathan Wakely wrote:
The standard rquires that std::make_optional is constrained similarly to
the std::optional constructors, which our implementation fails to do.
As a conforming extension this also adds a noexcept-specifier to each
std::make_optional overload.
libstdc++-v3/ChangeLog:
* include/std/optional (make_optional): Add enable_if
constraints and noexcept-specifier to each overload.
* testsuite/20_util/optional/make_optional-2.cc: New test.
Tested powerpc64le-linux, committed to trunk.
And with the patch attached this time ...
commit bcfe4681f9be68f96f0610f30356510ff518806b
Author: Jonathan Wakely
Date: Mon Jul 6 21:54:12 2020 +0100
libstdc++: Constrain std::make_optional
The standard rquires that std::make_optional is constrained similarly to
the std::optional constructors, which our implementation fails to do.
As a conforming extension this also adds a noexcept-specifier to each
std::make_optional overload.
libstdc++-v3/ChangeLog:
* include/std/optional (make_optional): Add enable_if
constraints and noexcept-specifier to each overload.
* testsuite/20_util/optional/make_optional-2.cc: New test.
diff --git a/libstdc++-v3/include/std/optional b/libstdc++-v3/include/std/optional
index 785c434412d..24821f81d0f 100644
--- a/libstdc++-v3/include/std/optional
+++ b/libstdc++-v3/include/std/optional
@@ -1220,19 +1220,28 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
swap(optional<_Tp>&, optional<_Tp>&) = delete;
template
-constexpr optional>
+constexpr
+enable_if_t, _Tp>,
+ optional>>
make_optional(_Tp&& __t)
-{ return optional> { std::forward<_Tp>(__t) }; }
+noexcept(is_nothrow_constructible_v>, _Tp>)
+{ return optional>{ std::forward<_Tp>(__t) }; }
- template
-constexpr optional<_Tp>
+ template
+constexpr
+enable_if_t,
+ optional<_Tp>>
make_optional(_Args&&... __args)
-{ return optional<_Tp> { in_place, std::forward<_Args>(__args)... }; }
+noexcept(is_nothrow_constructible_v<_Tp, _Args...>)
+{ return optional<_Tp>{ in_place, std::forward<_Args>(__args)... }; }
- template
-constexpr optional<_Tp>
+ template
+constexpr
+enable_if_t&, _Args...>,
+ optional<_Tp>>
make_optional(initializer_list<_Up> __il, _Args&&... __args)
-{ return optional<_Tp> { in_place, __il, std::forward<_Args>(__args)... }; }
+noexcept(is_nothrow_constructible_v<_Tp, initializer_list<_Up>&, _Args...>)
+{ return optional<_Tp>{ in_place, __il, std::forward<_Args>(__args)... }; }
// Hash.
diff --git a/libstdc++-v3/testsuite/20_util/optional/make_optional-2.cc b/libstdc++-v3/testsuite/20_util/optional/make_optional-2.cc
new file mode 100644
index 000..65a1fc78d80
--- /dev/null
+++ b/libstdc++-v3/testsuite/20_util/optional/make_optional-2.cc
@@ -0,0 +1,94 @@
+// { dg-do compile { target c++17 } }
+
+// Copyright (C) 2020 Free Software Foundation, Inc.
+//
+// This file is part of the GNU ISO C++ Library. This library 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.
+
+// This library 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 this library; see the file COPYING3. If not see
+//
[PATCH] middle-end: Improve RTL expansion in expand_mul_overflow,
This patch improves the RTL that the middle-end generates for testing
signed overflow following a widening multiplication. During this
expansion the middle-end generates a truncation which can get used
multiple times. Placing this intermediate value in a pseudo register
reduces the amount of code generated on platforms where this truncation
requires an explicit instruction.
This simple call to force_reg eliminates 368 lines of the -S output
from testsuite/c-c++-common/torture/builtin-arith-overflow-1.c on
nvptx-none. An example difference is in t120_1smul where the following
7 instruction sequence in which the 1st and 6th instructions perform
the same truncation:
< cvt.u32.u64 %r31, %r28; <- truncate %r28
< shr.s32 %r30, %r31, 31;
< cvt.u32.u64 %r32, %r29;
< setp.eq.u32 %r33, %r30, %r32;
< selp.u32%r24, 0, 1, %r33;
< cvt.u32.u64 %r25, %r28; <- truncate %r28
< setp.eq.u32 %r34, %r24, 0;
is now generated as a 4 instruction sequence without duplication:
> cvt.u32.u64 %r30, %r28;
> shr.s32 %r31, %r30, 31;
> cvt.u32.u64 %r32, %r29;
> setp.eq.u32 %r33, %r31, %r32;
On x86_64-pc-linux-gnu, where SUBREGs are free, this patch generates
exactly the same builtin-arith-overflow-1.s as before.
This patch has been tested on both x86_64-pc-linux-gnu with
"make bootstrap" and nvptx-none with "make", with no new
testsuite regressions on either platform.
Ok for mainline?
2020-07-06 Roger Sayle
gcc/ChangeLog:
* internal-fn.c (expand_mul_overflow): When checking for signed
overflow from a widening multiplication, we access the truncated
lowpart RES twice, so keep this value in a pseudo register.
Thanks in advance,
Roger
--
Roger Sayle
NextMove Software
Cambridge, UK
diff --git a/gcc/internal-fn.c b/gcc/internal-fn.c
index 0be2eb4..d1bd6cc 100644
--- a/gcc/internal-fn.c
+++ b/gcc/internal-fn.c
@@ -1627,6 +1627,9 @@ expand_mul_overflow (location_t loc, tree lhs, tree arg0,
tree arg1,
profile_probability::very_likely ());
else
{
+ /* RES is used more than once, place it in a pseudo. */
+ res = force_reg (mode, res);
+
rtx signbit = expand_shift (RSHIFT_EXPR, mode, res, prec - 1,
NULL_RTX, 0);
/* RES is low half of the double width result, HIPART
Re: RFA: Fix combine.c combining a move and a non-move into two non-moves, PR93372
Hi!
On Mon, Jul 06, 2020 at 03:11:17AM +0200, Hans-Peter Nilsson wrote:
> TL;DR: fixing a misdetection of what is a "simple move".
That is not a very correct characterisation of what this does :-)
> Looking into performace degradation after de-cc0 for CRIS, I
> noticed combine behaving badly; it changed a move and a
> right-shift into two right-shifts, where the "combined" move was
> not eliminated in later passes, and where the deficiency caused
> an extra insn in a hot loop: crcu16 (and crcu32) in coremark.
>
> Before de-cc0, the insns input to combine looked like:
>33: r58:SI=r56:SI 0>>r48:SI
> REG_DEAD r56:SI
>35: r37:HI=r58:SI#0
> and after:
>33: {r58:SI=r56:SI 0>>r48:SI;clobber dccr:CC;}
> REG_DEAD r56:SI
> REG_UNUSED dccr:CC
>35: {r37:HI=r58:SI#0;clobber dccr:CC;}
> REG_UNUSED dccr:CC
So a shift like this is at most as expensive as a move, on your target
(or, in your backend, anyway ;-) )
> That is, there's always a parallel with a clobber of the
> condition-codes register. Being a parallel, it's not an
> is_just_move, but e.g. a single_set.
>
> For the de-cc0:ed "combination", it ended up as
>33: {r58:SI=r56:SI 0>>r48:SI;clobber dccr:CC;}
> REG_UNUSED dccr:CC
>35: {r37:HI#0=r56:SI 0>>r48:SI;clobber dccr:CC;}
> REG_DEAD r56:SI
> REG_UNUSED dccr:CC
> That is, a move and a shift turned into two shifts; the extra
> shift is not eliminated by later passes, while the move was
> (with cc0, and "will be again") leading to redundant
> instructions.
Which was the whole point of the is_just_move() thing, yes. Combine
doesn't know most moves will be eliminated by RA (but many are useful to
do have before RA, because it gives RA much more freedom). If a move is
the same cost as a simple insn, doing two (say shift, like here) insns
in parallel is cheaper on most machines than having a shift and a move
sequentially. (2-2 combinations are helpful on single-scalar and even
in-order machines as well, btw).
> At first I thought this was due to parallel-ignorant old code
> but the "guilty" change is actually pretty recent. Regarding a
> parallel with a clobber not being "just" a move, there's only
> the two adjacent callers seen in the patch (obviously with the
> rename), and their use exactly matches to check that the
> argument is a single_set which is a move. It's always applied
> to an rtx_insn, so I changed the type and name to avoid the
> "just" issue. I had to adjust the type when calling single_set.
But it is *not* supposed to be the same as single_set.
> I checked the original commit, c4c5ad1d6d1e1e a.k.a r263067 and
> it seems parallels-as-sets were just overlooked and that this
They were not. It causes regressions. That is why it has a different
name, not something with "single set". "just_move" isn't a very good
name, I couldn't come up with something better, it is a pretty
complicated concept :-/
"i2_should_not_be_2_2_combined"?
I'll rerun some testing to show this. It'll take a while.
> patch appears to agree with the intent and the comment at the
> use of i2_was_move and i3_was_move, which has a clause saying
> "Also do this if we started with two insns neither of which was
> a simple move".
That says that we combine to 2 insns also if we started with only 2,
but neither of those was just a move.
> With this correction in place, the performance degradation
> related to de-cc0 of the CRIS port as measured by coremark is
> gone and turned into a small win. N.B.: there certainly is a
> code difference in other hot functions, and the swing between
> different functions is larger than this difference; to be dealt
> with separately.
>
> Tested cris-elf, x86_64-linux, powerpc64le-linux, 2/3 through
> aarch64-linux (unexpectedly slow).
>
> Ok to commit?
No, sorry.
One thing that could work is allowing (unused) clobbers of fixed
registers (like you have here), or maybe some hook is needed to say this
register is like a flags register, or similar. That should work for you,
and not regress other targets, maybe even help a little? We'll see.
Thanks,
Segher
Re: [PATCH] rs6000: Split movsf_from_si from high word before reload[PR89310]
Hi!
On Sun, Jul 05, 2020 at 09:17:57PM -0500, Xionghu Luo wrote:
> For extracting high part element from DImode register like:
>
> {%1:SF=unspec[r122:DI>>0x20#0] 86;clobber scratch;}
>
> split it before reload with "and mask" to avoid generating shift right
> 32 bit then shift left 32 bit.
>
> srdi 3,3,32
> sldi 9,3,32
> mtvsrd 1,9
> xscvspdpn 1,1
>
> =>
>
> rldicr 3,3,0,31
> mtvsrd 1,3
> xscvspdpn 1,1
Great :-)
> +;; For extracting high part element from DImode register like:
> +;; {%1:SF=unspec[r122:DI>>0x20#0] 86;clobber scratch;}
> +;; split it before reload with "and mask" to avoid generating shift right
> +;; 32 bit then shift left 32 bit.
> +(define_insn_and_split "movsf_from_si2"
> + [(set (match_operand:SF 0 "nonimmediate_operand"
> + "=!r, f, v, wa,m, Z,
> + Z, wa,?r,!r")
> + (unspec:SF [
> + (subreg:SI (ashiftrt:DI
> +(match_operand:DI 1 "input_operand"
> +"m, m, wY,Z, r, f,
> +wa,r, wa,r")
> + (const_int 32)) 0)]
> +UNSPEC_SF_FROM_SI))
> + (clobber (match_scratch:DI 2
> + "=X,X, X, X, X, X,
> + X, r, X, X"))]
> + "TARGET_NO_SF_SUBREG
> + && (register_operand (operands[0], SFmode)
> + && register_operand (operands[1], DImode))"
If the insn condition requires operands 0 and 1 to be register_operands,
it can ask for that in the predicates, instead: not nonimmediate_operand
and input_operand, but just gpc_reg_operand instead. You can leave out
the impossible alternatives as well (0, 1, 2, 3, 4, 5, 6), leaving just
(define_insn_and_split "movsf_from_si2"
[(set (match_operand:SF 0 "gpc_reg_operand" "=wa,?r,!r")
(unspec:SF
[(subreg:SI (ashiftrt:DI
(match_operand:DI 1 "input_operand" "r,wa,r")
(const_int 32))
0)]
UNSPEC_SF_FROM_SI)))]
"TARGET_NO_SF_SUBREG"
"@
#
mfvsrwz %0,%x1
mr %0,%1"
"&& !reload_completed
&& vsx_reg_sfsubreg_ok (operands[0], SFmode)"
[(const_int 0)]
{
rtx op0 = operands[0];
rtx op1 = operands[1];
rtx tmp = gen_reg_rtx (DImode);
You cannot call gen_reg_rtx too late in the pass pipeline. What we
usually do for such cases is put it as a match_scratch in the pattern,
and then do code like
if (GET_CODE (operands[2]) == SCRATCH)
operands[2] = gen_reg_rtx (DImode);
so that it will work both before and after reload.
/* Avoid split {r155:SI#0=unspec[r133:DI>>0x20#0] 86;clobber scratch;} from
PR42745. */
(This line is too long, btw.)
if (!SUBREG_P (operands[0]))
{
rtx mask = GEN_INT (HOST_WIDE_INT_M1U << 32);
emit_insn (gen_anddi3 (tmp, op1, mask));
emit_insn (gen_p8_mtvsrd_sf (op0, tmp));
emit_insn (gen_vsx_xscvspdpn_directmove (op0, op0));
DONE;
}
else
FAIL;
}
[(set_attr "length" "12,*,*")
(set_attr "type" "vecfloat,mffgpr,*")
(set_attr "isa" "p8v,p8v,*")])
I wonder what will happen if you actually do FAIL here... There then is
no insn alternative that can match, so we ICE? In that case, just leave
out the whole FAIL thing, it is useless? You can do a gcc_assert if you
want to check something.
Oh, and maybe you only want to handle GPRs here, not VSRs? So just the
"r", not the "wa" at all? What code would it generate for vector regs?
Lots of questions, sorry!
Segher
[PATCH] print notes only when -Wmismatched-tags succeeds (c++/96063)
The code for -Wmismatched-tags tests warn_mismatched_tags and when
it's non-zero assumes that calls to warning_at() necessarily succeed,
proceeding to call inform() to unconditionally issue informational
notes. This assumption is wrong when -Wmismatched-tags is enabled
(and warn_mismatched_tags is nonzero) but the instance of the warning
is for code in a system header and -Wsystem-headers is disabled. In
that case, the warning is suppressed but the notes are still printed.
The attached near-trivial change makes the calls to inform() conditional
on the result from warning_at().
I think a more robust solution than testing global variables like
warn_mismatched_tags is to introduce a functional API to query
the result of a warning_at() call without actually issuing a warning.
The patch doesn't do that but I have a follow-up enhancement to do
just that.
Martin
Avoid printing informational notes when -Wmismatched-tags is suppressed in system headers.
Related:
PR c++/96063 - mismatched-tags warnings in stdlib headers
gcc/cp/ChangeLog:
PR c++/96063
* parser.c (class_decl_loc_t::diag_mismatched_tags): Print notes only
if warning_at returns nonzero.
gcc/testsuite/ChangeLog:
PR c++/96063
* g++.dg/warn/Wmismatched-tags-7.C: New test.
* g++.dg/warn/Wmismatched-tags-8.C: New test.
diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index 6e7637c6016..e58d8eb298c 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -31443,25 +31443,26 @@ class_decl_loc_t::diag_mismatched_tags (tree type_decl)
/* Issue a warning for the first mismatched declaration.
Avoid using "%#qT" since the class-key for the same type will
be the same regardless of which one was used in the declaraion. */
- warning_at (loc, OPT_Wmismatched_tags,
- "%qT declared with a mismatched class-key %qs",
- type_decl, xmatchkstr);
-
- /* Suggest how to avoid the warning for each instance since
- the guidance may be different depending on context. */
- inform (loc,
- (key_redundant_p
- ? G_("remove the class-key or replace it with %qs")
- : G_("replace the class-key with %qs")),
- xpectkstr);
-
- /* Also point to the first declaration or definition that guided
- the decision to issue the warning above. */
- inform (cdlguide->location (idxguide),
- (def_p
- ? G_("%qT defined as %qs here")
- : G_("%qT first declared as %qs here")),
- type_decl, xpectkstr);
+ if (warning_at (loc, OPT_Wmismatched_tags,
+ "%qT declared with a mismatched class-key %qs",
+ type_decl, xmatchkstr))
+{
+ /* Suggest how to avoid the warning for each instance since
+ the guidance may be different depending on context. */
+ inform (loc,
+ (key_redundant_p
+ ? G_("remove the class-key or replace it with %qs")
+ : G_("replace the class-key with %qs")),
+ xpectkstr);
+
+ /* Also point to the first declaration or definition that guided
+ the decision to issue the warning above. */
+ inform (cdlguide->location (idxguide),
+ (def_p
+ ? G_("%qT defined as %qs here")
+ : G_("%qT first declared as %qs here")),
+ type_decl, xpectkstr);
+}
/* Issue warnings for the remaining inconsistent declarations. */
for (unsigned i = idx + 1; i != ndecls; ++i)
@@ -31476,16 +31477,16 @@ class_decl_loc_t::diag_mismatched_tags (tree type_decl)
key_redundant_p = key_redundant (i);
/* Set the function declaration to print in diagnostic context. */
current_function_decl = function (i);
- warning_at (loc, OPT_Wmismatched_tags,
- "%qT declared with a mismatched class-key %qs",
- type_decl, xmatchkstr);
- /* Suggest how to avoid the warning for each instance since
- the guidance may be different depending on context. */
- inform (loc,
- (key_redundant_p
- ? G_("remove the class-key or replace it with %qs")
- : G_("replace the class-key with %qs")),
- xpectkstr);
+ if (warning_at (loc, OPT_Wmismatched_tags,
+ "%qT declared with a mismatched class-key %qs",
+ type_decl, xmatchkstr))
+ /* Suggest how to avoid the warning for each instance since
+ the guidance may be different depending on context. */
+ inform (loc,
+ (key_redundant_p
+ ? G_("remove the class-key or replace it with %qs")
+ : G_("replace the class-key with %qs")),
+ xpectkstr);
}
/* Restore the current function in case it was replaced above. */
diff --git a/gcc/testsuite/g++.dg/warn/Wmismatched-tags-7.C b/gcc/testsuite/g++.dg/warn/Wmismatched-tags-7.C
new file mode 100644
index 000..3180c22c054
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/Wmismatched-tags-7.C
@@ -0,0 +1,13 @@
+/* Verify that -Wmismatched-tags doesn't print stray notes for warnings
+ disabled in system headers.
+ { dg-do "compile" }
+ { dg-options "-Wmismatched-tags" } */
+
+# 6 "Wmismatched-tags-7.C" 1
+# 1 "system-header.h" 1 3 4
+# 9 "system-header.h" 3 4
+class A;// { dg-bogus "first de
Re: RFA: Fix combine.c combining a move and a non-move into two non-moves, PR93372
> From: Segher Boessenkool
> Date: Tue, 7 Jul 2020 01:42:47 +0200
TL;DR: recognize a parallel with a clobber of TARGET_FLAGS_REGNUM?
> Hi!
>
> On Mon, Jul 06, 2020 at 03:11:17AM +0200, Hans-Peter Nilsson wrote:
> > TL;DR: fixing a misdetection of what is a "simple move".
>
> That is not a very correct characterisation of what this does :-)
That's apparently where we completely disagree. :-)
> > Looking into performace degradation after de-cc0 for CRIS, I
> > noticed combine behaving badly; it changed a move and a
> > right-shift into two right-shifts, where the "combined" move was
> > not eliminated in later passes, and where the deficiency caused
> > an extra insn in a hot loop: crcu16 (and crcu32) in coremark.
> >
> > Before de-cc0, the insns input to combine looked like:
> >33: r58:SI=r56:SI 0>>r48:SI
> > REG_DEAD r56:SI
> >35: r37:HI=r58:SI#0
> > and after:
> >33: {r58:SI=r56:SI 0>>r48:SI;clobber dccr:CC;}
> > REG_DEAD r56:SI
> > REG_UNUSED dccr:CC
> >35: {r37:HI=r58:SI#0;clobber dccr:CC;}
> > REG_UNUSED dccr:CC
>
> So a shift like this is at most as expensive as a move, on your target
> (or, in your backend, anyway ;-) )
On CRIS, the backend *and* the target, yes; one cycle, one short
instruction.
> > That is, there's always a parallel with a clobber of the
> > condition-codes register. Being a parallel, it's not an
> > is_just_move, but e.g. a single_set.
> >
> > For the de-cc0:ed "combination", it ended up as
> >33: {r58:SI=r56:SI 0>>r48:SI;clobber dccr:CC;}
> > REG_UNUSED dccr:CC
> >35: {r37:HI#0=r56:SI 0>>r48:SI;clobber dccr:CC;}
> > REG_DEAD r56:SI
> > REG_UNUSED dccr:CC
> > That is, a move and a shift turned into two shifts; the extra
> > shift is not eliminated by later passes, while the move was
> > (with cc0, and "will be again") leading to redundant
> > instructions.
>
> Which was the whole point of the is_just_move() thing, yes. Combine
> doesn't know most moves will be eliminated by RA (but many are useful to
> do have before RA, because it gives RA much more freedom). If a move is
> the same cost as a simple insn, doing two (say shift, like here) insns
> in parallel is cheaper on most machines than having a shift and a move
> sequentially.
Most parallel machines you mean, but why bring up them when
there's no means for combine to tell the difference? Here, the
end result is that it *added* an instruction to the hot loop.
It's a deficiency and it caused a performance regression, can't
argue with that.
> (2-2 combinations are helpful on single-scalar and even
> in-order machines as well, btw).
I certainly don't contest that the move can be eliminated, and
that most cost-effective 2-2 eliminations are helpful. (See my
other post about combine being eager with allowing same-cost
combinations.)
> > At first I thought this was due to parallel-ignorant old code
> > but the "guilty" change is actually pretty recent. Regarding a
> > parallel with a clobber not being "just" a move, there's only
> > the two adjacent callers seen in the patch (obviously with the
> > rename), and their use exactly matches to check that the
> > argument is a single_set which is a move. It's always applied
> > to an rtx_insn, so I changed the type and name to avoid the
> > "just" issue. I had to adjust the type when calling single_set.
>
> But it is *not* supposed to be the same as single_set.
(I'm not saying that a single_set is a move. But that's
obvious.) I guess you meant to say that a single_set with a
general_operand as a source (as in the patch) is not supposed to
be the same as a move. This is the only place in combine where
that distinction would be important. Why?
I think you're just overconcious about clobbers here. The
combine pass throws and adds them as it pleases in other places,
so I'm not sure what your worries are in *this* place. (It
collects them and adds them to the combinations.)
> > I checked the original commit, c4c5ad1d6d1e1e a.k.a r263067 and
> > it seems parallels-as-sets were just overlooked and that this
>
> They were not. It causes regressions.
Do you have some pointers to PR:s or something else backing that
statement, or is it your work-in-progress hinted below?
> That is why it has a different
> name, not something with "single set". "just_move" isn't a very good
> name, I couldn't come up with something better, it is a pretty
> complicated concept :-/
>
> "i2_should_not_be_2_2_combined"?
>
> I'll rerun some testing to show this. It'll take a while.
Care to fill in some details on what kind of testing?
> > patch appears to agree with the intent and the comment at the
> > use of i2_was_move and i3_was_move, which has a clause saying
> > "Also do this if we started with two insns neither of which was
> > a simple move".
>
> That says that we combine to 2 insns also if we started with only 2,
> but neither of those was just a move.
That's what I'm saying too.
> > With this corr
Re: RFA: Fix combine.c combining a move and a non-move into two non-moves, PR93372
> From: Segher Boessenkool > Date: Tue, 7 Jul 2020 01:42:47 +0200 (Regarding is_just_move in combine.c.) > But it is *not* supposed to be the same as single_set. > > > I checked the original commit, c4c5ad1d6d1e1e a.k.a r263067 and > > it seems parallels-as-sets were just overlooked and that this > > They were not. It causes regressions. That is why it has a different > name, not something with "single set". "just_move" isn't a very good > name, I couldn't come up with something better, it is a pretty > complicated concept :-/ BTW, if you feel you need to change it, *please* add a comment why single_set + general_operand is not sufficient (preferably more than "must not ignore clobbers"). It might save some time the next time someone has to go through that code. brgds, H-P
Re: [PATCH] print notes only when -Wmismatched-tags succeeds (c++/96063)
On 7/6/20 8:21 PM, Martin Sebor wrote: The code for -Wmismatched-tags tests warn_mismatched_tags and when it's non-zero assumes that calls to warning_at() necessarily succeed, proceeding to call inform() to unconditionally issue informational notes. This assumption is wrong when -Wmismatched-tags is enabled (and warn_mismatched_tags is nonzero) but the instance of the warning is for code in a system header and -Wsystem-headers is disabled. In that case, the warning is suppressed but the notes are still printed. The attached near-trivial change makes the calls to inform() conditional on the result from warning_at(). OK.
[PATCH] pass correct parameters to c_parser_do_statement
Hi, It is a patch to pass correct parameters to c_parser_do_statement. Can anyone help me install this patch? Thanks, Kaipeng Zhou code-cleanup-v1.diff Description: code-cleanup-v1.diff
Re: [PATCH] Enable GCC support for AMX
Hi Kirill, could you help review this patch? Hongyu Wang 于2020年7月6日周一 上午9:58写道： > > Hi: > > This patch is about to support Intel Advanced Matrix Extensions (AMX) > which will be enabled in GLC. > > AMX is a new 64-bit programming paradigm consisting of two > compo nents: a set of 2-dimensional registers (tiles) representing > sub-arrays from a larger 2-dimensional memory image, > and an accelerator able to operate on tiles > > Supported instructions are > > AMX-TILE:ldtilecfg/sttilecfg/tileloadd/tileloaddt1/tilezero/tilerelease > AMX-INT8:tdpbssd/tdpbsud/tdpbusd/tdpbuud > AMX-BF16:tdpbf16ps > > The intrinsics adopts constant tile register number as its input parameters. > > For detailed information, please refer to > https://software.intel.com/content/dam/develop/public/us/en/documents/architecture-instruction-set-extensions-programming-reference.pdf > > Bootstrap ok, regression test on i386/x86 backend is ok. > > OK for master? > > gcc/ChangeLog > > * common/config/i386/i386-common.c (OPTION_MASK_ISA2_AMX_TILE_SET, > OPTION_MASK_ISA2_AMX_INT8_SET, OPTION_MASK_ISA2_AMX_BF16_SET, > OPTION_MASK_ISA2_AMX_TILE_UNSET, > OPTION_MASK_ISA2_AMX_INT8_UNSET, OPTION_MASK_ISA2_AMX_BF16_UNSET): > New marcos. > (ix86_handle_option): Hanlde -mamx-tile, -mamx-int8, -mamx-bf16. > * common/config/i386/i386-cpuinfo.h (processor_types): Add > FEATURE_AMX_TILE, FEATURE_AMX_INT8, FEATURE_AMX_BF16. > * common/config/i386/cpuinfo.h (XSTATE_TILECFG, > XSTATE_TILEDATA, XCR_AMX_ENABLED_MASK): New macro. > (get_available_features): Enable AMX features only if > their states are suoorited by OSXSAVE. > * common/config/i386/i386-isas.h: Add ISA_NAME_TABLE_ENTRY > for amx-tile, amx-int8, amx-bf16. > * config.gcc: Add amxtileintrin.h, amxint8intrin.h, > amxbf16intrin.h to extra headers. > * config/i386/amxbf16intrin.h: New file. > * config/i386/amxint8intrin.h: Ditto. > * config/i386/amxtileintrin.h: Ditto. > * config/i386/cpuid.h (bit_AMX_BF16, bit_AMX_TILE, bit_AMX_INT8): > New macro. > * config/i386/i386-c.c (ix86_target_macros_internal): Define > __AMX_TILE__, __AMX_INT8__, AMX_BF16__. > * config/i386/i386-options.c (ix86_target_string): Add > -mamx-tile, -mamx-int8, -mamx-bf16. > (ix86_option_override_internal): Handle AMX-TILE, > AMX-INT8, AMX-BF16. > * config/i386/i386.h (TARGET_AMX_TILE, TARGET_AMX_TILE_P, > TARGET_AMX_INT8, TARGET_AMX_INT8_P, TARGET_AMX_BF16_P, > PTA_AMX_TILE, PTA_AMX_INT8, PTA_AMX_BF16): New macros. > * config/i386/i386.opt: Add -mamx-tile, -mamx-int8, -mamx-bf16. > * config/i386/immintrin.h: Include amxtileintrin.h, > amxint8intrin.h, amxbf16intrin.h. > * doc/invoke.texi: Document -mamx-tile, -mamx-int8, -mamx-bf16. > * doc/extend.texi: Document amx-tile, amx-int8, amx-bf16. > * doc/sourcebuild.texi ((Effective-Target Keywords, Other > hardware attributes): Document amx_int8, amx_tile, amx_bf16. > > gcc/testsuite/ChangeLog > > * lib/target-supports.exp (check_effective_target_amx_tile, > check_effective_target_amx_int8, > check_effective_target_amx_bf16): New proc. > * g++.dg/other/i386-2.C: Add -mamx-tile, -mamx-int8, -mamx-bf16. > * g++.dg/other/i386-3.C: Ditto. > * gcc.target/i386/sse-12.c: Ditto. > * gcc.target/i386/sse-13.c: Ditto. > * gcc.target/i386/sse-14.c: Ditto. > * gcc.target/i386/sse-22.c: Ditto. > * gcc.target/i386/sse-23.c: Ditto. > * gcc.target/i386/funcspec-56.inc: Add new target attribute. > * gcc.target/i386/amxbf16-asmatt-1.c: New test. > * gcc.target/i386/amxint8-asmatt-1.c: Ditto. > * gcc.target/i386/amxtile-asmatt-1.c: Ditto. > * gcc.target/i386/amxbf16-asmintel-1.c: Ditto. > * gcc.target/i386/amxint8-asmintel-1.c: Ditto. > * gcc.target/i386/amxtile-asmintel-1.c: Ditto. > * gcc.target/i386/amxbf16-asmatt-2.c: Ditto. > * gcc.target/i386/amxint8-asmatt-2.c: Ditto. > * gcc.target/i386/amxtile-asmatt-2.c: Ditto. > * gcc.target/i386/amxbf16-asmintel-2.c: Ditto. > * gcc.target/i386/amxint8-asmintel-2.c: Ditto. > * gcc.target/i386/amxtile-asmintel-2.c: Ditto.
RE: [PATCH] pass correct parameters to c_parser_do_statement
Sorry for my mistake. The previous patch description is incorrect. A new patch was attached. Can anyone help me install this patch? Thanks, Kaipeng Zhou > -Original Message- > From: zhoukaipeng (A) > Sent: Tuesday, July 7, 2020 11:26 AM > To: gcc-patches@gcc.gnu.org > Subject: [PATCH] pass correct parameters to c_parser_do_statement > > Hi, > > It is a patch to pass correct parameters to c_parser_do_statement. > > Can anyone help me install this patch? > > Thanks, > Kaipeng Zhou code-cleanup-v2.diff Description: code-cleanup-v2.diff
[PATCH 1/2] PR94600: fix volatile access to the whole of a compound object.
The store to the whole of each volatile object was picked apart
like there had been an individual assignment to each of the
fields. Reads were added as part of that; see PR for details.
The reads from volatile memory were a clear bug; individual
stores questionable. A separate patch clarifies the docs.
Tested x86_64-linux, powerpc64le-linux and cris-elf.
Ok to commit? Backport to gcc-10?
gcc:
PR middle-end/94600
* expr.c (expand_constructor): Make a temporary also if we're
storing to volatile memory.
gcc/testsuite:
PR middle-end/94600
* gcc.dg/pr94600-1.c, gcc.dg/pr94600-2.c, gcc.dg/pr94600-3.c,
gcc.dg/pr94600-4.c, gcc.dg/pr94600-5.c, gcc.dg/pr94600-6.c,
gcc.dg/pr94600-7.c, gcc.dg/pr94600-8.c: New tests.
---
gcc/expr.c | 5 -
gcc/testsuite/gcc.dg/pr94600-1.c | 36
gcc/testsuite/gcc.dg/pr94600-2.c | 34 ++
gcc/testsuite/gcc.dg/pr94600-3.c | 35 +++
gcc/testsuite/gcc.dg/pr94600-4.c | 34 ++
gcc/testsuite/gcc.dg/pr94600-5.c | 34 ++
gcc/testsuite/gcc.dg/pr94600-6.c | 33 +
gcc/testsuite/gcc.dg/pr94600-7.c | 33 +
gcc/testsuite/gcc.dg/pr94600-8.c | 33 +
9 files changed, 276 insertions(+), 1 deletion(-)
create mode 100644 gcc/testsuite/gcc.dg/pr94600-1.c
create mode 100644 gcc/testsuite/gcc.dg/pr94600-2.c
create mode 100644 gcc/testsuite/gcc.dg/pr94600-3.c
create mode 100644 gcc/testsuite/gcc.dg/pr94600-4.c
create mode 100644 gcc/testsuite/gcc.dg/pr94600-5.c
create mode 100644 gcc/testsuite/gcc.dg/pr94600-6.c
create mode 100644 gcc/testsuite/gcc.dg/pr94600-7.c
create mode 100644 gcc/testsuite/gcc.dg/pr94600-8.c
diff --git a/gcc/expr.c b/gcc/expr.c
index 3c68b0d754c..44ea577e03d 100644
--- a/gcc/expr.c
+++ b/gcc/expr.c
@@ -8379,7 +8379,10 @@ expand_constructor (tree exp, rtx target, enum
expand_modifier modifier,
/* Handle calls that pass values in multiple non-contiguous
locations. The Irix 6 ABI has examples of this. */
if (target == 0 || ! safe_from_p (target, exp, 1)
- || GET_CODE (target) == PARALLEL || modifier == EXPAND_STACK_PARM)
+ || GET_CODE (target) == PARALLEL || modifier == EXPAND_STACK_PARM
+ /* Also make a temporary if the store is to volatile memory, to
+avoid individual accesses to aggregate members. */
+ || (GET_CODE (target) == MEM && MEM_VOLATILE_P (target)))
{
if (avoid_temp_mem)
return NULL_RTX;
diff --git a/gcc/testsuite/gcc.dg/pr94600-1.c b/gcc/testsuite/gcc.dg/pr94600-1.c
new file mode 100644
index 000..b5913a0939c
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr94600-1.c
@@ -0,0 +1,36 @@
+/* { dg-do compile } */
+/* { dg-require-effective-target size32plus } */
+/* { dg-options "-fdump-rtl-final -O2" } */
+
+/* Assignments to a whole struct of suitable size (32 bytes) must not be
+ picked apart into field accesses. */
+
+typedef struct {
+ unsigned int f0 : 4;
+ unsigned int f1 : 11;
+ unsigned int f2 : 10;
+ unsigned int f3 : 7;
+} t0;
+
+static t0 a0[] = {
+ { .f0 = 7, .f1 = 99, .f3 = 1, },
+ { .f0 = 7, .f1 = 251, .f3 = 1, },
+ { .f0 = 8, .f1 = 127, .f3 = 5, },
+ { .f0 = 5, .f1 = 1, .f3 = 1, },
+ { .f0 = 5, .f1 = 1, .f3 = 1, },
+ { .f0 = 5, .f1 = 1, .f3 = 1, },
+};
+
+void
+foo(void)
+{
+ __SIZE_TYPE__ i;
+ __SIZE_TYPE__ base = 0x000a;
+ for (i = 0; i < (sizeof (a0) / sizeof ((a0)[0])); i++) {
+*(volatile t0 *) (base + 44 + i * 4) = a0[i];
+ }
+}
+
+/* The only volatile accesses should be the obvious writes. */
+/* { dg-final { scan-rtl-dump-times {\(mem/v} 6 "final" } } */
+/* { dg-final { scan-rtl-dump-times {\(set \(mem/v} 6 "final" } } */
diff --git a/gcc/testsuite/gcc.dg/pr94600-2.c b/gcc/testsuite/gcc.dg/pr94600-2.c
new file mode 100644
index 000..cb96cc98a2d
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/pr94600-2.c
@@ -0,0 +1,34 @@
+/* { dg-do compile } */
+/* { dg-require-effective-target size32plus } */
+/* { dg-options "-fdump-rtl-final -O2" } */
+
+/* Unrolled version of pr94600-1.c. */
+
+typedef struct {
+ unsigned int f0 : 4;
+ unsigned int f1 : 11;
+ unsigned int f2 : 10;
+ unsigned int f3 : 7;
+} t0;
+
+void
+bar(void)
+{
+ t0 a00 = { .f0 = 7, .f1 = 99, .f3 = 1, };
+ t0 a01 = { .f0 = 7, .f1 = 251, .f3 = 1, };
+ t0 a02 = { .f0 = 8, .f1 = 127, .f3 = 5, };
+ t0 a03 = { .f0 = 5, .f1 = 1, .f3 = 1, };
+ t0 a04 = { .f0 = 5, .f1 = 1, .f3 = 1, };
+ t0 a05 = { .f0 = 5, .f1 = 1, .f3 = 1, };
+ __SIZE_TYPE__ base = 0x000a;
+
+ *(volatile t0 *) ((base) + 44 + (0) * 4) = a00;
+ *(volatile t0 *) ((base) + 44 + (1) * 4) = a01;
+ *(volatile t0 *) ((base) + 44 + (2) * 4) = a02;
+ *(volatile t0 *) ((base) + 44 + (3) * 4) = a03;
+ *(volatile t0 *) ((base) + 44 + (4) * 4) = a04;
+ *(volatile t0 *) ((base) + 44 + (5) * 4) = a05;
