[llvm-dev] Q. about GlobalISel and order of instructions

[via llvm-dev]

Hi Quentin,

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.

Thanks,
--paulr

From: Quentin Colombet <qcolombet at apple.com>
Sent: Friday, January 29, 2021 5:24 PM
To: Robinson, Paul <paul.robinson at sony.com>
Cc: llvm-de >> LLVM Developers' List <llvm-dev at lists.llvm.org>; Amara Emerson <aemerson at apple.com>
Subject: Re: [llvm-dev] Q. about GlobalISel and order of instructions

Hi Paul,

This is a bug with the test.

The various legalizerhelper methods expect that the MIRBuilder is pointing to the input MI when doing their expansions.
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.

The patch below fixes your problem (you’ll have to do it for all of them).

It would be nice to have some kind of asserts here.

@Amara what do you think?

diff --git a/llvm/unittests/CodeGen/GlobalISel/LegalizerHelperTest.cpp b/llvm/unittests/CodeGen/GlobalISel/LegalizerHelperTest.cpp
index 5d31b6e876b8..dd7927acd6f5 100644
--- a/llvm/unittests/CodeGen/GlobalISel/LegalizerHelperTest.cpp
+++ b/llvm/unittests/CodeGen/GlobalISel/LegalizerHelperTest.cpp
@@ -965,6 +965,9 @@ TEST_F(AArch64GISelMITest, MoreElementsAnd) {
 }

 TEST_F(AArch64GISelMITest, FewerElementsPhi) {
+
+  setUp();
+
   if (!TM)
     return;

@@ -1027,6 +1030,7 @@ TEST_F(AArch64GISelMITest, FewerElementsPhi) {
   // Add some use instruction after the phis.
   B.buildAnd(PhiTy, Phi.getReg(0), Phi.getReg(0));

+  B.setInstr(*Phi);
   EXPECT_EQ(LegalizerHelper::LegalizeResult::Legalized,
             Helper.fewerElementsVector(*Phi, 0, v2s32));



Cheers,
-Quentin

On Jan 29, 2021, at 12:00 PM, paul.robinson at sony.com<mailto:paul.robinson at sony.com> wrote:




-----Original Message-----
From: Quentin Colombet <qcolombet at apple.com<mailto:qcolombet at apple.com>>
Sent: Friday, January 29, 2021 1:32 PM
To: Robinson, Paul <paul.robinson at sony.com<mailto:paul.robinson at sony.com>>
Cc: llvm-dev at lists.llvm.org<mailto:llvm-dev at lists.llvm.org>
Subject: Re: [llvm-dev] Q. about GlobalISel and order of instructions

Hi Paul,

Thanks for the heads-up!


On Jan 28, 2021, at 8:12 PM, via llvm-dev <llvm-dev at lists.llvm.org<mailto:llvm-dev at lists.llvm.org>>
wrote:


Hello Global ISel fans,

I am trying to patch up some unittests that accidentally weren't
actually running
(https://urldefense.com/v3/__https://reviews.llvm.org/D95257__;!!JmoZiZGBv<https://urldefense.com/v3/__https:/reviews.llvm.org/D95257__;!!JmoZiZGBv>
3RvKRSx!pxOXw4x2XmGszxqKSXAovQkJx-
JyBTV988c1AXpEDJn9K0UNJwwjhVEcDTvbulL5Dw$ ).  A number of

these look like all the right instructions are coming out, but
they aren't "in the right order."  That is, I see some tests
expect something along the lines of

  %2:_(s16) = G_TRUNC
  %3:_(s10) = G_TRUNC %2:_(s16)
  %4:_(s10) = G_SEXT_INREG %3:_(s10)
  %5:_(s16) = G_SEXT %4:_(s10)

That is, each instruction feeds nicely into the next one.
But what I'm actually seeing is reordered:

  %5:_(s16) = G_SEXT %4:_(s10)
  %2:_(s16) = G_TRUNC
  %4:_(s10) = G_SEXT_INREG %3:_(s10)
  %3:_(s10) = G_TRUNC %2:_(s16)

Now, I can certainly patch up the test to match the order I see,
but...  Is that actually correct?  Naively it looks like we have
uses preceding defs, which just looks weird.

That’s definitely broken!



If the order I'm seeing is wrong (and it's just a standard dump
of a MachineFunction) then can somebody fix it?

Are these the tests you marked as FIXME in the PR you’ve linked?

Thanks, Quentin!  Some of them have other more functional differences,
which are probably normal things where I can fix the test to match.
The ones that have just the ordering problem look like this set:

FewerElementsPhi
WidenScalarBuildVector
WidenSEXTINREG
NarrowSEXTINREG

Thanks!
--paulr



Cheers,
-Quentin


It will sure
make a lot less churn in the unittest.

Thanks,
--paulr


_______________________________________________
LLVM Developers mailing list
llvm-dev at lists.llvm.org<mailto:llvm-dev at lists.llvm.org>
https://urldefense.com/v3/__https://lists.llvm.org/cgi-
bin/mailman/listinfo/llvm-
dev__;!!JmoZiZGBv3RvKRSx!pxOXw4x2XmGszxqKSXAovQkJx-
JyBTV988c1AXpEDJn9K0UNJwwjhVEcDTsB1HBaAQ$


-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20210201/6bd41ebb/attachment-0001.html>