Bug 64950 - postpone expanding va_arg till pass_stdarg
Summary: postpone expanding va_arg till pass_stdarg
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: tree-optimization (show other bugs)
Version: 5.0
: P3 enhancement
Target Milestone: ---
Assignee: Tom de Vries
URL:
Keywords: patch
Depends on:
Blocks: 41089 51153
  Show dependency treegraph
 
Reported: 2015-02-05 17:05 UTC by Tom de Vries
Modified: 2015-05-05 10:40 UTC (History)
2 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2015-02-24 00:00:00


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Tom de Vries 2015-02-05 17:05:41 UTC
pass_stdarg optimizes cfun->va_list_gpr/fpr_size by detecting va_args.

However, va_args are expanded at gimple level, so pass_stdarg detects them by matching what the expansion of va_arg looks like after running through the passes between gimplification and pass_stdarg.

This is known to be fragile (see f.i. PR 41089). It makes more sense to do the pass_stdarg optimization while va_arg is still intact. And we want to run optimization passes before pass_stdarg to be able to eliminate some unwanted va_args. So, it makes sense to postpone expansion of va_arg untill pass_stdarg.

Michael Matz posted a prototype patch ( https://gcc.gnu.org/ml/gcc/2015-01/msg00304.html ). Latest version availabe at vries/expand-va-arg-at-pass-stdarg. Testing status of latest version available at https://gcc.gnu.org/ml/gcc/2015-02/msg00020.html .
Comment 1 Tom de Vries 2015-02-22 14:48:54 UTC
stage 1 patch submitted: https://gcc.gnu.org/ml/gcc-patches/2015-02/msg01166.html
Comment 2 Tom de Vries 2015-02-24 12:26:17 UTC
STATUS:

0001-Disable-lang_hooks.gimplify_expr-in-free_lang_data.patch
OK for stage1: https://gcc.gnu.org/ml/gcc-patches/2015-02/msg01182.html

0002-Add-gimple_find_sub_bbs.patch
OK for stage1: https://gcc.gnu.org/ml/gcc-patches/2015-02/msg01183.html
Updated with comment: https://gcc.gnu.org/ml/gcc-patches/2015-02/msg01330.html

0003-Factor-optimize_va_list_gpr_fpr_size-out-of-pass_std.patch
OK for stage1: https://gcc.gnu.org/ml/gcc-patches/2015-02/msg01184.html

0004-Handle-internal_fn-in-operand_equal_p.patch
OK for stage1: https://gcc.gnu.org/ml/gcc-patches/2015-02/msg01408.html

0005-Postpone-expanding-va_arg-until-pass_stdarg.patch
Submitted for stage1: https://gcc.gnu.org/ml/gcc-patches/2015-02/msg01332.html

0006-Set-PROP_gimple_lva-for-functions-without-IFN_VA_ARG.patch
Submitted for stage1: https://gcc.gnu.org/ml/gcc-patches/2015-02/msg01402.html
Comment 3 Tom de Vries 2015-03-10 19:10:49 UTC
> 0005-Postpone-expanding-va_arg-until-pass_stdarg.patch
> Submitted for stage1:
> https://gcc.gnu.org/ml/gcc-patches/2015-02/msg01332.html

Pinged: https://gcc.gnu.org/ml/gcc-patches/2015-03/msg00570.html

> 0006-Set-PROP_gimple_lva-for-functions-without-IFN_VA_ARG.patch
> Submitted for stage1:
> https://gcc.gnu.org/ml/gcc-patches/2015-02/msg01402.html

Pinged: https://gcc.gnu.org/ml/gcc-patches/2015-03/msg00571.html
Comment 5 Tom de Vries 2015-04-17 09:50:28 UTC
https://gcc.gnu.org/ml/gcc-cvs/2015-04/msg00371.html :

Author: vries
Date: Fri Apr 17 09:26:59 2015
New Revision: 222173

URL: https://gcc.gnu.org/viewcvs?rev=222173&root=gcc&view=rev
Log:
Postpone expanding va_arg until pass_stdarg

2015-04-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-*-*.

Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/cfgexpand.c
    trunk/gcc/gimple-iterator.c
    trunk/gcc/gimple-iterator.h
    trunk/gcc/gimplify.c
    trunk/gcc/gimplify.h
    trunk/gcc/internal-fn.c
    trunk/gcc/internal-fn.def
    trunk/gcc/passes.def
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/testsuite/gcc.dg/tree-ssa/stdarg-2.c
    trunk/gcc/tree-pass.h
    trunk/gcc/tree-stdarg.c
Comment 6 Tom de Vries 2015-04-17 09:50:59 UTC
https://gcc.gnu.org/ml/gcc-cvs/2015-04/msg00372.html :

Author: vries
Date: Fri Apr 17 09:27:08 2015
New Revision: 222174

URL: https://gcc.gnu.org/viewcvs?rev=222174&root=gcc&view=rev
Log:
Set PROP_gimple_lva for functions without IFN_VA_ARG

2015-04-17  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.

Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/gimplify.c
    trunk/gcc/tree-inline.c
Comment 7 Tom de Vries 2015-04-17 09:59:07 UTC
Patch series retested and committed.

Filed spinoff PR 65791 - Postpone expand_ifn_va_arg till after optimize_va_list_gpr_fpr_size, for the TODO to rewrite optimize_va_list_gpr_fpr_size to work on ifn_va_arg.

Marking resolved, fixed.
Comment 8 Uroš Bizjak 2015-04-17 12:53:40 UTC
(In reply to vries from comment #7)

> Marking resolved, fixed.

So, can PR41089 hack [1] finally be reverted?

[1] https://gcc.gnu.org/ml/gcc-patches/2010-08/msg00072.html
Comment 9 Tom de Vries 2015-04-17 14:09:38 UTC
(In reply to Uroš Bizjak from comment #8)
> (In reply to vries from comment #7)
> 
> > Marking resolved, fixed.
> 
> So, can PR41089 hack [1] finally be reverted?
> 
> [1] https://gcc.gnu.org/ml/gcc-patches/2010-08/msg00072.html

[ Adding alpha maintainer to cc. ]

AFAIU, yes.

Thanks,
- Tom
Comment 10 uros 2015-04-21 06:30:08 UTC
Author: uros
Date: Tue Apr 21 06:29:37 2015
New Revision: 222257

URL: https://gcc.gnu.org/viewcvs?rev=222257&root=gcc&view=rev
Log:
	PR tree-optimization/64950
	Revert:
	2010-08-02  Uros Bizjak  <ubizjak@gmail.com>

	PR target/41089
	* config/alpha/alpha.c (alpha_build_builtin_va_list): Mark __offset
	as volatile.


Modified:
    trunk/gcc/ChangeLog
    trunk/gcc/config/alpha/alpha.c
Comment 11 Rainer Orth 2015-05-04 13:10:04 UTC
Unfortunately, the gcc.dg/tree-ssa/stdarg-2.c part of the patch is wrong: the test
now FAILs on i686-unknown-linux-gnu, i686-apple-darwin, and i386-pc-solaris with
-m64: both dumps (i.e. -m32 and -m64) contain

m32/stdarg-2.c.084t.stdarg:f15: va_list escapes 1, needs to save all GPR units and all FPR units.
m64/stdarg-2.c.084t.stdarg:f15: va_list escapes 1, needs to save all GPR units and all FPR units.

  Rainer
Comment 12 Tom de Vries 2015-05-05 09:17:38 UTC
(In reply to Rainer Orth from comment #11)
> Unfortunately, the gcc.dg/tree-ssa/stdarg-2.c part of the patch is wrong:
> the test
> now FAILs on i686-unknown-linux-gnu, i686-apple-darwin, and i386-pc-solaris
> with
> -m64: both dumps (i.e. -m32 and -m64) contain
> 
> m32/stdarg-2.c.084t.stdarg:f15: va_list escapes 1, needs to save all GPR
> units and all FPR units.
> m64/stdarg-2.c.084t.stdarg:f15: va_list escapes 1, needs to save all GPR
> units and all FPR units.
> 
>   Rainer

Proposed patch: https://gcc.gnu.org/ml/gcc-patches/2015-05/msg00294.html
Comment 13 Tom de Vries 2015-05-05 10:38:25 UTC
Author: vries
Date: Tue May  5 10:32:28 2015
New Revision: 222802

URL: https://gcc.gnu.org/viewcvs?rev=222802&root=gcc&view=rev
Log:
Xfail gcc.dg/tree-ssa/stdarg-2.c f15 scans

2015-05-05  Tom de Vries  <tom@codesourcery.com>

	* gcc.dg/tree-ssa/stdarg-2.c: Xfail f15 scans which test for presence of
	'va_list escapes 0'.

Modified:
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/testsuite/gcc.dg/tree-ssa/stdarg-2.c
Comment 14 Tom de Vries 2015-05-05 10:40:10 UTC
patch committed adding xfails in stdarg-2.c.

xfails tracked in spin-off PRs:
- PR66010 '[6 Regression] Missed optimization after inlining va_list parameter'
- PR66013 'Missed optimization after inlining va_list parameter, -m32 case'

Re-marking resolved, fixed.