[Bug tree-optimization/81396] [7/8 Regression] Optimization of reading Little-Endian 64-bit number with portable code has a regression

[pinskia at gcc dot gnu.org]

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

--- Comment #10 from Andrew Pinski  ---
(In reply to Jakub Jelinek from comment #4)
> Either we can do something in the bswap pass with it as done in this
> untested patch, or we could consider match.pd optimization for:
>   _3 = BIT_FIELD_REF <_7, 8, 16>;
>   _32 = (typeof(_7)) _3;
>   _4 = _32 << 16;
> into:
>   _4 = _7 & (((1ULL << 8) - 1) << 16);

I am implementing the above because when I was adding detecting bit_insert
more, bswap goes not understand bit_insert, plus we can optimize this earlier.

[Bug tree-optimization/81396] [7/8 Regression] Optimization of reading Little-Endian 64-bit number with portable code has a regression

[glisse at gcc dot gnu.org]

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

--- Comment #9 from Marc Glisse  ---
Should we open a separate PR for the transformation you suggested in comment 4,
or does that seem not useful enough now, or will be part of bitfield gimple
lowering when that lands?

[Bug tree-optimization/81396] [7/8 Regression] Optimization of reading Little-Endian 64-bit number with portable code has a regression

[jakub at gcc dot gnu.org]

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

Jakub Jelinek  changed:

   What|Removed |Added

 Status|ASSIGNED|RESOLVED
 Resolution|--- |FIXED

--- Comment #8 from Jakub Jelinek  ---
Fixed for 8.1+, it is a new optimization, so I think it shouldn't be backported
to 7.x.

[Bug tree-optimization/81396] [7/8 Regression] Optimization of reading Little-Endian 64-bit number with portable code has a regression

[jakub at gcc dot gnu.org]

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

--- Comment #7 from Jakub Jelinek  ---
Author: jakub
Date: Mon Jul 17 08:14:16 2017
New Revision: 250257

URL: https://gcc.gnu.org/viewcvs?rev=250257=gcc=rev
Log:
PR tree-optimization/81396
* tree-ssa-math-opts.c (struct symbolic_number): Add n_ops field.
(init_symbolic_number): Initialize it to 1.
(perform_symbolic_merge): Add n_ops from both operands into the new
n_ops.
(find_bswap_or_nop): Don't consider n->n == cmpnop computations
without base_addr as useless if they need more than one operation.
(bswap_replace): Handle !bswap case for NULL base_addr.

* gcc.dg/tree-ssa/pr81396.c: New test.

Added:
trunk/gcc/testsuite/gcc.dg/tree-ssa/pr81396.c
Modified:
trunk/gcc/ChangeLog
trunk/gcc/testsuite/ChangeLog
trunk/gcc/tree-ssa-math-opts.c

[Bug tree-optimization/81396] [7/8 Regression] Optimization of reading Little-Endian 64-bit number with portable code has a regression

[glisse at gcc dot gnu.org]

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

--- Comment #6 from Marc Glisse  ---
(In reply to Jakub Jelinek from comment #5)
> Or both this bswap change and the match.pd addition.

Doing both sounds good to me :-)

[Bug tree-optimization/81396] [7/8 Regression] Optimization of reading Little-Endian 64-bit number with portable code has a regression

[jakub at gcc dot gnu.org]

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

--- Comment #5 from Jakub Jelinek  ---
Or both this bswap change and the match.pd addition.

[Bug tree-optimization/81396] [7/8 Regression] Optimization of reading Little-Endian 64-bit number with portable code has a regression

[jakub at gcc dot gnu.org]

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

Jakub Jelinek  changed:

   What|Removed |Added

 Status|NEW |ASSIGNED
   Assignee|unassigned at gcc dot gnu.org  |jakub at gcc dot gnu.org

--- Comment #4 from Jakub Jelinek  ---
Created attachment 41749
  --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=41749=edit
gcc8-pr81396.patch

While the bswap pass finds out this is a nop reshuffling, it hits:
  /* Useless bit manipulation performed by code.  */
  if (!n->base_addr && n->n == cmpnop)
return NULL;
and doesn't do anything (while in GCC6 it was less optimized and thus
n->base_addr was non-NULL and the bswap pass did something with it).
Either we can do something in the bswap pass with it as done in this untested
patch, or we could consider match.pd optimization for:
  _3 = BIT_FIELD_REF <_7, 8, 16>;
  _32 = (typeof(_7)) _3;
  _4 = _32 << 16;
into:
  _4 = _7 & (((1ULL << 8) - 1) << 16);
if the shift matches the bitpos and then rest of match.pd would be able to
figure it out, like it optimizes now at forwprop1 time e.g.:
typedef unsigned long long uint64_t;
uint64_t ReadLittleEndian64(uint64_t word) {
  uint64_t ret = 0;
  ret |= (word & 0xff);
  ret |= (((word & 0xff00) >> 8) << 8);
  ret |= (((word & 0xff) >> 16) << 16);
  ret |= (((word & 0xff00U) >> 24) << 24);
  ret |= (((word & 0xffULL) >> 32) << 32);
  ret |= (((word & 0xff00ULL) >> 40) << 40);
  ret |= (((word & 0xffULL) >> 48) << 48);
  ret |= (((word & 0xff00ULL) >> 56) << 56);
  return ret;
}

[Bug tree-optimization/81396] [7/8 Regression] Optimization of reading Little-Endian 64-bit number with portable code has a regression

[jakub at gcc dot gnu.org]

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

--- Comment #3 from Jakub Jelinek  ---
I'll have a look at that tomorrow.

[Bug tree-optimization/81396] [7/8 Regression] Optimization of reading Little-Endian 64-bit number with portable code has a regression

[glisse at gcc dot gnu.org]

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

--- Comment #2 from Marc Glisse  ---
bswap was happy dealing with

  _2 = MEM[(const unsigned char *)];
  _3 = (uint64_t) _2;
  _4 = MEM[(const unsigned char *) + 1B];
  _5 = (uint64_t) _4;
  _6 = _5 << 8;
  _8 = MEM[(const unsigned char *) + 2B];
  _9 = (uint64_t) _8;
  _10 = _9 << 16;
  _32 = _6 | _10;
  _11 = _3 | _32;

etc, but has trouble with

  _21 = word_31(D) & 255;
  _1 = BIT_FIELD_REF ;
  _23 = (uint64_t) _1;
  _2 = _23 << 8;
  _4 = BIT_FIELD_REF ;
  _24 = (uint64_t) _4;
  _5 = _24 << 16;
  _32 = _2 | _5;
  _6 = _21 | _32;

[Bug tree-optimization/81396] [7/8 Regression] Optimization of reading Little-Endian 64-bit number with portable code has a regression

[jakub at gcc dot gnu.org]

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

Jakub Jelinek  changed:

   What|Removed |Added

 Status|UNCONFIRMED |NEW
   Last reconfirmed||2017-07-11
 CC||jakub at gcc dot gnu.org
   Target Milestone|--- |7.2
Summary|Optimization of reading |[7/8 Regression]
   |Little-Endian 64-bit number |Optimization of reading
   |with portable code has a|Little-Endian 64-bit number
   |regression  |with portable code has a
   ||regression
 Ever confirmed|0   |1

--- Comment #1 from Jakub Jelinek  ---
Started with r239778.