This is the mail archive of the
gcc-patches@gcc.gnu.org
mailing list for the GCC project.
RE: [PATCH 2/7]: Ping3: Merge from Stack Branch - collect alignment info
- From: "Ye, Joey" <joey dot ye at intel dot com>
- To: "Richard Guenther" <richard dot guenther at gmail dot com>, "Mark Mitchell" <mark at codesourcery dot com>
- Cc: <gcc-patches at gcc dot gnu dot org>, "Lu, Hongjiu" <hongjiu dot lu at intel dot com>, "Guo, Xuepeng" <xuepeng dot guo at intel dot com>, "Jan Hubicka" <hubicka at ucw dot cz>, <jason at redhat dot com>, "Ian Lance Taylor" <iant at google dot com>
- Date: Wed, 28 May 2008 17:28:40 +0800
- Subject: RE: [PATCH 2/7]: Ping3: Merge from Stack Branch - collect alignment info
- References: <BB577BF501703042AD7E08EADD8E514FF5DD82@pdsmsx415.ccr.corp.intel.com> <84fc9c000805280219n455d7953ld536ba020d7136c2@mail.gmail.com>
Rich, Thanks for quick response. All three comments are reasonable to
me. I'll post a new version soon.
Mark, The reload is only inserted with several assertions. Would do
please take a look at it?
- Joey
-----Original Message-----
From: Richard Guenther [mailto:richard.guenther@gmail.com]
Sent: Wednesday, May 28, 2008 5:19 PM
To: Ye, Joey
Cc: gcc-patches@gcc.gnu.org; Lu, Hongjiu; Guo, Xuepeng; Jan Hubicka;
jason@redhat.com; Ian Lance Taylor; Mark Mitchell
Subject: Re: [PATCH 2/7]: Ping3: Merge from Stack Branch - collect
alignment info
On Wed, May 28, 2008 at 11:00 AM, Ye, Joey <joey.ye@intel.com> wrote:
> Thanks for Richard's review:
> http://gcc.gnu.org/ml/gcc-patches/2008-05/msg01589.html
> http://gcc.gnu.org/ml/gcc-patches/2008-05/msg01654.html
>
> OK for mainline?
+ else
+ {
+ gcc_assert (!crtl->stack_realign_finalized
+ && crtl->stack_realign_needed);
+ }
no braces needed here.
+ /* Assertion to check internal_arg_pointer is set to the right rtx
+ here. */
+ gcc_assert (crtl->args.internal_arg_pointer ==
+ virtual_incoming_args_rtx);
the == goes on the next line.
Index: reload1.c
===================================================================
--- reload1.c (revision 136046)
+++ reload1.c (working copy)
...
- reg_eliminate[i].can_eliminate = 0;
+ {
+ /* Must not disable reg eliminate because stack
realignment
+ must eliminate frame pointer to stack pointer. */
+ gcc_assert (! MAX_STACK_ALIGNMENT
+ || ! stack_realign_fp);
+ reg_eliminate[i].can_eliminate = 0;
+ }
there is a lot of changes like this in reload1.c. Can you instead add
an inline function like
static inline
void set_not_eliminatable (struct elim_table *p)
{
/* Must not disable reg eliminate because stack realignment
must eliminate frame pointer to stack pointer. */
gcc_assert (! MAX_STACK_ALIGNMENT
|| ! stack_realign_fp);
p->can_eliminate = 0;
}
and use that? That clutters reload1.c less.
Otherwise the patch looks good, but as it touches reload and I'm not
sure
this qualifies as a non-algorithmic change I think I cannot approve it.
I have CC'ed Ian and Mark to eventually have a look.
Thanks,
Richard.
> Thanks - Joey
>
> 2008-05-28 Joey Ye <joey.ye@intel.com>
> H.J. Lu <hongjiu.lu@intel.com>
>
> * builtins.c (expand_builtin_setjmp_receiver): Replace
> virtual_incoming_args_rtx with
> crtl->args.internal_arg_pointer.
> (expand_builtin_apply_args_1): Likewise.
> (expand_builtin_longjmp): DRAP will be needed if some builtins
> are
> called.
> (expand_builtin_apply): Likewise.
>
> * calls.c (expand_call): Replace virtual_incoming_args_rtx with
> crtl->args.internal_arg_pointer.
> (emit_call_1): DRAP will be needed if return pops.
>
> * caller-save.c (setup_save_areas): Call assign_stack_local_1
> instead of assign_stack_local to allow alignment reduction.
>
> * rtl.h (assign_stack_local_1): Declare new funtion.
>
> * emit-rtl.c (gen_reg_rtx): Estimate stack alignment when
> generating
> virtual registers.
>
> * cfgexpand.c (get_decl_align_unit): Estimate stack variable
> alignment and store to stack_alignment_estimated and
> max_used_stack_slot_alignment.
> (expand_one_var): Likewise.
> (gate_handle_drap): Gate new pass pass_handle_drap.
> (handle_drap): New function.
> (tree_expand_cfg): Calls handle_drap at end. Initialize
rtl_data
> fields.
>
> * defaults.h (MAX_VECTORIZE_STACK_ALIGNMENT): New.
>
> * function.c (assign_stack_local_1): Estimate stack variable
> alignment and store to stack_alignment_estimated.
> (instantiate_new_reg): Instantiate virtual incoming args rtx to
> vDRAP if stack realignment and DRAP is needed.
> (assign_parms): Collect parameter/return type alignment and
> contribute to stack_alignment_estimated.
> (locate_and_pad_parm): Likewise.
> (allocate_struct_function): Init stack_alignment_estimated and
> max_used_stack_slot_alignment.
> (get_arg_pointer_save_area): Replace virtual_incoming_args_rtx
> with crtl->args.internal_arg_pointer.
> (LOCAL_ALIGNMENT): Removed.
> (get_stack_local_alignment): New.
> (assign_stack_local): Use it. Set alignment on stack slot.
> (assign_stack_temp_for_type): Use get_stack_local_alignment.
>
>
> * global.c (compute_regsets): Set need_fp cannot_elim
> wrt stack_realign_needed. Set frame_pointer_needed.
>
>
> * stmt.c (expand_nl_goto_receiver): Replace
> virtual_incoming_args_rtx with
> crtl->args.internal_arg_pointer.
>
> * tree-vectorizer.c (vect_can_force_dr_alignment_p): Return
> true if alignment of variable on stack is less than or
> equal to MAX_VECTORIZE_STACK_ALIGNMENT.
>
> * reload1.c (set_label_offsets): Assert that frame pointer must
> be
> elimiated to stack pointer in case stack realignment is
> estimated
> to happen without DRAP.
> (elimination_effects): Likewise.
> (eliminate_regs_in_insn): Likewise.
> (mark_not_eliminable): Likewise.
> (update_eliminables): Likewise.
> (init_elim_table): Don't set frame_pointer_needed here.
>