<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=utf-8">
<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:Wingdings;
panose-1:5 0 0 0 0 0 0 0 0 0;}
@font-face
{font-family:Wingdings;
panose-1:5 0 0 0 0 0 0 0 0 0;}
@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:0cm;
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.MsoPlainText, li.MsoPlainText, div.MsoPlainText
{mso-style-priority:99;
mso-style-link:"Plain Text Char";
margin:0cm;
margin-bottom:.0001pt;
font-size:11.0pt;
font-family:"Calibri","sans-serif";}
p.MsoAcetate, li.MsoAcetate, div.MsoAcetate
{mso-style-priority:99;
mso-style-link:"Balloon Text Char";
margin:0cm;
margin-bottom:.0001pt;
font-size:8.0pt;
font-family:"Tahoma","sans-serif";}
p.MsoListParagraph, li.MsoListParagraph, div.MsoListParagraph
{mso-style-priority:34;
margin-top:0cm;
margin-right:0cm;
margin-bottom:0cm;
margin-left:36.0pt;
margin-bottom:.0001pt;
font-size:12.0pt;
font-family:"Times New Roman","serif";}
span.PlainTextChar
{mso-style-name:"Plain Text Char";
mso-style-priority:99;
mso-style-link:"Plain Text";
font-family:"Calibri","sans-serif";}
span.EmailStyle20
{mso-style-type:personal;
font-family:"Calibri","sans-serif";
color:windowtext;}
span.EmailStyle21
{mso-style-type:personal-reply;
font-family:"Calibri","sans-serif";
color:windowtext;}
span.BalloonTextChar
{mso-style-name:"Balloon Text Char";
mso-style-priority:99;
mso-style-link:"Balloon Text";
font-family:"Tahoma","sans-serif";}
.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;}
/* List Definitions */
@list l0
{mso-list-id:554121468;
mso-list-type:hybrid;
mso-list-template-ids:1727579286 134807567 134807555 134807557 134807553 134807555 134807557 134807553 134807555 134807557;}
@list l0:level1
{mso-level-tab-stop:none;
mso-level-number-position:left;
text-indent:-18.0pt;}
@list l0:level2
{mso-level-number-format:bullet;
mso-level-text:o;
mso-level-tab-stop:none;
mso-level-number-position:left;
text-indent:-18.0pt;
font-family:"Courier New";}
@list l0:level3
{mso-level-number-format:bullet;
mso-level-text:;
mso-level-tab-stop:none;
mso-level-number-position:left;
text-indent:-18.0pt;
font-family:Wingdings;}
@list l0:level4
{mso-level-number-format:bullet;
mso-level-text:;
mso-level-tab-stop:none;
mso-level-number-position:left;
text-indent:-18.0pt;
font-family:Symbol;}
@list l0:level5
{mso-level-number-format:bullet;
mso-level-text:o;
mso-level-tab-stop:none;
mso-level-number-position:left;
text-indent:-18.0pt;
font-family:"Courier New";}
@list l0:level6
{mso-level-number-format:bullet;
mso-level-text:;
mso-level-tab-stop:none;
mso-level-number-position:left;
text-indent:-18.0pt;
font-family:Wingdings;}
@list l0:level7
{mso-level-number-format:bullet;
mso-level-text:;
mso-level-tab-stop:none;
mso-level-number-position:left;
text-indent:-18.0pt;
font-family:Symbol;}
@list l0:level8
{mso-level-number-format:bullet;
mso-level-text:o;
mso-level-tab-stop:none;
mso-level-number-position:left;
text-indent:-18.0pt;
font-family:"Courier New";}
@list l0:level9
{mso-level-number-format:bullet;
mso-level-text:;
mso-level-tab-stop:none;
mso-level-number-position:left;
text-indent:-18.0pt;
font-family:Wingdings;}
ol
{margin-bottom:0cm;}
ul
{margin-bottom:0cm;}
--></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-GB" link="blue" vlink="purple">
<div class="WordSection1">
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif"">> Reed, please don't re-commit until you understand and make the requested change that the LGTM was conditional on. I'll add an explanation to the review.<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif""><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif"">I just went to explain on the review, but it seems you did actually make the change all the way back in r207790 (committed 1<sup>st</sup> of May). It shows up in Diff 2 and
3 of D3659. It seems I confused you by asking you to make a change you had already made, sorry. r210414 as committed looks good to me (apart from the commit message), you are ok to re-commit with a proper commit message.<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif""><o:p> </o:p></span></p>
<div style="border:none;border-left:solid blue 1.5pt;padding:0cm 0cm 0cm 4.0pt">
<div>
<div style="border:none;border-top:solid #B5C4DF 1.0pt;padding:3.0pt 0cm 0cm 0cm">
<p class="MsoNormal"><b><span lang="EN-US" style="font-size:10.0pt;font-family:"Tahoma","sans-serif"">From:</span></b><span lang="EN-US" style="font-size:10.0pt;font-family:"Tahoma","sans-serif""> Daniel Sanders
<br>
<b>Sent:</b> 08 June 2014 16:02<br>
<b>To:</b> 'James Molloy'; Alp Toker; Reed Kotler; llvm-commits@cs.uiuc.edu<br>
<b>Subject:</b> RE: [llvm] r210414 - Do materialize for floating point<o:p></o:p></span></p>
</div>
</div>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif"">Hi All,<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif""><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif"">Reed, please don't re-commit until you understand and make the requested change that the LGTM was conditional on. I'll add an explanation to the review.<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif""><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif"">I think Alp did the right thing by reverting the commit. He had serious concerns about the patch, the review, and whether the process had been followed. It's important to
note that the commit message is at least as important than the content. People should be able to read the commit message and form opinions on what the commit does and why. One important misconception I need to clear up here is that there's no such thing as
an 'authorized llvm Mips maintainer'. The developer policy only distinguishes between those who maintain/contributed the relevant code and those who don't/didn't. In this case, the question of maintainer/contributor is irrelevant since everybody has the same
rights to 'make obvious changes' such as reverting an 'obviously bad commit'.<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif""><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif"">I forgot to make the LGTM conditional on the commit message correction too, sorry. On the subject of the requested change that the LGTM was predicated on: I have a lot of
sympathy for the issues the 8 hour timezone difference causes but they shouldn't be used as an excuse to take shortcuts. I agree that understanding the request comes first, and the delay wouldn't have hurt.<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif""><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif"">Marking the review as accepted in Phabricator based on a conditional LGTM is a compromise caused by the timezone difference and a Phabricator update. It used to be the case
that 'arc commit' would allow committing unaccepted revisions and would close it. The commit message didn't report a reviewer in this case. One of the Phabricator updates changed this behaviour so that while you could commit an unaccepted revision, the revision
will not move to the closed state and stays open. The workarounds to this is either:<o:p></o:p></span></p>
<p class="MsoListParagraph" style="text-indent:-18.0pt;mso-list:l0 level1 lfo2"><![if !supportLists]><span style="font-size:11.0pt;font-family:"Calibri","sans-serif""><span style="mso-list:Ignore">1.<span style="font:7.0pt "Times New Roman"">
</span></span></span><![endif]><span style="font-size:11.0pt;font-family:"Calibri","sans-serif"">the author accepts his own patch in the web interface (resulting in the commit message reporting the author as the reviewer)<o:p></o:p></span></p>
<p class="MsoListParagraph" style="text-indent:-18.0pt;mso-list:l0 level1 lfo2"><![if !supportLists]><span style="font-size:11.0pt;font-family:"Calibri","sans-serif""><span style="mso-list:Ignore">2.<span style="font:7.0pt "Times New Roman"">
</span></span></span><![endif]><span style="font-size:11.0pt;font-family:"Calibri","sans-serif"">someone manually closes it<o:p></o:p></span></p>
<p class="MsoListParagraph" style="text-indent:-18.0pt;mso-list:l0 level1 lfo2"><![if !supportLists]><span style="font-size:11.0pt;font-family:"Calibri","sans-serif""><span style="mso-list:Ignore">3.<span style="font:7.0pt "Times New Roman"">
</span></span></span><![endif]><span style="font-size:11.0pt;font-family:"Calibri","sans-serif"">the reviewer accepts the revision in anticipation of the condition being met<o:p></o:p></span></p>
<p class="MsoListParagraph" style="text-indent:-18.0pt;mso-list:l0 level1 lfo2"><![if !supportLists]><span style="font-size:11.0pt;font-family:"Calibri","sans-serif""><span style="mso-list:Ignore">4.<span style="font:7.0pt "Times New Roman"">
</span></span></span><![endif]><span style="font-size:11.0pt;font-family:"Calibri","sans-serif"">wait a day for the reviewer to mark the new revision accepted<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif"">The third one seems to be the least bad in practice since everybody forgets to manually close it and waiting 24 hours for someone to simply click a button seems excessive.
It would be great if the old behaviour could return.<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif""><o:p> </o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif"">One other misconception that I need to clear up is that our pre-commit reviews in Phabricator are actually intended for public review. It is intended to create public-accessible
record that a review happened along with the content of that review. We then commit under the 'you can commit to code you maintain without pre-commit review' rule (rule 3 of
<a href="http://llvm.org/docs/DeveloperPolicy.html#obtaining-commit-access">http://llvm.org/docs/DeveloperPolicy.html#obtaining-commit-access</a>). As far as the LLVM community is concerned, it is unreviewed at this point but it is expected that the availability
of the pre-commit review makes most post-commit reviews trivial. Previously, we were trying to do this kind of thing with our public-accessible-yet-internal bugzilla but even though that system created a review that was publicly visible, the LLVM community
had no way of finding the review and therefore it effectively did not exist.<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif"">Alp, I believe this explains points 3 and 4 in your revert commit message. Do you agree?<o:p></o:p></span></p>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif""><o:p> </o:p></span></p>
<div style="border:none;border-left:solid blue 1.5pt;padding:0cm 0cm 0cm 4.0pt">
<div>
<div style="border:none;border-top:solid #B5C4DF 1.0pt;padding:3.0pt 0cm 0cm 0cm">
<p class="MsoNormal"><b><span lang="EN-US" style="font-size:10.0pt;font-family:"Tahoma","sans-serif"">From:</span></b><span lang="EN-US" style="font-size:10.0pt;font-family:"Tahoma","sans-serif""> James Molloy [mailto:mankeyrabbit@gmail.com]
<br>
<b>Sent:</b> 08 June 2014 11:57<br>
<b>To:</b> Alp Toker; Reed Kotler; Daniel Sanders; llvm-commits@cs.uiuc.edu<br>
<b>Subject:</b> RE: [llvm] r210414 - Do materialize for floating point<o:p></o:p></span></p>
</div>
</div>
<p class="MsoNormal"><o:p> </o:p></p>
<div>
<div>
<p class="MsoNormal"><span style="font-size:11.0pt;font-family:"Calibri","sans-serif"">Hi,<br>
<br>
To weigh in my two penniworth, the commit message was undescriptive and just seemed a disjointed set of sentences.
<br>
<br>
Please remember that even though you are code owner, this is an open source project and it must remain accessible to everyone. Commit messages like that just make it obvious that work is going on behind closed doors.<br>
<br>
If you're reviewing patches externally, it must be visible to others. Particularly, it looks in this case as if the code reviewer has had legitimate concerns (that I share) and you went and committed anyway. This utterly defeats the point of code review. If
you do this, you should certainly not mark the patch as signed off because it hasn't been!<br>
<br>
What we care about is that a due (and not particularly onerous!) process has been followed. The specifics of the patch in question (whether I understand fast-isel or not, for example ) is completely besides the point.<br>
<br>
Cheers,<br>
<br>
James<o:p></o:p></span></p>
</div>
</div>
<div>
<div class="MsoNormal" align="center" style="text-align:center">
<hr size="2" width="100%" align="center">
</div>
<p class="MsoNormal" style="margin-bottom:12.0pt"><b><span style="font-size:11.0pt;font-family:"Calibri","sans-serif"">From:
</span></b><span style="font-size:11.0pt;font-family:"Calibri","sans-serif""><a href="mailto:alp@nuanti.com">Alp Toker</a></span><br>
<b><span style="font-size:11.0pt;font-family:"Calibri","sans-serif"">Sent: </span>
</b><span style="font-size:11.0pt;font-family:"Calibri","sans-serif"">08/06/2014 11:27</span><br>
<b><span style="font-size:11.0pt;font-family:"Calibri","sans-serif"">To: </span></b><span style="font-size:11.0pt;font-family:"Calibri","sans-serif""><a href="mailto:reed.kotler@imgtec.com">Reed Kotler</a>;
<a href="mailto:Daniel.Sanders@imgtec.com">Daniel Sanders</a>; <a href="mailto:llvm-commits@cs.uiuc.edu">
llvm-commits@cs.uiuc.edu</a></span><br>
<b><span style="font-size:11.0pt;font-family:"Calibri","sans-serif"">Subject: </span>
</b><span style="font-size:11.0pt;font-family:"Calibri","sans-serif"">Re: [llvm] r210414 - Do materialize for floating point</span><o:p></o:p></p>
</div>
<p class="MsoNormal"><br>
On 08/06/2014 13:12, Reed Kotler wrote:<br>
> This is a very trivial change to "Do materialize for floating point".<br>
<br>
Hi Reed,<br>
<br>
That's great. LGTM with that description!<br>
<br>
Please use the code owner system responsibly in future to keep it <br>
viable. It shouldn't be used to cover up situations like:<br>
<br>
"I did not understand the comment about using dyn_cast instead of <br>
isa. I will commit as is and make the update after."<br>
<br>
Alp.<br>
<br>
<br>
<br>
<br>
><br>
> If you understand what fast-isel does, that is all you need to say.<br>
> Each port has a materialize function for floating point literals.<br>
> That is the summary of the check in. It implements materialize for <br>
> floating point.<br>
><br>
> I'm not used to using Phabricator yet but checked in a whole code <br>
> generator for Mips16<br>
> on my own and passed all of test suite without any problems.<br>
><br>
> I have a lot of changes backed up right now and don't have the perfect <br>
> work flow<br>
> with Daniel yet to deal with that and all these deltas and keep <br>
> phabricator straight<br>
> at check in time. I'm doing the best I can there but it's not a <br>
> requirement and most people<br>
> are not even using phabricator at this time.<br>
><br>
> Phabricator is being included just to make it easier for people to see <br>
> the checking<br>
> without having to just look at the diffs.<br>
><br>
> The commit is fine and it has very good test case.<br>
><br>
> Reed<br>
><br>
> On 06/08/2014 02:40 AM, Alp Toker wrote:<br>
>><br>
>> On 08/06/2014 11:42, Reed Kotler wrote:<br>
>>> The summary area in the putback comment was messed up but the patch <br>
>>> is fine.<br>
>>> I'm not very good with phabricator yet.<br>
>><br>
>> Reed, I understand that your commit log was messed up but that's not <br>
>> the actual problem.<br>
>><br>
>> I've reverted the change in r210424 with a full explanation and will <br>
>> follow up shortly to the list.<br>
>><br>
>> Alp.<br>
>><br>
>><br>
>>><br>
>>> I don't understand your concerns.<br>
>>><br>
>>> You should look at the other fast-isel implementations, i.e. ppc, <br>
>>> arm and x86.<br>
>>><br>
>>> This one is almost identical.<br>
>>><br>
>>> All the implementations are all essentially the same.<br>
>>><br>
>>> You are reading too much into the comment "creates lot of core <br>
>>> infrastructure".<br>
>>> It's not "core infrastructure" for llvm in general.<br>
>>><br>
>>> This comment actually applied to an earlier commit. I have a lot of <br>
>>> patches backed up for<br>
>>> review and phabricator was seeing earlier comments and I should have <br>
>>> deleted them<br>
>>> but forgot to.<br>
>>><br>
>>> The only thing this patch does is to add the additional <br>
>>> functionality for materializing<br>
>>> floats.<br>
>>><br>
>>> Reed<br>
>>><br>
>>> On 06/08/2014 12:11 AM, Alp Toker wrote:<br>
>>>> Hi Daniel,<br>
>>>><br>
>>>> I'm concerned about the quality of your review...<br>
>>>><br>
>>>><br>
>>>> On 08/06/2014 06:30, Reed Kotler wrote:<br>
>>>>> Author: rkotler<br>
>>>>> Date: Sat Jun 7 22:30:32 2014<br>
>>>>> New Revision: 210414<br>
>>>>><br>
>>>>> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=210414&view=rev">http://llvm.org/viewvc/llvm-project?rev=210414&view=rev</a><br>
>>>>> Log:<br>
>>>>> Do materialize for floating point<br>
>>>>><br>
>>>>> Summary:<br>
>>>>> start to do simple constants<br>
>>>>><br>
>>>>> finish simplestore<br>
>>>>><br>
>>>>> add test case<br>
>>>>><br>
>>>>> format<br>
>>>>><br>
>>>>> Merge branch 'master' into 1756_8<br>
>>>>><br>
>>>>> Add basic functionality for assignment of ints. This creates a lot <br>
>>>>> of core infrastructure in which to add, with little effort, quite <br>
>>>>> a bit more to mips fast-isel<br>
>>>><br>
>>>> A patch that "creates lot of core infrastructure" should have been <br>
>>>> reviewed on-list.<br>
>>>><br>
>>>>><br>
>>>>> Merge branch 'master' into 1756_8<br>
>>>>><br>
>>>>> Add basic functionality for assignment of ints. This creates a lot <br>
>>>>> of core infrastructure in which to add, with little effort, quite <br>
>>>>> a bit more to mips fast-isel<br>
>>>>><br>
>>>>> in progress<br>
>>>>><br>
>>>>> finish integer materialize<br>
>>>>><br>
>>>>> test cases<br>
>>>>><br>
>>>>> test cases<br>
>>>>><br>
>>>>> in progress<br>
>>>>><br>
>>>>> Finish up fast-isel materialize for ints.<br>
>>>>><br>
>>>>> Finish materialize for ints<br>
>>>>><br>
>>>>> test cases<br>
>>>>><br>
>>>>> simplestorei.ll<br>
>>>>><br>
>>>>> Merge branch 'master' into 1756_8<br>
>>>>><br>
>>>>> fix fp constants for fast-isel<br>
>>>>><br>
>>>>> Merge branch '1758_1' of dmz-portal.mips.com:llvm into 1758_1<br>
>>>>><br>
>>>>> in progress<br>
>>>>><br>
>>>>> lastest for fp materialization<br>
>>>>><br>
>>>>> clean up<br>
>>>>><br>
>>>>> Merge branch 'master' into 1758_1<br>
>>>>><br>
>>>>> formatting<br>
>>>>><br>
>>>>> add test case<br>
>>>>><br>
>>>>> finish test case<br>
>>>>><br>
>>>>> Merge branch 'master' into 1758_2<br>
>>>>><br>
>>>>> Test Plan:<br>
>>>>> simplestore.ll<br>
>>>>><br>
>>>>> simplestore.ll<br>
>>>>><br>
>>>>> Reviewers: dsanders<br>
>>>>><br>
>>>>> Reviewed By: dsanders<br>
>>>><br>
>>>> ?<br>
>>>><br>
>>>>><br>
>>>>> Differential Revision: <a href="http://reviews.llvm.org/D3659">http://reviews.llvm.org/D3659</a><br>
>>>><br>
>>>> If I'm reading correctly, the quality issues were identified and <br>
>>>> just ignored to get a final LGTM and commit.<br>
>>>><br>
>>>> What's going on here?<br>
>>>><br>
>>>> Alp.<br>
>>>><br>
>>>><br>
>>>>><br>
>>>>> Added:<br>
>>>>> llvm/trunk/test/CodeGen/Mips/Fast-ISel/simplestorefp1.ll<br>
>>>>> Modified:<br>
>>>>> llvm/trunk/lib/Target/Mips/MipsFastISel.cpp<br>
>>>>><br>
>>>>> Modified: llvm/trunk/lib/Target/Mips/MipsFastISel.cpp<br>
>>>>> URL: <br>
>>>>> <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/Mips/MipsFastISel.cpp?rev=210414&r1=210413&r2=210414&view=diff">
http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/Mips/MipsFastISel.cpp?rev=210414&r1=210413&r2=210414&view=diff</a><br>
>>>>> ==============================================================================
<br>
>>>>><br>
>>>>> --- llvm/trunk/lib/Target/Mips/MipsFastISel.cpp (original)<br>
>>>>> +++ llvm/trunk/lib/Target/Mips/MipsFastISel.cpp Sat Jun 7 <br>
>>>>> 22:30:32 2014<br>
>>>>> @@ -167,9 +167,14 @@ bool MipsFastISel::EmitStore(MVT VT, uns<br>
>>>>> //<br>
>>>>> // more cases will be handled here in following patches.<br>
>>>>> //<br>
>>>>> - if (VT != MVT::i32)<br>
>>>>> + if (VT == MVT::i32)<br>
>>>>> + EmitInstStore(Mips::SW, SrcReg, Addr.Base.Reg, Addr.Offset);<br>
>>>>> + else if (VT == MVT::f32)<br>
>>>>> + EmitInstStore(Mips::SWC1, SrcReg, Addr.Base.Reg, Addr.Offset);<br>
>>>>> + else if (VT == MVT::f64)<br>
>>>>> + EmitInstStore(Mips::SDC1, SrcReg, Addr.Base.Reg, Addr.Offset);<br>
>>>>> + else<br>
>>>>> return false;<br>
>>>>> - EmitInstStore(Mips::SW, SrcReg, Addr.Base.Reg, Addr.Offset);<br>
>>>>> return true;<br>
>>>>> }<br>
>>>>> @@ -229,6 +234,22 @@ bool MipsFastISel::TargetSelectInstructi<br>
>>>>> }<br>
>>>>> unsigned MipsFastISel::MaterializeFP(const ConstantFP *CFP, <br>
>>>>> MVT VT) {<br>
>>>>> + int64_t Imm = CFP->getValueAPF().bitcastToAPInt().getZExtValue();<br>
>>>>> + if (VT == MVT::f32) {<br>
>>>>> + const TargetRegisterClass *RC = &Mips::FGR32RegClass;<br>
>>>>> + unsigned DestReg = createResultReg(RC);<br>
>>>>> + unsigned TempReg = Materialize32BitInt(Imm, <br>
>>>>> &Mips::GPR32RegClass);<br>
>>>>> + EmitInst(Mips::MTC1, DestReg).addReg(TempReg);<br>
>>>>> + return DestReg;<br>
>>>>> + } else if (VT == MVT::f64) {<br>
>>>>> + const TargetRegisterClass *RC = &Mips::AFGR64RegClass;<br>
>>>>> + unsigned DestReg = createResultReg(RC);<br>
>>>>> + unsigned TempReg1 = Materialize32BitInt(Imm >> 32, <br>
>>>>> &Mips::GPR32RegClass);<br>
>>>>> + unsigned TempReg2 =<br>
>>>>> + Materialize32BitInt(Imm & 0xFFFFFFFF, &Mips::GPR32RegClass);<br>
>>>>> + EmitInst(Mips::BuildPairF64, <br>
>>>>> DestReg).addReg(TempReg2).addReg(TempReg1);<br>
>>>>> + return DestReg;<br>
>>>>> + }<br>
>>>>> return 0;<br>
>>>>> }<br>
>>>>><br>
>>>>> Added: llvm/trunk/test/CodeGen/Mips/Fast-ISel/simplestorefp1.ll<br>
>>>>> URL: <br>
>>>>> <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/Mips/Fast-ISel/simplestorefp1.ll?rev=210414&view=auto">
http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/Mips/Fast-ISel/simplestorefp1.ll?rev=210414&view=auto</a><br>
>>>>> ==============================================================================
<br>
>>>>><br>
>>>>> --- llvm/trunk/test/CodeGen/Mips/Fast-ISel/simplestorefp1.ll (added)<br>
>>>>> +++ llvm/trunk/test/CodeGen/Mips/Fast-ISel/simplestorefp1.ll Sat <br>
>>>>> Jun 7 22:30:32 2014<br>
>>>>> @@ -0,0 +1,39 @@<br>
>>>>> +; RUN: llc -march=mipsel -relocation-model=pic -O0 <br>
>>>>> -mips-fast-isel -fast-isel-abort -mcpu=mips32r2 \<br>
>>>>> +; RUN: < %s | FileCheck %s<br>
>>>>> +<br>
>>>>> +@f = common global float 0.000000e+00, align 4<br>
>>>>> +@de = common global double 0.000000e+00, align 8<br>
>>>>> +<br>
>>>>> +; Function Attrs: nounwind<br>
>>>>> +define void @f1() #0 {<br>
>>>>> +entry:<br>
>>>>> + store float 0x3FFA76C8C0000000, float* @f, align 4<br>
>>>>> + ret void<br>
>>>>> +; CHECK: .ent f1<br>
>>>>> +; CHECK: lui $[[REG1:[0-9]+]], 16339<br>
>>>>> +; CHECK: ori $[[REG2:[0-9]+]], $[[REG1]], 46662<br>
>>>>> +; CHECK: mtc1 $[[REG2]], $f[[REG3:[0-9]+]]<br>
>>>>> +; CHECK: lw $[[REG4:[0-9]+]], %got(f)(${{[0-9]+}})<br>
>>>>> +; CHECK: swc1 $f[[REG3]], 0($[[REG4]])<br>
>>>>> +; CHECK: .end f1<br>
>>>>> +<br>
>>>>> +}<br>
>>>>> +<br>
>>>>> +; Function Attrs: nounwind<br>
>>>>> +define void @d1() #0 {<br>
>>>>> +entry:<br>
>>>>> + store double 1.234567e+00, double* @de, align 8<br>
>>>>> +; CHECK: .ent d1<br>
>>>>> +; CHECK: lui $[[REG1a:[0-9]+]], 16371<br>
>>>>> +; CHECK: ori $[[REG2a:[0-9]+]], $[[REG1a]], 49353<br>
>>>>> +; CHECK: lui $[[REG1b:[0-9]+]], 21403<br>
>>>>> +; CHECK: ori $[[REG2b:[0-9]+]], $[[REG1b]], 34951<br>
>>>>> +; CHECK: mtc1 $[[REG2b]], $f[[REG3b:[0-9]+]]<br>
>>>>> +; CHECK: mtc1 $[[REG2a]], $f[[REG3a:[0-9]+]]<br>
>>>>> +; CHECK: sdc1 $f[[REG3b]], 0(${{[0-9]+}})<br>
>>>>> +; CHECK: .end d1<br>
>>>>> + ret void<br>
>>>>> +}<br>
>>>>> +<br>
>>>>> +attributes #0 = { nounwind "less-precise-fpmad"="false" <br>
>>>>> "no-frame-pointer-elim"="true" "no-frame-pointer-elim-non-leaf" <br>
>>>>> "no-infs-fp-math"="false" "no-nans-fp-math"="false" <br>
>>>>> "stack-protector-buffer-size"="8" "unsafe-fp-math"="false" <br>
>>>>> "use-soft-float"="false" }<br>
>>>>> +<br>
>>>>><br>
>>>>><br>
>>>>> _______________________________________________<br>
>>>>> llvm-commits mailing list<br>
>>>>> <a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
>>>>> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><br>
>>>><br>
>>><br>
>><br>
><br>
<br>
-- <br>
<a href="http://www.nuanti.com">http://www.nuanti.com</a><br>
the browser experts<br>
<br>
_______________________________________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@cs.uiuc.edu">llvm-commits@cs.uiuc.edu</a><br>
<a href="http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits">http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits</a><o:p></o:p></p>
</div>
</div>
</div>
</body>
</html>