[RFC] Volatile representation in Flang

Subprojects Flang
1

Flang currently lacks support for volatile variables. For some cases, the compiler produces TODO error messages and others are ignored. Some of our tests are like the example from C.4 Clause 8 notes: The VOLATILE attribute (8.5.20) and require volatile variables.

To represent volatile entities, we discussed four solutions. The sub-sections have reasons why I think the first solution is the right one and the other three are not, and some implementation considerations.

1. Ref/box types can be volatile

  1. Requires minimal changes. Explicit uses of ReferenceType can remain, but take an extra bool argument for volatility, defaulting to false.

  2. Volatile refs and regular refs to the same element types will not compare equal.

  3. Mishandling a volatile reference type will give us errors instead of silently dropping info (this applies to all the type-based solutions). If an op assumes it takes a non-volatile ref but the input is a volatile ref, we’ll get unrealized conversion casts in the IR instead of silently dropping volatility, as may happen if we represent volatility with an attribute on the ops instead of the type.

  4. Memory effects of ops that can take volatile refs can be updated to give read/write effects on an abstract memory resource when the reference type is volatile so MLIR optimizations don’t make bad assumptions about reads/writes to/from volatile memory.

  5. Represented as:

    `ref` `<` type (`, volatile` $volatile^)? `>`

    Or if we extend this with an explicitly asynchronous type:

    `ref` `<` type (`, volatile` $volatile^)? (`, async` $async^)? `>`

2. Duplicate all reference-like types with a volatile variant

  1. Something like fir.volatile_ref<T>/fir.volatile_box<T>
  2. This leads to an explosion in the type system. For example, will we do the same thing for async with fir.volatile_and_async_ref<T>? What about optional?
  3. Cumbersome - all explicit uses of ReferenceType would need to handle a VolatileReferenceType differently or use the convenience functions like fir::isa_ref_type. This might reduce the likelihood of mishandling volatile types but would be a huge amount of work.

3. Introduce a volatile type that thinly wraps another type

  1. Leads to ambiguities. When existing code gets the element type of a fir.ref<fir.volatile<T>>, should it get the volatile type or the inner type? If the former, we would have to check for volatility everywhere we unwrap types before unwrapping another layer to get the “real” element type. This gets worse if/when we add asynchrony and provides no extra benefits, since the user of the type can check for volatility on the reference type instead of unwrapping.

4. Propagate attributes on ops with memory effects on a volatile entity

  1. Very simple to get started with, but attributes are often lost in transformations/conversions. Each op with memory effects would have to copy over the attributes.
  2. If an attribute is dropped in a conversion pass, we’ll get no errors and silently do the wrong thing. Flang does this currently - if you declare a local variable to be volatile, the fortran_attrs attribute on the hlfir.declare op will correctly contain volatile, but they will be dropped in hlfir to fir conversion.
  3. This is how the LLVM dialect handles volatility, but representing volatility in the type allows for more consistent propagation through the compiler, and lowering to LLVM dialect ops with volatile attributes is simple. FIRCG can check if the reference type is volatile when creating a load/store/memcopy/etc.

Most of these ideas are @jeanPerier 's and not my own.

I opened a draft pr for the first solution. Thanks in advance for any feedback!

3 Likes
2

Looks good! I think solution 1 seems the nicest.

1 Like
3

+1 for solution 1

4

The problem with flags is that existing, new, and modified code paths have to be modified to check for the flag. Flags are harder to harder to maintain and harder to debug.

An advantage of a distinct type for VOLATILE entity is that it may be easier to implement if code is designed to fail at compile-time with unknown types.

5

I will argue the opposite: it is easier to control/make sure code is aware of VOLATILE aspect by adding a flag in the existing memory type and making it a mandatory (not optional) value in the type builder/ctor of that type. That way, people have to provide some VOLATILE parameter when construction new result types and think for a minute where this information should come from in their case.

If you make its own type, people can keep using the old one and drop the VOLATILE aspects on the floor.

