llvm dev - Dec 2015 - RFC: Extending atomic loads and stores to floating point and vector types

On 12/11/2015 12:05 AM, JF Bastien wrote:
> On Fri, Dec 11, 2015 at 3:22 AM, Philip Reames via llvm-dev 
> <llvm-dev at lists.llvm.org <mailto:llvm-dev at
lists.llvm.org>> wrote:
>
>     Currently, we limit atomic loads and stores to either pointer or
>     integer types.  I would like to propose that we extend this to
>     allow both floating point and vector types which meet the other
>     requirements.  (i.e. power-of-two multiple of 8 bits, and aligned)
>
>
> I support this.
Great.  I'll prepare a patch and send it for review.
>
>
>     This will enable a couple of follow on changes:
>     1) Teaching the vectorizer how to vectorize unordered atomic loads
>     and stores
>     2) Removing special casing around type canonicalization of loads
>     in various passes
>     3) Removing complexity from language frontends which need to
>     support atomic operations on floating point types.
>
>
> This may become relevant for C++, see http://wg21.link/p0020r0
>
>
>     My initial implementation plan will not require any changes from
>     the backends.  I plan to add a lowering step to the existing
>     AtomicExpandPass which will convert atomic operations on floats
>     and vectors to their equivalently sized integer counterparts. 
>     Over time, individual backends will be able to opt in - via a TTI
>     hook - to have the new types of atomics handled by the normal isel
>     machinery.
>
>     I've prototyped this approach with the x86 backend and get what
>     looks like correct and even fairly efficient instruction selection
>     taking place.  I haven't studied it too extensively, so it might
>     not work out in the end, but the approach appears generally feasible.
>
>
> Simpler path that D11382 <http://reviews.llvm.org/D11382>, generating
> the same code?
Actually, I think I was relying on exactly that change.  :)

My initial implementation just converts "store atomic float 0.0, float* 
%p unordered, align 4" to
%p.i = bitcast float* %p to i32*
%v = bitcast float 0.0 to i32
store atomic i32 %v, i32* %p.i unordered, align 4

(i.e. it just does what the frontend previously did in the AtomicExpand.)

The net result is that the backend still sees the bitcast idioms you 
added pattern matching for.  Once that parts in, I'll start changing the 
canonical form the backend expects, but given that's backend specific, I 
wanted a migration mechanism which worked for all backends.

p.s. There's still lots of work to do to improve isel for atomic loads 
and stores on these types.  I plan to eventually get there, but my first 
focus is on a) functional correctness, and b) getting the middle-end 
optimizers fixed up.
>
>
>     One open question I don't know the answer to: Are there any
>     special semantics required from floating point stores which aren't
>     met by simply bitcasting their result to i32 (float) or i64
>     (double) and storing the result?  In particular, I'm unsure of the
>     semantics around canonicalization here.  Are there any? Same for
>     loads?
>
>
> I'd go a bit further: should you also support basic FP operations 
> atomically? The above C++ paper adds add/sub, and we've discussed 
> adding FMA as well.
Just to be clear, I assume you mean extending the atomicrmw, and cmpxchg 
instructions right?  I agree this is worth doing, but I'm purposely 
separating that into a possible later proposal.  I don't need this right 
now and keeping the work scoped reasonably is key to making
progress.
>
> This raises similar issues around FP exceptions (are they 
> impl-defined, UB, unspecified? Do we care?).
In practice, we seem not to care.  This is much broader than my 
proposal, and I have no opinions on what the right solutions here are.

Philip

-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20151211/f6597d96/attachment.html>

On Fri, Dec 11, 2015 at 3:05 PM, Philip Reames via llvm-dev <
llvm-dev at lists.llvm.org> wrote:
>
> One open question I don't know the answer to: Are there any special
>> semantics required from floating point stores which aren't met by
simply
>> bitcasting their result to i32 (float) or i64 (double) and storing the
>> result?  In particular, I'm unsure of the semantics around
canonicalization
>> here.  Are there any? Same for loads?
>>
>
> I'd go a bit further: should you also support basic FP operations
> atomically? The above C++ paper adds add/sub, and we've discussed
adding
> FMA as well.
>
> Just to be clear, I assume you mean extending the atomicrmw, and cmpxchg
> instructions right?  I agree this is worth doing, but I'm purposely
> separating that into a possible later proposal.  I don't need this
right
> now and keeping the work scoped reasonably is key to making progress.
>
It seems a unfortunate, and potentially problematic if llvm doesn't
support, at least, "atomicrmw xchg" and "cmpxchg". If you
support just
those two additionally to load/store, it'll cover everything the C frontend
actually needs to be able to do *now* with floating point atomics.

Atomic floating point math isn't actually a supported operation in C now,
so it seems reasonable to leave the rest of the atomicrmw ops for a future
design.
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20151211/1b06a651/attachment.html>

On 12/11/2015 01:29 PM, James Y Knight wrote:
>
> On Fri, Dec 11, 2015 at 3:05 PM, Philip Reames via llvm-dev 
> <llvm-dev at lists.llvm.org <mailto:llvm-dev at
lists.llvm.org>> wrote:
>
>>         One open question I don't know the answer to: Are there any
>>         special semantics required from floating point stores which
>>         aren't met by simply bitcasting their result to i32 (float)
>>         or i64 (double) and storing the result?  In particular, I'm
>>         unsure of the semantics around canonicalization here.  Are
>>         there any? Same for loads?
>>
>>
>>     I'd go a bit further: should you also support basic FP
operations
>>     atomically? The above C++ paper adds add/sub, and we've
discussed
>>     adding FMA as well.
>     Just to be clear, I assume you mean extending the atomicrmw, and
>     cmpxchg instructions right?  I agree this is worth doing, but I'm
>     purposely separating that into a possible later proposal.  I don't
>     need this right now and keeping the work scoped reasonably is key
>     to making progress.
>
>
> It seems a unfortunate, and potentially problematic if llvm doesn't 
> support, at least, "atomicrmw xchg" and "cmpxchg". If
you support just
> those two additionally to load/store, it'll cover everything the C 
> frontend actually needs to be able to do *now* with floating point 
> atomics.
>
> Atomic floating point math isn't actually a supported operation in C 
> now, so it seems reasonable to leave the rest of the atomicrmw ops for 
> a future design.
I'm really not sure what you're trying to say here.  Your two paragraphs
seem to contradict each other?

For the record, I actually don't care about the C frontend at all. If my 
work helps the clang, great, but that's not my goal.  Others can step up 
to push the changes through clang if they want to see the benefit there.

Philip

-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20151211/6b3ff2a0/attachment.html>