<html xmlns:o="urn:schemas-microsoft-com:office:office" xmlns:w="urn:schemas-microsoft-com:office:word" xmlns:m="http://schemas.microsoft.com/office/2004/12/omml" xmlns="http://www.w3.org/TR/REC-html40">
<head>
<meta http-equiv="Content-Type" content="text/html; charset=us-ascii">
<meta name="Generator" content="Microsoft Word 15 (filtered medium)">
<style><!--
/* Font Definitions */
@font-face
        {font-family:"Cambria Math";
        panose-1:2 4 5 3 5 4 6 3 2 4;}
@font-face
        {font-family:Calibri;
        panose-1:2 15 5 2 2 2 4 3 2 4;}
/* Style Definitions */
p.MsoNormal, li.MsoNormal, div.MsoNormal
        {margin:0cm;
        font-size:10.0pt;
        font-family:"Calibri",sans-serif;}
a:link, span.MsoHyperlink
        {mso-style-priority:99;
        color:blue;
        text-decoration:underline;}
span.EmailStyle19
        {mso-style-type:personal-reply;
        font-family:"Calibri",sans-serif;
        color:windowtext;}
.MsoChpDefault
        {mso-style-type:export-only;
        font-size:10.0pt;}
@page WordSection1
        {size:612.0pt 792.0pt;
        margin:72.0pt 72.0pt 72.0pt 72.0pt;}
div.WordSection1
        {page:WordSection1;}
--></style>
</head>
<body lang="en-DE" link="blue" vlink="purple" style="word-wrap:break-word">
<div class="WordSection1">
<p class="MsoNormal"><span lang="DE" style="font-size:11.0pt;mso-fareast-language:EN-US">Hi Aleksey,<o:p></o:p></span></p>
<p class="MsoNormal"><span lang="DE" style="font-size:11.0pt;mso-fareast-language:EN-US"><o:p> </o:p></span></p>
<p class="MsoNormal"><span lang="EN-US" style="font-size:11.0pt;mso-fareast-language:EN-US">  > Any other ideas?<o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-US" style="font-size:11.0pt;mso-fareast-language:EN-US"><o:p> </o:p></span></p>
<p class="MsoNormal"><span lang="EN-US" style="font-size:11.0pt;mso-fareast-language:EN-US">Maybe the metadata section of a compiled frame could be extended with a slot for<o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-US" style="font-size:11.0pt;mso-fareast-language:EN-US">the JavaThread pointer which is then used for spilling? It could then be fix<o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-US" style="font-size:11.0pt;mso-fareast-language:EN-US">when thawing frames.<o:p></o:p></span></p>
<p class="MsoNormal"><span lang="EN-US" style="font-size:11.0pt;mso-fareast-language:EN-US"><o:p> </o:p></span></p>
<p class="MsoNormal"><span lang="DE" style="font-size:11.0pt;mso-fareast-language:EN-US">Richard.<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;mso-fareast-language:EN-US"><o:p> </o:p></span></p>
<div style="border:none;border-top:solid #B5C4DF 1.0pt;padding:3.0pt 0cm 0cm 0cm">
<p class="MsoNormal" style="margin-bottom:12.0pt"><b><span style="font-size:12.0pt;color:black">From:
</span></b><span style="font-size:12.0pt;color:black">hotspot-compiler-dev <hotspot-compiler-dev-retn@openjdk.org> on behalf of Aleksey Shipilev <shade@redhat.com><br>
<b>Date: </b>Thursday, 24. November 2022 at 18:25<br>
<b>To: </b>hotspot compiler <hotspot-compiler-dev@openjdk.java.net><br>
<b>Subject: </b>C2, ThreadLocalNode, and Loom<o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal" style="margin-bottom:12.0pt"><span style="font-size:11.0pt">Hi,<br>
<br>
In Loom x86_32 port, I am following up on the remaining C2 bugs.<br>
<br>
I believe one bug can be summarized as follows. C2 models thread-locals with ThreadLocalNode (TLN).
<br>
TLN is effectively a constant node: its only input is root, and it hashes like a normal node. This
<br>
was logically sound for decades, because the code never switched the threads. Therefore, code is
<br>
free to treat thread address as constant. In Loom, this guarantee no longer holds. If we ever store
<br>
the "old" value of TLN node somewhere and reuse it past the potential yield-resume point, we end up
<br>
using the *wrong* thread.<br>
<br>
How is this not a problem we saw before? On most architectures, we have the dedicated thread
<br>
register, and TLN matches to it. That dedicated thread register holds the true current thread
<br>
already. AFAICS, most TLN uses go straight to various AddP-s, so we are reasonably safe that no
<br>
naked TLS addresses are stored, and the majority (all?) uses reference that thread register. (I am
<br>
not sure what protects us from accidentally "caching" thread register into adhoc one. It would make
<br>
little sense from performance/compiler standpoint, but I cannot yet see what theoretically prevents
<br>
it in C2 code.)<br>
<br>
On x86_32, however, TLN matches to full MacroAssembler::get_thread call, and there storing the
<br>
thread address into an adhoc register is a normal thing to do. Reusing that register over the
<br>
continuation switch points visibly breaks x86_32. This usually manifests like a heap corruption
<br>
because multiple threads stomp over foreign TLABs, or a failure in runtime GC code. Current failures
<br>
in Loom x86_32 port, for example, can be easily reproduced by adding a simple assert in any G1
<br>
runtime method that pulls the (wrong) thread (mis)loaded from C2 barrier:<br>
<br>
// G1 pre write barrier slowpath<br>
JRT_LEAF(void, G1BarrierSetRuntime::write_ref_field_pre_entry(oopDesc* orig, JavaThread* thread))<br>
   assert(thread == JavaThread::current(), "write_ref_field_pre_entry sanity");<br>
