stfld not give .volatile annotation

[KevinRansom]

Opened on Codeplex by @mrange

Hi.
I have been experimenting a bit with VolatileFieldAttribute in F#. The documentation is rather sparse for VolatileFieldAttribute and I don't find many examples when checking stackoverflow etc. So I might have the wrong expectations what it's supposed to do.

Anyway; I am annotating a let bindings with VolatileFieldAttribute:

    [<VolatileField>] 
    let mutable creator = Unchecked.defaultof<_>

When checking what metadata the F# compiler generates I find this:

    [VolatileField]
    internal FSharpFunc<Unit, T> creator;

I was expecting this:

    [VolatileField]
    internal volatile FSharpFunc<Unit, T> creator;

When annotating a field in C#:

    volatile int x = 0;

I get what I expect:

    private volatile int x;

If VolatileFieldAttribute intended as a volatile attribute for F# I think the compiler should annotate the field with volatile.

I am obviously interested in volatile in order to prevent issues arising from memory reordering.

Regards,
Mårten

comments
jackpappas wrote Jul 27, 2014 at 7:55 AM [x]
Mårten -- When you mark a field with the 'volatile' modifier in C#, the C# compiler emits a special type for the field within the CLR metadata describing the field. In IL, your example looks like this:

 .field private int32 modreq([mscorlib]System.Runtime.CompilerServices.IsVolatile) x

The 'modreq' (required modifier) on the type works something like an attribute for the type -- it says that the field is an int32 with some special semantics that consumers of the field must obey. (If the special semantics were optional, a 'modopt' would be used instead.)

The important thing here is that in .NET, marking a type with these modifiers does nothing to alter the way the CLR treats the field itself. What is important is that it tells the compiler consuming the field that it needs to emit the 'volatile.' prefix on any IL instructions that load/store a value from/to the field. The F# compiler simply uses a different marker (the [] attribute), but if you compare the IL generated for C# and F# code accessing a volatile field, they should be roughly the same; at least, both of them should be using the 'volatile.' prefix on the instructions accessing the field.

Cheers,
Jack

marten_range wrote Jul 27, 2014 at 1:50 PM [x]
Thanks for the extensive answer.
I suspect that since one can't make public fields in F# (IIRC) there shouldn't be any C#/F# interop issues wrt volatiles.
I checked the generated IL code:

For loads it .volatile seems to be injected

IL_0017:  volatile.
IL_0019:  ldfld      <XXX>::creator

However I am missing it for stores:

IL_0036:  ldnull
IL_0037:  stfld      <XXX>::creator

Not sure if this is an issue or not but I checked code generated by the C# compiler which does seem to inject volatile for stores as well.

IL_0001:  volatile.
IL_0003:  ldfld      <XXX>::m_x
IL_0002:  volatile.
IL_0004:  stfld      <XXX>::m_x

Mårten

dsyme wrote Jul 29, 2014 at 11:37 AM [x]
The missing ".volatile" on stores is surely a bug.
The relevant line where "volatile" is added for reads is here: https://github.com/fsharp/fsharp/blob/master/src/fsharp/ilxgen.fs#L2225
The location where we should be adding this for stores is here:

https://github.com/fsharp/fsharp/blob/master/src/fsharp/ilxgen.fs#L2247
This should be fixed.