<html>
<head>
<meta http-equiv="content-type" content="text/html; charset=utf-8">
</head>
<body bgcolor="#FFFFFF" text="#000000">
<br>
<div class="moz-cite-prefix">On 12/24/2014 5:54 AM, Chandler Carruth
wrote:<br>
</div>
<blockquote class=" cite"
id="mid_CAGCO0Kh8hzc2CPcpcw1T_yJT3yEpXkRmmCL_3uyM__W9ykWhrg_mail_gmail_com"
cite="mid:CAGCO0Kh8hzc2CPcpcw1T-yJT3yEpXkRmmCL_3uyM--W9ykWhrg@mail.gmail.com"
type="cite">
<meta http-equiv="Content-Type" content="text/html; charset=utf-8">
<div dir="ltr">I've not had a good chance to look at the patches
in detail, but just to clarify one point:
<div><br>
</div>
<div>I don't really care whether we number things going up or
down from single threaded to "every thread". I just think it
makes sense to expose them in the in-memory IR interface as an
enum with a particular ordering so that code can use the
obvious sorts of tests for comparing two orderings and not
have to worry (overly much) about edge cases. This doesn't
really need to be reflected in the bitcode encoding though, so
I'm fine with whatever steps are needed to keep the bitcode
compatible and sane.</div>
</div>
</blockquote>
<br>
Right. The second version of my patches fixes the bitcode encoding.
But now I see another potential problem with future bitcode if we
require an ordering on the scopes. What happens when a backend later
introduces a new scope that goes into the middle of the order? If
they renumber the scopes to accomodate this, then existing bitcode
for that backend will no longer work. The bitcode reader/writer
cannot compensate for this since the values are backend-specific. If
we agree that this problem is real, then we cannot force an ordering
on the scope numbers.<br>
<br>
<blockquote class=" cite"
id="mid_CAGCO0Kh8hzc2CPcpcw1T_yJT3yEpXkRmmCL_3uyM__W9ykWhrg_mail_gmail_com"
cite="mid:CAGCO0Kh8hzc2CPcpcw1T-yJT3yEpXkRmmCL_3uyM--W9ykWhrg@mail.gmail.com"
type="cite">
<div dir="ltr">
<div>I also agree with having the text format use a symbolic
thing for both extremes. It doesn't seem super important, but
it seems nice.</div>
</div>
</blockquote>
<br>
So far, I have refrained from proposing a keyword for cross thread
scope in the text format, because (a) there never was one and (b) it
is not strictly needed since it is the default anyway. I am fine
either way, but we will first have to decide what the new keyword
should be. I find "allthreads" to be a decent counterpart for
"singlethread" ... "crossthread" is not good enough since
intermediate scopes have multiple threads too. <br>
<br>
<blockquote class=" cite"
id="mid_CAGCO0Kh8hzc2CPcpcw1T_yJT3yEpXkRmmCL_3uyM__W9ykWhrg_mail_gmail_com"
cite="mid:CAGCO0Kh8hzc2CPcpcw1T-yJT3yEpXkRmmCL_3uyM--W9ykWhrg@mail.gmail.com"
type="cite">
<div dir="ltr">
<div>Regarding the bitcode encoding, I would consider whether
one encoding is more space efficient than another. I don't
recall whether we default to zero or whether we use a varint
encoding in the bitcode here, but if we do, it would make
sense to optimize the encoding around cross thread being the
most common. I'm not really a bitcode expert, so I'd rather
defer to someone who has hacked on this part of LLVM more
recently there.</div>
</div>
</blockquote>
<br>
Indeed, the text format is defined around cross thread being the
most common, but strangely it was not encoded as zero, even in
bitcode. So the most common case turns out to be a one stored as a
uint32 in the bitcode. The new scopes fit into that existing space,
while the most common case changes from one to ~0U. Maintaining
forward compatibility for older bitcode would mean that we can't
optimize by changing the common case to zero.<br>
<br>
<blockquote class=" cite"
id="mid_CAGCO0Kh8hzc2CPcpcw1T_yJT3yEpXkRmmCL_3uyM__W9ykWhrg_mail_gmail_com"
cite="mid:CAGCO0Kh8hzc2CPcpcw1T-yJT3yEpXkRmmCL_3uyM--W9ykWhrg@mail.gmail.com"
type="cite">
<div dir="ltr">
<div>I can try to take a look at the higher level patches soon
though.</div>
</div>
</blockquote>
<br>
Great! I intend to clean up and submit the in-memory patches first.
These simply upgrade the representation from a single bit to an
unsigned integer, without affecting any "end points" of the
compiler.<br>
<br>
Sameer.<br>
<br>
<blockquote class=" cite"
id="mid_CAGCO0Kh8hzc2CPcpcw1T_yJT3yEpXkRmmCL_3uyM__W9ykWhrg_mail_gmail_com"
cite="mid:CAGCO0Kh8hzc2CPcpcw1T-yJT3yEpXkRmmCL_3uyM--W9ykWhrg@mail.gmail.com"
type="cite">
<div class="gmail_extra"><br>
<div class="gmail_quote">On Fri, Dec 12, 2014 at 10:25 AM,
Sahasrabuddhe, Sameer <span dir="ltr"><<a
moz-do-not-send="true"
href="mailto:sameer.sahasrabuddhe@amd.com" target="_blank">sameer.sahasrabuddhe@amd.com</a>></span>
wrote:<br>
<blockquote id="Cite_4746969" class="gmail_quote cite"
style="margin:0 0 0 .8ex;border-left:1px #ccc
solid;padding-left:1ex">
<div bgcolor="#FFFFFF" text="#000000"><span class=""> <br>
<div>On 12/11/2014 4:28 PM, Sahasrabuddhe, Sameer wrote:<br>
</div>
<blockquote class=" cite" id="Cite_3190924" type="cite">
<br>
Attached is a sequence of patches that changes the IR
to support more than two synchronization scopes. This
is still a work in progress, and these patches are
only meant to start a more detailed discussion on the
way forward.<br>
<br>
</blockquote>
<br>
</span> Ping! <br>
<br>
Found a simple way to preserve forward compatibility. See
below.<span class=""><br>
<br>
<blockquote class=" cite" id="Cite_4380467" type="cite">
One big issue is the absence of any backend that
actually makes use of intermediate synchronization
scopes. This work is meant to be just one part of the
ground work required for landing the much-anticipated
HSAIL backend. Also, more work might be needed for
emitting atomic instructions via Clang.<br>
<br>
The proposed syntax for synchronization scope is as
follows:<br>
<ol>
<li>Synchronization scopes are of arbitrary width,
but implemented as unsigned in the bitcode, just
like address spaces.</li>
<li>Cross-thread is default, but now encoded as 0.<br>
</li>
<li>Keyword 'singlethread' is unchanged, but now
encoded as the largest integer (which happens to
be ~0U in bitcode).<br>
</li>
<li>New syntax "synchscope(n)" for other scopes.</li>
<li>There is no keyword for cross-thread, but it can
be specified as "synchscope(0)".</li>
</ol>
<p>This change breaks forward compatibility for the
bitcode, since the meaning of the zero/one values
are now changed.<br>
</p>
<p><tt> enum SynchronizationScope {</tt><tt><br>
</tt><tt>- SingleThread = 0,</tt><tt><br>
</tt><tt>- CrossThread = 1</tt><tt><br>
</tt><tt>+ CrossThread = 0,</tt><tt><br>
</tt><tt>+ SingleThread = ~0U</tt><tt><br>
</tt><tt> };</tt><br>
</p>
<p>The change passes almost all lit tests including
one new test (see patch 0005). The failing tests are
specifically checking for forward compatibility:<br>
<br>
Failing Tests (3):<br>
LLVM :: Bitcode/cmpxchg-upgrade.ll<br>
LLVM :: Bitcode/memInstructions.3.2.ll<br>
LLVM :: Bitcode/weak-cmpxchg-upgrade.ll<br>
<br>
This breakage remains even if we reverse the order
of synchronization scopes. One simple way to
preserve compatibility is to retain 0 and 1 with
their current meanings, and specify that
intermediate scopes are represented in an ordered
way with numbers greater than one. But this is
pretty ugly to work with. Would appreciate inputs on
how to fix this!<br>
</p>
</blockquote>
<br>
</span> The issue here is purely in the bitcode, and we
need an encoding that can represent new intermediate
scopes while preserving the two known values of zero and
one. Note that the earlier zero is now ~0U in the
in-memory representation, and the earlier 1 is now zero.
This mapping can be easily accomplished with a simple
increment/decrement by one, ignoring overflow. So the
bitreader now subtracts a one when decoding the synch
scope, and bitwriter adds a one when encoding the synch
scope. The attached change number 0006 is meant to replace
changes 0003 and 0005 in the previous list, since the
assembly and the bitcode need to be updated simultaneously
for this to work.<br>
<br>
The new change passes all tests, including the ones
checking for forward compatibility.<span class="HOEnZb"><font
color="#888888"><br>
<br>
Sameer.<br>
<br>
</font></span></div>
</blockquote>
</div>
<br>
</div>
</blockquote>
<br>
<br>
</body>
</html>