[Bug target/66885] New: trunk/gcc/config/mcore/mcore.c:1656: poor code order ?

[Bug target/66885] New: trunk/gcc/config/mcore/mcore.c:1656: poor code order ?

[dcb314 at hotmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66885

            Bug ID: 66885
           Summary: trunk/gcc/config/mcore/mcore.c:1656: poor code order ?
           Product: gcc
           Version: 6.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: target
          Assignee: unassigned at gcc dot gnu.org
          Reporter: dcb314 at hotmail dot com
  Target Milestone: ---

[trunk/gcc/config/mcore/mcore.c:1656]: (style) Expression is always false
because 'else if' condition matches previous condition at line 1654.

Source code is

      if (bytes & 1)
    max = 4*4;
      else if (bytes & 3)
    max = 8*4;
      else
    max = 16*4;

Suggest check & 3 *before & 1.

[Bug target/66885] trunk/gcc/config/mcore/mcore.c:1656: poor code order ?

[schwab@linux-m68k.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66885

Andreas Schwab <schwab@linux-m68k.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |RESOLVED
         Resolution|---                         |INVALID

--- Comment #1 from Andreas Schwab <schwab@linux-m68k.org> ---
The second condition is *not* always false.

[Bug target/66885] trunk/gcc/config/mcore/mcore.c:1656: poor code order ?

[dcb314 at hotmail dot com]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66885

--- Comment #2 from David Binderman <dcb314 at hotmail dot com> ---
(In reply to Andreas Schwab from comment #1)
> The second condition is *not* always false.

After some further thought, agreed.

Would this be more clearly coded as

   switch (bytes % 4)
   {
   case 1:
   case 3:
       max = 16;
       break;
   case 2:
       max = 32;
       break;
   case 0:
       max = 64;
       break;
   }

or maybe even faster, some table lookup.

[Bug target/66885] trunk/gcc/config/mcore/mcore.c:1656: poor code order ?

[pinskia at gcc dot gnu.org]

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=66885

--- Comment #3 from Andrew Pinski <pinskia at gcc dot gnu.org> ---
(In reply to David Binderman from comment #2)
> (In reply to Andreas Schwab from comment #1)
> > The second condition is *not* always false.
> 
> After some further thought, agreed.
> 
> Would this be more clearly coded as
> 
>    switch (bytes % 4)
>    {
>    case 1:
>    case 3:
>        max = 16;
>        break;
>    case 2:
>        max = 32;
>        break;
>    case 0:
>        max = 64;
>        break;
>    }
> 
> or maybe even faster, some table lookup.

If we use a switch, GCC can convert it into a lookup table.  But it might be
faster with if anyways.