[Bug rtl-optimization/50088] New: movzbl is generated instead of movl

[Bug rtl-optimization/50088] New: movzbl is generated instead of movl

[enkovich.gnu at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50088

             Bug #: 50088
           Summary: movzbl is generated instead of movl
    Classification: Unclassified
           Product: gcc
           Version: 4.7.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: rtl-optimization
        AssignedTo: unassigned@gcc.gnu.org
        ReportedBy: enkovich.gnu@gmail.com


Created attachment 25016
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=25016
Reproducer

When spilled register is going to be used in subreg expression then short load
is generated to fill register.

Example:
    movl  %edx, 0x34(%esp)
    jz 0x1498 <Block 54>
  Block 34:
    movzxb  0x34(%esp), %ecx
    shl %cl, %eax

It is correct but may cause performance problems. I doubt there are situations
when zero extended load is better than natural one.

On Atom processors (and probably some others) such situations cause stalls
because store forwarding does not work for store/load pair using different
access sizes.

For example EEMBC 2.0/huffde has ~6% performance improvement on Atom if we
replace such movzbl with movl.

Attached reproducer demonstrates fills performed via movzbl.
Used compiler and options:

Target: x86_64-unknown-linux-gnu
Configured with: ../gcc1/configure --prefix=/export/users/gcc-perf/install
--enable-languages=c,c++,fortran
Thread model: posix
gcc version 4.7.0 20110615 (experimental) (GCC)
COLLECT_GCC_OPTIONS='-O2' '-m32' '-S' '-v' '-mtune=generic' '-march=x86-64'
 /export/users/gcc-perf/install/libexec/gcc/x86_64-unknown-linux-gnu/4.7.0/cc1
-quiet -v -imultilib 32 test_movzbl.c -quiet -dumpbase test_movzbl.c -m32
-mtune=generic -march=x86-64 -auxbase test_movzbl -O2 -version -o test_movzbl.s
GNU C (GCC) version 4.7.0 20110615 (experimental) (x86_64-unknown-linux-gnu)
        compiled by GNU C version 4.4.3, GMP version 4.3.1, MPFR version 2.4.2,
MPC version 0.8.1
GGC heuristics: --param ggc-min-expand=30 --param ggc-min-heapsize=4096

[Bug rtl-optimization/50088] movzbl is generated instead of movl

[rguenth at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50088

--- Comment #1 from Richard Guenther <rguenth at gcc dot gnu.org> 2011-08-15 13:07:50 UTC ---
I don't think we know at this point that the data is already properly
zero-extended.

[Bug rtl-optimization/50088] movzbl is generated instead of movl

[enkovich.gnu at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50088

--- Comment #2 from Ilya Enkovich <enkovich.gnu at gmail dot com> 2011-08-15 13:24:05 UTC ---
Actually we do not need any zero extensions here. Zero extended load appears
only after IRA if we have to spill/fill register.

Here is c code from reproducer:

      n1 = (n1 + 1) & 15;
      s += arr[i] << n1;

RTL before IRA:

(insn 67 66 68 4 (parallel [
            (set (reg/v:SI 97 [ n1 ])
                (plus:SI (reg/v:SI 97 [ n1 ])
                    (const_int 1 [0x1])))
            (clobber (reg:CC 17 flags))
        ]) test_movzbl.c:18 249 {*addsi_1}
     (expr_list:REG_UNUSED (reg:CC 17 flags)
        (nil)))

(insn 68 67 70 4 (parallel [
            (set (reg/v:SI 97 [ n1 ])
                (and:SI (reg/v:SI 97 [ n1 ])
                    (const_int 15 [0xf])))
            (clobber (reg:CC 17 flags))
        ]) test_movzbl.c:18 385 {*andsi_1}
     (expr_list:REG_UNUSED (reg:CC 17 flags)
        (nil)))

(insn 70 68 71 4 (set (reg:SI 262)
        (mem:SI (reg:SI 224 [ ivtmp.52 ]) [2 MEM[base: D.2889_232, offset:
0B]+0 S4 A32])) test_movzbl.c:20 64 {*movsi_internal}
     (nil))

(insn 71 70 72 4 (parallel [
            (set (reg:SI 262)
                (ashift:SI (reg:SI 262)
                    (subreg:QI (reg/v:SI 97 [ n1 ]) 0)))
            (clobber (reg:CC 17 flags))
        ]) test_movzbl.c:20 502 {*ashlsi3_1}
     (expr_list:REG_UNUSED (reg:CC 17 flags)
        (expr_list:REG_EQUAL (ashift:SI (mem:SI (reg:SI 224 [ ivtmp.52 ]) [2
MEM[base: D.2889_232, offset: 0B]+0 S4 A32])
                (subreg:QI (reg/v:SI 97 [ n1 ]) 0))
            (nil))))

IRA then introduces fill for shift instruction and use byte load for it:

(insn 155 70 71 4 (set (reg:QI 2 cx)
        (mem/c:QI (reg/f:SI 7 sp) [4 %sfp+-28 S1 A32])) test_movzbl.c:20 66
{*movqi_internal}
     (nil))

(insn 71 155 72 4 (parallel [
            (set (reg:SI 5 di [262])
                (ashift:SI (reg:SI 5 di [262])
                    (reg:QI 2 cx)))
            (clobber (reg:CC 17 flags))
        ]) test_movzbl.c:20 502 {*ashlsi3_1}
     (expr_list:REG_EQUAL (ashift:SI (mem:SI (reg:SI 0 ax [orig:224 ivtmp.52 ]
[224]) [2 MEM[base: D.2889_232, offset: 0B]+0 S4 A32])
            (subreg:QI (mem/c:SI (reg/f:SI 7 sp) [4 %sfp+-28 S4 A32]) 0))
        (nil)))

Load for shift then is emitted as movzbl.

[Bug rtl-optimization/50088] movzbl is generated instead of movl

[hjl.tools at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50088

H.J. Lu <hjl.tools at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |hjl.tools at gmail dot com

--- Comment #3 from H.J. Lu <hjl.tools at gmail dot com> 2011-08-15 14:47:51 UTC ---
It is done on purpose:

  /* X86_TUNE_MOVX: Enable to zero extend integer registers to avoid
     partial dependencies.  */
  m_PPRO | m_P4_NOCONA | m_CORE2I7 | m_ATOM | m_GEODE | m_AMD_MULTIPLE  |
m_GENERIC,

You can remove m_ATOM to see its performance impact with -mtune=atom
on Atom.

[Bug rtl-optimization/50088] movzbl is generated instead of movl

[rguenth at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50088

--- Comment #4 from Richard Guenther <rguenth at gcc dot gnu.org> 2011-08-15 15:26:19 UTC ---
(In reply to comment #3)
> It is done on purpose:
> 
>   /* X86_TUNE_MOVX: Enable to zero extend integer registers to avoid
>      partial dependencies.  */
>   m_PPRO | m_P4_NOCONA | m_CORE2I7 | m_ATOM | m_GEODE | m_AMD_MULTIPLE  |
> m_GENERIC,
> 
> You can remove m_ATOM to see its performance impact with -mtune=atom
> on Atom.

Well, yes, I think the proposal was to spill/load the full SImode instead
which would avoid both the partial dependency and the mismatched load/store
size.  No?

[Bug rtl-optimization/50088] movzbl is generated instead of movl

[hjl.tools at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50088

--- Comment #5 from H.J. Lu <hjl.tools at gmail dot com> 2011-08-15 15:47:39 UTC ---
(In reply to comment #4)
> (In reply to comment #3)
> > It is done on purpose:
> > 
> >   /* X86_TUNE_MOVX: Enable to zero extend integer registers to avoid
> >      partial dependencies.  */
> >   m_PPRO | m_P4_NOCONA | m_CORE2I7 | m_ATOM | m_GEODE | m_AMD_MULTIPLE  |
> > m_GENERIC,
> > 
> > You can remove m_ATOM to see its performance impact with -mtune=atom
> > on Atom.
> 
> Well, yes, I think the proposal was to spill/load the full SImode instead
> which would avoid both the partial dependency and the mismatched load/store
> size.  No?

It is for movqi.  We can only safely replace mozbl with movl if
the source is 4byte aligned.  It should a new backend option.

[Bug rtl-optimization/50088] movzbl is generated instead of movl

[hjl.tools at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50088

--- Comment #6 from H.J. Lu <hjl.tools at gmail dot com> 2011-08-15 17:19:22 UTC ---
Created attachment 25019
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=25019
A patch

[Bug rtl-optimization/50088] movzbl is generated instead of movl

[hjl.tools at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50088

H.J. Lu <hjl.tools at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |NEW
   Last reconfirmed|                            |2011-08-15
     Ever Confirmed|0                           |1

--- Comment #7 from H.J. Lu <hjl.tools at gmail dot com> 2011-08-15 17:26:50 UTC ---
(In reply to comment #6)
> Created attachment 25019 [details]
> A patch

misaligned_operand instead of aligned_operand is used since 
aligned_operand has

  /* All patterns using aligned_operand on memory operands ends up
     in promoting memory operand to 64bit and thus causing memory mismatch.  */
  if (TARGET_MEMORY_MISMATCH_STALL && !optimize_insn_for_size_p ())
    return false;

which isn't true for *movqi_internal.

[Bug rtl-optimization/50088] movzbl is generated instead of movl

[enkovich.gnu at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50088

--- Comment #8 from Ilya Enkovich <enkovich.gnu at gmail dot com> 2011-08-16 06:55:34 UTC ---
(In reply to comment #4)
> 
> Well, yes, I think the proposal was to spill/load the full SImode instead
> which would avoid both the partial dependency and the mismatched load/store
> size.  No?

Yes, I think we should generate full SImode spill/load.

[Bug rtl-optimization/50088] movzbl is generated instead of movl

[enkovich.gnu at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50088

--- Comment #9 from Ilya Enkovich <enkovich.gnu at gmail dot com> 2011-08-16 07:28:33 UTC ---
(In reply to comment #5)
> 
> It is for movqi.  We can only safely replace mozbl with movl if
> the source is 4byte aligned.  It should a new backend option.

That should work. 

A better solution here would be to not generate movqi at all. But probably it
was performed intentionally and is profitable for some platforms. In this case
we should choose movl generation for movqi.

[Bug rtl-optimization/50088] movzbl is generated instead of movl

[hjl.tools at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50088

--- Comment #10 from H.J. Lu <hjl.tools at gmail dot com> 2011-08-16 14:23:39 UTC ---
The real problem is the store forward issue on Atom:

        addl    $1, 4(%esp)     # 67    *addsi_1/2      [length = 5]
        andl    $15, 4(%esp)    # 68    *andsi_1/1      [length = 5]
        movl    (%eax), %edi    # 70    *movsi_internal/1       [length = 2]
        movzbl  4(%esp), %ecx   # 154   *movqi_internal/3       [length = 5]

That is we write 32bit and read 8bit, which performs very
poorly on Atom. We should write/read the same size.

[Bug rtl-optimization/50088] movzbl is generated instead of movl

[hjl.tools at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50088

--- Comment #11 from H.J. Lu <hjl.tools at gmail dot com> 2011-08-16 14:45:27 UTC ---
Can we model shift instructions to take any QI/HI/SI/DI register
as shift count and make IRA to match the size when reading/writing
shift count?

[Bug rtl-optimization/50088] movzbl is generated instead of movl

[hjl.tools at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50088

H.J. Lu <hjl.tools at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #25019|0                           |1
        is obsolete|                            |

--- Comment #12 from H.J. Lu <hjl.tools at gmail dot com> 2011-08-16 16:43:55 UTC ---
Created attachment 25025
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=25025
A patch to use the same mode for shift count

This is an untested patch to use the same mode for shift count.

[Bug rtl-optimization/50088] movzbl is generated instead of movl

[enkovich.gnu at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50088

--- Comment #13 from Ilya Enkovich <enkovich.gnu at gmail dot com> 2011-08-17 09:07:20 UTC ---
(In reply to comment #12)
> Created attachment 25025 [details]
> A patch to use the same mode for shift count
> 
> This is an untested patch to use the same mode for shift count.

We should find solution for the general problem. Not for its specific
appearance in reproducer. 

We may have the same issue for any other instructions consuming byte register
and it is better to fix the source of the problem (which is I suppose in IRA)
and do not introduce workaround for each such instruction.

BTW I think you should not increase size of immediate operands in your patch.

[Bug rtl-optimization/50088] movzbl is generated instead of movl

[hjl.tools at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50088

--- Comment #14 from H.J. Lu <hjl.tools at gmail dot com> 2011-08-17 13:37:39 UTC ---
(In reply to comment #13)
> (In reply to comment #12)
> > Created attachment 25025 [details]
> > A patch to use the same mode for shift count
> > 
> > This is an untested patch to use the same mode for shift count.
> 
> We should find solution for the general problem. Not for its specific
> appearance in reproducer. 
> 
> We may have the same issue for any other instructions consuming byte register
> and it is better to fix the source of the problem (which is I suppose in IRA)
> and do not introduce workaround for each such instruction.

I think this problem is unique to x86 since some instructions have
different sizes in register operands.  In this example, shift count
is CL regardless the source operand size. I am not sure how much RA
can help here. By making register operands in shift instructions to
have the same size (32bit or less), it may work for most cases.

> BTW I think you should not increase size of immediate operands in your patch.

My patch is totally untested and probably is wrong.

[Bug rtl-optimization/50088] movzbl is generated instead of movl

[enkovich.gnu at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50088

--- Comment #15 from Ilya Enkovich <enkovich.gnu at gmail dot com> 2011-08-17 14:16:27 UTC ---
(In reply to comment #14)
> 
> I think this problem is unique to x86 since some instructions have
> different sizes in register operands.  In this example, shift count
> is CL regardless the source operand size. I am not sure how much RA
> can help here. By making register operands in shift instructions to
> have the same size (32bit or less), it may work for most cases.
> 
We have a problem due to different sizes of spill and load generated by IRA for
the same var. I'm not sure that by patching shift instructions we cover all
cases when IRA may do that.

[Bug rtl-optimization/50088] movzbl is generated instead of movl

[hjl.tools at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50088

--- Comment #16 from H.J. Lu <hjl.tools at gmail dot com> 2011-08-17 14:37:35 UTC ---
The testcase has ...

  int n8 = (arr[7] * 9 + 8) & 15;

  for (i = 0; i < len; i+=8)
    {
      n1 = (n1 + 1) & 15;

      s += arr[i] << n1;

The shift count is 32bit, which causes 32bit spill. Since shift/rotate
instructions only take 8bit register (CL) as shift count, we load 8bit
into CL. How do we solve this?

[Bug rtl-optimization/50088] movzbl is generated instead of movl

[rguenth at gcc dot gnu.org]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50088

--- Comment #17 from Richard Guenther <rguenth at gcc dot gnu.org> 2011-08-17 14:42:11 UTC ---
(In reply to comment #16)
> The testcase has ...
> 
>   int n8 = (arr[7] * 9 + 8) & 15;
> 
>   for (i = 0; i < len; i+=8)
>     {
>       n1 = (n1 + 1) & 15;
> 
>       s += arr[i] << n1;
> 
> The shift count is 32bit, which causes 32bit spill. Since shift/rotate
> instructions only take 8bit register (CL) as shift count, we load 8bit
> into CL. How do we solve this?

Well, always load from the spill slot with the mode the reg was spilled
if the arch says that is prefered.

[Bug rtl-optimization/50088] movzbl is generated instead of movl

[hjl.tools at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50088

H.J. Lu <hjl.tools at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #25025|0                           |1
        is obsolete|                            |

--- Comment #18 from H.J. Lu <hjl.tools at gmail dot com> 2011-08-17 21:30:50 UTC ---
Created attachment 25040
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=25040
A new patch

[Bug rtl-optimization/50088] movzbl is generated instead of movl

[hjl.tools at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50088

H.J. Lu <hjl.tools at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #25040|0                           |1
        is obsolete|                            |

--- Comment #19 from H.J. Lu <hjl.tools at gmail dot com> 2011-08-17 22:28:55 UTC ---
Created attachment 25042
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=25042
A updated patch

[Bug rtl-optimization/50088] movzbl is generated instead of movl

[hjl.tools at gmail dot com]

http://gcc.gnu.org/bugzilla/show_bug.cgi?id=50088

H.J. Lu <hjl.tools at gmail dot com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
  Attachment #25042|0                           |1
        is obsolete|                            |

--- Comment #20 from H.J. Lu <hjl.tools at gmail dot com> 2011-08-17 22:38:40 UTC ---
Created attachment 25043
  --> http://gcc.gnu.org/bugzilla/attachment.cgi?id=25043
An updated patch