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

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>

On Dec 11, 2015, at 4:55 PM, Philip Reames <listmail at philipreames.com>
wrote:
> 
> 
> 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 <
<mailto:llvm-dev at lists.llvm.org>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.
I apologize for being unclear.

I only meant that in C and C++, "atomic_load",
"atomic_store", "atomic_exchange", and
"atomic_compare_exchange_{weak,strong} are supported for all types,
including integers, floats, vectors, aggregates, and pointers. On the other hand
the atomic_fetch_{add,sub,and,or,...} operations are not -- they only work on
integers and, for "add" and "sub", pointers.

I think that similarly makes sense in llvm: the instructions
"atomic_load", "atomic_store", "atomicrmw xchg",
and "cmpxchg" should all support float/vector types of the
target-allowed sizes: the mechanism you proposed of adding bitcasts in the
AtomicExpandPass should work for all four of those.

But supporting actual *arithmetic* operations on other types, e.g.
"atomicrmw add", "atomicrmw sub", or any of the others, is a
completely separate issue, and I don't think it makes any sense to consider
that as a part of what you're doing.
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20151211/6c79122d/attachment.html>

On 12/11/2015 03:27 PM, James Y Knight wrote:
> On Dec 11, 2015, at 4:55 PM, Philip Reames <listmail at philipreames.com
> <mailto:listmail at philipreames.com>> wrote:
>>
>>
>> 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> 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.
>
> I apologize for being unclear.
>
> I only meant that in C and C++, "atomic_load",
"atomic_store",
> "atomic_exchange", and
"atomic_compare_exchange_{weak,strong} are
> supported for all types, including integers, floats, vectors, 
> aggregates, and pointers. On the other hand the 
> atomic_fetch_{add,sub,and,or,...} operations are not -- they only work 
> on integers and, for "add" and "sub", pointers.
>
> I think that similarly makes sense in llvm: the instructions 
> "atomic_load", "atomic_store", "atomicrmw
xchg", and "cmpxchg" should
> all support float/vector types of the target-allowed sizes: the 
> mechanism you proposed of adding bitcasts in the AtomicExpandPass 
> should work for all four of those.
I agree, though, unless reviewers really push for it, I only plan on 
implementing the first two for now.  I may get to the later at a future 
point, but they're not on my must have list right now.

(Actually, I might get to at least cmpxchg sooner rather than later.  I 
just noticed we had a bit of code in our frontend hacking around that
too.)
>
> But supporting actual *arithmetic* operations on other types, e.g. 
> "atomicrmw add", "atomicrmw sub", or any of the others,
is a
> completely separate issue, and I don't think it makes any sense to 
> consider that as a part of what you're doing.
Again, I'm not sure what you're getting at.  We already support 
"atomicrmw add".  Did you possible mean "atomicrmw fadd"? 
Or were you
using "add" in place of "some_other_op_we_dont_yet_support"?

Philip

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