* [stage1] Postpone expanding va_arg until pass_stdarg
@ 2015-02-19 10:48 Tom de Vries
2015-02-19 10:51 ` [PATCH][1/5] Disable lang_hooks.gimplify_expr in free_lang_data Tom de Vries
` (4 more replies)
0 siblings, 5 replies; 39+ messages in thread
From: Tom de Vries @ 2015-02-19 10:48 UTC (permalink / raw)
To: Jakub Jelinek, Richard Biener; +Cc: GCC Patches, Michael Matz
Hi,
I'm posting this patch series for stage1:
- 0001-Disable-lang_hooks.gimplify_expr-in-free_lang_data.patch
- 0002-Add-gimple_find_sub_bbs.patch
- 0003-Factor-optimize_va_list_gpr_fpr_size-out-of-pass_std.patch
- 0004-Handle-internal_fn-in-operand_equal_p.patch
- 0005-Postpone-expanding-va_arg-until-pass_stdarg.patch
The patch series - based on Michael's initial patch - postpones expanding va_arg
until pass_stdarg, which makes pass_stdarg more robust.
Bootstrapped and reg-tested on x86_64 using all languages, with unix/ and
unix/-m32 testing.
I'll post the patches in reply to this email.
Thanks,
- Tom
^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH][1/5] Disable lang_hooks.gimplify_expr in free_lang_data
2015-02-19 10:48 [stage1] Postpone expanding va_arg until pass_stdarg Tom de Vries
@ 2015-02-19 10:51 ` Tom de Vries
2015-02-19 12:41 ` Richard Biener
2015-02-19 10:59 ` [PATCH][2/5] Add gimple_find_sub_bbs Tom de Vries
` (3 subsequent siblings)
4 siblings, 1 reply; 39+ messages in thread
From: Tom de Vries @ 2015-02-19 10:51 UTC (permalink / raw)
To: Jakub Jelinek, Richard Biener; +Cc: GCC Patches, Michael Matz
[-- Attachment #1: Type: text/plain, Size: 1119 bytes --]
On 19-02-15 11:29, Tom de Vries wrote:
> Hi,
>
> I'm posting this patch series for stage1:
> - 0001-Disable-lang_hooks.gimplify_expr-in-free_lang_data.patch
> - 0002-Add-gimple_find_sub_bbs.patch
> - 0003-Factor-optimize_va_list_gpr_fpr_size-out-of-pass_std.patch
> - 0004-Handle-internal_fn-in-operand_equal_p.patch
> - 0005-Postpone-expanding-va_arg-until-pass_stdarg.patch
>
> The patch series - based on Michael's initial patch - postpones expanding va_arg
> until pass_stdarg, which makes pass_stdarg more robust.
>
> Bootstrapped and reg-tested on x86_64 using all languages, with unix/ and
> unix/-m32 testing.
>
> I'll post the patches in reply to this email.
>
This patch disables lang_hooks.gimplify_expr in free_lang_data.
I ran into a situation ( mentioned here:
https://gcc.gnu.org/ml/gcc/2015-02/msg00074.html ) where gimplify_expr was
called during pass_stdarg, which called cp_gimplify_expr, which called
is_really_empty_class, which tried to access some language data (TYPE_BINFO)
that was already freed, which caused a segmentation fault. This patch fixes that.
OK for stage1?
Thanks,
- Tom
[-- Attachment #2: 0001-Disable-lang_hooks.gimplify_expr-in-free_lang_data.patch --]
[-- Type: text/x-patch, Size: 759 bytes --]
2015-02-17 Tom de Vries <tom@codesourcery.com>
* tree.c (free_lang_data): Disable lang_hooks.gimplify_expr.
---
gcc/tree.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/gcc/tree.c b/gcc/tree.c
index 29f70f8..94b7c56 100644
--- a/gcc/tree.c
+++ b/gcc/tree.c
@@ -5815,6 +5815,8 @@ free_lang_data (void)
still be used indirectly via the get_alias_set langhook. */
lang_hooks.dwarf_name = lhd_dwarf_name;
lang_hooks.decl_printable_name = gimple_decl_printable_name;
+ lang_hooks.gimplify_expr = lhd_gimplify_expr;
+
/* We do not want the default decl_assembler_name implementation,
rather if we have fixed everything we want a wrapper around it
asserting that all non-local symbols already got their assembler
--
1.9.1
^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH][2/5] Add gimple_find_sub_bbs
2015-02-19 10:48 [stage1] Postpone expanding va_arg until pass_stdarg Tom de Vries
2015-02-19 10:51 ` [PATCH][1/5] Disable lang_hooks.gimplify_expr in free_lang_data Tom de Vries
@ 2015-02-19 10:59 ` Tom de Vries
2015-02-19 12:41 ` Richard Biener
2015-02-19 11:06 ` [PATCH][3/5] Factor optimize_va_list_gpr_fpr_size out of pass_stdarg::execute Tom de Vries
` (2 subsequent siblings)
4 siblings, 1 reply; 39+ messages in thread
From: Tom de Vries @ 2015-02-19 10:59 UTC (permalink / raw)
To: Jakub Jelinek, Richard Biener; +Cc: GCC Patches, Michael Matz
[-- Attachment #1: Type: text/plain, Size: 891 bytes --]
On 19-02-15 11:29, Tom de Vries wrote:
> Hi,
>
> I'm posting this patch series for stage1:
> - 0001-Disable-lang_hooks.gimplify_expr-in-free_lang_data.patch
> - 0002-Add-gimple_find_sub_bbs.patch
> - 0003-Factor-optimize_va_list_gpr_fpr_size-out-of-pass_std.patch
> - 0004-Handle-internal_fn-in-operand_equal_p.patch
> - 0005-Postpone-expanding-va_arg-until-pass_stdarg.patch
>
> The patch series - based on Michael's initial patch - postpones expanding va_arg
> until pass_stdarg, which makes pass_stdarg more robust.
>
> Bootstrapped and reg-tested on x86_64 using all languages, with unix/ and
> unix/-m32 testing.
>
> I'll post the patches in reply to this email.
This patch adds gimple_find_sub_bbs. The function adds a gimple sequence at a
point in a basic block, and takes care to split up the sequence itself into new
basic blocks where necessary.
OK for stage1?
Thanks,
- Tom
[-- Attachment #2: 0002-Add-gimple_find_sub_bbs.patch --]
[-- Type: text/x-patch, Size: 9567 bytes --]
2015-02-17 Tom de Vries <tom@codesourcery.com>
Michael Matz <matz@suse.de>
* tree-cfg.c (make_blocks_1): Factor out of ...
(make_blocks): ... here.
(make_edges_bb): Factor out of ...
(make_edges): ... here.
(gimple_find_sub_bbs): New function.
* tree-cfg.h (gimple_find_sub_bbs): Declare.
---
gcc/tree-cfg.c | 258 +++++++++++++++++++++++++++++++++++----------------------
gcc/tree-cfg.h | 1 +
2 files changed, 159 insertions(+), 100 deletions(-)
diff --git a/gcc/tree-cfg.c b/gcc/tree-cfg.c
index 006bc08..51fcf9c 100644
--- a/gcc/tree-cfg.c
+++ b/gcc/tree-cfg.c
@@ -513,16 +513,15 @@ gimple_call_initialize_ctrl_altering (gimple stmt)
}
-/* Build a flowgraph for the sequence of stmts SEQ. */
+/* Insert SEQ after BB and build a flowgraph. */
-static void
-make_blocks (gimple_seq seq)
+static basic_block
+make_blocks_1 (gimple_seq seq, basic_block bb)
{
gimple_stmt_iterator i = gsi_start (seq);
gimple stmt = NULL;
bool start_new_block = true;
bool first_stmt_of_seq = true;
- basic_block bb = ENTRY_BLOCK_PTR_FOR_FN (cfun);
while (!gsi_end_p (i))
{
@@ -579,8 +578,16 @@ make_blocks (gimple_seq seq)
gsi_next (&i);
first_stmt_of_seq = false;
}
+ return bb;
}
+/* Build a flowgraph for the sequence of stmts SEQ. */
+
+static void
+make_blocks (gimple_seq seq)
+{
+ make_blocks_1 (seq, ENTRY_BLOCK_PTR_FOR_FN (cfun));
+}
/* Create and return a new empty basic block after bb AFTER. */
@@ -807,6 +814,112 @@ handle_abnormal_edges (basic_block *dispatcher_bbs,
make_edge (*dispatcher, for_bb, EDGE_ABNORMAL);
}
+/* Creates outgoing edges for BB. Returns 1 when it ends with an
+ computed goto, returns 2 when it ends with a statement that
+ might return to this function via an nonlocal goto, otherwise
+ return 0. Updates *PCUR_REGION with the OMP region this BB is in. */
+
+static int
+make_edges_bb (basic_block bb, struct omp_region **pcur_region, int *pomp_index)
+{
+ gimple last = last_stmt (bb);
+ bool fallthru = false;
+ int ret = 0;
+
+ if (!last)
+ return ret;
+
+ switch (gimple_code (last))
+ {
+ case GIMPLE_GOTO:
+ if (make_goto_expr_edges (bb))
+ ret = 1;
+ fallthru = false;
+ break;
+ case GIMPLE_RETURN:
+ {
+ edge e = make_edge (bb, EXIT_BLOCK_PTR_FOR_FN (cfun), 0);
+ e->goto_locus = gimple_location (last);
+ fallthru = false;
+ }
+ break;
+ case GIMPLE_COND:
+ make_cond_expr_edges (bb);
+ fallthru = false;
+ break;
+ case GIMPLE_SWITCH:
+ make_gimple_switch_edges (as_a <gswitch *> (last), bb);
+ fallthru = false;
+ break;
+ case GIMPLE_RESX:
+ make_eh_edges (last);
+ fallthru = false;
+ break;
+ case GIMPLE_EH_DISPATCH:
+ fallthru = make_eh_dispatch_edges (as_a <geh_dispatch *> (last));
+ break;
+
+ case GIMPLE_CALL:
+ /* If this function receives a nonlocal goto, then we need to
+ make edges from this call site to all the nonlocal goto
+ handlers. */
+ if (stmt_can_make_abnormal_goto (last))
+ ret = 2;
+
+ /* If this statement has reachable exception handlers, then
+ create abnormal edges to them. */
+ make_eh_edges (last);
+
+ /* BUILTIN_RETURN is really a return statement. */
+ if (gimple_call_builtin_p (last, BUILT_IN_RETURN))
+ {
+ make_edge (bb, EXIT_BLOCK_PTR_FOR_FN (cfun), 0);
+ fallthru = false;
+ }
+ /* Some calls are known not to return. */
+ else
+ fallthru = !(gimple_call_flags (last) & ECF_NORETURN);
+ break;
+
+ case GIMPLE_ASSIGN:
+ /* A GIMPLE_ASSIGN may throw internally and thus be considered
+ control-altering. */
+ if (is_ctrl_altering_stmt (last))
+ make_eh_edges (last);
+ fallthru = true;
+ break;
+
+ case GIMPLE_ASM:
+ make_gimple_asm_edges (bb);
+ fallthru = true;
+ break;
+
+ CASE_GIMPLE_OMP:
+ fallthru = make_gimple_omp_edges (bb, pcur_region, pomp_index);
+ break;
+
+ case GIMPLE_TRANSACTION:
+ {
+ tree abort_label
+ = gimple_transaction_label (as_a <gtransaction *> (last));
+ if (abort_label)
+ make_edge (bb, label_to_block (abort_label), EDGE_TM_ABORT);
+ fallthru = true;
+ }
+ break;
+
+ default:
+ gcc_assert (!stmt_ends_bb_p (last));
+ fallthru = true;
+ break;
+ }
+
+ if (fallthru)
+ make_edge (bb, bb->next_bb, EDGE_FALLTHRU);
+
+ return ret;
+}
+
/* Join all the blocks in the flowgraph. */
static void
@@ -828,107 +941,19 @@ make_edges (void)
/* Traverse the basic block array placing edges. */
FOR_EACH_BB_FN (bb, cfun)
{
- gimple last = last_stmt (bb);
- bool fallthru;
+ int mer;
if (bb_to_omp_idx)
bb_to_omp_idx[bb->index] = cur_omp_region_idx;
- if (last)
- {
- enum gimple_code code = gimple_code (last);
- switch (code)
- {
- case GIMPLE_GOTO:
- if (make_goto_expr_edges (bb))
- ab_edge_goto.safe_push (bb);
- fallthru = false;
- break;
- case GIMPLE_RETURN:
- {
- edge e = make_edge (bb, EXIT_BLOCK_PTR_FOR_FN (cfun), 0);
- e->goto_locus = gimple_location (last);
- fallthru = false;
- }
- break;
- case GIMPLE_COND:
- make_cond_expr_edges (bb);
- fallthru = false;
- break;
- case GIMPLE_SWITCH:
- make_gimple_switch_edges (as_a <gswitch *> (last), bb);
- fallthru = false;
- break;
- case GIMPLE_RESX:
- make_eh_edges (last);
- fallthru = false;
- break;
- case GIMPLE_EH_DISPATCH:
- fallthru = make_eh_dispatch_edges (as_a <geh_dispatch *> (last));
- break;
-
- case GIMPLE_CALL:
- /* If this function receives a nonlocal goto, then we need to
- make edges from this call site to all the nonlocal goto
- handlers. */
- if (stmt_can_make_abnormal_goto (last))
- ab_edge_call.safe_push (bb);
-
- /* If this statement has reachable exception handlers, then
- create abnormal edges to them. */
- make_eh_edges (last);
-
- /* BUILTIN_RETURN is really a return statement. */
- if (gimple_call_builtin_p (last, BUILT_IN_RETURN))
- {
- make_edge (bb, EXIT_BLOCK_PTR_FOR_FN (cfun), 0);
- fallthru = false;
- }
- /* Some calls are known not to return. */
- else
- fallthru = !(gimple_call_flags (last) & ECF_NORETURN);
- break;
+ mer = make_edges_bb (bb, &cur_region, &cur_omp_region_idx);
+ if (mer == 1)
+ ab_edge_goto.safe_push (bb);
+ else if (mer == 2)
+ ab_edge_call.safe_push (bb);
- case GIMPLE_ASSIGN:
- /* A GIMPLE_ASSIGN may throw internally and thus be considered
- control-altering. */
- if (is_ctrl_altering_stmt (last))
- make_eh_edges (last);
- fallthru = true;
- break;
-
- case GIMPLE_ASM:
- make_gimple_asm_edges (bb);
- fallthru = true;
- break;
-
- CASE_GIMPLE_OMP:
- fallthru = make_gimple_omp_edges (bb, &cur_region,
- &cur_omp_region_idx);
- if (cur_region && bb_to_omp_idx == NULL)
- bb_to_omp_idx = XCNEWVEC (int, n_basic_blocks_for_fn (cfun));
- break;
-
- case GIMPLE_TRANSACTION:
- {
- tree abort_label
- = gimple_transaction_label (as_a <gtransaction *> (last));
- if (abort_label)
- make_edge (bb, label_to_block (abort_label), EDGE_TM_ABORT);
- fallthru = true;
- }
- break;
-
- default:
- gcc_assert (!stmt_ends_bb_p (last));
- fallthru = true;
- }
- }
- else
- fallthru = true;
-
- if (fallthru)
- make_edge (bb, bb->next_bb, EDGE_FALLTHRU);
+ if (cur_region && bb_to_omp_idx == NULL)
+ bb_to_omp_idx = XCNEWVEC (int, n_basic_blocks_for_fn (cfun));
}
/* Computed gotos are hell to deal with, especially if there are
@@ -1008,6 +1033,39 @@ make_edges (void)
fold_cond_expr_cond ();
}
+/* Add SEQ after GSI. Start new bb after GSI, and created further bbs as
+ needed. Returns true if new bbs were created. */
+
+bool
+gimple_find_sub_bbs (gimple_seq seq, gimple_stmt_iterator *gsi)
+{
+ gimple stmt = gsi_stmt (*gsi);
+ basic_block bb = gimple_bb (stmt);
+ basic_block lastbb, afterbb;
+ int old_num_bbs = n_basic_blocks_for_fn (cfun);
+ edge e;
+ lastbb = make_blocks_1 (seq, bb);
+ if (old_num_bbs == n_basic_blocks_for_fn (cfun))
+ return false;
+ e = split_block (bb, stmt);
+ /* Move e->dest to come after the new basic blocks. */
+ afterbb = e->dest;
+ unlink_block (afterbb);
+ link_block (afterbb, lastbb);
+ redirect_edge_succ (e, bb->next_bb);
+ bb = bb->next_bb;
+ while (bb != afterbb)
+ {
+ struct omp_region *cur_region = NULL;
+ int cur_omp_region_idx = 0;
+ int mer = make_edges_bb (bb, &cur_region, &cur_omp_region_idx);
+ gcc_assert (!mer && !cur_region);
+ add_bb_to_loop (bb, afterbb->loop_father);
+ bb = bb->next_bb;
+ }
+ return true;
+}
+
/* Find the next available discriminator value for LOCUS. The
discriminator distinguishes among several basic blocks that
share a common locus, allowing for more accurate sample-based
diff --git a/gcc/tree-cfg.h b/gcc/tree-cfg.h
index cd28a80..2fc1e88 100644
--- a/gcc/tree-cfg.h
+++ b/gcc/tree-cfg.h
@@ -103,5 +103,6 @@ extern void extract_true_false_edges_from_block (basic_block, edge *, edge *);
extern unsigned int execute_fixup_cfg (void);
extern unsigned int split_critical_edges (void);
extern basic_block insert_cond_bb (basic_block, gimple, gimple);
+extern bool gimple_find_sub_bbs (gimple_seq, gimple_stmt_iterator *);
#endif /* _TREE_CFG_H */
--
1.9.1
^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH][3/5] Factor optimize_va_list_gpr_fpr_size out of pass_stdarg::execute
2015-02-19 10:48 [stage1] Postpone expanding va_arg until pass_stdarg Tom de Vries
2015-02-19 10:51 ` [PATCH][1/5] Disable lang_hooks.gimplify_expr in free_lang_data Tom de Vries
2015-02-19 10:59 ` [PATCH][2/5] Add gimple_find_sub_bbs Tom de Vries
@ 2015-02-19 11:06 ` Tom de Vries
2015-02-19 12:44 ` Richard Biener
2015-02-19 11:37 ` [PATCH][4/5] Handle internal_fn in operand_equal_p Tom de Vries
2015-02-19 12:08 ` [PATCH][5/5] Postpone expanding va_arg until pass_stdarg Tom de Vries
4 siblings, 1 reply; 39+ messages in thread
From: Tom de Vries @ 2015-02-19 11:06 UTC (permalink / raw)
To: Jakub Jelinek, Richard Biener; +Cc: GCC Patches, Michael Matz
[-- Attachment #1: Type: text/plain, Size: 911 bytes --]
On 19-02-15 11:29, Tom de Vries wrote:
> Hi,
>
> I'm posting this patch series for stage1:
> - 0001-Disable-lang_hooks.gimplify_expr-in-free_lang_data.patch
> - 0002-Add-gimple_find_sub_bbs.patch
> - 0003-Factor-optimize_va_list_gpr_fpr_size-out-of-pass_std.patch
> - 0004-Handle-internal_fn-in-operand_equal_p.patch
> - 0005-Postpone-expanding-va_arg-until-pass_stdarg.patch
>
> The patch series - based on Michael's initial patch - postpones expanding va_arg
> until pass_stdarg, which makes pass_stdarg more robust.
>
> Bootstrapped and reg-tested on x86_64 using all languages, with unix/ and
> unix/-m32 testing.
>
> I'll post the patches in reply to this email.
>
This patch factors optimize_va_list_gpr_fpr_size out of pass_stdarg::execute.
Now that we're adding more functionality in pass_stdarg, it's cleaner and
clearer to move the optimization to its own function.
OK for stage1?
Thanks,
- Tom
[-- Attachment #2: 0003-Factor-optimize_va_list_gpr_fpr_size-out-of-pass_std.patch --]
[-- Type: text/x-patch, Size: 2569 bytes --]
2015-02-17 Tom de Vries <tom@codesourcery.com>
* tree-stdarg.c (optimize_va_list_gpr_fpr_size): Factor out of ...
(pass_stdarg::execute): ... here.
---
gcc/tree-stdarg.c | 80 ++++++++++++++++++++++++++++++-------------------------
1 file changed, 44 insertions(+), 36 deletions(-)
diff --git a/gcc/tree-stdarg.c b/gcc/tree-stdarg.c
index 17d51a2..5eac56a 100644
--- a/gcc/tree-stdarg.c
+++ b/gcc/tree-stdarg.c
@@ -678,43 +678,10 @@ check_all_va_list_escapes (struct stdarg_info *si)
return false;
}
+/* Optimize FUN->va_list_gpr_size and FUN->va_list_fpr_size. */
-namespace {
-
-const pass_data pass_data_stdarg =
-{
- GIMPLE_PASS, /* type */
- "stdarg", /* name */
- OPTGROUP_NONE, /* optinfo_flags */
- TV_NONE, /* tv_id */
- ( PROP_cfg | PROP_ssa ), /* properties_required */
- 0, /* properties_provided */
- 0, /* properties_destroyed */
- 0, /* todo_flags_start */
- 0, /* todo_flags_finish */
-};
-
-class pass_stdarg : public gimple_opt_pass
-{
-public:
- pass_stdarg (gcc::context *ctxt)
- : gimple_opt_pass (pass_data_stdarg, ctxt)
- {}
-
- /* opt_pass methods: */
- virtual bool gate (function *fun)
- {
- return (flag_stdarg_opt
- /* This optimization is only for stdarg functions. */
- && fun->stdarg != 0);
- }
-
- virtual unsigned int execute (function *);
-
-}; // class pass_stdarg
-
-unsigned int
-pass_stdarg::execute (function *fun)
+static void
+optimize_va_list_gpr_fpr_size (function *fun)
{
basic_block bb;
bool va_list_escapes = false;
@@ -1047,6 +1014,47 @@ finish:
fprintf (dump_file, "%d", cfun->va_list_fpr_size);
fputs (" FPR units.\n", dump_file);
}
+}
+
+namespace {
+
+const pass_data pass_data_stdarg =
+{
+ GIMPLE_PASS, /* type */
+ "stdarg", /* name */
+ OPTGROUP_NONE, /* optinfo_flags */
+ TV_NONE, /* tv_id */
+ ( PROP_cfg | PROP_ssa ), /* properties_required */
+ 0, /* properties_provided */
+ 0, /* properties_destroyed */
+ 0, /* todo_flags_start */
+ 0, /* todo_flags_finish */
+};
+
+class pass_stdarg : public gimple_opt_pass
+{
+public:
+ pass_stdarg (gcc::context *ctxt)
+ : gimple_opt_pass (pass_data_stdarg, ctxt)
+ {}
+
+ /* opt_pass methods: */
+ virtual bool gate (function *fun)
+ {
+ return (flag_stdarg_opt
+ /* This optimization is only for stdarg functions. */
+ && fun->stdarg != 0);
+ }
+
+ virtual unsigned int execute (function *);
+
+}; // class pass_stdarg
+
+unsigned int
+pass_stdarg::execute (function *fun)
+{
+ optimize_va_list_gpr_fpr_size (fun);
+
return 0;
}
--
1.9.1
^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH][4/5] Handle internal_fn in operand_equal_p
2015-02-19 10:48 [stage1] Postpone expanding va_arg until pass_stdarg Tom de Vries
` (2 preceding siblings ...)
2015-02-19 11:06 ` [PATCH][3/5] Factor optimize_va_list_gpr_fpr_size out of pass_stdarg::execute Tom de Vries
@ 2015-02-19 11:37 ` Tom de Vries
2015-02-19 12:49 ` Richard Biener
2015-02-19 12:08 ` [PATCH][5/5] Postpone expanding va_arg until pass_stdarg Tom de Vries
4 siblings, 1 reply; 39+ messages in thread
From: Tom de Vries @ 2015-02-19 11:37 UTC (permalink / raw)
To: Jakub Jelinek, Richard Biener; +Cc: GCC Patches, Michael Matz
[-- Attachment #1: Type: text/plain, Size: 1333 bytes --]
On 19-02-15 11:29, Tom de Vries wrote:
> Hi,
>
> I'm posting this patch series for stage1:
> - 0001-Disable-lang_hooks.gimplify_expr-in-free_lang_data.patch
> - 0002-Add-gimple_find_sub_bbs.patch
> - 0003-Factor-optimize_va_list_gpr_fpr_size-out-of-pass_std.patch
> - 0004-Handle-internal_fn-in-operand_equal_p.patch
> - 0005-Postpone-expanding-va_arg-until-pass_stdarg.patch
>
> The patch series - based on Michael's initial patch - postpones expanding va_arg
> until pass_stdarg, which makes pass_stdarg more robust.
>
> Bootstrapped and reg-tested on x86_64 using all languages, with unix/ and
> unix/-m32 testing.
>
> I'll post the patches in reply to this email.
>
This patch adds handling of internal functions in operand_equal_p.
I ran into a situation here in operand_equal_p where it segfaulted on the
internal function IFN_VA_ARG, because the CALL_EXPR_FN of an internal function
is NULL, and operand_equal_p does not expect NULL arguments:
...
case CALL_EXPR:
/* If the CALL_EXPRs call different functions, then they
clearly can not be equal. */
if (! operand_equal_p (CALL_EXPR_FN (arg0), CALL_EXPR_FN (arg1),
flags))
return 0;
...
This patch fixes that by testing if CALL_EXPR_FN is NULL.
OK for stage1?
Thanks,
- Tom
[-- Attachment #2: 0004-Handle-internal_fn-in-operand_equal_p.patch --]
[-- Type: text/x-patch, Size: 775 bytes --]
2015-02-17 Tom de Vries <tom@codesourcery.com>
* fold-const.c (operand_equal_p): Handle INTERNAL_FNs.
---
gcc/fold-const.c | 5 +++++
1 file changed, 5 insertions(+)
diff --git a/gcc/fold-const.c b/gcc/fold-const.c
index 8377120..fbf76d0 100644
--- a/gcc/fold-const.c
+++ b/gcc/fold-const.c
@@ -3032,6 +3032,11 @@ operand_equal_p (const_tree arg0, const_tree arg1, unsigned int flags)
switch (TREE_CODE (arg0))
{
case CALL_EXPR:
+ /* Handle internal_fns conservatively. */
+ if (CALL_EXPR_FN (arg0) == NULL_TREE
+ || CALL_EXPR_FN (arg1) == NULL_TREE)
+ return 0;
+
/* If the CALL_EXPRs call different functions, then they
clearly can not be equal. */
if (! operand_equal_p (CALL_EXPR_FN (arg0), CALL_EXPR_FN (arg1),
--
1.9.1
^ permalink raw reply [flat|nested] 39+ messages in thread
* [PATCH][5/5] Postpone expanding va_arg until pass_stdarg
2015-02-19 10:48 [stage1] Postpone expanding va_arg until pass_stdarg Tom de Vries
` (3 preceding siblings ...)
2015-02-19 11:37 ` [PATCH][4/5] Handle internal_fn in operand_equal_p Tom de Vries
@ 2015-02-19 12:08 ` Tom de Vries
2015-02-19 13:05 ` Richard Biener
4 siblings, 1 reply; 39+ messages in thread
From: Tom de Vries @ 2015-02-19 12:08 UTC (permalink / raw)
To: Jakub Jelinek, Richard Biener; +Cc: GCC Patches, Michael Matz
[-- Attachment #1: Type: text/plain, Size: 2366 bytes --]
On 19-02-15 11:29, Tom de Vries wrote:
> Hi,
>
> I'm posting this patch series for stage1:
> - 0001-Disable-lang_hooks.gimplify_expr-in-free_lang_data.patch
> - 0002-Add-gimple_find_sub_bbs.patch
> - 0003-Factor-optimize_va_list_gpr_fpr_size-out-of-pass_std.patch
> - 0004-Handle-internal_fn-in-operand_equal_p.patch
> - 0005-Postpone-expanding-va_arg-until-pass_stdarg.patch
>
> The patch series - based on Michael's initial patch - postpones expanding va_arg
> until pass_stdarg, which makes pass_stdarg more robust.
>
> Bootstrapped and reg-tested on x86_64 using all languages, with unix/ and
> unix/-m32 testing.
>
> I'll post the patches in reply to this email.
>
This patch postpones expanding va_arg until pass_stdarg.
We add a new internal function IFN_VA_ARG. During gimplification, we map
VA_ARG_EXPR onto a CALL_EXPR with IFN_VA_ARG, which is then gimplified in to a
gimple_call. At pass_stdarg, we expand the IFN_VA_ARG gimple_call into actual code.
There are a few implementation details worth mentioning:
- passing the type beyond gimplification is done by adding a NULL pointer-
to-type to IFN_VA_ARG.
- there is special handling for IFN_VA_ARG that would be most suited to be
placed in gimplify_va_arg_expr. However, that function lacks the scope for
the special handling, so it's placed awkwardly in gimplify_modify_expr.
- there's special handling in case the va_arg type is variable-sized.
gimplify_modify_expr adds a WITH_SIZE_EXPR to the CALL_EXPR IFN_VA_ARG for
variable-sized types. However, this is gimplified into a gimple_call which
does not have the possibility to wrap it's result in a WITH_SIZE_EXPR. So
we're adding the size argument of the WITH_SIZE_EXPR as argument to
IFN_VA_ARG, and at expansion in pass_stdarg, wrap the result of the
gimplification of IFN_VA_ARG in a WITH_SIZE_EXPR, such that the subsequent
gimplify_assign will generate a memcpy if necessary.
- when gimplifying the va_arg argument ap, it may not be addressable. So
gimplification will generate a copy ap.1 = ap, and use &ap.1 as argument.
This means that we have to copy back the ap.1 value to ap after IFN_VA_ARG.
The copy is classified by the va_list_gpr/fpr_size optimization as an
escape, so it inhibits optimization. The tree-ssa/stdarg-2.c f15 update is
because of that.
OK for stage1?
Thanks,
- Tom
[-- Attachment #2: 0005-Postpone-expanding-va_arg-until-pass_stdarg.patch --]
[-- Type: text/x-patch, Size: 15499 bytes --]
2015-02-17 Tom de Vries <tom@codesourcery.com>
Michael Matz <matz@suse.de>
* gimple-iterator.c (update_modified_stmts): Remove static.
* gimple-iterator.h (update_modified_stmts): Declare.
* gimplify.c (gimplify_modify_expr): Handle IFN_VA_ARG.
(gimplify_va_arg_internal): New function.
(gimplify_va_arg_expr): Use IFN_VA_ARG.
* gimplify.h (gimplify_va_arg_internal): Declare.
* internal-fn.c (expand_VA_ARG): New unreachable function.
* internal-fn.def (VA_ARG): New DEF_INTERNAL_FN.
* tree-stdarg.c (gimple_call_ifn_va_arg_p, expand_ifn_va_arg): New
function.
(pass_stdarg::execute): Call expand_ifn_va_arg.
* gcc.dg/tree-ssa/stdarg-2.c: Change f15 scan-tree-dump for target
x86_64-*-*.
---
gcc/gimple-iterator.c | 2 +-
gcc/gimple-iterator.h | 1 +
gcc/gimplify.c | 109 +++++++++++++++++++++--------
gcc/gimplify.h | 2 +
gcc/internal-fn.c | 9 +++
gcc/internal-fn.def | 1 +
gcc/testsuite/gcc.dg/tree-ssa/stdarg-2.c | 4 +-
gcc/tree-stdarg.c | 115 +++++++++++++++++++++++++++++--
8 files changed, 208 insertions(+), 35 deletions(-)
diff --git a/gcc/gimple-iterator.c b/gcc/gimple-iterator.c
index a322390..df29123 100644
--- a/gcc/gimple-iterator.c
+++ b/gcc/gimple-iterator.c
@@ -72,7 +72,7 @@ update_modified_stmt (gimple stmt)
/* Mark the statements in SEQ as modified, and update them. */
-static void
+void
update_modified_stmts (gimple_seq seq)
{
gimple_stmt_iterator gsi;
diff --git a/gcc/gimple-iterator.h b/gcc/gimple-iterator.h
index 6be88dd..ab5759e 100644
--- a/gcc/gimple-iterator.h
+++ b/gcc/gimple-iterator.h
@@ -90,6 +90,7 @@ extern basic_block gsi_insert_seq_on_edge_immediate (edge, gimple_seq);
extern void gsi_commit_edge_inserts (void);
extern void gsi_commit_one_edge_insert (edge, basic_block *);
extern gphi_iterator gsi_start_phis (basic_block);
+extern void update_modified_stmts (gimple_seq);
/* Return a new iterator pointing to GIMPLE_SEQ's first statement. */
diff --git a/gcc/gimplify.c b/gcc/gimplify.c
index 1353ada..932200e 100644
--- a/gcc/gimplify.c
+++ b/gcc/gimplify.c
@@ -4564,6 +4564,7 @@ gimplify_modify_expr (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p,
gimple assign;
location_t loc = EXPR_LOCATION (*expr_p);
gimple_stmt_iterator gsi;
+ tree ap = NULL_TREE, ap_copy = NULL_TREE;
gcc_assert (TREE_CODE (*expr_p) == MODIFY_EXPR
|| TREE_CODE (*expr_p) == INIT_EXPR);
@@ -4640,6 +4641,27 @@ gimplify_modify_expr (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p,
if (ret == GS_ERROR)
return ret;
+ /* In case of va_arg internal fn wrappped in a WITH_SIZE_EXPR, add the type
+ size as argument to the the call. */
+ if (TREE_CODE (*from_p) == WITH_SIZE_EXPR)
+ {
+ tree call = TREE_OPERAND (*from_p, 0);
+ tree vlasize = TREE_OPERAND (*from_p, 1);
+
+ if (TREE_CODE (call) == CALL_EXPR
+ && CALL_EXPR_IFN (call) == IFN_VA_ARG)
+ {
+ tree type = TREE_TYPE (call);
+ tree ap = CALL_EXPR_ARG (call, 0);
+ tree tag = CALL_EXPR_ARG (call, 1);
+ tree newcall = build_call_expr_internal_loc (EXPR_LOCATION (call),
+ IFN_VA_ARG, type, 3, ap,
+ tag, vlasize);
+ tree *call_p = &(TREE_OPERAND (*from_p, 0));
+ *call_p = newcall;
+ }
+ }
+
/* Now see if the above changed *from_p to something we handle specially. */
ret = gimplify_modify_expr_rhs (expr_p, from_p, to_p, pre_p, post_p,
want_value);
@@ -4703,12 +4725,16 @@ gimplify_modify_expr (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p,
enum internal_fn ifn = CALL_EXPR_IFN (*from_p);
auto_vec<tree> vargs (nargs);
+ if (ifn == IFN_VA_ARG)
+ ap = unshare_expr (CALL_EXPR_ARG (*from_p, 0));
for (i = 0; i < nargs; i++)
{
gimplify_arg (&CALL_EXPR_ARG (*from_p, i), pre_p,
EXPR_LOCATION (*from_p));
vargs.quick_push (CALL_EXPR_ARG (*from_p, i));
}
+ if (ifn == IFN_VA_ARG)
+ ap_copy = CALL_EXPR_ARG (*from_p, 0);
call_stmt = gimple_build_call_internal_vec (ifn, vargs);
gimple_set_location (call_stmt, EXPR_LOCATION (*expr_p));
}
@@ -4753,6 +4779,17 @@ gimplify_modify_expr (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p,
gsi = gsi_last (*pre_p);
maybe_fold_stmt (&gsi);
+ /* When gimplifying the &ap argument of va_arg, we might end up with
+ ap.1 = ap
+ va_arg (&ap.1, 0B)
+ We need to assign ap.1 back to ap, otherwise va_arg has no effect on
+ ap. */
+ if (ap != NULL_TREE
+ && TREE_CODE (ap) == ADDR_EXPR
+ && TREE_CODE (ap_copy) == ADDR_EXPR
+ && TREE_OPERAND (ap, 0) != TREE_OPERAND (ap_copy, 0))
+ gimplify_assign (TREE_OPERAND (ap, 0), TREE_OPERAND (ap_copy, 0), pre_p);
+
if (want_value)
{
*expr_p = TREE_THIS_VOLATILE (*to_p) ? *from_p : unshare_expr (*to_p);
@@ -9290,6 +9327,42 @@ dummy_object (tree type)
return build2 (MEM_REF, type, t, t);
}
+/* Call the target expander for evaluating a va_arg call of VALIST
+ and TYPE. */
+
+tree
+gimplify_va_arg_internal (tree valist, tree type, location_t loc,
+ gimple_seq *pre_p, gimple_seq *post_p)
+{
+ tree have_va_type = TREE_TYPE (valist);
+ tree cano_type = targetm.canonical_va_list_type (have_va_type);
+
+ if (cano_type != NULL_TREE)
+ have_va_type = cano_type;
+
+ /* Make it easier for the backends by protecting the valist argument
+ from multiple evaluations. */
+ if (TREE_CODE (have_va_type) == ARRAY_TYPE)
+ {
+ /* For this case, the backends will be expecting a pointer to
+ TREE_TYPE (abi), but it's possible we've
+ actually been given an array (an actual TARGET_FN_ABI_VA_LIST).
+ So fix it. */
+ if (TREE_CODE (TREE_TYPE (valist)) == ARRAY_TYPE)
+ {
+ tree p1 = build_pointer_type (TREE_TYPE (have_va_type));
+ valist = fold_convert_loc (loc, p1,
+ build_fold_addr_expr_loc (loc, valist));
+ }
+
+ gimplify_expr (&valist, pre_p, post_p, is_gimple_val, fb_rvalue);
+ }
+ else
+ gimplify_expr (&valist, pre_p, post_p, is_gimple_min_lval, fb_lvalue);
+
+ return targetm.gimplify_va_arg_expr (valist, type, pre_p, post_p);
+}
+
/* Gimplify __builtin_va_arg, aka VA_ARG_EXPR, which is not really a
builtin function, but a very special sort of operator. */
@@ -9351,36 +9424,18 @@ gimplify_va_arg_expr (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p)
*expr_p = dummy_object (type);
return GS_ALL_DONE;
}
- else
+ else if (optimize && !optimize_debug)
{
- /* Make it easier for the backends by protecting the valist argument
- from multiple evaluations. */
- if (TREE_CODE (have_va_type) == ARRAY_TYPE)
- {
- /* For this case, the backends will be expecting a pointer to
- TREE_TYPE (abi), but it's possible we've
- actually been given an array (an actual TARGET_FN_ABI_VA_LIST).
- So fix it. */
- if (TREE_CODE (TREE_TYPE (valist)) == ARRAY_TYPE)
- {
- tree p1 = build_pointer_type (TREE_TYPE (have_va_type));
- valist = fold_convert_loc (loc, p1,
- build_fold_addr_expr_loc (loc, valist));
- }
+ tree ap, tag;
- gimplify_expr (&valist, pre_p, post_p, is_gimple_val, fb_rvalue);
- }
- else
- gimplify_expr (&valist, pre_p, post_p, is_gimple_min_lval, fb_lvalue);
-
- if (!targetm.gimplify_va_arg_expr)
- /* FIXME: Once most targets are converted we should merely
- assert this is non-null. */
- return GS_ALL_DONE;
-
- *expr_p = targetm.gimplify_va_arg_expr (valist, type, pre_p, post_p);
- return GS_OK;
+ /* Transform a VA_ARG_EXPR into an VA_ARG internal function. */
+ ap = build_fold_addr_expr_loc (loc, valist);
+ tag = build_int_cst (build_pointer_type (type), 0);
+ *expr_p = build_call_expr_internal_loc (loc, IFN_VA_ARG, type, 2, ap, tag);
}
+ else
+ *expr_p = gimplify_va_arg_internal (valist, type, loc, pre_p, post_p);
+ return GS_OK;
}
/* Build a new GIMPLE_ASSIGN tuple and append it to the end of *SEQ_P.
diff --git a/gcc/gimplify.h b/gcc/gimplify.h
index 615925c..bad8e0f 100644
--- a/gcc/gimplify.h
+++ b/gcc/gimplify.h
@@ -82,6 +82,8 @@ extern void gimplify_function_tree (tree);
extern enum gimplify_status gimplify_va_arg_expr (tree *, gimple_seq *,
gimple_seq *);
gimple gimplify_assign (tree, tree, gimple_seq *);
+extern tree gimplify_va_arg_internal (tree, tree, location_t, gimple_seq *,
+ gimple_seq *);
/* Return true if gimplify_one_sizepos doesn't need to gimplify
expr (when in TYPE_SIZE{,_UNIT} and similar type/decl size/bitsize
diff --git a/gcc/internal-fn.c b/gcc/internal-fn.c
index e402825..0053ed9 100644
--- a/gcc/internal-fn.c
+++ b/gcc/internal-fn.c
@@ -1972,6 +1972,15 @@ expand_BUILTIN_EXPECT (gcall *stmt)
emit_move_insn (target, val);
}
+/* IFN_VA_ARG is supposed to be expanded at pass_stdarg. So this dummy function
+ should never be called. */
+
+static void
+expand_VA_ARG (gcall *stmt ATTRIBUTE_UNUSED)
+{
+ gcc_unreachable ();
+}
+
/* Routines to expand each internal function, indexed by function number.
Each routine has the prototype:
diff --git a/gcc/internal-fn.def b/gcc/internal-fn.def
index 032ce6c..f557c64 100644
--- a/gcc/internal-fn.def
+++ b/gcc/internal-fn.def
@@ -62,3 +62,4 @@ DEF_INTERNAL_FN (ADD_OVERFLOW, ECF_CONST | ECF_LEAF | ECF_NOTHROW, NULL)
DEF_INTERNAL_FN (SUB_OVERFLOW, ECF_CONST | ECF_LEAF | ECF_NOTHROW, NULL)
DEF_INTERNAL_FN (MUL_OVERFLOW, ECF_CONST | ECF_LEAF | ECF_NOTHROW, NULL)
DEF_INTERNAL_FN (TSAN_FUNC_EXIT, ECF_NOVOPS | ECF_LEAF | ECF_NOTHROW, NULL)
+DEF_INTERNAL_FN (VA_ARG, 0, NULL)
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/stdarg-2.c b/gcc/testsuite/gcc.dg/tree-ssa/stdarg-2.c
index fe39da3..5a74280 100644
--- a/gcc/testsuite/gcc.dg/tree-ssa/stdarg-2.c
+++ b/gcc/testsuite/gcc.dg/tree-ssa/stdarg-2.c
@@ -288,9 +288,9 @@ f15 (int i, ...)
f15_1 (ap);
va_end (ap);
}
-/* { dg-final { scan-tree-dump "f15: va_list escapes 0, needs to save \[148\] GPR units and \[1-9\]\[0-9\]* FPR units" "stdarg" { target { { i?86-*-* x86_64-*-* } && { ! { ia32 || llp64 } } } } } } */
+/* { dg-final { scan-tree-dump "f15: va_list escapes 0, needs to save \[148\] GPR units and \[1-9\]\[0-9\]* FPR units" "stdarg" { target { { i?86-*-* } && { ! { ia32 || llp64 } } } } } } */
/* { dg-final { scan-tree-dump "f15: va_list escapes 0, needs to save \[148\] GPR units and \[1-9\]\[0-9\]* FPR units" "stdarg" { target { powerpc*-*-linux* && { powerpc_fprs && ilp32 } } } } } */
-/* { dg-final { scan-tree-dump "f15: va_list escapes 1, needs to save all GPR units and all FPR units" "stdarg" { target alpha*-*-linux* } } } */
+/* { dg-final { scan-tree-dump "f15: va_list escapes 1, needs to save all GPR units and all FPR units" "stdarg" { target { { alpha*-*-linux* } || { { x86_64-*-* } && { ! { ia32 || llp64 } } } } } } } */
/* { dg-final { scan-tree-dump "f15: va_list escapes 0, needs to save 1 GPR units and 2 FPR units" "stdarg" { target s390*-*-linux* } } } */
/* { dg-final { scan-tree-dump-not "f15: va_list escapes 0, needs to save 0 GPR units" "stdarg" { target { { i?86-*-* x86_64-*-* } && ia32 } } } } */
/* { dg-final { scan-tree-dump-not "f15: va_list escapes 0, needs to save 0 GPR units" "stdarg" { target ia64-*-* } } } */
diff --git a/gcc/tree-stdarg.c b/gcc/tree-stdarg.c
index 5eac56a..9dc5fe5 100644
--- a/gcc/tree-stdarg.c
+++ b/gcc/tree-stdarg.c
@@ -52,11 +52,14 @@ along with GCC; see the file COPYING3. If not see
#include "gimple-iterator.h"
#include "gimple-walk.h"
#include "gimple-ssa.h"
+#include "gimplify.h"
#include "tree-phinodes.h"
#include "ssa-iterators.h"
#include "stringpool.h"
#include "tree-ssanames.h"
+#include "tree-into-ssa.h"
#include "sbitmap.h"
+#include "tree-cfg.h"
#include "tree-pass.h"
#include "tree-stdarg.h"
@@ -1016,6 +1019,101 @@ finish:
}
}
+/* Return true if STMT is IFN_VA_ARG. */
+
+static bool
+gimple_call_ifn_va_arg_p (gimple stmt)
+{
+ return (is_gimple_call (stmt)
+ && gimple_call_internal_p (stmt)
+ && gimple_call_internal_fn (stmt) == IFN_VA_ARG);
+}
+
+/* Expand IFN_VA_ARGs in FUN. */
+
+static void
+expand_ifn_va_arg (function *fun)
+{
+ bool modified = false;
+ basic_block bb;
+ gimple_stmt_iterator i;
+
+ FOR_EACH_BB_FN (bb, fun)
+ for (i = gsi_start_bb (bb); !gsi_end_p (i); gsi_next (&i))
+ {
+ gimple stmt = gsi_stmt (i);
+ tree ap, expr, lhs, type;
+ gimple_seq pre = NULL, post = NULL;
+
+ if (!gimple_call_ifn_va_arg_p (stmt))
+ continue;
+
+ modified = true;
+
+ type = TREE_TYPE (TREE_TYPE (gimple_call_arg (stmt, 1)));
+ ap = gimple_call_arg (stmt, 0);
+ ap = build_fold_indirect_ref (ap);
+
+ push_gimplify_context (false);
+
+ expr = gimplify_va_arg_internal (ap, type, gimple_location (stmt),
+ &pre, &post);
+
+ lhs = gimple_call_lhs (stmt);
+ if (lhs != NULL_TREE)
+ {
+ gcc_assert (useless_type_conversion_p (TREE_TYPE (lhs), type));
+
+ if (gimple_call_num_args (stmt) == 3)
+ {
+ /* We've transported the size of with WITH_SIZE_EXPR here as
+ the 3rd argument of the internal fn call. Now reinstate
+ it. */
+ tree size = gimple_call_arg (stmt, 2);
+ expr = build2 (WITH_SIZE_EXPR, TREE_TYPE (expr), expr, size);
+ }
+
+ /* We use gimplify_assign here, rather than gimple_build_assign,
+ because gimple_assign knows how to deal with variable-sized
+ types.*/
+ gimplify_assign (lhs, expr, &pre);
+ }
+
+ pop_gimplify_context (NULL);
+
+ gimple_seq_add_seq (&pre, post);
+ update_modified_stmts (pre);
+
+ /* Add the sequence after IFN_VA_ARG. This splits the bb right
+ after IFN_VA_ARG, and adds the sequence in one or more new bbs
+ inbetween. */
+ gimple_find_sub_bbs (pre, &i);
+
+ /* Remove the IFN_VA_ARG gimple_call. It's the last stmt in the
+ bb. */
+ gsi_remove (&i, true);
+ gcc_assert (gsi_end_p (i));
+
+ /* We're walking here into the bbs which contain the expansion of
+ IFN_VA_ARG, and will not contain another IFN_VA_ARG that needs
+ expanding. We could try to skip walking these bbs, perhaps by
+ walking backwards over gimples and bbs. */
+ break;
+ }
+
+#if ENABLE_CHECKING
+ FOR_EACH_BB_FN (bb, fun)
+ for (i = gsi_start_bb (bb); !gsi_end_p (i); gsi_next (&i))
+ gcc_assert (!gimple_call_ifn_va_arg_p (gsi_stmt (i)));
+#endif
+
+ if (modified)
+ {
+ free_dominance_info (CDI_DOMINATORS);
+ update_ssa (TODO_update_ssa);
+ }
+}
+
namespace {
const pass_data pass_data_stdarg =
@@ -1039,11 +1137,13 @@ public:
{}
/* opt_pass methods: */
- virtual bool gate (function *fun)
+ virtual bool gate (function *)
{
- return (flag_stdarg_opt
- /* This optimization is only for stdarg functions. */
- && fun->stdarg != 0);
+ /* Always run this pass, in order to expand va_arg internal_fns. We
+ also need to do that if fun->stdarg == 0, because a va_arg may also
+ occur in a function without varargs, f.i. if when passing a va_list to
+ another function. */
+ return true;
}
virtual unsigned int execute (function *);
@@ -1053,7 +1153,12 @@ public:
unsigned int
pass_stdarg::execute (function *fun)
{
- optimize_va_list_gpr_fpr_size (fun);
+ expand_ifn_va_arg (fun);
+
+ if (flag_stdarg_opt
+ /* This optimization is only for stdarg functions. */
+ && fun->stdarg != 0)
+ optimize_va_list_gpr_fpr_size (fun);
return 0;
}
--
1.9.1
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH][1/5] Disable lang_hooks.gimplify_expr in free_lang_data
2015-02-19 10:51 ` [PATCH][1/5] Disable lang_hooks.gimplify_expr in free_lang_data Tom de Vries
@ 2015-02-19 12:41 ` Richard Biener
0 siblings, 0 replies; 39+ messages in thread
From: Richard Biener @ 2015-02-19 12:41 UTC (permalink / raw)
To: Tom de Vries; +Cc: Jakub Jelinek, GCC Patches, Michael Matz
On Thu, 19 Feb 2015, Tom de Vries wrote:
> On 19-02-15 11:29, Tom de Vries wrote:
> > Hi,
> >
> > I'm posting this patch series for stage1:
> > - 0001-Disable-lang_hooks.gimplify_expr-in-free_lang_data.patch
> > - 0002-Add-gimple_find_sub_bbs.patch
> > - 0003-Factor-optimize_va_list_gpr_fpr_size-out-of-pass_std.patch
> > - 0004-Handle-internal_fn-in-operand_equal_p.patch
> > - 0005-Postpone-expanding-va_arg-until-pass_stdarg.patch
> >
> > The patch series - based on Michael's initial patch - postpones expanding
> > va_arg
> > until pass_stdarg, which makes pass_stdarg more robust.
> >
> > Bootstrapped and reg-tested on x86_64 using all languages, with unix/ and
> > unix/-m32 testing.
> >
> > I'll post the patches in reply to this email.
> >
>
> This patch disables lang_hooks.gimplify_expr in free_lang_data.
>
> I ran into a situation ( mentioned here:
> https://gcc.gnu.org/ml/gcc/2015-02/msg00074.html ) where gimplify_expr was
> called during pass_stdarg, which called cp_gimplify_expr, which called
> is_really_empty_class, which tried to access some language data (TYPE_BINFO)
> that was already freed, which caused a segmentation fault. This patch fixes
> that.
>
> OK for stage1?
Ok.
Thanks,
Richard.
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH][2/5] Add gimple_find_sub_bbs
2015-02-19 10:59 ` [PATCH][2/5] Add gimple_find_sub_bbs Tom de Vries
@ 2015-02-19 12:41 ` Richard Biener
2015-02-19 12:51 ` Jakub Jelinek
2015-02-22 12:47 ` Tom de Vries
0 siblings, 2 replies; 39+ messages in thread
From: Richard Biener @ 2015-02-19 12:41 UTC (permalink / raw)
To: Tom de Vries; +Cc: Jakub Jelinek, GCC Patches, Michael Matz
On Thu, 19 Feb 2015, Tom de Vries wrote:
> On 19-02-15 11:29, Tom de Vries wrote:
> > Hi,
> >
> > I'm posting this patch series for stage1:
> > - 0001-Disable-lang_hooks.gimplify_expr-in-free_lang_data.patch
> > - 0002-Add-gimple_find_sub_bbs.patch
> > - 0003-Factor-optimize_va_list_gpr_fpr_size-out-of-pass_std.patch
> > - 0004-Handle-internal_fn-in-operand_equal_p.patch
> > - 0005-Postpone-expanding-va_arg-until-pass_stdarg.patch
> >
> > The patch series - based on Michael's initial patch - postpones expanding
> > va_arg
> > until pass_stdarg, which makes pass_stdarg more robust.
> >
> > Bootstrapped and reg-tested on x86_64 using all languages, with unix/ and
> > unix/-m32 testing.
> >
> > I'll post the patches in reply to this email.
>
> This patch adds gimple_find_sub_bbs. The function adds a gimple sequence at a
> point in a basic block, and takes care to split up the sequence itself into
> new basic blocks where necessary.
>
> OK for stage1?
I hope we can get rid of this again (which needs re-writing of all
targets va-arg gimplification hooks... - we are in need of some
generic diamond/triangle CFG pattern builders anyway).
Ok with a comment on the function that this is transitional and
should not be used in new code.
Thanks,
Richard.
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH][3/5] Factor optimize_va_list_gpr_fpr_size out of pass_stdarg::execute
2015-02-19 11:06 ` [PATCH][3/5] Factor optimize_va_list_gpr_fpr_size out of pass_stdarg::execute Tom de Vries
@ 2015-02-19 12:44 ` Richard Biener
0 siblings, 0 replies; 39+ messages in thread
From: Richard Biener @ 2015-02-19 12:44 UTC (permalink / raw)
To: Tom de Vries; +Cc: Jakub Jelinek, GCC Patches, Michael Matz
On Thu, 19 Feb 2015, Tom de Vries wrote:
> On 19-02-15 11:29, Tom de Vries wrote:
> > Hi,
> >
> > I'm posting this patch series for stage1:
> > - 0001-Disable-lang_hooks.gimplify_expr-in-free_lang_data.patch
> > - 0002-Add-gimple_find_sub_bbs.patch
> > - 0003-Factor-optimize_va_list_gpr_fpr_size-out-of-pass_std.patch
> > - 0004-Handle-internal_fn-in-operand_equal_p.patch
> > - 0005-Postpone-expanding-va_arg-until-pass_stdarg.patch
> >
> > The patch series - based on Michael's initial patch - postpones expanding
> > va_arg
> > until pass_stdarg, which makes pass_stdarg more robust.
> >
> > Bootstrapped and reg-tested on x86_64 using all languages, with unix/ and
> > unix/-m32 testing.
> >
> > I'll post the patches in reply to this email.
> >
>
> This patch factors optimize_va_list_gpr_fpr_size out of pass_stdarg::execute.
>
> Now that we're adding more functionality in pass_stdarg, it's cleaner and
> clearer to move the optimization to its own function.
>
> OK for stage1?
Ok.
Thanks,
Richard.
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH][4/5] Handle internal_fn in operand_equal_p
2015-02-19 11:37 ` [PATCH][4/5] Handle internal_fn in operand_equal_p Tom de Vries
@ 2015-02-19 12:49 ` Richard Biener
2015-02-19 12:59 ` Jakub Jelinek
0 siblings, 1 reply; 39+ messages in thread
From: Richard Biener @ 2015-02-19 12:49 UTC (permalink / raw)
To: Tom de Vries; +Cc: Jakub Jelinek, GCC Patches, Michael Matz
On Thu, 19 Feb 2015, Tom de Vries wrote:
> On 19-02-15 11:29, Tom de Vries wrote:
> > Hi,
> >
> > I'm posting this patch series for stage1:
> > - 0001-Disable-lang_hooks.gimplify_expr-in-free_lang_data.patch
> > - 0002-Add-gimple_find_sub_bbs.patch
> > - 0003-Factor-optimize_va_list_gpr_fpr_size-out-of-pass_std.patch
> > - 0004-Handle-internal_fn-in-operand_equal_p.patch
> > - 0005-Postpone-expanding-va_arg-until-pass_stdarg.patch
> >
> > The patch series - based on Michael's initial patch - postpones expanding
> > va_arg
> > until pass_stdarg, which makes pass_stdarg more robust.
> >
> > Bootstrapped and reg-tested on x86_64 using all languages, with unix/ and
> > unix/-m32 testing.
> >
> > I'll post the patches in reply to this email.
> >
>
> This patch adds handling of internal functions in operand_equal_p.
>
> I ran into a situation here in operand_equal_p where it segfaulted on the
> internal function IFN_VA_ARG, because the CALL_EXPR_FN of an internal
> function is NULL, and operand_equal_p does not expect NULL arguments:
> ...
> case CALL_EXPR:
> /* If the CALL_EXPRs call different functions, then they
> clearly can not be equal. */
> if (! operand_equal_p (CALL_EXPR_FN (arg0), CALL_EXPR_FN (arg1),
> flags))
> return 0;
> ...
>
> This patch fixes that by testing if CALL_EXPR_FN is NULL.
>
> OK for stage1?
I'd call it a bug though, and we do have internal fns in
generic already thus the issue is latent (with ubsan at least).
Which means ok for trunk now.
Thanks,
Richard.
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH][2/5] Add gimple_find_sub_bbs
2015-02-19 12:41 ` Richard Biener
@ 2015-02-19 12:51 ` Jakub Jelinek
2015-02-19 13:03 ` Marek Polacek
2015-02-19 13:07 ` Richard Biener
2015-02-22 12:47 ` Tom de Vries
1 sibling, 2 replies; 39+ messages in thread
From: Jakub Jelinek @ 2015-02-19 12:51 UTC (permalink / raw)
To: Richard Biener; +Cc: Tom de Vries, GCC Patches, Michael Matz
On Thu, Feb 19, 2015 at 01:41:05PM +0100, Richard Biener wrote:
> I hope we can get rid of this again (which needs re-writing of all
> targets va-arg gimplification hooks... - we are in need of some
> generic diamond/triangle CFG pattern builders anyway).
We already have some in asan.c - create_cond_insert_point - perhaps
it isn't best named and could use some changes to be generally usable.
Jakub
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH][4/5] Handle internal_fn in operand_equal_p
2015-02-19 12:49 ` Richard Biener
@ 2015-02-19 12:59 ` Jakub Jelinek
2015-02-19 13:09 ` Richard Biener
0 siblings, 1 reply; 39+ messages in thread
From: Jakub Jelinek @ 2015-02-19 12:59 UTC (permalink / raw)
To: Richard Biener; +Cc: Tom de Vries, GCC Patches, Michael Matz
On Thu, Feb 19, 2015 at 01:44:46PM +0100, Richard Biener wrote:
> I'd call it a bug though, and we do have internal fns in
> generic already thus the issue is latent (with ubsan at least).
>
> Which means ok for trunk now.
But the patch should better handle the internal calls right.
I.e. return 0 only if only one, not both CALL_EXPR_FNs are NULL,
or if both are NULL and CALL_EXPR_IFN is different, or if
call_expr_nargs is different.
Jakub
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH][2/5] Add gimple_find_sub_bbs
2015-02-19 12:51 ` Jakub Jelinek
@ 2015-02-19 13:03 ` Marek Polacek
2015-02-19 13:31 ` Richard Biener
2015-02-19 13:07 ` Richard Biener
1 sibling, 1 reply; 39+ messages in thread
From: Marek Polacek @ 2015-02-19 13:03 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: Richard Biener, Tom de Vries, GCC Patches, Michael Matz
On Thu, Feb 19, 2015 at 01:48:58PM +0100, Jakub Jelinek wrote:
> On Thu, Feb 19, 2015 at 01:41:05PM +0100, Richard Biener wrote:
> > I hope we can get rid of this again (which needs re-writing of all
> > targets va-arg gimplification hooks... - we are in need of some
> > generic diamond/triangle CFG pattern builders anyway).
>
> We already have some in asan.c - create_cond_insert_point - perhaps
> it isn't best named and could use some changes to be generally usable.
Yeah, create_cond_insert_point is used in ubsan.c many times, and
there's nothing *san specific in it.
Marek
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH][5/5] Postpone expanding va_arg until pass_stdarg
2015-02-19 12:08 ` [PATCH][5/5] Postpone expanding va_arg until pass_stdarg Tom de Vries
@ 2015-02-19 13:05 ` Richard Biener
2015-02-22 13:24 ` Tom de Vries
0 siblings, 1 reply; 39+ messages in thread
From: Richard Biener @ 2015-02-19 13:05 UTC (permalink / raw)
To: Tom de Vries; +Cc: Jakub Jelinek, GCC Patches, Michael Matz
On Thu, 19 Feb 2015, Tom de Vries wrote:
> On 19-02-15 11:29, Tom de Vries wrote:
> > Hi,
> >
> > I'm posting this patch series for stage1:
> > - 0001-Disable-lang_hooks.gimplify_expr-in-free_lang_data.patch
> > - 0002-Add-gimple_find_sub_bbs.patch
> > - 0003-Factor-optimize_va_list_gpr_fpr_size-out-of-pass_std.patch
> > - 0004-Handle-internal_fn-in-operand_equal_p.patch
> > - 0005-Postpone-expanding-va_arg-until-pass_stdarg.patch
> >
> > The patch series - based on Michael's initial patch - postpones expanding
> > va_arg
> > until pass_stdarg, which makes pass_stdarg more robust.
> >
> > Bootstrapped and reg-tested on x86_64 using all languages, with unix/ and
> > unix/-m32 testing.
> >
> > I'll post the patches in reply to this email.
> >
>
> This patch postpones expanding va_arg until pass_stdarg.
>
> We add a new internal function IFN_VA_ARG. During gimplification, we map
> VA_ARG_EXPR onto a CALL_EXPR with IFN_VA_ARG, which is then gimplified in to a
> gimple_call. At pass_stdarg, we expand the IFN_VA_ARG gimple_call into actual
> code.
>
> There are a few implementation details worth mentioning:
> - passing the type beyond gimplification is done by adding a NULL pointer-
> to-type to IFN_VA_ARG.
> - there is special handling for IFN_VA_ARG that would be most suited to be
> placed in gimplify_va_arg_expr. However, that function lacks the scope for
> the special handling, so it's placed awkwardly in gimplify_modify_expr.
> - there's special handling in case the va_arg type is variable-sized.
> gimplify_modify_expr adds a WITH_SIZE_EXPR to the CALL_EXPR IFN_VA_ARG for
> variable-sized types. However, this is gimplified into a gimple_call which
> does not have the possibility to wrap it's result in a WITH_SIZE_EXPR. So
> we're adding the size argument of the WITH_SIZE_EXPR as argument to
> IFN_VA_ARG, and at expansion in pass_stdarg, wrap the result of the
> gimplification of IFN_VA_ARG in a WITH_SIZE_EXPR, such that the subsequent
> gimplify_assign will generate a memcpy if necessary.
> - when gimplifying the va_arg argument ap, it may not be addressable. So
> gimplification will generate a copy ap.1 = ap, and use &ap.1 as argument.
> This means that we have to copy back the ap.1 value to ap after IFN_VA_ARG.
> The copy is classified by the va_list_gpr/fpr_size optimization as an
> escape, so it inhibits optimization. The tree-ssa/stdarg-2.c f15 update is
> because of that.
>
> OK for stage1?
Looks mostly good, though it looks like with -O0 this doesn't delay
lowering of va-arg and thus won't "fix" offloading. Can you instead
introduce a PROP_gimple_lva, provide it by the stdarg pass and add
a pass_lower_vaarg somewhere where pass_lower_complex_O0 is run
that runs of !PROP_gimple_lva (and also provides it), and require
PROP_gimple_lva by pass_expand? (just look for PROP_gimple_lcx for
the complex stuff to get an idea what needs to be touched)
Thanks,
Richard.
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH][2/5] Add gimple_find_sub_bbs
2015-02-19 12:51 ` Jakub Jelinek
2015-02-19 13:03 ` Marek Polacek
@ 2015-02-19 13:07 ` Richard Biener
1 sibling, 0 replies; 39+ messages in thread
From: Richard Biener @ 2015-02-19 13:07 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: Tom de Vries, GCC Patches, Michael Matz
On Thu, 19 Feb 2015, Jakub Jelinek wrote:
> On Thu, Feb 19, 2015 at 01:41:05PM +0100, Richard Biener wrote:
> > I hope we can get rid of this again (which needs re-writing of all
> > targets va-arg gimplification hooks... - we are in need of some
> > generic diamond/triangle CFG pattern builders anyway).
>
> We already have some in asan.c - create_cond_insert_point - perhaps
> it isn't best named and could use some changes to be generally usable.
Yeah - we should look at individual uses to see what is needed.
I guess some are to generate a COND_EXPR with arms that can have
side-effects, thus produce a single value in two different ways
dependent on a condition. Others are to just guard a code region.
Richard.
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH][4/5] Handle internal_fn in operand_equal_p
2015-02-19 12:59 ` Jakub Jelinek
@ 2015-02-19 13:09 ` Richard Biener
2015-02-19 15:31 ` Tom de Vries
0 siblings, 1 reply; 39+ messages in thread
From: Richard Biener @ 2015-02-19 13:09 UTC (permalink / raw)
To: Jakub Jelinek; +Cc: Tom de Vries, GCC Patches, Michael Matz
On Thu, 19 Feb 2015, Jakub Jelinek wrote:
> On Thu, Feb 19, 2015 at 01:44:46PM +0100, Richard Biener wrote:
> > I'd call it a bug though, and we do have internal fns in
> > generic already thus the issue is latent (with ubsan at least).
> >
> > Which means ok for trunk now.
>
> But the patch should better handle the internal calls right.
> I.e. return 0 only if only one, not both CALL_EXPR_FNs are NULL,
> or if both are NULL and CALL_EXPR_IFN is different, or if
> call_expr_nargs is different.
The question is whether generic call handling works (esp. call_expr_flags
works correctly - the argument compare should work fine already).
Tom - care to update the patch?
Thanks,
Richard.
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH][2/5] Add gimple_find_sub_bbs
2015-02-19 13:03 ` Marek Polacek
@ 2015-02-19 13:31 ` Richard Biener
0 siblings, 0 replies; 39+ messages in thread
From: Richard Biener @ 2015-02-19 13:31 UTC (permalink / raw)
To: Marek Polacek; +Cc: Jakub Jelinek, Tom de Vries, GCC Patches, Michael Matz
On Thu, 19 Feb 2015, Marek Polacek wrote:
> On Thu, Feb 19, 2015 at 01:48:58PM +0100, Jakub Jelinek wrote:
> > On Thu, Feb 19, 2015 at 01:41:05PM +0100, Richard Biener wrote:
> > > I hope we can get rid of this again (which needs re-writing of all
> > > targets va-arg gimplification hooks... - we are in need of some
> > > generic diamond/triangle CFG pattern builders anyway).
> >
> > We already have some in asan.c - create_cond_insert_point - perhaps
> > it isn't best named and could use some changes to be generally usable.
>
> Yeah, create_cond_insert_point is used in ubsan.c many times, and
> there's nothing *san specific in it.
It's also quite low-level. I'm more looking for sth like
val = gimple_build_cond_value (EQ_EXPR, comp_lhs, comp_rhs,
true_value, true_gimple_seq,
false_value, false_gimple_seq);
which then returns a simplified value (in case all is optimizable
to a constant for example) or produces
if (comp_lhs == comp_rhs)
true_gimple_seq;
else
false_gimple_seq;
val = PHI <true_value, false_value>
so basically allows you to build the true/false values with
gimple_build (or force_gimple_operand for legacy code) and
constructs the diamond/triangle and a PHI together with the
controlling predicate.
Richard.
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH][4/5] Handle internal_fn in operand_equal_p
2015-02-19 13:09 ` Richard Biener
@ 2015-02-19 15:31 ` Tom de Vries
2015-02-20 11:59 ` Richard Biener
0 siblings, 1 reply; 39+ messages in thread
From: Tom de Vries @ 2015-02-19 15:31 UTC (permalink / raw)
To: Richard Biener, Jakub Jelinek; +Cc: GCC Patches, Michael Matz
On 19-02-15 14:07, Richard Biener wrote:
> On Thu, 19 Feb 2015, Jakub Jelinek wrote:
>
>> On Thu, Feb 19, 2015 at 01:44:46PM +0100, Richard Biener wrote:
>>> I'd call it a bug though, and we do have internal fns in
>>> generic already thus the issue is latent (with ubsan at least).
>>>
>>> Which means ok for trunk now.
>>
>> But the patch should better handle the internal calls right.
>> I.e. return 0 only if only one, not both CALL_EXPR_FNs are NULL,
>> or if both are NULL and CALL_EXPR_IFN is different, or if
>> call_expr_nargs is different.
>
> The question is whether generic call handling works (esp. call_expr_flags
> works correctly - the argument compare should work fine already).
>
> Tom - care to update the patch?
>
I agree, the current patch is conservative and we can do better.
But I think it's wiser to do that as a stage1 follow-up, and commit this
conservative patch for stage4. Is that acceptable?
Thanks,
- Tom
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH][4/5] Handle internal_fn in operand_equal_p
2015-02-19 15:31 ` Tom de Vries
@ 2015-02-20 11:59 ` Richard Biener
2015-02-22 13:13 ` Tom de Vries
0 siblings, 1 reply; 39+ messages in thread
From: Richard Biener @ 2015-02-20 11:59 UTC (permalink / raw)
To: Tom de Vries; +Cc: Jakub Jelinek, GCC Patches, Michael Matz
On Thu, 19 Feb 2015, Tom de Vries wrote:
> On 19-02-15 14:07, Richard Biener wrote:
> > On Thu, 19 Feb 2015, Jakub Jelinek wrote:
> >
> > > On Thu, Feb 19, 2015 at 01:44:46PM +0100, Richard Biener wrote:
> > > > I'd call it a bug though, and we do have internal fns in
> > > > generic already thus the issue is latent (with ubsan at least).
> > > >
> > > > Which means ok for trunk now.
> > >
> > > But the patch should better handle the internal calls right.
> > > I.e. return 0 only if only one, not both CALL_EXPR_FNs are NULL,
> > > or if both are NULL and CALL_EXPR_IFN is different, or if
> > > call_expr_nargs is different.
> >
> > The question is whether generic call handling works (esp. call_expr_flags
> > works correctly - the argument compare should work fine already).
> >
> > Tom - care to update the patch?
> >
>
> I agree, the current patch is conservative and we can do better.
> But I think it's wiser to do that as a stage1 follow-up, and commit this
> conservative patch for stage4. Is that acceptable?
Then just defer it for stage1 completely. If a problem pops up with
GCC 5 we can backport the proper patch together with a testcase.
Richard.
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH][2/5] Add gimple_find_sub_bbs
2015-02-19 12:41 ` Richard Biener
2015-02-19 12:51 ` Jakub Jelinek
@ 2015-02-22 12:47 ` Tom de Vries
1 sibling, 0 replies; 39+ messages in thread
From: Tom de Vries @ 2015-02-22 12:47 UTC (permalink / raw)
To: Richard Biener; +Cc: Jakub Jelinek, GCC Patches, Michael Matz
[-- Attachment #1: Type: text/plain, Size: 1351 bytes --]
On 19-02-15 13:41, Richard Biener wrote:
> On Thu, 19 Feb 2015, Tom de Vries wrote:
>
>> On 19-02-15 11:29, Tom de Vries wrote:
>>> Hi,
>>>
>>> I'm posting this patch series for stage1:
>>> - 0001-Disable-lang_hooks.gimplify_expr-in-free_lang_data.patch
>>> - 0002-Add-gimple_find_sub_bbs.patch
>>> - 0003-Factor-optimize_va_list_gpr_fpr_size-out-of-pass_std.patch
>>> - 0004-Handle-internal_fn-in-operand_equal_p.patch
>>> - 0005-Postpone-expanding-va_arg-until-pass_stdarg.patch
>>>
>>> The patch series - based on Michael's initial patch - postpones expanding
>>> va_arg
>>> until pass_stdarg, which makes pass_stdarg more robust.
>>>
>>> Bootstrapped and reg-tested on x86_64 using all languages, with unix/ and
>>> unix/-m32 testing.
>>>
>>> I'll post the patches in reply to this email.
>>
>> This patch adds gimple_find_sub_bbs. The function adds a gimple sequence at a
>> point in a basic block, and takes care to split up the sequence itself into
>> new basic blocks where necessary.
>>
>> OK for stage1?
>
> I hope we can get rid of this again (which needs re-writing of all
> targets va-arg gimplification hooks... - we are in need of some
> generic diamond/triangle CFG pattern builders anyway).
>
> Ok with a comment on the function that this is transitional and
> should not be used in new code.
>
Updated with comment.
Thanks,
- Tom
[-- Attachment #2: 0002-Add-gimple_find_sub_bbs.patch --]
[-- Type: text/x-patch, Size: 9862 bytes --]
2015-02-17 Tom de Vries <tom@codesourcery.com>
Michael Matz <matz@suse.de>
* tree-cfg.c (make_blocks_1): Factor out of ...
(make_blocks): ... here.
(make_edges_bb): Factor out of ...
(make_edges): ... here.
(gimple_find_sub_bbs): New function.
* tree-cfg.h (gimple_find_sub_bbs): Declare.
---
gcc/tree-cfg.c | 262 +++++++++++++++++++++++++++++++++++----------------------
gcc/tree-cfg.h | 1 +
2 files changed, 163 insertions(+), 100 deletions(-)
diff --git a/gcc/tree-cfg.c b/gcc/tree-cfg.c
index 006bc08..3aaa0b9 100644
--- a/gcc/tree-cfg.c
+++ b/gcc/tree-cfg.c
@@ -513,16 +513,15 @@ gimple_call_initialize_ctrl_altering (gimple stmt)
}
-/* Build a flowgraph for the sequence of stmts SEQ. */
+/* Insert SEQ after BB and build a flowgraph. */
-static void
-make_blocks (gimple_seq seq)
+static basic_block
+make_blocks_1 (gimple_seq seq, basic_block bb)
{
gimple_stmt_iterator i = gsi_start (seq);
gimple stmt = NULL;
bool start_new_block = true;
bool first_stmt_of_seq = true;
- basic_block bb = ENTRY_BLOCK_PTR_FOR_FN (cfun);
while (!gsi_end_p (i))
{
@@ -579,8 +578,16 @@ make_blocks (gimple_seq seq)
gsi_next (&i);
first_stmt_of_seq = false;
}
+ return bb;
}
+/* Build a flowgraph for the sequence of stmts SEQ. */
+
+static void
+make_blocks (gimple_seq seq)
+{
+ make_blocks_1 (seq, ENTRY_BLOCK_PTR_FOR_FN (cfun));
+}
/* Create and return a new empty basic block after bb AFTER. */
@@ -807,6 +814,112 @@ handle_abnormal_edges (basic_block *dispatcher_bbs,
make_edge (*dispatcher, for_bb, EDGE_ABNORMAL);
}
+/* Creates outgoing edges for BB. Returns 1 when it ends with an
+ computed goto, returns 2 when it ends with a statement that
+ might return to this function via an nonlocal goto, otherwise
+ return 0. Updates *PCUR_REGION with the OMP region this BB is in. */
+
+static int
+make_edges_bb (basic_block bb, struct omp_region **pcur_region, int *pomp_index)
+{
+ gimple last = last_stmt (bb);
+ bool fallthru = false;
+ int ret = 0;
+
+ if (!last)
+ return ret;
+
+ switch (gimple_code (last))
+ {
+ case GIMPLE_GOTO:
+ if (make_goto_expr_edges (bb))
+ ret = 1;
+ fallthru = false;
+ break;
+ case GIMPLE_RETURN:
+ {
+ edge e = make_edge (bb, EXIT_BLOCK_PTR_FOR_FN (cfun), 0);
+ e->goto_locus = gimple_location (last);
+ fallthru = false;
+ }
+ break;
+ case GIMPLE_COND:
+ make_cond_expr_edges (bb);
+ fallthru = false;
+ break;
+ case GIMPLE_SWITCH:
+ make_gimple_switch_edges (as_a <gswitch *> (last), bb);
+ fallthru = false;
+ break;
+ case GIMPLE_RESX:
+ make_eh_edges (last);
+ fallthru = false;
+ break;
+ case GIMPLE_EH_DISPATCH:
+ fallthru = make_eh_dispatch_edges (as_a <geh_dispatch *> (last));
+ break;
+
+ case GIMPLE_CALL:
+ /* If this function receives a nonlocal goto, then we need to
+ make edges from this call site to all the nonlocal goto
+ handlers. */
+ if (stmt_can_make_abnormal_goto (last))
+ ret = 2;
+
+ /* If this statement has reachable exception handlers, then
+ create abnormal edges to them. */
+ make_eh_edges (last);
+
+ /* BUILTIN_RETURN is really a return statement. */
+ if (gimple_call_builtin_p (last, BUILT_IN_RETURN))
+ {
+ make_edge (bb, EXIT_BLOCK_PTR_FOR_FN (cfun), 0);
+ fallthru = false;
+ }
+ /* Some calls are known not to return. */
+ else
+ fallthru = !(gimple_call_flags (last) & ECF_NORETURN);
+ break;
+
+ case GIMPLE_ASSIGN:
+ /* A GIMPLE_ASSIGN may throw internally and thus be considered
+ control-altering. */
+ if (is_ctrl_altering_stmt (last))
+ make_eh_edges (last);
+ fallthru = true;
+ break;
+
+ case GIMPLE_ASM:
+ make_gimple_asm_edges (bb);
+ fallthru = true;
+ break;
+
+ CASE_GIMPLE_OMP:
+ fallthru = make_gimple_omp_edges (bb, pcur_region, pomp_index);
+ break;
+
+ case GIMPLE_TRANSACTION:
+ {
+ tree abort_label
+ = gimple_transaction_label (as_a <gtransaction *> (last));
+ if (abort_label)
+ make_edge (bb, label_to_block (abort_label), EDGE_TM_ABORT);
+ fallthru = true;
+ }
+ break;
+
+ default:
+ gcc_assert (!stmt_ends_bb_p (last));
+ fallthru = true;
+ break;
+ }
+
+ if (fallthru)
+ make_edge (bb, bb->next_bb, EDGE_FALLTHRU);
+
+ return ret;
+}
+
/* Join all the blocks in the flowgraph. */
static void
@@ -828,107 +941,19 @@ make_edges (void)
/* Traverse the basic block array placing edges. */
FOR_EACH_BB_FN (bb, cfun)
{
- gimple last = last_stmt (bb);
- bool fallthru;
+ int mer;
if (bb_to_omp_idx)
bb_to_omp_idx[bb->index] = cur_omp_region_idx;
- if (last)
- {
- enum gimple_code code = gimple_code (last);
- switch (code)
- {
- case GIMPLE_GOTO:
- if (make_goto_expr_edges (bb))
- ab_edge_goto.safe_push (bb);
- fallthru = false;
- break;
- case GIMPLE_RETURN:
- {
- edge e = make_edge (bb, EXIT_BLOCK_PTR_FOR_FN (cfun), 0);
- e->goto_locus = gimple_location (last);
- fallthru = false;
- }
- break;
- case GIMPLE_COND:
- make_cond_expr_edges (bb);
- fallthru = false;
- break;
- case GIMPLE_SWITCH:
- make_gimple_switch_edges (as_a <gswitch *> (last), bb);
- fallthru = false;
- break;
- case GIMPLE_RESX:
- make_eh_edges (last);
- fallthru = false;
- break;
- case GIMPLE_EH_DISPATCH:
- fallthru = make_eh_dispatch_edges (as_a <geh_dispatch *> (last));
- break;
-
- case GIMPLE_CALL:
- /* If this function receives a nonlocal goto, then we need to
- make edges from this call site to all the nonlocal goto
- handlers. */
- if (stmt_can_make_abnormal_goto (last))
- ab_edge_call.safe_push (bb);
-
- /* If this statement has reachable exception handlers, then
- create abnormal edges to them. */
- make_eh_edges (last);
-
- /* BUILTIN_RETURN is really a return statement. */
- if (gimple_call_builtin_p (last, BUILT_IN_RETURN))
- {
- make_edge (bb, EXIT_BLOCK_PTR_FOR_FN (cfun), 0);
- fallthru = false;
- }
- /* Some calls are known not to return. */
- else
- fallthru = !(gimple_call_flags (last) & ECF_NORETURN);
- break;
+ mer = make_edges_bb (bb, &cur_region, &cur_omp_region_idx);
+ if (mer == 1)
+ ab_edge_goto.safe_push (bb);
+ else if (mer == 2)
+ ab_edge_call.safe_push (bb);
- case GIMPLE_ASSIGN:
- /* A GIMPLE_ASSIGN may throw internally and thus be considered
- control-altering. */
- if (is_ctrl_altering_stmt (last))
- make_eh_edges (last);
- fallthru = true;
- break;
-
- case GIMPLE_ASM:
- make_gimple_asm_edges (bb);
- fallthru = true;
- break;
-
- CASE_GIMPLE_OMP:
- fallthru = make_gimple_omp_edges (bb, &cur_region,
- &cur_omp_region_idx);
- if (cur_region && bb_to_omp_idx == NULL)
- bb_to_omp_idx = XCNEWVEC (int, n_basic_blocks_for_fn (cfun));
- break;
-
- case GIMPLE_TRANSACTION:
- {
- tree abort_label
- = gimple_transaction_label (as_a <gtransaction *> (last));
- if (abort_label)
- make_edge (bb, label_to_block (abort_label), EDGE_TM_ABORT);
- fallthru = true;
- }
- break;
-
- default:
- gcc_assert (!stmt_ends_bb_p (last));
- fallthru = true;
- }
- }
- else
- fallthru = true;
-
- if (fallthru)
- make_edge (bb, bb->next_bb, EDGE_FALLTHRU);
+ if (cur_region && bb_to_omp_idx == NULL)
+ bb_to_omp_idx = XCNEWVEC (int, n_basic_blocks_for_fn (cfun));
}
/* Computed gotos are hell to deal with, especially if there are
@@ -1008,6 +1033,43 @@ make_edges (void)
fold_cond_expr_cond ();
}
+/* Add SEQ after GSI. Start new bb after GSI, and created further bbs as
+ needed. Returns true if new bbs were created.
+ Note: This is transitional code, and should not be used for new code. We
+ should be able to get rid of this by rewriting all target va-arg
+ gimplification hooks to use an interface gimple_build_cond_value as described
+ in https://gcc.gnu.org/ml/gcc-patches/2015-02/msg01194.html. */
+
+bool
+gimple_find_sub_bbs (gimple_seq seq, gimple_stmt_iterator *gsi)
+{
+ gimple stmt = gsi_stmt (*gsi);
+ basic_block bb = gimple_bb (stmt);
+ basic_block lastbb, afterbb;
+ int old_num_bbs = n_basic_blocks_for_fn (cfun);
+ edge e;
+ lastbb = make_blocks_1 (seq, bb);
+ if (old_num_bbs == n_basic_blocks_for_fn (cfun))
+ return false;
+ e = split_block (bb, stmt);
+ /* Move e->dest to come after the new basic blocks. */
+ afterbb = e->dest;
+ unlink_block (afterbb);
+ link_block (afterbb, lastbb);
+ redirect_edge_succ (e, bb->next_bb);
+ bb = bb->next_bb;
+ while (bb != afterbb)
+ {
+ struct omp_region *cur_region = NULL;
+ int cur_omp_region_idx = 0;
+ int mer = make_edges_bb (bb, &cur_region, &cur_omp_region_idx);
+ gcc_assert (!mer && !cur_region);
+ add_bb_to_loop (bb, afterbb->loop_father);
+ bb = bb->next_bb;
+ }
+ return true;
+}
+
/* Find the next available discriminator value for LOCUS. The
discriminator distinguishes among several basic blocks that
share a common locus, allowing for more accurate sample-based
diff --git a/gcc/tree-cfg.h b/gcc/tree-cfg.h
index cd28a80..2fc1e88 100644
--- a/gcc/tree-cfg.h
+++ b/gcc/tree-cfg.h
@@ -103,5 +103,6 @@ extern void extract_true_false_edges_from_block (basic_block, edge *, edge *);
extern unsigned int execute_fixup_cfg (void);
extern unsigned int split_critical_edges (void);
extern basic_block insert_cond_bb (basic_block, gimple, gimple);
+extern bool gimple_find_sub_bbs (gimple_seq, gimple_stmt_iterator *);
#endif /* _TREE_CFG_H */
--
1.9.1
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH][4/5] Handle internal_fn in operand_equal_p
2015-02-20 11:59 ` Richard Biener
@ 2015-02-22 13:13 ` Tom de Vries
2015-02-23 10:19 ` Richard Biener
0 siblings, 1 reply; 39+ messages in thread
From: Tom de Vries @ 2015-02-22 13:13 UTC (permalink / raw)
To: Richard Biener; +Cc: Jakub Jelinek, GCC Patches, Michael Matz
[-- Attachment #1: Type: text/plain, Size: 1295 bytes --]
On 20-02-15 12:54, Richard Biener wrote:
> On Thu, 19 Feb 2015, Tom de Vries wrote:
>
>> On 19-02-15 14:07, Richard Biener wrote:
>>> On Thu, 19 Feb 2015, Jakub Jelinek wrote:
>>>
>>>> On Thu, Feb 19, 2015 at 01:44:46PM +0100, Richard Biener wrote:
>>>>> I'd call it a bug though, and we do have internal fns in
>>>>> generic already thus the issue is latent (with ubsan at least).
>>>>>
>>>>> Which means ok for trunk now.
>>>>
>>>> But the patch should better handle the internal calls right.
>>>> I.e. return 0 only if only one, not both CALL_EXPR_FNs are NULL,
>>>> or if both are NULL and CALL_EXPR_IFN is different, or if
>>>> call_expr_nargs is different.
>>>
>>> The question is whether generic call handling works (esp. call_expr_flags
>>> works correctly - the argument compare should work fine already).
>>>
>>> Tom - care to update the patch?
>>>
>>
>> I agree, the current patch is conservative and we can do betterns,
-
.
>> But I think it's wiser to do that as a stage1 follow-up, and commit this
>> conservative patch for stage4. Is that acceptable?
>
> Then just defer it for stage1 completely. If a problem pops up with
> GCC 5 we can backport the proper patch together with a testcase.
>
Updated patch according to Jakub's comments, retested.
OK for stage1?
Thanks,
- Tom
[-- Attachment #2: 0004-Handle-internal_fn-in-operand_equal_p.patch --]
[-- Type: text/x-patch, Size: 1836 bytes --]
2015-02-17 Tom de Vries <tom@codesourcery.com>
* fold-const.c (operand_equal_p): Handle INTERNAL_FNs.
* calls.c (call_expr_flags): Same.
---
gcc/calls.c | 2 ++
gcc/fold-const.c | 23 +++++++++++++++++++----
2 files changed, 21 insertions(+), 4 deletions(-)
diff --git a/gcc/calls.c b/gcc/calls.c
index ec44624..2919464 100644
--- a/gcc/calls.c
+++ b/gcc/calls.c
@@ -847,6 +847,8 @@ call_expr_flags (const_tree t)
if (decl)
flags = flags_from_decl_or_type (decl);
+ else if (CALL_EXPR_FN (t) == NULL_TREE)
+ flags = internal_fn_flags (CALL_EXPR_IFN (t));
else
{
t = TREE_TYPE (CALL_EXPR_FN (t));
diff --git a/gcc/fold-const.c b/gcc/fold-const.c
index 8377120..3013adb 100644
--- a/gcc/fold-const.c
+++ b/gcc/fold-const.c
@@ -3032,11 +3032,26 @@ operand_equal_p (const_tree arg0, const_tree arg1, unsigned int flags)
switch (TREE_CODE (arg0))
{
case CALL_EXPR:
- /* If the CALL_EXPRs call different functions, then they
- clearly can not be equal. */
- if (! operand_equal_p (CALL_EXPR_FN (arg0), CALL_EXPR_FN (arg1),
- flags))
+ if ((CALL_EXPR_FN (arg0) == NULL_TREE)
+ != (CALL_EXPR_FN (arg1) == NULL_TREE))
+ /* If not both CALL_EXPRs are either internal or normal function
+ functions, then they are not equal. */
return 0;
+ else if (CALL_EXPR_FN (arg0) == NULL_TREE)
+ {
+ /* If the CALL_EXPRs call different internal functions, then they
+ are not equal. */
+ if (CALL_EXPR_IFN (arg0) != CALL_EXPR_IFN (arg1))
+ return 0;
+ }
+ else
+ {
+ /* If the CALL_EXPRs call different functions, then they are not
+ equal. */
+ if (! operand_equal_p (CALL_EXPR_FN (arg0), CALL_EXPR_FN (arg1),
+ flags))
+ return 0;
+ }
{
unsigned int cef = call_expr_flags (arg0);
--
1.9.1
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH][5/5] Postpone expanding va_arg until pass_stdarg
2015-02-19 13:05 ` Richard Biener
@ 2015-02-22 13:24 ` Tom de Vries
2015-02-23 9:03 ` Michael Matz
2015-03-10 15:31 ` [PING] [PATCH][5a/5] Postpone expanding va_arg until pass_stdarg Tom de Vries
0 siblings, 2 replies; 39+ messages in thread
From: Tom de Vries @ 2015-02-22 13:24 UTC (permalink / raw)
To: Richard Biener; +Cc: Jakub Jelinek, GCC Patches, Michael Matz
[-- Attachment #1: Type: text/plain, Size: 3568 bytes --]
On 19-02-15 14:03, Richard Biener wrote:
> On Thu, 19 Feb 2015, Tom de Vries wrote:
>
>> On 19-02-15 11:29, Tom de Vries wrote:
>>> Hi,
>>>
>>> I'm posting this patch series for stage1:
>>> - 0001-Disable-lang_hooks.gimplify_expr-in-free_lang_data.patch
>>> - 0002-Add-gimple_find_sub_bbs.patch
>>> - 0003-Factor-optimize_va_list_gpr_fpr_size-out-of-pass_std.patch
>>> - 0004-Handle-internal_fn-in-operand_equal_p.patch
>>> - 0005-Postpone-expanding-va_arg-until-pass_stdarg.patch
>>>
>>> The patch series - based on Michael's initial patch - postpones expanding
>>> va_arg
>>> until pass_stdarg, which makes pass_stdarg more robust.
>>>
>>> Bootstrapped and reg-tested on x86_64 using all languages, with unix/ and
>>> unix/-m32 testing.
>>>
>>> I'll post the patches in reply to this email.
>>>
>>
>> This patch postpones expanding va_arg until pass_stdarg.
>>
>> We add a new internal function IFN_VA_ARG. During gimplification, we map
>> VA_ARG_EXPR onto a CALL_EXPR with IFN_VA_ARG, which is then gimplified in to a
>> gimple_call. At pass_stdarg, we expand the IFN_VA_ARG gimple_call into actual
>> code.
>>
>> There are a few implementation details worth mentioning:
>> - passing the type beyond gimplification is done by adding a NULL pointer-
>> to-type to IFN_VA_ARG.
>> - there is special handling for IFN_VA_ARG that would be most suited to be
>> placed in gimplify_va_arg_expr. However, that function lacks the scope for
>> the special handling, so it's placed awkwardly in gimplify_modify_expr.
>> - there's special handling in case the va_arg type is variable-sized.
>> gimplify_modify_expr adds a WITH_SIZE_EXPR to the CALL_EXPR IFN_VA_ARG for
>> variable-sized types. However, this is gimplified into a gimple_call which
>> does not have the possibility to wrap it's result in a WITH_SIZE_EXPR. So
>> we're adding the size argument of the WITH_SIZE_EXPR as argument to
>> IFN_VA_ARG, and at expansion in pass_stdarg, wrap the result of the
>> gimplification of IFN_VA_ARG in a WITH_SIZE_EXPR, such that the subsequent
>> gimplify_assign will generate a memcpy if necessary.
>> - when gimplifying the va_arg argument ap, it may not be addressable. So
>> gimplification will generate a copy ap.1 = ap, and use &ap.1 as argument.
>> This means that we have to copy back the ap.1 value to ap after IFN_VA_ARG.
>> The copy is classified by the va_list_gpr/fpr_size optimization as an
>> escape, so it inhibits optimization. The tree-ssa/stdarg-2.c f15 update is
>> because of that.
>>
>> OK for stage1?
>
> Looks mostly good, though it looks like with -O0 this doesn't delay
> lowering of va-arg and thus won't "fix" offloading. Can you instead
> introduce a PROP_gimple_lva, provide it by the stdarg pass and add
> a pass_lower_vaarg somewhere where pass_lower_complex_O0 is run
> that runs of !PROP_gimple_lva (and also provides it), and require
> PROP_gimple_lva by pass_expand? (just look for PROP_gimple_lcx for
> the complex stuff to get an idea what needs to be touched)
>
Updated according to comments.
Furthermore (having updated the patch series to recent trunk), I'm dropping the
ACCEL_COMPILER bit in pass_stdarg::gate. AFAIU the comment there relates to this
patch.
Retested as before.
OK for stage1?
Btw, I'm wondering if as run-time optimization we can tentatively set
PROP_gimple_lva at the start of the gimple pass, and unset it in
gimplify_va_arg_expr. That way we would avoid the loop in expand_ifn_va_arg_1
(over all bbs and gimples) in functions without va_arg.
Thanks,
- Tom
[-- Attachment #2: 0005-Postpone-expanding-va_arg-until-pass_stdarg.patch --]
[-- Type: text/x-patch, Size: 20341 bytes --]
2015-02-17 Tom de Vries <tom@codesourcery.com>
Michael Matz <matz@suse.de>
* gimple-iterator.c (update_modified_stmts): Remove static.
* gimple-iterator.h (update_modified_stmts): Declare.
* gimplify.c (gimplify_modify_expr): Handle IFN_VA_ARG.
(gimplify_va_arg_internal): New function.
(gimplify_va_arg_expr): Use IFN_VA_ARG.
* gimplify.h (gimplify_va_arg_internal): Declare.
* internal-fn.c (expand_VA_ARG): New unreachable function.
* internal-fn.def (VA_ARG): New DEF_INTERNAL_FN.
* tree-stdarg.c (gimple_call_ifn_va_arg_p, expand_ifn_va_arg_1)
(expand_ifn_va_arg): New function.
(pass_data_stdarg): Add PROP_gimple_lva to properties_provided field.
(pass_stdarg::execute): Call expand_ifn_va_arg.
(pass_data_lower_vaarg): New pass_data.
(pass_lower_vaarg): New gimple_opt_pass.
(pass_lower_vaarg::gate, pass_lower_vaarg::execute)
(make_pass_lower_vaarg): New function.
* cfgexpand.c (pass_data_expand): Add PROP_gimple_lva to
properties_required field.
* passes.def (all_passes): Add pass_lower_vaarg.
* tree-pass.h (PROP_gimple_lva): Add define.
(make_pass_lower_vaarg): Declare.
* gcc.dg/tree-ssa/stdarg-2.c: Change f15 scan-tree-dump for target
x86_64-*-*.
---
gcc/cfgexpand.c | 3 +-
gcc/gimple-iterator.c | 2 +-
gcc/gimple-iterator.h | 1 +
gcc/gimplify.c | 111 ++++++++++++++-----
gcc/gimplify.h | 2 +
gcc/internal-fn.c | 9 ++
gcc/internal-fn.def | 1 +
gcc/passes.def | 1 +
gcc/testsuite/gcc.dg/tree-ssa/stdarg-2.c | 4 +-
gcc/tree-pass.h | 2 +
gcc/tree-stdarg.c | 184 ++++++++++++++++++++++++++++---
11 files changed, 273 insertions(+), 47 deletions(-)
diff --git a/gcc/cfgexpand.c b/gcc/cfgexpand.c
index 7dfe1f6..af5a652 100644
--- a/gcc/cfgexpand.c
+++ b/gcc/cfgexpand.c
@@ -5860,7 +5860,8 @@ const pass_data pass_data_expand =
TV_EXPAND, /* tv_id */
( PROP_ssa | PROP_gimple_leh | PROP_cfg
| PROP_gimple_lcx
- | PROP_gimple_lvec ), /* properties_required */
+ | PROP_gimple_lvec
+ | PROP_gimple_lva), /* properties_required */
PROP_rtl, /* properties_provided */
( PROP_ssa | PROP_trees ), /* properties_destroyed */
0, /* todo_flags_start */
diff --git a/gcc/gimple-iterator.c b/gcc/gimple-iterator.c
index a322390..df29123 100644
--- a/gcc/gimple-iterator.c
+++ b/gcc/gimple-iterator.c
@@ -72,7 +72,7 @@ update_modified_stmt (gimple stmt)
/* Mark the statements in SEQ as modified, and update them. */
-static void
+void
update_modified_stmts (gimple_seq seq)
{
gimple_stmt_iterator gsi;
diff --git a/gcc/gimple-iterator.h b/gcc/gimple-iterator.h
index 6be88dd..ab5759e 100644
--- a/gcc/gimple-iterator.h
+++ b/gcc/gimple-iterator.h
@@ -90,6 +90,7 @@ extern basic_block gsi_insert_seq_on_edge_immediate (edge, gimple_seq);
extern void gsi_commit_edge_inserts (void);
extern void gsi_commit_one_edge_insert (edge, basic_block *);
extern gphi_iterator gsi_start_phis (basic_block);
+extern void update_modified_stmts (gimple_seq);
/* Return a new iterator pointing to GIMPLE_SEQ's first statement. */
diff --git a/gcc/gimplify.c b/gcc/gimplify.c
index 1353ada..8ac6a35 100644
--- a/gcc/gimplify.c
+++ b/gcc/gimplify.c
@@ -4564,6 +4564,7 @@ gimplify_modify_expr (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p,
gimple assign;
location_t loc = EXPR_LOCATION (*expr_p);
gimple_stmt_iterator gsi;
+ tree ap = NULL_TREE, ap_copy = NULL_TREE;
gcc_assert (TREE_CODE (*expr_p) == MODIFY_EXPR
|| TREE_CODE (*expr_p) == INIT_EXPR);
@@ -4640,6 +4641,27 @@ gimplify_modify_expr (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p,
if (ret == GS_ERROR)
return ret;
+ /* In case of va_arg internal fn wrappped in a WITH_SIZE_EXPR, add the type
+ size as argument to the the call. */
+ if (TREE_CODE (*from_p) == WITH_SIZE_EXPR)
+ {
+ tree call = TREE_OPERAND (*from_p, 0);
+ tree vlasize = TREE_OPERAND (*from_p, 1);
+
+ if (TREE_CODE (call) == CALL_EXPR
+ && CALL_EXPR_IFN (call) == IFN_VA_ARG)
+ {
+ tree type = TREE_TYPE (call);
+ tree ap = CALL_EXPR_ARG (call, 0);
+ tree tag = CALL_EXPR_ARG (call, 1);
+ tree newcall = build_call_expr_internal_loc (EXPR_LOCATION (call),
+ IFN_VA_ARG, type, 3, ap,
+ tag, vlasize);
+ tree *call_p = &(TREE_OPERAND (*from_p, 0));
+ *call_p = newcall;
+ }
+ }
+
/* Now see if the above changed *from_p to something we handle specially. */
ret = gimplify_modify_expr_rhs (expr_p, from_p, to_p, pre_p, post_p,
want_value);
@@ -4703,12 +4725,16 @@ gimplify_modify_expr (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p,
enum internal_fn ifn = CALL_EXPR_IFN (*from_p);
auto_vec<tree> vargs (nargs);
+ if (ifn == IFN_VA_ARG)
+ ap = unshare_expr (CALL_EXPR_ARG (*from_p, 0));
for (i = 0; i < nargs; i++)
{
gimplify_arg (&CALL_EXPR_ARG (*from_p, i), pre_p,
EXPR_LOCATION (*from_p));
vargs.quick_push (CALL_EXPR_ARG (*from_p, i));
}
+ if (ifn == IFN_VA_ARG)
+ ap_copy = CALL_EXPR_ARG (*from_p, 0);
call_stmt = gimple_build_call_internal_vec (ifn, vargs);
gimple_set_location (call_stmt, EXPR_LOCATION (*expr_p));
}
@@ -4753,6 +4779,17 @@ gimplify_modify_expr (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p,
gsi = gsi_last (*pre_p);
maybe_fold_stmt (&gsi);
+ /* When gimplifying the &ap argument of va_arg, we might end up with
+ ap.1 = ap
+ va_arg (&ap.1, 0B)
+ We need to assign ap.1 back to ap, otherwise va_arg has no effect on
+ ap. */
+ if (ap != NULL_TREE
+ && TREE_CODE (ap) == ADDR_EXPR
+ && TREE_CODE (ap_copy) == ADDR_EXPR
+ && TREE_OPERAND (ap, 0) != TREE_OPERAND (ap_copy, 0))
+ gimplify_assign (TREE_OPERAND (ap, 0), TREE_OPERAND (ap_copy, 0), pre_p);
+
if (want_value)
{
*expr_p = TREE_THIS_VOLATILE (*to_p) ? *from_p : unshare_expr (*to_p);
@@ -9290,16 +9327,53 @@ dummy_object (tree type)
return build2 (MEM_REF, type, t, t);
}
+/* Call the target expander for evaluating a va_arg call of VALIST
+ and TYPE. */
+
+tree
+gimplify_va_arg_internal (tree valist, tree type, location_t loc,
+ gimple_seq *pre_p, gimple_seq *post_p)
+{
+ tree have_va_type = TREE_TYPE (valist);
+ tree cano_type = targetm.canonical_va_list_type (have_va_type);
+
+ if (cano_type != NULL_TREE)
+ have_va_type = cano_type;
+
+ /* Make it easier for the backends by protecting the valist argument
+ from multiple evaluations. */
+ if (TREE_CODE (have_va_type) == ARRAY_TYPE)
+ {
+ /* For this case, the backends will be expecting a pointer to
+ TREE_TYPE (abi), but it's possible we've
+ actually been given an array (an actual TARGET_FN_ABI_VA_LIST).
+ So fix it. */
+ if (TREE_CODE (TREE_TYPE (valist)) == ARRAY_TYPE)
+ {
+ tree p1 = build_pointer_type (TREE_TYPE (have_va_type));
+ valist = fold_convert_loc (loc, p1,
+ build_fold_addr_expr_loc (loc, valist));
+ }
+
+ gimplify_expr (&valist, pre_p, post_p, is_gimple_val, fb_rvalue);
+ }
+ else
+ gimplify_expr (&valist, pre_p, post_p, is_gimple_min_lval, fb_lvalue);
+
+ return targetm.gimplify_va_arg_expr (valist, type, pre_p, post_p);
+}
+
/* Gimplify __builtin_va_arg, aka VA_ARG_EXPR, which is not really a
builtin function, but a very special sort of operator. */
enum gimplify_status
-gimplify_va_arg_expr (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p)
+gimplify_va_arg_expr (tree *expr_p, gimple_seq *pre_p,
+ gimple_seq *post_p ATTRIBUTE_UNUSED)
{
tree promoted_type, have_va_type;
tree valist = TREE_OPERAND (*expr_p, 0);
tree type = TREE_TYPE (*expr_p);
- tree t;
+ tree t, tag, ap;
location_t loc = EXPR_LOCATION (*expr_p);
/* Verify that valist is of the proper type. */
@@ -9351,36 +9425,13 @@ gimplify_va_arg_expr (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p)
*expr_p = dummy_object (type);
return GS_ALL_DONE;
}
- else
- {
- /* Make it easier for the backends by protecting the valist argument
- from multiple evaluations. */
- if (TREE_CODE (have_va_type) == ARRAY_TYPE)
- {
- /* For this case, the backends will be expecting a pointer to
- TREE_TYPE (abi), but it's possible we've
- actually been given an array (an actual TARGET_FN_ABI_VA_LIST).
- So fix it. */
- if (TREE_CODE (TREE_TYPE (valist)) == ARRAY_TYPE)
- {
- tree p1 = build_pointer_type (TREE_TYPE (have_va_type));
- valist = fold_convert_loc (loc, p1,
- build_fold_addr_expr_loc (loc, valist));
- }
-
- gimplify_expr (&valist, pre_p, post_p, is_gimple_val, fb_rvalue);
- }
- else
- gimplify_expr (&valist, pre_p, post_p, is_gimple_min_lval, fb_lvalue);
- if (!targetm.gimplify_va_arg_expr)
- /* FIXME: Once most targets are converted we should merely
- assert this is non-null. */
- return GS_ALL_DONE;
+ /* Transform a VA_ARG_EXPR into an VA_ARG internal function. */
+ ap = build_fold_addr_expr_loc (loc, valist);
+ tag = build_int_cst (build_pointer_type (type), 0);
+ *expr_p = build_call_expr_internal_loc (loc, IFN_VA_ARG, type, 2, ap, tag);
- *expr_p = targetm.gimplify_va_arg_expr (valist, type, pre_p, post_p);
- return GS_OK;
- }
+ return GS_OK;
}
/* Build a new GIMPLE_ASSIGN tuple and append it to the end of *SEQ_P.
diff --git a/gcc/gimplify.h b/gcc/gimplify.h
index 615925c..bad8e0f 100644
--- a/gcc/gimplify.h
+++ b/gcc/gimplify.h
@@ -82,6 +82,8 @@ extern void gimplify_function_tree (tree);
extern enum gimplify_status gimplify_va_arg_expr (tree *, gimple_seq *,
gimple_seq *);
gimple gimplify_assign (tree, tree, gimple_seq *);
+extern tree gimplify_va_arg_internal (tree, tree, location_t, gimple_seq *,
+ gimple_seq *);
/* Return true if gimplify_one_sizepos doesn't need to gimplify
expr (when in TYPE_SIZE{,_UNIT} and similar type/decl size/bitsize
diff --git a/gcc/internal-fn.c b/gcc/internal-fn.c
index e402825..0053ed9 100644
--- a/gcc/internal-fn.c
+++ b/gcc/internal-fn.c
@@ -1972,6 +1972,15 @@ expand_BUILTIN_EXPECT (gcall *stmt)
emit_move_insn (target, val);
}
+/* IFN_VA_ARG is supposed to be expanded at pass_stdarg. So this dummy function
+ should never be called. */
+
+static void
+expand_VA_ARG (gcall *stmt ATTRIBUTE_UNUSED)
+{
+ gcc_unreachable ();
+}
+
/* Routines to expand each internal function, indexed by function number.
Each routine has the prototype:
diff --git a/gcc/internal-fn.def b/gcc/internal-fn.def
index 032ce6c..f557c64 100644
--- a/gcc/internal-fn.def
+++ b/gcc/internal-fn.def
@@ -62,3 +62,4 @@ DEF_INTERNAL_FN (ADD_OVERFLOW, ECF_CONST | ECF_LEAF | ECF_NOTHROW, NULL)
DEF_INTERNAL_FN (SUB_OVERFLOW, ECF_CONST | ECF_LEAF | ECF_NOTHROW, NULL)
DEF_INTERNAL_FN (MUL_OVERFLOW, ECF_CONST | ECF_LEAF | ECF_NOTHROW, NULL)
DEF_INTERNAL_FN (TSAN_FUNC_EXIT, ECF_NOVOPS | ECF_LEAF | ECF_NOTHROW, NULL)
+DEF_INTERNAL_FN (VA_ARG, 0, NULL)
diff --git a/gcc/passes.def b/gcc/passes.def
index 2bc5dcd..b9d396f 100644
--- a/gcc/passes.def
+++ b/gcc/passes.def
@@ -342,6 +342,7 @@ along with GCC; see the file COPYING3. If not see
NEXT_PASS (pass_tm_edges);
POP_INSERT_PASSES ()
NEXT_PASS (pass_vtable_verify);
+ NEXT_PASS (pass_lower_vaarg);
NEXT_PASS (pass_lower_vector);
NEXT_PASS (pass_lower_complex_O0);
NEXT_PASS (pass_asan_O0);
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/stdarg-2.c b/gcc/testsuite/gcc.dg/tree-ssa/stdarg-2.c
index fe39da3..5a74280 100644
--- a/gcc/testsuite/gcc.dg/tree-ssa/stdarg-2.c
+++ b/gcc/testsuite/gcc.dg/tree-ssa/stdarg-2.c
@@ -288,9 +288,9 @@ f15 (int i, ...)
f15_1 (ap);
va_end (ap);
}
-/* { dg-final { scan-tree-dump "f15: va_list escapes 0, needs to save \[148\] GPR units and \[1-9\]\[0-9\]* FPR units" "stdarg" { target { { i?86-*-* x86_64-*-* } && { ! { ia32 || llp64 } } } } } } */
+/* { dg-final { scan-tree-dump "f15: va_list escapes 0, needs to save \[148\] GPR units and \[1-9\]\[0-9\]* FPR units" "stdarg" { target { { i?86-*-* } && { ! { ia32 || llp64 } } } } } } */
/* { dg-final { scan-tree-dump "f15: va_list escapes 0, needs to save \[148\] GPR units and \[1-9\]\[0-9\]* FPR units" "stdarg" { target { powerpc*-*-linux* && { powerpc_fprs && ilp32 } } } } } */
-/* { dg-final { scan-tree-dump "f15: va_list escapes 1, needs to save all GPR units and all FPR units" "stdarg" { target alpha*-*-linux* } } } */
+/* { dg-final { scan-tree-dump "f15: va_list escapes 1, needs to save all GPR units and all FPR units" "stdarg" { target { { alpha*-*-linux* } || { { x86_64-*-* } && { ! { ia32 || llp64 } } } } } } } */
/* { dg-final { scan-tree-dump "f15: va_list escapes 0, needs to save 1 GPR units and 2 FPR units" "stdarg" { target s390*-*-linux* } } } */
/* { dg-final { scan-tree-dump-not "f15: va_list escapes 0, needs to save 0 GPR units" "stdarg" { target { { i?86-*-* x86_64-*-* } && ia32 } } } } */
/* { dg-final { scan-tree-dump-not "f15: va_list escapes 0, needs to save 0 GPR units" "stdarg" { target ia64-*-* } } } */
diff --git a/gcc/tree-pass.h b/gcc/tree-pass.h
index b59ae7a..bcefb3e 100644
--- a/gcc/tree-pass.h
+++ b/gcc/tree-pass.h
@@ -221,6 +221,7 @@ protected:
#define PROP_loops (1 << 11) /* preserve loop structures */
#define PROP_gimple_lvec (1 << 12) /* lowered vector */
#define PROP_gimple_eomp (1 << 13) /* no OpenMP directives */
+#define PROP_gimple_lva (1 << 14) /* No va_arg internal function. */
#define PROP_trees \
(PROP_gimple_any | PROP_gimple_lcf | PROP_gimple_leh | PROP_gimple_lomp)
@@ -588,6 +589,7 @@ extern gimple_opt_pass *make_pass_early_inline (gcc::context *ctxt);
extern gimple_opt_pass *make_pass_inline_parameters (gcc::context *ctxt);
extern gimple_opt_pass *make_pass_update_address_taken (gcc::context *ctxt);
extern gimple_opt_pass *make_pass_convert_switch (gcc::context *ctxt);
+extern gimple_opt_pass *make_pass_lower_vaarg (gcc::context *ctxt);
/* Current optimization pass. */
extern opt_pass *current_pass;
diff --git a/gcc/tree-stdarg.c b/gcc/tree-stdarg.c
index 8d221a4..16a9e2c 100644
--- a/gcc/tree-stdarg.c
+++ b/gcc/tree-stdarg.c
@@ -52,11 +52,14 @@ along with GCC; see the file COPYING3. If not see
#include "gimple-iterator.h"
#include "gimple-walk.h"
#include "gimple-ssa.h"
+#include "gimplify.h"
#include "tree-phinodes.h"
#include "ssa-iterators.h"
#include "stringpool.h"
#include "tree-ssanames.h"
+#include "tree-into-ssa.h"
#include "sbitmap.h"
+#include "tree-cfg.h"
#include "tree-pass.h"
#include "tree-stdarg.h"
@@ -1016,6 +1019,112 @@ finish:
}
}
+/* Return true if STMT is IFN_VA_ARG. */
+
+static bool
+gimple_call_ifn_va_arg_p (gimple stmt)
+{
+ return (is_gimple_call (stmt)
+ && gimple_call_internal_p (stmt)
+ && gimple_call_internal_fn (stmt) == IFN_VA_ARG);
+}
+
+/* Expand IFN_VA_ARGs in FUN. */
+
+static void
+expand_ifn_va_arg_1 (function *fun)
+{
+ bool modified = false;
+ basic_block bb;
+ gimple_stmt_iterator i;
+
+ FOR_EACH_BB_FN (bb, fun)
+ for (i = gsi_start_bb (bb); !gsi_end_p (i); gsi_next (&i))
+ {
+ gimple stmt = gsi_stmt (i);
+ tree ap, expr, lhs, type;
+ gimple_seq pre = NULL, post = NULL;
+
+ if (!gimple_call_ifn_va_arg_p (stmt))
+ continue;
+
+ modified = true;
+
+ type = TREE_TYPE (TREE_TYPE (gimple_call_arg (stmt, 1)));
+ ap = gimple_call_arg (stmt, 0);
+ ap = build_fold_indirect_ref (ap);
+
+ push_gimplify_context (false);
+
+ expr = gimplify_va_arg_internal (ap, type, gimple_location (stmt),
+ &pre, &post);
+
+ lhs = gimple_call_lhs (stmt);
+ if (lhs != NULL_TREE)
+ {
+ gcc_assert (useless_type_conversion_p (TREE_TYPE (lhs), type));
+
+ if (gimple_call_num_args (stmt) == 3)
+ {
+ /* We've transported the size of with WITH_SIZE_EXPR here as
+ the 3rd argument of the internal fn call. Now reinstate
+ it. */
+ tree size = gimple_call_arg (stmt, 2);
+ expr = build2 (WITH_SIZE_EXPR, TREE_TYPE (expr), expr, size);
+ }
+
+ /* We use gimplify_assign here, rather than gimple_build_assign,
+ because gimple_assign knows how to deal with variable-sized
+ types. */
+ gimplify_assign (lhs, expr, &pre);
+ }
+
+ pop_gimplify_context (NULL);
+
+ gimple_seq_add_seq (&pre, post);
+ update_modified_stmts (pre);
+
+ /* Add the sequence after IFN_VA_ARG. This splits the bb right
+ after IFN_VA_ARG, and adds the sequence in one or more new bbs
+ inbetween. */
+ gimple_find_sub_bbs (pre, &i);
+
+ /* Remove the IFN_VA_ARG gimple_call. It's the last stmt in the
+ bb. */
+ gsi_remove (&i, true);
+ gcc_assert (gsi_end_p (i));
+
+ /* We're walking here into the bbs which contain the expansion of
+ IFN_VA_ARG, and will not contain another IFN_VA_ARG that needs
+ expanding. We could try to skip walking these bbs, perhaps by
+ walking backwards over gimples and bbs. */
+ break;
+ }
+
+ if (!modified)
+ return;
+
+ free_dominance_info (CDI_DOMINATORS);
+ update_ssa (TODO_update_ssa);
+}
+
+/* Expand IFN_VA_ARGs in FUN, if necessary. */
+
+static void
+expand_ifn_va_arg (function *fun)
+{
+ if ((fun->curr_properties & PROP_gimple_lva) == 0)
+ expand_ifn_va_arg_1 (fun);
+
+#if ENABLE_CHECKING
+ basic_block bb;
+ gimple_stmt_iterator i;
+ FOR_EACH_BB_FN (bb, fun)
+ for (i = gsi_start_bb (bb); !gsi_end_p (i); gsi_next (&i))
+ gcc_assert (!gimple_call_ifn_va_arg_p (gsi_stmt (i)));
+#endif
+}
+
namespace {
const pass_data pass_data_stdarg =
@@ -1025,7 +1134,7 @@ const pass_data pass_data_stdarg =
OPTGROUP_NONE, /* optinfo_flags */
TV_NONE, /* tv_id */
( PROP_cfg | PROP_ssa ), /* properties_required */
- 0, /* properties_provided */
+ PROP_gimple_lva, /* properties_provided */
0, /* properties_destroyed */
0, /* todo_flags_start */
0, /* todo_flags_finish */
@@ -1039,18 +1148,13 @@ public:
{}
/* opt_pass methods: */
- virtual bool gate (function *fun)
+ virtual bool gate (function *)
{
- return (flag_stdarg_opt
-#ifdef ACCEL_COMPILER
- /* Disable for GCC5 in the offloading compilers, as
- va_list and gpr/fpr counter fields are not merged.
- In GCC6 when stdarg is lowered late this shouldn't be
- an issue. */
- && !in_lto_p
-#endif
- /* This optimization is only for stdarg functions. */
- && fun->stdarg != 0);
+ /* Always run this pass, in order to expand va_arg internal_fns. We
+ also need to do that if fun->stdarg == 0, because a va_arg may also
+ occur in a function without varargs, f.i. if when passing a va_list to
+ another function. */
+ return true;
}
virtual unsigned int execute (function *);
@@ -1060,7 +1164,14 @@ public:
unsigned int
pass_stdarg::execute (function *fun)
{
- optimize_va_list_gpr_fpr_size (fun);
+ /* TODO: Postpone expand_ifn_va_arg till after
+ optimize_va_list_gpr_fpr_size. */
+ expand_ifn_va_arg (fun);
+
+ if (flag_stdarg_opt
+ /* This optimization is only for stdarg functions. */
+ && fun->stdarg != 0)
+ optimize_va_list_gpr_fpr_size (fun);
return 0;
}
@@ -1072,3 +1183,50 @@ make_pass_stdarg (gcc::context *ctxt)
{
return new pass_stdarg (ctxt);
}
+
+namespace {
+
+const pass_data pass_data_lower_vaarg =
+{
+ GIMPLE_PASS, /* type */
+ "lower_vaarg", /* name */
+ OPTGROUP_NONE, /* optinfo_flags */
+ TV_NONE, /* tv_id */
+ ( PROP_cfg | PROP_ssa ), /* properties_required */
+ PROP_gimple_lva, /* properties_provided */
+ 0, /* properties_destroyed */
+ 0, /* todo_flags_start */
+ 0, /* todo_flags_finish */
+};
+
+class pass_lower_vaarg : public gimple_opt_pass
+{
+public:
+ pass_lower_vaarg (gcc::context *ctxt)
+ : gimple_opt_pass (pass_data_lower_vaarg, ctxt)
+ {}
+
+ /* opt_pass methods: */
+ virtual bool gate (function *)
+ {
+ return (cfun->curr_properties & PROP_gimple_lva) == 0;
+ }
+
+ virtual unsigned int execute (function *);
+
+}; // class pass_lower_vaarg
+
+unsigned int
+pass_lower_vaarg::execute (function *fun)
+{
+ expand_ifn_va_arg (fun);
+ return 0;
+}
+
+} // anon namespace
+
+gimple_opt_pass *
+make_pass_lower_vaarg (gcc::context *ctxt)
+{
+ return new pass_lower_vaarg (ctxt);
+}
--
1.9.1
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH][5/5] Postpone expanding va_arg until pass_stdarg
2015-02-22 13:24 ` Tom de Vries
@ 2015-02-23 9:03 ` Michael Matz
2015-02-23 10:36 ` Tom de Vries
2015-03-10 15:31 ` [PING] [PATCH][5a/5] Postpone expanding va_arg until pass_stdarg Tom de Vries
1 sibling, 1 reply; 39+ messages in thread
From: Michael Matz @ 2015-02-23 9:03 UTC (permalink / raw)
To: Tom de Vries; +Cc: Richard Biener, Jakub Jelinek, GCC Patches
Hi,
On Sun, 22 Feb 2015, Tom de Vries wrote:
> Btw, I'm wondering if as run-time optimization we can tentatively set
> PROP_gimple_lva at the start of the gimple pass, and unset it in
> gimplify_va_arg_expr. That way we would avoid the loop in
> expand_ifn_va_arg_1 (over all bbs and gimples) in functions without
> va_arg.
That makes sense.
Ciao,
Michael.
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH][4/5] Handle internal_fn in operand_equal_p
2015-02-22 13:13 ` Tom de Vries
@ 2015-02-23 10:19 ` Richard Biener
2015-02-24 13:09 ` Fwd: " Tom de Vries
0 siblings, 1 reply; 39+ messages in thread
From: Richard Biener @ 2015-02-23 10:19 UTC (permalink / raw)
To: Tom de Vries; +Cc: Jakub Jelinek, GCC Patches, Michael Matz
On Sun, 22 Feb 2015, Tom de Vries wrote:
> On 20-02-15 12:54, Richard Biener wrote:
> > On Thu, 19 Feb 2015, Tom de Vries wrote:
> >
> > > On 19-02-15 14:07, Richard Biener wrote:
> > > > On Thu, 19 Feb 2015, Jakub Jelinek wrote:
> > > >
> > > > > On Thu, Feb 19, 2015 at 01:44:46PM +0100, Richard Biener wrote:
> > > > > > I'd call it a bug though, and we do have internal fns in
> > > > > > generic already thus the issue is latent (with ubsan at least).
> > > > > >
> > > > > > Which means ok for trunk now.
> > > > >
> > > > > But the patch should better handle the internal calls right.
> > > > > I.e. return 0 only if only one, not both CALL_EXPR_FNs are NULL,
> > > > > or if both are NULL and CALL_EXPR_IFN is different, or if
> > > > > call_expr_nargs is different.
> > > >
> > > > The question is whether generic call handling works (esp.
> > > > call_expr_flags
> > > > works correctly - the argument compare should work fine already).
> > > >
> > > > Tom - care to update the patch?
> > > >
> > >
> > > I agree, the current patch is conservative and we can do betterns,
> -
> .
> > > But I think it's wiser to do that as a stage1 follow-up, and commit this
> > > conservative patch for stage4. Is that acceptable?
> >
> > Then just defer it for stage1 completely. If a problem pops up with
> > GCC 5 we can backport the proper patch together with a testcase.
> >
>
> Updated patch according to Jakub's comments, retested.
>
> OK for stage1?
Ok.
Thanks,
Richard.
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH][5/5] Postpone expanding va_arg until pass_stdarg
2015-02-23 9:03 ` Michael Matz
@ 2015-02-23 10:36 ` Tom de Vries
2015-02-24 8:26 ` Tom de Vries
0 siblings, 1 reply; 39+ messages in thread
From: Tom de Vries @ 2015-02-23 10:36 UTC (permalink / raw)
To: Michael Matz; +Cc: Richard Biener, Jakub Jelinek, GCC Patches
[-- Attachment #1: Type: text/plain, Size: 464 bytes --]
On 23-02-15 09:26, Michael Matz wrote:
> Hi,
>
> On Sun, 22 Feb 2015, Tom de Vries wrote:
>
>> Btw, I'm wondering if as run-time optimization we can tentatively set
>> PROP_gimple_lva at the start of the gimple pass, and unset it in
>> gimplify_va_arg_expr. That way we would avoid the loop in
>> expand_ifn_va_arg_1 (over all bbs and gimples) in functions without
>> va_arg.
>
> That makes sense.
>
I'll put this follow-up patch through testing.
Thanks,
- Tom
[-- Attachment #2: 0006-Set-PROP_gimple_lva-for-functions-without-IFN_VA_ARG.patch --]
[-- Type: text/x-patch, Size: 2329 bytes --]
2015-02-23 Tom de Vries <tom@codesourcery.com>
* gimplify.c (gimplify_function_tree): Tentatively set PROP_gimple_lva
in cfun->curr_properties.
(gimplify_va_arg_expr): Clear PROP_gimple_lva in cfun->curr_properties
if we generate an IFN_VA_ARG.
* tree-inline.c (expand_call_inline): Reset PROP_gimple_lva in dest
function if PROP_gimple_lva is not set in src function.
---
gcc/gimplify.c | 10 +++++++++-
gcc/tree-inline.c | 8 ++++++++
2 files changed, 17 insertions(+), 1 deletion(-)
diff --git a/gcc/gimplify.c b/gcc/gimplify.c
index 8ac6a35..497e2fa 100644
--- a/gcc/gimplify.c
+++ b/gcc/gimplify.c
@@ -9218,6 +9218,10 @@ gimplify_function_tree (tree fndecl)
else
push_struct_function (fndecl);
+ /* Tentatively set PROP_gimple_lva here, and reset it in gimplify_va_arg_expr
+ if necessary. */
+ cfun->curr_properties |= PROP_gimple_lva;
+
for (parm = DECL_ARGUMENTS (fndecl); parm ; parm = DECL_CHAIN (parm))
{
/* Preliminarily mark non-addressed complex variables as eligible
@@ -9312,7 +9316,7 @@ gimplify_function_tree (tree fndecl)
}
DECL_SAVED_TREE (fndecl) = NULL_TREE;
- cfun->curr_properties = PROP_gimple_any;
+ cfun->curr_properties |= PROP_gimple_any;
pop_cfun ();
}
@@ -9431,6 +9435,10 @@ gimplify_va_arg_expr (tree *expr_p, gimple_seq *pre_p,
tag = build_int_cst (build_pointer_type (type), 0);
*expr_p = build_call_expr_internal_loc (loc, IFN_VA_ARG, type, 2, ap, tag);
+ /* Clear the tentatively set PROP_gimple_lva, to indicate that IFN_VA_ARG
+ needs to be expanded. */
+ cfun->curr_properties &= ~PROP_gimple_lva;
+
return GS_OK;
}
diff --git a/gcc/tree-inline.c b/gcc/tree-inline.c
index d8abe03..7dfef4f 100644
--- a/gcc/tree-inline.c
+++ b/gcc/tree-inline.c
@@ -4524,6 +4524,14 @@ expand_call_inline (basic_block bb, gimple stmt, copy_body_data *id)
id->src_cfun = DECL_STRUCT_FUNCTION (fn);
id->call_stmt = stmt;
+ /* If the the src function contains an IFN_VA_ARG, then so will the dst
+ function after inlining. */
+ if ((id->src_cfun->curr_properties & PROP_gimple_lva) == 0)
+ {
+ struct function *dst_cfun = DECL_STRUCT_FUNCTION (id->dst_fn);
+ dst_cfun->curr_properties &= ~PROP_gimple_lva;
+ }
+
gcc_assert (!id->src_cfun->after_inlining);
id->entry_bb = bb;
--
1.9.1
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PATCH][5/5] Postpone expanding va_arg until pass_stdarg
2015-02-23 10:36 ` Tom de Vries
@ 2015-02-24 8:26 ` Tom de Vries
2015-03-10 15:31 ` [PING] [PATCH][5b/5] Avoid running expand_ifn_va_arg_1 in functions without va_arg Tom de Vries
0 siblings, 1 reply; 39+ messages in thread
From: Tom de Vries @ 2015-02-24 8:26 UTC (permalink / raw)
To: Richard Biener, Jakub Jelinek; +Cc: Michael Matz, GCC Patches
[-- Attachment #1: Type: text/plain, Size: 619 bytes --]
On 23-02-15 11:09, Tom de Vries wrote:
> On 23-02-15 09:26, Michael Matz wrote:
>> Hi,
>>
>> On Sun, 22 Feb 2015, Tom de Vries wrote:
>>
>>> Btw, I'm wondering if as run-time optimization we can tentatively set
>>> PROP_gimple_lva at the start of the gimple pass, and unset it in
>>> gimplify_va_arg_expr. That way we would avoid the loop in
>>> expand_ifn_va_arg_1 (over all bbs and gimples) in functions without
>>> va_arg.
>>
>> That makes sense.
>>
>
> I'll put this follow-up patch through testing.
Bootstrapped and reg-tested as before together with the rest of the patch series.
OK for stage1?
Thanks,
- Tom
[-- Attachment #2: 0006-Set-PROP_gimple_lva-for-functions-without-IFN_VA_ARG.patch --]
[-- Type: text/x-patch, Size: 2329 bytes --]
2015-02-23 Tom de Vries <tom@codesourcery.com>
* gimplify.c (gimplify_function_tree): Tentatively set PROP_gimple_lva
in cfun->curr_properties.
(gimplify_va_arg_expr): Clear PROP_gimple_lva in cfun->curr_properties
if we generate an IFN_VA_ARG.
* tree-inline.c (expand_call_inline): Reset PROP_gimple_lva in dest
function if PROP_gimple_lva is not set in src function.
---
gcc/gimplify.c | 10 +++++++++-
gcc/tree-inline.c | 8 ++++++++
2 files changed, 17 insertions(+), 1 deletion(-)
diff --git a/gcc/gimplify.c b/gcc/gimplify.c
index 8ac6a35..497e2fa 100644
--- a/gcc/gimplify.c
+++ b/gcc/gimplify.c
@@ -9218,6 +9218,10 @@ gimplify_function_tree (tree fndecl)
else
push_struct_function (fndecl);
+ /* Tentatively set PROP_gimple_lva here, and reset it in gimplify_va_arg_expr
+ if necessary. */
+ cfun->curr_properties |= PROP_gimple_lva;
+
for (parm = DECL_ARGUMENTS (fndecl); parm ; parm = DECL_CHAIN (parm))
{
/* Preliminarily mark non-addressed complex variables as eligible
@@ -9312,7 +9316,7 @@ gimplify_function_tree (tree fndecl)
}
DECL_SAVED_TREE (fndecl) = NULL_TREE;
- cfun->curr_properties = PROP_gimple_any;
+ cfun->curr_properties |= PROP_gimple_any;
pop_cfun ();
}
@@ -9431,6 +9435,10 @@ gimplify_va_arg_expr (tree *expr_p, gimple_seq *pre_p,
tag = build_int_cst (build_pointer_type (type), 0);
*expr_p = build_call_expr_internal_loc (loc, IFN_VA_ARG, type, 2, ap, tag);
+ /* Clear the tentatively set PROP_gimple_lva, to indicate that IFN_VA_ARG
+ needs to be expanded. */
+ cfun->curr_properties &= ~PROP_gimple_lva;
+
return GS_OK;
}
diff --git a/gcc/tree-inline.c b/gcc/tree-inline.c
index d8abe03..7dfef4f 100644
--- a/gcc/tree-inline.c
+++ b/gcc/tree-inline.c
@@ -4524,6 +4524,14 @@ expand_call_inline (basic_block bb, gimple stmt, copy_body_data *id)
id->src_cfun = DECL_STRUCT_FUNCTION (fn);
id->call_stmt = stmt;
+ /* If the the src function contains an IFN_VA_ARG, then so will the dst
+ function after inlining. */
+ if ((id->src_cfun->curr_properties & PROP_gimple_lva) == 0)
+ {
+ struct function *dst_cfun = DECL_STRUCT_FUNCTION (id->dst_fn);
+ dst_cfun->curr_properties &= ~PROP_gimple_lva;
+ }
+
gcc_assert (!id->src_cfun->after_inlining);
id->entry_bb = bb;
--
1.9.1
^ permalink raw reply [flat|nested] 39+ messages in thread
* Fwd: Re: [PATCH][4/5] Handle internal_fn in operand_equal_p
2015-02-23 10:19 ` Richard Biener
@ 2015-02-24 13:09 ` Tom de Vries
0 siblings, 0 replies; 39+ messages in thread
From: Tom de Vries @ 2015-02-24 13:09 UTC (permalink / raw)
To: GCC Patches
[ forwarding. for some reason, this email didn't make it to gcc-patches ml archive ]
-------- Forwarded Message --------
Subject: Re: [PATCH][4/5] Handle internal_fn in operand_equal_p
Date: Mon, 23 Feb 2015 10:03:34 +0100
From: Richard Biener <rguenther@suse.de>
To: Tom de Vries <Tom_deVries@mentor.com>
CC: Jakub Jelinek <jakub@redhat.com>, GCC Patches <gcc-patches@gcc.gnu.org>,
Michael Matz <matz@suse.de>
On Sun, 22 Feb 2015, Tom de Vries wrote:
> On 20-02-15 12:54, Richard Biener wrote:
> > On Thu, 19 Feb 2015, Tom de Vries wrote:
> >
> > > On 19-02-15 14:07, Richard Biener wrote:
> > > > On Thu, 19 Feb 2015, Jakub Jelinek wrote:
> > > >
> > > > > On Thu, Feb 19, 2015 at 01:44:46PM +0100, Richard Biener wrote:
> > > > > > I'd call it a bug though, and we do have internal fns in
> > > > > > generic already thus the issue is latent (with ubsan at least).
> > > > > >
> > > > > > Which means ok for trunk now.
> > > > >
> > > > > But the patch should better handle the internal calls right.
> > > > > I.e. return 0 only if only one, not both CALL_EXPR_FNs are NULL,
> > > > > or if both are NULL and CALL_EXPR_IFN is different, or if
> > > > > call_expr_nargs is different.
> > > >
> > > > The question is whether generic call handling works (esp.
> > > > call_expr_flags
> > > > works correctly - the argument compare should work fine already).
> > > >
> > > > Tom - care to update the patch?
> > > >
> > >
> > > I agree, the current patch is conservative and we can do betterns,
> -
> .
> > > But I think it's wiser to do that as a stage1 follow-up, and commit this
> > > conservative patch for stage4. Is that acceptable?
> >
> > Then just defer it for stage1 completely. If a problem pops up with
> > GCC 5 we can backport the proper patch together with a testcase.
> >
>
> Updated patch according to Jakub's comments, retested.
>
> OK for stage1?
Ok.
Thanks,
Richard.
^ permalink raw reply [flat|nested] 39+ messages in thread
* [PING] [PATCH][5b/5] Avoid running expand_ifn_va_arg_1 in functions without va_arg
2015-02-24 8:26 ` Tom de Vries
@ 2015-03-10 15:31 ` Tom de Vries
2015-04-16 8:50 ` [PING^2] " Tom de Vries
0 siblings, 1 reply; 39+ messages in thread
From: Tom de Vries @ 2015-03-10 15:31 UTC (permalink / raw)
To: Richard Biener, Jakub Jelinek; +Cc: Michael Matz, GCC Patches
[stage1 ping]
[was: Postpone expanding va_arg until pass_stdarg]
On 24-02-15 07:48, Tom de Vries wrote:
> On 23-02-15 11:09, Tom de Vries wrote:
>> On 23-02-15 09:26, Michael Matz wrote:
>>> Hi,
>>>
>>> On Sun, 22 Feb 2015, Tom de Vries wrote:
>>>
>>>> Btw, I'm wondering if as run-time optimization we can tentatively set
>>>> PROP_gimple_lva at the start of the gimple pass, and unset it in
>>>> gimplify_va_arg_expr. That way we would avoid the loop in
>>>> expand_ifn_va_arg_1 (over all bbs and gimples) in functions without
>>>> va_arg.
>>>
>>> That makes sense.
>>>
>>
>> I'll put this follow-up patch through testing.
>
> Bootstrapped and reg-tested as before together with the rest of the patch series.
>
> OK for stage1?
>
Ping.
Thanks,
- Tom
> 0006-Set-PROP_gimple_lva-for-functions-without-IFN_VA_ARG.patch
>
>
> 2015-02-23 Tom de Vries<tom@codesourcery.com>
>
> * gimplify.c (gimplify_function_tree): Tentatively set PROP_gimple_lva
> in cfun->curr_properties.
> (gimplify_va_arg_expr): Clear PROP_gimple_lva in cfun->curr_properties
> if we generate an IFN_VA_ARG.
> * tree-inline.c (expand_call_inline): Reset PROP_gimple_lva in dest
> function if PROP_gimple_lva is not set in src function.
> ---
> gcc/gimplify.c | 10 +++++++++-
> gcc/tree-inline.c | 8 ++++++++
> 2 files changed, 17 insertions(+), 1 deletion(-)
>
> diff --git a/gcc/gimplify.c b/gcc/gimplify.c
> index 8ac6a35..497e2fa 100644
> --- a/gcc/gimplify.c
> +++ b/gcc/gimplify.c
> @@ -9218,6 +9218,10 @@ gimplify_function_tree (tree fndecl)
> else
> push_struct_function (fndecl);
>
> + /* Tentatively set PROP_gimple_lva here, and reset it in gimplify_va_arg_expr
> + if necessary. */
> + cfun->curr_properties |= PROP_gimple_lva;
> +
> for (parm = DECL_ARGUMENTS (fndecl); parm ; parm = DECL_CHAIN (parm))
> {
> /* Preliminarily mark non-addressed complex variables as eligible
> @@ -9312,7 +9316,7 @@ gimplify_function_tree (tree fndecl)
> }
>
> DECL_SAVED_TREE (fndecl) = NULL_TREE;
> - cfun->curr_properties = PROP_gimple_any;
> + cfun->curr_properties |= PROP_gimple_any;
>
> pop_cfun ();
> }
> @@ -9431,6 +9435,10 @@ gimplify_va_arg_expr (tree *expr_p, gimple_seq *pre_p,
> tag = build_int_cst (build_pointer_type (type), 0);
> *expr_p = build_call_expr_internal_loc (loc, IFN_VA_ARG, type, 2, ap, tag);
>
> + /* Clear the tentatively set PROP_gimple_lva, to indicate that IFN_VA_ARG
> + needs to be expanded. */
> + cfun->curr_properties &= ~PROP_gimple_lva;
> +
> return GS_OK;
> }
>
> diff --git a/gcc/tree-inline.c b/gcc/tree-inline.c
> index d8abe03..7dfef4f 100644
> --- a/gcc/tree-inline.c
> +++ b/gcc/tree-inline.c
> @@ -4524,6 +4524,14 @@ expand_call_inline (basic_block bb, gimple stmt, copy_body_data *id)
> id->src_cfun = DECL_STRUCT_FUNCTION (fn);
> id->call_stmt = stmt;
>
> + /* If the the src function contains an IFN_VA_ARG, then so will the dst
> + function after inlining. */
> + if ((id->src_cfun->curr_properties & PROP_gimple_lva) == 0)
> + {
> + struct function *dst_cfun = DECL_STRUCT_FUNCTION (id->dst_fn);
> + dst_cfun->curr_properties &= ~PROP_gimple_lva;
> + }
> +
> gcc_assert (!id->src_cfun->after_inlining);
>
> id->entry_bb = bb;
> -- 1.9.1
>
^ permalink raw reply [flat|nested] 39+ messages in thread
* [PING] [PATCH][5a/5] Postpone expanding va_arg until pass_stdarg
2015-02-22 13:24 ` Tom de Vries
2015-02-23 9:03 ` Michael Matz
@ 2015-03-10 15:31 ` Tom de Vries
2015-04-16 8:50 ` [PING^2] " Tom de Vries
1 sibling, 1 reply; 39+ messages in thread
From: Tom de Vries @ 2015-03-10 15:31 UTC (permalink / raw)
To: Richard Biener; +Cc: Jakub Jelinek, GCC Patches, Michael Matz
[stage1 ping]
On 22-02-15 14:13, Tom de Vries wrote:
> On 19-02-15 14:03, Richard Biener wrote:
>> On Thu, 19 Feb 2015, Tom de Vries wrote:
>>
>>> On 19-02-15 11:29, Tom de Vries wrote:
>>>> Hi,
>>>>
>>>> I'm posting this patch series for stage1:
>>>> - 0001-Disable-lang_hooks.gimplify_expr-in-free_lang_data.patch
>>>> - 0002-Add-gimple_find_sub_bbs.patch
>>>> - 0003-Factor-optimize_va_list_gpr_fpr_size-out-of-pass_std.patch
>>>> - 0004-Handle-internal_fn-in-operand_equal_p.patch
>>>> - 0005-Postpone-expanding-va_arg-until-pass_stdarg.patch
>>>>
>>>> The patch series - based on Michael's initial patch - postpones expanding
>>>> va_arg
>>>> until pass_stdarg, which makes pass_stdarg more robust.
>>>>
>>>> Bootstrapped and reg-tested on x86_64 using all languages, with unix/ and
>>>> unix/-m32 testing.
>>>>
>>>> I'll post the patches in reply to this email.
>>>>
>>>
>>> This patch postpones expanding va_arg until pass_stdarg.
>>>
>>> We add a new internal function IFN_VA_ARG. During gimplification, we map
>>> VA_ARG_EXPR onto a CALL_EXPR with IFN_VA_ARG, which is then gimplified in to a
>>> gimple_call. At pass_stdarg, we expand the IFN_VA_ARG gimple_call into actual
>>> code.
>>>
>>> There are a few implementation details worth mentioning:
>>> - passing the type beyond gimplification is done by adding a NULL pointer-
>>> to-type to IFN_VA_ARG.
>>> - there is special handling for IFN_VA_ARG that would be most suited to be
>>> placed in gimplify_va_arg_expr. However, that function lacks the scope for
>>> the special handling, so it's placed awkwardly in gimplify_modify_expr.
>>> - there's special handling in case the va_arg type is variable-sized.
>>> gimplify_modify_expr adds a WITH_SIZE_EXPR to the CALL_EXPR IFN_VA_ARG for
>>> variable-sized types. However, this is gimplified into a gimple_call which
>>> does not have the possibility to wrap it's result in a WITH_SIZE_EXPR. So
>>> we're adding the size argument of the WITH_SIZE_EXPR as argument to
>>> IFN_VA_ARG, and at expansion in pass_stdarg, wrap the result of the
>>> gimplification of IFN_VA_ARG in a WITH_SIZE_EXPR, such that the subsequent
>>> gimplify_assign will generate a memcpy if necessary.
>>> - when gimplifying the va_arg argument ap, it may not be addressable. So
>>> gimplification will generate a copy ap.1 = ap, and use &ap.1 as argument.
>>> This means that we have to copy back the ap.1 value to ap after IFN_VA_ARG.
>>> The copy is classified by the va_list_gpr/fpr_size optimization as an
>>> escape, so it inhibits optimization. The tree-ssa/stdarg-2.c f15 update is
>>> because of that.
>>>
>>> OK for stage1?
>>
>> Looks mostly good, though it looks like with -O0 this doesn't delay
>> lowering of va-arg and thus won't "fix" offloading. Can you instead
>> introduce a PROP_gimple_lva, provide it by the stdarg pass and add
>> a pass_lower_vaarg somewhere where pass_lower_complex_O0 is run
>> that runs of !PROP_gimple_lva (and also provides it), and require
>> PROP_gimple_lva by pass_expand? (just look for PROP_gimple_lcx for
>> the complex stuff to get an idea what needs to be touched)
>>
>
> Updated according to comments.
>
> Furthermore (having updated the patch series to recent trunk), I'm dropping the
> ACCEL_COMPILER bit in pass_stdarg::gate. AFAIU the comment there relates to this
> patch.
>
> Retested as before.
>
> OK for stage1?
>
Ping.
> Btw, I'm wondering if as run-time optimization we can tentatively set
> PROP_gimple_lva at the start of the gimple pass, and unset it in
> gimplify_va_arg_expr. That way we would avoid the loop in expand_ifn_va_arg_1
> (over all bbs and gimples) in functions without va_arg.
>
Taken care of in follow-up patch 5b.
Thanks,
- Tom
> 0005-Postpone-expanding-va_arg-until-pass_stdarg.patch
>
>
> 2015-02-17 Tom de Vries<tom@codesourcery.com>
> Michael Matz<matz@suse.de>
>
> * gimple-iterator.c (update_modified_stmts): Remove static.
> * gimple-iterator.h (update_modified_stmts): Declare.
> * gimplify.c (gimplify_modify_expr): Handle IFN_VA_ARG.
> (gimplify_va_arg_internal): New function.
> (gimplify_va_arg_expr): Use IFN_VA_ARG.
> * gimplify.h (gimplify_va_arg_internal): Declare.
> * internal-fn.c (expand_VA_ARG): New unreachable function.
> * internal-fn.def (VA_ARG): New DEF_INTERNAL_FN.
> * tree-stdarg.c (gimple_call_ifn_va_arg_p, expand_ifn_va_arg_1)
> (expand_ifn_va_arg): New function.
> (pass_data_stdarg): Add PROP_gimple_lva to properties_provided field.
> (pass_stdarg::execute): Call expand_ifn_va_arg.
> (pass_data_lower_vaarg): New pass_data.
> (pass_lower_vaarg): New gimple_opt_pass.
> (pass_lower_vaarg::gate, pass_lower_vaarg::execute)
> (make_pass_lower_vaarg): New function.
> * cfgexpand.c (pass_data_expand): Add PROP_gimple_lva to
> properties_required field.
> * passes.def (all_passes): Add pass_lower_vaarg.
> * tree-pass.h (PROP_gimple_lva): Add define.
> (make_pass_lower_vaarg): Declare.
>
> * gcc.dg/tree-ssa/stdarg-2.c: Change f15 scan-tree-dump for target
> x86_64-*-*.
> ---
> gcc/cfgexpand.c | 3 +-
> gcc/gimple-iterator.c | 2 +-
> gcc/gimple-iterator.h | 1 +
> gcc/gimplify.c | 111 ++++++++++++++-----
> gcc/gimplify.h | 2 +
> gcc/internal-fn.c | 9 ++
> gcc/internal-fn.def | 1 +
> gcc/passes.def | 1 +
> gcc/testsuite/gcc.dg/tree-ssa/stdarg-2.c | 4 +-
> gcc/tree-pass.h | 2 +
> gcc/tree-stdarg.c | 184 ++++++++++++++++++++++++++++---
> 11 files changed, 273 insertions(+), 47 deletions(-)
>
> diff --git a/gcc/cfgexpand.c b/gcc/cfgexpand.c
> index 7dfe1f6..af5a652 100644
> --- a/gcc/cfgexpand.c
> +++ b/gcc/cfgexpand.c
> @@ -5860,7 +5860,8 @@ const pass_data pass_data_expand =
> TV_EXPAND, /* tv_id */
> ( PROP_ssa | PROP_gimple_leh | PROP_cfg
> | PROP_gimple_lcx
> - | PROP_gimple_lvec ), /* properties_required */
> + | PROP_gimple_lvec
> + | PROP_gimple_lva), /* properties_required */
> PROP_rtl, /* properties_provided */
> ( PROP_ssa | PROP_trees ), /* properties_destroyed */
> 0, /* todo_flags_start */
> diff --git a/gcc/gimple-iterator.c b/gcc/gimple-iterator.c
> index a322390..df29123 100644
> --- a/gcc/gimple-iterator.c
> +++ b/gcc/gimple-iterator.c
> @@ -72,7 +72,7 @@ update_modified_stmt (gimple stmt)
>
> /* Mark the statements in SEQ as modified, and update them. */
>
> -static void
> +void
> update_modified_stmts (gimple_seq seq)
> {
> gimple_stmt_iterator gsi;
> diff --git a/gcc/gimple-iterator.h b/gcc/gimple-iterator.h
> index 6be88dd..ab5759e 100644
> --- a/gcc/gimple-iterator.h
> +++ b/gcc/gimple-iterator.h
> @@ -90,6 +90,7 @@ extern basic_block gsi_insert_seq_on_edge_immediate (edge, gimple_seq);
> extern void gsi_commit_edge_inserts (void);
> extern void gsi_commit_one_edge_insert (edge, basic_block *);
> extern gphi_iterator gsi_start_phis (basic_block);
> +extern void update_modified_stmts (gimple_seq);
>
> /* Return a new iterator pointing to GIMPLE_SEQ's first statement. */
>
> diff --git a/gcc/gimplify.c b/gcc/gimplify.c
> index 1353ada..8ac6a35 100644
> --- a/gcc/gimplify.c
> +++ b/gcc/gimplify.c
> @@ -4564,6 +4564,7 @@ gimplify_modify_expr (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p,
> gimple assign;
> location_t loc = EXPR_LOCATION (*expr_p);
> gimple_stmt_iterator gsi;
> + tree ap = NULL_TREE, ap_copy = NULL_TREE;
>
> gcc_assert (TREE_CODE (*expr_p) == MODIFY_EXPR
> || TREE_CODE (*expr_p) == INIT_EXPR);
> @@ -4640,6 +4641,27 @@ gimplify_modify_expr (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p,
> if (ret == GS_ERROR)
> return ret;
>
> + /* In case of va_arg internal fn wrappped in a WITH_SIZE_EXPR, add the type
> + size as argument to the the call. */
> + if (TREE_CODE (*from_p) == WITH_SIZE_EXPR)
> + {
> + tree call = TREE_OPERAND (*from_p, 0);
> + tree vlasize = TREE_OPERAND (*from_p, 1);
> +
> + if (TREE_CODE (call) == CALL_EXPR
> + && CALL_EXPR_IFN (call) == IFN_VA_ARG)
> + {
> + tree type = TREE_TYPE (call);
> + tree ap = CALL_EXPR_ARG (call, 0);
> + tree tag = CALL_EXPR_ARG (call, 1);
> + tree newcall = build_call_expr_internal_loc (EXPR_LOCATION (call),
> + IFN_VA_ARG, type, 3, ap,
> + tag, vlasize);
> + tree *call_p = &(TREE_OPERAND (*from_p, 0));
> + *call_p = newcall;
> + }
> + }
> +
> /* Now see if the above changed *from_p to something we handle specially. */
> ret = gimplify_modify_expr_rhs (expr_p, from_p, to_p, pre_p, post_p,
> want_value);
> @@ -4703,12 +4725,16 @@ gimplify_modify_expr (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p,
> enum internal_fn ifn = CALL_EXPR_IFN (*from_p);
> auto_vec<tree> vargs (nargs);
>
> + if (ifn == IFN_VA_ARG)
> + ap = unshare_expr (CALL_EXPR_ARG (*from_p, 0));
> for (i = 0; i < nargs; i++)
> {
> gimplify_arg (&CALL_EXPR_ARG (*from_p, i), pre_p,
> EXPR_LOCATION (*from_p));
> vargs.quick_push (CALL_EXPR_ARG (*from_p, i));
> }
> + if (ifn == IFN_VA_ARG)
> + ap_copy = CALL_EXPR_ARG (*from_p, 0);
> call_stmt = gimple_build_call_internal_vec (ifn, vargs);
> gimple_set_location (call_stmt, EXPR_LOCATION (*expr_p));
> }
> @@ -4753,6 +4779,17 @@ gimplify_modify_expr (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p,
> gsi = gsi_last (*pre_p);
> maybe_fold_stmt (&gsi);
>
> + /* When gimplifying the &ap argument of va_arg, we might end up with
> + ap.1 = ap
> + va_arg (&ap.1, 0B)
> + We need to assign ap.1 back to ap, otherwise va_arg has no effect on
> + ap. */
> + if (ap != NULL_TREE
> + && TREE_CODE (ap) == ADDR_EXPR
> + && TREE_CODE (ap_copy) == ADDR_EXPR
> + && TREE_OPERAND (ap, 0) != TREE_OPERAND (ap_copy, 0))
> + gimplify_assign (TREE_OPERAND (ap, 0), TREE_OPERAND (ap_copy, 0), pre_p);
> +
> if (want_value)
> {
> *expr_p = TREE_THIS_VOLATILE (*to_p) ? *from_p : unshare_expr (*to_p);
> @@ -9290,16 +9327,53 @@ dummy_object (tree type)
> return build2 (MEM_REF, type, t, t);
> }
>
> +/* Call the target expander for evaluating a va_arg call of VALIST
> + and TYPE. */
> +
> +tree
> +gimplify_va_arg_internal (tree valist, tree type, location_t loc,
> + gimple_seq *pre_p, gimple_seq *post_p)
> +{
> + tree have_va_type = TREE_TYPE (valist);
> + tree cano_type = targetm.canonical_va_list_type (have_va_type);
> +
> + if (cano_type != NULL_TREE)
> + have_va_type = cano_type;
> +
> + /* Make it easier for the backends by protecting the valist argument
> + from multiple evaluations. */
> + if (TREE_CODE (have_va_type) == ARRAY_TYPE)
> + {
> + /* For this case, the backends will be expecting a pointer to
> + TREE_TYPE (abi), but it's possible we've
> + actually been given an array (an actual TARGET_FN_ABI_VA_LIST).
> + So fix it. */
> + if (TREE_CODE (TREE_TYPE (valist)) == ARRAY_TYPE)
> + {
> + tree p1 = build_pointer_type (TREE_TYPE (have_va_type));
> + valist = fold_convert_loc (loc, p1,
> + build_fold_addr_expr_loc (loc, valist));
> + }
> +
> + gimplify_expr (&valist, pre_p, post_p, is_gimple_val, fb_rvalue);
> + }
> + else
> + gimplify_expr (&valist, pre_p, post_p, is_gimple_min_lval, fb_lvalue);
> +
> + return targetm.gimplify_va_arg_expr (valist, type, pre_p, post_p);
> +}
> +
> /* Gimplify __builtin_va_arg, aka VA_ARG_EXPR, which is not really a
> builtin function, but a very special sort of operator. */
>
> enum gimplify_status
> -gimplify_va_arg_expr (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p)
> +gimplify_va_arg_expr (tree *expr_p, gimple_seq *pre_p,
> + gimple_seq *post_p ATTRIBUTE_UNUSED)
> {
> tree promoted_type, have_va_type;
> tree valist = TREE_OPERAND (*expr_p, 0);
> tree type = TREE_TYPE (*expr_p);
> - tree t;
> + tree t, tag, ap;
> location_t loc = EXPR_LOCATION (*expr_p);
>
> /* Verify that valist is of the proper type. */
> @@ -9351,36 +9425,13 @@ gimplify_va_arg_expr (tree *expr_p, gimple_seq *pre_p, gimple_seq *post_p)
> *expr_p = dummy_object (type);
> return GS_ALL_DONE;
> }
> - else
> - {
> - /* Make it easier for the backends by protecting the valist argument
> - from multiple evaluations. */
> - if (TREE_CODE (have_va_type) == ARRAY_TYPE)
> - {
> - /* For this case, the backends will be expecting a pointer to
> - TREE_TYPE (abi), but it's possible we've
> - actually been given an array (an actual TARGET_FN_ABI_VA_LIST).
> - So fix it. */
> - if (TREE_CODE (TREE_TYPE (valist)) == ARRAY_TYPE)
> - {
> - tree p1 = build_pointer_type (TREE_TYPE (have_va_type));
> - valist = fold_convert_loc (loc, p1,
> - build_fold_addr_expr_loc (loc, valist));
> - }
> -
> - gimplify_expr (&valist, pre_p, post_p, is_gimple_val, fb_rvalue);
> - }
> - else
> - gimplify_expr (&valist, pre_p, post_p, is_gimple_min_lval, fb_lvalue);
>
> - if (!targetm.gimplify_va_arg_expr)
> - /* FIXME: Once most targets are converted we should merely
> - assert this is non-null. */
> - return GS_ALL_DONE;
> + /* Transform a VA_ARG_EXPR into an VA_ARG internal function. */
> + ap = build_fold_addr_expr_loc (loc, valist);
> + tag = build_int_cst (build_pointer_type (type), 0);
> + *expr_p = build_call_expr_internal_loc (loc, IFN_VA_ARG, type, 2, ap, tag);
>
> - *expr_p = targetm.gimplify_va_arg_expr (valist, type, pre_p, post_p);
> - return GS_OK;
> - }
> + return GS_OK;
> }
>
> /* Build a new GIMPLE_ASSIGN tuple and append it to the end of *SEQ_P.
> diff --git a/gcc/gimplify.h b/gcc/gimplify.h
> index 615925c..bad8e0f 100644
> --- a/gcc/gimplify.h
> +++ b/gcc/gimplify.h
> @@ -82,6 +82,8 @@ extern void gimplify_function_tree (tree);
> extern enum gimplify_status gimplify_va_arg_expr (tree *, gimple_seq *,
> gimple_seq *);
> gimple gimplify_assign (tree, tree, gimple_seq *);
> +extern tree gimplify_va_arg_internal (tree, tree, location_t, gimple_seq *,
> + gimple_seq *);
>
> /* Return true if gimplify_one_sizepos doesn't need to gimplify
> expr (when in TYPE_SIZE{,_UNIT} and similar type/decl size/bitsize
> diff --git a/gcc/internal-fn.c b/gcc/internal-fn.c
> index e402825..0053ed9 100644
> --- a/gcc/internal-fn.c
> +++ b/gcc/internal-fn.c
> @@ -1972,6 +1972,15 @@ expand_BUILTIN_EXPECT (gcall *stmt)
> emit_move_insn (target, val);
> }
>
> +/* IFN_VA_ARG is supposed to be expanded at pass_stdarg. So this dummy function
> + should never be called. */
> +
> +static void
> +expand_VA_ARG (gcall *stmt ATTRIBUTE_UNUSED)
> +{
> + gcc_unreachable ();
> +}
> +
> /* Routines to expand each internal function, indexed by function number.
> Each routine has the prototype:
>
> diff --git a/gcc/internal-fn.def b/gcc/internal-fn.def
> index 032ce6c..f557c64 100644
> --- a/gcc/internal-fn.def
> +++ b/gcc/internal-fn.def
> @@ -62,3 +62,4 @@ DEF_INTERNAL_FN (ADD_OVERFLOW, ECF_CONST | ECF_LEAF | ECF_NOTHROW, NULL)
> DEF_INTERNAL_FN (SUB_OVERFLOW, ECF_CONST | ECF_LEAF | ECF_NOTHROW, NULL)
> DEF_INTERNAL_FN (MUL_OVERFLOW, ECF_CONST | ECF_LEAF | ECF_NOTHROW, NULL)
> DEF_INTERNAL_FN (TSAN_FUNC_EXIT, ECF_NOVOPS | ECF_LEAF | ECF_NOTHROW, NULL)
> +DEF_INTERNAL_FN (VA_ARG, 0, NULL)
> diff --git a/gcc/passes.def b/gcc/passes.def
> index 2bc5dcd..b9d396f 100644
> --- a/gcc/passes.def
> +++ b/gcc/passes.def
> @@ -342,6 +342,7 @@ along with GCC; see the file COPYING3. If not see
> NEXT_PASS (pass_tm_edges);
> POP_INSERT_PASSES ()
> NEXT_PASS (pass_vtable_verify);
> + NEXT_PASS (pass_lower_vaarg);
> NEXT_PASS (pass_lower_vector);
> NEXT_PASS (pass_lower_complex_O0);
> NEXT_PASS (pass_asan_O0);
> diff --git a/gcc/testsuite/gcc.dg/tree-ssa/stdarg-2.c b/gcc/testsuite/gcc.dg/tree-ssa/stdarg-2.c
> index fe39da3..5a74280 100644
> --- a/gcc/testsuite/gcc.dg/tree-ssa/stdarg-2.c
> +++ b/gcc/testsuite/gcc.dg/tree-ssa/stdarg-2.c
> @@ -288,9 +288,9 @@ f15 (int i, ...)
> f15_1 (ap);
> va_end (ap);
> }
> -/* { dg-final { scan-tree-dump "f15: va_list escapes 0, needs to save \[148\] GPR units and \[1-9\]\[0-9\]* FPR units" "stdarg" { target { { i?86-*-* x86_64-*-* } && { ! { ia32 || llp64 } } } } } } */
> +/* { dg-final { scan-tree-dump "f15: va_list escapes 0, needs to save \[148\] GPR units and \[1-9\]\[0-9\]* FPR units" "stdarg" { target { { i?86-*-* } && { ! { ia32 || llp64 } } } } } } */
> /* { dg-final { scan-tree-dump "f15: va_list escapes 0, needs to save \[148\] GPR units and \[1-9\]\[0-9\]* FPR units" "stdarg" { target { powerpc*-*-linux* && { powerpc_fprs && ilp32 } } } } } */
> -/* { dg-final { scan-tree-dump "f15: va_list escapes 1, needs to save all GPR units and all FPR units" "stdarg" { target alpha*-*-linux* } } } */
> +/* { dg-final { scan-tree-dump "f15: va_list escapes 1, needs to save all GPR units and all FPR units" "stdarg" { target { { alpha*-*-linux* } || { { x86_64-*-* } && { ! { ia32 || llp64 } } } } } } } */
> /* { dg-final { scan-tree-dump "f15: va_list escapes 0, needs to save 1 GPR units and 2 FPR units" "stdarg" { target s390*-*-linux* } } } */
> /* { dg-final { scan-tree-dump-not "f15: va_list escapes 0, needs to save 0 GPR units" "stdarg" { target { { i?86-*-* x86_64-*-* } && ia32 } } } } */
> /* { dg-final { scan-tree-dump-not "f15: va_list escapes 0, needs to save 0 GPR units" "stdarg" { target ia64-*-* } } } */
> diff --git a/gcc/tree-pass.h b/gcc/tree-pass.h
> index b59ae7a..bcefb3e 100644
> --- a/gcc/tree-pass.h
> +++ b/gcc/tree-pass.h
> @@ -221,6 +221,7 @@ protected:
> #define PROP_loops (1 << 11) /* preserve loop structures */
> #define PROP_gimple_lvec (1 << 12) /* lowered vector */
> #define PROP_gimple_eomp (1 << 13) /* no OpenMP directives */
> +#define PROP_gimple_lva (1 << 14) /* No va_arg internal function. */
>
> #define PROP_trees \
> (PROP_gimple_any | PROP_gimple_lcf | PROP_gimple_leh | PROP_gimple_lomp)
> @@ -588,6 +589,7 @@ extern gimple_opt_pass *make_pass_early_inline (gcc::context *ctxt);
> extern gimple_opt_pass *make_pass_inline_parameters (gcc::context *ctxt);
> extern gimple_opt_pass *make_pass_update_address_taken (gcc::context *ctxt);
> extern gimple_opt_pass *make_pass_convert_switch (gcc::context *ctxt);
> +extern gimple_opt_pass *make_pass_lower_vaarg (gcc::context *ctxt);
>
> /* Current optimization pass. */
> extern opt_pass *current_pass;
> diff --git a/gcc/tree-stdarg.c b/gcc/tree-stdarg.c
> index 8d221a4..16a9e2c 100644
> --- a/gcc/tree-stdarg.c
> +++ b/gcc/tree-stdarg.c
> @@ -52,11 +52,14 @@ along with GCC; see the file COPYING3. If not see
> #include "gimple-iterator.h"
> #include "gimple-walk.h"
> #include "gimple-ssa.h"
> +#include "gimplify.h"
> #include "tree-phinodes.h"
> #include "ssa-iterators.h"
> #include "stringpool.h"
> #include "tree-ssanames.h"
> +#include "tree-into-ssa.h"
> #include "sbitmap.h"
> +#include "tree-cfg.h"
> #include "tree-pass.h"
> #include "tree-stdarg.h"
>
> @@ -1016,6 +1019,112 @@ finish:
> }
> }
>
> +/* Return true if STMT is IFN_VA_ARG. */
> +
> +static bool
> +gimple_call_ifn_va_arg_p (gimple stmt)
> +{
> + return (is_gimple_call (stmt)
> + && gimple_call_internal_p (stmt)
> + && gimple_call_internal_fn (stmt) == IFN_VA_ARG);
> +}
> +
> +/* Expand IFN_VA_ARGs in FUN. */
> +
> +static void
> +expand_ifn_va_arg_1 (function *fun)
> +{
> + bool modified = false;
> + basic_block bb;
> + gimple_stmt_iterator i;
> +
> + FOR_EACH_BB_FN (bb, fun)
> + for (i = gsi_start_bb (bb); !gsi_end_p (i); gsi_next (&i))
> + {
> + gimple stmt = gsi_stmt (i);
> + tree ap, expr, lhs, type;
> + gimple_seq pre = NULL, post = NULL;
> +
> + if (!gimple_call_ifn_va_arg_p (stmt))
> + continue;
> +
> + modified = true;
> +
> + type = TREE_TYPE (TREE_TYPE (gimple_call_arg (stmt, 1)));
> + ap = gimple_call_arg (stmt, 0);
> + ap = build_fold_indirect_ref (ap);
> +
> + push_gimplify_context (false);
> +
> + expr = gimplify_va_arg_internal (ap, type, gimple_location (stmt),
> + &pre, &post);
> +
> + lhs = gimple_call_lhs (stmt);
> + if (lhs != NULL_TREE)
> + {
> + gcc_assert (useless_type_conversion_p (TREE_TYPE (lhs), type));
> +
> + if (gimple_call_num_args (stmt) == 3)
> + {
> + /* We've transported the size of with WITH_SIZE_EXPR here as
> + the 3rd argument of the internal fn call. Now reinstate
> + it. */
> + tree size = gimple_call_arg (stmt, 2);
> + expr = build2 (WITH_SIZE_EXPR, TREE_TYPE (expr), expr, size);
> + }
> +
> + /* We use gimplify_assign here, rather than gimple_build_assign,
> + because gimple_assign knows how to deal with variable-sized
> + types. */
> + gimplify_assign (lhs, expr, &pre);
> + }
> +
> + pop_gimplify_context (NULL);
> +
> + gimple_seq_add_seq (&pre, post);
> + update_modified_stmts (pre);
> +
> + /* Add the sequence after IFN_VA_ARG. This splits the bb right
> + after IFN_VA_ARG, and adds the sequence in one or more new bbs
> + inbetween. */
> + gimple_find_sub_bbs (pre, &i);
> +
> + /* Remove the IFN_VA_ARG gimple_call. It's the last stmt in the
> + bb. */
> + gsi_remove (&i, true);
> + gcc_assert (gsi_end_p (i));
> +
> + /* We're walking here into the bbs which contain the expansion of
> + IFN_VA_ARG, and will not contain another IFN_VA_ARG that needs
> + expanding. We could try to skip walking these bbs, perhaps by
> + walking backwards over gimples and bbs. */
> + break;
> + }
> +
> + if (!modified)
> + return;
> +
> + free_dominance_info (CDI_DOMINATORS);
> + update_ssa (TODO_update_ssa);
> +}
> +
> +/* Expand IFN_VA_ARGs in FUN, if necessary. */
> +
> +static void
> +expand_ifn_va_arg (function *fun)
> +{
> + if ((fun->curr_properties & PROP_gimple_lva) == 0)
> + expand_ifn_va_arg_1 (fun);
> +
> +#if ENABLE_CHECKING
> + basic_block bb;
> + gimple_stmt_iterator i;
> + FOR_EACH_BB_FN (bb, fun)
> + for (i = gsi_start_bb (bb); !gsi_end_p (i); gsi_next (&i))
> + gcc_assert (!gimple_call_ifn_va_arg_p (gsi_stmt (i)));
> +#endif
> +}
> +
> namespace {
>
> const pass_data pass_data_stdarg =
> @@ -1025,7 +1134,7 @@ const pass_data pass_data_stdarg =
> OPTGROUP_NONE, /* optinfo_flags */
> TV_NONE, /* tv_id */
> ( PROP_cfg | PROP_ssa ), /* properties_required */
> - 0, /* properties_provided */
> + PROP_gimple_lva, /* properties_provided */
> 0, /* properties_destroyed */
> 0, /* todo_flags_start */
> 0, /* todo_flags_finish */
> @@ -1039,18 +1148,13 @@ public:
> {}
>
> /* opt_pass methods: */
> - virtual bool gate (function *fun)
> + virtual bool gate (function *)
> {
> - return (flag_stdarg_opt
> -#ifdef ACCEL_COMPILER
> - /* Disable for GCC5 in the offloading compilers, as
> - va_list and gpr/fpr counter fields are not merged.
> - In GCC6 when stdarg is lowered late this shouldn't be
> - an issue. */
> - && !in_lto_p
> -#endif
> - /* This optimization is only for stdarg functions. */
> - && fun->stdarg != 0);
> + /* Always run this pass, in order to expand va_arg internal_fns. We
> + also need to do that if fun->stdarg == 0, because a va_arg may also
> + occur in a function without varargs, f.i. if when passing a va_list to
> + another function. */
> + return true;
> }
>
> virtual unsigned int execute (function *);
> @@ -1060,7 +1164,14 @@ public:
> unsigned int
> pass_stdarg::execute (function *fun)
> {
> - optimize_va_list_gpr_fpr_size (fun);
> + /* TODO: Postpone expand_ifn_va_arg till after
> + optimize_va_list_gpr_fpr_size. */
> + expand_ifn_va_arg (fun);
> +
> + if (flag_stdarg_opt
> + /* This optimization is only for stdarg functions. */
> + && fun->stdarg != 0)
> + optimize_va_list_gpr_fpr_size (fun);
>
> return 0;
> }
> @@ -1072,3 +1183,50 @@ make_pass_stdarg (gcc::context *ctxt)
> {
> return new pass_stdarg (ctxt);
> }
> +
> +namespace {
> +
> +const pass_data pass_data_lower_vaarg =
> +{
> + GIMPLE_PASS, /* type */
> + "lower_vaarg", /* name */
> + OPTGROUP_NONE, /* optinfo_flags */
> + TV_NONE, /* tv_id */
> + ( PROP_cfg | PROP_ssa ), /* properties_required */
> + PROP_gimple_lva, /* properties_provided */
> + 0, /* properties_destroyed */
> + 0, /* todo_flags_start */
> + 0, /* todo_flags_finish */
> +};
> +
> +class pass_lower_vaarg : public gimple_opt_pass
> +{
> +public:
> + pass_lower_vaarg (gcc::context *ctxt)
> + : gimple_opt_pass (pass_data_lower_vaarg, ctxt)
> + {}
> +
> + /* opt_pass methods: */
> + virtual bool gate (function *)
> + {
> + return (cfun->curr_properties & PROP_gimple_lva) == 0;
> + }
> +
> + virtual unsigned int execute (function *);
> +
> +}; // class pass_lower_vaarg
> +
> +unsigned int
> +pass_lower_vaarg::execute (function *fun)
> +{
> + expand_ifn_va_arg (fun);
> + return 0;
> +}
> +
> +} // anon namespace
> +
> +gimple_opt_pass *
> +make_pass_lower_vaarg (gcc::context *ctxt)
> +{
> + return new pass_lower_vaarg (ctxt);
> +}
> -- 1.9.1
>
^ permalink raw reply [flat|nested] 39+ messages in thread
* [PING^2] [PATCH][5a/5] Postpone expanding va_arg until pass_stdarg
2015-03-10 15:31 ` [PING] [PATCH][5a/5] Postpone expanding va_arg until pass_stdarg Tom de Vries
@ 2015-04-16 8:50 ` Tom de Vries
2015-04-16 8:55 ` Richard Biener
0 siblings, 1 reply; 39+ messages in thread
From: Tom de Vries @ 2015-04-16 8:50 UTC (permalink / raw)
To: Richard Biener; +Cc: Jakub Jelinek, GCC Patches, Michael Matz
[stage1 ping^2]
On 10-03-15 16:30, Tom de Vries wrote:
> [stage1 ping]
> On 22-02-15 14:13, Tom de Vries wrote:
>> On 19-02-15 14:03, Richard Biener wrote:
>>> On Thu, 19 Feb 2015, Tom de Vries wrote:
>>>
>>>> On 19-02-15 11:29, Tom de Vries wrote:
>>>>> Hi,
>>>>>
>>>>> I'm posting this patch series for stage1:
>>>>> - 0001-Disable-lang_hooks.gimplify_expr-in-free_lang_data.patch
>>>>> - 0002-Add-gimple_find_sub_bbs.patch
>>>>> - 0003-Factor-optimize_va_list_gpr_fpr_size-out-of-pass_std.patch
>>>>> - 0004-Handle-internal_fn-in-operand_equal_p.patch
>>>>> - 0005-Postpone-expanding-va_arg-until-pass_stdarg.patch
>>>>>
>>>>> The patch series - based on Michael's initial patch - postpones expanding
>>>>> va_arg
>>>>> until pass_stdarg, which makes pass_stdarg more robust.
>>>>>
>>>>> Bootstrapped and reg-tested on x86_64 using all languages, with unix/ and
>>>>> unix/-m32 testing.
>>>>>
>>>>> I'll post the patches in reply to this email.
>>>>>
>>>>
>>>> This patch postpones expanding va_arg until pass_stdarg.
>>>>
>>>> We add a new internal function IFN_VA_ARG. During gimplification, we map
>>>> VA_ARG_EXPR onto a CALL_EXPR with IFN_VA_ARG, which is then gimplified in to a
>>>> gimple_call. At pass_stdarg, we expand the IFN_VA_ARG gimple_call into actual
>>>> code.
>>>>
>>>> There are a few implementation details worth mentioning:
>>>> - passing the type beyond gimplification is done by adding a NULL pointer-
>>>> to-type to IFN_VA_ARG.
>>>> - there is special handling for IFN_VA_ARG that would be most suited to be
>>>> placed in gimplify_va_arg_expr. However, that function lacks the scope for
>>>> the special handling, so it's placed awkwardly in gimplify_modify_expr.
>>>> - there's special handling in case the va_arg type is variable-sized.
>>>> gimplify_modify_expr adds a WITH_SIZE_EXPR to the CALL_EXPR IFN_VA_ARG for
>>>> variable-sized types. However, this is gimplified into a gimple_call which
>>>> does not have the possibility to wrap it's result in a WITH_SIZE_EXPR. So
>>>> we're adding the size argument of the WITH_SIZE_EXPR as argument to
>>>> IFN_VA_ARG, and at expansion in pass_stdarg, wrap the result of the
>>>> gimplification of IFN_VA_ARG in a WITH_SIZE_EXPR, such that the subsequent
>>>> gimplify_assign will generate a memcpy if necessary.
>>>> - when gimplifying the va_arg argument ap, it may not be addressable. So
>>>> gimplification will generate a copy ap.1 = ap, and use &ap.1 as argument.
>>>> This means that we have to copy back the ap.1 value to ap after IFN_VA_ARG.
>>>> The copy is classified by the va_list_gpr/fpr_size optimization as an
>>>> escape, so it inhibits optimization. The tree-ssa/stdarg-2.c f15 update is
>>>> because of that.
>>>>
>>>> OK for stage1?
>>>
>>> Looks mostly good, though it looks like with -O0 this doesn't delay
>>> lowering of va-arg and thus won't "fix" offloading. Can you instead
>>> introduce a PROP_gimple_lva, provide it by the stdarg pass and add
>>> a pass_lower_vaarg somewhere where pass_lower_complex_O0 is run
>>> that runs of !PROP_gimple_lva (and also provides it), and require
>>> PROP_gimple_lva by pass_expand? (just look for PROP_gimple_lcx for
>>> the complex stuff to get an idea what needs to be touched)
>>>
>>
>> Updated according to comments.
>>
>> Furthermore (having updated the patch series to recent trunk), I'm dropping the
>> ACCEL_COMPILER bit in pass_stdarg::gate. AFAIU the comment there relates to this
>> patch.
>>
>> Retested as before.
>>
>> OK for stage1?
>>
>
> Ping.
Ping again.
Patch originally posted at:
https://gcc.gnu.org/ml/gcc-patches/2015-02/msg01332.html .
Thanks,
- Tom
>> Btw, I'm wondering if as run-time optimization we can tentatively set
>> PROP_gimple_lva at the start of the gimple pass, and unset it in
>> gimplify_va_arg_expr. That way we would avoid the loop in expand_ifn_va_arg_1
>> (over all bbs and gimples) in functions without va_arg.
>>
>
> Taken care of in follow-up patch 5b.
^ permalink raw reply [flat|nested] 39+ messages in thread
* [PING^2] [PATCH][5b/5] Avoid running expand_ifn_va_arg_1 in functions without va_arg
2015-03-10 15:31 ` [PING] [PATCH][5b/5] Avoid running expand_ifn_va_arg_1 in functions without va_arg Tom de Vries
@ 2015-04-16 8:50 ` Tom de Vries
2015-04-16 8:56 ` Richard Biener
0 siblings, 1 reply; 39+ messages in thread
From: Tom de Vries @ 2015-04-16 8:50 UTC (permalink / raw)
To: Richard Biener, Jakub Jelinek; +Cc: Michael Matz, GCC Patches
[stage1 ping^2]
On 10-03-15 16:31, Tom de Vries wrote:
> [stage1 ping]
> [was: Postpone expanding va_arg until pass_stdarg]
> On 24-02-15 07:48, Tom de Vries wrote:
>> On 23-02-15 11:09, Tom de Vries wrote:
>>> On 23-02-15 09:26, Michael Matz wrote:
>>>> Hi,
>>>>
>>>> On Sun, 22 Feb 2015, Tom de Vries wrote:
>>>>
>>>>> Btw, I'm wondering if as run-time optimization we can tentatively set
>>>>> PROP_gimple_lva at the start of the gimple pass, and unset it in
>>>>> gimplify_va_arg_expr. That way we would avoid the loop in
>>>>> expand_ifn_va_arg_1 (over all bbs and gimples) in functions without
>>>>> va_arg.
>>>>
>>>> That makes sense.
>>>>
>>>
>>> I'll put this follow-up patch through testing.
>>
>> Bootstrapped and reg-tested as before together with the rest of the patch series.
>>
>> OK for stage1?
>>
>
> Ping.
>
Ping again.
Patch originally posted at:
https://gcc.gnu.org/ml/gcc-patches/2015-02/msg01402.html .
Thanks,
- Tom
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PING^2] [PATCH][5a/5] Postpone expanding va_arg until pass_stdarg
2015-04-16 8:50 ` [PING^2] " Tom de Vries
@ 2015-04-16 8:55 ` Richard Biener
2015-04-21 5:32 ` Bin.Cheng
2015-06-09 11:04 ` Alan Lawrence
0 siblings, 2 replies; 39+ messages in thread
From: Richard Biener @ 2015-04-16 8:55 UTC (permalink / raw)
To: Tom de Vries; +Cc: Jakub Jelinek, GCC Patches, Michael Matz
On Thu, 16 Apr 2015, Tom de Vries wrote:
> [stage1 ping^2]
> On 10-03-15 16:30, Tom de Vries wrote:
> > [stage1 ping]
> > On 22-02-15 14:13, Tom de Vries wrote:
> > > On 19-02-15 14:03, Richard Biener wrote:
> > > > On Thu, 19 Feb 2015, Tom de Vries wrote:
> > > >
> > > > > On 19-02-15 11:29, Tom de Vries wrote:
> > > > > > Hi,
> > > > > >
> > > > > > I'm posting this patch series for stage1:
> > > > > > - 0001-Disable-lang_hooks.gimplify_expr-in-free_lang_data.patch
> > > > > > - 0002-Add-gimple_find_sub_bbs.patch
> > > > > > - 0003-Factor-optimize_va_list_gpr_fpr_size-out-of-pass_std.patch
> > > > > > - 0004-Handle-internal_fn-in-operand_equal_p.patch
> > > > > > - 0005-Postpone-expanding-va_arg-until-pass_stdarg.patch
> > > > > >
> > > > > > The patch series - based on Michael's initial patch - postpones
> > > > > > expanding
> > > > > > va_arg
> > > > > > until pass_stdarg, which makes pass_stdarg more robust.
> > > > > >
> > > > > > Bootstrapped and reg-tested on x86_64 using all languages, with
> > > > > > unix/ and
> > > > > > unix/-m32 testing.
> > > > > >
> > > > > > I'll post the patches in reply to this email.
> > > > > >
> > > > >
> > > > > This patch postpones expanding va_arg until pass_stdarg.
> > > > >
> > > > > We add a new internal function IFN_VA_ARG. During gimplification, we
> > > > > map
> > > > > VA_ARG_EXPR onto a CALL_EXPR with IFN_VA_ARG, which is then gimplified
> > > > > in to a
> > > > > gimple_call. At pass_stdarg, we expand the IFN_VA_ARG gimple_call into
> > > > > actual
> > > > > code.
> > > > >
> > > > > There are a few implementation details worth mentioning:
> > > > > - passing the type beyond gimplification is done by adding a NULL
> > > > > pointer-
> > > > > to-type to IFN_VA_ARG.
> > > > > - there is special handling for IFN_VA_ARG that would be most suited
> > > > > to be
> > > > > placed in gimplify_va_arg_expr. However, that function lacks the
> > > > > scope for
> > > > > the special handling, so it's placed awkwardly in
> > > > > gimplify_modify_expr.
> > > > > - there's special handling in case the va_arg type is variable-sized.
> > > > > gimplify_modify_expr adds a WITH_SIZE_EXPR to the CALL_EXPR
> > > > > IFN_VA_ARG for
> > > > > variable-sized types. However, this is gimplified into a
> > > > > gimple_call which
> > > > > does not have the possibility to wrap it's result in a
> > > > > WITH_SIZE_EXPR. So
> > > > > we're adding the size argument of the WITH_SIZE_EXPR as argument to
> > > > > IFN_VA_ARG, and at expansion in pass_stdarg, wrap the result of the
> > > > > gimplification of IFN_VA_ARG in a WITH_SIZE_EXPR, such that the
> > > > > subsequent
> > > > > gimplify_assign will generate a memcpy if necessary.
> > > > > - when gimplifying the va_arg argument ap, it may not be addressable.
> > > > > So
> > > > > gimplification will generate a copy ap.1 = ap, and use &ap.1 as
> > > > > argument.
> > > > > This means that we have to copy back the ap.1 value to ap after
> > > > > IFN_VA_ARG.
> > > > > The copy is classified by the va_list_gpr/fpr_size optimization as
> > > > > an
> > > > > escape, so it inhibits optimization. The tree-ssa/stdarg-2.c f15
> > > > > update is
> > > > > because of that.
> > > > >
> > > > > OK for stage1?
> > > >
> > > > Looks mostly good, though it looks like with -O0 this doesn't delay
> > > > lowering of va-arg and thus won't "fix" offloading. Can you instead
> > > > introduce a PROP_gimple_lva, provide it by the stdarg pass and add
> > > > a pass_lower_vaarg somewhere where pass_lower_complex_O0 is run
> > > > that runs of !PROP_gimple_lva (and also provides it), and require
> > > > PROP_gimple_lva by pass_expand? (just look for PROP_gimple_lcx for
> > > > the complex stuff to get an idea what needs to be touched)
> > > >
> > >
> > > Updated according to comments.
> > >
> > > Furthermore (having updated the patch series to recent trunk), I'm
> > > dropping the
> > > ACCEL_COMPILER bit in pass_stdarg::gate. AFAIU the comment there relates
> > > to this
> > > patch.
> > >
> > > Retested as before.
> > >
> > > OK for stage1?
> > >
> >
> > Ping.
>
> Ping again.
>
> Patch originally posted at:
> https://gcc.gnu.org/ml/gcc-patches/2015-02/msg01332.html .
Ok.
Thanks,
Richard.
> Thanks,
> - Tom
>
> > > Btw, I'm wondering if as run-time optimization we can tentatively set
> > > PROP_gimple_lva at the start of the gimple pass, and unset it in
> > > gimplify_va_arg_expr. That way we would avoid the loop in
> > > expand_ifn_va_arg_1
> > > (over all bbs and gimples) in functions without va_arg.
> > >
> >
> > Taken care of in follow-up patch 5b.
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PING^2] [PATCH][5b/5] Avoid running expand_ifn_va_arg_1 in functions without va_arg
2015-04-16 8:50 ` [PING^2] " Tom de Vries
@ 2015-04-16 8:56 ` Richard Biener
0 siblings, 0 replies; 39+ messages in thread
From: Richard Biener @ 2015-04-16 8:56 UTC (permalink / raw)
To: Tom de Vries; +Cc: Jakub Jelinek, Michael Matz, GCC Patches
On Thu, 16 Apr 2015, Tom de Vries wrote:
> [stage1 ping^2]
> On 10-03-15 16:31, Tom de Vries wrote:
> > [stage1 ping]
> > [was: Postpone expanding va_arg until pass_stdarg]
> > On 24-02-15 07:48, Tom de Vries wrote:
> > > On 23-02-15 11:09, Tom de Vries wrote:
> > > > On 23-02-15 09:26, Michael Matz wrote:
> > > > > Hi,
> > > > >
> > > > > On Sun, 22 Feb 2015, Tom de Vries wrote:
> > > > >
> > > > > > Btw, I'm wondering if as run-time optimization we can tentatively
> > > > > > set
> > > > > > PROP_gimple_lva at the start of the gimple pass, and unset it in
> > > > > > gimplify_va_arg_expr. That way we would avoid the loop in
> > > > > > expand_ifn_va_arg_1 (over all bbs and gimples) in functions without
> > > > > > va_arg.
> > > > >
> > > > > That makes sense.
> > > > >
> > > >
> > > > I'll put this follow-up patch through testing.
> > >
> > > Bootstrapped and reg-tested as before together with the rest of the patch
> > > series.
> > >
> > > OK for stage1?
> > >
> >
> > Ping.
> >
>
> Ping again.
>
> Patch originally posted at:
> https://gcc.gnu.org/ml/gcc-patches/2015-02/msg01402.html .
Ok.
Thanks,
Richard.
> Thanks,
> - Tom
>
>
>
--
Richard Biener <rguenther@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Jennifer Guild,
Dilip Upmanyu, Graham Norton HRB 21284 (AG Nuernberg)
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PING^2] [PATCH][5a/5] Postpone expanding va_arg until pass_stdarg
2015-04-16 8:55 ` Richard Biener
@ 2015-04-21 5:32 ` Bin.Cheng
2015-06-09 11:04 ` Alan Lawrence
1 sibling, 0 replies; 39+ messages in thread
From: Bin.Cheng @ 2015-04-21 5:32 UTC (permalink / raw)
To: Richard Biener; +Cc: Tom de Vries, Jakub Jelinek, GCC Patches, Michael Matz
On Thu, Apr 16, 2015 at 4:55 PM, Richard Biener <rguenther@suse.de> wrote:
> On Thu, 16 Apr 2015, Tom de Vries wrote:
>
>> [stage1 ping^2]
>> On 10-03-15 16:30, Tom de Vries wrote:
>> > [stage1 ping]
>> > On 22-02-15 14:13, Tom de Vries wrote:
>> > > On 19-02-15 14:03, Richard Biener wrote:
>> > > > On Thu, 19 Feb 2015, Tom de Vries wrote:
>> > > >
>> > > > > On 19-02-15 11:29, Tom de Vries wrote:
>> > > > > > Hi,
>> > > > > >
>> > > > > > I'm posting this patch series for stage1:
>> > > > > > - 0001-Disable-lang_hooks.gimplify_expr-in-free_lang_data.patch
>> > > > > > - 0002-Add-gimple_find_sub_bbs.patch
>> > > > > > - 0003-Factor-optimize_va_list_gpr_fpr_size-out-of-pass_std.patch
>> > > > > > - 0004-Handle-internal_fn-in-operand_equal_p.patch
>> > > > > > - 0005-Postpone-expanding-va_arg-until-pass_stdarg.patch
>> > > > > >
>> > > > > > The patch series - based on Michael's initial patch - postpones
>> > > > > > expanding
>> > > > > > va_arg
>> > > > > > until pass_stdarg, which makes pass_stdarg more robust.
>> > > > > >
>> > > > > > Bootstrapped and reg-tested on x86_64 using all languages, with
>> > > > > > unix/ and
>> > > > > > unix/-m32 testing.
>> > > > > >
>> > > > > > I'll post the patches in reply to this email.
>> > > > > >
>> > > > >
>> > > > > This patch postpones expanding va_arg until pass_stdarg.
>> > > > >
>> > > > > We add a new internal function IFN_VA_ARG. During gimplification, we
>> > > > > map
>> > > > > VA_ARG_EXPR onto a CALL_EXPR with IFN_VA_ARG, which is then gimplified
>> > > > > in to a
>> > > > > gimple_call. At pass_stdarg, we expand the IFN_VA_ARG gimple_call into
>> > > > > actual
>> > > > > code.
>> > > > >
>> > > > > There are a few implementation details worth mentioning:
>> > > > > - passing the type beyond gimplification is done by adding a NULL
>> > > > > pointer-
>> > > > > to-type to IFN_VA_ARG.
>> > > > > - there is special handling for IFN_VA_ARG that would be most suited
>> > > > > to be
>> > > > > placed in gimplify_va_arg_expr. However, that function lacks the
>> > > > > scope for
>> > > > > the special handling, so it's placed awkwardly in
>> > > > > gimplify_modify_expr.
>> > > > > - there's special handling in case the va_arg type is variable-sized.
>> > > > > gimplify_modify_expr adds a WITH_SIZE_EXPR to the CALL_EXPR
>> > > > > IFN_VA_ARG for
>> > > > > variable-sized types. However, this is gimplified into a
>> > > > > gimple_call which
>> > > > > does not have the possibility to wrap it's result in a
>> > > > > WITH_SIZE_EXPR. So
>> > > > > we're adding the size argument of the WITH_SIZE_EXPR as argument to
>> > > > > IFN_VA_ARG, and at expansion in pass_stdarg, wrap the result of the
>> > > > > gimplification of IFN_VA_ARG in a WITH_SIZE_EXPR, such that the
>> > > > > subsequent
>> > > > > gimplify_assign will generate a memcpy if necessary.
>> > > > > - when gimplifying the va_arg argument ap, it may not be addressable.
>> > > > > So
>> > > > > gimplification will generate a copy ap.1 = ap, and use &ap.1 as
>> > > > > argument.
>> > > > > This means that we have to copy back the ap.1 value to ap after
>> > > > > IFN_VA_ARG.
>> > > > > The copy is classified by the va_list_gpr/fpr_size optimization as
>> > > > > an
>> > > > > escape, so it inhibits optimization. The tree-ssa/stdarg-2.c f15
>> > > > > update is
>> > > > > because of that.
>> > > > >
>> > > > > OK for stage1?
>> > > >
>> > > > Looks mostly good, though it looks like with -O0 this doesn't delay
>> > > > lowering of va-arg and thus won't "fix" offloading. Can you instead
>> > > > introduce a PROP_gimple_lva, provide it by the stdarg pass and add
>> > > > a pass_lower_vaarg somewhere where pass_lower_complex_O0 is run
>> > > > that runs of !PROP_gimple_lva (and also provides it), and require
>> > > > PROP_gimple_lva by pass_expand? (just look for PROP_gimple_lcx for
>> > > > the complex stuff to get an idea what needs to be touched)
>> > > >
>> > >
>> > > Updated according to comments.
>> > >
>> > > Furthermore (having updated the patch series to recent trunk), I'm
>> > > dropping the
>> > > ACCEL_COMPILER bit in pass_stdarg::gate. AFAIU the comment there relates
>> > > to this
>> > > patch.
>> > >
>> > > Retested as before.
>> > >
>> > > OK for stage1?
>> > >
>> >
>> > Ping.
>>
>> Ping again.
>>
>> Patch originally posted at:
>> https://gcc.gnu.org/ml/gcc-patches/2015-02/msg01332.html .
>
> Ok.
>
> Thanks,
> Richard.
>
>> Thanks,
>> - Tom
>>
>> > > Btw, I'm wondering if as run-time optimization we can tentatively set
>> > > PROP_gimple_lva at the start of the gimple pass, and unset it in
>> > > gimplify_va_arg_expr. That way we would avoid the loop in
>> > > expand_ifn_va_arg_1
>> > > (over all bbs and gimples) in functions without va_arg.
>> > >
>> >
>> > Taken care of in follow-up patch 5b.
Hi,
This patch causes stdarg test failure on aarch64 for both linux and
elf variants. Seems wrong code is generated.
FAIL: gcc.c-torture/execute/stdarg-1.c -O0 execution test
FAIL: gcc.c-torture/execute/stdarg-1.c -O1 execution test
FAIL: gcc.c-torture/execute/stdarg-1.c -O2 execution test
FAIL: gcc.c-torture/execute/stdarg-1.c -O2 -flto
-fno-use-linker-plugin -flto-partition=none execution test
FAIL: gcc.c-torture/execute/stdarg-1.c -O2 -flto -fuse-linker-plugin
-fno-fat-lto-objects execution test
FAIL: gcc.c-torture/execute/stdarg-1.c -O3 -fomit-frame-pointer
execution test
FAIL: gcc.c-torture/execute/stdarg-1.c -O3 -g execution test
FAIL: gcc.c-torture/execute/stdarg-1.c -Os execution test
FAIL: gcc.c-torture/execute/va-arg-11.c -O0 execution test
FAIL: gcc.c-torture/execute/va-arg-11.c -O1 execution test
FAIL: gcc.c-torture/execute/va-arg-11.c -O2 execution test
FAIL: gcc.c-torture/execute/va-arg-11.c -O2 -flto
-fno-use-linker-plugin -flto-partition=none execution test
FAIL: gcc.c-torture/execute/va-arg-11.c -O2 -flto
-fuse-linker-plugin -fno-fat-lto-objects execution test
FAIL: gcc.c-torture/execute/va-arg-11.c -O3 -fomit-frame-pointer
execution test
FAIL: gcc.c-torture/execute/va-arg-11.c -O3 -fomit-frame-pointer
-funroll-all-loops -finline-functions execution test
FAIL: gcc.c-torture/execute/va-arg-11.c -O3 -fomit-frame-pointer
-funroll-loops execution test
FAIL: gcc.c-torture/execute/va-arg-11.c -O3 -g execution test
FAIL: gcc.c-torture/execute/va-arg-11.c -Os execution test
It may also causes ICE on arm-none-linux-gnueabihf.
compiler exited with status 1
output is:
/projects/.../src/gcc/gcc/testsuite/gcc.c-torture/execute/stdarg-2.c:
In function 'f3':
/projects/.../src/gcc/gcc/testsuite/gcc.c-torture/execute/stdarg-2.c:61:1:
error: incorrect sharing of tree nodes
aps[4]
# .MEM_5 = VDEF <.MEM_11>
aps[4] = aps[4];
/projects/.../src/gcc/gcc/testsuite/gcc.c-torture/execute/stdarg-2.c:61:1:
internal compiler error: verify_gimple failed
0xae4893 verify_gimple_in_cfg(function*, bool)
/projects/.../src/gcc/gcc/tree-cfg.c:5136
0x9e3f2f execute_function_todo
/projects/.../src/gcc/gcc/passes.c:1946
0x9e48d3 execute_todo
/projects/.../src/gcc/gcc/passes.c:2003
Please submit a full bug report,
with preprocessed source if appropriate.
Please include the complete backtrace with any bug report.
See <http://gcc.gnu.org/bugs.html> for instructions.
FAIL: gcc.c-torture/execute/stdarg-2.c -O0 (internal compiler error)
FAIL: gcc.c-torture/execute/stdarg-2.c -O0 (test for excess errors)
Thanks,
bin
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PING^2] [PATCH][5a/5] Postpone expanding va_arg until pass_stdarg
2015-04-16 8:55 ` Richard Biener
2015-04-21 5:32 ` Bin.Cheng
@ 2015-06-09 11:04 ` Alan Lawrence
2015-06-09 11:07 ` Richard Biener
1 sibling, 1 reply; 39+ messages in thread
From: Alan Lawrence @ 2015-06-09 11:04 UTC (permalink / raw)
To: Richard Biener; +Cc: Tom de Vries, Jakub Jelinek, GCC Patches, Michael Matz
Hmmm. One side effect of this is that the line number information available in
the target hook gimplify_va_arg_expr, is now just the name of the containing
function, rather than the specific use of va_arg. Is there some way to get this
more precise location (e.g. gimple_location(stmt) in expand_ifn_va_arg_1, the
only caller of said hook)? I don't really want to have to add an extra parameter
to the target hook...
--Alan
Richard Biener wrote:
> On Thu, 16 Apr 2015, Tom de Vries wrote:
>
>> [stage1 ping^2]
>> On 10-03-15 16:30, Tom de Vries wrote:
>>> [stage1 ping]
>>> On 22-02-15 14:13, Tom de Vries wrote:
>>>> On 19-02-15 14:03, Richard Biener wrote:
>>>>> On Thu, 19 Feb 2015, Tom de Vries wrote:
>>>>>
>>>>>> On 19-02-15 11:29, Tom de Vries wrote:
>>>>>>> Hi,
>>>>>>>
>>>>>>> I'm posting this patch series for stage1:
>>>>>>> - 0001-Disable-lang_hooks.gimplify_expr-in-free_lang_data.patch
>>>>>>> - 0002-Add-gimple_find_sub_bbs.patch
>>>>>>> - 0003-Factor-optimize_va_list_gpr_fpr_size-out-of-pass_std.patch
>>>>>>> - 0004-Handle-internal_fn-in-operand_equal_p.patch
>>>>>>> - 0005-Postpone-expanding-va_arg-until-pass_stdarg.patch
>>>>>>>
>>>>>>> The patch series - based on Michael's initial patch - postpones
>>>>>>> expanding
>>>>>>> va_arg
>>>>>>> until pass_stdarg, which makes pass_stdarg more robust.
>>>>>>>
>>>>>>> Bootstrapped and reg-tested on x86_64 using all languages, with
>>>>>>> unix/ and
>>>>>>> unix/-m32 testing.
>>>>>>>
>>>>>>> I'll post the patches in reply to this email.
>>>>>>>
>>>>>> This patch postpones expanding va_arg until pass_stdarg.
>>>>>>
>>>>>> We add a new internal function IFN_VA_ARG. During gimplification, we
>>>>>> map
>>>>>> VA_ARG_EXPR onto a CALL_EXPR with IFN_VA_ARG, which is then gimplified
>>>>>> in to a
>>>>>> gimple_call. At pass_stdarg, we expand the IFN_VA_ARG gimple_call into
>>>>>> actual
>>>>>> code.
>>>>>>
>>>>>> There are a few implementation details worth mentioning:
>>>>>> - passing the type beyond gimplification is done by adding a NULL
>>>>>> pointer-
>>>>>> to-type to IFN_VA_ARG.
>>>>>> - there is special handling for IFN_VA_ARG that would be most suited
>>>>>> to be
>>>>>> placed in gimplify_va_arg_expr. However, that function lacks the
>>>>>> scope for
>>>>>> the special handling, so it's placed awkwardly in
>>>>>> gimplify_modify_expr.
>>>>>> - there's special handling in case the va_arg type is variable-sized.
>>>>>> gimplify_modify_expr adds a WITH_SIZE_EXPR to the CALL_EXPR
>>>>>> IFN_VA_ARG for
>>>>>> variable-sized types. However, this is gimplified into a
>>>>>> gimple_call which
>>>>>> does not have the possibility to wrap it's result in a
>>>>>> WITH_SIZE_EXPR. So
>>>>>> we're adding the size argument of the WITH_SIZE_EXPR as argument to
>>>>>> IFN_VA_ARG, and at expansion in pass_stdarg, wrap the result of the
>>>>>> gimplification of IFN_VA_ARG in a WITH_SIZE_EXPR, such that the
>>>>>> subsequent
>>>>>> gimplify_assign will generate a memcpy if necessary.
>>>>>> - when gimplifying the va_arg argument ap, it may not be addressable.
>>>>>> So
>>>>>> gimplification will generate a copy ap.1 = ap, and use &ap.1 as
>>>>>> argument.
>>>>>> This means that we have to copy back the ap.1 value to ap after
>>>>>> IFN_VA_ARG.
>>>>>> The copy is classified by the va_list_gpr/fpr_size optimization as
>>>>>> an
>>>>>> escape, so it inhibits optimization. The tree-ssa/stdarg-2.c f15
>>>>>> update is
>>>>>> because of that.
>>>>>>
>>>>>> OK for stage1?
>>>>> Looks mostly good, though it looks like with -O0 this doesn't delay
>>>>> lowering of va-arg and thus won't "fix" offloading. Can you instead
>>>>> introduce a PROP_gimple_lva, provide it by the stdarg pass and add
>>>>> a pass_lower_vaarg somewhere where pass_lower_complex_O0 is run
>>>>> that runs of !PROP_gimple_lva (and also provides it), and require
>>>>> PROP_gimple_lva by pass_expand? (just look for PROP_gimple_lcx for
>>>>> the complex stuff to get an idea what needs to be touched)
>>>>>
>>>> Updated according to comments.
>>>>
>>>> Furthermore (having updated the patch series to recent trunk), I'm
>>>> dropping the
>>>> ACCEL_COMPILER bit in pass_stdarg::gate. AFAIU the comment there relates
>>>> to this
>>>> patch.
>>>>
>>>> Retested as before.
>>>>
>>>> OK for stage1?
>>>>
>>> Ping.
>> Ping again.
>>
>> Patch originally posted at:
>> https://gcc.gnu.org/ml/gcc-patches/2015-02/msg01332.html .
>
> Ok.
>
> Thanks,
> Richard.
>
>> Thanks,
>> - Tom
>>
>>>> Btw, I'm wondering if as run-time optimization we can tentatively set
>>>> PROP_gimple_lva at the start of the gimple pass, and unset it in
>>>> gimplify_va_arg_expr. That way we would avoid the loop in
>>>> expand_ifn_va_arg_1
>>>> (over all bbs and gimples) in functions without va_arg.
>>>>
>>> Taken care of in follow-up patch 5b.
>
>
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PING^2] [PATCH][5a/5] Postpone expanding va_arg until pass_stdarg
2015-06-09 11:04 ` Alan Lawrence
@ 2015-06-09 11:07 ` Richard Biener
2015-06-09 13:03 ` Tom de Vries
0 siblings, 1 reply; 39+ messages in thread
From: Richard Biener @ 2015-06-09 11:07 UTC (permalink / raw)
To: Alan Lawrence; +Cc: Tom de Vries, Jakub Jelinek, GCC Patches, Michael Matz
On Tue, 9 Jun 2015, Alan Lawrence wrote:
> Hmmm. One side effect of this is that the line number information available in
> the target hook gimplify_va_arg_expr, is now just the name of the containing
> function, rather than the specific use of va_arg. Is there some way to get
> this more precise location (e.g. gimple_location(stmt) in expand_ifn_va_arg_1,
> the only caller of said hook)? I don't really want to have to add an extra
> parameter to the target hook...
The x86 variant doesn't use any locations but if then the caller of
the target hook (expand_ifn_va_arg_1) should assign the IFNs location
to all statements expanded from it (it could set input_location to
that during the target hook call...)
Richard.
> --Alan
>
> Richard Biener wrote:
> > On Thu, 16 Apr 2015, Tom de Vries wrote:
> >
> > > [stage1 ping^2]
> > > On 10-03-15 16:30, Tom de Vries wrote:
> > > > [stage1 ping]
> > > > On 22-02-15 14:13, Tom de Vries wrote:
> > > > > On 19-02-15 14:03, Richard Biener wrote:
> > > > > > On Thu, 19 Feb 2015, Tom de Vries wrote:
> > > > > >
> > > > > > > On 19-02-15 11:29, Tom de Vries wrote:
> > > > > > > > Hi,
> > > > > > > >
> > > > > > > > I'm posting this patch series for stage1:
> > > > > > > > - 0001-Disable-lang_hooks.gimplify_expr-in-free_lang_data.patch
> > > > > > > > - 0002-Add-gimple_find_sub_bbs.patch
> > > > > > > > -
> > > > > > > > 0003-Factor-optimize_va_list_gpr_fpr_size-out-of-pass_std.patch
> > > > > > > > - 0004-Handle-internal_fn-in-operand_equal_p.patch
> > > > > > > > - 0005-Postpone-expanding-va_arg-until-pass_stdarg.patch
> > > > > > > >
> > > > > > > > The patch series - based on Michael's initial patch - postpones
> > > > > > > > expanding
> > > > > > > > va_arg
> > > > > > > > until pass_stdarg, which makes pass_stdarg more robust.
> > > > > > > >
> > > > > > > > Bootstrapped and reg-tested on x86_64 using all languages, with
> > > > > > > > unix/ and
> > > > > > > > unix/-m32 testing.
> > > > > > > >
> > > > > > > > I'll post the patches in reply to this email.
> > > > > > > >
> > > > > > > This patch postpones expanding va_arg until pass_stdarg.
> > > > > > >
> > > > > > > We add a new internal function IFN_VA_ARG. During gimplification,
> > > > > > > we
> > > > > > > map
> > > > > > > VA_ARG_EXPR onto a CALL_EXPR with IFN_VA_ARG, which is then
> > > > > > > gimplified
> > > > > > > in to a
> > > > > > > gimple_call. At pass_stdarg, we expand the IFN_VA_ARG gimple_call
> > > > > > > into
> > > > > > > actual
> > > > > > > code.
> > > > > > >
> > > > > > > There are a few implementation details worth mentioning:
> > > > > > > - passing the type beyond gimplification is done by adding a NULL
> > > > > > > pointer-
> > > > > > > to-type to IFN_VA_ARG.
> > > > > > > - there is special handling for IFN_VA_ARG that would be most
> > > > > > > suited
> > > > > > > to be
> > > > > > > placed in gimplify_va_arg_expr. However, that function lacks
> > > > > > > the
> > > > > > > scope for
> > > > > > > the special handling, so it's placed awkwardly in
> > > > > > > gimplify_modify_expr.
> > > > > > > - there's special handling in case the va_arg type is
> > > > > > > variable-sized.
> > > > > > > gimplify_modify_expr adds a WITH_SIZE_EXPR to the CALL_EXPR
> > > > > > > IFN_VA_ARG for
> > > > > > > variable-sized types. However, this is gimplified into a
> > > > > > > gimple_call which
> > > > > > > does not have the possibility to wrap it's result in a
> > > > > > > WITH_SIZE_EXPR. So
> > > > > > > we're adding the size argument of the WITH_SIZE_EXPR as
> > > > > > > argument to
> > > > > > > IFN_VA_ARG, and at expansion in pass_stdarg, wrap the result of
> > > > > > > the
> > > > > > > gimplification of IFN_VA_ARG in a WITH_SIZE_EXPR, such that the
> > > > > > > subsequent
> > > > > > > gimplify_assign will generate a memcpy if necessary.
> > > > > > > - when gimplifying the va_arg argument ap, it may not be
> > > > > > > addressable.
> > > > > > > So
> > > > > > > gimplification will generate a copy ap.1 = ap, and use &ap.1 as
> > > > > > > argument.
> > > > > > > This means that we have to copy back the ap.1 value to ap after
> > > > > > > IFN_VA_ARG.
> > > > > > > The copy is classified by the va_list_gpr/fpr_size optimization
> > > > > > > as
> > > > > > > an
> > > > > > > escape, so it inhibits optimization. The tree-ssa/stdarg-2.c
> > > > > > > f15
> > > > > > > update is
> > > > > > > because of that.
> > > > > > >
> > > > > > > OK for stage1?
> > > > > > Looks mostly good, though it looks like with -O0 this doesn't delay
> > > > > > lowering of va-arg and thus won't "fix" offloading. Can you instead
> > > > > > introduce a PROP_gimple_lva, provide it by the stdarg pass and add
> > > > > > a pass_lower_vaarg somewhere where pass_lower_complex_O0 is run
> > > > > > that runs of !PROP_gimple_lva (and also provides it), and require
> > > > > > PROP_gimple_lva by pass_expand? (just look for PROP_gimple_lcx for
> > > > > > the complex stuff to get an idea what needs to be touched)
> > > > > >
> > > > > Updated according to comments.
> > > > >
> > > > > Furthermore (having updated the patch series to recent trunk), I'm
> > > > > dropping the
> > > > > ACCEL_COMPILER bit in pass_stdarg::gate. AFAIU the comment there
> > > > > relates
> > > > > to this
> > > > > patch.
> > > > >
> > > > > Retested as before.
> > > > >
> > > > > OK for stage1?
> > > > >
> > > > Ping.
> > > Ping again.
> > >
> > > Patch originally posted at:
> > > https://gcc.gnu.org/ml/gcc-patches/2015-02/msg01332.html .
> >
> > Ok.
> >
> > Thanks,
> > Richard.
> >
> > > Thanks,
> > > - Tom
> > >
> > > > > Btw, I'm wondering if as run-time optimization we can tentatively set
> > > > > PROP_gimple_lva at the start of the gimple pass, and unset it in
> > > > > gimplify_va_arg_expr. That way we would avoid the loop in
> > > > > expand_ifn_va_arg_1
> > > > > (over all bbs and gimples) in functions without va_arg.
> > > > >
> > > > Taken care of in follow-up patch 5b.
> >
> >
>
>
--
Richard Biener <rguenther@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Dilip Upmanyu, Graham Norton, HRB 21284 (AG Nuernberg)
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PING^2] [PATCH][5a/5] Postpone expanding va_arg until pass_stdarg
2015-06-09 11:07 ` Richard Biener
@ 2015-06-09 13:03 ` Tom de Vries
2015-06-09 13:07 ` Richard Biener
2015-06-09 13:30 ` Alan Lawrence
0 siblings, 2 replies; 39+ messages in thread
From: Tom de Vries @ 2015-06-09 13:03 UTC (permalink / raw)
To: Richard Biener, Alan Lawrence; +Cc: Jakub Jelinek, GCC Patches, Michael Matz
[-- Attachment #1: Type: text/plain, Size: 1044 bytes --]
On 09/06/15 13:03, Richard Biener wrote:
> On Tue, 9 Jun 2015, Alan Lawrence wrote:
>
>> Hmmm. One side effect of this is that the line number information available in
>> the target hook gimplify_va_arg_expr, is now just the name of the containing
>> function, rather than the specific use of va_arg. Is there some way to get
>> this more precise location (e.g. gimple_location(stmt) in expand_ifn_va_arg_1,
>> the only caller of said hook)? I don't really want to have to add an extra
>> parameter to the target hook...
>
> The x86 variant doesn't use any locations but if then the caller of
> the target hook (expand_ifn_va_arg_1) should assign the IFNs location
> to all statements expanded from it (it could set input_location to
> that during the target hook call...)
>
That seems to work.
The scan-assembler-not test in the testcase in attached patch:
- fails without the expand_ifn_va_arg_1 patch hunk, and
- passes with that hunk.
I'll put it through bootstrap and reg-test on x86_64.
OK for trunk if that goes well?
Thanks,
- Tom
[-- Attachment #2: 0001-Handle-location-in-expand_ifn_va_arg_1.patch --]
[-- Type: text/x-patch, Size: 2113 bytes --]
Handle location in expand_ifn_va_arg_1
2015-06-09 Tom de Vries <tom@codesourcery.com>
* tree-stdarg.c (expand_ifn_va_arg_1): Handle location.
* gcc.target/i386/vararg-loc.c: New test.
---
gcc/testsuite/gcc.target/i386/vararg-loc.c | 27 +++++++++++++++++++++++++++
gcc/tree-stdarg.c | 4 ++++
2 files changed, 31 insertions(+)
create mode 100644 gcc/testsuite/gcc.target/i386/vararg-loc.c
diff --git a/gcc/testsuite/gcc.target/i386/vararg-loc.c b/gcc/testsuite/gcc.target/i386/vararg-loc.c
new file mode 100644
index 0000000..f236fe3
--- /dev/null
+++ b/gcc/testsuite/gcc.target/i386/vararg-loc.c
@@ -0,0 +1,27 @@
+/* { dg-do compile } */
+/* { dg-options "-g -O0" } */
+
+#include <stdarg.h>
+
+int /* 6. */
+ /* 7. */
+f (int a, ...) /* 8. */
+ /* 9. */
+{
+
+ int sum = a;
+
+ va_list ap;
+
+ va_start (ap, a);
+
+ sum += va_arg (ap, int); /* 18. */
+
+ sum += va_arg (ap, int); /* 20. */
+
+ return sum;
+}
+
+/* { dg-final { scan-assembler-not "\\.loc 1 \[6789\] 0" } } */
+/* { dg-final { scan-assembler-times "\\.loc 1 18 0" 1 } } */
+/* { dg-final { scan-assembler-times "\\.loc 1 20 0" 1 } } */
diff --git a/gcc/tree-stdarg.c b/gcc/tree-stdarg.c
index 08d10b5..65fe9f9 100644
--- a/gcc/tree-stdarg.c
+++ b/gcc/tree-stdarg.c
@@ -1031,6 +1031,7 @@ expand_ifn_va_arg_1 (function *fun)
bool modified = false;
basic_block bb;
gimple_stmt_iterator i;
+ location_t saved_location;
FOR_EACH_BB_FN (bb, fun)
for (i = gsi_start_bb (bb); !gsi_end_p (i); gsi_next (&i))
@@ -1051,6 +1052,8 @@ expand_ifn_va_arg_1 (function *fun)
ap = build_fold_indirect_ref (ap);
push_gimplify_context (false);
+ saved_location = input_location;
+ input_location = gimple_location (stmt);
/* Make it easier for the backends by protecting the valist argument
from multiple evaluations. */
@@ -1081,6 +1084,7 @@ expand_ifn_va_arg_1 (function *fun)
else
gimplify_expr (&expr, &pre, &post, is_gimple_lvalue, fb_lvalue);
+ input_location = saved_location;
pop_gimplify_context (NULL);
gimple_seq_add_seq (&pre, post);
--
1.9.1
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PING^2] [PATCH][5a/5] Postpone expanding va_arg until pass_stdarg
2015-06-09 13:03 ` Tom de Vries
@ 2015-06-09 13:07 ` Richard Biener
2015-06-09 13:30 ` Alan Lawrence
1 sibling, 0 replies; 39+ messages in thread
From: Richard Biener @ 2015-06-09 13:07 UTC (permalink / raw)
To: Tom de Vries; +Cc: Alan Lawrence, Jakub Jelinek, GCC Patches, Michael Matz
On Tue, 9 Jun 2015, Tom de Vries wrote:
> On 09/06/15 13:03, Richard Biener wrote:
> > On Tue, 9 Jun 2015, Alan Lawrence wrote:
> >
> > > Hmmm. One side effect of this is that the line number information
> > > available in
> > > the target hook gimplify_va_arg_expr, is now just the name of the
> > > containing
> > > function, rather than the specific use of va_arg. Is there some way to get
> > > this more precise location (e.g. gimple_location(stmt) in
> > > expand_ifn_va_arg_1,
> > > the only caller of said hook)? I don't really want to have to add an extra
> > > parameter to the target hook...
> >
> > The x86 variant doesn't use any locations but if then the caller of
> > the target hook (expand_ifn_va_arg_1) should assign the IFNs location
> > to all statements expanded from it (it could set input_location to
> > that during the target hook call...)
> >
>
> That seems to work.
>
> The scan-assembler-not test in the testcase in attached patch:
> - fails without the expand_ifn_va_arg_1 patch hunk, and
> - passes with that hunk.
>
> I'll put it through bootstrap and reg-test on x86_64.
>
> OK for trunk if that goes well?
Ok.
Thanks,
Richard.
> Thanks,
> - Tom
>
--
Richard Biener <rguenther@suse.de>
SUSE LINUX GmbH, GF: Felix Imendoerffer, Jane Smithard, Dilip Upmanyu, Graham Norton, HRB 21284 (AG Nuernberg)
^ permalink raw reply [flat|nested] 39+ messages in thread
* Re: [PING^2] [PATCH][5a/5] Postpone expanding va_arg until pass_stdarg
2015-06-09 13:03 ` Tom de Vries
2015-06-09 13:07 ` Richard Biener
@ 2015-06-09 13:30 ` Alan Lawrence
1 sibling, 0 replies; 39+ messages in thread
From: Alan Lawrence @ 2015-06-09 13:30 UTC (permalink / raw)
To: Tom de Vries; +Cc: Richard Biener, Jakub Jelinek, GCC Patches, Michael Matz
Tom de Vries wrote:
> On 09/06/15 13:03, Richard Biener wrote:
>> On Tue, 9 Jun 2015, Alan Lawrence wrote:
>>
>>> Hmmm. One side effect of this is that the line number information available in
>>> the target hook gimplify_va_arg_expr, is now just the name of the containing
>>> function, rather than the specific use of va_arg. Is there some way to get
>>> this more precise location (e.g. gimple_location(stmt) in expand_ifn_va_arg_1,
>>> the only caller of said hook)? I don't really want to have to add an extra
>>> parameter to the target hook...
>> The x86 variant doesn't use any locations but if then the caller of
>> the target hook (expand_ifn_va_arg_1) should assign the IFNs location
>> to all statements expanded from it (it could set input_location to
>> that during the target hook call...)
>>
>
> That seems to work.
>
> The scan-assembler-not test in the testcase in attached patch:
> - fails without the expand_ifn_va_arg_1 patch hunk, and
> - passes with that hunk.
>
> I'll put it through bootstrap and reg-test on x86_64.
>
> OK for trunk if that goes well?
>
> Thanks,
> - Tom
That fixes the issue for me (I'm working with on a patch which uses the line
info in error output) - thank you very much!
Cheers, Alan
^ permalink raw reply [flat|nested] 39+ messages in thread
end of thread, other threads:[~2015-06-09 13:29 UTC | newest]
Thread overview: 39+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2015-02-19 10:48 [stage1] Postpone expanding va_arg until pass_stdarg Tom de Vries
2015-02-19 10:51 ` [PATCH][1/5] Disable lang_hooks.gimplify_expr in free_lang_data Tom de Vries
2015-02-19 12:41 ` Richard Biener
2015-02-19 10:59 ` [PATCH][2/5] Add gimple_find_sub_bbs Tom de Vries
2015-02-19 12:41 ` Richard Biener
2015-02-19 12:51 ` Jakub Jelinek
2015-02-19 13:03 ` Marek Polacek
2015-02-19 13:31 ` Richard Biener
2015-02-19 13:07 ` Richard Biener
2015-02-22 12:47 ` Tom de Vries
2015-02-19 11:06 ` [PATCH][3/5] Factor optimize_va_list_gpr_fpr_size out of pass_stdarg::execute Tom de Vries
2015-02-19 12:44 ` Richard Biener
2015-02-19 11:37 ` [PATCH][4/5] Handle internal_fn in operand_equal_p Tom de Vries
2015-02-19 12:49 ` Richard Biener
2015-02-19 12:59 ` Jakub Jelinek
2015-02-19 13:09 ` Richard Biener
2015-02-19 15:31 ` Tom de Vries
2015-02-20 11:59 ` Richard Biener
2015-02-22 13:13 ` Tom de Vries
2015-02-23 10:19 ` Richard Biener
2015-02-24 13:09 ` Fwd: " Tom de Vries
2015-02-19 12:08 ` [PATCH][5/5] Postpone expanding va_arg until pass_stdarg Tom de Vries
2015-02-19 13:05 ` Richard Biener
2015-02-22 13:24 ` Tom de Vries
2015-02-23 9:03 ` Michael Matz
2015-02-23 10:36 ` Tom de Vries
2015-02-24 8:26 ` Tom de Vries
2015-03-10 15:31 ` [PING] [PATCH][5b/5] Avoid running expand_ifn_va_arg_1 in functions without va_arg Tom de Vries
2015-04-16 8:50 ` [PING^2] " Tom de Vries
2015-04-16 8:56 ` Richard Biener
2015-03-10 15:31 ` [PING] [PATCH][5a/5] Postpone expanding va_arg until pass_stdarg Tom de Vries
2015-04-16 8:50 ` [PING^2] " Tom de Vries
2015-04-16 8:55 ` Richard Biener
2015-04-21 5:32 ` Bin.Cheng
2015-06-09 11:04 ` Alan Lawrence
2015-06-09 11:07 ` Richard Biener
2015-06-09 13:03 ` Tom de Vries
2015-06-09 13:07 ` Richard Biener
2015-06-09 13:30 ` Alan Lawrence
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).