Also, if you look at Asher’s patch, there is little code that manipulate the exact type coming in into an operation, it just need to be a raw pointer (or descriptor), and most code will use helpers to extract the info they care about. So the new type usage and “discovery” would be reduced to the is_volatile_ref helper which would be little different from the type flag approach.

That means that to be safe, all these helpers that extract information from “raw pointer” types should be disabled by default (and other helpers should be used in code cleared to deal with VOLATILE), that is doable, but will be a long road of compiler asserts given VOLATILE can basically appear in most of the Fortran feature, so we need to handle it everywhere.

Of course, ignoring VOLATILE is not good either, the main risk I see are FIR to FIR transformations that would not check for the VOLATILE aspect and do something not legal, and most of these passes are usually not checking the exact type (only raw pointer vs descriptor).

The safest way I see would be to duplicate all of our operations accessing VOLATILE memory, so that no FIR optimization is applied to them unexpectedly (since passes/patterns works on an operation basis, this would yield a strong go/no-go design). There would still be a weakness because the flag checking duties would just be pushed to lowering from parse-tree to FIR (in many current and future places). Lowering also does not really care what Expressions are exactly (that is, MATMUL lowering is not set-up to check if it first argument is a VOLATILE symbol), so it would not be trivial to place TODOs at that level. That approach would cost a lot of kloc with little exercise from the outside world (VOLATILE usages are not very common and do not appear in “complex” language situations in the real world, when they technically can). In my opinion we would introduce more bugs because of the duplications.

So, I fully agree the type attribute approach will likely have some bugs now or in the future where someone write a pass that does not check generic read/write effect, or add an operation that does not properly advertise the correct side effect when accessing a VOLATILE input. But I will argue we will have less bugs going that way and not making VOLATILE specific passes/code that will be poorly tested (unless you can give me couple thousands test using VOLATILE with “actual” volatility on variables used in most Fortran feature/intrinsic).

Last point is that the Fortran standard has only a very loose specification of VOLATILE. In section 8.5.20 “The Fortran processor should use the most recent definition of a volatile object each time its value is required.” Notice the “should” that according to ISO is only a recommendation.

We can maybe restrict VOLATILE types to be used in only a specific set of well defined operations (the safest way to do it being to do it at the IR verification level which may not be very user friendly).

6

I mostly agree with these points from Steve:

  1. Flags are harder to harder to maintain and harder to debug (in general)
  2. The advantage of a distinct type is that it may be easier to implement if code is designed to fail at compile-time with unknown types

After partially implementing both solutions though, I still think a flag on the plain reference type is better for these reasons:

  1. My first pass at this used a completely distinct type fir.volatile_ref. When I updated the ops that previously took/produced fir.refs to take either type I got compile-time errors in places where I needed to add casts/type switches (great!). However, this was just a subset of the changes I needed to make since most places use utility functions to get the type info they care about like “is this a ref- or pointer-like thing” which still holds true since I added VolatileReferenceType to the type switches used in the utility functions. This line from the PR for example: if we make the reference type constructor take an argument for volatility without a default like Jean suggests, it seems as explicit as the distinct type version to me. The author is still forced to decide what to about volatile, it’s just an argument instead of a different constructor.
    // Solution 1
const bool isVolatile = fir::isa_volatile_ref_type(variable.getType());
return fir::ReferenceType::get(eleTy, isVolatile);

// Solution 2
const bool isVolatile = fir::isa_volatile_ref_type(variable.getType());
return isVolatile
    ? fir::VolatileReferenceType::get(eleTy)
    : fir::ReferenceType::get(eleTy);
  2. With solution 1, when a volatile ref was not properly handled, the IR was littered with unrealized_conversion_casts and I still got compilation errors instead of missing info. The process of finding all the dropped volatile type info was mostly the same for both solutions.

Thanks for all the input!

7

Kiran had a really helpful suggestion: an explicit fir.volatile_cast op that can only change a type’s volatility. fir.convert and ops that return boxes/references can check that they are not changing the volatility of an op, this way volatility is only ever changed by an explicit use of fir.volatile_cast and we get an error if it’s not accounted for. This still requires that we add verifier checks but has already turned up lots of places where volatility is currently unhandled.

2 Likes