[Bug target/55752] New: __builtin_ia32_ldmxcsr / __builtin_ia32_stmxcsr are not scheduling barriers

[Bug target/55752] New: __builtin_ia32_ldmxcsr / __builtin_ia32_stmxcsr are not scheduling barriers

[rguenth at gcc dot gnu.org]

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

             Bug #: 55752
           Summary: __builtin_ia32_ldmxcsr / __builtin_ia32_stmxcsr are
                    not scheduling barriers
    Classification: Unclassified
           Product: gcc
           Version: 4.8.0
            Status: UNCONFIRMED
          Keywords: wrong-code
          Severity: normal
          Priority: P3
         Component: target
        AssignedTo: unassigned@gcc.gnu.org
        ReportedBy: rguenth@gcc.gnu.org
            Target: x86_64-*-*


float foo (float x, float f32)
{
  unsigned int mxscr_stat;
  mxscr_stat = __builtin_ia32_stmxcsr ();
  __builtin_ia32_ldmxcsr (mxscr_stat | 0x00000800);
  f32 = (x + f32) - f32;
  mxscr_stat = mxscr_stat & 0xffffffc0;
  __builtin_ia32_ldmxcsr (mxscr_stat);
  return f32;
}

Compiled at O2 yields:

foo:
.LFB0:
        .cfi_startproc
        stmxcsr -4(%rsp)
        movl    -4(%rsp), %eax
        movl    %eax, %edx
        orb     $8, %dh
        movl    %edx, -4(%rsp)
        ldmxcsr -4(%rsp)
        addss   %xmm1, %xmm0
        andl    $-64, %eax
        movl    %eax, -4(%rsp)
        ldmxcsr -4(%rsp)
        subss   %xmm1, %xmm0
        ret

note how the subss is scheduled after the ldmxcsr call.

It's ok (by pure luck of course) at the GIMPLE level.

[Bug target/55752] __builtin_ia32_ldmxcsr / __builtin_ia32_stmxcsr are not scheduling barriers

[ubizjak at gmail dot com]

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

--- Comment #1 from Uros Bizjak <ubizjak at gmail dot com> 2012-12-20 12:59:40 UTC ---
The RTXes that corresponds to builtins are all declared unspec_volatile.
According to the comment in sched-deps.c, around line 2723, it is assumed that
unspec_volatile clobbers all registers.

[Bug target/55752] __builtin_ia32_ldmxcsr / __builtin_ia32_stmxcsr are not scheduling barriers

[rguenther at suse dot de]

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

--- Comment #2 from rguenther at suse dot de <rguenther at suse dot de> 2012-12-20 13:07:20 UTC ---
On Thu, 20 Dec 2012, ubizjak at gmail dot com wrote:

> 
> http://gcc.gnu.org/bugzilla/show_bug.cgi?id=55752
> 
> --- Comment #1 from Uros Bizjak <ubizjak at gmail dot com> 2012-12-20 12:59:40 UTC ---
> The RTXes that corresponds to builtins are all declared unspec_volatile.
> According to the comment in sched-deps.c, around line 2723, it is assumed that
> unspec_volatile clobbers all registers.

Ok, it seems it's TER that moves the subtraction.

Richard.

[Bug target/55752] __builtin_ia32_ldmxcsr / __builtin_ia32_stmxcsr are not scheduling barriers

[rguenth at gcc dot gnu.org]

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

Richard Biener <rguenth at gcc dot gnu.org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |ASSIGNED
   Last reconfirmed|                            |2012-12-20
         AssignedTo|unassigned at gcc dot       |rguenth at gcc dot gnu.org
                   |gnu.org                     |
     Ever Confirmed|0                           |1

--- Comment #3 from Richard Biener <rguenth at gcc dot gnu.org> 2012-12-20 13:32:02 UTC ---
TER already avoids moving things across calls but:

      /* Increment counter if this is a non BUILT_IN call. We allow
         replacement over BUILT_IN calls since many will expand to inline
         insns instead of a true call.  */
      if (is_gimple_call (stmt)
          && !((fndecl = gimple_call_fndecl (stmt))
               && DECL_BUILT_IN (fndecl)))
        cur_call_cnt++;

so it special-cases all builtins (I can see __builtin_sqrt as a good
example where this is a good idea).  OTOH __builtin_ia32_stmxcsr/
__builtin_ia32_ldmxcsr are neither const nor pure, so maybe
restricting this further, like

      /* Increment counter if this is not a BUILT_IN call without
         side-effects.  We allow replacement over BUILT_IN calls 
         since many will expand to inline insns instead of a true call.  */
      if (is_gimple_call (stmt)
          && (!((fndecl = gimple_call_fndecl (stmt))
                && DECL_BUILT_IN (fndecl))
              || gimple_has_side_effects (stmt)))
        cur_call_cnt++;