<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 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;}
@font-face
{font-family:Menlo;
panose-1:0 0 0 0 0 0 0 0 0 0;}
/* Style Definitions */
p.MsoNormal, li.MsoNormal, div.MsoNormal
{margin:0in;
font-size:11.0pt;
font-family:"Calibri",sans-serif;}
a:link, span.MsoHyperlink
{mso-style-priority:99;
color:blue;
text-decoration:underline;}
span.EmailStyle18
{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: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" style="word-wrap:break-word;-webkit-nbsp-mode: space;line-break:after-white-space">
<div class="WordSection1">
<p class="MsoNormal">Hi Quentin,<o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal">Thanks for the investigation and sample patch! I will apply that in this week’s 10% time, so I should have a new patch up on Friday.<o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<p class="MsoNormal">Thanks,<o:p></o:p></p>
<p class="MsoNormal">--paulr<o:p></o:p></p>
<p class="MsoNormal"><o:p> </o:p></p>
<div style="border:none;border-left:solid blue 1.5pt;padding:0in 0in 0in 4.0pt">
<div>
<div style="border:none;border-top:solid #E1E1E1 1.0pt;padding:3.0pt 0in 0in 0in">
<p class="MsoNormal"><b>From:</b> Quentin Colombet <qcolombet@apple.com> <br>
<b>Sent:</b> Friday, January 29, 2021 5:24 PM<br>
<b>To:</b> Robinson, Paul <paul.robinson@sony.com><br>
<b>Cc:</b> llvm-de >> LLVM Developers' List <llvm-dev@lists.llvm.org>; Amara Emerson <aemerson@apple.com><br>
<b>Subject:</b> Re: [llvm-dev] Q. about GlobalISel and order of instructions<o:p></o:p></p>
</div>
</div>
<p class="MsoNormal"><o:p> </o:p></p>
<div>
<p class="MsoNormal">Hi Paul,<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">This is a bug with the test.<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">The various legalizerhelper methods expect that the MIRBuilder is pointing to the input MI when doing their expansions.<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal">This is set automatically when you use the legalizeInstrStep API, but if you use another method directly, looks like this is the client responsibility to set it.<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">The patch below fixes your problem (you’ll have to do it for all of them).<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">It would be nice to have some kind of asserts here.<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">@Amara what do you think? <o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<div>
<p class="MsoNormal"><b><span style="font-size:8.5pt;font-family:"Menlo",serif">diff --git a/llvm/unittests/CodeGen/GlobalISel/LegalizerHelperTest.cpp b/llvm/unittests/CodeGen/GlobalISel/LegalizerHelperTest.cpp</span></b><span style="font-size:8.5pt;font-family:"Menlo",serif"><o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><b><span style="font-size:8.5pt;font-family:"Menlo",serif">index 5d31b6e876b8..dd7927acd6f5 100644</span></b><span style="font-size:8.5pt;font-family:"Menlo",serif"><o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><b><span style="font-size:8.5pt;font-family:"Menlo",serif">--- a/llvm/unittests/CodeGen/GlobalISel/LegalizerHelperTest.cpp</span></b><span style="font-size:8.5pt;font-family:"Menlo",serif"><o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><b><span style="font-size:8.5pt;font-family:"Menlo",serif">+++ b/llvm/unittests/CodeGen/GlobalISel/LegalizerHelperTest.cpp</span></b><span style="font-size:8.5pt;font-family:"Menlo",serif"><o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:8.5pt;font-family:"Menlo",serif;color:#2EAEBB">@@ -965,6 +965,9 @@</span><span style="font-size:8.5pt;font-family:"Menlo",serif"> TEST_F(AArch64GISelMITest, MoreElementsAnd) {<o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:8.5pt;font-family:"Menlo",serif"> }<o:p></o:p></span></p>
</div>
<p class="MsoNormal" style="font-stretch: normal;min-height: 13px"><span style="font-size:8.5pt;font-family:"Menlo",serif"> <o:p></o:p></span></p>
<div>
<p class="MsoNormal"><span style="font-size:8.5pt;font-family:"Menlo",serif"> TEST_F(AArch64GISelMITest, FewerElementsPhi) {<o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:8.5pt;font-family:"Menlo",serif;color:#2FB41D">+<o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:8.5pt;font-family:"Menlo",serif;color:#2FB41D">+ setUp();<o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:8.5pt;font-family:"Menlo",serif;color:#2FB41D">+<o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:8.5pt;font-family:"Menlo",serif"> if (!TM)<o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:8.5pt;font-family:"Menlo",serif"> return;<o:p></o:p></span></p>
</div>
<p class="MsoNormal" style="font-stretch: normal;min-height: 13px"><span style="font-size:8.5pt;font-family:"Menlo",serif"> <o:p></o:p></span></p>
<div>
<p class="MsoNormal"><span style="font-size:8.5pt;font-family:"Menlo",serif;color:#2EAEBB">@@ -1027,6 +1030,7 @@</span><span style="font-size:8.5pt;font-family:"Menlo",serif"> TEST_F(AArch64GISelMITest, FewerElementsPhi) {<o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:8.5pt;font-family:"Menlo",serif"> // Add some use instruction after the phis.<o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:8.5pt;font-family:"Menlo",serif"> B.buildAnd(PhiTy, Phi.getReg(0), Phi.getReg(0));<o:p></o:p></span></p>
</div>
<p class="MsoNormal" style="font-stretch: normal;min-height: 13px"><span style="font-size:8.5pt;font-family:"Menlo",serif"> <o:p></o:p></span></p>
<div>
<p class="MsoNormal"><span style="font-size:8.5pt;font-family:"Menlo",serif;color:#2FB41D">+ B.setInstr(*Phi);<o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:8.5pt;font-family:"Menlo",serif"> EXPECT_EQ(LegalizerHelper::LegalizeResult::Legalized,<o:p></o:p></span></p>
</div>
<div>
<p class="MsoNormal"><span style="font-size:8.5pt;font-family:"Menlo",serif"> Helper.fewerElementsVector(*Phi, 0, v2s32));<o:p></o:p></span></p>
</div>
</div>
<div>
<p class="MsoNormal"><br>
<br>
<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
<div>
<p class="MsoNormal">Cheers,<o:p></o:p></p>
</div>
<div>
<p class="MsoNormal">-Quentin<br>
<br>
<o:p></o:p></p>
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
<div>
<p class="MsoNormal">On Jan 29, 2021, at 12:00 PM, <a href="mailto:paul.robinson@sony.com">
paul.robinson@sony.com</a> wrote:<o:p></o:p></p>
</div>
<p class="MsoNormal"><o:p> </o:p></p>
<div>
<div>
<p class="MsoNormal"><br>
<br>
<br>
<o:p></o:p></p>
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
<p class="MsoNormal">-----Original Message-----<br>
From: Quentin Colombet <<a href="mailto:qcolombet@apple.com">qcolombet@apple.com</a>><br>
Sent: Friday, January 29, 2021 1:32 PM<br>
To: Robinson, Paul <<a href="mailto:paul.robinson@sony.com">paul.robinson@sony.com</a>><br>
Cc: <a href="mailto:llvm-dev@lists.llvm.org">llvm-dev@lists.llvm.org</a><br>
Subject: Re: [llvm-dev] Q. about GlobalISel and order of instructions<br>
<br>
Hi Paul,<br>
<br>
Thanks for the heads-up!<br>
<br>
<br>
<o:p></o:p></p>
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
<p class="MsoNormal">On Jan 28, 2021, at 8:12 PM, via llvm-dev <<a href="mailto:llvm-dev@lists.llvm.org">llvm-dev@lists.llvm.org</a>><o:p></o:p></p>
</blockquote>
<p class="MsoNormal">wrote:<br>
<br>
<o:p></o:p></p>
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
<p class="MsoNormal"><br>
Hello Global ISel fans,<br>
<br>
I am trying to patch up some unittests that accidentally weren't<br>
actually running<o:p></o:p></p>
</blockquote>
<p class="MsoNormal">(<a href="https://urldefense.com/v3/__https:/reviews.llvm.org/D95257__;!!JmoZiZGBv">https://urldefense.com/v3/__https://reviews.llvm.org/D95257__;!!JmoZiZGBv</a><br>
3RvKRSx!pxOXw4x2XmGszxqKSXAovQkJx-<br>
JyBTV988c1AXpEDJn9K0UNJwwjhVEcDTvbulL5Dw$ ). A number of<br>
<br>
<o:p></o:p></p>
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
<p class="MsoNormal">these look like all the right instructions are coming out, but<br>
they aren't "in the right order." That is, I see some tests<br>
expect something along the lines of<br>
<br>
%2:_(s16) = G_TRUNC<br>
%3:_(s10) = G_TRUNC %2:_(s16)<br>
%4:_(s10) = G_SEXT_INREG %3:_(s10)<br>
%5:_(s16) = G_SEXT %4:_(s10)<br>
<br>
That is, each instruction feeds nicely into the next one.<br>
But what I'm actually seeing is reordered:<br>
<br>
%5:_(s16) = G_SEXT %4:_(s10)<br>
%2:_(s16) = G_TRUNC<br>
%4:_(s10) = G_SEXT_INREG %3:_(s10)<br>
%3:_(s10) = G_TRUNC %2:_(s16)<br>
<br>
Now, I can certainly patch up the test to match the order I see,<br>
but... Is that actually correct? Naively it looks like we have<br>
uses preceding defs, which just looks weird.<o:p></o:p></p>
</blockquote>
<p class="MsoNormal"><br>
That’s definitely broken!<br>
<br>
<br>
<o:p></o:p></p>
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
<p class="MsoNormal"><br>
If the order I'm seeing is wrong (and it's just a standard dump<br>
of a MachineFunction) then can somebody fix it?<o:p></o:p></p>
</blockquote>
<p class="MsoNormal"><br>
Are these the tests you marked as FIXME in the PR you’ve linked?<o:p></o:p></p>
</blockquote>
<p class="MsoNormal"><br>
Thanks, Quentin! Some of them have other more functional differences,<br>
which are probably normal things where I can fix the test to match.<br>
The ones that have just the ordering problem look like this set:<br>
<br>
FewerElementsPhi<br>
WidenScalarBuildVector<br>
WidenSEXTINREG<br>
NarrowSEXTINREG<br>
<br>
Thanks!<br>
--paulr<br>
<br>
<br>
<o:p></o:p></p>
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
<p class="MsoNormal"><br>
Cheers,<br>
-Quentin<br>
<br>
<br>
<o:p></o:p></p>
<blockquote style="margin-top:5.0pt;margin-bottom:5.0pt">
<p class="MsoNormal">It will sure<br>
make a lot less churn in the unittest.<br>
<br>
Thanks,<br>
--paulr<br>
<br>
<br>
_______________________________________________<br>
LLVM Developers mailing list<br>
<a href="mailto:llvm-dev@lists.llvm.org">llvm-dev@lists.llvm.org</a><br>
https://urldefense.com/v3/__https://lists.llvm.org/cgi-<o:p></o:p></p>
</blockquote>
<p class="MsoNormal">bin/mailman/listinfo/llvm-<br>
dev__;!!JmoZiZGBv3RvKRSx!pxOXw4x2XmGszxqKSXAovQkJx-<br>
JyBTV988c1AXpEDJn9K0UNJwwjhVEcDTsB1HBaAQ$<o:p></o:p></p>
</blockquote>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
</div>
</blockquote>
</div>
<p class="MsoNormal"><o:p> </o:p></p>
</div>
</div>
</body>
</html>