<html xmlns:v="urn:schemas-microsoft-com:vml" 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 14 (filtered medium)">
<!--[if !mso]><style>v\:* {behavior:url(#default#VML);}
o\:* {behavior:url(#default#VML);}
w\:* {behavior:url(#default#VML);}
.shape {behavior:url(#default#VML);}
</style><![endif]--><style><!--
/* Font Definitions */
@font-face
{font-family:Calibri;
panose-1:2 15 5 2 2 2 4 3 2 4;}
@font-face
{font-family:Tahoma;
panose-1:2 11 6 4 3 5 4 4 2 4;}
/* Style Definitions */
p.MsoNormal, li.MsoNormal, div.MsoNormal
{margin:0in;
margin-bottom:.0001pt;
font-size:12.0pt;
font-family:"Times New Roman","serif";}
a:link, span.MsoHyperlink
{mso-style-priority:99;
color:blue;
text-decoration:underline;}
a:visited, span.MsoHyperlinkFollowed
{mso-style-priority:99;
color:purple;
text-decoration:underline;}
p.MsoAcetate, li.MsoAcetate, div.MsoAcetate
{mso-style-priority:99;
mso-style-link:"Balloon Text Char";
margin:0in;
margin-bottom:.0001pt;
font-size:8.0pt;
font-family:"Tahoma","sans-serif";}
p.msochpdefault, li.msochpdefault, div.msochpdefault
{mso-style-name:msochpdefault;
mso-margin-top-alt:auto;
margin-right:0in;
mso-margin-bottom-alt:auto;
margin-left:0in;
font-size:10.0pt;
font-family:"Times New Roman","serif";}
span.emailstyle17
{mso-style-name:emailstyle17;
font-family:"Calibri","sans-serif";
color:#1F497D;}
span.BalloonTextChar
{mso-style-name:"Balloon Text Char";
mso-style-priority:99;
mso-style-link:"Balloon Text";
font-family:"Tahoma","sans-serif";}
span.EmailStyle21
{mso-style-type:personal-reply;
font-family:"Calibri","sans-serif";
color:#1F497D;}
.MsoChpDefault
{mso-style-type:export-only;
font-size:10.0pt;}
@page WordSection1
{size:8.5in 11.0in;
margin:1.0in 1.0in 1.0in 1.0in;}
div.WordSection1
{page:WordSection1;}
--></style><!--[if gte mso 9]><xml>
<o:shapedefaults v:ext="edit" spidmax="1026" />
</xml><![endif]--><!--[if gte mso 9]><xml>
<o:shapelayout v:ext="edit">
<o:idmap v:ext="edit" data="1" />
</o:shapelayout></xml><![endif]-->
</head>
<body lang="EN-US" link="blue" vlink="purple">
<div class="WordSection1">
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D">Hi Petar,<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D"><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D">In the case of ELF::R_X86_64_PC32, FinalAddress refers to the address that the relocation target will have when the generated code is executed, so calculating
it based on Section.LoadAddress is correct.<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D"><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D">Unless I have completely misunderstood your patch, the value you are calculating is the address of the stub and it should be possible to calculate that entirely
based on Value and Addend. If it isn’t possible, I’d suspect that the relocation was created incorrectly.<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D"><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D">The Value parameter comes from one of two places. For relocations created using addRelocationForSection, it will always be the load address of the section
specified when the relocation was added. (This should always be the case for your stub handlers.) For relocations created using addRelocationForSymbol, it will be the load address of the symbol.<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D"><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D">Since you are redefining the meaning of R_MIPS_26 you should add very explicit comments saying so and explaining why.<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D"><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D">I think you should be able to trigger the stub-already-defined code path by modifying one of the remote tests to call some function from multiple places.<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D"><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D">-Andy<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D"><o:p> </o:p></span></p>
<div>
<div style="border:none;border-top:solid #B5C4DF 1.0pt;padding:3.0pt 0in 0in 0in">
<p class="MsoNormal"><b><span style="font-size:10.0pt;font-family:"Tahoma","sans-serif"">From:</span></b><span style="font-size:10.0pt;font-family:"Tahoma","sans-serif""> Petar Jovanovic [mailto:Petar.Jovanovic@imgtec.com]
<br>
<b>Sent:</b> Monday, November 18, 2013 3:40 PM<br>
<b>To:</b> Kaylor, Andrew; llvm-commits@cs.uiuc.edu<br>
<b>Cc:</b> NAKAMURA Takumi<br>
<b>Subject:</b> RE: [PATCH] [mips] Resolve relocation for the stubs in MCJIT when load address is known<o:p></o:p></span></p>
</div>
</div>
<p class="MsoNormal"><o:p> </o:p></p>
<div>
<div>
<p class="MsoNormal"><span style="color:black">Hi Andy,<o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="color:black"><o:p> </o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="color:black">thanks for the response.<o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="color:black">Relocation R_MIPS_26 in this particular case in runtime linker is used to hold information where the stub is (the relocation itself and addend are used here differently than in regular linker for the same relocation
- this is offtopic, but just to say I am aware of that if anyone notices). The original relocation does not get called/resolved later (after processRelocationRef), so the idea was to emit the relocation, so it can be resolved once the final address is known.<o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="color:black"><o:p> </o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="color:black">In resolveMIPSRelocation(), Value will not always point to Section.LoadAddress at the function entry, thus an option is to use LoadAddress, this is similar how FinalAddress is calculated for ELF::R_X86_64_PC32.<o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="color:black"><o:p> </o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="color:black">Last, I am aware I skipped the part when stub is already generated. I can add in the patch similar change, e.g. <o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="color:black"><o:p> </o:p></span></p>
</div>
<div>
<p class="MsoNormal"><i><span style="font-size:10.0pt;font-family:"Courier New";color:black">@@ -1031,8 +1033,9 @@ void RuntimeDyldELF::processRelocationRef(unsigned SectionID,</span></i><span style="color:black"><o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><i><span style="font-size:10.0pt;font-family:"Courier New";color:black"> // Look up for existing stub.</span></i><span style="color:black"><o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><i><span style="font-size:10.0pt;font-family:"Courier New";color:black"> StubMap::const_iterator i = Stubs.find(Value);</span></i><span style="color:black"><o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><i><span style="font-size:10.0pt;font-family:"Courier New";color:black"> if (i != Stubs.end()) {</span></i><span style="color:black"><o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><i><span style="font-size:10.0pt;font-family:"Courier New";color:black">- resolveRelocation(Section, Offset,</span></i><span style="color:black"><o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><i><span style="font-size:10.0pt;font-family:"Courier New";color:black">- (uint64_t)Section.Address + i->second, RelType, 0);</span></i><span style="color:black"><o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><i><span style="font-size:10.0pt;font-family:"Courier New";color:black">+ RelocationEntry RE(SectionID, Offset, RelType, i->second);</span></i><span style="color:black"><o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><i><span style="font-size:10.0pt;font-family:"Courier New";color:black">+ addRelocationForSection(RE, Value.SectionID);</span></i><span style="color:black"><o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><i><span style="font-size:10.0pt;font-family:"Courier New";color:black">+</span></i><span style="color:black"><o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><i><span style="font-size:10.0pt;font-family:"Courier New";color:black"> DEBUG(dbgs() << " Stub function found\n");</span></i><span style="color:black"><o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><i><span style="font-size:10.0pt;font-family:"Courier New";color:black"> } else {</span></i><span style="color:black"><o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><i><span style="font-size:10.0pt;font-family:"Courier New";color:black"> // Create a new stub function.</span></i><span style="color:black"><o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="color:black"><o:p> </o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="color:black"><o:p> </o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="color:black">The reason why this was skipped in the first place was that none of the tests triggered that path, so I just left it untouched until I can find the case in which the change would be effective and correct.<o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="color:black"><o:p> </o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="color:black">Petar<o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="color:black"><o:p> </o:p></span></p>
</div>
<div>
<div class="MsoNormal" align="center" style="text-align:center"><span style="color:black">
<hr size="2" width="100%" align="center">
</span></div>
<div id="divRpF80968">
<p class="MsoNormal" style="margin-bottom:12.0pt"><b><span style="font-size:10.0pt;font-family:"Tahoma","sans-serif";color:black">From:</span></b><span style="font-size:10.0pt;font-family:"Tahoma","sans-serif";color:black"> Kaylor, Andrew [andrew.kaylor@intel.com]<br>
<b>Sent:</b> Monday, November 18, 2013 10:49 PM<br>
<b>To:</b> Petar Jovanovic; <a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
<b>Cc:</b> NAKAMURA Takumi<br>
<b>Subject:</b> RE: [PATCH] [mips] Resolve relocation for the stubs in MCJIT when load address is known</span><span style="color:black"><o:p></o:p></span></p>
</div>
<div>
<div>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D">I don’t think this patch is quite correct for two reasons.</span><span style="color:black"><o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D"> </span><span style="color:black"><o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D">First, in the call to resolveMIPSRelocation, the Section parameter refers to the section which contains the location to which the relocation is being applied
while the Value parameter contains the LoadAddress of the symbol to which the relocation refers (in this case the section containing the stub).</span><span style="color:black"><o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D"> </span><span style="color:black"><o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D">It looks like the RuntimeDyldELF handling of this relocation guarantees the section containing the symbol to which the relocation refers will always be the
same as the section which is the target of the relocation for the R_MIPS_26 relocation type, but in the general case (that is, across the ELF relocation handling) that isn’t true. The change you made to resolveMIPSRelocation will calculate the right value,
I think, but I don’t believe that change is even necessary. Unless I’m mistaken, the line “Value = Section.LoadAddress + Addend” will not change the value of Value, which already contained the Value input argument (which should have been the section load
address) + the Addend argument. If this is true, the new change will just introduce potentially misleading code.</span><span style="color:black"><o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D"> </span><span style="color:black"><o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D">The second issue is more serious. In RuntimeDyldELF::processRelocationRef, where you made your other change, there were two possible places where the old code
would have attempted to apply the relocation immediately. The place where you made your change handles the case where a new stub is created. However, a few lines above that there is a place where the code found that the needed stub already existed and it
attempts to immediately apply a relocation referencing that stub location. I believe you also need to update that code.</span><span style="color:black"><o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D"> </span><span style="color:black"><o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D">-Andy</span><span style="color:black"><o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D"> </span><span style="color:black"><o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif";color:#1F497D"> </span><span style="color:black"><o:p></o:p></span></p>
<div>
<div style="border:none;border-top:solid #B5C4DF 1.0pt;padding:3.0pt 0in 0in 0in">
<p class="MsoNormal"><b><span style="font-size:10.0pt;font-family:"Tahoma","sans-serif";color:black">From:</span></b><span style="font-size:10.0pt;font-family:"Tahoma","sans-serif";color:black"> Petar Jovanovic [<a href="mailto:Petar.Jovanovic@imgtec.com">mailto:Petar.Jovanovic@imgtec.com</a>]
<br>
<b>Sent:</b> Monday, November 18, 2013 12:46 PM<br>
<b>To:</b> <a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
<b>Cc:</b> Kaylor, Andrew; NAKAMURA Takumi<br>
<b>Subject:</b> [PATCH] [mips] Resolve relocation for the stubs in MCJIT when load address is known</span><span style="color:black"><o:p></o:p></span></p>
</div>
</div>
<p class="MsoNormal"><span style="color:black"> <o:p></o:p></span></p>
<div>
<div>
<p class="MsoNormal"><span style="color:black">Hi everyone,<o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="color:black"> <o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="color:black">here is a patch for several MIPS MCJIT failures.<o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="color:black"> <o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="color:black">Regards,<o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="color:black">Petar<o:p></o:p></span></p>
</div>
</div>
</div>
</div>
</div>
</div>
</div>
</body>
</html>