<br>
So, while this manifests on x86_32, I think this is symptom of a larger problem with assuming TLN
<br>
const-ness. At this point, I am trying to come up with solutions:<br>
<br>
1) Dodge the problem in x86_32 and then pretend all arches have dedicated thread registers<br>
<br>
Sacrifice one x86_32 register for thread address. This would likely to penalize performance a little
<br>
bit, because x86_32 does not have lots of general purpose registers to begin with. We can probably
<br>
try and go to FS/GS instead of carrying the address in the register; but I don't know how much work
<br>
would that entail.<br>
<br>
This feels like a cowardly way out, and it would still break any future arch that does not have
<br>
dedicated thread registers. And, it would break if we ever replace and ThreadLocalNode with the call
<br>
to Thread::current().<br>
<br>
2) Remodel ThreadLocalNode as non-constant<br>
<br>
What partially solves the problem: saying that ThreadLocalNode::hash() is NO_HASH. AFAICS, this
<br>
successfully prevents collapsing TLNs in the same compilation. This still does not solve the case
<br>
where a single TLN gets yanked to the earliest block and its value cached in the register.<br>
<br>
AFAIU, we only want to make sure that TLN is reloaded after the potential continuation yield, which
<br>
also serves as the point of return. Since continuation yields are modeled as calls, and calls
<br>
produce both control and memory, we might need to hook up TLN to either control or memory.<br>
<br>
I tried to hook up the current control to every TLN node [1]. It works with a few wrinkles, but the
<br>
patch shows there are ripple effects throughout C2 code, and it sometimes breaks the graph. Some
<br>
pattern matching code (for example AddP matching code in EA) also asserts, probably assuming that
<br>
TLNs have no inputs. I suspect other places might have implicit dependencies like these as well.
<br>
This would be the inevitable consequence for any patch that changes ThreadLocalNode inputs/outputs.<br>
<br>
3) Some other easy way out I am overlooking?<br>
<br>
Any other ideas?<br>
<br>
-- <br>
Thanks,<br>
-Aleksey<br>
<br>
[1] <a href="https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fcr.openjdk.java.net%2F~shade%2Floom%2Fx86_32%2Ftln-ctrl-1.patch&amp;data=05%7C01%7Crichard.reingruber%40sap.com%7Ccc312cd71fa0412a44b008dace40f0ff%7C42f7676cf455423c82f6dc2d99791af7%7C0%7C0%7C638049075574372652%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=FEe7WrKgW9Iu8okF7vwSih1DKfifasXPiV4ccV4ls1M%3D&amp;reserved=0">
https://eur03.safelinks.protection.outlook.com/?url=https%3A%2F%2Fcr.openjdk.java.net%2F~shade%2Floom%2Fx86_32%2Ftln-ctrl-1.patch&amp;data=05%7C01%7Crichard.reingruber%40sap.com%7Ccc312cd71fa0412a44b008dace40f0ff%7C42f7676cf455423c82f6dc2d99791af7%7C0%7C0%7C638049075574372652%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C3000%7C%7C%7C&amp;sdata=FEe7WrKgW9Iu8okF7vwSih1DKfifasXPiV4ccV4ls1M%3D&amp;reserved=0</a><o:p></o:p></span></p>
</div>
</div>
</body>
</html>