Re: [Ada] Fix segfault on double record extension
> This is a regression present on the mainline and 5 branch, for a double > record extension involving a size clause on the root type and a > discriminant with variant part on the first extension... > > Tested on x86_64-suse-linux, applied on the mainline and 5 branch. > > > 2016-03-27 Eric Botcazou > > * gcc-interface/decl.c (components_to_record): Add special case for > single field with representation clause at offset 0. The attached patch restricts the change to the problematic cases. Tested on x86_64-suse-linux, applied on the mainline and 5 branch. * gcc-interface/decl.c (components_to_record): Restrict the previous change to fields with variable size. -- Eric BotcazouIndex: gcc-interface/decl.c === --- gcc-interface/decl.c (revision 234695) +++ gcc-interface/decl.c (working copy) @@ -7606,13 +7606,15 @@ components_to_record (tree gnu_record_ty if (p_gnu_rep_list && gnu_rep_list) *p_gnu_rep_list = chainon (*p_gnu_rep_list, gnu_rep_list); - /* If only one field has a rep clause and it starts at 0, put back the field - at the head of the regular field list. This will avoid creating a useless - REP part below and deal with the annoying case of an extension of a record - with variable size and rep clause, for which the _Parent field is forced - at offset 0 and has variable size, which we do not support below. */ + /* Deal with the annoying case of an extension of a record with variable size + and partial rep clause, for which the _Parent field is forced at offset 0 + and has variable size, which we do not support below. Note that we cannot + do it if the field has fixed size because we rely on the presence of the + REP part built below to trigger the reordering of the fields in a derived + record type when all the fields have a fixed position. */ else if (gnu_rep_list && !DECL_CHAIN (gnu_rep_list) + && TREE_CODE (DECL_SIZE (gnu_rep_list)) != INTEGER_CST && !variants_have_rep && first_free_pos && integer_zerop (first_free_pos)
Re: [patch, fortran] PR68566 ICE on using unusable array in reshape
Hi Jerry, > ... > I will add an additional test case for the original posted problem in the PR. > Two existing tests get exercised, changing the error message. Finding the > problems earlier in the matchers I think is the right way to go. I am curious > if > the old checks ever get triggered (I will look into that a little later. (1) In real_dimension_1.f, s/defualt/default/; (2) Before your patch the errors were Error: Expression at (1) must be of INTEGER type, found REAL How difficult is it to restore the "found … « ? (3) dimension(n,d) used to give two errors, one for n and one for d. How difficult is it to restore this behavior. > Regression tested on x86-64-linux. OK for trunk? I see FAIL: gfortran.dg/pr36192_1.f90 -O (test for errors, line 5) FAIL: gfortran.dg/pr36192_1.f90 -O (test for excess errors) Thanks for working on this PR, Dominique
Re: Fix for PR70498 in Libiberty Demangler
> On 2 Apr 2016, at 1:44 AM, Bernd Schmidt wrote:
>
> On 04/01/2016 07:41 PM, Pedro Alves wrote:
>> On 04/01/2016 11:21 AM, Marcel Böhme wrote:
>>> static inline void
>>> -d_append_num (struct d_print_info *dpi, long l)
>>> +d_append_num (struct d_print_info *dpi, int l)
>>> {
>>>char buf[25];
>>>sprintf (buf,"%ld", l);
>>
>> Doesn't this warn about %ld format vs int type mismatch?
>
> Well spotted. Marcel, please correct this issue and check for other warnings.
> Unless libiberty is somehow exempt from -Werror, this should have shown up in
> a bootstrap.
Bootstrapped and regression tested on x86_64-pc-linux-gnu. Test cases added +
checked PR70498 is resolved.
The patch now also accounts for overflows in d_compact_number which is supposed
to return -1 in case of negative numbers.
Thanks,
- Marcel
--
Index: libiberty/cp-demangle.c
===
--- libiberty/cp-demangle.c (revision 234663)
+++ libiberty/cp-demangle.c (working copy)
@@ -398,7 +398,7 @@ d_make_dtor (struct d_info *, enum gnu_v
struct demangle_component *);
static struct demangle_component *
-d_make_template_param (struct d_info *, long);
+d_make_template_param (struct d_info *, int);
static struct demangle_component *
d_make_sub (struct d_info *, const char *, int);
@@ -421,9 +421,9 @@ static struct demangle_component *d_unqu
static struct demangle_component *d_source_name (struct d_info *);
-static long d_number (struct d_info *);
+static int d_number (struct d_info *);
-static struct demangle_component *d_identifier (struct d_info *, long);
+static struct demangle_component *d_identifier (struct d_info *, int);
static struct demangle_component *d_operator_name (struct d_info *);
@@ -1119,7 +1119,7 @@ d_make_dtor (struct d_info *di, enum gnu
/* Add a new template parameter. */
static struct demangle_component *
-d_make_template_param (struct d_info *di, long i)
+d_make_template_param (struct d_info *di, int i)
{
struct demangle_component *p;
@@ -1135,7 +1135,7 @@ d_make_template_param (struct d_info *di
/* Add a new function parameter. */
static struct demangle_component *
-d_make_function_param (struct d_info *di, long i)
+d_make_function_param (struct d_info *di, int i)
{
struct demangle_component *p;
@@ -1620,7 +1620,7 @@ d_unqualified_name (struct d_info *di)
static struct demangle_component *
d_source_name (struct d_info *di)
{
- long len;
+ int len;
struct demangle_component *ret;
len = d_number (di);
@@ -1633,12 +1633,12 @@ d_source_name (struct d_info *di)
/* number ::= [n] <(non-negative decimal integer)> */
-static long
+static int
d_number (struct d_info *di)
{
int negative;
char peek;
- long ret;
+ int ret;
negative = 0;
peek = d_peek_char (di);
@@ -1681,7 +1681,7 @@ d_number_component (struct d_info *di)
/* identifier ::= <(unqualified source code identifier)> */
static struct demangle_component *
-d_identifier (struct d_info *di, long len)
+d_identifier (struct d_info *di, int len)
{
const char *name;
@@ -1702,7 +1702,7 @@ d_identifier (struct d_info *di, long le
/* Look for something which looks like a gcc encoding of an
anonymous namespace, and replace it with a more user friendly
name. */
- if (len >= (long) ANONYMOUS_NAMESPACE_PREFIX_LEN + 2
+ if (len >= (int) ANONYMOUS_NAMESPACE_PREFIX_LEN + 2
&& memcmp (name, ANONYMOUS_NAMESPACE_PREFIX,
ANONYMOUS_NAMESPACE_PREFIX_LEN) == 0)
{
@@ -1870,7 +1870,7 @@ d_java_resource (struct d_info *di)
{
struct demangle_component *p = NULL;
struct demangle_component *next = NULL;
- long len, i;
+ int len, i;
char c;
const char *str;
@@ -2012,7 +2012,7 @@ d_special_name (struct d_info *di)
case 'C':
{
struct demangle_component *derived_type;
- long offset;
+ int offset;
struct demangle_component *base_type;
derived_type = cplus_demangle_type (di);
@@ -2946,10 +2946,10 @@ d_pointer_to_member_type (struct d_info
/* _ */
-static long
+static int
d_compact_number (struct d_info *di)
{
- long num;
+ int num;
if (d_peek_char (di) == '_')
num = 0;
else if (d_peek_char (di) == 'n')
@@ -2957,7 +2957,7 @@ d_compact_number (struct d_info *di)
else
num = d_number (di) + 1;
- if (! d_check_char (di, '_'))
+ if (num < 0 || ! d_check_char (di, '_'))
return -1;
return num;
}
@@ -2969,7 +2969,7 @@ d_compact_number (struct d_info *di)
static struct demangle_component *
d_template_param (struct d_info *di)
{
- long param;
+ int param;
if (! d_check_char (di, 'T'))
return NULL;
@@ -3502,7 +3502,7 @@ d_local_name (struct d_info *di)
static int
d_discriminator (struct d_info *di)
{
- long discrim;
+ int discrim;
if (d_peek_char (di) != '_')
return 1;
@@ -3558,7 +3558,7 @@ static struct demangle_component *
d_unnamed_typ
[wwwdocs] [1/3] projects/cxx-status.html -- introduce global CSS for tables
Jason pointed out that https://gcc.gnu.org/projects/cxx-status.html
looks as if it ignored all the CSS on that page, whereas things look
fine on his local system.
This is another victim of the strict Content Security Policy someone
put in place earlier this year, and luckily one of the last instances.
Which I am going to take care of now. ;-)
Below the first patch, after which things start looking at least
remotely sensible again.
(Note that we do not need this for the table itself; doing this
for its cells is sufficient.)
Committed.
Gerald
ChangeLog:
Introduce a new CSS class table.cxxstatus that draws a grid around
table cells. Use that class throughout projects/cxx-status.html
and add a missing empty cell (). Remove local CSS that
accomplished the same.
Index: gcc.css
===
RCS file: /cvs/gcc/wwwdocs/htdocs/gcc.css,v
retrieving revision 1.33
diff -u -r1.33 gcc.css
--- gcc.css 7 Feb 2016 15:05:47 - 1.33
+++ gcc.css 2 Apr 2016 09:33:10 -
@@ -64,6 +64,9 @@
/* Quote an e-mail. The first has the sender, the second the quote. */
blockquote.mail div:nth-child(2) { border-left: solid blue; padding-left: 4pt;
}
+/* C++ status tables. */
+table.cxxstatus th, td { border: 1px solid gray; }
+
/* Classpath versus libgcj merge status page. */
.classpath-only { background-color: #AA; }
Index: projects/cxx-status.html
===
RCS file: /cvs/gcc/wwwdocs/htdocs/projects/cxx-status.html,v
retrieving revision 1.2
diff -u -r1.2 cxx-status.html
--- projects/cxx-status.html16 Mar 2016 03:20:58 - 1.2
+++ projects/cxx-status.html2 Apr 2016 09:33:11 -
@@ -6,7 +6,6 @@
tr.separator { background: #cc}
.supported { color: green }
.unsupported { color: red }
- table, td, th { border: 1px solid gray }
/* ]]> */
@@ -57,7 +56,7 @@
version of GCC that contains an implementation of this feature (if
it has been implemented).
-
+
Language Feature
Proposal
@@ -192,7 +191,7 @@
-->
-
+
Technical Specification
Document
@@ -233,7 +232,7 @@
feature, while the "Available in GCC?" column indicates the first
version of GCC that contains an implementation of this feature.
-
+
Language Feature
Proposal
@@ -341,7 +340,7 @@
part of the published standard; as a result, it has been removed from
the compiler.
-
+
Language Feature
Proposal
@@ -378,7 +377,7 @@
GCC 4.8 C++11 Status
-
+
Language Feature
Proposal
@@ -390,6 +389,7 @@
http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2006/n2118.html";>N2118
GCC
4.3
__cpp_rvalue_references >= 200610
+
Rvalue references for *this
Re: [patch, fortran] PR68566 ICE on using unusable array in reshape
> Le 2 avr. 2016 à 11:44, Dominique d'Humières a écrit :
>
> Hi Jerry,
>
>> ...
>> I will add an additional test case for the original posted problem in the PR.
>> Two existing tests get exercised, changing the error message. Finding the
>> problems earlier in the matchers I think is the right way to go. I am
>> curious if
>> the old checks ever get triggered (I will look into that a little later.
>
> (2) Before your patch the errors were
>
> Error: Expression at (1) must be of INTEGER type, found REAL
>
> How difficult is it to restore the "found … « ?
--- ../_clean/gcc/fortran/array.c 2016-01-04 19:51:09.0 +0100
+++ gcc/fortran/array.c 2016-04-02 14:31:08.0 +0200
@@ -421,10 +421,15 @@ match_array_element_spec (gfc_array_spec
if (!gfc_expr_check_typed (*upper, gfc_current_ns, false))
return AS_UNKNOWN;
- if ((*upper)->expr_type == EXPR_FUNCTION && (*upper)->ts.type == BT_UNKNOWN
- && (*upper)->symtree && strcmp ((*upper)->symtree->name, "null") == 0)
+ if (((*upper)->expr_type == EXPR_CONSTANT
+ && (*upper)->ts.type != BT_INTEGER) ||
+ ((*upper)->expr_type == EXPR_FUNCTION
+ && (*upper)->ts.type == BT_UNKNOWN
+ && (*upper)->symtree
+ && strcmp ((*upper)->symtree->name, "null") == 0))
{
- gfc_error ("Expecting a scalar INTEGER expression at %C");
+ gfc_error ("Expecting a scalar INTEGER expression at %C, found %s",
+gfc_basic_typename ((*upper)->ts.type));
return AS_UNKNOWN;
}
@@ -448,10 +453,16 @@ match_array_element_spec (gfc_array_spec
if (!gfc_expr_check_typed (*upper, gfc_current_ns, false))
return AS_UNKNOWN;
- if ((*upper)->expr_type == EXPR_FUNCTION && (*upper)->ts.type == BT_UNKNOWN
- && (*upper)->symtree && strcmp ((*upper)->symtree->name, "null") == 0)
-{
- gfc_error ("Expecting a scalar INTEGER expression at %C");
+ if (((*upper)->expr_type == EXPR_CONSTANT
+ && (*upper)->ts.type != BT_INTEGER) ||
+ ((*upper)->expr_type == EXPR_FUNCTION
+ && (*upper)->ts.type == BT_UNKNOWN
+ && (*upper)->symtree
+ && strcmp ((*upper)->symtree->name, "null") == 0))
+{
+ /* gfc_error ("Expecting a scalar INTEGER expression at %C"); */
+ gfc_error ("Expecting a scalar INTEGER expression at %C, found %s",
+gfc_basic_typename ((*upper)->ts.type));
return AS_UNKNOWN;
}
Does the trick (not regtested yet).
Dominique
[wwwdocs] [2/3] projects/cxx-status.html -- introduce global CSS for tables
This patch shows the power of CSS. In my quest to replace local
styles with ones coming from the global style sheet, this replaces
91 instances of
style="text-align:center;"
by a single line
table.cxxstatus td:nth-child(3) { text-align:center; }
and any future changes/additions to any of these tables will
automatically get this formatting.
Cool, isn't it?
Committed.
Gerald
Index: gcc.css
===
RCS file: /cvs/gcc/wwwdocs/htdocs/gcc.css,v
retrieving revision 1.34
diff -u -r1.34 gcc.css
--- gcc.css 2 Apr 2016 10:03:18 - 1.34
+++ gcc.css 2 Apr 2016 10:14:28 -
@@ -66,6 +66,7 @@
/* C++ status tables. */
table.cxxstatus th, td { border: 1px solid gray; }
+table.cxxstatus td:nth-child(3) { text-align:center; }
/* Classpath versus libgcj merge status page. */
Index: projects/cxx-status.html
===
RCS file: /cvs/gcc/wwwdocs/htdocs/projects/cxx-status.html,v
retrieving revision 1.3
diff -u -r1.3 cxx-status.html
--- projects/cxx-status.html2 Apr 2016 10:03:18 - 1.3
+++ projects/cxx-status.html2 Apr 2016 10:14:30 -
@@ -66,27 +67,25 @@
Removing trigraphs
http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2014/n4086.html";>N4086
- 5.1
+ 5.1
u8 character literals
http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2014/n4267.html";>N4267
- 6
+ 6
__cpp_unicode_characters >= 201411
Folding expressions
http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2014/n4295.html";>N4295
- 6
+ 6
__cpp_fold_expressions >= 201411
Attributes for namespaces and enumerators
http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2014/n4266.html";>N4266
-
-4.9 (namespaces) 6 (enumerators)
-
+ 4.9 (namespaces) 6 (enumerators)
__cpp_namespace_attributes >= 201411
__cpp_enumerator_attributes >= 201411
@@ -95,80 +94,80 @@
Nested namespace definitions
http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2014/n4230.html";>N4230
- 6
+ 6
__cpp_nested_namespace_definitions >= 201411
Allow constant evaluation for all non-type template arguments
http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2014/n4268.html";>N4268
- 6
+ 6
__cpp_nontype_template_args >= 201411
Extending static_assert
http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2014/n3928.html";>N3928
- 6
+ 6
__cpp_static_assert >= 201411
[[fallthrough]] attribute
http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2016/p0188r1.pdf";>P0188R1
- N/A (no fallthrough warning)
+ N/A (no fallthrough warning)
__has_cpp_attribute(fallthrough)
[[nodiscard]] attribute
http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2016/p0189r1.pdf";>P0189R1
- [[gnu::warn_unused_result]]
+ [[gnu::warn_unused_result]]
__has_cpp_attribute(nodiscard)
[[maybe_unused]] attribute
http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2016/p0212r1.pdf";>P0212R1
- [[gnu::unused]]
+ [[gnu::unused]]
__has_cpp_attribute(maybe_unused)
Extension to aggregate initialization
http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2015/p0017r1.html";>P0017R1
- No
+ No
__cpp_aggregate_bases >= 201603
Wording for constexpr lambda
http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2016/p0170r1.pdf";>P0170R1
- No
+ No
__cpp_constexpr >= 201603
Unary Folds and Empty Parameter Packs
http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2015/p0036r0.pdf";>P0036R0
- 6
+ 6
__cpp_fold_expressions >= 201603
Generalizing the Range-Based For Loop
http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2016/p0184r0.html";>P0184R0
- 6
+ 6
__cpp_range_based_for >= 201603
Lambda capture of *this by Value
http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2016/p0018r3.html";>P0018R3
- No
+ No
__cpp_capture_star_this >= 201603
Construction Rules for enum class variables
http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2016/p0138r2.pdf";>P0138R2
- No
+ No
__cpp_enum_class_init >= 201603
Hexadecimal floating literals for C++
http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2016/p0245r1.html";>P0245R1
- 3.0
+ 3.0
__cpp_hex_float >= 201603
@@ -191,7 +190,7 @@
-->
-
+
Technical Specification
Document
@@ -242,13 +241,
[wwwdocs] [3/3] projects/cxx-status.html -- introduce global CSS for tables
And with this, colors are back again, and we nearly are done.
Jason, I suggest to color the cells instead of just the font,
alas with a lighter color, and committed this per the patch
below.
If you'd like to make this look exactly the same as it used to,
we can of course.
Gerald
Move color styles for "supported" and "unsupported" to global
stylesheet. Change from text colors to lighter background colors.
Index: gcc.css
===
RCS file: /cvs/gcc/wwwdocs/htdocs/gcc.css,v
retrieving revision 1.35
diff -u -r1.35 gcc.css
--- gcc.css 2 Apr 2016 10:39:24 - 1.35
+++ gcc.css 2 Apr 2016 13:40:40 -
@@ -67,6 +67,9 @@
/* C++ status tables. */
table.cxxstatus th, td { border: 1px solid gray; }
table.cxxstatus td:nth-child(3) { text-align:center; }
+.supported { background-color: lightgreen; }
+.unsupported { background-color: lightsalmon; }
/* Classpath versus libgcj merge status page. */
Index: projects/cxx-status.html
===
RCS file: /cvs/gcc/wwwdocs/htdocs/projects/cxx-status.html,v
retrieving revision 1.5
diff -u -r1.5 cxx-status.html
--- projects/cxx-status.html2 Apr 2016 10:45:24 - 1.5
+++ projects/cxx-status.html2 Apr 2016 13:40:41 -
@@ -4,8 +4,6 @@
/* */
Re: [patch, fortran] PR68566 ICE on using unusable array in reshape
On 04/02/2016 05:42 AM, Dominique d'Humières wrote:
>
>> Le 2 avr. 2016 à 11:44, Dominique d'Humières a écrit :
>>
>> Hi Jerry,
>>
>>> ...
>>> I will add an additional test case for the original posted problem in the
>>> PR.
>>> Two existing tests get exercised, changing the error message. Finding the
>>> problems earlier in the matchers I think is the right way to go. I am
>>> curious if
>>> the old checks ever get triggered (I will look into that a little later.
>>
>> (2) Before your patch the errors were
>>
>> Error: Expression at (1) must be of INTEGER type, found REAL
>>
>> How difficult is it to restore the "found … « ?
I like that idea and not too difficult to do. ;)
>
> --- ../_clean/gcc/fortran/array.c 2016-01-04 19:51:09.0 +0100
> +++ gcc/fortran/array.c 2016-04-02 14:31:08.0 +0200
> @@ -421,10 +421,15 @@ match_array_element_spec (gfc_array_spec
>if (!gfc_expr_check_typed (*upper, gfc_current_ns, false))
> return AS_UNKNOWN;
>
> - if ((*upper)->expr_type == EXPR_FUNCTION && (*upper)->ts.type == BT_UNKNOWN
> - && (*upper)->symtree && strcmp ((*upper)->symtree->name, "null") == 0)
> + if (((*upper)->expr_type == EXPR_CONSTANT
> + && (*upper)->ts.type != BT_INTEGER) ||
> + ((*upper)->expr_type == EXPR_FUNCTION
> + && (*upper)->ts.type == BT_UNKNOWN
> + && (*upper)->symtree
> + && strcmp ((*upper)->symtree->name, "null") == 0))
> {
> - gfc_error ("Expecting a scalar INTEGER expression at %C");
> + gfc_error ("Expecting a scalar INTEGER expression at %C, found %s",
> + gfc_basic_typename ((*upper)->ts.type));
>return AS_UNKNOWN;
> }
>
> @@ -448,10 +453,16 @@ match_array_element_spec (gfc_array_spec
>if (!gfc_expr_check_typed (*upper, gfc_current_ns, false))
> return AS_UNKNOWN;
>
> - if ((*upper)->expr_type == EXPR_FUNCTION && (*upper)->ts.type == BT_UNKNOWN
> - && (*upper)->symtree && strcmp ((*upper)->symtree->name, "null") == 0)
> -{
> - gfc_error ("Expecting a scalar INTEGER expression at %C");
> + if (((*upper)->expr_type == EXPR_CONSTANT
> + && (*upper)->ts.type != BT_INTEGER) ||
> + ((*upper)->expr_type == EXPR_FUNCTION
> + && (*upper)->ts.type == BT_UNKNOWN
> + && (*upper)->symtree
> + && strcmp ((*upper)->symtree->name, "null") == 0))
> +{
> + /* gfc_error ("Expecting a scalar INTEGER expression at %C"); */
> + gfc_error ("Expecting a scalar INTEGER expression at %C, found %s",
> + gfc_basic_typename ((*upper)->ts.type));
>return AS_UNKNOWN;
> }
>
> Does the trick (not regtested yet).
>
> Dominique
I will do this, also the other test that you showed failed, I thought I fixed
already, but will double check it.
Jerry
Re: [PATCH] PR libitm/70456: Allocate aligned memory in gtm_thread operator new
On Wed, Mar 30, 2016 at 5:34 AM, H.J. Lu wrote:
> Since GTM::gtm_thread has
>
> gtm_thread *next_thread __attribute__((__aligned__(HW_CACHELINE_SIZE)));
>
> GTM::gtm_thread::operator new should allocate aligned memory.
>
> Tested on Linux/x86-64. OK for trunk.
>
>
This patch is better. Tested on Linux/x86-64. OK for trunk?
--
H.J.
From 461ada452acc2fc6decb281f136ab1136fe46ab2 Mon Sep 17 00:00:00 2001
From: "H.J. Lu"
Date: Sat, 2 Apr 2016 07:18:05 -0700
Subject: [PATCH] Allocate memory on cache line if requested
Since GTM::gtm_thread has
gtm_thread *next_thread __attribute__((__aligned__(HW_CACHELINE_SIZE)));
GTM::gtm_thread::operator new () calls xmalloc with separate_cl == true.
xmalloc must return memory on cache line in this case.
PR libitm/70456
* util.cc (xmalloc): Use posix_memalign to allocate memory on
on cache line if requested.
---
libitm/util.cc | 22 --
1 file changed, 16 insertions(+), 6 deletions(-)
diff --git a/libitm/util.cc b/libitm/util.cc
index 16e5d03..f89b2e5 100644
--- a/libitm/util.cc
+++ b/libitm/util.cc
@@ -61,12 +61,22 @@ GTM_fatal (const char *fmt, ...)
void *
xmalloc (size_t size, bool separate_cl)
{
- // TODO Use posix_memalign if separate_cl is true, or some other allocation
- // method that will avoid sharing cache lines with data used by other
- // threads.
- void *r = malloc (size);
- if (r == 0)
-GTM_fatal ("Out of memory allocating %lu bytes", (unsigned long) size);
+ void *r;
+#ifdef HAVE_POSIX_MEMALIGN
+ if (separate_cl)
+{
+ if (posix_memalign (&r, HW_CACHELINE_SIZE, size))
+ GTM_fatal ("Out of memory allocating %lu bytes aligned on cache line",
+ (unsigned long) size);
+}
+ else
+#endif
+{
+ r = malloc (size);
+ if (r == 0)
+ GTM_fatal ("Out of memory allocating %lu bytes",
+ (unsigned long) size);
+}
return r;
}
--
2.5.5
Re: [PATCH] Fix PR c++/70452 (regression in C++ parsing performance)
On Fri, 1 Apr 2016, Jason Merrill wrote:
> I like this approach a lot. One thing, though:
>
> On 04/01/2016 03:13 PM, Patrick Palka wrote:
> > +struct GTY((chain_next ("%h.prev"))) bpr_entry
> > +{
> > + tree body;
> > + tree parms;
> > + tree res;
> > + struct bpr_entry *prev;
> > +};
> > +
> > /* Representation of entries in the constexpr function definition table.
> > */
> >
> > struct GTY((for_user)) constexpr_fundef {
> > tree decl;
> > tree body;
> > + /* A chain of unused copies of this function's body awaiting reuse for
> > + CALL_EXPR evaluation. */
> > + struct bpr_entry *bpr_freelist;
> > };
>
> The freelist should be discarded on GC. I think the way to achieve that is to
> put all the freelists into a separate deletable hash table rather than hang
> them off the fundefs.
>
> Jason
>
>
Here's a version that uses a separate deletable table to cache the
function copies. For simplicity I used a hash_map instead of a
hash_table. Does this look OK to commit after bootstrap + regtest?
On a related note, I noticed that the constexpr_call_table is not marked
deletable. Marking it deletable speeds up the test case in the PR by
about 10% and saves about 10MB. Do you think doing so is a good idea?
On another related note, I noticed that marking something as both
GTY((deletable, cache)) doesn't work as intended, because the
gt_cleare_cache functions run _after_ all deletable roots get
zeroed out. So during GC the gt_cleare_cache function of a root
marked "deletable, cache" would always be a no-op. Concretely I think
this means that our cv_cache and fold_cache leak memory during GC
because their underlying hash_map (allocated by operator new) is zeroed
before gc_cleare_cache could clear it.
gcc/cp/ChangeLog:
PR c++/70452
* constexpr.c (struct fun_copy): New struct.
(struct fundef_copies_t): New struct.
(fundef_copies_table): New static variable.
(maybe_initialize_fundef_copies_table): New static function.
(get_fun_copy): New static function.
(save_fun_copy): New static function.
(cxx_eval_call_expression): Use
maybe_initialize_fundef_copies_table, get_fun_copy, and
save_fun_copy.
gcc/testsuite/ChangeLog:
PR c++/70452
* g++.dg/ext/constexpr-vla4.C: New test.
---
gcc/cp/constexpr.c| 96 +--
gcc/testsuite/g++.dg/ext/constexpr-vla4.C | 17 ++
2 files changed, 109 insertions(+), 4 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/ext/constexpr-vla4.C
diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c
index ea605dc..e1a5a4e 100644
--- a/gcc/cp/constexpr.c
+++ b/gcc/cp/constexpr.c
@@ -965,6 +965,76 @@ maybe_initialize_constexpr_call_table (void)
constexpr_call_table = hash_table::create_ggc (101);
}
+/* The representation of a single node in the per-function freelist maintained
+ by FUNDEF_COPIES_TABLE. */
+
+struct fun_copy
+{
+ tree body;
+ tree parms;
+ tree res;
+ fun_copy *prev;
+};
+
+/* During constexpr CALL_EXPR evaluation, to avoid issues with sharing when
+ a function happens to get called recursively, we unshare the callee
+ function's body and evaluate this unshared copy instead of evaluating the
+ original body.
+
+ FUNDEF_COPIES_TABLE is a per-function freelist of these unshared function
+ copies. The underlying data structure of FUNDEF_COPIES_TABLE is a hash_map
+ that's keyed off of the original FUNCTION_DECL and whose value is the chain
+ of this function's unused copies awaiting reuse. */
+
+struct fundef_copies_table_t
+{
+ hash_map *map;
+};
+
+static GTY((deletable)) fundef_copies_table_t fundef_copies_table;
+
+/* Initialize FUNDEF_COPIES_TABLE if it's not initialized. */
+
+static void
+maybe_initialize_fundef_copies_table ()
+{
+ if (fundef_copies_table.map == NULL)
+fundef_copies_table.map = hash_map::create_ggc (101);
+}
+
+/* Reuse or create a new unshared copy of the function FUN.
+ Return this copy. */
+
+static fun_copy *
+get_fun_copy (tree fun)
+{
+ fun_copy *copy;
+ fun_copy **slot = &fundef_copies_table.map->get_or_insert (fun, NULL);
+ if (*slot == NULL)
+{
+ copy = ggc_alloc ();
+ copy->body = copy_fn (fun, copy->parms, copy->res);
+ copy->prev = NULL;
+}
+ else
+{
+ copy = *slot;
+ *slot = (*slot)->prev;
+}
+
+ return copy;
+}
+
+/* Save the copy COPY of function FUN for later reuse by get_fun_copy(). */
+
+static void
+save_fun_copy (tree fun, fun_copy *copy)
+{
+ fun_copy **slot = &fundef_copies_table.map->get_or_insert (fun, NULL);
+ copy->prev = *slot;
+ *slot = copy;
+}
+
/* We have an expression tree T that represents a call, either CALL_EXPR
or AGGR_INIT_EXPR. If the call is lexically to a named function,
retrun the _DECL for that function. */
@@ -1377,10 +1447,14 @@ cxx_eval_call_expression (const constexpr_ctx *ctx,
tree t,
if (!result || result == error_mark
Re: [PATCH] Fix PR c++/70452 (regression in C++ parsing performance)
On Sat, Apr 02, 2016 at 05:18:31PM -0400, Patrick Palka wrote:
> On Fri, 1 Apr 2016, Jason Merrill wrote:
>
> > I like this approach a lot. One thing, though:
> >
> > On 04/01/2016 03:13 PM, Patrick Palka wrote:
> > > +struct GTY((chain_next ("%h.prev"))) bpr_entry
> > > +{
> > > + tree body;
> > > + tree parms;
> > > + tree res;
> > > + struct bpr_entry *prev;
> > > +};
> > > +
> > > /* Representation of entries in the constexpr function definition table.
> > > */
> > >
> > > struct GTY((for_user)) constexpr_fundef {
> > > tree decl;
> > > tree body;
> > > + /* A chain of unused copies of this function's body awaiting reuse for
> > > + CALL_EXPR evaluation. */
> > > + struct bpr_entry *bpr_freelist;
> > > };
> >
> > The freelist should be discarded on GC. I think the way to achieve that is
> > to
> > put all the freelists into a separate deletable hash table rather than hang
> > them off the fundefs.
> >
> > Jason
> >
> >
>
> Here's a version that uses a separate deletable table to cache the
> function copies. For simplicity I used a hash_map instead of a
> hash_table. Does this look OK to commit after bootstrap + regtest?
>
> On a related note, I noticed that the constexpr_call_table is not marked
> deletable. Marking it deletable speeds up the test case in the PR by
> about 10% and saves about 10MB. Do you think doing so is a good idea?
>
> On another related note, I noticed that marking something as both
> GTY((deletable, cache)) doesn't work as intended, because the
> gt_cleare_cache functions run _after_ all deletable roots get
> zeroed out. So during GC the gt_cleare_cache function of a root
> marked "deletable, cache" would always be a no-op. Concretely I think
> this means that our cv_cache and fold_cache leak memory during GC
> because their underlying hash_map (allocated by operator new) is zeroed
> before gc_cleare_cache could clear it.
yeah, I think that's true. I'm not really sure what the expected point
of making something both cache and deletable is maybe we should just ban
it? The way this ties in with the gc is... wierd ;) but I'm not really
sure how I'd fix it.
Trev
>
> gcc/cp/ChangeLog:
>
> PR c++/70452
> * constexpr.c (struct fun_copy): New struct.
> (struct fundef_copies_t): New struct.
> (fundef_copies_table): New static variable.
> (maybe_initialize_fundef_copies_table): New static function.
> (get_fun_copy): New static function.
> (save_fun_copy): New static function.
> (cxx_eval_call_expression): Use
> maybe_initialize_fundef_copies_table, get_fun_copy, and
> save_fun_copy.
>
> gcc/testsuite/ChangeLog:
>
> PR c++/70452
> * g++.dg/ext/constexpr-vla4.C: New test.
> ---
> gcc/cp/constexpr.c| 96
> +--
> gcc/testsuite/g++.dg/ext/constexpr-vla4.C | 17 ++
> 2 files changed, 109 insertions(+), 4 deletions(-)
> create mode 100644 gcc/testsuite/g++.dg/ext/constexpr-vla4.C
>
> diff --git a/gcc/cp/constexpr.c b/gcc/cp/constexpr.c
> index ea605dc..e1a5a4e 100644
> --- a/gcc/cp/constexpr.c
> +++ b/gcc/cp/constexpr.c
> @@ -965,6 +965,76 @@ maybe_initialize_constexpr_call_table (void)
> constexpr_call_table = hash_table::create_ggc
> (101);
> }
>
> +/* The representation of a single node in the per-function freelist
> maintained
> + by FUNDEF_COPIES_TABLE. */
> +
> +struct fun_copy
> +{
> + tree body;
> + tree parms;
> + tree res;
> + fun_copy *prev;
> +};
> +
> +/* During constexpr CALL_EXPR evaluation, to avoid issues with sharing when
> + a function happens to get called recursively, we unshare the callee
> + function's body and evaluate this unshared copy instead of evaluating the
> + original body.
> +
> + FUNDEF_COPIES_TABLE is a per-function freelist of these unshared function
> + copies. The underlying data structure of FUNDEF_COPIES_TABLE is a
> hash_map
> + that's keyed off of the original FUNCTION_DECL and whose value is the
> chain
> + of this function's unused copies awaiting reuse. */
> +
> +struct fundef_copies_table_t
> +{
> + hash_map *map;
> +};
> +
> +static GTY((deletable)) fundef_copies_table_t fundef_copies_table;
> +
> +/* Initialize FUNDEF_COPIES_TABLE if it's not initialized. */
> +
> +static void
> +maybe_initialize_fundef_copies_table ()
> +{
> + if (fundef_copies_table.map == NULL)
> +fundef_copies_table.map = hash_map::create_ggc (101);
> +}
> +
> +/* Reuse or create a new unshared copy of the function FUN.
> + Return this copy. */
> +
> +static fun_copy *
> +get_fun_copy (tree fun)
> +{
> + fun_copy *copy;
> + fun_copy **slot = &fundef_copies_table.map->get_or_insert (fun, NULL);
> + if (*slot == NULL)
> +{
> + copy = ggc_alloc ();
> + copy->body = copy_fn (fun, copy->parms, copy->res);
> + copy->prev = NULL;
> +}
> + else
> +{
> + copy = *slot;
> + *slot = (*slot)->prev;
> +}
> +
> + return co
