[PATCH v4] Fix alignment for local variable [PR90811]
Kito Cheng
kito.cheng@sifive.com
Wed May 20 07:20:32 GMT 2020
Hi Richard:
Tested and committed with fixes, thanks your review :)
On Mon, May 18, 2020 at 6:22 PM Richard Biener
<richard.guenther@gmail.com> wrote:
>
> On Mon, May 18, 2020 at 9:27 AM Kito Cheng <kito.cheng@sifive.com> wrote:
> >
> > ping
> >
> > On Tue, Apr 14, 2020 at 2:53 PM Kito Cheng <kito.cheng@sifive.com> wrote:
> >>
> >> - The alignment for local variable was adjust during estimate_stack_frame_size,
> >> however it seems wrong spot to adjust that, expand phase will adjust that
> >> but it little too late to some gimple optimization, which rely on certain
> >> target hooks need to check alignment, forwprop is an example for
> >> that, result of simplify_builtin_call rely on the alignment on some
> >> target like ARM or RISC-V.
> >>
> >> - Exclude static local var and hard register var in the process of
> >> alignment adjustment.
> >>
> >> - This patch fix gfortran.dg/pr45636.f90 for arm and riscv.
> >>
> >> - Regression test on riscv32/riscv64 and x86_64-linux-gnu, no new fail
> >> introduced.
> >>
> >> gcc/ChangeLog
> >>
> >> PR target/90811
> >> * Makefile.in (OBJS): Add tree-adjust-alignment.o.
> >> * tree-adjust-alignment.cc (pass_data_adjust_alignment): New.
> >> (pass_adjust_alignment): New.
> >> (-pass_adjust_alignment::execute): New.
> >> (make_pass_adjust_alignment): New.
> >> * tree-pass.h (make_pass_adjust_alignment): New.
> >> * passes.def: Add pass_adjust_alignment.
> >> ---
> >> gcc/Makefile.in | 1 +
> >> gcc/passes.def | 1 +
> >> gcc/tree-adjust-alignment.cc | 92 ++++++++++++++++++++++++++++++++++++
> >> gcc/tree-pass.h | 1 +
> >> 4 files changed, 95 insertions(+)
> >> create mode 100644 gcc/tree-adjust-alignment.cc
> >>
> >> diff --git a/gcc/Makefile.in b/gcc/Makefile.in
> >> index fa9923bb270..9b73288f776 100644
> >> --- a/gcc/Makefile.in
> >> +++ b/gcc/Makefile.in
> >> @@ -1545,6 +1545,7 @@ OBJS = \
> >> ubsan.o \
> >> sanopt.o \
> >> sancov.o \
> >> + tree-adjust-alignment.o \
>
> Please rename the file to just 'adjust-alignment.c'
>
> >> tree-call-cdce.o \
> >> tree-cfg.o \
> >> tree-cfgcleanup.o \
> >> diff --git a/gcc/passes.def b/gcc/passes.def
> >> index 2bf2cb78fc5..92cbe587a8a 100644
> >> --- a/gcc/passes.def
> >> +++ b/gcc/passes.def
> >> @@ -183,6 +183,7 @@ along with GCC; see the file COPYING3. If not see
> >> NEXT_PASS (pass_oacc_device_lower);
> >> NEXT_PASS (pass_omp_device_lower);
> >> NEXT_PASS (pass_omp_target_link);
> >> + NEXT_PASS (pass_adjust_alignment);
> >> NEXT_PASS (pass_all_optimizations);
> >> PUSH_INSERT_PASSES_WITHIN (pass_all_optimizations)
> >> NEXT_PASS (pass_remove_cgraph_callee_edges);
> >> diff --git a/gcc/tree-adjust-alignment.cc b/gcc/tree-adjust-alignment.cc
> >> new file mode 100644
> >> index 00000000000..77f38f6949b
> >> --- /dev/null
> >> +++ b/gcc/tree-adjust-alignment.cc
> >> @@ -0,0 +1,92 @@
> >> +/* Adjust alignment for local variable.
> >> + Copyright (C) 2003-2020 Free Software Foundation, Inc.
>
> Just 2020 please
>
> >> + Contributed by Dorit Naishlos <dorit@il.ibm.com>
>
> Eh? Please put in your name.
>
> >> +
> >> +This file is part of GCC.
> >> +
> >> +GCC is free software; you can redistribute it and/or modify it under
> >> +the terms of the GNU General Public License as published by the Free
> >> +Software Foundation; either version 3, or (at your option) any later
> >> +version.
> >> +
> >> +GCC is distributed in the hope that it will be useful, but WITHOUT ANY
> >> +WARRANTY; without even the implied warranty of MERCHANTABILITY or
> >> +FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License
> >> +for more details.
> >> +
> >> +You should have received a copy of the GNU General Public License
> >> +along with GCC; see the file COPYING3. If not see
> >> +<http://www.gnu.org/licenses/>. */
> >> +
> >> +#include "config.h"
> >> +#include "system.h"
> >> +#include "coretypes.h"
> >> +#include "backend.h"
> >> +#include "target.h"
> >> +#include "tree.h"
> >> +#include "gimple.h"
> >> +#include "tree-pass.h"
> >> +#include "cgraph.h"
> >> +#include "fold-const.h"
> >> +#include "gimple-iterator.h"
> >> +#include "tree-cfg.h"
> >> +#include "cfgloop.h"
> >> +#include "tree-vectorizer.h"
> >> +#include "tm_p.h"
>
> Did you try to narrow down the set of includes?
> Please do so, tree-vectorizer.h does not look needed.
>
> >> +namespace {
> >> +
> >> +const pass_data pass_data_adjust_alignment =
> >> +{
> >> + GIMPLE_PASS, /* type */
> >> + "adjust_alignment", /* name */
> >> + OPTGROUP_NONE, /* optinfo_flags */
> >> + TV_NONE, /* tv_id */
> >> + 0, /* properties_required */
> >> + 0, /* properties_provided */
> >> + 0, /* properties_destroyed */
> >> + 0, /* todo_flags_start */
> >> + 0, /* todo_flags_finish */
> >> +};
> >> +
> >> +class pass_adjust_alignment : public gimple_opt_pass
> >> +{
> >> +public:
> >> + pass_adjust_alignment (gcc::context *ctxt)
> >> + : gimple_opt_pass (pass_data_adjust_alignment, ctxt)
> >> + {}
> >> +
> >> + virtual unsigned int execute (function *);
> >> +
>
> excessive vertical space
>
> >> +}; // class pass_adjust_alignment
> >> +
> >> +} // anon namespace
> >> +
> >> +/* Entry point to adjust_alignment pass. */
> >> +unsigned int
> >> +pass_adjust_alignment::execute (function *fun) {
>
> The { goes to the next line and the following lines indents
> are off then.
>
> >> + size_t i;
> >> + tree var;
> >> + unsigned int align;
>
> declare variables where they are used, thus...
>
> >> +
> >> + FOR_EACH_LOCAL_DECL (fun, i, var)
> >> + {
> >> + /* Don't adjust aligment for static local var and hard register var. */
> >> + if (is_global_var (var) || DECL_HARD_REGISTER (var))
> >> + continue;
> >> +
> >> + align = LOCAL_DECL_ALIGNMENT (var);
>
> unsigned align = LOCAL...
>
> for example.
>
> OK with those changes.
>
> Thanks,
> Richard.
>
> >> +
> >> + /* Make sure alignment only increase. */
> >> + gcc_assert (align >= DECL_ALIGN (var));
> >> +
> >> + SET_DECL_ALIGN (var, align);
> >> + }
> >> + return 0;
> >> +}
> >> +
> >> +gimple_opt_pass *
> >> +make_pass_adjust_alignment (gcc::context *ctxt)
> >> +{
> >> + return new pass_adjust_alignment (ctxt);
> >> +}
> >> diff --git a/gcc/tree-pass.h b/gcc/tree-pass.h
> >> index a1207a20a3c..576b3f67434 100644
> >> --- a/gcc/tree-pass.h
> >> +++ b/gcc/tree-pass.h
> >> @@ -480,6 +480,7 @@ extern gimple_opt_pass *make_pass_sprintf_length (gcc::context *ctxt);
> >> extern gimple_opt_pass *make_pass_walloca (gcc::context *ctxt);
> >> extern gimple_opt_pass *make_pass_coroutine_lower_builtins (gcc::context *ctxt);
> >> extern gimple_opt_pass *make_pass_coroutine_early_expand_ifns (gcc::context *ctxt);
> >> +extern gimple_opt_pass *make_pass_adjust_alignment (gcc::context *ctxt);
> >>
> >> /* IPA Passes */
> >> extern simple_ipa_opt_pass *make_pass_ipa_lower_emutls (gcc::context *ctxt);
> >> --
> >> 2.26.0
> >>
More information about the Gcc-patches
mailing list