[Bug target/56027] New: ldmxcsr permuted with asm

[Bug target/56027] New: ldmxcsr permuted with asm

[glisse at gcc dot gnu.org]

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

             Bug #: 56027
           Summary: ldmxcsr permuted with asm
    Classification: Unclassified
           Product: gcc
           Version: 4.8.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: target
        AssignedTo: unassigned@gcc.gnu.org
        ReportedBy: glisse@gcc.gnu.org
            Target: x86_64-linux-gnu


Hello,

up to now, I had the impression that the use of a function like "opaque" below
was enough to prevent bad optimizations. However, in the following program,
compiled with g++ -O1 -frounding-math, the assertion fails. sqrtsd and *mxcsr
seem to get permuted (possibly in the loop2_* RTL passes). Defining macro V1 or
V2 seems to fix it, but it is slower and I don't know if it is completely safe.
Marking the asm volatile also works (and should have less impact on speed), but
again I don't know how safe that is.

#include <xmmintrin.h>
#include <assert.h>
#include <fenv.h>
inline double opaque(double x){
#ifndef V1
  asm("":"+xm"(x));
#else
  asm("":"+m"(x));
#endif
  return x;
}
inline double f(double x){
  double ret = __builtin_sqrt(opaque(x));
  return opaque(ret);
}
int main(){
  double d=2;
  double l,h;
  for(int i=0;i<100;++i){
#ifndef V2
  _MM_SET_ROUNDING_MODE(_MM_ROUND_DOWN);
#else
  fesetround(FE_DOWNWARD);
#endif
  l=f(d);
#ifndef V2
  _MM_SET_ROUNDING_MODE(_MM_ROUND_UP);
#else
  fesetround(FE_UPWARD);
#endif
  h=f(d);
  }
  assert(l!=h);
}

[Bug target/56027] ldmxcsr permuted with asm

[rguenth at gcc dot gnu.org]

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

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

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|UNCONFIRMED                 |WAITING
   Last reconfirmed|                            |2013-01-18
     Ever Confirmed|0                           |1

--- Comment #1 from Richard Biener <rguenth at gcc dot gnu.org> 2013-01-18 10:45:55 UTC ---
I think you want a pass-thru:

#define opaque(x) __asm volatile ("# x" : "=g" (x) : "0" (x))

[Bug target/56027] ldmxcsr permuted with asm

[glisse at gcc dot gnu.org]

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

--- Comment #2 from Marc Glisse <glisse at gcc dot gnu.org> 2013-01-18 11:14:53 UTC ---
(In reply to comment #1)
> I think you want a pass-thru:
> 
> #define opaque(x) __asm volatile ("# x" : "=g" (x) : "0" (x))

(opaque returns a value in my example, but that's a detail)
Replacing my asm with yours indeed works (as I said, adding volatile seems to
be enough).

The reasons we are using "mx" instead of "g" are:
* on 387 it has the bonus side effect of forcing the number to 64 bits,
* on some version of gcc (can't remember which) "g" somehow did not work as an
optimization barrier while "mx" did,
* there are few other places a double could be on x86,
* we have an alternate code for non-gcc or non-x86 where we write/read a
volatile variable.

I'll probably change it to "g" later though on platforms where it is safe,
thanks.

Is there a particular benefit in using "=g" and "0" instead of the shorter
"+g"?

I am mostly wondering what guarantees I have there won't be re-ordering. *mxcsr
are unspec_volatile and thus can commute with asm (register) but not asm
volatile or asm (memory in V1)? And function calls (fesetenv in V2) can't
commute with regular asm, volatile isn't required there?

(otherwise it's just the usual "-frounding-math doesn't work", the asm
shouldn't be necessary at all, but I'd like to be sure my workarounds work...)

[Bug target/56027] ldmxcsr permuted with asm

[rguenth at gcc dot gnu.org]

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

--- Comment #3 from Richard Biener <rguenth at gcc dot gnu.org> 2013-01-18 11:49:27 UTC ---
(In reply to comment #2)
> (In reply to comment #1)
> > I think you want a pass-thru:
> > 
> > #define opaque(x) __asm volatile ("# x" : "=g" (x) : "0" (x))
> 
> (opaque returns a value in my example, but that's a detail)
> Replacing my asm with yours indeed works (as I said, adding volatile seems to
> be enough).

Ah, yes.

> The reasons we are using "mx" instead of "g" are:
> * on 387 it has the bonus side effect of forcing the number to 64 bits,
> * on some version of gcc (can't remember which) "g" somehow did not work as an
> optimization barrier while "mx" did,
> * there are few other places a double could be on x86,
> * we have an alternate code for non-gcc or non-x86 where we write/read a
> volatile variable.
> 
> I'll probably change it to "g" later though on platforms where it is safe,
> thanks.
> 
> Is there a particular benefit in using "=g" and "0" instead of the shorter
> "+g"?

Not sure - I've just copied what is done elsewhere.  +g should work as well.

> I am mostly wondering what guarantees I have there won't be re-ordering. *mxcsr
> are unspec_volatile and thus can commute with asm (register) but not asm
> volatile or asm (memory in V1)? And function calls (fesetenv in V2) can't
> commute with regular asm, volatile isn't required there?

You always need volatile here, even if in practice it seems that it is
not required for a function call.  volatile tells it that the asm is
a scheduling barrier for other volatile instructions.

> (otherwise it's just the usual "-frounding-math doesn't work", the asm
> shouldn't be necessary at all, but I'd like to be sure my workarounds work...)

Yeah...

So - it works for you then and we can close this bug?

[Bug target/56027] ldmxcsr permuted with asm

[glisse at gcc dot gnu.org]

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

--- Comment #4 from Marc Glisse <glisse at gcc dot gnu.org> 2013-01-18 12:08:52 UTC ---
(In reply to comment #3)
> > I am mostly wondering what guarantees I have there won't be re-ordering. *mxcsr
> > are unspec_volatile and thus can commute with asm (register) but not asm
> > volatile or asm (memory in V1)? And function calls (fesetenv in V2) can't
> > commute with regular asm, volatile isn't required there?
> 
> You always need volatile here, even if in practice it seems that it is
> not required for a function call.  volatile tells it that the asm is
> a scheduling barrier for other volatile instructions.

Thanks, I'll do that.

> So - it works for you then and we can close this bug?

Give me a few hours, I'll close it this afternoon or tomorrow unless I come up
with a reason not to. I probably should have asked on gcc-help instead of
opening a PR.

[Bug target/56027] ldmxcsr permuted with asm

[glisse at gcc dot gnu.org]

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

Marc Glisse <glisse at gcc dot gnu.org> changed:

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

--- Comment #5 from Marc Glisse <glisse at gcc dot gnu.org> 2013-01-18 15:33:15 UTC ---
volatile is necessary for asm to play the barrier role I was expecting, see
Richard's explanations.