RE: [PATCH] Rename parameters which are within scop
> Date: Sat, 18 Jul 2015 02:22:19 +0200
> From: tob...@grosser.es
> To: hiradi...@msn.com; gcc-patches@gcc.gnu.org
> CC: seb...@gmail.com; richard.guent...@gmail.com
> Subject: Re: [PATCH] Rename parameters which are within scop
>
> Hi Aditya,
>
> could you possible expand the commit message a little bit to explain
> what you are doing?
My bad,
I sent a different patch which did not have the updated commit message. I have
send another patch (https://gcc.gnu.org/ml/gcc-patches/2015-07/msg01614.html)
which has comments and the email title is also relevant to the change. Sorry
for the confusion.
-Aditya
>
> Tobias
>
>
> On 07/18/2015 01:00 AM, Aditya Kumar wrote:> ---
>> gcc/graphite-isl-ast-to-gimple.c | 153
>> +++
>> 1 file changed, 122 insertions(+), 31 deletions(-)
>>
>> diff --git a/gcc/graphite-isl-ast-to-gimple.c
>> b/gcc/graphite-isl-ast-to-gimple.c
>> index b32781a..3e2c1fa 100644
>> --- a/gcc/graphite-isl-ast-to-gimple.c
>> +++ b/gcc/graphite-isl-ast-to-gimple.c
>> @@ -124,9 +124,84 @@ void ivs_params_clear (ivs_params &ip)
>> }
>> }
>>
>> -static tree
>> -gcc_expression_from_isl_expression (tree type, __isl_take isl_ast_expr *,
>> - ivs_params &ip);
>> +class translate_isl_ast_to_gimple
>> +{
>> +public:
>> + translate_isl_ast_to_gimple (sese r)
>> + : region (r)
>> + { }
>> +
>> + edge translate_isl_ast (loop_p context_loop, __isl_keep isl_ast_node *node,
>> + edge next_e, ivs_params &ip);
>> +
>> + edge translate_isl_ast_node_for (loop_p context_loop,
>> + __isl_keep isl_ast_node *node,
>> + edge next_e, ivs_params &ip);
>> +
>> + edge translate_isl_ast_for_loop (loop_p context_loop,
>> + __isl_keep isl_ast_node *node_for,
>> + edge next_e,
>> + tree type, tree lb, tree ub,
>> + ivs_params &ip);
>> +
>> + edge translate_isl_ast_node_if (loop_p context_loop,
>> + __isl_keep isl_ast_node *node,
>> + edge next_e, ivs_params &ip);
>> +
>> + edge translate_isl_ast_node_user (__isl_keep isl_ast_node *node,
>> + edge next_e, ivs_params &ip);
>> +
>> + edge translate_isl_ast_node_block (loop_p context_loop,
>> + __isl_keep isl_ast_node *node,
>> + edge next_e, ivs_params &ip);
>> +
>> + tree unary_op_to_tree (tree type, __isl_take isl_ast_expr *expr,
>> + ivs_params &ip);
>> +
>> + tree binary_op_to_tree (tree type, __isl_take isl_ast_expr *expr,
>> + ivs_params &ip);
>> +
>> + tree ternary_op_to_tree (tree type, __isl_take isl_ast_expr *expr,
>> + ivs_params &ip);
>> +
>> + tree nary_op_to_tree (tree type, __isl_take isl_ast_expr *expr,
>> + ivs_params &ip);
>> +
>> + tree gcc_expression_from_isl_expression (tree type,
>> + __isl_take isl_ast_expr *,
>> + ivs_params &ip);
>> +
>> + tree gcc_expression_from_isl_ast_expr_id (tree type,
>> + __isl_keep isl_ast_expr *expr_id,
>> + ivs_params &ip);
>> +
>> + tree gcc_expression_from_isl_expr_int (tree type,
>> + __isl_take isl_ast_expr *expr);
>> +
>> + tree gcc_expression_from_isl_expr_op (tree type,
>> + __isl_take isl_ast_expr *expr,
>> + ivs_params &ip);
>> +
>> + struct loop *graphite_create_new_loop (edge entry_edge,
>> + __isl_keep isl_ast_node *node_for,
>> + loop_p outer, tree type,
>> + tree lb, tree ub, ivs_params &ip);
>> +
>> + edge graphite_create_new_guard (edge entry_edge,
>> + __isl_take isl_ast_expr *if_cond,
>> + ivs_params &ip);
>> +
>> + edge graphite_create_new_loop_guard (edge entry_edge,
>> + __isl_keep isl_ast_node *node_for,
>> + tree *type,
>> + tree *lb, tree *ub, ivs_params &ip);
>> +
>> + void build_iv_mapping (vec iv_map, gimple_bb_p gbb,
>> + __isl_keep isl_ast_expr *user_expr, ivs_params &ip,
>> + sese region);
>> +private:
>> + sese region;
>> +};
>>
>> /* Return the tree variable that corresponds to the given isl ast identifier
>> expression (an isl_ast_expr of type isl_ast_expr_id).
>> @@ -136,7 +211,8 @@ gcc_expression_from_isl_expression (tree type,
>> __isl_take isl_ast_expr *,
>> converting type sizes may be problematic when we switch to smaller
>> types. */
>>
>> -static tree
>> +tree
>> +translate_isl_ast_to_gimple::
>> gcc_expression_from_isl_ast_expr_id (tree type,
>> __isl_keep isl_ast_expr *expr_id,
>> ivs_params &ip)
>> @@ -147,7 +223,7 @@ gcc_expression_from_isl_ast_expr_id (tree type,
>> res = ip.find (tmp_isl_id);
>> isl_id_free (tmp_isl_id);
>> gcc_assert (res != ip.end () &&
>> - "Could not map isl_id to tree expression");
>> + "Could not map isl_id to tree expression");
>> isl_ast_expr_free (expr_id);
>> return fold_convert (type, res->second);
>> }
>> @@ -155,7 +231,8 @@ gcc_expression_from_isl_ast_expr_id (tree type,
>> /* Converts an isl_ast_expr_int expression E to a GCC expression tree of
>> type TYPE. */
>>
>> -static tree
>> +tree
>> +translate_isl_ast_to_gimple::
>> gcc_expression_from_isl_expr_int (tree type, __isl_take isl_ast_expr *expr)
>> {
>> gcc_assert (isl_ast_expr_get_type (expr) == isl_ast_expr_int);
>> @@ -176,7 +253,8 @@ gcc_expression_from_isl_expr_int (tree type, __isl_take
>> isl_ast_expr *expr
RE: [PATCH 1/2] [graphite] Move codegen related functions to graphite-isl-ast-to-gimple.c
Thanks for the update. I'll fix that asap. -Aditya > Date: Thu, 19 Nov 2015 08:36:58 -0500 > Subject: Re: [PATCH 1/2] [graphite] Move codegen related functions to > graphite-isl-ast-to-gimple.c > From: dje@gmail.com > To: hiradi...@msn.com; aditya...@samsung.com; s@samsung.com > CC: gcc-patches@gcc.gnu.org > > This patch broke bootstrap when ISL is not enabled. > > graphite-isl-ast-to-gimple.c is protected by HAVE_isl but > get_false_edge_from_guard_bb() is used outside of Graphite, including > sese.c, which is not restricted to HAVE_isl. > > Please fix. > > Thanks, David
Refactor gimple_expr_type
Hi,
I have tried to refactor gimple_expr_type to make it more readable. Removed the
switch block and redundant if.
Please review this patch.
Thanks,
-Aditya
gcc/ChangeLog:
2015-05-15 hiraditya
* gimple.h (gimple_expr_type): Refactor to make it concise. Remove
redundant if.
diff --git a/gcc/gimple.h b/gcc/gimple.h
index 95e4fc8..168d3ba 100644
--- a/gcc/gimple.h
+++ b/gcc/gimple.h
@@ -5717,35 +5717,28 @@ static inline tree
gimple_expr_type (const_gimple stmt)
{
enum gimple_code code = gimple_code (stmt);
-
- if (code == GIMPLE_ASSIGN || code == GIMPLE_CALL)
+ tree type;
+ /* In general we want to pass out a type that can be substituted
+ for both the RHS and the LHS types if there is a possibly
+ useless conversion involved. That means returning the
+ original RHS type as far as we can reconstruct it. */
+ if (code == GIMPLE_CALL)
{
- tree type;
- /* In general we want to pass out a type that can be substituted
- for both the RHS and the LHS types if there is a possibly
- useless conversion involved. That means returning the
- original RHS type as far as we can reconstruct it. */
- if (code == GIMPLE_CALL)
- {
- const gcall *call_stmt = as_a (stmt);
- if (gimple_call_internal_p (call_stmt)
- && gimple_call_internal_fn (call_stmt) == IFN_MASK_STORE)
- type = TREE_TYPE (gimple_call_arg (call_stmt, 3));
- else
- type = gimple_call_return_type (call_stmt);
- }
+ const gcall *call_stmt = as_a (stmt);
+ if (gimple_call_internal_p (call_stmt)
+ && gimple_call_internal_fn (call_stmt) == IFN_MASK_STORE)
+ type = TREE_TYPE (gimple_call_arg (call_stmt, 3));
+ else
+ type = gimple_call_return_type (call_stmt);
+ return type;
+ }
+ else if (code == GIMPLE_ASSIGN)
+ {
+ if (gimple_assign_rhs_code (stmt) == POINTER_PLUS_EXPR)
+ type = TREE_TYPE (gimple_assign_rhs1 (stmt));
else
- switch (gimple_assign_rhs_code (stmt))
- {
- case POINTER_PLUS_EXPR:
- type = TREE_TYPE (gimple_assign_rhs1 (stmt));
- break;
-
- default:
- /* As fallback use the type of the LHS. */
- type = TREE_TYPE (gimple_get_lhs (stmt));
- break;
- }
+ /* As fallback use the type of the LHS. */
+ type = TREE_TYPE (gimple_get_lhs (stmt));
return type;
}
else if (code == GIMPLE_COND)
RE: Refactor gimple_expr_type
> Date: Sat, 16 May 2015 11:53:57 -0400
> From: tbsau...@tbsaunde.org
> To: hiradi...@msn.com
> CC: gcc-patches@gcc.gnu.org
> Subject: Re: Refactor gimple_expr_type
>
> On Fri, May 15, 2015 at 07:13:35AM +, Aditya K wrote:
>> Hi,
>> I have tried to refactor gimple_expr_type to make it more readable. Removed
>> the switch block and redundant if.
>>
>> Please review this patch.
>
> for some reason your mail client seems to be inserting non breaking
> spaces all over the place. Please either configure it to not do that,
> or use git send-email for patches.
Please see the updated patch.
gcc/ChangeLog:
2015-05-15 hiraditya
* gimple.h (gimple_expr_type): Refactor to make it concise. Remove
redundant if.
diff --git a/gcc/gimple.h b/gcc/gimple.h
index 95e4fc8..3a83e8f 100644
--- a/gcc/gimple.h
+++ b/gcc/gimple.h
@@ -5717,36 +5717,26 @@ static inline tree
gimple_expr_type (const_gimple stmt)
{
enum gimple_code code = gimple_code (stmt);
-
- if (code == GIMPLE_ASSIGN || code == GIMPLE_CALL)
+ /* In general we want to pass out a type that can be substituted
+ for both the RHS and the LHS types if there is a possibly
+ useless conversion involved. That means returning the
+ original RHS type as far as we can reconstruct it. */
+ if (code == GIMPLE_CALL)
{
- tree type;
- /* In general we want to pass out a type that can be substituted
- for both the RHS and the LHS types if there is a possibly
- useless conversion involved. That means returning the
- original RHS type as far as we can reconstruct it. */
- if (code == GIMPLE_CALL)
- {
- const gcall *call_stmt = as_a (stmt);
- if (gimple_call_internal_p (call_stmt)
- && gimple_call_internal_fn (call_stmt) == IFN_MASK_STORE)
- type = TREE_TYPE (gimple_call_arg (call_stmt, 3));
- else
- type = gimple_call_return_type (call_stmt);
- }
+ const gcall *call_stmt = as_a (stmt);
+ if (gimple_call_internal_p (call_stmt)
+ && gimple_call_internal_fn (call_stmt) == IFN_MASK_STORE)
+ return TREE_TYPE (gimple_call_arg (call_stmt, 3));
+ else
+ return gimple_call_return_type (call_stmt);
+ }
+ else if (code == GIMPLE_ASSIGN)
+ {
+ if (gimple_assign_rhs_code (stmt) == POINTER_PLUS_EXPR)
+ return TREE_TYPE (gimple_assign_rhs1 (stmt));
else
- switch (gimple_assign_rhs_code (stmt))
- {
- case POINTER_PLUS_EXPR:
- type = TREE_TYPE (gimple_assign_rhs1 (stmt));
- break;
-
- default:
- /* As fallback use the type of the LHS. */
- type = TREE_TYPE (gimple_get_lhs (stmt));
- break;
- }
- return type;
+ /* As fallback use the type of the LHS. */
+ return TREE_TYPE (gimple_get_lhs (stmt));
}
else if (code == GIMPLE_COND)
return boolean_type_node;
Thanks,
-Aditya
>
>>
>> Thanks,
>> -Aditya
>>
>>
>> gcc/ChangeLog:
>>
>> 2015-05-15 hiraditya
>>
>> * gimple.h (gimple_expr_type): Refactor to make it concise. Remove
>> redundant if.
>>
>> diff --git a/gcc/gimple.h b/gcc/gimple.h
>> index 95e4fc8..168d3ba 100644
>> --- a/gcc/gimple.h
>> +++ b/gcc/gimple.h
>> @@ -5717,35 +5717,28 @@ static inline tree
>> gimple_expr_type (const_gimple stmt)
>> {
>>enum gimple_code code = gimple_code (stmt);
>> -
>> - if (code == GIMPLE_ASSIGN || code == GIMPLE_CALL)
>> + tree type;
>> + /* In general we want to pass out a type that can be substituted
>> + for both the RHS and the LHS types if there is a possibly
>> + useless conversion involved. That means returning the
>> + original RHS type as far as we can reconstruct it. */
>> + if (code == GIMPLE_CALL)
>> {
>> - tree type;
>> - /* In general we want to pass out a type that can be substituted
>> - for both the RHS and the LHS types if there is a possibly
>> -useless conversion involved. That means returning the
>> -original RHS type as far as we can reconstruct it. */
>> - if (code == GIMPLE_CALL)
>> - {
>> - const gcall *call_stmt = as_a (stmt);
>> - if (gimple_call_internal_p (call_stmt)
>> - && gimple_call_internal_fn (call_stmt) == IFN_MASK_STORE)
>> - type = TREE_TYPE (gimple_call_arg (call_stmt, 3));
>> - else
>> - type = gimple_call_return_type (call_stmt);
>> - }
>> + const gcall *call_stmt = as_a (stmt);
>> + if (gimple_call_internal_p (call_stmt)
>>
Remove splay_tree from gimplify.c
The function `splay_tree_node splay_tree_lookup (splay_tree, splay_tree_key);'
updates the nodes every time a lookup is done.
IIUC, There are places where we call this function in a loop i.e., we lookup
different elements every time.
e.g.,
In this exaple we are looking for a different `t' in each iteration.
gcc/gimplify.c:1096: splay_tree_lookup (ctx->variables, (splay_tree_key) t) ==
NULL)
Here, we change the tree itself `ctx'
gcc/gimplify.c:5532: n = splay_tree_lookup (ctx->variables,
(splay_tree_key)decl);
I think we don't need to update the tree in these cases at least.
I have pasted the patch which removes splay_tree with hash_map. Another patch
(attached) removes splay_tree with std::map.
Please review the patches.
Thanks,
-Aditya
gcc/ChangeLog:
2015-05-17 hiraditya
Remove splay_tree from gimplify.c
* gimplify.c (struct gimplify_omp_ctx): Replaced splay_tree with
hash_map
(splay_tree_compare_decl_uid): Likewise
(new_omp_context): Likewise
(delete_omp_context): Likewise
(gimplify_bind_expr): Likewise
(omp_firstprivatize_variable): Likewise
(omp_add_variable): Likewise
(omp_notice_threadprivate_variable): Likewise
(omp_notice_variable): Likewise
(omp_is_private): Likewise
(omp_check_private): Likewise
(gimplify_adjust_omp_clauses_1): Likewise
(gimplify_adjust_omp_clauses): Likewise
(gimplify_omp_for): Likewise
* hash-map.h: Added typedefs.
diff --git a/gcc/gimplify.c b/gcc/gimplify.c
index 4846478..4454ec4 100644
--- a/gcc/gimplify.c
+++ b/gcc/gimplify.c
@@ -92,6 +92,20 @@ along with GCC; see the file COPYING3. If not see
#include "tree-pass.h" /* FIXME: only for PROP_gimple_any */
#include "builtins.h"
+struct tree_compare_decl_uid : default_hashmap_traits {
+ static inline hashval_t hash(tree t)
+ {
+ return iterative_hash_expr(t, 0);
+ }
+
+ static inline bool equal_keys(const tree &xa, const tree &xb)
+ {
+ return DECL_UID (xa) == DECL_UID (xb);
+ }
+};
+
+typedef hash_map gimplify_tree_t;
+
enum gimplify_omp_var_data
{
GOVD_SEEN = 1,
@@ -166,7 +180,7 @@ struct gimplify_ctx
struct gimplify_omp_ctx
{
struct gimplify_omp_ctx *outer_context;
- splay_tree variables;
+ gimplify_tree_t *variables;
hash_set *privatized_types;
location_t location;
enum omp_clause_default_kind default_kind;
@@ -364,17 +378,6 @@ gimple_pop_condition (gimple_seq *pre_p)
}
}
-/* A stable comparison routine for use with splay trees and DECLs. */
-
-static int
-splay_tree_compare_decl_uid (splay_tree_key xa, splay_tree_key xb)
-{
- tree a = (tree) xa;
- tree b = (tree) xb;
-
- return DECL_UID (a) - DECL_UID (b);
-}
-
/* Create a new omp construct that deals with variable remapping. */
static struct gimplify_omp_ctx *
@@ -384,7 +387,7 @@ new_omp_context (enum omp_region_type region_type)
c = XCNEW (struct gimplify_omp_ctx);
c->outer_context = gimplify_omp_ctxp;
- c->variables = splay_tree_new (splay_tree_compare_decl_uid, 0, 0);
+ c->variables = new gimplify_tree_t;
c->privatized_types = new hash_set;
c->location = input_location;
c->region_type = region_type;
@@ -401,7 +404,7 @@ new_omp_context (enum omp_region_type region_type)
static void
delete_omp_context (struct gimplify_omp_ctx *c)
{
- splay_tree_delete (c->variables);
+ delete c->variables;
delete c->privatized_types;
XDELETE (c);
}
@@ -1093,8 +1096,7 @@ gimplify_bind_expr (tree *expr_p, gimple_seq *pre_p)
/* Mark variable as local. */
if (ctx && !DECL_EXTERNAL (t)
&& (! DECL_SEEN_IN_BIND_EXPR_P (t)
- || splay_tree_lookup (ctx->variables,
- (splay_tree_key) t) == NULL))
+ || ctx->variables->get(t) == NULL))
{
if (ctx->region_type == ORT_SIMD
&& TREE_ADDRESSABLE (t)
@@ -5550,20 +5552,20 @@ gimplify_stmt (tree *stmt_p, gimple_seq *seq_p)
void
omp_firstprivatize_variable (struct gimplify_omp_ctx *ctx, tree decl)
{
- splay_tree_node n;
+ gimplify_tree_t::data_type *n;
if (decl == NULL || !DECL_P (decl))
return;
do
{
- n = splay_tree_lookup (ctx->variables, (splay_tree_key)decl);
+ n = ctx->variables->get(decl);
if (n != NULL)
{
- if (n->value & GOVD_SHARED)
- n->value = GOVD_FIRSTPRIVATE | (n->value & GOVD_SEEN);
- else if (n->value & GOVD_MAP)
- n->value |= GOVD_MAP_TO_ONLY;
+ if (*n & GOVD_SHARED)
+ *n = GOVD_FIRSTPRIVATE | (*n & GOVD_SEEN);
+ else if (*n & GOVD_MAP)
+ *n |= GOVD_MAP_TO_ONLY;
else
return;
}
@@ -5640,7 +5642,7 @@ omp_firstprivatize_type_sizes (struct gimplify_omp_ctx
*ctx, tree type)
static void
omp_add_variable (struct gimplify_omp_ctx *ctx, tree decl, unsigned int flags)
{
- splay_tree_node n;
+ gimplify_tree_t::data_type *n;
RE: Refactor gimple_expr_type
> Date: Mon, 18 May 2015 12:08:58 +0200
> Subject: Re: Refactor gimple_expr_type
> From: richard.guent...@gmail.com
> To: hiradi...@msn.com
> CC: tbsau...@tbsaunde.org; gcc-patches@gcc.gnu.org
>
> On Sun, May 17, 2015 at 5:31 PM, Aditya K wrote:
>>
>>
>>
>>> Date: Sat, 16 May 2015 11:53:57 -0400
>>> From: tbsau...@tbsaunde.org
>>> To: hiradi...@msn.com
>>> CC: gcc-patches@gcc.gnu.org
>>> Subject: Re: Refactor gimple_expr_type
>>>
>>> On Fri, May 15, 2015 at 07:13:35AM +, Aditya K wrote:
>>>> Hi,
>>>> I have tried to refactor gimple_expr_type to make it more readable.
>>>> Removed the switch block and redundant if.
>>>>
>>>> Please review this patch.
>>>
>>> for some reason your mail client seems to be inserting non breaking
>>> spaces all over the place. Please either configure it to not do that,
>>> or use git send-email for patches.
>>
>> Please see the updated patch.
>
> Ok if this passed bootstrap and regtest. (I wish if gimple_expr_type
> didn't exist btw...)
Thanks for the review. Do you have any suggestions on how to remove
gimple_expr_type. Are there any alternatives to it?
I can look into refactoring more (if it is not too complicated) since I'm
already doing this.
-Aditya
>
> Thanks,
> Richard.
>
>> gcc/ChangeLog:
>>
>> 2015-05-15 hiraditya
>>
>> * gimple.h (gimple_expr_type): Refactor to make it concise. Remove redundant
>> if.
>>
>> diff --git a/gcc/gimple.h b/gcc/gimple.h
>> index 95e4fc8..3a83e8f 100644
>> --- a/gcc/gimple.h
>> +++ b/gcc/gimple.h
>> @@ -5717,36 +5717,26 @@ static inline tree
>> gimple_expr_type (const_gimple stmt)
>> {
>> enum gimple_code code = gimple_code (stmt);
>> -
>> - if (code == GIMPLE_ASSIGN || code == GIMPLE_CALL)
>> + /* In general we want to pass out a type that can be substituted
>> + for both the RHS and the LHS types if there is a possibly
>> + useless conversion involved. That means returning the
>> + original RHS type as far as we can reconstruct it. */
>> + if (code == GIMPLE_CALL)
>> {
>> - tree type;
>> - /* In general we want to pass out a type that can be substituted
>> - for both the RHS and the LHS types if there is a possibly
>> - useless conversion involved. That means returning the
>> - original RHS type as far as we can reconstruct it. */
>> - if (code == GIMPLE_CALL)
>> - {
>> - const gcall *call_stmt = as_a (stmt);
>> - if (gimple_call_internal_p (call_stmt)
>> - && gimple_call_internal_fn (call_stmt) == IFN_MASK_STORE)
>> - type = TREE_TYPE (gimple_call_arg (call_stmt, 3));
>> - else
>> - type = gimple_call_return_type (call_stmt);
>> - }
>> + const gcall *call_stmt = as_a (stmt);
>> + if (gimple_call_internal_p (call_stmt)
>> + && gimple_call_internal_fn (call_stmt) == IFN_MASK_STORE)
>> + return TREE_TYPE (gimple_call_arg (call_stmt, 3));
>> + else
>> + return gimple_call_return_type (call_stmt);
>> + }
>> + else if (code == GIMPLE_ASSIGN)
>> + {
>> + if (gimple_assign_rhs_code (stmt) == POINTER_PLUS_EXPR)
>> + return TREE_TYPE (gimple_assign_rhs1 (stmt));
>> else
>> - switch (gimple_assign_rhs_code (stmt))
>> - {
>> - case POINTER_PLUS_EXPR:
>> - type = TREE_TYPE (gimple_assign_rhs1 (stmt));
>> - break;
>> -
>> - default:
>> - /* As fallback use the type of the LHS. */
>> - type = TREE_TYPE (gimple_get_lhs (stmt));
>> - break;
>> - }
>> - return type;
>> + /* As fallback use the type of the LHS. */
>> + return TREE_TYPE (gimple_get_lhs (stmt));
>> }
>> else if (code == GIMPLE_COND)
>> return boolean_type_node;
>>
>>
>> Thanks,
>> -Aditya
>>
>>
>>
>>
>>
>>>
>>>>
>>>> Thanks,
>>>> -Aditya
>>>>
>>>>
>>>> gcc/ChangeLog:
>>>>
>>>> 2015-05-15 hiraditya
>>>>
>>>> * gimple.h (gimple_expr_type): Refactor to make it concise. Remove
>>>> redundant if.
>>>>
>>>> diff --git a/gcc/gimple.h b/gcc/gimple.h
>>>> index 95e4fc8..168d3ba 100644
>>>> --- a/gcc/gimple.h
>>>> +++ b/gcc/gimple.h
>>>> @@ -5717,35 +5717,28 @@ static inline tree
>>>> gimple_expr_type (const_gimple stmt)
>>>> {
>>>> enum gimple_code code =
Fix PR48052: loop not vectorized if index is "unsigned int"
w.r.t. the PR48052, here is the patch which finds out if scev would wrap or not.
The patch symbolically evaluates if valid_niter>= loop->nb_iterations is true.
In that case the scev would not wrap (??).
Currently, we only look for two special 'patterns', which are sufficient to
analyze the simple test cases.
valid_niter = ~s (= UNIT_MAX - s)
We have to prove that valid_niter>= loop->nb_iterations
Pattern1 loop->nb_iterations: s>= e ? s - e : 0
Pattern2 loop->nb_iterations: (e - s) -1
In the first case we prove that valid_niter>= loop->nb_iterations in both the
cases i.e., when s>=e and when not.
In the second case we prove valid_niter>= loop->nb_iterations, by simple
analysis that UINT_MAX>= e is true in all cases.
I haven't tested this patch completely. I'm looking for feedback and any scope
for improvement.
hth,
-Aditya
Vectorize loops which has typecast.
2015-05-19 hiraditya
* gcc.dg/vect/pr48052.c: New test.
gcc/ChangeLog:
2015-05-19 hiraditya
* tree-ssa-loop-niter.c (fold_binary_cond_p): Fold a conditional
operation when additional constraints are
available.
(fold_binary_minus_p): Fold a subtraction operations of the form (A - B
-1) when additional constraints are
available.
(scev_probably_wraps_p): Use the above two functions to find whether
valid_niter>= loop->nb_iterations.
diff --git a/gcc/testsuite/gcc.dg/vect/pr48052.c
b/gcc/testsuite/gcc.dg/vect/pr48052.c
new file mode 100644
index 000..8e406d7
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/vect/pr48052.c
@@ -0,0 +1,27 @@
+/* { dg-do compile } */
+/* { dg-additional-options "-O3" } */
+/* { dg-final { scan-tree-dump-times "vectorized 1 loops" 2 "vect" } } */
+/* { dg-final { cleanup-tree-dump "vect" } } */
+
+int foo(int* A, int* B, unsigned start, unsigned BS)
+{
+ int s;
+ for (unsigned k = start; k < start + BS; k++)
+ {
+ s += A[k] * B[k];
+ }
+
+ return s;
+}
+
+int bar(int* A, int* B, unsigned BS)
+{
+ int s;
+ for (unsigned k = 0; k < BS; k++)
+ {
+ s += A[k] * B[k];
+ }
+
+ return s;
+}
+
diff --git a/gcc/tree-ssa-loop-niter.c b/gcc/tree-ssa-loop-niter.c
index 042f8df..ddc00cc 100644
--- a/gcc/tree-ssa-loop-niter.c
+++ b/gcc/tree-ssa-loop-niter.c
@@ -3773,6 +3773,117 @@ nowrap_type_p (tree type)
return false;
}
+/* Return true when op0>= op1.
+ For example:
+ Where, op0 = ~start_3(D);
+ op1 = start_3(D) <= stop_6(D) ? stop_6(D) - start_3(D) : 0;
+ In this case op0 = UINT_MAX - start_3(D);
+ So, op0>= op1 in all cases because UINT_MAX>= stop_6(D),
+ when TREE_TYPE(stop_6(D)) == unsigned int; */
+bool
+fold_binary_cond_p (enum tree_code code, tree type, tree op0, tree op1)
+{
+ gcc_assert (type == boolean_type_node);
+
+ if (TREE_TYPE (op0) != TREE_TYPE (op1))
+ return false;
+
+ /* TODO: Handle other operations. */
+ if (code != GE_EXPR)
+ return false;
+ // The type of op0 and op1 should be unsigned.
+ if (!TYPE_UNSIGNED (TREE_TYPE(op0)))
+ return false;
+ if ((TREE_CODE (op0) != BIT_NOT_EXPR) || (TREE_CODE (op1) != COND_EXPR))
+ return false;
+
+ /* We have to show that in both the cases,
+ (when cond is true and when cond is false) op (op0, op1) is true. */
+ tree neg_op0 = TREE_OPERAND (op0, 0);
+ tree cond_op1 = TREE_OPERAND (op1, 0);
+ tree true_op1 = TREE_OPERAND (op1, 1);
+ tree false_op1 = TREE_OPERAND (op1, 2);
+ gcc_assert(neg_op0 && cond_op1 && true_op1 && false_op1);
+
+ /* When cond is false. Evaluate op (op0, false_op1). */
+ tree running_exp = fold_binary (code, boolean_type_node, op0, false_op1);
+ if (running_exp == NULL || integer_zerop (running_exp))
+ /* TODO: Handle more cases here. */
+ return false;
+
+ /* When cond is true. Evaluate op (op0, true_op1). */
+ running_exp = fold_binary (code, boolean_type_node, op0, true_op1);
+ if (running_exp != NULL && integer_nonzerop (running_exp))
+ return true;
+
+ tree smaller, bigger;
+ if (TREE_CODE (cond_op1) == LE_EXPR)
+ {
+ smaller = TREE_OPERAND (cond_op1, 0);
+ bigger = TREE_OPERAND (cond_op1, 1);
+ } else return false;
+
+ if (TREE_CODE (true_op1) == MINUS_EXPR)
+ {
+ tree minuend = TREE_OPERAND (true_op1, 0);
+ tree subtrahend = TREE_OPERAND (true_op1, 1);
+ if (subtrahend == neg_op0 && subtrahend == smaller && minuend == bigger)
+ {
+ tree extreme = upper_bound_in_type (TREE_TYPE (neg_op0),
+ TREE_TYPE (neg_op0));
+ running_exp = fold_binary (code, boolean_type_node, extreme,
minuend);
+ return running_exp != NULL && integer_nonzerop(running_exp);
+ } else return false;
+ } else return false;
+}
+
+/* Return true when op0>= op1 and
+ op0 is ~start3(D) or, UINT_MAX - start3(D)
+ op1 is (_21 - start_3(D)) - 1; */
+bool
+fold_binary_minus_p (enum tree_code code, tree type, tree op0, tree op1)
+{
+ gcc_assert (type == boolean_type_node);
+
+ if (TREE_TYPE (op0
RE: Refactor gimple_expr_type
> Date: Tue, 19 May 2015 11:33:16 +0200
> Subject: Re: Refactor gimple_expr_type
> From: richard.guent...@gmail.com
> To: hiradi...@msn.com
> CC: tbsau...@tbsaunde.org; gcc-patches@gcc.gnu.org
>
> On Tue, May 19, 2015 at 12:04 AM, Aditya K wrote:
>>
>>
>>
>>> Date: Mon, 18 May 2015 12:08:58 +0200
>>> Subject: Re: Refactor gimple_expr_type
>>> From: richard.guent...@gmail.com
>>> To: hiradi...@msn.com
>>> CC: tbsau...@tbsaunde.org; gcc-patches@gcc.gnu.org
>>>
>>> On Sun, May 17, 2015 at 5:31 PM, Aditya K wrote:
>>>>
>>>>
>>>>
>>>>> Date: Sat, 16 May 2015 11:53:57 -0400
>>>>> From: tbsau...@tbsaunde.org
>>>>> To: hiradi...@msn.com
>>>>> CC: gcc-patches@gcc.gnu.org
>>>>> Subject: Re: Refactor gimple_expr_type
>>>>>
>>>>> On Fri, May 15, 2015 at 07:13:35AM +, Aditya K wrote:
>>>>>> Hi,
>>>>>> I have tried to refactor gimple_expr_type to make it more readable.
>>>>>> Removed the switch block and redundant if.
>>>>>>
>>>>>> Please review this patch.
>>>>>
>>>>> for some reason your mail client seems to be inserting non breaking
>>>>> spaces all over the place. Please either configure it to not do that,
>>>>> or use git send-email for patches.
>>>>
>>>> Please see the updated patch.
>>>
>>> Ok if this passed bootstrap and regtest. (I wish if gimple_expr_type
>>> didn't exist btw...)
>>
>> Thanks for the review. Do you have any suggestions on how to remove
>> gimple_expr_type. Are there any alternatives to it?
>> I can look into refactoring more (if it is not too complicated) since I'm
>> already doing this.
>
> Look at each caller - usually they should be fine with using TREE_TYPE
> (gimple_get_lhs ()) (or a more specific one
> dependent on what stmts are expected at the place). You might want to
> first refactor the code
>
> else if (code == GIMPLE_COND)
> gcc_unreachable ();
>
> and deal with the fallout in callers (similar for the void_type_node return).
Thanks for the suggestions. I looked at the use cases there are 47 usages in
different files. That might be a lot of changes I assume, and would take some
time.
This patch passes bootstrap and make check (although I'm not very confident
that my way of make check ran all the regtests)
If this patch is okay to merge please do that. I'll continue working on
removing gimle_expr_type.
Thanks,
-Aditya
>
> Richard.
>
>
>> -Aditya
>>
>>>
>>> Thanks,
>>> Richard.
>>>
>>>> gcc/ChangeLog:
>>>>
>>>> 2015-05-15 hiraditya
>>>>
>>>> * gimple.h (gimple_expr_type): Refactor to make it concise. Remove
>>>> redundant if.
>>>>
>>>> diff --git a/gcc/gimple.h b/gcc/gimple.h
>>>> index 95e4fc8..3a83e8f 100644
>>>> --- a/gcc/gimple.h
>>>> +++ b/gcc/gimple.h
>>>> @@ -5717,36 +5717,26 @@ static inline tree
>>>> gimple_expr_type (const_gimple stmt)
>>>> {
>>>> enum gimple_code code = gimple_code (stmt);
>>>> -
>>>> - if (code == GIMPLE_ASSIGN || code == GIMPLE_CALL)
>>>> + /* In general we want to pass out a type that can be substituted
>>>> + for both the RHS and the LHS types if there is a possibly
>>>> + useless conversion involved. That means returning the
>>>> + original RHS type as far as we can reconstruct it. */
>>>> + if (code == GIMPLE_CALL)
>>>> {
>>>> - tree type;
>>>> - /* In general we want to pass out a type that can be substituted
>>>> - for both the RHS and the LHS types if there is a possibly
>>>> - useless conversion involved. That means returning the
>>>> - original RHS type as far as we can reconstruct it. */
>>>> - if (code == GIMPLE_CALL)
>>>> - {
>>>> - const gcall *call_stmt = as_a (stmt);
>>>> - if (gimple_call_internal_p (call_stmt)
>>>> - && gimple_call_internal_fn (call_stmt) == IFN_MASK_STORE)
>>>> - type = TREE_TYPE (gimple_call_arg (call_stmt, 3));
>>>> - else
>>>> - type = gimple_call_return_type (call_stmt);
>>>> - }
>>>> + const gcall *call_stmt = as_a (stmt);
>>>> +
RE: Refactor gimple_expr_type
> Date: Wed, 20 May 2015 11:11:52 +0200
> Subject: Re: Refactor gimple_expr_type
> From: richard.guent...@gmail.com
> To: hiradi...@msn.com
> CC: tbsau...@tbsaunde.org; gcc-patches@gcc.gnu.org
>
> On Tue, May 19, 2015 at 6:50 PM, Aditya K wrote:
>>
>>
>>
>>> Date: Tue, 19 May 2015 11:33:16 +0200
>>> Subject: Re: Refactor gimple_expr_type
>>> From: richard.guent...@gmail.com
>>> To: hiradi...@msn.com
>>> CC: tbsau...@tbsaunde.org; gcc-patches@gcc.gnu.org
>>>
>>> On Tue, May 19, 2015 at 12:04 AM, Aditya K wrote:
>>>>
>>>>
>>>>
>>>>> Date: Mon, 18 May 2015 12:08:58 +0200
>>>>> Subject: Re: Refactor gimple_expr_type
>>>>> From: richard.guent...@gmail.com
>>>>> To: hiradi...@msn.com
>>>>> CC: tbsau...@tbsaunde.org; gcc-patches@gcc.gnu.org
>>>>>
>>>>> On Sun, May 17, 2015 at 5:31 PM, Aditya K wrote:
>>>>>>
>>>>>>
>>>>>> --------
>>>>>>> Date: Sat, 16 May 2015 11:53:57 -0400
>>>>>>> From: tbsau...@tbsaunde.org
>>>>>>> To: hiradi...@msn.com
>>>>>>> CC: gcc-patches@gcc.gnu.org
>>>>>>> Subject: Re: Refactor gimple_expr_type
>>>>>>>
>>>>>>> On Fri, May 15, 2015 at 07:13:35AM +, Aditya K wrote:
>>>>>>>> Hi,
>>>>>>>> I have tried to refactor gimple_expr_type to make it more readable.
>>>>>>>> Removed the switch block and redundant if.
>>>>>>>>
>>>>>>>> Please review this patch.
>>>>>>>
>>>>>>> for some reason your mail client seems to be inserting non breaking
>>>>>>> spaces all over the place. Please either configure it to not do that,
>>>>>>> or use git send-email for patches.
>>>>>>
>>>>>> Please see the updated patch.
>>>>>
>>>>> Ok if this passed bootstrap and regtest. (I wish if gimple_expr_type
>>>>> didn't exist btw...)
>>>>
>>>> Thanks for the review. Do you have any suggestions on how to remove
>>>> gimple_expr_type. Are there any alternatives to it?
>>>> I can look into refactoring more (if it is not too complicated) since I'm
>>>> already doing this.
>>>
>>> Look at each caller - usually they should be fine with using TREE_TYPE
>>> (gimple_get_lhs ()) (or a more specific one
>>> dependent on what stmts are expected at the place). You might want to
>>> first refactor the code
>>>
>>> else if (code == GIMPLE_COND)
>>> gcc_unreachable ();
>>>
>>> and deal with the fallout in callers (similar for the void_type_node
>>> return).
>>
>> Thanks for the suggestions. I looked at the use cases there are 47 usages in
>> different files. That might be a lot of changes I assume, and would take
>> some time.
>> This patch passes bootstrap and make check (although I'm not very confident
>> that my way of make check ran all the regtests)
>>
>> If this patch is okay to merge please do that. I'll continue working on
>> removing gimle_expr_type.
>
> Please re-send the patch as attachment, your mailer garbles the text
> (send mails as non-unicode text/plain).
>
Sure. I have attached the file.
Thanks,
-Aditya
> Richard.
>
>> Thanks,
>> -Aditya
>>
>>
>>>
>>> Richard.
>>>
>>>
>>>> -Aditya
>>>>
>>>>>
>>>>> Thanks,
>>>>> Richard.
>>>>>
>>>>>> gcc/ChangeLog:
>>>>>>
>>>>>> 2015-05-15 hiraditya
>>>>>>
>>>>>> * gimple.h (gimple_expr_type): Refactor to make it concise. Remove
>>>>>> redundant if.
>>>>>>
>>>>>> diff --git a/gcc/gimple.h b/gcc/gimple.h
>>>>>> index 95e4fc8..3a83e8f 100644
>>>>>> --- a/gcc/gimple.h
>>>>>> +++ b/gcc/gimple.h
>>>>>> @@ -5717,36 +5717,26 @@ static inline tree
>>>>>> gimple_expr_type (const_gimple stmt)
>>>>>> {
>>>>>> enum gimple_code code = gimple_code (stmt);
>&
RE: Fix PR48052: loop not vectorized if index is "unsigned int"
I tested this patch and it passes bootstrap and no extra failures.
Thanks
-Aditya
Symbolically evaluate conditionals, and subtraction when additional constraints
are provided.
Adding this evaluation mechanism helps vectorize some loops on 64bit machines
because on 64bit, a typecast appears
which causes scev to bail out.
gcc/ChangeLog:
2015-05-21 hiraditya
2015-05-21 Sebastian Pop
2015-05-21 Abderrazek Zaafrani
* gcc.dg/vect/pr48052.c: New test.
* tree-ssa-loop-niter.c (fold_binary_cond_p): Fold a conditional
operation when additional constraints are
available.
(fold_binary_minus_p): Fold a subtraction operations of the form (A - B
-1) when additional constraints are
available.
(scev_probably_wraps_p): Use the above two functions to find whether
valid_niter>= loop->nb_iterations.
diff --git a/gcc/testsuite/gcc.dg/vect/pr48052.c
b/gcc/testsuite/gcc.dg/vect/pr48052.c
new file mode 100644
index 000..8e406d7
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/vect/pr48052.c
@@ -0,0 +1,27 @@
+/* { dg-do compile } */
+/* { dg-additional-options "-O3" } */
+/* { dg-final { scan-tree-dump-times "vectorized 1 loops" 2 "vect" } } */
+/* { dg-final { cleanup-tree-dump "vect" } } */
+
+int foo(int* A, int* B, unsigned start, unsigned BS)
+{
+ int s;
+ for (unsigned k = start; k < start + BS; k++)
+ {
+ s += A[k] * B[k];
+ }
+
+ return s;
+}
+
+int bar(int* A, int* B, unsigned BS)
+{
+ int s;
+ for (unsigned k = 0; k < BS; k++)
+ {
+ s += A[k] * B[k];
+ }
+
+ return s;
+}
+
> From: hiradi...@msn.com
> To: gcc-patches@gcc.gnu.org; a.zaafr...@samsung.com; seb...@gmail.com;
> l...@redhat.com; richard.guent...@gmail.com
> Subject: Fix PR48052: loop not vectorized if index is "unsigned int"
> Date: Tue, 19 May 2015 16:12:26 +
>
> w.r.t. the PR48052, here is the patch which finds out if scev would wrap or
> not.
> The patch symbolically evaluates if valid_niter>= loop->nb_iterations is
> true. In that case the scev would not wrap (??).
> Currently, we only look for two special 'patterns', which are sufficient to
> analyze the simple test cases.
>
> valid_niter = ~s (= UNIT_MAX - s)
> We have to prove that valid_niter>= loop->nb_iterations
>
> Pattern1 loop->nb_iterations: s>= e ? s - e : 0
> Pattern2 loop->nb_iterations: (e - s) -1
>
> In the first case we prove that valid_niter>= loop->nb_iterations in both the
> cases i.e., when s>=e and when not.
> In the second case we prove valid_niter>= loop->nb_iterations, by simple
> analysis that UINT_MAX>= e is true in all cases.
>
> I haven't tested this patch completely. I'm looking for feedback and any
> scope for improvement.
>
>
> hth,
> -Aditya
>
>
>
> Vectorize loops which has typecast.
>
> 2015-05-19 hiraditya
> 2015-05-19 Sebastian Pop
> 2015-05-19 Abderrazek Zaafrani
>
> * gcc.dg/vect/pr48052.c: New test.
>
> gcc/ChangeLog:
>
> 2015-05-19 hiraditya
>
> * tree-ssa-loop-niter.c (fold_binary_cond_p): Fold a conditional
> operation when additional constraints are
> available.
> (fold_binary_minus_p): Fold a subtraction operations of the form (A -
> B -1) when additional constraints are
> available.
> (scev_probably_wraps_p): Use the above two functions to find whether
> valid_niter>= loop->nb_iterations.
>
>
> diff --git a/gcc/testsuite/gcc.dg/vect/pr48052.c
> b/gcc/testsuite/gcc.dg/vect/pr48052.c
> new file mode 100644
> index 000..8e406d7
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/vect/pr48052.c
> @@ -0,0 +1,27 @@
> +/* { dg-do compile } */
> +/* { dg-additional-options "-O3" } */
> +/* { dg-final { scan-tree-dump-times "vectorized 1 loops" 2 "vect" } } */
> +/* { dg-final { cleanup-tree-dump "vect" } } */
> +
> +int foo(int* A, int* B, unsigned start, unsigned BS)
> +{
> + int s;
> + for (unsigned k = start; k < start + BS; k++)
> +{
> + s += A[k] * B[k];
> +}
> +
> + return s;
> +}
> +
> +int bar(int* A, int* B, unsigned BS)
> +{
> + int s;
> + for (unsigned k = 0; k < BS; k++)
> +{
> + s += A[k] * B[k];
> +}
> +
> + return s;
> +}
> +
> diff --git a/gcc/tree-ssa-loop-niter.c b/gcc/tree-ssa-loop-niter.c
> index 042f8df..ddc00cc 100644
> --- a/gcc/tree-ssa-loop-niter.c
> +++ b/gcc/tree-ssa-loop-niter.c
> @@ -3773,6 +3773,117 @@ nowrap_type_p (tree type)
>return false;
> }
>
> +/* Return true when op0>= op1.
> + For example:
> + Where, op0 = ~start_3(D);
> + op1 = start_3(D) <= stop_6(D) ? stop_6(D) - start_3(D) : 0;
> + In this case op0 = UINT_MAX - start_3(D);
> + So, op0>= op1 in all cases because UINT_MAX>= stop_6(D),
> + when TREE_TYPE(stop_6(D)) == unsigned int; */
> +bool
> +fold_binary_cond_p (enum tree_code code, tree type, tree op0, tree op1)
> +{
> + gcc_assert (type == boolean_type_node);
> +
> + if (TREE_TYPE (op0) != TREE_TYPE (op1))
> +return false;
> +
> + /* TO
[PATCH] Print Pass Names
Currently, when we print the passes it does not print its name. This becomes
confusing when we want to print all the passes at once (e.g.,
-fdump-tree-all-all=stderr &> pass.dump).
This patch adds functionality to print the pass name. It passes bootstrap (with
default configurations).
Hope this is useful.
Thanks,
-Aditya
gcc/ChangeLog:
2015-05-22 Aditya Kumar
* passes.c (execute_todo): Added a parameter to pass the pass name.
(execute_one_pass): Likewise
* statistics.c (statistics_fini_pass): Likewise
* statistics.h: Likewise
---
gcc/passes.c | 9 +
gcc/statistics.c | 4 ++--
gcc/statistics.h | 2 +-
3 files changed, 8 insertions(+), 7 deletions(-)
diff --git a/gcc/passes.c b/gcc/passes.c
index 04ff042..7d41bd8 100644
--- a/gcc/passes.c
+++ b/gcc/passes.c
@@ -1984,7 +1984,7 @@ execute_function_todo (function *fn, void *data)
/* Perform all TODO actions. */
static void
-execute_todo (unsigned int flags)
+execute_todo (unsigned int flags, const char *pass_name=NULL)
{
#if defined ENABLE_CHECKING
if (cfun
@@ -1997,7 +1997,7 @@ execute_todo (unsigned int flags)
/* Inform the pass whether it is the first time it is run. */
first_pass_instance = (flags & TODO_mark_first_instance) != 0;
- statistics_fini_pass ();
+ statistics_fini_pass (pass_name);
if (flags)
do_per_function (execute_function_todo, (void *)(size_t) flags);
@@ -2302,7 +2302,7 @@ execute_one_pass (opt_pass *pass)
pass_init_dump_file (pass);
/* Run pre-pass verification. */
- execute_todo (pass->todo_flags_start);
+ execute_todo (pass->todo_flags_start, pass->name);
#ifdef ENABLE_CHECKING
do_per_function (verify_curr_properties,
@@ -2327,7 +2327,8 @@ execute_one_pass (opt_pass *pass)
check_profile_consistency (pass->static_pass_number, 0, true);
/* Run post-pass cleanup and verification. */
- execute_todo (todo_after | pass->todo_flags_finish | TODO_verify_il);
+ execute_todo (todo_after | pass->todo_flags_finish | TODO_verify_il,
+pass->name);
if (profile_report && cfun && (cfun->curr_properties & PROP_cfg))
check_profile_consistency (pass->static_pass_number, 1, true);
diff --git a/gcc/statistics.c b/gcc/statistics.c
index 8cbe88d..54f81d5 100644
--- a/gcc/statistics.c
+++ b/gcc/statistics.c
@@ -192,7 +192,7 @@ statistics_fini_pass_3 (statistics_counter_t **slot,
/* Dump the current statistics incrementally. */
void
-statistics_fini_pass (void)
+statistics_fini_pass (const char *pass_name)
{
if (current_pass->static_pass_number == -1)
return;
@@ -201,7 +201,7 @@ statistics_fini_pass (void)
&& dump_flags & TDF_STATS)
{
fprintf (dump_file, "\n");
- fprintf (dump_file, "Pass statistics:\n");
+ fprintf (dump_file, "Pass statistics of \"%s\":\n", pass_name);
fprintf (dump_file, "\n");
curr_statistics_hash ()
->traverse_noresize (NULL);
diff --git a/gcc/statistics.h b/gcc/statistics.h
index 0b871ec..4348b7a 100644
--- a/gcc/statistics.h
+++ b/gcc/statistics.h
@@ -64,7 +64,7 @@ struct function;
extern void statistics_early_init (void);
extern void statistics_init (void);
extern void statistics_fini (void);
-extern void statistics_fini_pass (void);
+extern void statistics_fini_pass (const char *pass_name = NULL);
extern void statistics_counter_event (struct function *, const char *, int);
extern void statistics_histogram_event (struct function *, const char *, int);
--
2.1.0.243.g30d45f7
RE: [PATCH] Print Pass Names
> Subject: Re: [PATCH] Print Pass Names
> From: richard.guent...@gmail.com
> Date: Fri, 22 May 2015 21:32:24 +0200
> To: hiradi...@msn.com; gcc-patches@gcc.gnu.org
>
> On May 22, 2015 6:32:38 PM GMT+02:00, Aditya K wrote:
>>Currently, when we print the passes it does not print its name. This
>>becomes confusing when we want to print all the passes at once (e.g.,
>>-fdump-tree-all-all=stderr &> pass.dump).
>>This patch adds functionality to print the pass name. It passes
>>bootstrap (with default configurations).
>>
>>Hope this is useful.
>
> Can't you just use current_pass->name?
You are right. I have updated the patch.
Thanks
-Aditya
gcc/ChangeLog:
2015-05-22 Aditya Kumar
* statistics.c (statistics_fini_pass): Print pass name.
---
gcc/statistics.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/gcc/statistics.c b/gcc/statistics.c
index 8cbe88d..50b41d1 100644
--- a/gcc/statistics.c
+++ b/gcc/statistics.c
@@ -201,7 +201,7 @@ statistics_fini_pass (void)
&& dump_flags & TDF_STATS)
{
fprintf (dump_file, "\n");
- fprintf (dump_file, "Pass statistics:\n");
+ fprintf (dump_file, "Pass statistics of \"%s\": ", current_pass->name);
fprintf (dump_file, "\n");
curr_statistics_hash ()
->traverse_noresize (NULL);
--
2.1.0.243.g30d45f7
>
> Richard.
>
>>Thanks,
>>-Aditya
>>
>>
>>gcc/ChangeLog:
>>
>>2015-05-22 Aditya Kumar
>>
>> * passes.c (execute_todo): Added a parameter to pass the pass name.
>> (execute_one_pass): Likewise
>> * statistics.c (statistics_fini_pass): Likewise
>> * statistics.h: Likewise
>>
>>---
>> gcc/passes.c | 9 +
>> gcc/statistics.c | 4 ++--
>> gcc/statistics.h | 2 +-
>> 3 files changed, 8 insertions(+), 7 deletions(-)
>>
>>diff --git a/gcc/passes.c b/gcc/passes.c
>>index 04ff042..7d41bd8 100644
>>--- a/gcc/passes.c
>>+++ b/gcc/passes.c
>>@@ -1984,7 +1984,7 @@ execute_function_todo (function *fn, void *data)
>>
>> /* Perform all TODO actions. */
>> static void
>>-execute_todo (unsigned int flags)
>>+execute_todo (unsigned int flags, const char *pass_name=NULL)
>> {
>> #if defined ENABLE_CHECKING
>> if (cfun
>>@@ -1997,7 +1997,7 @@ execute_todo (unsigned int flags)
>> /* Inform the pass whether it is the first time it is run. */
>> first_pass_instance = (flags & TODO_mark_first_instance) != 0;
>>
>>- statistics_fini_pass ();
>>+ statistics_fini_pass (pass_name);
>>
>> if (flags)
>> do_per_function (execute_function_todo, (void *)(size_t) flags);
>>@@ -2302,7 +2302,7 @@ execute_one_pass (opt_pass *pass)
>> pass_init_dump_file (pass);
>>
>> /* Run pre-pass verification. */
>>- execute_todo (pass->todo_flags_start);
>>+ execute_todo (pass->todo_flags_start, pass->name);
>>
>> #ifdef ENABLE_CHECKING
>> do_per_function (verify_curr_properties,
>>@@ -2327,7 +2327,8 @@ execute_one_pass (opt_pass *pass)
>> check_profile_consistency (pass->static_pass_number, 0, true);
>>
>> /* Run post-pass cleanup and verification. */
>>- execute_todo (todo_after | pass->todo_flags_finish |
>>TODO_verify_il);
>>+ execute_todo (todo_after | pass->todo_flags_finish | TODO_verify_il,
>>+ pass->name);
>> if (profile_report && cfun && (cfun->curr_properties & PROP_cfg))
>> check_profile_consistency (pass->static_pass_number, 1, true);
>>
>>diff --git a/gcc/statistics.c b/gcc/statistics.c
>>index 8cbe88d..54f81d5 100644
>>--- a/gcc/statistics.c
>>+++ b/gcc/statistics.c
>>@@ -192,7 +192,7 @@ statistics_fini_pass_3 (statistics_counter_t
>>**slot,
>> /* Dump the current statistics incrementally. */
>>
>> void
>>-statistics_fini_pass (void)
>>+statistics_fini_pass (const char *pass_name)
>> {
>> if (current_pass->static_pass_number == -1)
>> return;
>>@@ -201,7 +201,7 @@ statistics_fini_pass (void)
>> && dump_flags & TDF_STATS)
>> {
>> fprintf (dump_file, "\n");
>>- fprintf (dump_file, "Pass statistics:\n");
>>+ fprintf (dump_file, "Pass statistics of \"%s\":\n", pass_name);
>> fprintf (dump_file, "\n");
>> curr_statistics_hash ()
>> ->traverse_noresize (NULL);
>>diff --git a/gcc/statistics.h b/gcc/statistics.h
>>index 0b871ec..4348b7a 100644
>>--- a/gcc/statistics.h
>>+++ b/gcc/statistics.h
>>@@ -64,7 +64,7 @@ struct function;
>> extern void statistics_early_init (void);
>> extern void statistics_init (void);
>> extern void statistics_fini (void);
>>-extern void statistics_fini_pass (void);
>>+extern void statistics_fini_pass (const char *pass_name = NULL);
>>extern void statistics_counter_event (struct function *, const char *,
>>int);
>>extern void statistics_histogram_event (struct function *, const char
>>*, int);
>>
>
>
RE: [PATCH] Break when has_sample is true
I don't have commit access. I would appreciate if someone does that for me. Thanks, -Aditya > Date: Tue, 26 May 2015 08:14:41 -0600 > From: l...@redhat.com > To: hiradi...@msn.com; gcc-patches@gcc.gnu.org > Subject: Re: [PATCH] Break when has_sample is true > > On 05/26/2015 08:07 AM, Aditya Kumar wrote: >> gcc/ChangeLog: >> >> 2015-05-26 Aditya Kumar >> >> * auto-profile.c (afdo_calculate_branch_prob): Break once has_sample is true. > OK. Please install onto the trunk. > > Thanks, > jeff >
RE: [PATCH] Print Pass Names
I don't have commit access. I would appreciate if someone does that for me. Thanks, -Aditya > Date: Fri, 22 May 2015 14:52:29 -0600 > From: l...@redhat.com > To: hiradi...@msn.com; richard.guent...@gmail.com; gcc-patches@gcc.gnu.org > Subject: Re: [PATCH] Print Pass Names > > On 05/22/2015 02:38 PM, Aditya K wrote: >> >> >> >>> Subject: Re: [PATCH] Print Pass Names >>> From: richard.guent...@gmail.com >>> Date: Fri, 22 May 2015 21:32:24 +0200 >>> To: hiradi...@msn.com; gcc-patches@gcc.gnu.org >>> >>> On May 22, 2015 6:32:38 PM GMT+02:00, Aditya K wrote: >>>> Currently, when we print the passes it does not print its name. This >>>> becomes confusing when we want to print all the passes at once (e.g., >>>> -fdump-tree-all-all=stderr &> pass.dump). >>>> This patch adds functionality to print the pass name. It passes >>>> bootstrap (with default configurations). >>>> >>>> Hope this is useful. >>> >>> Can't you just use current_pass->name? >> >> You are right. I have updated the patch. >> Thanks >> -Aditya >> >> gcc/ChangeLog: >> >> 2015-05-22 Aditya Kumar >> >> * statistics.c (statistics_fini_pass): Print pass name.OK. > jeff >
RE: Remove splay_tree from gimplify.c
> Date: Wed, 27 May 2015 18:41:55 +0200 > From: ja...@redhat.com > To: l...@redhat.com > CC: hiradi...@msn.com; gcc-patches@gcc.gnu.org > Subject: Re: Remove splay_tree from gimplify.c > > On Wed, May 27, 2015 at 10:34:46AM -0600, Jeff Law wrote: >> So the question here is whether or not the other uses are commonly looking >> up elements we've already searched for -- that's the whole purpose of a >> splay tree, to improve lookup performance for commonly hit items. > > First of all, this is only used for OpenMP/OpenACC/Cilk+ constructs, > so it really isn't that performance criticial. > The code probably dates back to Richard's and Diego's changes. > And, I believe splay trees are the right thing to use here, while sometimes > you lookup different vars, looking up the same var many times in a row is > very common. If that is the case then I guess we can abandon the patch. I do not have a way to measure the compile time for OpenMP. Thanks for the review. -Aditya > > Jakub
RE: [PATCH] Print Pass Names
> Date: Wed, 27 May 2015 09:07:30 -0600 > From: l...@redhat.com > To: hiradi...@msn.com; richard.guent...@gmail.com; gcc-patches@gcc.gnu.org > Subject: Re: [PATCH] Print Pass Names > > On 05/26/2015 08:32 AM, Aditya K wrote: >> I don't have commit access. I would appreciate if someone does that for me. >> >> Thanks, >> -Aditya >> >> >>> Date: Fri, 22 May 2015 14:52:29 -0600 >>> From: l...@redhat.com >>> To: hiradi...@msn.com; richard.guent...@gmail.com; gcc-patches@gcc.gnu.org >>> Subject: Re: [PATCH] Print Pass Names >>> >>> On 05/22/2015 02:38 PM, Aditya K wrote: >>>> >>>> >>>> >>>>> Subject: Re: [PATCH] Print Pass Names >>>>> From: richard.guent...@gmail.com >>>>> Date: Fri, 22 May 2015 21:32:24 +0200 >>>>> To: hiradi...@msn.com; gcc-patches@gcc.gnu.org >>>>> >>>>> On May 22, 2015 6:32:38 PM GMT+02:00, Aditya K wrote: >>>>>> Currently, when we print the passes it does not print its name. This >>>>>> becomes confusing when we want to print all the passes at once (e.g., >>>>>> -fdump-tree-all-all=stderr &> pass.dump). >>>>>> This patch adds functionality to print the pass name. It passes >>>>>> bootstrap (with default configurations). >>>>>> >>>>>> Hope this is useful. >>>>> >>>>> Can't you just use current_pass->name? >>>> >>>> You are right. I have updated the patch. >>>> Thanks >>>> -Aditya >>>> >>>> gcc/ChangeLog: >>>> >>>> 2015-05-22 Aditya Kumar >>>> >>>> * statistics.c (statistics_fini_pass): Print pass name.OK. >>> jeff >>> >> >> > Installed on the trunk after a bootstrap and regression test run on > x86-linux-gnu. Thanks for merging the patches. -Aditya > > Thanks, > Jeff
Refactor gcc/tree-vectorize.c:vectorize_loops
Hi, This is a small patch where I moved a portion of code from the function vectorize_loops outside to improve readability. Please see the patch attached and give comments for improvements. Thanks -Aditya refactor-vect Description: Binary data
RE: Refactor gcc/tree-vectorize.c:vectorize_loops
Thanks for the feedback. I have added comment and properly indented the code.
-Aditya
> Date: Wed, 29 Apr 2015 09:31:46 +0200
> From: ja...@redhat.com
> To: l...@redhat.com
> CC: hiradi...@msn.com; gcc-patches@gcc.gnu.org
> Subject: Re: Refactor gcc/tree-vectorize.c:vectorize_loops
>
> On Tue, Apr 28, 2015 at 09:53:24PM -0600, Jeff Law wrote:
>> On 04/28/2015 08:20 PM, Aditya K wrote:
>>>Hi,
>>>This is a small patch where I moved a portion of code from the function
>>>vectorize_loops outside to improve readability.
>>>Please see the patch attached and give comments for improvements.
>> You need to add a comment for the new function. These function comments
>> should describe what the function does, in terms of its arguments and return
>> value (if any).
>>
>> With a function comment, this refactoring would be fine.
>
> --- a/gcc/tree-vectorizer.c
> +++ b/gcc/tree-vectorizer.c
> @@ -399,6 +399,39 @@ fold_loop_vectorized_call (gimple g, tree value)
> }
> }
>
> +static void
> +set_uid_loop_bbs(loop_vec_info loop_vinfo, gimple loop_vectorized_call)
>
> The formatting is incorrect too, missing space before ( here.
>
> +{
> + tree arg = gimple_call_arg (loop_vectorized_call, 1);
>
> Lines should be indented by 2 spaces rather than after {
>
> + basic_block *bbs;
> + unsigned int i;
> + struct loop *scalar_loop = get_loop (cfun, tree_to_shwi (arg));
> +
> + LOOP_VINFO_SCALAR_LOOP (loop_vinfo) = scalar_loop;
> + gcc_checking_assert (vect_loop_vectorized_call
> + (LOOP_VINFO_SCALAR_LOOP (loop_vinfo))
> + == loop_vectorized_call);
> + bbs = get_loop_body (scalar_loop);
> + for (i = 0; i < scalar_loop->num_nodes; i++)
> + {
> + basic_block bb = bbs[i];
> + gimple_stmt_iterator gsi;
> + for (gsi = gsi_start_phis (bb); !gsi_end_p (gsi);
> + gsi_next (&gsi))
>
> With the refactoring, this for should fit on one line.
>
> + {
> + gimple phi = gsi_stmt (gsi);
> + gimple_set_uid (phi, 0);
> + }
> + for (gsi = gsi_start_bb (bb); !gsi_end_p (gsi);
> + gsi_next (&gsi))
>
> Likewise.
>
> if (loop_vectorized_call)
> - {
> + set_uid_loop_bbs(loop_vinfo, loop_vectorized_call);
>
> Again, missing space before (. And you are using only spaces
> when a tab plus two spaces should be used.
>
> Jakub
refactor-vectorize-loops.patch
Description: Binary data
RE: Refactor gcc/tree-vectorize.c:vectorize_loops
Thank you very much Jeff. -Aditya > Date: Wed, 29 Apr 2015 23:44:20 -0600 > From: l...@redhat.com > To: hiradi...@msn.com; ja...@redhat.com > CC: gcc-patches@gcc.gnu.org > Subject: Re: Refactor gcc/tree-vectorize.c:vectorize_loops > > On 04/29/2015 08:37 AM, Aditya K wrote: >> >> Thanks for the feedback. I have added comment and properly indented the code. > I made a couple more formatting fixes (spaces -> tab & line wrapping), > improved the ChangeLog, did a bootstrap & regression test on > x86_64-linux-gnu and installed the final patch on the trunk. > > Thanks, > Jeff >
Fix compiler warnings
I recently compiled gcc with clang and found few useful warnings (https://gcc.gnu.org/ml/gcc/2015-05/msg00015.html, https://gcc.gnu.org/ml/gcc/2015-05/msg00041.html). I have a patch to fix some of those, it passes bootstrap, please apply these if it is useful. Thanks, -Aditya warnings.patch Description: Binary data
RE: Fix compiler warnings
Thanks! Updated patch.
2015-05-06 Aditya Kumar
* gcov-tool.c (do_merge): Refactored to remove int ret.
* ipa-icf.c (sem_item::hash_referenced_symbol_properties): Changed
(!type == FUNC) to (type != FUNC).
* reload.h (struct target_reload): Changed to type of
x_spill_indirect_levels from bool to unsigned char.
diff --git a/gcc/gcov-tool.c b/gcc/gcov-tool.c
index fd27d7c..b2a4583 100644
--- a/gcc/gcov-tool.c
+++ b/gcc/gcov-tool.c
@@ -193,7 +193,6 @@ static int
do_merge (int argc, char **argv)
{
int opt;
- int ret;
const char *output_dir = 0;
int w1 = 1, w2 = 1;
@@ -222,12 +221,10 @@ do_merge (int argc, char **argv)
if (output_dir == NULL)
output_dir = "merged_profile";
- if (argc - optind == 2)
- ret = profile_merge (argv[optind], argv[optind+1], output_dir, w1, w2);
- else
+ if (argc - optind != 2)
merge_usage ();
- return ret;
+ return profile_merge (argv[optind], argv[optind+1], output_dir, w1, w2);
}
/* If N_VAL is no-zero, normalize the profile by setting the largest counter
diff --git a/gcc/ipa-icf.c b/gcc/ipa-icf.c
index 1fbdf6d..3c4ac05 100644
--- a/gcc/ipa-icf.c
+++ b/gcc/ipa-icf.c
@@ -505,7 +505,7 @@ sem_item::hash_referenced_symbol_properties (symtab_node
*ref,
{
if (is_a (ref))
{
- if ((!type == FUNC || address || !opt_for_fn (decl, optimize_size))
+ if ((type != FUNC || address || !opt_for_fn (decl, optimize_size))
&& !opt_for_fn (ref->decl, optimize_size)
&& !DECL_UNINLINABLE (ref->decl))
{
diff --git a/gcc/reload.h b/gcc/reload.h
index a58b902..fb4a547 100644
--- a/gcc/reload.h
+++ b/gcc/reload.h
@@ -168,7 +168,7 @@ struct target_reload {
value indicates the level of indirect addressing supported, e.g., two
means that (MEM (MEM (REG n))) is also valid if (REG n) does not get
a hard register. */
- bool x_spill_indirect_levels;
+ unsigned char x_spill_indirect_levels;
/* True if caller-save has been reinitialized. */
bool x_caller_save_initialized_p;
> Date: Wed, 6 May 2015 18:26:10 +0200
> From: ja...@redhat.com
> To: hiradi...@msn.com
> CC: rdsandif...@googlemail.com; pola...@redhat.com; gcc-patches@gcc.gnu.org
> Subject: Re: Fix compiler warnings
>
> On Wed, May 06, 2015 at 04:22:13PM +, Aditya K wrote:
>> Thanks Richard, Jakub and Trevor for the feedback. I have reformatted the
>> changelog, and modified the patch addressing your comments.
>>
>> -Aditya
>>
>>
>> 2015-05-06 Aditya Kumar
>>
>> * gcov-tool.c (do_merge):
>> * ipa-icf.c (sem_item::hash_referenced_symbol_properties):
>> * reload.h (struct target_reload):
>
> After the : you need to say what has changed.
>
>> --- a/gcc/gcov-tool.c
>> +++ b/gcc/gcov-tool.c
>> @@ -193,7 +193,7 @@ static int
>> do_merge (int argc, char **argv)
>> {
>>int opt;
>> - int ret;
>> + int ret = 0;
>
> Initializing ret makes no sense then.
>
>>const char *output_dir = 0;
>>int w1 = 1, w2 = 1;
>>
>> @@ -222,11 +222,11 @@ do_merge (int argc, char **argv)
>>if (output_dir == NULL)
>> output_dir = "merged_profile";
>>
>> - if (argc - optind == 2)
>> -ret = profile_merge (argv[optind], argv[optind+1], output_dir, w1, w2);
>> - else
>> + if (argc - optind != 2)
>> merge_usage ();
>>
>> + ret = profile_merge (argv[optind], argv[optind+1], output_dir, w1, w2);
>> +
>>return ret;
>
> Perhaps better would be just
> return profile_merge (argv[optind], argv[optind+1], output_dir, w1, w2);
> and remove ret.
>
>> --- a/gcc/reload.h
>> +++ b/gcc/reload.h
>> @@ -168,7 +168,7 @@ struct target_reload {
>> value indicates the level of indirect addressing supported, e.g., two
>> means that (MEM (MEM (REG n))) is also valid if (REG n) does not get
>> a hard register. */
>> - bool x_spill_indirect_levels;
>> + signed char x_spill_indirect_levels;
>
> As negative values make no sense, this is better unsigned char than signed.
>
> Jakub
Patch: Refactor number_of_iterations_exit
Hi, I refactored number_of_iterations_exit a little bit. I hope it is helpful. The idea is to move the call to function dominated_by_p after some sanity checks so as to avoid call to it. Thanks, -Aditya 2015-05-07 Aditya Kumar * tree-ssa-loop-niter.c (number_of_iterations_exit): Move call to dominated_by_p later in the function. diff --git a/gcc/tree-ssa-loop-niter.c b/gcc/tree-ssa-loop-niter.c index 042f8df..3d49bb8 100644 --- a/gcc/tree-ssa-loop-niter.c +++ b/gcc/tree-ssa-loop-niter.c @@ -1959,11 +1959,6 @@ number_of_iterations_exit (struct loop *loop, edge exit, affine_iv iv0, iv1; bool safe; - safe = dominated_by_p (CDI_DOMINATORS, loop->latch, exit->src); - - if (every_iteration && !safe) - return false; - niter->assumptions = boolean_false_node; last = last_stmt (exit->src); if (!last) @@ -1972,6 +1967,11 @@ number_of_iterations_exit (struct loop *loop, edge exit, if (!stmt) return false; + safe = dominated_by_p (CDI_DOMINATORS, loop->latch, exit->src); + + if (every_iteration && !safe) + return false; + /* We want the condition for staying inside loop. */ code = gimple_cond_code (stmt); if (exit->flags & EDGE_TRUE_VALUE)
RE: Fix compiler warnings
Thank you very much Jakub. -Aditya > Date: Thu, 7 May 2015 19:58:11 +0200 > From: ja...@redhat.com > To: hiradi...@msn.com > CC: gcc-patches@gcc.gnu.org > Subject: Re: Fix compiler warnings > > On Wed, May 06, 2015 at 04:37:49PM +, Aditya K wrote: >> Thanks! Updated patch. >> >> >> 2015-05-06 Aditya Kumar >> >> * gcov-tool.c (do_merge): Refactored to remove int ret. >> * ipa-icf.c (sem_item::hash_referenced_symbol_properties): Changed >> (!type == FUNC) to (type != FUNC). >> * reload.h (struct target_reload): Changed to type of >> x_spill_indirect_levels from bool to unsigned char. > > I've rewritten the ChangeLog entry, replaced non-breaking spaces in the > patch with normal spaces (otherwise it doesn't apply), > bootstrapped/regtested on x86_64-linux and i686-linux and committed to > trunk. Thanks. > > Jakub
RE: Patch: Refactor number_of_iterations_exit
> Subject: Re: Patch: Refactor number_of_iterations_exit > From: richard.guent...@gmail.com > Date: Thu, 7 May 2015 19:47:37 +0200 > To: hiradi...@msn.com; gcc-patches@gcc.gnu.org > > On May 7, 2015 6:27:28 PM GMT+02:00, Aditya K wrote: >>Hi, >>I refactored number_of_iterations_exit a little bit. I hope it is >>helpful. >>The idea is to move the call to function dominated_by_p after some >>sanity checks so as to avoid call to it. > > The call is very cheap though. Do you have data that shows it often passes > when the later checks fail? I don't have any data. I was going through this code and realized that when last_stmt(exit->src) would fail or when the last_stmt is not a conditional it would be pointless to find if exit->src dominates loop->latch (which would be true for most simple loops, but then most simple loops would have conditionals as last statements. So I'm not very sure about this.) -Aditya > > Richard. > >> >>Thanks, >>-Aditya >> >> >>2015-05-07 Aditya Kumar >> >>* tree-ssa-loop-niter.c (number_of_iterations_exit): Move call >>to dominated_by_p later in the function. >> >>diff --git a/gcc/tree-ssa-loop-niter.c b/gcc/tree-ssa-loop-niter.c >>index 042f8df..3d49bb8 100644 >>--- a/gcc/tree-ssa-loop-niter.c >>+++ b/gcc/tree-ssa-loop-niter.c >>@@ -1959,11 +1959,6 @@ number_of_iterations_exit (struct loop *loop, >>edge exit, >> affine_iv iv0, iv1; >> bool safe; >> >>- safe = dominated_by_p (CDI_DOMINATORS, loop->latch, exit->src); >>- >>- if (every_iteration && !safe) >>-return false; >>- >> niter->assumptions = boolean_false_node; >> last = last_stmt (exit->src); >> if (!last) >>@@ -1972,6 +1967,11 @@ number_of_iterations_exit (struct loop *loop, >>edge exit, >> if (!stmt) >> return false; >> >>+ safe = dominated_by_p (CDI_DOMINATORS, loop->latch, exit->src); >>+ >>+ if (every_iteration && !safe) >>+return false; >>+ >> /* We want the condition for staying inside loop. */ >> code = gimple_cond_code (stmt); >> if (exit->flags & EDGE_TRUE_VALUE) >> >> > >
RE: [PATCH] Discard Scops for which entry==exit
Hi Tobias,
A test case (gcc/testsuite/gcc.dg/graphite/pr18792.c) came up when we removed
`graphite-scop-detection.c:limit_scops'.
The test case is a scop where entry==exit,
BB5 (*#) -> BB6 (#);
BB6 -> BB5;
In this case BB2 is out of the scop. This is basically an empty (infinite) loop
with no entry.
--
(gdb) p debug_loops(3)
loop_0 (header = 0, latch = 1, niter = )
{
bb_2 (preds = {bb_0 }, succs = {bb_8 bb_3 })
{
:
# VUSE <.MEM_7(D)>
_5 = *x_10(D)[3];
if (_5 < 0.0)
goto ;
else
goto ;
}
bb_3 (preds = {bb_2 }, succs = {bb_4 bb_7 })
{
:
if (_5>= 0.0)
goto ;
else
goto ;
}
bb_4 (preds = {bb_3 bb_8 }, succs = {bb_9 })
{
:
# .MEM_19 = PHI <.MEM_7(D)(3), .MEM_14(8)>
}
bb_9 (preds = {bb_4 }, succs = {bb_5 })
{
:
}
bb_7 (preds = {bb_3 }, succs = {bb_1 })
{
:
# VUSE <.MEM_7(D)>
return;
}
bb_8 (preds = {bb_2 }, succs = {bb_4 })
{
:
# .MEM_14 = VDEF <.MEM_7(D)>
*x_10(D)[3] = 0.0;
goto ;
}
loop_2 (header = 5, latch = 6, niter = scev_not_known)
{
bb_5 (preds = {bb_9 bb_6 }, succs = {bb_6 })
{
:
# .MEM_25 = PHI <.MEM_19(9), .MEM_25(6)>
}
bb_6 (preds = {bb_5 }, succs = {bb_5 })
{
:
goto ;
}
}
}
digraph all {
0 [label=<
0
>, shape=box, style="setlinewidth(0)"]
2 [label=<
2
>, shape=box, style="setlinewidth(0)"]
3 [label=<
3
>, shape=box, style="setlinewidth(0)"]
4 [label=<
4
>, shape=box, style="setlinewidth(0)"]
9 [label=<
9
>, shape=box, style="setlinewidth(0)"]
5 [label=<
5*#
>, shape=box, style="setlinewidth(0)"]
6 [label=<
6
>, shape=box, style="setlinewidth(0)"]
7 [label=<
7
>, shape=box, style="setlinewidth(0)"]
8 [label=<
8
>, shape=box, style="setlinewidth(0)"]
1 [label=<
1
>, shape=box, style="setlinewidth(0)"]
0 -> 2;
2 -> 8;
2 -> 3;
3 -> 4;
3 -> 7;
4 -> 9;
9 -> 5;
5 -> 6;
6 -> 5;
7 -> 1;
8 -> 4;
}
-Aditya
> Date: Tue, 30 Jun 2015 08:11:01 +0200
> From: tob...@grosser.es
> To: seb...@gmail.com; hiradi...@msn.com
> CC: gcc-patches@gcc.gnu.org
> Subject: Re: [PATCH] Discard Scops for which entry==exit
>
> On 06/30/2015 02:09 AM, Sebastian Pop wrote:
>> On Mon, Jun 29, 2015 at 3:04 PM, Aditya Kumar wrote:
>>> In this patch we discard the scops where entry and exit are the same BB.
>>> This is an effort to remove graphite-scop-detection.c:limit_scops.
>>> Removing the limit_scops function introduces correctness regressions.
>>> We are making relevant changes in incremental steps to fix those bugs,
>>> and finally we intend to remove limit_scops.
>>>
>>> 2015-06-29 Aditya Kumar
>>> Sebastian Pop
>>>
>>> * graphite-scop-detection.c (build_scops_1): Discard scops for which
>>> entry==exit
>>
>> Looks good to me.
>> Let's wait on comments from Tobi before pushing this patch.
>
> Hi Sebastian,
>
> the commit message should probably give a short reasoning why scops with
> entry == exit need to be discarded. I currently don't see why they would
> be incorrect/problematic (despite being possibly very small/empty).
>
> Tobias
RE: [PATCH] Discard Scops for which entry==exit
A test case (gcc/testsuite/gcc.dg/graphite/pr18792.c) came up when we removed
`graphite-scop-detection.c:limit_scops'.
The test case is a scop where entry==exit,
BB5 (*#) -> BB6 (#);
BB6 -> BB5;
In this case BB2 is out of the scop. This is basically an empty (infinite) loop.
2015-06-29 Aditya Kumar
Sebastian Pop
* graphite-scop-detection.c (build_scops_1): Discard scops for which
entry==exit
---
gcc/graphite-scop-detection.c | 9 -
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/gcc/graphite-scop-detection.c b/gcc/graphite-scop-detection.c
index e8ddecd..f57cc4a 100644
--- a/gcc/graphite-scop-detection.c
+++ b/gcc/graphite-scop-detection.c
@@ -810,7 +810,14 @@ build_scops_1 (basic_block current, loop_p outermost_loop,
{
open_scop.exit = sinfo.exit;
gcc_assert (open_scop.exit);
- scops->safe_push (open_scop);
+ if (open_scop.entry != open_scop.exit)
+ scops->safe_push (open_scop);
+ else
+ {
+ sinfo.difficult = true;
+ sinfo.exits = false;
+ sinfo.exit = NULL;
+ }
}
result.exit = sinfo.exit;
--
2.1.0.243.g30d45f7
> Date: Thu, 2 Jul 2015 09:53:25 +0200
> From: tob...@grosser.es
> To: hiradi...@msn.com; seb...@gmail.com
> CC: gcc-patches@gcc.gnu.org
> Subject: Re: [PATCH] Discard Scops for which entry==exit
>
> On 06/30/2015 05:47 PM, Aditya K wrote:
>> Hi Tobias,
>> A test case (gcc/testsuite/gcc.dg/graphite/pr18792.c) came up when we
>> removed `graphite-scop-detection.c:limit_scops'.
>> The test case is a scop where entry==exit,
>>
>> BB5 (*#) -> BB6 (#);
>> BB6 -> BB5;
>>
>> In this case BB2 is out of the scop. This is basically an empty (infinite)
>> loop with no entr
>
> OK, maybe mention this in the commit message.
>
>
> Best,
> Tobias
>
Re: [PATCH] improve string find algorithm
> Could you try the corrected patch on your benchmarks? For the test-case you gave there is a regression. Benchmark Time CPU Iterations --- Without the patch: BM_StringRegression 81 ns 81 ns 8503740 With the patch: BM_StringRegression 109 ns 109 ns 6346500 The real advantage is when there are fewer matches as seen in BM_StringFindNoMatch. The code for the benchmark can be found in https://github.com/llvm-mirror/libcxx/blob/master/benchmarks/string.bench.cpp However, I have written an independent std-benchmark that can be used just by exporting the CC, CXX, LD_LIBRARY_FLAGS: https://github.com/hiraditya/std-benchmark Following are the results: -- Without the patch: Run on (8 X 3403.85 MHz CPU s) 2017-01-06 15:47:30 ***WARNING*** CPU scaling is enabled, the benchmark real time measurements may be noisy and will incur extra overhead. Benchmark Time CPU Iterations --- BM_StringFindNoMatch/10 6 ns 6 ns 114499418 BM_StringFindNoMatch/64 34 ns 34 ns 20578576 BM_StringFindNoMatch/512 222 ns 222 ns 3136787 BM_StringFindNoMatch/4096 1728 ns 1729 ns 401913 BM_StringFindNoMatch/32768 13679 ns 13684 ns 50680 BM_StringFindNoMatch/131072 54570 ns 54591 ns 12506 BM_StringFindAllMatch/1 4 ns 4 ns 180640260 BM_StringFindAllMatch/8 6 ns 6 ns 119682220 BM_StringFindAllMatch/64 7 ns 7 ns 97679753 BM_StringFindAllMatch/512 19 ns 19 ns 36035174 BM_StringFindAllMatch/4096 92 ns 92 ns 7516363 BM_StringFindAllMatch/32768 849 ns 849 ns 809284 BM_StringFindAllMatch/131072 3610 ns 3611 ns 193894 BM_StringFindMatch1/1 27273 ns 27283 ns 25579 BM_StringFindMatch1/8 27289 ns 27300 ns 25516 BM_StringFindMatch1/64 27297 ns 27307 ns 25561 BM_StringFindMatch1/512 27303 ns 27314 ns 25579 BM_StringFindMatch1/4096 27488 ns 27499 ns 25366 BM_StringFindMatch1/32768 28157 ns 28168 ns 24750 BM_StringFindMatch2/1 27273 ns 27284 ns 25562 BM_StringFindMatch2/8 27296 ns 27306 ns 2 BM_StringFindMatch2/64 27312 ns 27323 ns 25549 BM_StringFindMatch2/512 27327 ns 27338 ns 25558 BM_StringFindMatch2/4096 27513 ns 27524 ns 25349 BM_StringFindMatch2/32768 28161 ns 28172 ns 24788 BM_StringRegression 81 ns 81 ns 8503740 With the patch Run on (8 X 1071.8 MHz CPU s) 2017-01-06 16:06:29 ***WARNING*** CPU scaling is enabled, the benchmark real time measurements may be noisy and will incur extra overhead. Benchmark Time CPU Iterations --- BM_StringFindNoMatch/10 6 ns 6 ns 121302159 BM_StringFindNoMatch/64 7 ns 7 ns 102003502 BM_StringFindNoMatch/512 15 ns 15 ns 44820639 BM_StringFindNoMatch/4096 77 ns 77 ns 9016958 BM_StringFindNoMatch/32768 555 ns 555 ns 1227219 BM_StringFindNoMatch/131072 2688 ns 2689 ns 259488 BM_StringFindAllMatch/1 8 ns 8 ns 85893410 BM_StringFindAllMatch/8 9 ns 9 ns 80811804 BM_StringFindAllMatch/64 9 ns 9 ns 74237599 BM_StringFindAllMatch/512 23 ns 23 ns 31163379 BM_StringFindAllMatch/4096 94 ns 94 ns 7317385 BM_StringFindAllMatch/32768 847 ns 848 ns 803901 BM_StringFindAllMatch/131072 3551 ns 3552 ns 196844 BM_StringFindMatch1/1 1337 ns 1337 ns 518042 BM_StringFindMatch1/8 1338 ns 1338 ns 519431 BM_StringFindMatch1/64 1340 ns 1341 ns 513974 BM_StringFindMatch1/512 1355 ns 1356 ns 511857 BM_StringFindMatch1/4096 1489 ns 1489 ns 465629 BM_StringFindMatch1/32768 2203 ns 2204 ns 316044 BM_StringFindMatch2/1 1337 ns 1338 ns 519057 BM_StringFindMatch2/8 1337 ns 1337 ns 517745 BM_StringFindMatch2/64 1347 ns 1347 ns 514813 BM_StringFindMatch2/512 1362 ns 1363 ns 507408 BM_StringFindM
Re: [PATCH] improve string find algorithm
Thanks, -Aditya From: Jonathan Wakely Sent: Monday, January 9, 2017 6:33 AM To: Aditya K Cc: Aditya Kumar; Sebastian Pop; libstd...@gcc.gnu.org; gcc-patches@gcc.gnu.org Subject: Re: [PATCH] improve string find algorithm On 06/01/17 22:19 +, Aditya K wrote: >> Could you try the corrected patch on your benchmarks? > >For the test-case you gave there is a regression. > >Benchmark >Time CPU Iterations >--- >Without the patch: BM_StringRegression 81 ns 81 ns >8503740 >With the patch: BM_StringRegression 109 ns 109 ns >6346500 > > >The real advantage is when there are fewer matches as seen in >BM_StringFindNoMatch. The code for the benchmark can be found in >https://github.com/llvm-mirror/libcxx/blob/master/benchmarks/string.bench.cpp llvm-mirror/libcxx github.com Mirror of official libcxx git repository located at http://llvm.org/git/libcxx. Updated every five minutes. >However, I have written an independent std-benchmark that can be used just by >exporting the CC, CXX, LD_LIBRARY_FLAGS: >https://github.com/hiraditya/std-benchmark hiraditya/std-benchmark github.com std-benchmark - A benchmark for standard libraries I think the large improvements are worth the smaller regression, so I'm committing the patch I sent last week. >Following are the results: >-- >Without the patch: > > >Run on (8 X 3403.85 MHz CPU s) >2017-01-06 15:47:30 >***WARNING*** CPU scaling is enabled, the benchmark real time measurements may >be noisy and will incur extra overhead. >Benchmark Time CPU Iterations >--- >BM_StringFindNoMatch/10 6 ns 6 ns 114499418 >BM_StringFindNoMatch/64 34 ns 34 ns 20578576 >BM_StringFindNoMatch/512 222 ns 222 ns 3136787 >BM_StringFindNoMatch/4096 1728 ns 1729 ns 401913 >BM_StringFindNoMatch/32768 13679 ns 13684 ns 50680 >BM_StringFindNoMatch/131072 54570 ns 54591 ns 12506 >BM_StringFindAllMatch/1 4 ns 4 ns 180640260 >BM_StringFindAllMatch/8 6 ns 6 ns 119682220 >BM_StringFindAllMatch/64 7 ns 7 ns 97679753 >BM_StringFindAllMatch/512 19 ns 19 ns 36035174 >BM_StringFindAllMatch/4096 92 ns 92 ns 7516363 >BM_StringFindAllMatch/32768 849 ns 849 ns 809284 >BM_StringFindAllMatch/131072 3610 ns 3611 ns 193894 >BM_StringFindMatch1/1 27273 ns 27283 ns 25579 >BM_StringFindMatch1/8 27289 ns 27300 ns 25516 >BM_StringFindMatch1/64 27297 ns 27307 ns 25561 >BM_StringFindMatch1/512 27303 ns 27314 ns 25579 >BM_StringFindMatch1/4096 27488 ns 27499 ns 25366 >BM_StringFindMatch1/32768 28157 ns 28168 ns 24750 >BM_StringFindMatch2/1 27273 ns 27284 ns 25562 >BM_StringFindMatch2/8 27296 ns 27306 ns 2 >BM_StringFindMatch2/64 27312 ns 27323 ns 25549 >BM_StringFindMatch2/512 27327 ns 27338 ns 25558 >BM_StringFindMatch2/4096 27513 ns 27524 ns 25349 >BM_StringFindMatch2/32768 28161 ns 28172 ns 24788 >BM_StringRegression 81 ns 81 ns 8503740 > > > >With the patch > > >Run on (8 X 1071.8 MHz CPU s) >2017-01-06 16:06:29 >***WARNING*** CPU scaling is enabled, the benchmark real time measurements may >be noisy and will incur extra overhead. >Benchmark Time CPU Iterations >--- >BM_StringFindNoMatch/10 6 ns 6 ns 121302159 >BM_StringFindNoMatch/64 7 ns 7 ns 102003502 >BM_StringFindNoMatch/512 15 ns 15 ns 44820639 >BM_StringFindNoMatch/4096 77 ns 77 ns 9016958 >BM_StringFindNoMatch/32768 555 ns 555 ns 1227219 >BM_StringFindNoMatch/131072 2688 ns 2689 ns 259488 >BM_StringFindAllMatch/1 8 ns 8 ns 85893410 >BM_StringFindAllMatch/8 9 ns 9 ns 80811804 >BM_StringFindAllMatch/64 9 ns 9 ns 74237599 >BM_StringFindAllMatch
[PATCH] Added noexcept on constructors
---
libstdc++-v3/ChangeLog | 3 +++
libstdc++-v3/src/c++11/shared_ptr.cc | 4 ++--
2 files changed, 5 insertions(+), 2 deletions(-)
diff --git a/libstdc++-v3/ChangeLog b/libstdc++-v3/ChangeLog
index 08d9229..18924c4 100644
--- a/libstdc++-v3/ChangeLog
+++ b/libstdc++-v3/ChangeLog
@@ -1,3 +1,6 @@
+2016-12-03 Aditya Kumar
+ * src/c++11/shared_ptr.cc: Added noexcept on constructors.
+
2016-12-01 David Edelsohn
* testsuite/26_numerics/headers/cmath/hypot.cc: XFAIL on AIX.
diff --git a/libstdc++-v3/src/c++11/shared_ptr.cc
b/libstdc++-v3/src/c++11/shared_ptr.cc
index 9028040..b4addd0 100644
--- a/libstdc++-v3/src/c++11/shared_ptr.cc
+++ b/libstdc++-v3/src/c++11/shared_ptr.cc
@@ -56,7 +56,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
{ return _Hash_impl::hash(addr) & __gnu_internal::mask; }
}
- _Sp_locker::_Sp_locker(const void* p)
+ _Sp_locker::_Sp_locker(const void* p) noexcept
{
if (__gthread_active_p())
{
@@ -67,7 +67,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
_M_key1 = _M_key2 = __gnu_internal::invalid;
}
- _Sp_locker::_Sp_locker(const void* p1, const void* p2)
+ _Sp_locker::_Sp_locker(const void* p1, const void* p2) noexcept
{
if (__gthread_active_p())
{
--
2.5.0
Re: [PATCH] improve string find algorithm
Ping. From: Aditya Kumar Sent: Wednesday, December 7, 2016 11:46 AM To: libstd...@gcc.gnu.org Cc: gcc-patches@gcc.gnu.org; hiradi...@msn.com; Aditya Kumar Subject: [PATCH] improve string find algorithm Here is an improved version of basic_string::find. The idea is to split the string find in two parts: 1. search for the first match by using traits_type::find (this gets converted to memchr for x86) 2. see if there is a match (this gets converted to memcmp for x86) Passes bootstrap on x86-64. The patch results in good improvements on a synthetic test case I wrote using the google-benchmark. following are the results. Branch: master without patch $ ./bin/string.libcxx.out Run on (24 X 1899.12 MHz CPU s) 2016-12-06 16:41:55 ***WARNING*** CPU scaling is enabled, the benchmark real time measurements may be noisy and will incur extra overhead. Benchmark Time CPU Iterations - BM_StringFindNoMatch/10 8 ns 8 ns 81880927 BM_StringFindNoMatch/64 52 ns 52 ns 13235018 BM_StringFindNoMatch/512 355 ns 355 ns 1962488 BM_StringFindNoMatch/4k 2769 ns 2772 ns 249090 BM_StringFindNoMatch/32k 22598 ns 22619 ns 30984 BM_StringFindNoMatch/128k 89745 ns 89830 ns 7996 BM_StringFindAllMatch/1 7 ns 7 ns 102893835 BM_StringFindAllMatch/8 9 ns 9 ns 75403364 BM_StringFindAllMatch/64 12 ns 12 ns 60766893 BM_StringFindAllMatch/512 31 ns 31 ns 23163999 BM_StringFindAllMatch/4k 141 ns 141 ns 4980386 BM_StringFindAllMatch/32k 1402 ns 1403 ns 483581 BM_StringFindAllMatch/128k 5604 ns 5609 ns 126123 BM_StringFindMatch1/1 44430 ns 44473 ns 15804 BM_StringFindMatch1/8 44315 ns 44357 ns 15741 BM_StringFindMatch1/64 44689 ns 44731 ns 15712 BM_StringFindMatch1/512 44247 ns 44290 ns 15724 BM_StringFindMatch1/4k 45010 ns 45053 ns 15678 BM_StringFindMatch1/32k 45717 ns 45761 ns 15278 BM_StringFindMatch2/1 44307 ns 44349 ns 15730 BM_StringFindMatch2/8 44631 ns 44674 ns 15721 BM_StringFindMatch2/64 44300 ns 44342 ns 15750 BM_StringFindMatch2/512 44239 ns 44281 ns 15713 BM_StringFindMatch2/4k 44886 ns 44928 ns 15787 Branch: master with patch $ ./bin/string.libcxx.out Run on (24 X 2892.28 MHz CPU s) 2016-12-06 18:51:38 ***WARNING*** CPU scaling is enabled, the benchmark real time measurements may be noisy and will incur extra overhead. Benchmark Time CPU Iterations - BM_StringFindNoMatch/10 11 ns 11 ns 63049677 BM_StringFindNoMatch/64 12 ns 12 ns 57259381 BM_StringFindNoMatch/512 27 ns 27 ns 25495432 BM_StringFindNoMatch/4k 130 ns 130 ns 5301301 BM_StringFindNoMatch/32k 858 ns 859 ns 824048 BM_StringFindNoMatch/128k 4091 ns 4095 ns 171493 BM_StringFindAllMatch/1 14 ns 14 ns 53023977 BM_StringFindAllMatch/8 14 ns 14 ns 51516536 BM_StringFindAllMatch/64 17 ns 17 ns 40992668 BM_StringFindAllMatch/512 37 ns 37 ns 18503267 BM_StringFindAllMatch/4k 153 ns 153 ns 4494458 BM_StringFindAllMatch/32k 1460 ns 1461 ns 483380 BM_StringFindAllMatch/128k 5801 ns 5806 ns 120680 BM_StringFindMatch1/1 2062 ns 2064 ns 333144 BM_StringFindMatch1/8 2057 ns 2059 ns 335496 BM_StringFindMatch1/64 2083 ns 2085 ns 341469 BM_StringFindMatch1/512 2134 ns 2136 ns 336880 BM_StringFindMatch1/4k 2309 ns 2312 ns 308745 BM_StringFindMatch1/32k 3413 ns 3417 ns 208206 BM_StringFindMatch2/1 2053 ns 2055 ns 341523 BM_StringFindMatch2/8 2061 ns 2063 ns 343999 BM_StringFindMatch2/64 2075 ns 2077 ns 338479 BM_StringFindMatch2/512 2102 ns 2104 ns 332276 BM_StringFindMatch2/4k 2286 ns 2288 ns 300416 BM_StringFindMatch2/32k 3385 ns 3388 ns 204158 ChangeLog: 2016-12-07 Aditya Kumar * include/bits/basic_string.tcc(find(const _CharT* __s, size_type __pos, size_type __n) const)): Improve the algorithm --- libstdc++-v3/include/bits/basic_string.tcc | 31 ++ 1 file changed, 23 insertions(+), 8 deletions(-) diff --git a/libstdc++-v3/include/bits/basic_string.tcc b/libstdc++-v3/include/bits/basic_string.t
RE: [PATCH] [graphite] Constrain only on INTEGER_TYPE
> Date: Thu, 13 Aug 2015 12:02:43 +0200 > Subject: Re: [PATCH] [graphite] Constrain only on INTEGER_TYPE > From: richard.guent...@gmail.com > To: tob...@grosser.es > CC: hiradi...@msn.com; gcc-patches@gcc.gnu.org; s@samsung.com; > seb...@gmail.com > > On Wed, Aug 12, 2015 at 10:41 PM, Tobias Grosser wrote: >> On 08/12/2015 10:33 PM, Aditya Kumar wrote: >>> >>> Passes bootstrap, no regressions. >>> >>> With this patch gcc bootstraps with graphite. >>> make BOOT_CFLAGS="-g -O2 -fgraphite-identity -floop-interchange >>> -floop-block" >> >> >> LGTM, but please use a longer sentence to explain what you do. > > As the middle-end generally freely exchanges INTEGER_TYPE > ENUMERAL_TYPE and BOOLEAN_TYPE > you want to use INTEGRAL_TYPE_P here. > Thanks. I tried INTEGRAL_TYPE_P, and that fails bootstrap. After a little bit of debugging I figured out that adding ENUMERAL_TYPE causes the failure (miscompile) in tree-vect-data-refs.c. I can add INTEGER_TYPE and BOOLEAN_TYPE (bootstrap passes with these). But that would be inconsistent with the type-checks at other places in graphite-*.c. Currently, we are only checking for INTEGER_TYPE at other places. -Aditya > RIchard. > >> Tobias
RE: [PATCH] [graphite] Constrain only on INTEGER_TYPE
> Date: Fri, 14 Aug 2015 09:31:32 +0200
> Subject: Re: [PATCH] [graphite] Constrain only on INTEGER_TYPE
> From: richard.guent...@gmail.com
> To: hiradi...@msn.com
> CC: tob...@grosser.es; gcc-patches@gcc.gnu.org; s@samsung.com;
> seb...@gmail.com
>
> On Thu, Aug 13, 2015 at 7:56 PM, Aditya K wrote:
>>
>>
>>> Date: Thu, 13 Aug 2015 12:02:43 +0200
>>> Subject: Re: [PATCH] [graphite] Constrain only on INTEGER_TYPE
>>> From: richard.guent...@gmail.com
>>> To: tob...@grosser.es
>>> CC: hiradi...@msn.com; gcc-patches@gcc.gnu.org; s@samsung.com;
>>> seb...@gmail.com
>>>
>>> On Wed, Aug 12, 2015 at 10:41 PM, Tobias Grosser
>>> wrote:
>>>> On 08/12/2015 10:33 PM, Aditya Kumar wrote:
>>>>>
>>>>> Passes bootstrap, no regressions.
>>>>>
>>>>> With this patch gcc bootstraps with graphite.
>>>>> make BOOT_CFLAGS="-g -O2 -fgraphite-identity -floop-interchange
>>>>> -floop-block"
>>>>
>>>>
>>>> LGTM, but please use a longer sentence to explain what you do.
>>>
>>> As the middle-end generally freely exchanges INTEGER_TYPE
>>> ENUMERAL_TYPE and BOOLEAN_TYPE
>>> you want to use INTEGRAL_TYPE_P here.
>>>
>>
>> Thanks.
>> I tried INTEGRAL_TYPE_P, and that fails bootstrap. After a little bit of
>> debugging I figured out that it is
>> ENUMERAL_TYPE that causes the failure (miscompile) in tree-vect-data-refs.c.
>> I can add INTEGER_TYPE and BOOLEAN_TYPE (bootstrap passes with these). But
>> that would be inconsistent with the type-checks at other places in
>> graphite-*.c.
>> Currently, we are only checking for INTEGER_TYPE at other places.
>
> This is weird - the code you replace did allow both ENUMERAL_TYPE and
> BOOLEAN_TYPE.
>
> my suggestion was
>
> /* We can not handle REAL_TYPE. Failed for pr39260. */
> - || TREE_CODE (TREE_TYPE (op)) == REAL_TYPE)
> + || ! INTEGRAL_TYPE_P (TREE_TYPE (op))
>
> you also need to adjust the comment btw.
This is exactly what I did and that resulted in miscompile. Then out of
curiosity I tried to debug by putting
/* We can not handle REAL_TYPE. Failed for pr39260. */
- || TREE_CODE (TREE_TYPE (op)) == REAL_TYPE)
+ || TREE_CODE (TREE_TYPE (op)) != INTEGER_TYPE)
+ || TREE_CODE (TREE_TYPE (op)) != BOOLEAN_TYPE))
And that passed bootstrap.
Update patch:
Passes bootstrap, no regressions.
With this patch gcc bootstraps with graphite.
make BOOT_CFLAGS="-g -O2 -fgraphite-identity -floop-interchange -floop-block"
gcc/ChangeLog:
2015-08-12 Aditya Kumar
* graphite-scop-detection.c (stmt_simple_for_scop_p):
Constrain only on INTEGER_TYPE
---
gcc/graphite-scop-detection.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/gcc/graphite-scop-detection.c b/gcc/graphite-scop-detection.c
index fb7247e..0f02a71 100644
--- a/gcc/graphite-scop-detection.c
+++ b/gcc/graphite-scop-detection.c
@@ -409,8 +409,8 @@ stmt_simple_for_scop_p (basic_block scop_entry, loop_p
outermost_loop,
{
tree op = gimple_op (stmt, i);
if (!graphite_can_represent_expr (scop_entry, loop, op)
- /* We can not handle REAL_TYPE. Failed for pr39260. */
- || TREE_CODE (TREE_TYPE (op)) == REAL_TYPE)
+ /* We can only constrain on integer type. */
+ || (TREE_CODE (TREE_TYPE (op)) != INTEGER_TYPE))
{
if (dump_file && (dump_flags & TDF_DETAILS))
{
--
2.1.4
>
> Richard.
>
>> -Aditya
>>
>>
>>> RIchard.
>>>
>>>> Tobias
RE: [PATCH] [graphite] Constrain only on INTEGER_TYPE
> Date: Fri, 14 Aug 2015 11:47:05 +0200
> Subject: Re: [PATCH] [graphite] Constrain only on INTEGER_TYPE
> From: richard.guent...@gmail.com
> To: hiradi...@msn.com
> CC: tob...@grosser.es; gcc-patches@gcc.gnu.org; s@samsung.com;
> seb...@gmail.com
>
> On Fri, Aug 14, 2015 at 11:25 AM, Aditya K wrote:
>>
>>
>>
>>> Date: Fri, 14 Aug 2015 09:31:32 +0200
>>> Subject: Re: [PATCH] [graphite] Constrain only on INTEGER_TYPE
>>> From: richard.guent...@gmail.com
>>> To: hiradi...@msn.com
>>> CC: tob...@grosser.es; gcc-patches@gcc.gnu.org; s....@samsung.com;
>>> seb...@gmail.com
>>>
>>> On Thu, Aug 13, 2015 at 7:56 PM, Aditya K wrote:
>>>>
>>>>
>>>>> Date: Thu, 13 Aug 2015 12:02:43 +0200
>>>>> Subject: Re: [PATCH] [graphite] Constrain only on INTEGER_TYPE
>>>>> From: richard.guent...@gmail.com
>>>>> To: tob...@grosser.es
>>>>> CC: hiradi...@msn.com; gcc-patches@gcc.gnu.org; s@samsung.com;
>>>>> seb...@gmail.com
>>>>>
>>>>> On Wed, Aug 12, 2015 at 10:41 PM, Tobias Grosser
>>>>> wrote:
>>>>>> On 08/12/2015 10:33 PM, Aditya Kumar wrote:
>>>>>>>
>>>>>>> Passes bootstrap, no regressions.
>>>>>>>
>>>>>>> With this patch gcc bootstraps with graphite.
>>>>>>> make BOOT_CFLAGS="-g -O2 -fgraphite-identity -floop-interchange
>>>>>>> -floop-block"
>>>>>>
>>>>>>
>>>>>> LGTM, but please use a longer sentence to explain what you do.
>>>>>
>>>>> As the middle-end generally freely exchanges INTEGER_TYPE
>>>>> ENUMERAL_TYPE and BOOLEAN_TYPE
>>>>> you want to use INTEGRAL_TYPE_P here.
>>>>>
>>>>
>>>> Thanks.
>>>> I tried INTEGRAL_TYPE_P, and that fails bootstrap. After a little bit of
>>>> debugging I figured out that it is
>>>> ENUMERAL_TYPE that causes the failure (miscompile) in
>>>> tree-vect-data-refs.c.
>>>> I can add INTEGER_TYPE and BOOLEAN_TYPE (bootstrap passes with these). But
>>>> that would be inconsistent with the type-checks at other places in
>>>> graphite-*.c.
>>>> Currently, we are only checking for INTEGER_TYPE at other places.
>>>
>>> This is weird - the code you replace did allow both ENUMERAL_TYPE and
>>> BOOLEAN_TYPE.
>>>
>>> my suggestion was
>>>
>>> /* We can not handle REAL_TYPE. Failed for pr39260. */
>>> - || TREE_CODE (TREE_TYPE (op)) == REAL_TYPE)
>>> + || ! INTEGRAL_TYPE_P (TREE_TYPE (op))
>>>
>>> you also need to adjust the comment btw.
>>
>> This is exactly what I did and that resulted in miscompile. Then out of
>> curiosity I tried to debug by putting
>> /* We can not handle REAL_TYPE. Failed for pr39260. */
>> - || TREE_CODE (TREE_TYPE (op)) == REAL_TYPE)
>> + || TREE_CODE (TREE_TYPE (op)) != INTEGER_TYPE)
>> + || TREE_CODE (TREE_TYPE (op)) != BOOLEAN_TYPE))
>>
>
> well, each op is either != INTEGER or != BOOLEAN ... this is equal to
> doing || true
My bad, thanks for correction.
I tried this way (using BOOLEAN_TYPE) and this also failed bootstrap.
@@ -410,7 +410,8 @@ stmt_simple_for_scop_p (basic_block scop_entry, loop_p
outermost_loop,
tree op = gimple_op (stmt, i);
if (!graphite_can_represent_expr (scop_entry, loop, op)
/* We can not handle REAL_TYPE. Failed for pr39260. */
- || TREE_CODE (TREE_TYPE (op)) == REAL_TYPE)
+ || ((TREE_CODE (TREE_TYPE (op)) != INTEGER_TYPE)
+ && (TREE_CODE (TREE_TYPE (op)) != BOOLEAN_TYPE)))
{
if (dump_file && (dump_flags & TDF_DETAILS))
While, using ENUMERAL_TYPE passed bootstrap.
@@ -410,7 +410,8 @@ stmt_simple_for_scop_p (basic_block scop_entry, loop_p
outermost_loop,
tree op = gimple_op (stmt, i);
if (!graphite_can_represent_expr (scop_entry, loop, op)
/* We can not handle REAL_TYPE. Failed for pr39260. */
- || TREE_CODE (TREE_TYPE (op)) == REAL_TYPE)
+ || ((TREE_CODE (TREE_TYPE (op)) != INTEGER_TYPE)
+ && (TREE_CODE (TREE_TYPE (op)) != ENUMERAL_TYPE)))
{
if (dump_file && (dump_flags & TDF_DETAILS))
Where,
BOOT_CFLAGS="-g -O2 -fgraphite-identity -floop-block -floop-interchange&
RE: [PATCH] [graphite] Constrain only on INTEGER_TYPE
> Subject: RE: [PATCH] [graphite] Constrain only on INTEGER_TYPE > From: richard.guent...@gmail.com > Date: Fri, 14 Aug 2015 16:47:41 +0200 > To: hiradi...@msn.com > CC: tob...@grosser.es; gcc-patches@gcc.gnu.org; s@samsung.com; > seb...@gmail.com > > On August 14, 2015 4:31:23 PM GMT+02:00, Aditya K wrote: >> >> >> >>> Date: Fri, 14 Aug 2015 11:47:05 +0200 >>> Subject: Re: [PATCH] [graphite] Constrain only on INTEGER_TYPE >>> From: richard.guent...@gmail.com >>> To: hiradi...@msn.com >>> CC: tob...@grosser.es; gcc-patches@gcc.gnu.org; s@samsung.com; >>seb...@gmail.com >>> >>> On Fri, Aug 14, 2015 at 11:25 AM, Aditya K wrote: >>>> >>>> >>>> >>>>> Date: Fri, 14 Aug 2015 09:31:32 +0200 >>>>> Subject: Re: [PATCH] [graphite] Constrain only on INTEGER_TYPE >>>>> From: richard.guent...@gmail.com >>>>> To: hiradi...@msn.com >>>>> CC: tob...@grosser.es; gcc-patches@gcc.gnu.org; s@samsung.com; >>seb...@gmail.com >>>>> >>>>> On Thu, Aug 13, 2015 at 7:56 PM, Aditya K >>wrote: >>>>>> >>>>>> >>>>>>> Date: Thu, 13 Aug 2015 12:02:43 +0200 >>>>>>> Subject: Re: [PATCH] [graphite] Constrain only on INTEGER_TYPE >>>>>>> From: richard.guent...@gmail.com >>>>>>> To: tob...@grosser.es >>>>>>> CC: hiradi...@msn.com; gcc-patches@gcc.gnu.org; >>s@samsung.com; >>>>>>> seb...@gmail.com >>>>>>> >>>>>>> On Wed, Aug 12, 2015 at 10:41 PM, Tobias Grosser >> >>>>>>> wrote: >>>>>>>> On 08/12/2015 10:33 PM, Aditya Kumar wrote: >>>>>>>>> >>>>>>>>> Passes bootstrap, no regressions. >>>>>>>>> >>>>>>>>> With this patch gcc bootstraps with graphite. >>>>>>>>> make BOOT_CFLAGS="-g -O2 -fgraphite-identity -floop-interchange >>>>>>>>> -floop-block" >>>>>>>> >>>>>>>> >>>>>>>> LGTM, but please use a longer sentence to explain what you do. >>>>>>> >>>>>>> As the middle-end generally freely exchanges INTEGER_TYPE >>>>>>> ENUMERAL_TYPE and BOOLEAN_TYPE >>>>>>> you want to use INTEGRAL_TYPE_P here. >>>>>>> >>>>>> >>>>>> Thanks. >>>>>> I tried INTEGRAL_TYPE_P, and that fails bootstrap. After a little >>bit of >>>>>> debugging I figured out that it is >>>>>> ENUMERAL_TYPE that causes the failure (miscompile) in >>tree-vect-data-refs.c. >>>>>> I can add INTEGER_TYPE and BOOLEAN_TYPE (bootstrap passes with >>these). But >>>>>> that would be inconsistent with the type-checks at other places in >>>>>> graphite-*.c. >>>>>> Currently, we are only checking for INTEGER_TYPE at other places. >>>>> >>>>> This is weird - the code you replace did allow both ENUMERAL_TYPE >>and >>>>> BOOLEAN_TYPE. >>>>> >>>>> my suggestion was >>>>> >>>>> /* We can not handle REAL_TYPE. Failed for pr39260. */ >>>>> - || TREE_CODE (TREE_TYPE (op)) == REAL_TYPE) >>>>> + || ! INTEGRAL_TYPE_P (TREE_TYPE (op)) >>>>> >>>>> you also need to adjust the comment btw. >>>> >>>> This is exactly what I did and that resulted in miscompile. Then out >>of curiosity I tried to debug by putting >>>> /* We can not handle REAL_TYPE. Failed for pr39260. */ >>>> - || TREE_CODE (TREE_TYPE (op)) == REAL_TYPE) >>>> + || TREE_CODE (TREE_TYPE (op)) != INTEGER_TYPE) >>>> + || TREE_CODE (TREE_TYPE (op)) != BOOLEAN_TYPE)) >>>> >>> >>> well, each op is either != INTEGER or != BOOLEAN ... this is equal to >>> doing || true >> >> >>My bad, thanks for correction. >>I tried this way (using BOOLEAN_TYPE) and this also failed bootstrap. > > As previously said the REAL_TYPE condition also allowed BOOLEAN_TYPE and thus > also should have failed to bootstrap then(?) > Yet it did fail before. I started working on this patch because gcc was failing bootstrap with BOOT_CFLAGS="-g -O2 -fgraphite-identity -floop-block -floop-interchange" -Aditya > Richard. > >
[PATCH] libstdc++: Use string::push_back instead of string::operator+=
>From db5036e40ed7ac43b66ca74c44ec8d0bdc934b07 Mon Sep 17 00:00:00 2001
From: AdityaK <1108430...@users.noreply.github.com>
Date: Sun, 29 Dec 2024 18:14:29 -0800
Subject: [PATCH] libstdc++: Use string::push_back instead of string::operator+=
operator+= returns string& which is ignored anyways.
---
libstdc++-v3/ChangeLog | 5 +
libstdc++-v3/include/bits/basic_string.tcc | 2 +-
2 files changed, 6 insertions(+), 1 deletion(-)
diff --git a/libstdc++-v3/ChangeLog b/libstdc++-v3/ChangeLog
index 9ab5eeb55a5..be90bfd47e8 100644
--- a/libstdc++-v3/ChangeLog
+++ b/libstdc++-v3/ChangeLog
@@ -1,3 +1,8 @@
+2024-12-29 Aditya Kumar
+ * include/bits/basic_string.tcc (getline): Use string::push_back
+ instead of string::operator+=
+
+
2024-12-29 Gerald Pfeifer
* doc/html/manual/profile_mode_diagnostics.html: Delete.
diff --git a/libstdc++-v3/include/bits/basic_string.tcc
b/libstdc++-v3/include/bits/basic_string.tcc
index caeddaf2f5b..ddb41c8e7e2 100644
--- a/libstdc++-v3/include/bits/basic_string.tcc
+++ b/libstdc++-v3/include/bits/basic_string.tcc
@@ -935,7 +935,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
&& !_Traits::eq_int_type(__c, __eof)
&& !_Traits::eq_int_type(__c, __idelim))
{
- __str += _Traits::to_char_type(__c);
+ __str.push_back(_Traits::to_char_type(__c));
++__extracted;
__c = __in.rdbuf()->snextc();
}
--
2.47.1.613.gc27f4b7a9f-goog
Re: [PATCH] libstdc++: Use string::push_back instead of string::operator+=
pinging in case this was missed.
From: Aditya K
Sent: Sunday, December 29, 2024 6:36 PM
To: gcc-patches@gcc.gnu.org ; libstd...@gcc.gnu.org
Cc: jwak...@redhat.com
Subject: [PATCH] libstdc++: Use string::push_back instead of string::operator+=
>From db5036e40ed7ac43b66ca74c44ec8d0bdc934b07 Mon Sep 17 00:00:00 2001
From: AdityaK <1108430...@users.noreply.github.com>
Date: Sun, 29 Dec 2024 18:14:29 -0800
Subject: [PATCH] libstdc++: Use string::push_back instead of string::operator+=
operator+= returns string& which is ignored anyways.
---
libstdc++-v3/ChangeLog | 5 +
libstdc++-v3/include/bits/basic_string.tcc | 2 +-
2 files changed, 6 insertions(+), 1 deletion(-)
diff --git a/libstdc++-v3/ChangeLog b/libstdc++-v3/ChangeLog
index 9ab5eeb55a5..be90bfd47e8 100644
--- a/libstdc++-v3/ChangeLog
+++ b/libstdc++-v3/ChangeLog
@@ -1,3 +1,8 @@
+2024-12-29 Aditya Kumar
+ * include/bits/basic_string.tcc (getline): Use string::push_back
+ instead of string::operator+=
+
+
2024-12-29 Gerald Pfeifer
* doc/html/manual/profile_mode_diagnostics.html: Delete.
diff --git a/libstdc++-v3/include/bits/basic_string.tcc
b/libstdc++-v3/include/bits/basic_string.tcc
index caeddaf2f5b..ddb41c8e7e2 100644
--- a/libstdc++-v3/include/bits/basic_string.tcc
+++ b/libstdc++-v3/include/bits/basic_string.tcc
@@ -935,7 +935,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
&& !_Traits::eq_int_type(__c, __eof)
&& !_Traits::eq_int_type(__c, __idelim))
{
- __str += _Traits::to_char_type(__c);
+ __str.push_back(_Traits::to_char_type(__c));
++__extracted;
__c = __in.rdbuf()->snextc();
}
--
2.47.1.613.gc27f4b7a9f-goog
Re: [PATCH] libstdc++: Use string::push_back instead of string::operator+=
>> From db5036e40ed7ac43b66ca74c44ec8d0bdc934b07 Mon Sep 17 00:00:00 2001
>> From: AdityaK
>> <1108430...@users.noreply.github.com<mailto:1108430...@users.noreply.github.com>>
>> Date: Sun, 29 Dec 2024 18:14:29 -0800
>> Subject: [PATCH] libstdc++: Use string::push_back instead of
>> string::operator+=
>>
>> operator+= returns string& which is ignored anyways.
>Why does this matter? The compiler can see that the return value isn't used.
>Using += seems more readable to me.
nvm, i see both produce the same code. sorry for the noise.
From: Aditya K
Sent: Tuesday, January 14, 2025 12:05 PM
To: gcc-patches@gcc.gnu.org ; libstd...@gcc.gnu.org
Cc: jwak...@redhat.com
Subject: Re: [PATCH] libstdc++: Use string::push_back instead of
string::operator+=
pinging in case this was missed.
From: Aditya K
Sent: Sunday, December 29, 2024 6:36 PM
To: gcc-patches@gcc.gnu.org ; libstd...@gcc.gnu.org
Cc: jwak...@redhat.com
Subject: [PATCH] libstdc++: Use string::push_back instead of string::operator+=
>From db5036e40ed7ac43b66ca74c44ec8d0bdc934b07 Mon Sep 17 00:00:00 2001
From: AdityaK <1108430...@users.noreply.github.com>
Date: Sun, 29 Dec 2024 18:14:29 -0800
Subject: [PATCH] libstdc++: Use string::push_back instead of string::operator+=
operator+= returns string& which is ignored anyways.
---
libstdc++-v3/ChangeLog | 5 +
libstdc++-v3/include/bits/basic_string.tcc | 2 +-
2 files changed, 6 insertions(+), 1 deletion(-)
diff --git a/libstdc++-v3/ChangeLog b/libstdc++-v3/ChangeLog
index 9ab5eeb55a5..be90bfd47e8 100644
--- a/libstdc++-v3/ChangeLog
+++ b/libstdc++-v3/ChangeLog
@@ -1,3 +1,8 @@
+2024-12-29 Aditya Kumar
+ * include/bits/basic_string.tcc (getline): Use string::push_back
+ instead of string::operator+=
+
+
2024-12-29 Gerald Pfeifer
* doc/html/manual/profile_mode_diagnostics.html: Delete.
diff --git a/libstdc++-v3/include/bits/basic_string.tcc
b/libstdc++-v3/include/bits/basic_string.tcc
index caeddaf2f5b..ddb41c8e7e2 100644
--- a/libstdc++-v3/include/bits/basic_string.tcc
+++ b/libstdc++-v3/include/bits/basic_string.tcc
@@ -935,7 +935,7 @@ _GLIBCXX_BEGIN_NAMESPACE_VERSION
&& !_Traits::eq_int_type(__c, __eof)
&& !_Traits::eq_int_type(__c, __idelim))
{
- __str += _Traits::to_char_type(__c);
+ __str.push_back(_Traits::to_char_type(__c));
++__extracted;
__c = __in.rdbuf()->snextc();
}
--
2.47.1.613.gc27f4b7a9f-goog
Add cold attribute to one time construction APIs
This would help compiler optimize local static objects. ``` commit e2f299679ddf56a6d6d71ea9d589cd76b2ca107b Author: Aditya Kumar <1894981+hiradi...@users.noreply.github.com> Date: Thu Aug 13 09:41:34 2020 -0700 Add cold attribute to one time construction APIs __cxa_guard_acquire is used for only one purpose, namely guarding local static variable initialization, and since that purpose is definitionally cold, it should be attributed as cold. Similarly for __cxa_guard_release and __cxa_guard_abort diff --git a/libstdc++-v3/include/bits/c++config b/libstdc++-v3/include/bits/c++config index b1fad59d4..359e955a7 100644 --- a/libstdc++-v3/include/bits/c++config +++ b/libstdc++-v3/include/bits/c++config @@ -39,20 +39,24 @@ // Macros for various attributes. // _GLIBCXX_PURE // _GLIBCXX_CONST // _GLIBCXX_NORETURN // _GLIBCXX_NOTHROW // _GLIBCXX_VISIBILITY #ifndef _GLIBCXX_PURE # define _GLIBCXX_PURE __attribute__ ((__pure__)) #endif +#ifndef _GLIBCXX_COLD +# define _GLIBCXX_COLD __attribute__ ((cold)) +#endif + #ifndef _GLIBCXX_CONST # define _GLIBCXX_CONST __attribute__ ((__const__)) #endif #ifndef _GLIBCXX_NORETURN # define _GLIBCXX_NORETURN __attribute__ ((__noreturn__)) #endif // See below for C++ #ifndef _GLIBCXX_NOTHROW diff --git a/libstdc++-v3/libsupc++/cxxabi.h b/libstdc++-v3/libsupc++/cxxabi.h index 000713ecd..24c1366e2 100644 --- a/libstdc++-v3/libsupc++/cxxabi.h +++ b/libstdc++-v3/libsupc++/cxxabi.h @@ -108,27 +108,27 @@ namespace __cxxabiv1 __cxa_vec_delete2(void* __array_address, size_t __element_size, size_t __padding_size, __cxa_cdtor_type __destructor, void (*__dealloc) (void*)); void __cxa_vec_delete3(void* __array_address, size_t __element_size, size_t __padding_size, __cxa_cdtor_type __destructor, void (*__dealloc) (void*, size_t)); int - __cxa_guard_acquire(__guard*); + __cxa_guard_acquire(__guard*) _GLIBCXX_COLD; void - __cxa_guard_release(__guard*) _GLIBCXX_NOTHROW; + __cxa_guard_release(__guard*) _GLIBCXX_NOTHROW _GLIBCXX_COLD; void - __cxa_guard_abort(__guard*) _GLIBCXX_NOTHROW; + __cxa_guard_abort(__guard*) _GLIBCXX_NOTHROW _GLIBCXX_COLD; // DSO destruction. int __cxa_atexit(void (*)(void*), void*, void*) _GLIBCXX_NOTHROW; void __cxa_finalize(void*); // TLS destruction. int ```
Re: Add cold attribute to one time construction APIs
Revised patch with _GLIBCXX_COLD added at the end. ``` commit 3dc9f9a8461b1c88e991ceb517e5fdd81f268d1e Author: Aditya Kumar <1894981+hiradi...@users.noreply.github.com> Date: Thu Aug 13 09:41:34 2020 -0700 Add cold attribute to one time construction APIs __cxa_guard_acquire is used for only one purpose, namely guarding local static variable initialization, and since that purpose is definitionally cold, it should be attributed as cold. Similarly for __cxa_guard_release and __cxa_guard_abort diff --git a/libstdc++-v3/include/bits/c++config b/libstdc++-v3/include/bits/c++config index b1fad59d4..f6f954eef 100644 --- a/libstdc++-v3/include/bits/c++config +++ b/libstdc++-v3/include/bits/c++config @@ -35,20 +35,21 @@ // The datestamp of the C++ library in compressed ISO date format. #define __GLIBCXX__ // Macros for various attributes. // _GLIBCXX_PURE // _GLIBCXX_CONST // _GLIBCXX_NORETURN // _GLIBCXX_NOTHROW // _GLIBCXX_VISIBILITY +// _GLIBCXX_COLD #ifndef _GLIBCXX_PURE # define _GLIBCXX_PURE __attribute__ ((__pure__)) #endif #ifndef _GLIBCXX_CONST # define _GLIBCXX_CONST __attribute__ ((__const__)) #endif #ifndef _GLIBCXX_NORETURN # define _GLIBCXX_NORETURN __attribute__ ((__noreturn__)) @@ -67,20 +68,24 @@ #define _GLIBCXX_HAVE_ATTRIBUTE_VISIBILITY #if _GLIBCXX_HAVE_ATTRIBUTE_VISIBILITY # define _GLIBCXX_VISIBILITY(V) __attribute__ ((__visibility__ (#V))) #else // If this is not supplied by the OS-specific or CPU-specific // headers included below, it will be defined to an empty default. # define _GLIBCXX_VISIBILITY(V) _GLIBCXX_PSEUDO_VISIBILITY(V) #endif +#ifndef _GLIBCXX_COLD +# define _GLIBCXX_COLD __attribute__ ((cold)) +#endif + // Macros for deprecated attributes. // _GLIBCXX_USE_DEPRECATED // _GLIBCXX_DEPRECATED // _GLIBCXX17_DEPRECATED // _GLIBCXX20_DEPRECATED( string-literal ) #ifndef _GLIBCXX_USE_DEPRECATED # define _GLIBCXX_USE_DEPRECATED 1 #endif #if defined(__DEPRECATED) && (__cplusplus >= 201103L) diff --git a/libstdc++-v3/libsupc++/cxxabi.h b/libstdc++-v3/libsupc++/cxxabi.h index 000713ecd..24c1366e2 100644 --- a/libstdc++-v3/libsupc++/cxxabi.h +++ b/libstdc++-v3/libsupc++/cxxabi.h @@ -108,27 +108,27 @@ namespace __cxxabiv1 __cxa_vec_delete2(void* __array_address, size_t __element_size, size_t __padding_size, __cxa_cdtor_type __destructor, void (*__dealloc) (void*)); void __cxa_vec_delete3(void* __array_address, size_t __element_size, size_t __padding_size, __cxa_cdtor_type __destructor, void (*__dealloc) (void*, size_t)); int - __cxa_guard_acquire(__guard*); + __cxa_guard_acquire(__guard*) _GLIBCXX_COLD; void - __cxa_guard_release(__guard*) _GLIBCXX_NOTHROW; + __cxa_guard_release(__guard*) _GLIBCXX_NOTHROW _GLIBCXX_COLD; void - __cxa_guard_abort(__guard*) _GLIBCXX_NOTHROW; + __cxa_guard_abort(__guard*) _GLIBCXX_NOTHROW _GLIBCXX_COLD; // DSO destruction. int __cxa_atexit(void (*)(void*), void*, void*) _GLIBCXX_NOTHROW; void __cxa_finalize(void*); // TLS destruction. int ``` From: Aditya K Sent: Thursday, August 13, 2020 10:47 AM To: Jeff Law via Gcc-patches ; jwakely@gmail.com Subject: Add cold attribute to one time construction APIs This would help compiler optimize local static objects. ``` commit e2f299679ddf56a6d6d71ea9d589cd76b2ca107b Author: Aditya Kumar <1894981+hiradi...@users.noreply.github.com> Date: Thu Aug 13 09:41:34 2020 -0700 Add cold attribute to one time construction APIs __cxa_guard_acquire is used for only one purpose, namely guarding local static variable initialization, and since that purpose is definitionally cold, it should be attributed as cold. Similarly for __cxa_guard_release and __cxa_guard_abort diff --git a/libstdc++-v3/include/bits/c++config b/libstdc++-v3/include/bits/c++config index b1fad59d4..359e955a7 100644 --- a/libstdc++-v3/include/bits/c++config +++ b/libstdc++-v3/include/bits/c++config @@ -39,20 +39,24 @@ // Macros for various attributes. // _GLIBCXX_PURE // _GLIBCXX_CONST // _GLIBCXX_NORETURN // _GLIBCXX_NOTHROW // _GLIBCXX_VISIBILITY #ifndef _GLIBCXX_PURE # define _GLIBCXX_PURE __attribute__ ((__pure__)) #endif +#ifndef _GLIBCXX_COLD +# define _GLIBCXX_COLD __attribute__ ((cold)) +#endif + #ifndef _GLIBCXX_CONST # define _GLIBCXX_CONST __attribute__ ((__const__)) #endif #ifndef _GLIBCXX_NORETURN # define _GLIBCXX_NORETURN __attribute__ ((__noreturn__)) #endif // See below for C++ #ifndef _GLIBCXX_NOTHROW diff --git a/libstdc++-v3/libsupc++/cxxabi.h b/libstdc++-v3/libsupc++/cxxabi.h index 000713ecd..24c1366e2 100644 --- a/libstdc++-v3/libsupc++/cxxabi.h +++ b/libstdc++-v3/libsupc++/cxxabi.h @@ -108,27 +108,27 @@ namespace __cxxabiv1 __cxa_vec_delete2(void* __array_address, size_t __element_size,
Re: Add cold attribute to one time construction APIs
FYI libc++ patch sent for review: https://reviews.llvm.org/D85873
Re: Add cold attribute to one time construction APIs
sure. -- From: Jonathan Wakely Sent: Thursday, August 13, 2020 11:13 AM To: Aditya K Cc: libstdc++ ; gcc-patches Subject: Re: Add cold attribute to one time construction APIs Please CC the libstdc++ list on all libstdc++ patches. On Thu, 13 Aug 2020 at 17:51, Aditya K wrote: > > Revised patch with _GLIBCXX_COLD added at the end. > > ``` > commit 3dc9f9a8461b1c88e991ceb517e5fdd81f268d1e > Author: Aditya Kumar <1894981+hiradi...@users.noreply.github.com> > Date: Thu Aug 13 09:41:34 2020 -0700 > > Add cold attribute to one time construction APIs > > __cxa_guard_acquire is used for only one purpose, > namely guarding local static variable initialization, > and since that purpose is definitionally cold, it should be attributed as >cold. > Similarly for __cxa_guard_release and __cxa_guard_abort > > diff --git a/libstdc++-v3/include/bits/c++config > b/libstdc++-v3/include/bits/c++config > index b1fad59d4..f6f954eef 100644 > --- a/libstdc++-v3/include/bits/c++config > +++ b/libstdc++-v3/include/bits/c++config > @@ -35,20 +35,21 @@ > > // The datestamp of the C++ library in compressed ISO date format. > #define __GLIBCXX__ > > // Macros for various attributes. > // _GLIBCXX_PURE > // _GLIBCXX_CONST > // _GLIBCXX_NORETURN > // _GLIBCXX_NOTHROW > // _GLIBCXX_VISIBILITY > +// _GLIBCXX_COLD > #ifndef _GLIBCXX_PURE > # define _GLIBCXX_PURE __attribute__ ((__pure__)) > #endif > > #ifndef _GLIBCXX_CONST > # define _GLIBCXX_CONST __attribute__ ((__const__)) > #endif > > #ifndef _GLIBCXX_NORETURN > # define _GLIBCXX_NORETURN __attribute__ ((__noreturn__)) > @@ -67,20 +68,24 @@ > #define _GLIBCXX_HAVE_ATTRIBUTE_VISIBILITY > > #if _GLIBCXX_HAVE_ATTRIBUTE_VISIBILITY > # define _GLIBCXX_VISIBILITY(V) __attribute__ ((__visibility__ (#V))) > #else > // If this is not supplied by the OS-specific or CPU-specific > // headers included below, it will be defined to an empty default. > # define _GLIBCXX_VISIBILITY(V) _GLIBCXX_PSEUDO_VISIBILITY(V) > #endif > > +#ifndef _GLIBCXX_COLD > +# define _GLIBCXX_COLD __attribute__ ((cold)) > +#endif > + > // Macros for deprecated attributes. > // _GLIBCXX_USE_DEPRECATED > // _GLIBCXX_DEPRECATED > // _GLIBCXX17_DEPRECATED > // _GLIBCXX20_DEPRECATED( string-literal ) > #ifndef _GLIBCXX_USE_DEPRECATED > # define _GLIBCXX_USE_DEPRECATED 1 > #endif > > #if defined(__DEPRECATED) && (__cplusplus >= 201103L) > diff --git a/libstdc++-v3/libsupc++/cxxabi.h b/libstdc++-v3/libsupc++/cxxabi.h > index 000713ecd..24c1366e2 100644 > --- a/libstdc++-v3/libsupc++/cxxabi.h > +++ b/libstdc++-v3/libsupc++/cxxabi.h > @@ -108,27 +108,27 @@ namespace __cxxabiv1 > __cxa_vec_delete2(void* __array_address, size_t __element_size, > size_t __padding_size, __cxa_cdtor_type __destructor, > void (*__dealloc) (void*)); > > void > __cxa_vec_delete3(void* __array_address, size_t __element_size, > size_t __padding_size, __cxa_cdtor_type __destructor, > void (*__dealloc) (void*, size_t)); > > int > - __cxa_guard_acquire(__guard*); > + __cxa_guard_acquire(__guard*) _GLIBCXX_COLD; > > void > - __cxa_guard_release(__guard*) _GLIBCXX_NOTHROW; > + __cxa_guard_release(__guard*) _GLIBCXX_NOTHROW _GLIBCXX_COLD; > > void > - __cxa_guard_abort(__guard*) _GLIBCXX_NOTHROW; > + __cxa_guard_abort(__guard*) _GLIBCXX_NOTHROW _GLIBCXX_COLD; > > // DSO destruction. > int > __cxa_atexit(void (*)(void*), void*, void*) _GLIBCXX_NOTHROW; > > void > __cxa_finalize(void*); > > // TLS destruction. > int > ``` > > From: Aditya K > Sent: Thursday, August 13, 2020 10:47 AM > To: Jeff Law via Gcc-patches ; jwakely@gmail.com > > Subject: Add cold attribute to one time construction APIs > > This would help compiler optimize local static objects. > > ``` > commit e2f299679ddf56a6d6d71ea9d589cd76b2ca107b > Author: Aditya Kumar <1894981+hiradi...@users.noreply.github.com> > Date: Thu Aug 13 09:41:34 2020 -0700 > > Add cold attribute to one time construction APIs > > __cxa_guard_acquire is used for only one purpose, > namely guarding local static variable initialization, > and since that purpose is definitionally cold, it should be attributed as >cold. > Similarly for __cxa_guard_release and __cxa_guard_abort > > diff --git a/libstdc++-v3/include/bits/c++config > b/libstdc++-v3/include/bits/c++config > index b1fad59d4..359e955a7 100644 > --- a/libstdc++-v3/include/bits/c++config > +++ b/libstdc++-v3/include/bits/c++conf
