<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=UTF-8">
</head>
<body text="#000000" bgcolor="#FFFFFF">
<p><br>
</p>
<pre class="moz-signature" cols="72">-------------
Best regards,
Alexey Bataev</pre>
<div class="moz-cite-prefix">03.07.2019 11:33, Narayanaswamy, Ravi
пишет:<br>
</div>
<blockquote type="cite"
cite="mid:BCFA27423AEC4E41B6CBD7B6A8C696B6C2477FF0@ORSMSX105.amr.corp.intel.com">
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">To put it into target task allocated memory is completely different problem, which requires some redesign and full support of target tasks.
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">We do have a target task in libomp. We recently added the device number to this target task for async support.
Ravi</pre>
</blockquote>
<p>Yes, but it does not have any target specific functionality in
it, it just calls the regular task alloc function. As I
understand, there should be some additional functionality for
target tasks.<br>
</p>
<p>What I want to do in this patch, I just want to fix the bug in
the existing solution. I don't know how much we'll have to live
with this, I think it is good to fix the bugs in the existing
solution even if we plan to have a new one. <br>
</p>
<blockquote type="cite"
cite="mid:BCFA27423AEC4E41B6CBD7B6A8C696B6C2477FF0@ORSMSX105.amr.corp.intel.com">
<pre class="moz-quote-pre" wrap="">
-----Original Message-----
From: Alexey Bataev via Phabricator [<a class="moz-txt-link-freetext" href="mailto:reviews@reviews.llvm.org">mailto:reviews@reviews.llvm.org</a>]
Sent: Tuesday, July 2, 2019 5:48 PM
To: <a class="moz-txt-link-abbreviated" href="mailto:a.bataev@hotmail.com">a.bataev@hotmail.com</a>; <a class="moz-txt-link-abbreviated" href="mailto:george_rokos@hotmail.com">george_rokos@hotmail.com</a>; <a class="moz-txt-link-abbreviated" href="mailto:jdoerfert@anl.gov">jdoerfert@anl.gov</a>; <a class="moz-txt-link-abbreviated" href="mailto:alexe@us.ibm.com">alexe@us.ibm.com</a>
Cc: <a class="moz-txt-link-abbreviated" href="mailto:hfinkel@anl.gov">hfinkel@anl.gov</a>; Churbanov, Andrey <a class="moz-txt-link-rfc2396E" href="mailto:Andrey.Churbanov@intel.com"><Andrey.Churbanov@intel.com></a>; <a class="moz-txt-link-abbreviated" href="mailto:zhang.guansong@gmail.com">zhang.guansong@gmail.com</a>; <a class="moz-txt-link-abbreviated" href="mailto:jfbastien@apple.com">jfbastien@apple.com</a>; <a class="moz-txt-link-abbreviated" href="mailto:openmp-commits@lists.llvm.org">openmp-commits@lists.llvm.org</a>; <a class="moz-txt-link-abbreviated" href="mailto:kkwli0@gmail.com">kkwli0@gmail.com</a>; <a class="moz-txt-link-abbreviated" href="mailto:caomhin@us.ibm.com">caomhin@us.ibm.com</a>; <a class="moz-txt-link-abbreviated" href="mailto:deachempat@cray.com">deachempat@cray.com</a>; <a class="moz-txt-link-abbreviated" href="mailto:ron.lieberman@amd.com">ron.lieberman@amd.com</a>; <a class="moz-txt-link-abbreviated" href="mailto:jatin.bhateja@gmail.com">jatin.bhateja@gmail.com</a>; <a class="moz-txt-link-abbreviated" href="mailto:lildmh@gmail.com">lildmh@gmail.com</a>; <a class="moz-txt-link-abbreviated" href="mailto:Gregory.Rodgers@amd.com">Gregory.Rodgers@amd.com</a>; <a class="moz-txt-link-abbreviated" href="mailto:acjacob@us.ibm.com">acjacob@us.ibm.com</a>; Narayanaswamy, Ravi <a class="moz-txt-link-rfc2396E" href="mailto:ravi.narayanaswamy@intel.com"><ravi.narayanaswamy@intel.com></a>; <a class="moz-txt-link-abbreviated" href="mailto:jlebar@google.com">jlebar@google.com</a>
Subject: [PATCH] D64080: [OPENMP]Make __kmpc_push_tripcount thread safe.
ABataev added a comment.
In D64080#1567533 <a class="moz-txt-link-rfc2396E" href="https://reviews.llvm.org/D64080#1567533"><https://reviews.llvm.org/D64080#1567533></a>, @hfinkel wrote:
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">In D64080#1567348 <a class="moz-txt-link-rfc2396E" href="https://reviews.llvm.org/D64080#1567348"><https://reviews.llvm.org/D64080#1567348></a>, @ABataev wrote:
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">In D64080#1567327 <a class="moz-txt-link-rfc2396E" href="https://reviews.llvm.org/D64080#1567327"><https://reviews.llvm.org/D64080#1567327></a>, @grokos wrote:
</pre>
<blockquote type="cite">
<pre class="moz-quote-pre" wrap="">A long time ago we had identified the problem with the loop trip count and if I recall correctly the proposed solution was to store the trip count per OpenMP task. The fix would be implemented in libomp because libomptarget has no notion of tasks.
@AlexEichenberger Can you confirm whether or not this is the case? I may confuse this with something else...
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
The patch does exactly what you said: stores tripcount per task, but in the libomptarget, not libomp. When it will be implemented in libomp, we could remove this code. But we need fully functional implementation now, not in the future.
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
If this should really be fixed in libomp, then we should fix it there. At the very least, there should be a FIXME about removing the workaround once the libomp fix is in place. @AndreyChurbanov, any thoughts on this?
</pre>
</blockquote>
<pre class="moz-quote-pre" wrap="">
Hal, currently target tripcount has nothing to do with libomp. It is stored as part of the device in libomptarget. So, I think, to fix this problem in libomptarget is fine.
To put it into target task allocated memory is completely different problem, which requires some redesign and full support of target tasks. Plus, just like I said, it is not true that libomptarget is thread agnostic. Actually, it is not, since it uses `__kmpc_omp_taskwait(NULL, 0)` for dependendent target regions. Moreover, it is not thread safe too, because uses not real thread ID, but hardcoded `0` thread ID. It may lead to problems even in a single-threaded environment if the caller thread is not a master thread.
Repository:
rOMP OpenMP
CHANGES SINCE LAST ACTION
<a class="moz-txt-link-freetext" href="https://reviews.llvm.org/D64080/new/">https://reviews.llvm.org/D64080/new/</a>
<a class="moz-txt-link-freetext" href="https://reviews.llvm.org/D64080">https://reviews.llvm.org/D64080</a>
</pre>
</blockquote>
</body>
</html>