[Bug d/94425] New: [D] Consider always settings ASM_VOLATILE_P on asm statements

[Bug d/94425] New: [D] Consider always settings ASM_VOLATILE_P on asm statements

[ibuclaw at gdcproject dot org]

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

            Bug ID: 94425
           Summary: [D] Consider always settings ASM_VOLATILE_P on asm
                    statements
           Product: gcc
           Version: 10.0
            Status: UNCONFIRMED
          Severity: normal
          Priority: P3
         Component: d
          Assignee: ibuclaw at gdcproject dot org
          Reporter: ibuclaw at gdcproject dot org
  Target Milestone: ---

Currently, the following have equivalents with the c-family front-ends.

asm      { ""; } -> asm __volatile__ ("");
asm pure { ""; } -> asm ("");

In the case of 'asm pure' the result can be cached, with duplicated statements
being DCE'd.  This happens in the std.math unittests when compiling with -O2.

The example below distils what the test looks like after inlining.  The last
asm statement is removed, and the first result is reused, which produces wrong
run-time results.  The use of 'asm pure' is in line with expectations however,
particularly as the getters are 'pure' in the D sense.

Either will have to say that every asm statement is volatile, or will have to
apply a stricter condition.

In C, asm statements without output operands are automatically considered
volatile, but that won't help us here.

---
import core.stdc.fenv;

enum ROUNDING_MASK = FE_TONEAREST|FE_DOWNWARD|FE_UPWARD|FE_TOWARDZERO;

void main()
{
    uint newMode = FE_DOWNWARD;
    fexcept_t oldState;
    asm pure
    {
        "fstcw %0;" : "=m" (oldState);
    }
    fexcept_t newState = cast(fexcept_t)((oldState & (-1 - ROUNDING_MASK))
                                         | (newMode & ROUNDING_MASK));
    asm
    {
        "fclex; fldcw %0;" : : "m" (newState);
    }
    uint mxcsr;
    asm
    {
        "stmxcsr %0" : "=m" (mxcsr);
    }
    mxcsr &= ~(ROUNDING_MASK << 3);
    mxcsr |= (newState & ROUNDING_MASK) << 3;
    mxcsr &= ~(FE_ALL_EXCEPT << 7);
    mxcsr |= (newState & FE_ALL_EXCEPT) << 7;
    asm
    {
        "ldmxcsr %0" : : "m" (mxcsr);
    }
    fexcept_t contState;
    asm pure
    {
        "fstcw %0;" : "=m" (contState);
    }
    assert(cast(uint)(contState & ROUNDING_MASK) == FE_DOWNWARD);
}

[Bug d/94425] [D] Consider always settings ASM_VOLATILE_P on asm statements

[rguenth at gcc dot gnu.org]

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

--- Comment #1 from Richard Biener <rguenth at gcc dot gnu.org> ---
Well, there's no dependence visible to the compiler between the control word
stores and loads so it's obvious the asms cannot be pure.  Is 'asm' a D
feature or a GCC extension?  How does D model dependences between asms?

[Bug d/94425] [D] Consider always settings ASM_VOLATILE_P on asm statements

[ibuclaw at gdcproject dot org]

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

--- Comment #2 from Iain Buclaw <ibuclaw at gdcproject dot org> ---
(In reply to Richard Biener from comment #1)
> Well, there's no dependence visible to the compiler between the control word
> stores and loads so it's obvious the asms cannot be pure.  Is 'asm' a D
> feature or a GCC extension?  How does D model dependences between asms?

It should be considered a feature, though the spec is ignored for the sake of
both convenience and portability.

As far as the semantic analyzer treats them, they are just black holes assumed
to do anything.

The reference compiler (DMD) even goes as far as refusing to inline functions
with that contain any asm statements.  That could be another solution, but
allow overriding with pragma(inline).

[Bug d/94425] [D] Consider always settings ASM_VOLATILE_P on asm statements

[ibuclaw at gdcproject dot org]

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

--- Comment #3 from Iain Buclaw <ibuclaw at gdcproject dot org> ---
For now I'll default it to ASM_VOLATILE_P.  Perhaps with a possibility to turn
it off in the future as an aggressive optimization outlined in pr94496.

[Bug d/94425] [D] Consider always settings ASM_VOLATILE_P on asm statements

[cvs-commit at gcc dot gnu.org]

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

--- Comment #4 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Iain Buclaw <ibuclaw@gcc.gnu.org>:

https://gcc.gnu.org/g:30d26118f96fa542ee078834bc3cb4eef6730451

commit r10-7583-g30d26118f96fa542ee078834bc3cb4eef6730451
Author: Iain Buclaw <ibuclaw@gdcproject.org>
Date:   Tue Mar 31 00:24:13 2020 +0200

    d: Always set ASM_VOLATILE_P on asm statements (PR94425)

    gcc/d/ChangeLog:

            PR d/94425
            * toir.cc (IRVisitor::visit (GccAsmStatement *)): Set
ASM_VOLATILE_P
            on all asm statements.

[Bug d/94425] [D] Consider always settings ASM_VOLATILE_P on asm statements

[ibuclaw at gdcproject dot org]

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

Iain Buclaw <ibuclaw at gdcproject dot org> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
         Resolution|---                         |FIXED
             Status|UNCONFIRMED                 |RESOLVED

--- Comment #5 from Iain Buclaw <ibuclaw at gdcproject dot org> ---
Done.