[Bug tree-optimization/101139] Unable to remove double byteswap in fast path

[pinskia at gcc dot gnu.org via Gcc-bugs]

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

--- Comment #5 from Andrew Pinski  ---
Created attachment 57993
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=57993=edit
Patch but it does not work for the code in this testcase

I have to look into why it is not working for the testcase in comment #0
(factor_out_conditional_operation is not even called) but it does work for:
```
short f(int a, short b, short c)
{
  if (a)
return __builtin_bswap16(b);
  return __builtin_bswap16(c);
}
```

[Bug tree-optimization/101139] Unable to remove double byteswap in fast path

[steinar+gcc at gunderson dot no via Gcc-bugs]

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

--- Comment #4 from Steinar H. Gunderson  ---
Yes, the integer promotion actually costs some performance. It happens on both
x86 and Arm. Should I file that as a separate bug?

[Bug tree-optimization/101139] Unable to remove double byteswap in fast path

[rguenth at gcc dot gnu.org via Gcc-bugs]

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

--- Comment #3 from Richard Biener  ---
One odd thing is that while __builtin_bswap16 is declared as taking an uint16
argument the frontend promotes it to 'int' and that stays that way in GIMPLE:

  _16 = (short unsigned int) f$ab_14;
  _17 = (int) _16;
  _18 = __builtin_bswap16 (_17);

[Bug tree-optimization/101139] Unable to remove double byteswap in fast path

[pinskia at gcc dot gnu.org via Gcc-bugs]

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

--- Comment #2 from Andrew Pinski  ---
(In reply to Andrew Pinski from comment #1)
>   if (b_13 < h.0_15)
> goto ; [51.12%]
>   else
> goto ; [48.88%]
> 
>[local count: 548896825]:
>   _16 = (short unsigned int) f$ab_14;
>   _17 = (int) _16;
>   _18 = __builtin_bswap16 (_17);
>   goto ; [100.00%]
> 
>[local count: 524845000]:
>   k_22 = i ();
>   c.1_23 = (short unsigned int) k_22;
>   _24 = (int) c.1_23;
>   _25 = __builtin_bswap16 (_24);
> 
>[local count: 1073741824]:
>   # prephitmp_32 = PHI <_18(4), _25(5)>
> 
> Basically the same issue as PR 13563.

Once that issue is fixed we should get:
  if (b_13 < h.0_15)
goto ; [51.12%]
  else
goto ; [48.88%]

   [local count: 548896825]:
  _16 = (short unsigned int) f$ab_14;
  goto ; [100.00%]

   [local count: 524845000]:
  k_22 = i ();
  c.1_23 = (short unsigned int) k_22;

   [local count: 1073741824]:
  # c.1_23 = PHI <_16(4), c.1_23(5)>

  _24 = (int) c.1_23;
  prephitmp_32 = __builtin_bswap16 (_32);
  _2 = (int) prephitmp_32;
  _3 = __builtin_bswap16 (_2);
  _4 = (int) _3;

Which then should just optimize later on to:  if (b_13 < h.0_15)
goto ; [51.12%]
  else
goto ; [48.88%]

   [local count: 548896825]:
  _16 = (short unsigned int) f$ab_14;
  goto ; [100.00%]

   [local count: 524845000]:
  k_22 = i ();
  c.1_23 = (short unsigned int) k_22;

   [local count: 1073741824]:
  # c.1_23 = PHI <_16(4), c.1_23(5)>

  _24 = (int) c.1_23;
  _4 = _24;
(If I did this conversion right)

[Bug tree-optimization/101139] Unable to remove double byteswap in fast path

[pinskia at gcc dot gnu.org via Gcc-bugs]

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

Andrew Pinski  changed:

   What|Removed |Added

   Last reconfirmed||2021-06-20
   Assignee|unassigned at gcc dot gnu.org  |pinskia at gcc dot 
gnu.org
 Depends on||13563
 Status|UNCONFIRMED |ASSIGNED
   Keywords||missed-optimization
 Ever confirmed|0   |1

--- Comment #1 from Andrew Pinski  ---
  if (b_13 < h.0_15)
goto ; [51.12%]
  else
goto ; [48.88%]

   [local count: 548896825]:
  _16 = (short unsigned int) f$ab_14;
  _17 = (int) _16;
  _18 = __builtin_bswap16 (_17);
  goto ; [100.00%]

   [local count: 524845000]:
  k_22 = i ();
  c.1_23 = (short unsigned int) k_22;
  _24 = (int) c.1_23;
  _25 = __builtin_bswap16 (_24);

   [local count: 1073741824]:
  # prephitmp_32 = PHI <_18(4), _25(5)>

Basically the same issue as PR 13563.


Referenced Bugs:

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=13563
[Bug 13563] if-conversion not agressive enough