Thread overview
[Bug 84] Writes to struct members marked as shared are not volatile
Nov 10, 2013
Johannes Pfau
Nov 10, 2013
Iain Buclaw
Feb 13, 2014
Johannes Pfau
Feb 13, 2014
Iain Buclaw
Feb 13, 2014
Iain Buclaw
Feb 13, 2014
Johannes Pfau
November 10, 2013
http://bugzilla.gdcproject.org/show_bug.cgi?id=84

--- Comment #1 from Johannes Pfau <johannespfau@gmail.com> 2013-11-10 09:18:22 GMT ---
In the generated ASM it looks like at least part of the problem is that the struct is loaded into a register. Of course, then volatile has no effect. Quoting from c-decl.c:

--------------------------------------
    /* If a type has volatile components, it should be stored in memory.
       Otherwise, the fact that those components are volatile
       will be ignored, and would even crash the compiler.
       Of course, this only makes sense on  VAR,PARM, and RESULT decl's.   */
    if (C_TYPE_FIELDS_VOLATILE (TREE_TYPE (decl))
    && (TREE_CODE (decl) == VAR_DECL ||  TREE_CODE (decl) == PARM_DECL
      || TREE_CODE (decl) == RESULT_DECL))
      {
    /* It is not an error for a structure with volatile fields to
       be declared register, but reset DECL_REGISTER since it
       cannot actually go in a register.  */
    int was_reg = C_DECL_REGISTER (decl);
    C_DECL_REGISTER (decl) = 0;
    DECL_REGISTER (decl) = 0;
    c_mark_addressable (decl);
    C_DECL_REGISTER (decl) = was_reg;
      }
--------------------------------------


Also not working:
--------------------------------------
shared struct Register
{
    size_t a;
}

Register* reg = cast(Register*)0xFFDDCCAA;

void main()
{
     for(size_t i = 0; i < 10; i++)
         reg.a = i;
}
--------------------------------------


Working:
--------------------------------------
shared struct Register //Also working without shared here
{
    size_t a;
}

shared(Register*) reg = cast(shared(Register*))0xFFDDCCAA;

void main()
{
     for(size_t i = 0; i < 10; i++)
         reg.a = i;
}
--------------------------------------

-- 
Configure bugmail: http://bugzilla.gdcproject.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching all bug changes.
November 10, 2013
http://bugzilla.gdcproject.org/show_bug.cgi?id=84

--- Comment #2 from Iain Buclaw <ibuclaw@gdcproject.org> 2013-11-10 11:23:45 GMT ---
(In reply to comment #1)
> In the generated ASM it looks like at least part of the problem is that the struct is loaded into a register. Of course, then volatile has no effect.
> 

Yep, marking the decl as addressable should make the problem go away.

-- 
Configure bugmail: http://bugzilla.gdcproject.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching all bug changes.
February 13, 2014
http://bugzilla.gdcproject.org/show_bug.cgi?id=84

Johannes Pfau <johannespfau@gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|                            |WONTFIX

--- Comment #3 from Johannes Pfau <johannespfau@gmail.com> 2014-02-13 07:58:06 GMT ---
I had this discussion with Kagamin lately and came to the conclusion that GDC should not treat shared variables as volatile. There are some optimizations that are legal on shared variables but not on volatile ones, so requiring that shared implies volatile is a bad idea.

Of course we need some way to use volatile nevertheless. I'll post some ideas on the newsgroup soon.

-- 
Configure bugmail: http://bugzilla.gdcproject.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching all bug changes.
February 13, 2014
http://bugzilla.gdcproject.org/show_bug.cgi?id=84

--- Comment #4 from Iain Buclaw <ibuclaw@gdcproject.org> 2014-02-13 08:26:55 GMT ---
I'm pretty persistent on forcing shared variables to not go into registers. However we need a better system for setting TREE_ADDRESSABLE on the type without causing GCC backend to panic and die on us.

-- 
Configure bugmail: http://bugzilla.gdcproject.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching all bug changes.
February 13, 2014
http://bugzilla.gdcproject.org/show_bug.cgi?id=84

--- Comment #5 from Iain Buclaw <ibuclaw@gdcproject.org> 2014-02-13 08:27:22 GMT ---
(If alternatives is what we should go for)

-- 
Configure bugmail: http://bugzilla.gdcproject.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching all bug changes.
February 13, 2014
http://bugzilla.gdcproject.org/show_bug.cgi?id=84

--- Comment #6 from Johannes Pfau <johannespfau@gmail.com> 2014-02-13 15:01:52 GMT ---
Not into registers might be OK, but if we have code like this:

-------------
shared int a;
void doSomething()
{
    for(int i = 0; i < 10; i++)
        atomicStore(a, i);
}
-------------

I think it is legal* to optimize the for loop into a single assignment, 'a = 9', for shared variables. This wouldn't be allowed for volatile variables.

* http://www.drdobbs.com/parallel/volatile-vs-volatile/212701484
Basically the 'as if' rule: If the 'doSomething' thread executes much faster
than a thread reading 'a' the reading thread might only see the final value, 9,
anyway.

-- 
Configure bugmail: http://bugzilla.gdcproject.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are watching all bug changes.