This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: Merge switch statements in tree-cfgcleanup


On Mon, Jul 18, 2016 at 6:07 PM, Bernd Schmidt <bschmidt@redhat.com> wrote:
> The motivating example for this patch was a change that was submitted for
> genattrtab last year, which would have made us generate
>
> switch (type = get_attr_type (insn))
>   {
>    ... some cases ...
>    default:
>      switch (type = get_attr_type (insn)))
>        {
>        ... some other cases ...
>        }
>   }
>
> The idea was to optimize this by merging the code into a single switch. My
> expectation was that this was most likely to occur in machine-generated
> code, but there are a few instances of this pattern in the gcc sources
> themselves. One case is
>
>    code = gimple_code (stmt);
>    switch (code)
>      {
>      ....
>      default:
>        if (is_gimple_omp (code))
>          {
>          }
>      }
>
> where is_gimple_omp expands into another switch. More cases exist in the
> compiler as shown by various bootstrap failures along the way; sometimes
> these are exposed after other optimizations. One is in the Ada runtime
> library somewhere, and another (which currently cannot be transformed by the
> patch) is in the Fortran frontend.
>
> In the future we could also look for if statements making another comparison
> of the variable in the default branch, that would be a minor extension.
>
> The motivating example currently can't be transformed because get_attr_type
> calls are in the way.
>
> Bootstrapped and tested on x86_64-linux. Ok?

This is not appropriate for CFG cleanup due to its complexity not
being O(# bbs + # edges).
I tried hard in the past to make it so (at least when no transform is done).

Please move this transform elsewhere.  I suggest the switch-conversion
pass or if that
is not a good fit then maybe if-combine (whose transforms are remotely related).

Not looking closer at the patch but missing some comments on how it deals with
common cases (you see to handle fallthrus to the default label by
ignoring them?)

Thanks,
Richard.


>
> Bernd


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]