[committed] Avoid -Wunused-value warnings on #pragma omp atomic capture (PR c++/89796)
Hi!
c-omp.c calls omit_one_operand which in several cases can wrap a MODIFY_EXPR
into NON_LVALUE_EXPR and similar. An atomic is always a whole
statement that has its side-effect(s), so -Wunused-value for that is not
really useful.
Fixed thusly, bootstrapped/regtested on x86_64-linux and i686-linux,
committed to trunk, queued for backports.
2019-03-26 Jakub Jelinek
PR c++/89796
* semantics.c (finish_omp_atomic): Add warning_sentinel for
-Wunused-value around finish_expr_stmt call.
* g++.dg/gomp/pr89796.C: New test.
* gcc.dg/gomp/pr89796.c: New test.
--- gcc/cp/semantics.c.jj 2019-03-22 15:42:02.469993735 +0100
+++ gcc/cp/semantics.c 2019-03-25 16:51:27.411936064 +0100
@@ -8997,6 +8997,11 @@ finish_omp_atomic (location_t loc, enum
OMP_ATOMIC_MEMORY_ORDER (stmt) = mo;
SET_EXPR_LOCATION (stmt, loc);
}
+
+ /* Avoid -Wunused-value warnings here, the whole construct has side-effects
+ and even if it might be wrapped from fold-const.c or c-omp.c wrapped
+ in some tree that appears to be unused, the value is not unused. */
+ warning_sentinel w (warn_unused_value);
finish_expr_stmt (stmt);
}
--- gcc/testsuite/g++.dg/gomp/pr89796.C.jj 2019-03-25 17:06:08.144778018
+0100
+++ gcc/testsuite/g++.dg/gomp/pr89796.C 2019-03-25 17:04:51.054018606 +0100
@@ -0,0 +1,53 @@
+// PR c++/89796
+// { dg-do compile }
+// { dg-additional-options "-Wunused-value" }
+
+int
+f1 (int &c)
+{
+ int r;
+ #pragma omp atomic capture // { dg-bogus "value computed is not used" }
+ { r = c; c++; }
+ return r;
+}
+
+template
+int
+f2 (int &c)
+{
+ int r;
+ #pragma omp atomic capture // { dg-bogus "value computed is not used" }
+ { r = c; c++; }
+ return r;
+}
+
+int
+f3 (int &c)
+{
+ return f2 <0> (c);
+}
+
+int
+f4 (int *p)
+{
+ int r;
+ #pragma omp atomic capture // { dg-bogus "value computed is not used" }
+ { r = *p; (*p)++; }
+ return r;
+}
+
+template
+int
+f5 (int *p)
+{
+ int r;
+ #pragma omp atomic capture // { dg-bogus "value computed is not used" }
+ { r = *p; (*p)++; }
+ return r;
+}
+
+int
+f6 (int *p)
+{
+ return f5 <0> (p);
+}
--- gcc/testsuite/gcc.dg/gomp/pr89796.c.jj 2019-03-25 17:06:28.446451316
+0100
+++ gcc/testsuite/gcc.dg/gomp/pr89796.c 2019-03-25 17:05:16.944601962 +0100
@@ -0,0 +1,23 @@
+/* PR c++/89796 */
+/* { dg-do compile } */
+/* { dg-additional-options "-Wunused-value" } */
+
+int
+f1 (int *p)
+{
+ int r;
+ #pragma omp atomic capture /* { dg-bogus "value computed is not
used" } */
+ { r = *p; (*p)++; }
+ return r;
+}
+
+int
+f2 (int *p)
+{
+ int s
+= ({ int r;
+#pragma omp atomic capture /* { dg-bogus "value computed is not
used" } */
+{ r = *p; (*p)++; }
+r; });
+ return s;
+}
Jakub
Re: New target hook for function scratch registers (PR 89628)
On Mon, Mar 25, 2019 at 6:48 PM Richard Sandiford wrote: > > Richard Biener writes: > > On Tue, Mar 12, 2019 at 10:50 AM Richard Sandiford > > wrote: > >> > >> Steve Ellcey writes: > >> > Richard, > >> > > >> > I don't necessarily disagree with anything in your comments and long > >> > term I think that is the right direction, but I wonder if that level of > >> > change is appropriate for GCC Stage 4 which is where we are now. Your > >> > changes would require fixes in shared code, whereas setting > >> > REG_ALLOC_ORDER only affects Aarch64 and seems like a safer change. > >> > >> I guess it's weighing invasiveness in terms of lines of code/location of > >> code vs. invasiveness in terms of the effect the code has. Changing > >> REG_ALLOC_ORDER affects all functinos that use significant numbers of > >> SIMD registers, which could have unexpected knock-on effects like > >> performance regressions or exposing latent bugs. It would also make > >> the RA try V24 before V16 for callers of vector PCS functions, > >> even though they should prefer V16. > >> > >> Keeping the change specific to callees that use the new vector PCS > >> feature seems more conservative from that point of view. > >> > >> > I am not sure how long it would take me to implement something along > >> > the lines of what you are suggesting. > >> > >> OK, in that case, I thought I'd give a go. > >> > >> This patch adds a new target hook that says which registers a function > >> can use without saving them in the prologue and restoring them in the > >> epilogue. By default this is call_used_reg_set. > >> > >> The patch only makes IRA use the hook, and in that context it's just > >> an (important) optimisation. But I think we need the same thing for > >> passes like regrename, where it'll be a correctness issue. I'll follow > >> on with patches there if this one is OK. > >> > >> Tested on aarch64-linux-gnu (with and without SVE) and x86_64-linux-gnu. > >> OK to install? > > > > So why is call_used_reg_set not updated appropriately? As you say > > if you introduce sth else it becomes a correctness issue to use that "else" > > rather than call_used_reg_set which was OK up until now. I wonder how > > other targets with a per-function different ABI handle this situation. Or > > are > > you the first and others at most affect calling conventions here? That is, > > it looks like making (parts of?) this_taret_hard_regs per function or > > re-computed at set_cfun time. There's alrady a hook for that btw, > > targetm.set_current_function (). Why not change call_used_reg_set there? > > The problem is that call_used_reg_set is a global that describes the > base ABI. It's used both for what the current function can use without > saving and restoring, and for what the target of a call insn might clobber. > Most uses are for the latter. > > The global set still needs to describe the base ABI, since the default > assumption is that a call insn uses the base ABI unless something says > otherwise. TARGET_HARD_REGNO_CALL_PART_CLOBBERED can be used for call > insns that use other ABIs, so it handles the case in which the current > function is the caller. But there's no hook for handling other ABIs > when the current function is the callee. > > So the purpose of the new hook is to split out what the current function > needs to save and restore as a separate bit of data that doesn't affect > call insns. Since it's an rtl-level thing, I thought it'd be better > to put it in crtl (i.e. the rtl info about the current function). > That way we only need to set it once at the start of the rtl passes, > rather than every time we change cfun via targetm.set_current_function. > (Well, longer-term, we might want to set it after RA too, so that the > scratch registers only include what the prologue and epilogue actually > save & restore.) > > Targets can already use targetm.hard_regno_scratch_ok and HARD_REGNO_RENAME_OK > to stop the post-RA passes using a particular register. I guess that's > been enough for what targets have needed till now, and we could use them > for the correctness thing I mentioned above. But that wouldn't solve the > RA problem. Making the information directly available seems better than > adding more hacks around it. Fair enough. I'm not too deep into this but it seems to me this is an area that needs proper documentation and a well-designed extension - both don't seem like a good fit for stage 4. Richard. > Thanks, > Richard
Re: One more patch for PR89676
> On Mar 26, 2019, at 12:22 AM, Vladimir Makarov wrote:
>
> Jeff Law recently found that my latest patch break some existing code
> compilation (the code is big to make test out of it).
>
> Here is the patch to fix it. The patch was successfully bootstrapped on
> x86-64. The patch actually results in less new transformations the previous
> patch introduced. So it should be safe.
>
> Committed as rev. 269924.
Hi Vladimir,
FWIW, this fixed linux kernel builds on AArch64 and ARM, which your first patch
caused.
The failure was:
===
slub.s: Assembler messages:
slub.s:26: Error: reg pair must start from even reg at operand 1 -- `casp
x1,x0,x3,x5,[x6]'
===
from a reduced testcase:
===
void *a;
long b, c;
void d(void) {
typeof(0) e=0;
asm(" casp\t%[old1], %[old2], %[new1], %[new2], %[v]\n"
: [old1] "+&r"(b), [old2] "+&r"(c), [v] "+Q"(a)
: [new1] "r"(d), [new2] "r"(e));
}
===
Is this the same bug that Jeff reported?
Thanks,
--
Maxim Kuvyrkov
www.linaro.org
Re: [PATCH PR81740]Enforce dependence check for outer loop vectorization
On Tue, Mar 26, 2019 at 1:56 AM Richard Sandiford
wrote:
>
> Richard Biener writes:
> > On Fri, Mar 22, 2019 at 7:12 AM Bin.Cheng wrote:
> >>
> >> On Thu, Mar 21, 2019 at 8:24 PM Richard Biener
> >> wrote:
> >> >
> >> > On Mon, Dec 18, 2017 at 1:37 PM Richard Biener
> >> > wrote:
> >> > >
> >> > > On Fri, Dec 15, 2017 at 7:39 PM, Bin.Cheng
> >> > > wrote:
> >> > > > On Fri, Dec 15, 2017 at 1:19 PM, Richard Biener
> >> > > > wrote:
> >> > > >> On Fri, Dec 15, 2017 at 1:35 PM, Bin.Cheng
> >> > > >> wrote:
> >> > > >>> On Fri, Dec 15, 2017 at 12:09 PM, Bin.Cheng
> >> > > >>> wrote:
> >> > > On Fri, Dec 15, 2017 at 11:55 AM, Richard Biener
> >> > > wrote:
> >> > > > On Fri, Dec 15, 2017 at 12:30 PM, Bin Cheng
> >> > > > wrote:
> >> > > >> Hi,
> >> > > >> As explained in the PR, given below test case:
> >> > > >> int a[8][10] = { [2][5] = 4 }, c;
> >> > > >>
> >> > > >> int
> >> > > >> main ()
> >> > > >> {
> >> > > >> short b;
> >> > > >> int i, d;
> >> > > >> for (b = 4; b >= 0; b--)
> >> > > >> for (c = 0; c <= 6; c++)
> >> > > >> a[c + 1][b + 2] = a[c][b + 1];
> >> > > >> for (i = 0; i < 8; i++)
> >> > > >> for (d = 0; d < 10; d++)
> >> > > >> if (a[i][d] != (i == 3 && d == 6) * 4)
> >> > > >> __builtin_abort ();
> >> > > >> return 0;
> >> > > >>
> >> > > >> the loop nest is illegal for vectorization without reversing
> >> > > >> inner loop. The issue
> >> > > >> is in data dependence checking of vectorizer, I believe the
> >> > > >> mentioned revision just
> >> > > >> exposed this. Previously the vectorization is skipped because
> >> > > >> of unsupported memory
> >> > > >> operation. The outer loop vectorization unrolls the outer loop
> >> > > >> into:
> >> > > >>
> >> > > >> for (b = 4; b > 0; b -= 4)
> >> > > >> {
> >> > > >> for (c = 0; c <= 6; c++)
> >> > > >> a[c + 1][6] = a[c][5];
> >> > > >> for (c = 0; c <= 6; c++)
> >> > > >> a[c + 1][5] = a[c][4];
> >> > > >> for (c = 0; c <= 6; c++)
> >> > > >> a[c + 1][4] = a[c][3];
> >> > > >> for (c = 0; c <= 6; c++)
> >> > > >> a[c + 1][3] = a[c][2];
> >> > > >> }
> >> > > >> Then four inner loops are fused into:
> >> > > >> for (b = 4; b > 0; b -= 4)
> >> > > >> {
> >> > > >> for (c = 0; c <= 6; c++)
> >> > > >> {
> >> > > >> a[c + 1][6] = a[c][5]; // S1
> >> > > >> a[c + 1][5] = a[c][4]; // S2
> >> > > >> a[c + 1][4] = a[c][3];
> >> > > >> a[c + 1][3] = a[c][2];
> >> > > >> }
> >> > > >> }
> >> > > >
> >> > > > Note that they are not really "fused" but they are interleaved.
> >> > > > With
> >> > > > GIMPLE in mind
> >> > > > that makes a difference, you should get the equivalent of
> >> > > >
> >> > > >for (c = 0; c <= 6; c++)
> >> > > > {
> >> > > >tem1 = a[c][5];
> >> > > >tem2 = a[c][4];
> >> > > >tem3 = a[c][3];
> >> > > >tem4 = a[c][2];
> >> > > >a[c+1][6] = tem1;
> >> > > >a[c +1][5] = tem2;
> >> > > > a[c+1][4] = tem3;
> >> > > > a[c+1][3] = tem4;
> >> > > > }
> >> > > Yeah, I will double check if this abstract breaks the patch and
> >> > > how.
> >> > > >>> Hmm, I think this doesn't break it, well at least for part of the
> >> > > >>> analysis, because it is loop carried (backward) dependence goes
> >> > > >>> wrong,
> >> > > >>> interleaving or not with the same iteration doesn't matter here.
> >> > > >>
> >> > > >> I think the idea is that forward dependences are always fine
> >> > > >> (negative distance)
> >> > > >> to vectorize. But with backward dependences we have to adhere to
> >> > > >> max_vf.
> >> > > >>
> >> > > >> It looks like for outer loop vectorization we only look at the
> >> > > >> distances in the
> >> > > >> outer loop but never at inner ones. But here the same applies but
> >> > > >> isn't that
> >> > > >> independend on the distances with respect to the outer loop?
> >> > > >>
> >> > > >> But maybe I'm misunderstanding how "distances" work here.
> >> > > > Hmm, I am not sure I understand "distance" correctly. With
> >> > > > description as in book like "Optimizing compilers for Modern
> >> > > > Architectures", distance is "# of iteration of sink ref - # of
> >> > > > iteration of source ref". Given below example:
> >> > > > for (i = 0; i < N; ++i)
> >> > > > {
> >> > > > x = arr[idx_1]; // S1
> >> > > > arr[idx_2] = x; // S2
> >> > > > }
> >> > > > if S1 is source ref, distance = idx_2 - idx_1, and distance > 0.
> >> > > > Also
> >> > > > this is forward dependence. For example, idx_1 is i + 1 and idx_2 is
> >> > > > i;
> >> > > > If S2 is source ref, distance = idx_1 - idx_
Re: [PATCH] Fix PR71598, aliasing between enums and compatible types
On Mon, 18 Mar 2019, Richard Biener wrote:
> On Fri, 15 Mar 2019, Jason Merrill wrote:
>
> > On 3/15/19 9:33 AM, Richard Biener wrote:
> > >
> > > The following is an attempt to fix PR71598 where C (and C++?) have
> > > an implementation-defined compatible integer type for each enum
> > > and the TBAA rules mandate that accesses using a compatible type
> > > are allowed.
> >
> > This does not apply to C++; an enum does not alias its underlying type.
>
> Thus the following different patch, introducing c_get_alias_set and
> only doing the special handling for C family frontends (I assume
> that at least ObjC is also affected).
>
> Bootstrap & regtest running on x86_64-unknown-linux-gnu, OK?
Ping. Also consider the additional testcase below to be added.
Richard.
2019-03-18 Richard Biener
PR c/71598
* gimple.c: Include langhooks.h.
(gimple_get_alias_set): Treat enumeral types as the underlying
integer type.
c/
* c-tree.h (c_get_alias_set): Declare.
* c-objc-common.h (LANG_HOOKS_GET_ALIAS_SET): Use c_get_alias_set.
* c-objc-common.c (c_get_alias_set): Treat enumeral types
as the underlying integer type.
* gcc.dg/torture/pr71598-1.c: New testcase.
* gcc.dg/torture/pr71598-2.c: Likewise.
* gcc.dg/torture/pr71598-3.c: Likewise.
Index: gcc/testsuite/gcc.dg/torture/pr71598-3.c
===
--- gcc/testsuite/gcc.dg/torture/pr71598-3.c(nonexistent)
+++ gcc/testsuite/gcc.dg/torture/pr71598-3.c(working copy)
@@ -0,0 +1,21 @@
+/* { dg-do run } */
+
+enum e1 { A, B };
+enum e2 { C, D };
+
+__attribute__((noinline,noclone))
+enum e1 f(unsigned int *p)
+{
+ *(enum e1 *)p = A;
+ *(enum e2 *)p = D;
+ return *(enum e1 *)p;
+}
+
+int main()
+{
+ unsigned int storage;
+
+ if (f(&storage) != B)
+__builtin_abort();
+ return 0;
+}
> Thanks,
> Richard.
>
> 2019-03-18 Richard Biener
>
> PR c/71598
> * gimple.c: Include langhooks.h.
> (gimple_get_alias_set): Treat enumeral types as the underlying
> integer type.
>
> c/
> * c-tree.h (c_get_alias_set): Declare.
> * c-objc-common.h (LANG_HOOKS_GET_ALIAS_SET): Use c_get_alias_set.
> * c-objc-common.c (c_get_alias_set): Treat enumeral types
> as the underlying integer type.
>
> * gcc.dg/torture/pr71598-1.c: New testcase.
> * gcc.dg/torture/pr71598-2.c: Likewise.
>
>
> Index: gcc/c/c-objc-common.c
> ===
> --- gcc/c/c-objc-common.c (revision 269752)
> +++ gcc/c/c-objc-common.c (working copy)
> @@ -265,3 +265,22 @@ c_vla_unspec_p (tree x, tree fn ATTRIBUT
> {
>return c_vla_type_p (x);
> }
> +
> +/* Special routine to get the alias set of T for C. */
> +
> +alias_set_type
> +c_get_alias_set (tree t)
> +{
> + /* Allow aliasing between enumeral types and the underlying
> + integer type. This is required since those are compatible types. */
> + if (TREE_CODE (t) == ENUMERAL_TYPE)
> +{
> + tree t1 = c_common_type_for_size (tree_to_uhwi (TYPE_SIZE (t)),
> + /* short-cut commoning to signed
> +type. */
> + false);
> + return get_alias_set (t1);
> +}
> +
> + return c_common_get_alias_set (t);
> +}
> Index: gcc/c/c-objc-common.h
> ===
> --- gcc/c/c-objc-common.h (revision 269752)
> +++ gcc/c/c-objc-common.h (working copy)
> @@ -43,7 +43,7 @@ along with GCC; see the file COPYING3.
> #undef LANG_HOOKS_POST_OPTIONS
> #define LANG_HOOKS_POST_OPTIONS c_common_post_options
> #undef LANG_HOOKS_GET_ALIAS_SET
> -#define LANG_HOOKS_GET_ALIAS_SET c_common_get_alias_set
> +#define LANG_HOOKS_GET_ALIAS_SET c_get_alias_set
> #undef LANG_HOOKS_PARSE_FILE
> #define LANG_HOOKS_PARSE_FILE c_common_parse_file
> #undef LANG_HOOKS_FINISH_INCOMPLETE_DECL
> Index: gcc/c/c-tree.h
> ===
> --- gcc/c/c-tree.h(revision 269752)
> +++ gcc/c/c-tree.h(working copy)
> @@ -623,6 +623,7 @@ extern bool c_missing_noreturn_ok_p (tre
> extern bool c_warn_unused_global_decl (const_tree);
> extern void c_initialize_diagnostics (diagnostic_context *);
> extern bool c_vla_unspec_p (tree x, tree fn);
> +extern alias_set_type c_get_alias_set (tree);
>
> /* in c-typeck.c */
> extern int in_alignof;
> Index: gcc/gimple.c
> ===
> --- gcc/gimple.c (revision 269752)
> +++ gcc/gimple.c (working copy)
> @@ -44,6 +44,7 @@ along with GCC; see the file COPYING3.
> #include "stringpool.h"
> #include "attribs.h"
> #include "asan.h"
> +#include "langhooks.h"
>
>
> /* All the tuples have their operand vector (if present) at the very bottom
> @@ -2587,6
[PATCH][OBVIOUS] Fix wrong option wrapping.
Hi. This is a fix for wrong option name wrapping. It was seen by Joseph. I'm going to install that as obvious. Thanks, Martin gcc/lto/ChangeLog: 2019-03-26 Martin Liska * lto-symtab.c (lto_symtab_merge_decls_2): Fix option name wrapping --- gcc/lto/lto-symtab.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/gcc/lto/lto-symtab.c b/gcc/lto/lto-symtab.c index ea9d21d33ce..d573ea74b9f 100644 --- a/gcc/lto/lto-symtab.c +++ b/gcc/lto/lto-symtab.c @@ -750,7 +750,7 @@ lto_symtab_merge_decls_2 (symtab_node *first, bool diagnosed_p) if (tbaa_p) inform (DECL_SOURCE_LOCATION (prevailing->decl), "code may be misoptimized unless " - "%<-fno-strict-aliasing is used%>"); + "%<-fno-strict-aliasing%> is used"); mismatches.release (); }
[PATCH] Fix up --enable-gather-detailed-mem-stats with lazy construction of hash_{table,set}
On Mon, Mar 25, 2019 at 09:42:01PM +0100, Jakub Jelinek wrote:
> On Mon, Mar 25, 2019 at 03:10:04PM -0400, Jason Merrill wrote:
> > > 2) has the false -> true fixed
> > > 3) ditto, but furthermore is moved out of add_capture into the lambda
> > > introducer parsing routine only, tests for [this, this] and [this, *this]
> > > etc. are done using LAMBDA_EXPR_THIS_CAPTURE and the duplicate check for
> > > capture_id is simplified too.
> >
> > I needed to make the below change to avoid crashing with
> > --enable-gather-detailed-mem-stats, make sense? Incidentally, why not set
>
> CCing Martin on this, I really don't know how the mem-stats.h stuff is
> supposed to work and how it works on reallocations.
> Seems release_instance_overhead is used in expand method without the second
> argument true and in the dtor with true, so the latter likely does both
> what expand needs to do and undo what register_descriptor did.
> And because register_descriptor has been called, it needs to be undone, but
> as no register_instance_overhead has been called, that part should not be.
> We don't have unregister_descriptor or something similar though.
> So, the fix probably needs to add something in mem-stats.h and do what your
> patch did + else if (m_gather_mem_stats) call this new mem_stat.h method
> after the if (!Lazy || m_entries) block.
Here is a patch that does that.
Bootstrapped on x86_64-linux with --enable-gather-detailed-mem-stats,
tested on a couple of lambda testcases with -fmem-report, some of them with
zero or one explicit captures, on others with more of them (e.g. on
lambda-init1{8,9}.C) and tested build without
--enable-gather-detailed-mem-stats. Ok for trunk?
2019-03-26 Jason Merrill
Jakub Jelinek
* mem-stats.h (mem_alloc_description::unregister_descriptor): New
method.
(mem_alloc_description::release_object_overhead): Fix comment typos.
* hash-table.h (hash_table::~hash_table): Call
release_instance_overhead only if m_entries is non-NULL, otherwise
call unregister_descriptor.
--- gcc/mem-stats.h.jj 2019-02-26 21:35:28.959081094 +0100
+++ gcc/mem-stats.h 2019-03-26 09:25:10.132128088 +0100
@@ -342,9 +342,15 @@ public:
T *release_instance_overhead (void *ptr, size_t size,
bool remove_from_map = false);
- /* Release intance object identified by PTR pointer. */
+ /* Release instance object identified by PTR pointer. */
void release_object_overhead (void *ptr);
+ /* Unregister a memory allocation descriptor registered with
+ register_descriptor (remove from reverse map), unless it is
+ unregistered through release_instance_overhead with
+ REMOVE_FROM_MAP = true. */
+ void unregister_descriptor (void *ptr);
+
/* Get sum value for ORIGIN type of allocation for the descriptor. */
T get_sum (mem_alloc_origin origin);
@@ -522,7 +528,7 @@ mem_alloc_description::release_instan
return usage;
}
-/* Release intance object identified by PTR pointer. */
+/* Release instance object identified by PTR pointer. */
template
inline void
@@ -536,6 +542,17 @@ mem_alloc_description::release_object
}
}
+/* Unregister a memory allocation descriptor registered with
+ register_descriptor (remove from reverse map), unless it is
+ unregistered through release_instance_overhead with
+ REMOVE_FROM_MAP = true. */
+template
+inline void
+mem_alloc_description::unregister_descriptor (void *ptr)
+{
+ m_reverse_map->remove (ptr);
+}
+
/* Default contructor. */
template
--- gcc/hash-table.h.jj 2019-03-26 08:52:52.739640547 +0100
+++ gcc/hash-table.h2019-03-26 09:26:27.697864773 +0100
@@ -652,12 +652,13 @@ hash_table:
Allocator ::data_free (m_entries);
else
ggc_free (m_entries);
+ if (m_gather_mem_stats)
+ hash_table_usage ().release_instance_overhead (this,
+ sizeof (value_type)
+ * m_size, true);
}
-
- if (m_gather_mem_stats)
-hash_table_usage ().release_instance_overhead (this,
- sizeof (value_type)
- * m_size, true);
+ else if (m_gather_mem_stats)
+hash_table_usage ().unregister_descriptor (this);
}
/* This function returns an array of empty hash table elements. */
Jakub
Re: [PATCH][stage1] Wrap option names in gcc internal messages with %< and %>.
On 3/22/19 11:02 PM, Joseph Myers wrote: > This one seems wrong: > >> - "-fno-strict-aliasing is used"); >> + "%<-fno-strict-aliasing is used%>"); > > (only the option name should be quoted, not "is used"). > Hi. It's fixed now. Thanks, Martin
[PATCH, committed] gdc-test.exp: Remove duplicate options from permute args
Hi,
Some of the dmd-style options converted in gdc-convert-args are mapped
to two separate options, notably those that control warning and
deprecation messages (-w, -de, -dw). When given as arguments to
PERMUTE_ARGS, there is no need to test with the same gdc-style option
given twice.
Ran the D testsuite on x86_64-linux-gnu, and number of tests executed
dropped from 30030 to 28350.
Committed to trunk as r269937.
--
Iain
---
gcc/testsuite/ChangeLog:
2019-03-26 Iain Buclaw
* gdc.test/gdc-test.exp (gdc-do-test): Sort and remove duplicate
options in permute args tests.
---
diff --git a/gcc/testsuite/gdc.test/gdc-test.exp b/gcc/testsuite/gdc.test/gdc-test.exp
index f2772e9c9e6..59abf74dc89 100644
--- a/gcc/testsuite/gdc.test/gdc-test.exp
+++ b/gcc/testsuite/gdc.test/gdc-test.exp
@@ -393,7 +393,7 @@ proc gdc-do-test { } {
if { $dir == "runnable" } {
append PERMUTE_ARGS " $SHARED_OPTION"
}
- set options [gdc-permute-options $PERMUTE_ARGS]
+ set options [gdc-permute-options [lsort -unique $PERMUTE_ARGS]]
switch $dir {
runnable {
Re: [PATCH] Improve hash-table.h generated code without --enable-gather-detailed-mem-stats
On 3/26/19 12:21 AM, Jakub Jelinek wrote: > I know it isn't that much, but 74KB savings on .text seems to be worth to > me with such a small patch. Ok for trunk? Hi. I like the suggested trick with const static field in case of memory reporting is disabled. Martin
Re: [PATCH] Fix up --enable-gather-detailed-mem-stats with lazy construction of hash_{table,set}
On 3/26/19 10:56 AM, Jakub Jelinek wrote:
> On Mon, Mar 25, 2019 at 09:42:01PM +0100, Jakub Jelinek wrote:
>> On Mon, Mar 25, 2019 at 03:10:04PM -0400, Jason Merrill wrote:
2) has the false -> true fixed
3) ditto, but furthermore is moved out of add_capture into the lambda
introducer parsing routine only, tests for [this, this] and [this, *this]
etc. are done using LAMBDA_EXPR_THIS_CAPTURE and the duplicate check for
capture_id is simplified too.
>>>
>>> I needed to make the below change to avoid crashing with
>>> --enable-gather-detailed-mem-stats, make sense? Incidentally, why not set
>>
>> CCing Martin on this, I really don't know how the mem-stats.h stuff is
>> supposed to work and how it works on reallocations.
>> Seems release_instance_overhead is used in expand method without the second
>> argument true and in the dtor with true, so the latter likely does both
>> what expand needs to do and undo what register_descriptor did.
>> And because register_descriptor has been called, it needs to be undone, but
>> as no register_instance_overhead has been called, that part should not be.
>> We don't have unregister_descriptor or something similar though.
>> So, the fix probably needs to add something in mem-stats.h and do what your
>> patch did + else if (m_gather_mem_stats) call this new mem_stat.h method
>> after the if (!Lazy || m_entries) block.
>
> Here is a patch that does that.
The patch is correct. Note that unregistering of a descriptor is not a critical,
but yes, it occupies a memory.
Martin
>
> Bootstrapped on x86_64-linux with --enable-gather-detailed-mem-stats,
> tested on a couple of lambda testcases with -fmem-report, some of them with
> zero or one explicit captures, on others with more of them (e.g. on
> lambda-init1{8,9}.C) and tested build without
> --enable-gather-detailed-mem-stats. Ok for trunk?
>
> 2019-03-26 Jason Merrill
> Jakub Jelinek
>
> * mem-stats.h (mem_alloc_description::unregister_descriptor): New
> method.
> (mem_alloc_description::release_object_overhead): Fix comment typos.
> * hash-table.h (hash_table::~hash_table): Call
> release_instance_overhead only if m_entries is non-NULL, otherwise
> call unregister_descriptor.
>
> --- gcc/mem-stats.h.jj2019-02-26 21:35:28.959081094 +0100
> +++ gcc/mem-stats.h 2019-03-26 09:25:10.132128088 +0100
> @@ -342,9 +342,15 @@ public:
>T *release_instance_overhead (void *ptr, size_t size,
> bool remove_from_map = false);
>
> - /* Release intance object identified by PTR pointer. */
> + /* Release instance object identified by PTR pointer. */
>void release_object_overhead (void *ptr);
>
> + /* Unregister a memory allocation descriptor registered with
> + register_descriptor (remove from reverse map), unless it is
> + unregistered through release_instance_overhead with
> + REMOVE_FROM_MAP = true. */
> + void unregister_descriptor (void *ptr);
> +
>/* Get sum value for ORIGIN type of allocation for the descriptor. */
>T get_sum (mem_alloc_origin origin);
>
> @@ -522,7 +528,7 @@ mem_alloc_description::release_instan
>return usage;
> }
>
> -/* Release intance object identified by PTR pointer. */
> +/* Release instance object identified by PTR pointer. */
>
> template
> inline void
> @@ -536,6 +542,17 @@ mem_alloc_description::release_object
> }
> }
>
> +/* Unregister a memory allocation descriptor registered with
> + register_descriptor (remove from reverse map), unless it is
> + unregistered through release_instance_overhead with
> + REMOVE_FROM_MAP = true. */
> +template
> +inline void
> +mem_alloc_description::unregister_descriptor (void *ptr)
> +{
> + m_reverse_map->remove (ptr);
> +}
> +
> /* Default contructor. */
>
> template
> --- gcc/hash-table.h.jj 2019-03-26 08:52:52.739640547 +0100
> +++ gcc/hash-table.h 2019-03-26 09:26:27.697864773 +0100
> @@ -652,12 +652,13 @@ hash_table:
> Allocator ::data_free (m_entries);
>else
> ggc_free (m_entries);
> + if (m_gather_mem_stats)
> + hash_table_usage ().release_instance_overhead (this,
> +sizeof (value_type)
> +* m_size, true);
> }
> -
> - if (m_gather_mem_stats)
> -hash_table_usage ().release_instance_overhead (this,
> -sizeof (value_type)
> -* m_size, true);
> + else if (m_gather_mem_stats)
> +hash_table_usage ().unregister_descriptor (this);
> }
>
> /* This function returns an array of empty hash table elements. */
>
>
> Jakub
>
Re: C++ PATCH for c++/89214 - ICE when initializing aggregates with bases
I don't see any of the warnings in the tests on ia64. FAIL: g++.dg/cpp1z/aggr-base8.C -std=c++17 (test for warnings, line 33) FAIL: g++.dg/cpp1z/aggr-base8.C -std=c++17 (test for warnings, line 34) FAIL: g++.dg/cpp1z/aggr-base8.C -std=c++17 (test for warnings, line 35) FAIL: g++.dg/cpp1z/aggr-base8.C -std=c++17 (test for warnings, line 45) FAIL: g++.dg/cpp1z/aggr-base8.C -std=c++17 (test for warnings, line 46) FAIL: g++.dg/cpp1z/aggr-base8.C -std=c++17 (test for warnings, line 47) PASS: g++.dg/cpp1z/aggr-base8.C -std=c++17 (test for excess errors) FAIL: g++.dg/cpp1z/aggr-base9.C -std=c++17 (test for warnings, line 22) FAIL: g++.dg/cpp1z/aggr-base9.C -std=c++17 (test for warnings, line 23) FAIL: g++.dg/cpp1z/aggr-base9.C -std=c++17 (test for warnings, line 31) FAIL: g++.dg/cpp1z/aggr-base9.C -std=c++17 (test for warnings, line 32) PASS: g++.dg/cpp1z/aggr-base9.C -std=c++17 (test for excess errors) Andreas. -- Andreas Schwab, SUSE Labs, sch...@suse.de GPG Key fingerprint = 0196 BAD8 1CE9 1970 F4BE 1748 E4D4 88E3 0EEA B9D7 "And now for something completely different."
[PATCH][PR89341]Fix ICE on function definition with weakref/alias attributes attached
Hi
According to gnu document of function attributes, neither weakref nor alias
could be attached to a function defined in current translation unit.
Although GCC checks the attributes under some circumstances, it still fails
on some cases and even causes ICE.
This patch checks whether alias/weakref attribute attaches on a function
declaration which has body, and removes the attribute later.
This also avoid the ICE.
Bootstrapped/regtested on x86_64-linux, ok for GCC10?
Regards
JunMa
gcc/ChangeLog
2019-03-26 Jun Ma
PR89341
* cgraphunit.c (handle_alias_pairs): Check whether alias attribute
attaches
on a function declaration which has body.
gcc/testsuite/ChangeLog
2019-03-26 Jun Ma
PR89341
* gcc.dg/attr-alias-6.c: New test.
* gcc.dg/attr-weakref-5.c: Likewise.
---
gcc/cgraphunit.c | 11 ---
gcc/testsuite/gcc.dg/attr-alias-6.c | 4
gcc/testsuite/gcc.dg/attr-weakref-5.c | 4
3 files changed, 16 insertions(+), 3 deletions(-)
create mode 100644 gcc/testsuite/gcc.dg/attr-alias-6.c
create mode 100644 gcc/testsuite/gcc.dg/attr-weakref-5.c
diff --git a/gcc/cgraphunit.c b/gcc/cgraphunit.c
index 8bfbd0b..733e184 100644
--- a/gcc/cgraphunit.c
+++ b/gcc/cgraphunit.c
@@ -1405,14 +1405,20 @@ handle_alias_pairs (void)
for (i = 0; alias_pairs && alias_pairs->iterate (i, &p);)
{
symtab_node *target_node = symtab_node::get_for_asmname (p->target);
-
+ symtab_node *node = symtab_node::get (p->decl);
+ if (node && TREE_CODE (p->decl) == FUNCTION_DECL
+ && DECL_SAVED_TREE (p->decl))
+ {
+ node->alias = false;
+ alias_pairs->unordered_remove (i);
+ continue;
+ }
/* Weakrefs with target not defined in current unit are easy to handle:
they behave just as external variables except we need to note the
alias flag to later output the weakref pseudo op into asm file. */
if (!target_node
&& lookup_attribute ("weakref", DECL_ATTRIBUTES (p->decl)) != NULL)
{
- symtab_node *node = symtab_node::get (p->decl);
if (node)
{
node->alias_target = p->target;
@@ -1426,7 +1432,6 @@ handle_alias_pairs (void)
else if (!target_node)
{
error ("%q+D aliased to undefined symbol %qE", p->decl, p->target);
- symtab_node *node = symtab_node::get (p->decl);
if (node)
node->alias = false;
alias_pairs->unordered_remove (i);
diff --git a/gcc/testsuite/gcc.dg/attr-alias-6.c
b/gcc/testsuite/gcc.dg/attr-alias-6.c
new file mode 100644
index 000..a3ec311
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/attr-alias-6.c
@@ -0,0 +1,4 @@
+/* { dg-do compile } */
+/* { dg-require-alias "" } */
+static void __attribute__((alias("bar"))) foo(void) { } /* { dg-warning
"attribute ignored because function is defined" } */
+void bar(void);
diff --git a/gcc/testsuite/gcc.dg/attr-weakref-5.c
b/gcc/testsuite/gcc.dg/attr-weakref-5.c
new file mode 100644
index 000..bb21126
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/attr-weakref-5.c
@@ -0,0 +1,4 @@
+/* { dg-do compile } */
+/* { dg-require-weak "" } */
+static void __attribute__((weakref("bar"))) foo(void) { } /* { dg-warning
"attribute ignored because function is defined" } */
+void foo(void);
--
1.8.3.1
Re: [patch] PR jit/87808: Allow libgccjit to work without an external gcc driver
On 22.03.19 23:00, David Malcolm wrote: > On Thu, 2019-03-21 at 12:26 +0100, Matthias Klose wrote: >> Fix PR jit/87808, the embedded driver still needing the external gcc >> driver to >> find the gcc_lib_dir. This can happen in a packaging context when >> libgccjit >> doesn't depend on the gcc package, but just on binutils and libgcc- >> dev packages. >> libgccjit probably could use /proc/self/maps to find the gcc_lib_dir, >> but that >> doesn't seem to be very portable. >> >> Ok for the trunk and the branches? >> >> Matthias > > [CCing the jit list] > > I've been trying to reproduce this bug in a working copy, and failing. > > Matthias, do you have a recipe you've been using to reproduce this? the JIT debug log shows the driver names that it wants to call. Are you sure that this driver isn't available anywhere? I configure the gcc build with --program-suffix=-8 --program-prefix=x86_64-linux-gnu-, and that one was only available in one place, /usr/bin. Matthias
[v3 PATCH] PR libstdc++/89824
A minor thinko, very easy to fix. 2019-03-26 Ville Voutilainen PR libstdc++/89824 Fix based on a suggestion by Antony Polukhin. * include/std/variant (__gen_vtable): Don't reserve an additional table slot, _Multi_array already does that. diff --git a/libstdc++-v3/include/std/variant b/libstdc++-v3/include/std/variant index 0984e13..3631463 100644 --- a/libstdc++-v3/include/std/variant +++ b/libstdc++-v3/include/std/variant @@ -904,9 +904,7 @@ namespace __variant using _Func_ptr = _Result_type (*)(_Visitor&&, _Variants...); using _Array_type = _Multi_array<_Func_ptr, - (variant_size_v> - + (is_same_v<_Result_type, __variant_cookie> ? 1 : 0)) - ...>; + variant_size_v>...>; static constexpr _Array_type _S_apply()
Re: One more patch for PR89676
On 3/26/19 4:25 AM, Maxim Kuvyrkov wrote:
On Mar 26, 2019, at 12:22 AM, Vladimir Makarov wrote:
Jeff Law recently found that my latest patch break some existing code
compilation (the code is big to make test out of it).
Here is the patch to fix it. The patch was successfully bootstrapped on
x86-64. The patch actually results in less new transformations the previous
patch introduced. So it should be safe.
Committed as rev. 269924.
Hi Vladimir,
FWIW, this fixed linux kernel builds on AArch64 and ARM, which your first patch
caused.
The failure was:
===
slub.s: Assembler messages:
slub.s:26: Error: reg pair must start from even reg at operand 1 -- `casp
x1,x0,x3,x5,[x6]'
===
from a reduced testcase:
===
void *a;
long b, c;
void d(void) {
typeof(0) e=0;
asm(" casp\t%[old1], %[old2], %[new1], %[new2], %[v]\n"
: [old1] "+&r"(b), [old2] "+&r"(c), [v] "+Q"(a)
: [new1] "r"(d), [new2] "r"(e));
}
===
Is this the same bug that Jeff reported?
Yes, it looks similar (reg pair and casp). I did know it was a kernel. I only
had a preprocessed file slub.i.
The reduced case is actually wrong. The constraints +&r does not guarantee
that b and c will be in the subsequent regs. In the original test case b and c
were also local *register* variables (x0 and x1). Generally speaking even this
does no guarantee (according to the documentation) that the original variable
values will be in subsequent regs for casp. But as people assume such
(undocumented) semantics, we should maintain this.
Re: One more patch for PR89676
> On Mar 26, 2019, at 3:20 PM, Vladimir Makarov wrote:
>
> On 3/26/19 4:25 AM, Maxim Kuvyrkov wrote:
>>> On Mar 26, 2019, at 12:22 AM, Vladimir Makarov wrote:
>>>
>>> Jeff Law recently found that my latest patch break some existing code
>>> compilation (the code is big to make test out of it).
>>>
>>> Here is the patch to fix it. The patch was successfully bootstrapped on
>>> x86-64. The patch actually results in less new transformations the
>>> previous patch introduced. So it should be safe.
>>>
>>> Committed as rev. 269924.
>> Hi Vladimir,
>>
>> FWIW, this fixed linux kernel builds on AArch64 and ARM, which your first
>> patch caused.
>>
>> The failure was:
>> ===
>> slub.s: Assembler messages:
>> slub.s:26: Error: reg pair must start from even reg at operand 1 -- `casp
>> x1,x0,x3,x5,[x6]'
>> ===
>>
>> from a reduced testcase:
>> ===
>> void *a;
>> long b, c;
>> void d(void) {
>> typeof(0) e=0;
>> asm(" casp\t%[old1], %[old2], %[new1], %[new2], %[v]\n"
>> : [old1] "+&r"(b), [old2] "+&r"(c), [v] "+Q"(a)
>> : [new1] "r"(d), [new2] "r"(e));
>> }
>> ===
>>
>> Is this the same bug that Jeff reported?
>>
> Yes, it looks similar (reg pair and casp). I did know it was a kernel. I
> only had a preprocessed file slub.i.
>
> The reduced case is actually wrong. The constraints +&r does not guarantee
> that b and c will be in the subsequent regs.
Right, this was reduction artifact.
> In the original test case b and c were also local *register* variables (x0
> and x1). Generally speaking even this does no guarantee (according to the
> documentation) that the original variable values will be in subsequent regs
> for casp.
I don't follow. Do you mean that in the below testcase it's not guaranteed
that casp will get its first two arguments in x0 and x1? (If so, why?)
===
void *a;
long b, c;
void d(void) {
typeof(0) e=0;
register long x0 asm ("x0") = b;
register long x1 asm ("x1") = c;
asm(" casp\t%[old1], %[old2], %[new1], %[new2], %[v]\n"
: [old1] "+&r"(b), [old2] "+&r"(c), [v] "+Q"(a)
: [new1] "r"(d), [new2] "r"(e));
}
===
IIRC, there is plenty of syscall code in glibc that relies on asms getting
variables in "right" registers.
> But as people assume such (undocumented) semantics, we should maintain this.
Regards,
--
Maxim Kuvyrkov
www.linaro.org
Re: [v3 PATCH] PR libstdc++/89824
On 26/03/19 14:14 +0200, Ville Voutilainen wrote: A minor thinko, very easy to fix. OK, thanks.
Re: [PATCH] Fix up --enable-gather-detailed-mem-stats with lazy construction of hash_{table,set}
On Tue, 26 Mar 2019, Martin Liška wrote:
> On 3/26/19 10:56 AM, Jakub Jelinek wrote:
> > On Mon, Mar 25, 2019 at 09:42:01PM +0100, Jakub Jelinek wrote:
> >> On Mon, Mar 25, 2019 at 03:10:04PM -0400, Jason Merrill wrote:
> 2) has the false -> true fixed
> 3) ditto, but furthermore is moved out of add_capture into the lambda
> introducer parsing routine only, tests for [this, this] and [this, *this]
> etc. are done using LAMBDA_EXPR_THIS_CAPTURE and the duplicate check for
> capture_id is simplified too.
> >>>
> >>> I needed to make the below change to avoid crashing with
> >>> --enable-gather-detailed-mem-stats, make sense? Incidentally, why not set
> >>
> >> CCing Martin on this, I really don't know how the mem-stats.h stuff is
> >> supposed to work and how it works on reallocations.
> >> Seems release_instance_overhead is used in expand method without the second
> >> argument true and in the dtor with true, so the latter likely does both
> >> what expand needs to do and undo what register_descriptor did.
> >> And because register_descriptor has been called, it needs to be undone, but
> >> as no register_instance_overhead has been called, that part should not be.
> >> We don't have unregister_descriptor or something similar though.
> >> So, the fix probably needs to add something in mem-stats.h and do what your
> >> patch did + else if (m_gather_mem_stats) call this new mem_stat.h method
> >> after the if (!Lazy || m_entries) block.
> >
> > Here is a patch that does that.
>
> The patch is correct. Note that unregistering of a descriptor is not a
> critical,
> but yes, it occupies a memory.
Likewise OK.
Richard.
> Martin
>
> >
> > Bootstrapped on x86_64-linux with --enable-gather-detailed-mem-stats,
> > tested on a couple of lambda testcases with -fmem-report, some of them with
> > zero or one explicit captures, on others with more of them (e.g. on
> > lambda-init1{8,9}.C) and tested build without
> > --enable-gather-detailed-mem-stats. Ok for trunk?
> >
> > 2019-03-26 Jason Merrill
> > Jakub Jelinek
> >
> > * mem-stats.h (mem_alloc_description::unregister_descriptor): New
> > method.
> > (mem_alloc_description::release_object_overhead): Fix comment typos.
> > * hash-table.h (hash_table::~hash_table): Call
> > release_instance_overhead only if m_entries is non-NULL, otherwise
> > call unregister_descriptor.
> >
> > --- gcc/mem-stats.h.jj 2019-02-26 21:35:28.959081094 +0100
> > +++ gcc/mem-stats.h 2019-03-26 09:25:10.132128088 +0100
> > @@ -342,9 +342,15 @@ public:
> >T *release_instance_overhead (void *ptr, size_t size,
> > bool remove_from_map = false);
> >
> > - /* Release intance object identified by PTR pointer. */
> > + /* Release instance object identified by PTR pointer. */
> >void release_object_overhead (void *ptr);
> >
> > + /* Unregister a memory allocation descriptor registered with
> > + register_descriptor (remove from reverse map), unless it is
> > + unregistered through release_instance_overhead with
> > + REMOVE_FROM_MAP = true. */
> > + void unregister_descriptor (void *ptr);
> > +
> >/* Get sum value for ORIGIN type of allocation for the descriptor. */
> >T get_sum (mem_alloc_origin origin);
> >
> > @@ -522,7 +528,7 @@ mem_alloc_description::release_instan
> >return usage;
> > }
> >
> > -/* Release intance object identified by PTR pointer. */
> > +/* Release instance object identified by PTR pointer. */
> >
> > template
> > inline void
> > @@ -536,6 +542,17 @@ mem_alloc_description::release_object
> > }
> > }
> >
> > +/* Unregister a memory allocation descriptor registered with
> > + register_descriptor (remove from reverse map), unless it is
> > + unregistered through release_instance_overhead with
> > + REMOVE_FROM_MAP = true. */
> > +template
> > +inline void
> > +mem_alloc_description::unregister_descriptor (void *ptr)
> > +{
> > + m_reverse_map->remove (ptr);
> > +}
> > +
> > /* Default contructor. */
> >
> > template
> > --- gcc/hash-table.h.jj 2019-03-26 08:52:52.739640547 +0100
> > +++ gcc/hash-table.h2019-03-26 09:26:27.697864773 +0100
> > @@ -652,12 +652,13 @@ hash_table:
> > Allocator ::data_free (m_entries);
> >else
> > ggc_free (m_entries);
> > + if (m_gather_mem_stats)
> > + hash_table_usage ().release_instance_overhead (this,
> > + sizeof (value_type)
> > + * m_size, true);
> > }
> > -
> > - if (m_gather_mem_stats)
> > -hash_table_usage ().release_instance_overhead (this,
> > - sizeof (value_type)
> > - * m_size, true);
> > + else if (m_gather_mem_stats)
> > +hash_table_usage ().unregister_descriptor (this);
> > }
> >
> > /* This function returns an a
Re: [PATCH] Improve hash-table.h generated code without --enable-gather-detailed-mem-stats
On Tue, 26 Mar 2019, Martin Liška wrote: > On 3/26/19 12:21 AM, Jakub Jelinek wrote: > > I know it isn't that much, but 74KB savings on .text seems to be worth to > > me with such a small patch. Ok for trunk? > > Hi. > > I like the suggested trick with const static field in case > of memory reporting is disabled. OK then. Richard.
[PATCH] Avoid redundant BLOCK lookup/set
Just found this in one of my dev trees and think it's suitable at
this stage as a general speedup.
gimple_block involves querying an on-the-side map in libcpp
since it is encoded in the location_t location. This means
caching is beneficial. It's also pointless to copy
gimple_block once you've copied gimple_location.
Bootstrapped on x86_64-unknown-linux-gnu, testing in progress.
Richard.
2019-03-26 Richard Biener
* tree-inline.c (remap_gimple_stmt): Cache gimple_block.
(copy_debug_stmt): Likewise.
(expand_call_inline): Likewise.
(copy_bb): Avoid redundant lookup & set of gimple_block.
* gimple-low.c (lower_gimple_return): Likewise.
(lower_builtin_setjmp): Likewise.
Index: gcc/tree-inline.c
===
--- gcc/tree-inline.c (revision 269937)
+++ gcc/tree-inline.c (working copy)
@@ -1776,10 +1776,10 @@ remap_gimple_stmt (gimple *stmt, copy_bo
}
/* If STMT has a block defined, map it to the newly constructed block. */
- if (gimple_block (copy))
+ if (tree block = gimple_block (copy))
{
tree *n;
- n = id->decl_map->get (gimple_block (copy));
+ n = id->decl_map->get (block);
gcc_assert (n);
gimple_set_block (copy, *n);
}
@@ -1951,8 +1951,8 @@ copy_bb (copy_body_data *id, basic_block
GF_CALL_VA_ARG_PACK. */
gimple_call_copy_flags (new_call, call_stmt);
gimple_call_set_va_arg_pack (new_call, false);
+ /* location includes block. */
gimple_set_location (new_call, gimple_location (stmt));
- gimple_set_block (new_call, gimple_block (stmt));
gimple_call_set_lhs (new_call, gimple_call_lhs (call_stmt));
gsi_replace (©_gsi, new_call, false);
@@ -2891,9 +2891,9 @@ copy_debug_stmt (gdebug *stmt, copy_body
tree t, *n;
struct walk_stmt_info wi;
- if (gimple_block (stmt))
+ if (tree block = gimple_block (stmt))
{
- n = id->decl_map->get (gimple_block (stmt));
+ n = id->decl_map->get (block);
gimple_set_block (stmt, n ? *n : id->block);
}
@@ -4589,7 +4589,7 @@ expand_call_inline (basic_block bb, gimp
artificial decls inserted by the compiler itself. We need to
either link the inlined blocks into the caller block tree or
not refer to them in any way to not break GC for locations. */
- if (gimple_block (stmt))
+ if (tree block = gimple_block (stmt))
{
/* We do want to assign a not UNKNOWN_LOCATION BLOCK_SOURCE_LOCATION
to make inlined_function_outer_scope_p return true on this BLOCK. */
@@ -4601,7 +4601,7 @@ expand_call_inline (basic_block bb, gimp
id->block = make_node (BLOCK);
BLOCK_ABSTRACT_ORIGIN (id->block) = DECL_ORIGIN (fn);
BLOCK_SOURCE_LOCATION (id->block) = loc;
- prepend_lexical_block (gimple_block (stmt), id->block);
+ prepend_lexical_block (block, id->block);
}
/* Local declarations will be replaced by their equivalents in this map. */
Index: gcc/gimple-low.c
===
--- gcc/gimple-low.c(revision 269939)
+++ gcc/gimple-low.c(working copy)
@@ -723,8 +723,8 @@ lower_gimple_return (gimple_stmt_iterato
if (!optimize && gimple_has_location (stmt))
DECL_ARTIFICIAL (tmp_rs.label) = 0;
t = gimple_build_goto (tmp_rs.label);
+ /* location includes block. */
gimple_set_location (t, gimple_location (stmt));
- gimple_set_block (t, gimple_block (stmt));
gsi_insert_before (gsi, t, GSI_SAME_STMT);
gsi_remove (gsi, false);
}
@@ -806,8 +806,8 @@ lower_builtin_setjmp (gimple_stmt_iterat
arg = build_addr (next_label);
t = builtin_decl_implicit (BUILT_IN_SETJMP_SETUP);
g = gimple_build_call (t, 2, gimple_call_arg (stmt, 0), arg);
+ /* location includes block. */
gimple_set_location (g, loc);
- gimple_set_block (g, gimple_block (stmt));
gsi_insert_before (gsi, g, GSI_SAME_STMT);
/* Build 'DEST = 0' and insert. */
@@ -815,7 +815,6 @@ lower_builtin_setjmp (gimple_stmt_iterat
{
g = gimple_build_assign (dest, build_zero_cst (TREE_TYPE (dest)));
gimple_set_location (g, loc);
- gimple_set_block (g, gimple_block (stmt));
gsi_insert_before (gsi, g, GSI_SAME_STMT);
}
@@ -832,7 +831,6 @@ lower_builtin_setjmp (gimple_stmt_iterat
t = builtin_decl_implicit (BUILT_IN_SETJMP_RECEIVER);
g = gimple_build_call (t, 1, arg);
gimple_set_location (g, loc);
- gimple_set_block (g, gimple_block (stmt));
gsi_insert_before (gsi, g, GSI_SAME_STMT);
/* Build 'DEST = 1' and insert. */
@@ -841,7 +839,6 @@ lower_builtin_setjmp (gimple_stmt_iterat
g = gimple_build_assign (dest, fold_convert_loc (loc, TREE_TYPE (dest),
integer_one_node));
gimple_set_location (g, loc);
- gimple_set_block (g, gimple_block (stmt));
g
Re: [PATCH] Avoid redundant BLOCK lookup/set
On Tue, Mar 26, 2019 at 02:34:40PM +0100, Richard Biener wrote: > Just found this in one of my dev trees and think it's suitable at > this stage as a general speedup. > > gimple_block involves querying an on-the-side map in libcpp > since it is encoded in the location_t location. This means > caching is beneficial. It's also pointless to copy > gimple_block once you've copied gimple_location. The removals of gimple_set_block means we can't at some point change our minds easily and put block somewhere else, but guess that we'd need to go through all spots that copy location now anyway. So LGTM. Jakub
[v3 PATCH] PR libstdc++/89825
2019-03-26 Ville Voutilainen
PR libstdc++/89825
Fix based on a suggestion by Antony Polukhin.
* include/std/variant (_Extra_visit_slot_needed): New.
(_Multi_array): Use it.
(_S_apply_all_alts): Likewise.
diff --git a/libstdc++-v3/include/std/variant b/libstdc++-v3/include/std/variant
index 3631463..27ea516 100644
--- a/libstdc++-v3/include/std/variant
+++ b/libstdc++-v3/include/std/variant
@@ -747,6 +747,20 @@ namespace __variant
void* __get_storage(_Variant&& __v)
{ return __v._M_storage(); }
+ template
+ struct _Extra_visit_slot_needed
+ {
+template struct _Variant_never_valueless;
+
+template
+struct _Variant_never_valueless>
+ : bool_constant<(is_trivially_copyable_v<_Types>&&...)> {};
+
+static constexpr bool value =
+ is_same_v<_Maybe_variant_cookie, __variant_cookie>
+ && !_Variant_never_valueless<__remove_cvref_t<_Variant>>::value;
+ };
+
// Used for storing multi-dimensional vtable.
template
struct _Multi_array
@@ -764,8 +778,11 @@ namespace __variant
size_t __first, size_t... __rest>
struct _Multi_array<_Ret(*)(_Visitor, _Variants...), __first, __rest...>
{
+ static constexpr size_t __index =
+ sizeof...(_Variants) - sizeof...(__rest) - 1;
+ using _Variant = typename _Nth_type<__index, _Variants...>::type;
static constexpr int __do_cookie =
- is_same_v<_Ret, __variant_cookie> ? 1 : 0;
+ _Extra_visit_slot_needed<_Ret, _Variant>::value ? 1 : 0;
using _Tp = _Ret(*)(_Visitor, _Variants...);
template
constexpr const _Tp&
@@ -832,7 +849,7 @@ namespace __variant
_S_apply_all_alts(_Array_type& __vtable,
std::index_sequence<__var_indices...>)
{
- if constexpr (is_same_v<_Result_type, __variant_cookie>)
+ if constexpr (_Extra_visit_slot_needed<_Result_type, _Next>::value)
(_S_apply_single_alt(
__vtable._M_arr[__var_indices + 1],
&(__vtable._M_arr[0])), ...);
[PATCH] RX: Add rx-*-linux target
I ported linux kernel to Renesas RX.
rx-*-elf target output a binary different from the standard ELF.
It has the same format as the Renesas compiler.
But the linux kernel requires the standard ELF format.
I want to define a rx-*-linux target so that I can generate
a standard ELF binary.
diff --git a/gcc/config.gcc b/gcc/config.gcc
index 3eb2e800fc5..2fef8922578 100644
--- a/gcc/config.gcc
+++ b/gcc/config.gcc
@@ -2800,6 +2800,11 @@ rl78-*-elf*)
rx-*-elf*)
tm_file="dbxelf.h elfos.h newlib-stdint.h ${tm_file}"
tmake_file="${tmake_file} rx/t-rx"
+ extra_options="${extra_options} rx/elf.opt"
+ ;;
+rx-*-linux*)
+ tm_file="elfos.h linux.h glibc-stdint.h rx/linux.h
../../libgcc/config/rx/rx-abi.h"
+ tmake_file="${tmake_file} rx/t-linux"
;;
s390-*-linux*)
tm_file="s390/s390.h dbxelf.h elfos.h gnu-user.h linux.h glibc-stdint.h
s390/linux.h"
diff --git a/gcc/config/rx/elf.opt b/gcc/config/rx/elf.opt
new file mode 100644
index 000..907e030df8e
--- /dev/null
+++ b/gcc/config/rx/elf.opt
@@ -0,0 +1,44 @@
+; Command line options for the Renesas RX port of GCC.
+; Copyright (C) 2008, 2009, 2010 Free Software Foundation, Inc.
+; Contributed by Red Hat.
+;
+; This file is part of GCC.
+;
+; GCC is free software; you can redistribute it and/or modify it under
+; the terms of the GNU General Public License as published by the Free
+; Software Foundation; either version 3, or (at your option) any later
+; version.
+;
+; GCC is distributed in the hope that it will be useful, but WITHOUT ANY
+; WARRANTY; without even the implied warranty of MERCHANTABILITY or
+; FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
+; for more details.
+;
+; You should have received a copy of the GNU General Public License
+; along with GCC; see the file COPYING3. If not see
+;
[PATCH, d] Committed merge with upstream dmd
Hi,
This patch merges the D front-end implementation with dmd upstream ab702e73e.
Backports memory leak fix in the mangler, and introduces recognition
and rejection of more C types and directives.
Bootstrapped and regression tested on x86_64-linux-gnu.
Committed to trunk as r269945.
--
Iain
---
diff --git a/gcc/d/dmd/MERGE b/gcc/d/dmd/MERGE
index 3017f0d34af..ffad6cb524d 100644
--- a/gcc/d/dmd/MERGE
+++ b/gcc/d/dmd/MERGE
@@ -1,4 +1,4 @@
-74ac873be1862090b7ec0e4a876fd1b758520359
+ab702e73e56aefb3b77b8f8f42da94bc22143eeb
The first line of this file holds the git revision number of the last
merge done from the dlang/dmd repository.
diff --git a/gcc/d/dmd/dmangle.c b/gcc/d/dmd/dmangle.c
index 7f13947ae2b..44f4f826b41 100644
--- a/gcc/d/dmd/dmangle.c
+++ b/gcc/d/dmd/dmangle.c
@@ -306,8 +306,9 @@ public:
buf2.reserve(32);
Mangler v(&buf2);
v.paramsToDecoBuffer(t->arguments);
+const char *s = buf2.peekString();
int len = (int)buf2.offset;
-buf->printf("%d%.*s", len, len, buf2.extractData());
+buf->printf("%d%.*s", len, len, s);
}
void visit(TypeNull *t)
diff --git a/gcc/d/dmd/dscope.c b/gcc/d/dmd/dscope.c
index 27c3fd58947..def448a2408 100644
--- a/gcc/d/dmd/dscope.c
+++ b/gcc/d/dmd/dscope.c
@@ -685,14 +685,16 @@ Dsymbol *Scope::search_correct(Identifier *ident)
const char *Scope::search_correct_C(Identifier *ident)
{
TOK tok;
-if (ident == Id::_NULL)
+if (ident == Id::C_NULL)
tok = TOKnull;
-else if (ident == Id::_TRUE)
+else if (ident == Id::C_TRUE)
tok = TOKtrue;
-else if (ident == Id::_FALSE)
+else if (ident == Id::C_FALSE)
tok = TOKfalse;
-else if (ident == Id::_unsigned)
+else if (ident == Id::C_unsigned)
tok = TOKuns32;
+else if (ident == Id::C_wchar_t)
+tok = global.params.isWindows ? TOKwchar : TOKdchar;
else
return NULL;
return Token::toChars(tok);
diff --git a/gcc/d/dmd/idgen.c b/gcc/d/dmd/idgen.c
index ec26b2c7008..e75004893aa 100644
--- a/gcc/d/dmd/idgen.c
+++ b/gcc/d/dmd/idgen.c
@@ -374,10 +374,11 @@ Msgtable msgtable[] =
{ "udaSelector", "selector" },
// C names, for undefined identifier error messages
-{ "_NULL", "NULL" },
-{ "_TRUE", "TRUE" },
-{ "_FALSE", "FALSE" },
-{ "_unsigned", "unsigned" },
+{ "C_NULL", "NULL" },
+{ "C_TRUE", "TRUE" },
+{ "C_FALSE", "FALSE" },
+{ "C_unsigned", "unsigned" },
+{ "C_wchar_t", "wchar_t" },
};
diff --git a/gcc/d/dmd/lexer.c b/gcc/d/dmd/lexer.c
index b466f17e4be..8a2c90f1dfd 100644
--- a/gcc/d/dmd/lexer.c
+++ b/gcc/d/dmd/lexer.c
@@ -901,16 +901,25 @@ void Lexer::scan(Token *t)
p++;
Token n;
scan(&n);
-if (n.value == TOKidentifier && n.ident == Id::line)
+if (n.value == TOKidentifier)
{
-poundLine();
-continue;
+ if (n.ident == Id::line)
+ {
+ poundLine();
+ continue;
+ }
+ else
+ {
+ const Loc locx = loc();
+ warning(locx, "C preprocessor directive `#%s` is not supported", n.ident->toChars());
+ }
}
-else
+else if (n.value == TOKif)
{
-t->value = TOKpound;
-return;
+error("C preprocessor directive `#if` is not supported, use `version` or `static if`");
}
+t->value = TOKpound;
+return;
}
default:
diff --git a/gcc/d/dmd/parse.c b/gcc/d/dmd/parse.c
index e0ee299eb6d..3afdbc257f8 100644
--- a/gcc/d/dmd/parse.c
+++ b/gcc/d/dmd/parse.c
@@ -3076,7 +3076,23 @@ Type *Parser::parseBasicType(bool dontLookDotIdents)
case TOKuns16: t = Type::tuns16; goto LabelX;
case TOKint32: t = Type::tint32; goto LabelX;
case TOKuns32: t = Type::tuns32; goto LabelX;
-case TOKint64: t = Type::tint64; goto LabelX;
+case TOKint64:
+t = Type::tint64;
+nextToken();
+if (token.value == TOKint64)// if `long long`
+{
+error("use `long` for a 64 bit integer instead of `long long`");
+nextToken();
+}
+else if (token.value == TOKfloat64) // if `long double`
+{
+error("use `real` instead of `long double`");
+t = Type::tfloat80;
+nextToken();
+
+}
+break;
+
case TOKuns64: t = Type::tuns64; goto LabelX;
case TOKint128: t = Type::tint128; goto LabelX;
case TOKuns128: t = Type::tuns128; goto LabelX;
diff --git a/gcc/testsuite/gdc.test/fail_compilation/cerrors.d b/gcc/test
Re: [C++] compiler incompatibility with lambdas
Hi Nathan,
> This patch addresses a compiler incompatibility with non-capturing lambdas.
> Specifically, when a lambda's functions are comdat, we place the static
> _FUN function in the same comdat group as the operator() function.
>
> This breaks link compatibility with clang, where the static function is
> named __invoker and not placed in the same group. The case we hit was a
> link failure when the clang-generated comdat group was chosen and the _FUN
> symbol dropped.
>
> I'm not sure of the rationale of placing them in the same group. The
> inliner would still be correct to inline the operator() it can see into the
> _FUN body, as all comdat groups are supposed to be equivalent, so even if a
> clang-generated operator() ends up in the executable, that's fine to
> interoperate with a gcc-generated _FUN. (we did initially think this was
> an LTO bug when it applied identical code folding to the two functions, as
> they are indeed identical)
>
> inter-compiler interoperation of capturing lambdas is a different issue --
> not sure if the ABI mandates layout. But that's not relevant in this case
> as the lambda is an empty object, and that's the only case we can generate
> a static _FUN executor.
>
> Jason, any objections for trunk?
the new test FAILs on Solaris when the native assembler is in use, which
uses a different comdat group syntax:
+FAIL: g++.dg/abi/lambda-static-1.C -std=c++14 scan-assembler .section[\\t
]*.text._ZZ5lambyvENKUlvE_clEv,[^\\n\\r]*,_ZZ5lambyvENKUlvE_clEv,comdat
+FAIL: g++.dg/abi/lambda-static-1.C -std=c++14 scan-assembler .section[\\t
]*.text._ZZ5lambyvENUlvE_4_FUNEv,[^\\n\\r]*,_ZZ5lambyvENUlvE_4_FUNEv,comdat
+FAIL: g++.dg/abi/lambda-static-1.C -std=c++17 scan-assembler .section[\\t
]*.text._ZZ5lambyvENKUlvE_clEv,[^\\n\\r]*,_ZZ5lambyvENKUlvE_clEv,comdat
+FAIL: g++.dg/abi/lambda-static-1.C -std=c++17 scan-assembler .section[\\t
]*.text._ZZ5lambyvENUlvE_4_FUNEv,[^\\n\\r]*,_ZZ5lambyvENUlvE_4_FUNEv,comdat
While on i386 with gas I have
.section
.text._ZZ5lambyvENKUlvE_clEv,"axG",@progbits,_ZZ5lambyvENKUlvE_clEv,comdat
i386 with as emits
.section
.text._ZZ5lambyvENKUlvE_clEv%_ZZ5lambyvENKUlvE_clEv,"ax",@progbits
.group
_ZZ5lambyvENKUlvE_clEv,.text._ZZ5lambyvENKUlvE_clEv%_ZZ5lambyvENKUlvE_clEv,#comdat
and sparc with as is again slightly different:
.section
".text._ZZ5lambyvENKUlvE_clEv%_ZZ5lambyvENKUlvE_clEv",#alloc,#execinstr,#progbits
.group
_ZZ5lambyvENKUlvE_clEv,".text._ZZ5lambyvENKUlvE_clEv%_ZZ5lambyvENKUlvE_clEv",#comdat
The following patch allows for all three variants, only looking at the
.group line with as since that's enough to check for different group
names.
Tested on i386-pc-solaris2.11 with gas and as and
sparc-sun-solaris2.11. Ok for mainline?
Rainer
--
-
Rainer Orth, Center for Biotechnology, Bielefeld University
2019-03-26 Rainer Orth
* g++.dg/abi/lambda-static-1.C: Handle Solaris as comdat group
syntax.
# HG changeset patch
# Parent ddd55451e9cb2406fe2855aef46c6695b2c7b542
Fix g++.dg/abi/lambda-static-1.C with Solaris as
diff --git a/gcc/testsuite/g++.dg/abi/lambda-static-1.C b/gcc/testsuite/g++.dg/abi/lambda-static-1.C
--- a/gcc/testsuite/g++.dg/abi/lambda-static-1.C
+++ b/gcc/testsuite/g++.dg/abi/lambda-static-1.C
@@ -21,5 +21,7 @@ void indirect ()
// The call operator and the static invoker should be comdat, but not
// the same group. (that would be a compiler incompatibility)
-// { dg-final { scan-assembler ".section\[\t ]*.text._ZZ5lambyvENKUlvE_clEv,\[^\n\r]*,_ZZ5lambyvENKUlvE_clEv,comdat" } }
-// { dg-final { scan-assembler ".section\[\t ]*.text._ZZ5lambyvENUlvE_4_FUNEv,\[^\n\r]*,_ZZ5lambyvENUlvE_4_FUNEv,comdat" } }
+// { dg-final { scan-assembler ".section\[\t ]*.text._ZZ5lambyvENKUlvE_clEv,\[^\n\r]*,_ZZ5lambyvENKUlvE_clEv,comdat" { target { { ! *-*-solaris2.* } || { gas } } } } }
+// { dg-final { scan-assembler ".section\[\t ]*.text._ZZ5lambyvENUlvE_4_FUNEv,\[^\n\r]*,_ZZ5lambyvENUlvE_4_FUNEv,comdat" { target { { ! *-*-solaris2.* } || { gas } } } } }
+// { dg-final { scan-assembler ".group\[\t \]*_ZZ5lambyvENKUlvE_clEv,\[^\n\r\]*,#comdat" { target { *-*-solaris2.* && { ! gas } } } } }
+// { dg-final { scan-assembler ".group\[\t \]*_ZZ5lambyvENUlvE_4_FUNEv,\[^\n\r\]*,#comdat" { target { *-*-solaris2.* && { ! gas } } } } }
Re: One more patch for PR89676
On 3/26/19 8:35 AM, Maxim Kuvyrkov wrote:
I don't follow. Do you mean that in the below testcase it's not guaranteed
that casp will get its first two arguments in x0 and x1? (If so, why?)
Sorry for not to be clear. With my first patch only, it was not
guaranteed for some complicated code cases. With the additional patch
it is guaranteed as it was before.
===
void *a;
long b, c;
void d(void) {
typeof(0) e=0;
register long x0 asm ("x0") = b;
register long x1 asm ("x1") = c;
asm(" casp\t%[old1], %[old2], %[new1], %[new2], %[v]\n"
: [old1] "+&r"(b), [old2] "+&r"(c), [v] "+Q"(a)
: [new1] "r"(d), [new2] "r"(e));
}
Re: [v3 PATCH] PR libstdc++/89825
Slight tweak, the condition for _M_valid and _Extra_visit_slot_needed
is now in sync.
2019-03-26 Ville Voutilainen
PR libstdc++/89825
Fix based on a suggestion by Antony Polukhin.
* include/std/variant (__never_valueless): New.
(_M_valid): Use it.
(_Extra_visit_slot_needed): New.
(_Multi_array): Use it.
(_S_apply_all_alts): Likewise.
diff --git a/libstdc++-v3/include/std/variant b/libstdc++-v3/include/std/variant
index 3631463..3932a8a 100644
--- a/libstdc++-v3/include/std/variant
+++ b/libstdc++-v3/include/std/variant
@@ -323,6 +323,12 @@ namespace __variant
_Variadic_union<_Rest...> _M_rest;
};
+ template
+ constexpr bool __never_valueless()
+ {
+return (is_trivially_copyable_v<_Types> && ...);
+ }
+
// Defines index and the dtor, possibly trivial.
template
struct _Variant_storage;
@@ -408,7 +414,7 @@ namespace __variant
constexpr bool
_M_valid() const noexcept
{
- if constexpr ((is_trivially_copyable_v<_Types> && ...))
+ if constexpr (__never_valueless<_Types...>())
return true;
return this->_M_index != __index_type(variant_npos);
}
@@ -747,6 +753,20 @@ namespace __variant
void* __get_storage(_Variant&& __v)
{ return __v._M_storage(); }
+ template
+ struct _Extra_visit_slot_needed
+ {
+template struct _Variant_never_valueless;
+
+template
+struct _Variant_never_valueless>
+ : bool_constant<__never_valueless<_Types...>()> {};
+
+static constexpr bool value =
+ is_same_v<_Maybe_variant_cookie, __variant_cookie>
+ && !_Variant_never_valueless<__remove_cvref_t<_Variant>>::value;
+ };
+
// Used for storing multi-dimensional vtable.
template
struct _Multi_array
@@ -764,8 +784,11 @@ namespace __variant
size_t __first, size_t... __rest>
struct _Multi_array<_Ret(*)(_Visitor, _Variants...), __first, __rest...>
{
+ static constexpr size_t __index =
+ sizeof...(_Variants) - sizeof...(__rest) - 1;
+ using _Variant = typename _Nth_type<__index, _Variants...>::type;
static constexpr int __do_cookie =
- is_same_v<_Ret, __variant_cookie> ? 1 : 0;
+ _Extra_visit_slot_needed<_Ret, _Variant>::value ? 1 : 0;
using _Tp = _Ret(*)(_Visitor, _Variants...);
template
constexpr const _Tp&
@@ -832,7 +855,7 @@ namespace __variant
_S_apply_all_alts(_Array_type& __vtable,
std::index_sequence<__var_indices...>)
{
- if constexpr (is_same_v<_Result_type, __variant_cookie>)
+ if constexpr (_Extra_visit_slot_needed<_Result_type, _Next>::value)
(_S_apply_single_alt(
__vtable._M_arr[__var_indices + 1],
&(__vtable._M_arr[0])), ...);
Re: [v3 PATCH] PR libstdc++/89825
On 26/03/19 16:48 +0200, Ville Voutilainen wrote: Slight tweak, the condition for _M_valid and _Extra_visit_slot_needed is now in sync. OK, thanks. PR libstdc++/89825 Fix based on a suggestion by Antony Polukhin. * include/std/variant (__never_valueless): New. (_M_valid): Use it. (_Extra_visit_slot_needed): New. (_Multi_array): Use it. (_S_apply_all_alts): Likewise.
Re: C++ PATCH for c++/89214 - ICE when initializing aggregates with bases
On Tue, Mar 26, 2019 at 12:34:03PM +0100, Andreas Schwab wrote:
> I don't see any of the warnings in the tests on ia64.
>
> FAIL: g++.dg/cpp1z/aggr-base8.C -std=c++17 (test for warnings, line 33)
> FAIL: g++.dg/cpp1z/aggr-base8.C -std=c++17 (test for warnings, line 34)
> FAIL: g++.dg/cpp1z/aggr-base8.C -std=c++17 (test for warnings, line 35)
> FAIL: g++.dg/cpp1z/aggr-base8.C -std=c++17 (test for warnings, line 45)
> FAIL: g++.dg/cpp1z/aggr-base8.C -std=c++17 (test for warnings, line 46)
> FAIL: g++.dg/cpp1z/aggr-base8.C -std=c++17 (test for warnings, line 47)
> PASS: g++.dg/cpp1z/aggr-base8.C -std=c++17 (test for excess errors)
>
> FAIL: g++.dg/cpp1z/aggr-base9.C -std=c++17 (test for warnings, line 22)
> FAIL: g++.dg/cpp1z/aggr-base9.C -std=c++17 (test for warnings, line 23)
> FAIL: g++.dg/cpp1z/aggr-base9.C -std=c++17 (test for warnings, line 31)
> FAIL: g++.dg/cpp1z/aggr-base9.C -std=c++17 (test for warnings, line 32)
> PASS: g++.dg/cpp1z/aggr-base9.C -std=c++17 (test for excess errors)
I don't have access to an ia64 box -- I see none on Compile Farm. But if
it doesn't ICE, can we just add ! { target ia64-*-* } to the dg-warnings?
Marek
[PATCH] PR libstdc++/85965 delay static assertions until types are complete
The static assertions added for PR libstdc++/48101 were at class scope and so were evaluated too eagerly, when it might not be possible to determine whether the function objects are invocable with the key types. The problematic cases are where the key type is not known to be convertible to the argument type(s) of the function object until later, after a type has been completed. Specifically, if the key type is a pointer to a derived class and the function object's argument type is a pointer to a base class, then the derived-to-base conversion is only valid once the derived type is complete. By moving the static assertions to the destructor they will only be evaluated when the destructor is instantiated, at which point whether the key type can be passed to the function object should be knowable. The ideal place to do the checks would be only when the function objects are actually invoked, but that would mean adding the checks in numerous places, so the destructor is used instead. The tests need to be adjusted because the "required from here" line is now the location of the destructor, not the point of instantiation in the test file. For the map and multimap tests which check two specializations, the dg-error matching the assertion text matches both cases. Also check the diagnostic output for the template arguments, to ensure both specializations trigger the assertion. PR libstdc++/85965 * include/bits/hashtable.h (_Hashtable): Move static assertions to destructor so they are not evaluated until the _Key type is complete. * include/bits/stl_tree.h (_Rb_tree): Likewise. * testsuite/23_containers/set/85965.cc: New test. * testsuite/23_containers/unordered_set/85965.cc: New test. * testsuite/23_containers/map/48101_neg.cc: Replace "here" errors with regexp matching the corresponding _Rb_tree specialization. * testsuite/23_containers/multimap/48101_neg.cc: Likewise. * testsuite/23_containers/multiset/48101_neg.cc: Remove "here" error. * testsuite/23_containers/set/48101_neg.cc: Likewise. * testsuite/23_containers/unordered_map/48101_neg.cc: Likewise. * testsuite/23_containers/unordered_multimap/48101_neg.cc: Likewise. * testsuite/23_containers/unordered_multiset/48101_neg.cc: Likewise. * testsuite/23_containers/unordered_set/48101_neg.cc: Likewise. Tested powerpc64le-linux, committed to trunk. Backport for gcc-8-branch to follow. commit 47de5e5b3f7eedd2306872a6fe2f5d3935872fcb Author: Jonathan Wakely Date: Sat Mar 23 00:14:29 2019 + PR libstdc++/85965 delay static assertions until types are complete The static assertions added for PR libstdc++/48101 were at class scope and so were evaluated too eagerly, when it might not be possible to determine whether the function objects are invocable with the key types. The problematic cases are where the key type is not known to be convertible to the argument type(s) of the function object until later, after a type has been completed. Specifically, if the key type is a pointer to a derived class and the function object's argument type is a pointer to a base class, then the derived-to-base conversion is only valid once the derived type is complete. By moving the static assertions to the destructor they will only be evaluated when the destructor is instantiated, at which point whether the key type can be passed to the function object should be knowable. The ideal place to do the checks would be only when the function objects are actually invoked, but that would mean adding the checks in numerous places, so the destructor is used instead. The tests need to be adjusted because the "required from here" line is now the location of the destructor, not the point of instantiation in the test file. For the map and multimap tests which check two specializations, the dg-error matching the assertion text matches both cases. Also check the diagnostic output for the template arguments, to ensure both specializations trigger the assertion. PR libstdc++/85965 * include/bits/hashtable.h (_Hashtable): Move static assertions to destructor so they are not evaluated until the _Key type is complete. * include/bits/stl_tree.h (_Rb_tree): Likewise. * testsuite/23_containers/set/85965.cc: New test. * testsuite/23_containers/unordered_set/85965.cc: New test. * testsuite/23_containers/map/48101_neg.cc: Replace "here" errors with regexp matching the corresponding _Rb_tree specialization. * testsuite/23_containers/multimap/48101_neg.cc: Likewise. * testsuite/23_containers/multiset/48101_neg.cc: Remove "here" error. * testsuite/23_containers/set/48101_neg.cc: Likewise. * testsuite/23_containers/unordered_map/48101_neg.cc:
Re: [PATCH] correct maximum valid alignment in error message (PR 89812)
On Mon, Mar 25, 2019 at 11:41:35AM -0600, Jeff Law wrote:
> > PR c/89812 - incorrect maximum in error: requested alignment
> > ‘536870912’ exceeds maximum 2147483648
> >
> > gcc/c-family/ChangeLog:
> >
> > PR c/89812
> > * c-common.c (check_user_alignment): Rename local. Correct maximum
> > alignment in diagnostic. Avoid assuming argument fits in SHWI,
> > convert it to UHWI when it fits.
> >
> > gcc/testsuite/ChangeLog:
> >
> > PR c/89812
> > * gcc.dg/attr-aligned-3.c: New test.
> OK
The test FAILs on all 32-bit targets (where __UINT64_TYPE__ is unsigned long
long) due to -pedantic-errors, and I bet will fail on all non-ELF targets on
AVR, because only config/elfos.h defines 1 << 28 as MAX_OFILE_ALIGNMENT (in
bytes) and the test relies on exactly that value, nothing else.
Fixed thusly, tested on x86_64-linux (-m32/-m64), ok for trunk?
If we have some elf targets that still don't use elfos.h, we might need to
add them next to avr too.
2019-03-26 Jakub Jelinek
PR c/89812
* gcc.dg/attr-aligned-3.c: Limit the test to known ELF targets
other than AVR. Add dg-options "".
--- gcc/testsuite/gcc.dg/attr-aligned-3.c.jj2019-03-26 08:52:54.510611778
+0100
+++ gcc/testsuite/gcc.dg/attr-aligned-3.c 2019-03-26 16:25:44.518085453
+0100
@@ -1,7 +1,10 @@
/* PR c/89812 - incorrect maximum in error: requested alignment '536870912'
exceeds maximum 2147483648
- { dg-do compile }
- { dg-require-effective-target size32plus } */
+ Limit to ELF targets that are known to use MAX_OFILE_ALIGNMENT
+ (1 << 28) * BITS_PER_UNIT.
+ { dg-do compile { target { { *-*-elf* *-*-gnu* } && { ! avr*-*-* } } } }
+ { dg-require-effective-target size32plus }
+ { dg-options "" } */
#define POWALIGN(N) __attribute__ ((aligned ((__UINT64_TYPE__)1 << (N
Jakub
Re: [C++] compiler incompatibility with lambdas
On 3/26/19 10:45 AM, Rainer Orth wrote: Hi Nathan, This patch addresses a compiler incompatibility with non-capturing lambdas. Specifically, when a lambda's functions are comdat, we place the static _FUN function in the same comdat group as the operator() function. the new test FAILs on Solaris when the native assembler is in use, which uses a different comdat group syntax: While on i386 with gas I have i386 with as emits and sparc with as is again slightly different: The following patch allows for all three variants, only looking at the .group line with as since that's enough to check for different group names. Yes, thanks. nathan -- Nathan Sidwell
[C++ PATCH] PR c++/86429 - constexpr variable in lambda.
When we refer to a captured variable from a constant-expression context
inside a lambda, the closure (like any function parameter) is not constant
because we aren't in a call, so we don't have an argument. So the capture
is non-constant. But if the captured variable is constant, we might be able
to use it directly in constexpr evaluation.
Tested x86_64-pc-linux-gnu, applying to trunk.
PR c++/82643
PR c++/87327
* constexpr.c (cxx_eval_constant_expression): In a lambda function,
try evaluating the captured variable directly.
---
gcc/cp/constexpr.c| 25 +--
.../g++.dg/cpp1y/lambda-generic-const10.C | 24 ++
.../g++.dg/cpp1y/lambda-generic-const9.C | 16
.../g++.dg/cpp1z/constexpr-lambda24.C | 23 +
gcc/cp/ChangeLog | 8 ++
5 files changed, 94 insertions(+), 2 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/cpp1y/lambda-generic-const10.C
create mode 100644 gcc/testsuite/g++.dg/cpp1y/lambda-generic-const9.C
create mode 100644 gcc/testsuite/g++.dg/cpp1z/constexpr-lambda24.C
diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c
index e92ec55317b..c00d642fcfe 100644
--- a/gcc/cp/constexpr.c
+++ b/gcc/cp/constexpr.c
@@ -4442,8 +4442,29 @@ cxx_eval_constant_expression (const constexpr_ctx *ctx,
tree t,
case VAR_DECL:
if (DECL_HAS_VALUE_EXPR_P (t))
- return cxx_eval_constant_expression (ctx, DECL_VALUE_EXPR (t),
-lval, non_constant_p, overflow_p);
+ {
+ if (is_normal_capture_proxy (t)
+ && current_function_decl == DECL_CONTEXT (t))
+ {
+ /* Function parms aren't constexpr within the function
+definition, so don't try to look at the closure. But if the
+captured variable is constant, try to evaluate it directly. */
+ r = DECL_CAPTURED_VARIABLE (t);
+ tree type = TREE_TYPE (t);
+ if (TYPE_REF_P (type) != TYPE_REF_P (TREE_TYPE (r)))
+ {
+ /* Adjust r to match the reference-ness of t. */
+ if (TYPE_REF_P (type))
+ r = build_address (r);
+ else
+ r = convert_from_reference (r);
+ }
+ }
+ else
+ r = DECL_VALUE_EXPR (t);
+ return cxx_eval_constant_expression (ctx, r, lval, non_constant_p,
+ overflow_p);
+ }
/* fall through */
case CONST_DECL:
/* We used to not check lval for CONST_DECL, but darwin.c uses
diff --git a/gcc/testsuite/g++.dg/cpp1y/lambda-generic-const10.C
b/gcc/testsuite/g++.dg/cpp1y/lambda-generic-const10.C
new file mode 100644
index 000..e0080b3d4f6
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp1y/lambda-generic-const10.C
@@ -0,0 +1,24 @@
+// PR c++/82643
+// { dg-do compile { target c++14 } }
+
+int main()
+{
+ struct A {
+constexpr int operator()() const { return 42; }
+ };
+
+ auto f = A();
+ constexpr auto x = f(); //ok, call constexpr const non-static method
+
+ [](auto const &f) {
+constexpr auto x = f(); /*ok*/
+ }(f);
+
+ [&]() {
+constexpr auto x = f(); //ko, __closure is not a constant expression
+ };
+
+ [=]() {
+constexpr auto x = f(); //same ko, __closure is not a constant expression
+ };
+}
diff --git a/gcc/testsuite/g++.dg/cpp1y/lambda-generic-const9.C
b/gcc/testsuite/g++.dg/cpp1y/lambda-generic-const9.C
new file mode 100644
index 000..491c7c322c3
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp1y/lambda-generic-const9.C
@@ -0,0 +1,16 @@
+// PR c++/86429
+// { dg-do compile { target c++14 } }
+
+struct A
+{
+ int i;
+ constexpr int f(const int&) const { return i; }
+};
+
+void g()
+{
+ constexpr A a = { 42 };
+ [&](auto x) {
+constexpr auto y = a.f(x);
+ }(24);
+}
diff --git a/gcc/testsuite/g++.dg/cpp1z/constexpr-lambda24.C
b/gcc/testsuite/g++.dg/cpp1z/constexpr-lambda24.C
new file mode 100644
index 000..2edb24e41ac
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp1z/constexpr-lambda24.C
@@ -0,0 +1,23 @@
+// PR c++/87327
+// { dg-do compile { target c++17 } }
+
+template
+struct Foo {
+constexpr auto size() const {
+return N;
+}
+};
+
+constexpr int foo() {
+constexpr auto a = Foo<5>{};
+
+[&] {
+Foo it = {};
+
+return it;
+}();
+
+return 42;
+}
+
+constexpr int i = foo();
diff --git a/gcc/cp/ChangeLog b/gcc/cp/ChangeLog
index 80d4ae3f1b9..550b7541d9f 100644
--- a/gcc/cp/ChangeLog
+++ b/gcc/cp/ChangeLog
@@ -1,3 +1,11 @@
+2019-03-26 Jason Merrill
+
+ PR c++/86429 - constexpr variable in lambda.
+ PR c++/82643
+ PR c++/87327
+ * constexpr.c (cxx_eval_constant_expression): In a lambda function,
+ try evaluating the captured variable directly.
+
2019-03-26 Jakub Jelinek
PR c++/89
Re: [PATCH][Tree-optimization/PR89730]grant always_inline when -flive-patching=inline-only-static
Hi, Richard, thanks for the suggestion. I tried it yesterday, but it did not work. the reason is: inside “can_inline_edge_by_limits_p”, the “allowance for always_inline” is guarded by the following condition: else if “caller_tree != callee_tree” this condition is ONLY true when when callee has different optimization level than the caller. So, I think that the correct spot for the checking of live-patching=inline-only-static should still be inside “can_inline_edge_p”. so my previous patch for this bug should be fine. what’s your opinion? thanks. Qing > On Mar 25, 2019, at 7:23 AM, Richard Biener > wrote: > > On Wed, Mar 20, 2019 at 4:16 PM Qing Zhao wrote: >> >> Hi, >> >> there is a bug in the current support for -flive-patching=inline-only-static: >> >> it rejects inlining of external always_inline routine, therefore triggers a >> compilation time error. >> >> we should always inline “always_inline” routines. >> >> please review the following simple patch, it has been bootstrapped and >> regression tested on aarch64. >> >> Okay for committing? > > To me this seems like can_inline_edge_p was the wrong spot to disable inlining > for -flive-patching=inline-only-static and instead it should have been in > can_inline_edge_by_limits_p which already has proper allowance for > always-inline. > > So can you move the check there, like after > > /* gcc.dg/pr43564.c. Apply user-forced inline even at -O0. */ > else if (always_inline) >; > > ? > >> thanks. >> >> Qing. >> >> gcc/ChangeLog: >> >> 2019-03-20 qing zhao >> >>PR tree-optimization/89730 >> * ipa-inline.c (can_inline_edge_p): Grant always_inline even when >> -flive-patching=inline-only-static. >> >> gcc/testsuite/ChangeLog: >> >> 2019-03-20 qing zhao >> >>* gcc.dg/live-patching-4.c: New test. >>
Re: [PATCH] Update assertion in sched-ebb.c to cope with table jumps
On Mon, Mar 25, 2019 at 05:12:05PM -0600, Jeff Law wrote: > To touch on one of the issues I raised. AFAICT the schedulers don't use > SCHED_GROUP_P for dealing with tablejumps. They're used for > cc0-user/setter, fused insns and the like. That's a bit of a surprise. > > Given that the table isn't actually associated with the block with the > tablejump, SCHED_GROUP_P might actually create a whole new set of problems. > > Why oh why is the jump table data not actually attached to the tablejump > insn itself. Nearly 30 years of working on GCC and I can't answer that one. It somewhat makes sense for targets where the jump table is emitted in the text section; we then need to know its size for jumps over it, etc. Segher
Re: [RS6000] Don't rely on rs6000_hard_regno_mode_ok being zero
On Tue, Mar 26, 2019 at 03:40:05PM +1030, Alan Modra wrote: > rs6000_hard_regno_mode_ok_uncached result depends on target flags, > and nowadays it is possible that TARGET_OPTION_OVERRIDE be called not > just at the start of compilation but per-function by the action of > function attributes or "#pragma GCC target". At the start of > compilation rs6000_hard_regno_mode_ok_p[][] is a clean slate, but > not so later on. > > Bootstrapped etc. powerpc64le-linux. If gcc wasn't in stage 4 I'd > apply this as obvious, but I doubt it is a regression, and the patch > is just a tiny bit more than a doc fix. So, OK now or wait for > stage 1? Now please. Thanks! Segher > * config/rs6000/rs6000.c (rs6000_init_hard_regno_mode_ok): Always > assign rs6000_hard_regno_mode_ok_p[m][r]. Formatting.
Re: [RS6000] Rename NON_SPECIAL_REGS to GEN_OR_FLOAT_REGS
On Tue, Mar 26, 2019 at 04:14:07PM +1030, Alan Modra wrote: > This just chooses a more descriptive name for this register class. > Bootstrapped etc. powerpc64le-linux. OK for next stage1? It's okay now, even. Thanks! Segher
Re: [PATCH] Update assertion in sched-ebb.c to cope with table jumps
On 3/26/19 11:52 AM, Segher Boessenkool wrote: > On Mon, Mar 25, 2019 at 05:12:05PM -0600, Jeff Law wrote: >> To touch on one of the issues I raised. AFAICT the schedulers don't use >> SCHED_GROUP_P for dealing with tablejumps. They're used for >> cc0-user/setter, fused insns and the like. That's a bit of a surprise. >> >> Given that the table isn't actually associated with the block with the >> tablejump, SCHED_GROUP_P might actually create a whole new set of problems. >> >> Why oh why is the jump table data not actually attached to the tablejump >> insn itself. Nearly 30 years of working on GCC and I can't answer that one. > > It somewhat makes sense for targets where the jump table is emitted in the > text section; we then need to know its size for jumps over it, etc. > ISTM if you attach the table to the indirect jump, then you could include the size of hte table in the size of the indirect jump on targets where the table is emitted inline in the text section. I don't think there's anything we're doing now that couldn't be done with the table attached to the jump. I would even claim that some things become notably easier :-) But none of that is gcc-9 material. jeff
Re: [PATCH] Update assertion in sched-ebb.c to cope with table jumps
On Tue, Mar 26, 2019 at 12:15:26PM -0600, Jeff Law wrote: > On 3/26/19 11:52 AM, Segher Boessenkool wrote: > > On Mon, Mar 25, 2019 at 05:12:05PM -0600, Jeff Law wrote: > >> To touch on one of the issues I raised. AFAICT the schedulers don't use > >> SCHED_GROUP_P for dealing with tablejumps. They're used for > >> cc0-user/setter, fused insns and the like. That's a bit of a surprise. > >> > >> Given that the table isn't actually associated with the block with the > >> tablejump, SCHED_GROUP_P might actually create a whole new set of problems. > >> > >> Why oh why is the jump table data not actually attached to the tablejump > >> insn itself. Nearly 30 years of working on GCC and I can't answer that > >> one. > > > > It somewhat makes sense for targets where the jump table is emitted in the > > text section; we then need to know its size for jumps over it, etc. > > > ISTM if you attach the table to the indirect jump, then you could > include the size of hte table in the size of the indirect jump on > targets where the table is emitted inline in the text section. > > I don't think there's anything we're doing now that couldn't be done > with the table attached to the jump. I would even claim that some > things become notably easier :-) I agree; I was musing why it grew this way historically. > But none of that is gcc-9 material. Right, but it would be lovely if someone picked it up for GCC 10. Hint hint. Segher
[PATCH, i386]: Fix PR89827, ICE: in convert_op
Attached patch fixes a corner case in STV pass where the shift operand
register equals shift count register. The specialization for shift
insns marked register as processed, but didn't process shift input
operand, leaving an unprocessed DImode register.
2019-03-26 Uroš Bizjak
PR target/89827
* config/i386/i386.c (dimode_scalar_chain::convert_reg):
Also process XEXP (src, 0) of a shift insn.
testsuite/ChangeLog:
2019-03-26 Uroš Bizjak
PR target/89827
* gcc.target/i386/pr89827.c: New test.
Bootstrapped and regression tested on x86_64-linux-gnu {,-m32}.
Committed to mainline SVN, will be backported to gcc-8 branch.
Uros.
Index: config/i386/i386.c
===
--- config/i386/i386.c (revision 269950)
+++ config/i386/i386.c (working copy)
@@ -2040,6 +2040,7 @@ dimode_scalar_chain::convert_reg (unsigned regno)
emit_insn_before (seq, insn);
+ XEXP (src, 0) = replace_with_subreg (XEXP (src, 0), reg, reg);
XEXP (src, 1) = gen_rtx_SUBREG (DImode, tmp2, 0);
}
else if (!MEM_P (dst) || !REG_P (src))
Index: testsuite/gcc.target/i386/pr89827.c
===
--- testsuite/gcc.target/i386/pr89827.c (nonexistent)
+++ testsuite/gcc.target/i386/pr89827.c (working copy)
@@ -0,0 +1,11 @@
+/* PR target/89827 */
+/* { dg-do compile { target ia32 } } */
+/* { dg-options "-O2 -msse2 -mstv -mno-stackrealign" } */
+
+unsigned long long a;
+
+void
+foo (void)
+{
+ a >>= (unsigned short) a;
+}
[PATCH] RISC-V: Add sifive-7 pipeline description.
This adds a pipeline description for the SiFive 7 Series parts, and all of the supporting patches to make this work. There is quite a bit of infrastructure to add, as this is the first non-rocket-chip core that we are adding to the RISC-V backend. This also adds options for the existing sifive 3 and 5 cores which are rocket chip based. I ran into one interesting problem testing this, which appears to be a latent bug in the __builtin_longjmp support. I will file a bug report for that. For now, I added a RISC-V specific restore_stack_nonlocal pattern to work around that. Otherwise, this all should be pretty straightforward stuff. This was tested with 32-bit/64-bit elf/linux toolchains, with and without a --with-tune=sifive-7-series configure option. There were no regressions. Most of the work was done by Andrew Waterman. Committed. Jim * config/riscv/generic.md (generic_alu, generic_load, generic_store) (generic_xfer, generic_branch, generic_imul, generic_idivsi) (generic_idivdi, generic_fmul_single, generic_fmul_double) (generic_fdiv, generic_fsqrt): Add check for generic tune. (generic_alu): Add auipc to type list. * config/riscv/riscv-opts.h (enum riscv_microarchitecture_type): New. (riscv_microarchitecture): Declare. * config/riscv/riscv-protos.h (riscv_store_data_bypass_p): Declare. * config/riscv/riscv.c (struct riscv_cpu_info): Add microarchitecture field. (riscv_microarchitecture): New. (sifive_7_tune_info): New. (riscv_cpu_info_table): Add microarchitecture value for rocket and size. Add sifive-3-series, sifive-5-series, and sifive-7-series entries. (riscv_store_data_bypass_p): New. (riscv_option_override): Set riscv_microarchitecture from cpu->microarchitecture. * config/riscv/riscv.md: Include sifive-7.md. (type): Add auipc. (tune): New. (auipc): Change type to auipc. (restore_stack_nonlocal): New. * config/riscv/sifive-7.md: New. * doc/invoke.texi (RISC-V Options): Update mtune docs. --- gcc/config/riscv/generic.md | 44 +++- gcc/config/riscv/riscv-opts.h | 7 ++ gcc/config/riscv/riscv-protos.h | 1 + gcc/config/riscv/riscv.c| 114 +- gcc/config/riscv/riscv.md | 28 +++- gcc/config/riscv/sifive-7.md| 120 gcc/doc/invoke.texi | 11 ++- 7 files changed, 304 insertions(+), 21 deletions(-) create mode 100644 gcc/config/riscv/sifive-7.md diff --git a/gcc/config/riscv/generic.md b/gcc/config/riscv/generic.md index 13c19811319..fcd78b990fe 100644 --- a/gcc/config/riscv/generic.md +++ b/gcc/config/riscv/generic.md @@ -26,53 +26,65 @@ (define_cpu_unit "fdivsqrt" "pipe0") (define_insn_reservation "generic_alu" 1 - (eq_attr "type" "unknown,const,arith,shift,slt,multi,nop,logical,move") + (and (eq_attr "tune" "generic") + (eq_attr "type" "unknown,const,arith,shift,slt,multi,auipc,nop,logical,move")) "alu") (define_insn_reservation "generic_load" 3 - (eq_attr "type" "load,fpload") + (and (eq_attr "tune" "generic") + (eq_attr "type" "load,fpload")) "alu") (define_insn_reservation "generic_store" 1 - (eq_attr "type" "store,fpstore") + (and (eq_attr "tune" "generic") + (eq_attr "type" "store,fpstore")) "alu") (define_insn_reservation "generic_xfer" 3 - (eq_attr "type" "mfc,mtc,fcvt,fmove,fcmp") + (and (eq_attr "tune" "generic") + (eq_attr "type" "mfc,mtc,fcvt,fmove,fcmp")) "alu") (define_insn_reservation "generic_branch" 1 - (eq_attr "type" "branch,jump,call") + (and (eq_attr "tune" "generic") + (eq_attr "type" "branch,jump,call")) "alu") (define_insn_reservation "generic_imul" 10 - (eq_attr "type" "imul") + (and (eq_attr "tune" "generic") + (eq_attr "type" "imul")) "imuldiv*10") (define_insn_reservation "generic_idivsi" 34 - (and (eq_attr "type" "idiv") - (eq_attr "mode" "SI")) + (and (eq_attr "tune" "generic") + (and (eq_attr "type" "idiv") + (eq_attr "mode" "SI"))) "imuldiv*34") (define_insn_reservation "generic_idivdi" 66 - (and (eq_attr "type" "idiv") - (eq_attr "mode" "DI")) + (and (eq_attr "tune" "generic") + (and (eq_attr "type" "idiv") + (eq_attr "mode" "DI"))) "imuldiv*66") (define_insn_reservation "generic_fmul_single" 5 - (and (eq_attr "type" "fadd,fmul,fmadd") - (eq_attr "mode" "SF")) + (and (eq_attr "tune" "generic") + (and (eq_attr "type" "fadd,fmul,fmadd") + (eq_attr "mode" "SF"))) "alu") (define_insn_reservation "generic_fmul_double" 7 - (and (eq_attr "type" "fadd,fmul,fmadd") - (eq_attr "mode" "DF")) + (and (eq_attr "tune" "generic") + (and (eq_attr "type" "fadd,fmul,fmadd") + (eq_attr "mode" "DF"))) "alu") (define_insn_reservation "generic_fdiv" 20 - (eq_attr "t
Re: [Patch, fortran] PR88247 - [8/9 Regression] ICE in get_array_ctor_var_strlen, at fortran/trans-array.c:2068
Hi Paul, Bootstraps and regtests on FC29/x86_64 - OK for 8- and 9-branches? OK. Thanks for the patch! Regards Thomas
[C++ PATCH] Fix SWITCH_STMT handling in potential_constant_expression_1 (PR c++/89785)
Hi!
As the testcase shows, the SWITCH_STMT handling in
potential_constant_expression_1
is quite conservative, it doesn't recurse on the body of the switch stmt,
because at least for the case where the switch condition isn't some easily
determinable constant, we'd need to try all possible values of the switch
expression (or start walking from all possible case labels, see PR for
details), but right now potential_constant_expression_1 doesn't have the
careful stmt skipping logic cxx_eval_* has anyway.
So, the following patch still doesn't recurse on SWITCH_STMT_BODY using
potential_constant_expression_1, but instead checks if the body contains
a return or continue stmt (the latter only if not nested in some loop body
inside of the switch body). If there is no return nor continue, assuming
there is no endless loop (which wouldn't be constant expression due to the
ops limit) the switch body must fall through to the code after it.
If there is a return or continue, we set *jump_target to it.
Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk?
2019-03-26 Jakub Jelinek
PR c++/89785
* constexpr.c (struct check_for_return_continue_data): New type.
(check_for_return_continue): New function.
(potential_constant_expression_1) : Walk
SWITCH_STMT_BODY to find RETURN_EXPRs or CONTINUE_STMTs not nested
in loop bodies and set *jump_target to that if found.
* g++.dg/cpp1y/constexpr-89785-1.C: New test.
* g++.dg/cpp1y/constexpr-89785-2.C: New test.
--- gcc/cp/constexpr.c.jj 2019-03-22 15:39:58.175990033 +0100
+++ gcc/cp/constexpr.c 2019-03-26 20:35:06.745157623 +0100
@@ -5710,6 +5710,86 @@ check_automatic_or_tls (tree ref)
}
#endif
+/* Data structure for passing data from potential_constant_expression_1
+ to check_for_return_continue via cp_walk_tree. */
+struct check_for_return_continue_data {
+ hash_set *pset;
+ tree continue_stmt;
+};
+
+/* Helper function for potential_constant_expression_1 SWITCH_STMT handling,
+ called through cp_walk_tree. Return the first RETURN_EXPR found, or note
+ the first CONTINUE_STMT if RETURN_EXPR is not found. */
+static tree
+check_for_return_continue (tree *tp, int *walk_subtrees, void *data)
+{
+ tree t = *tp, s;
+ check_for_return_continue_data *d = (check_for_return_continue_data *) data;
+ switch (TREE_CODE (t))
+{
+case RETURN_EXPR:
+ return t;
+
+case CONTINUE_STMT:
+ if (d->continue_stmt == NULL_TREE)
+ d->continue_stmt = t;
+ break;
+
+#define RECUR(x) \
+ if (tree r = cp_walk_tree (&x, check_for_return_continue, data, \
+d->pset)) \
+ return r
+
+ /* For loops, walk subtrees manually, so that continue stmts found
+inside of the bodies of the loops are ignored. */
+case DO_STMT:
+ *walk_subtrees = 0;
+ RECUR (DO_COND (t));
+ s = d->continue_stmt;
+ RECUR (DO_BODY (t));
+ d->continue_stmt = s;
+ break;
+
+case WHILE_STMT:
+ *walk_subtrees = 0;
+ RECUR (WHILE_COND (t));
+ s = d->continue_stmt;
+ RECUR (WHILE_BODY (t));
+ d->continue_stmt = s;
+ break;
+
+case FOR_STMT:
+ *walk_subtrees = 0;
+ RECUR (FOR_INIT_STMT (t));
+ RECUR (FOR_COND (t));
+ RECUR (FOR_EXPR (t));
+ s = d->continue_stmt;
+ RECUR (FOR_BODY (t));
+ d->continue_stmt = s;
+ break;
+
+case RANGE_FOR_STMT:
+ *walk_subtrees = 0;
+ RECUR (RANGE_FOR_EXPR (t));
+ s = d->continue_stmt;
+ RECUR (RANGE_FOR_BODY (t));
+ d->continue_stmt = s;
+ break;
+#undef RECUR
+
+case STATEMENT_LIST:
+case CONSTRUCTOR:
+ break;
+
+default:
+ if (!EXPR_P (t))
+ *walk_subtrees = 0;
+ break;
+}
+
+ return NULL_TREE;
+}
+
/* Return true if T denotes a potentially constant expression. Issue
diagnostic as appropriate under control of FLAGS. If WANT_RVAL is true,
an lvalue-rvalue conversion is implied. If NOW is true, we want to
@@ -6175,7 +6255,24 @@ potential_constant_expression_1 (tree t,
if (!RECUR (SWITCH_STMT_COND (t), rval))
return false;
/* FIXME we don't check SWITCH_STMT_BODY currently, because even
-unreachable labels would be checked. */
+unreachable labels would be checked and it is enough if there is
+a single switch cond value for which it is a valid constant
+expression. We need to check if there are any RETURN_EXPRs
+or CONTINUE_STMTs inside of the body though, as in that case
+we need to set *jump_target. */
+ else
+ {
+ hash_set pset;
+ check_for_return_continue_data data = { &pset, NULL_TREE };
+ if (tree ret_expr
+ = cp_walk_tree (&SWITCH_STMT_BODY (t), check_for_return_continue,
+ &data, &pset))
+ /* The switch might return. */
+ *jump_
Re: [PATCH] libgomp: Add master thread to thread pool
On Mon, 25 Mar 2019 19:30:57 +0100 Jakub Jelinek wrote: > On Fri, Feb 22, 2019 at 06:11:44PM -0700, Kevin Buettner wrote: > > For debugging purposes, I need to be able to find the master thread > > in the thread pool. > > > > Without this patch, I see over 20 failures in the tests that I've > > written for GDB. > > > > I've also tested this in the gcc tree - no regressions. > > > > libgomp/ChangeLog: > > > > * team.c (gomp_team_start): Initialize pool->threads[0]. > > If it is really needed, then it might be much better to just do it > unconditionally, i.e. > pool->threads[0] = thr; > after the realloc, I don't see what doing it conditionally buys us, > gomp_realloc is already slow enough and even from cache-line ownership POV > at least when it is actually reallocated it shouldn't make a difference. That's fine with me. > That said, I'd like to better understand what do you need it for. I need to be able to find a thread's parent thread. Given a thread T, I want to find a thread P which is the "parent" of thread T. By parent thread, I mean the one which spawned the child thread. For the non-nested parallelism case, the master thread is the parent of each of the other top level threads. Finding the parent thread for nested (non top level) threads is harder. I don't currently have an acceptable solution for this case. (See below for a link to my earlier solution.) > When do you need to find the master thread, do you have some special case > when number of threads in the team is 1? Because in that case pool->threads > will be often NULL. I admit handling that case is quite trivial, but > I'm still asking if it is handled. Yes, the case where the number of threads is (just) 1 is handled by the code that I've written. > What do you do for the case of nested parallelism? There are no thread > pools ATM, so how do you find the master thread in that case? I presented a solution for finding the thread parent of a non top level thread a while back, but that solution wasn't acceptable to you at that time. See: https://gcc.gnu.org/ml/gcc-patches/2017-10/msg02259.html I have not yet come up with a better approach. I'm still trying to get the rest of my work to land. I plan to look at it again once I'm further along. I'll note however, that the code which I've written for GDB does attempt to follow the complete ancestor chain. The missing piece of the puzzle is being able to find the information I need from libgomp. Here is a simpler patch which sets the master thread unconditionally: libgomp/ChangeLog: * team.c (gomp_team_start): Initialize pool->threads[0]. diff --git a/libgomp/team.c b/libgomp/team.c index 2b2e9750da5..c422da3701d 100644 --- a/libgomp/team.c +++ b/libgomp/team.c @@ -482,6 +482,8 @@ gomp_team_start (void (*fn) (void *), void *data, unsigned nthreads, = gomp_realloc (pool->threads, pool->threads_size * sizeof (struct gomp_thread *)); + /* Add current (master) thread to threads[]. */ + pool->threads[0] = thr; } /* Release existing idle threads. */
Re: [PATCH] correct maximum valid alignment in error message (PR 89812)
Hi Jakub, > On Mon, Mar 25, 2019 at 11:41:35AM -0600, Jeff Law wrote: >> > PR c/89812 - incorrect maximum in error: requested alignment >> > ‘536870912’ exceeds maximum 2147483648 >> > >> > gcc/c-family/ChangeLog: >> > >> >PR c/89812 >> >* c-common.c (check_user_alignment): Rename local. Correct maximum >> >alignment in diagnostic. Avoid assuming argument fits in SHWI, >> >convert it to UHWI when it fits. >> > >> > gcc/testsuite/ChangeLog: >> > >> >PR c/89812 >> >* gcc.dg/attr-aligned-3.c: New test. >> OK > > The test FAILs on all 32-bit targets (where __UINT64_TYPE__ is unsigned long > long) due to -pedantic-errors, and I bet will fail on all non-ELF targets on > AVR, because only config/elfos.h defines 1 << 28 as MAX_OFILE_ALIGNMENT (in > bytes) and the test relies on exactly that value, nothing else. > > Fixed thusly, tested on x86_64-linux (-m32/-m64), ok for trunk? > > If we have some elf targets that still don't use elfos.h, we might need to > add them next to avr too. FWIW, adding *-*-solaris2.* to the target list lets the test also PASS on i386-pc-solaris2.11 and sparc-sun-solaris2.11 (32 and 64-bit each). Rainer -- - Rainer Orth, Center for Biotechnology, Bielefeld University
Re: [PATCH] libgomp: Add master thread to thread pool
On Tue, Mar 26, 2019 at 03:42:09PM -0700, Kevin Buettner wrote: > libgomp/ChangeLog: > > * team.c (gomp_team_start): Initialize pool->threads[0]. > > diff --git a/libgomp/team.c b/libgomp/team.c > index 2b2e9750da5..c422da3701d 100644 > --- a/libgomp/team.c > +++ b/libgomp/team.c > @@ -482,6 +482,8 @@ gomp_team_start (void (*fn) (void *), void *data, > unsigned nthreads, > = gomp_realloc (pool->threads, > pool->threads_size > * sizeof (struct gomp_thread *)); > + /* Add current (master) thread to threads[]. */ > + pool->threads[0] = thr; > } > >/* Release existing idle threads. */ Ok for trunk, let's deal with nested threads in a follow-up. Jakub
Re: [Patch] Bug85667-(x86_64) ms_abi rules aren't followed when returning short structs with float values(32-bit)
> IMO you ought not to duplicate most of function_value_32 here. I modified the patch. Please let me know your thoughts. -- Lokesh On Sat, Mar 23, 2019 at 5:59 PM Eric Botcazou wrote: > > > The attached patch (pr85667.patch) fixes the subjected issue for 32-bit. > > Please let me know your thoughts on the patch. > > IMO you ought not to duplicate most of function_value_32 here. > > -- > Eric Botcazou 85667.patch Description: Binary data
