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

On Mon, Dec 14, 2015 at 11:46 AM, Philip Reames <listmail at
philipreames.com>
wrote:
>
>
> On 12/12/2015 01:44 PM, JF Bastien wrote:
>
> On Sat, Dec 12, 2015 at 1:24 AM, Philip Reames <listmail at
philipreames.com>
> wrote:
>
>> Patch posted for review: http://reviews.llvm.org/D15471
>
>
> Looking at the patch, I think we should do FP only for now as vectors have
> extra complexities which IMO warrant more discussion.
>
> I'm fine with splitting the patch up to make progress.  I'll post a
split
> patch shortly.
>
Thanks. FWIW I think we'll want to do vectors, but I think it's trickier
than just FP.


Would you mind explaining what complexities you see for vectors?  As per
my
> direct email, the set of vectors which can practically be made atomic may
> be smaller than we'd like, but the existing atomic semantics seem to
map
> cleanly.  What am I missing?
>

I'm also concerned about:

   - Alignment is the big one, I think we'll want to discuss having
   entirely atomic vectors as well as vectors whose elements are atomic only.
   - Having vectors of pointers without fully supporting atomic pointer.
   - Vectors of unusual sizes or integer types being atomic, and how they
   get legalized. e.g. <3 x i32> or <256 x i1>.
   - Once we add vector, should we consider adding other composite types in
   general, including structs? C++ allows this, but has substantial issues
   w.r.t. padding.
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20151214/5971cffb/attachment.html>

On 12/14/2015 12:46 PM, JF Bastien wrote:
> On Mon, Dec 14, 2015 at 11:46 AM, Philip Reames 
> <listmail at philipreames.com <mailto:listmail at
philipreames.com>> wrote:
>
>
>
>     On 12/12/2015 01:44 PM, JF Bastien wrote:
>>     On Sat, Dec 12, 2015 at 1:24 AM, Philip Reames
>>     <listmail at philipreames.com <mailto:listmail at
philipreames.com>> wrote:
>>
>>         Patch posted for review: http://reviews.llvm.org/D15471
>>
>>
>>     Looking at the patch, I think we should do FP only for now as
>>     vectors have extra complexities which IMO warrant more discussion.
>     I'm fine with splitting the patch up to make progress. I'll
post a
>     split patch shortly.
>
>
> Thanks. FWIW I think we'll want to do vectors, but I think it's 
> trickier than just FP.
Agreed.  :)
>
>
>     Would you mind explaining what complexities you see for vectors? 
>     As per my direct email, the set of vectors which can practically
>     be made atomic may be smaller than we'd like, but the existing
>     atomic semantics seem to map cleanly.  What am I missing?
>
>
>
> I'm also concerned about:
>
>   * Alignment is the big one, I think we'll want to discuss having
>     entirely atomic vectors as well as vectors whose elements are
>     atomic only.
>
Not sure how the second part relates to the first part.  I agree that we 
probably want a notion of element-wise atomicity for vector (and 
possibly struct/array) types, but I think that should come later.  
Specifically, I think we'd need an alternate spelling for an 
element-wise for on atomic, so I see no harm in having the support for 
fully atomic vectors added now.
>   * Having vectors of pointers without fully supporting atomic pointer.
>
We support atomic pointer operations in loads and stores.  Again, what 
we support in load/store is currently separate from what we support in 
atomicrmw/cmpxchg.  We should unify the later with the former, but 
that's a separate issue.
>
>   * Vectors of unusual sizes or integer types being atomic, and how
>     they get legalized. e.g. <3 x i32> or <256 x i1>.
>
Well, the former isn't legal.  It isn't a power of two size.  I'm
not
suggesting we relax that requirement.

If it has a power of two store size, then it should get the equivalent 
handling to an iX of the same size.   I don't see the issue here.  How 
is the second example any different from an atomic i256?

Actually, this brings up a related issue.  We seem to have no 
documentation in the lang ref about how vector types are represented in 
memory.  The actual implementation appears to assumed packed bits, but 
the docs don't say.  Depending on what the semantics here are, my 
assumptions in the last two paragraphs may not be valid.
>   * Once we add vector, should we consider adding other composite
>     types in general, including structs? C++ allows this, but has
>     substantial issues w.r.t. padding.
>
I'd say possibly, but FCAs are poorly supported in the IR in general.  I 
am not willing to block changes for vectors on changes for FCAs.  This 
has never been our policy in the past and should not become so now.

Philip

p.s. Thanks for asking clarifying questions.  Getting this all hammered 
out is definitely a good idea.  I just want to make sure we close the 
loop quickly so that this doesn't get stalled.


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

>
> Would you mind explaining what complexities you see for vectors?  As per
>> my direct email, the set of vectors which can practically be made
atomic
>> may be smaller than we'd like, but the existing atomic semantics
seem to
>> map cleanly.  What am I missing?
>>
>
>
> I'm also concerned about:
>
>    - Alignment is the big one, I think we'll want to discuss having
>    entirely atomic vectors as well as vectors whose elements are atomic
only.
>
> Not sure how the second part relates to the first part.  I agree that we
> probably want a notion of element-wise atomicity for vector (and possibly
> struct/array) types, but I think that should come later.  Specifically, I
> think we'd need an alternate spelling for an element-wise for on
atomic, so
> I see no harm in having the support for fully atomic vectors added now.
>
>
>    - Having vectors of pointers without fully supporting atomic pointer.
>
> We support atomic pointer operations in loads and stores.  Again, what we
> support in load/store is currently separate from what we support in
> atomicrmw/cmpxchg.  We should unify the later with the former, but
that's a
> separate issue.
>
Ha, I didn't realize we did, I though it had to go through intptr casts
just like FP does but r162146 added it back in 2012. The documentation and
verifier look slightly wrong, here's a proposed fix
<http://reviews.llvm.org/D15512>.

>
>    - Vectors of unusual sizes or integer types being atomic, and how they
>    get legalized. e.g. <3 x i32> or <256 x i1>.
>
> Well, the former isn't legal.  It isn't a power of two size. 
I'm not
> suggesting we relax that requirement.
>
Indeed, I'd like to make sure we don't and have some pretty explicit
testing for it.


If it has a power of two store size, then it should get the
equivalent
> handling to an iX of the same size.   I don't see the issue here.  How
is
> the second example any different from an atomic i256?
>
i1 isn't storable as-is :-)


Actually, this brings up a related issue.  We seem to have no
documentation
> in the lang ref about how vector types are represented in memory.  The
> actual implementation appears to assumed packed bits, but the docs
don't
> say.  Depending on what the semantics here are, my assumptions in the last
> two paragraphs may not be valid.
>
Indeed!


>    - Once we add vector, should we consider adding other composite types
>    in general, including structs? C++ allows this, but has substantial
issues
>    w.r.t. padding.
>
> I'd say possibly, but FCAs are poorly supported in the IR in general. 
I
> am not willing to block changes for vectors on changes for FCAs.  This has
> never been our policy in the past and should not become so now.
>
Oh yeah I don't think we should block it. FWIW I expect that some of these
will generate calls to the runtime's global atomic lock shard, which is
horrible.


p.s. Thanks for asking clarifying questions.  Getting this all hammered
out
> is definitely a good idea.  I just want to make sure we close the loop
> quickly so that this doesn't get stalled.
>
You mean, more stalled that the holidays will already stall it? ;-)
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20151214/be86c6a0/attachment.html>