From 6b96623dcb0cdb092eccf27b4d0fff25ed28f4b5 Mon Sep 17 00:00:00 2001 From: Dominik Montada Date: Thu, 12 Mar 2020 09:03:08 +0100 Subject: [PATCH] [GlobalISel] fix crash in narrowScalarExtract if DstRegs only has one register Summary: When narrowing a scalar G_EXTRACT where the destination lines up perfectly with a single result of the emitted G_UNMERGE_VALUES a COPY should be emitted instead of unconditionally trying to emit a G_MERGE_VALUES. Reviewers: arsenm, dsanders Reviewed By: arsenm Subscribers: wdng, rovka, hiraditya, volkan, llvm-commits Tags: #llvm Differential Revision: https://reviews.llvm.org/D75743 --- .../CodeGen/GlobalISel/LegalizerHelper.cpp | 6 ++- .../GlobalISel/LegalizerHelperTest.cpp | 39 +++++++++++++++++++ 2 files changed, 43 insertions(+), 2 deletions(-) diff --git a/llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp b/llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp index 6384cc9dc44f..d0a51ed03f00 100644 --- a/llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp +++ b/llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp @@ -3703,10 +3703,12 @@ LegalizerHelper::narrowScalarExtract(MachineInstr &MI, unsigned TypeIdx, } Register DstReg = MI.getOperand(0).getReg(); - if(MRI.getType(DstReg).isVector()) + if (MRI.getType(DstReg).isVector()) MIRBuilder.buildBuildVector(DstReg, DstRegs); - else + else if (DstRegs.size() > 1) MIRBuilder.buildMerge(DstReg, DstRegs); + else + MIRBuilder.buildCopy(DstReg, DstRegs[0]); MI.eraseFromParent(); return Legalized; } diff --git a/llvm/unittests/CodeGen/GlobalISel/LegalizerHelperTest.cpp b/llvm/unittests/CodeGen/GlobalISel/LegalizerHelperTest.cpp index 130db7289dd6..d359ab203a78 100644 --- a/llvm/unittests/CodeGen/GlobalISel/LegalizerHelperTest.cpp +++ b/llvm/unittests/CodeGen/GlobalISel/LegalizerHelperTest.cpp @@ -2302,4 +2302,43 @@ TEST_F(GISelMITest, LibcallFNearbyInt) { // Check EXPECT_TRUE(CheckMachineFunction(*MF, CheckStr)) << *MF; } + +TEST_F(GISelMITest, NarrowScalarExtract) { + setUp(); + if (!TM) + return; + + // Declare your legalization info + DefineLegalizerInfo(A, { + getActionDefinitionsBuilder(G_UNMERGE_VALUES).legalFor({{s32, s64}}); + getActionDefinitionsBuilder(G_EXTRACT).legalFor({{s16, s32}}); + }); + + LLT S16{LLT::scalar(16)}; + LLT S32{LLT::scalar(32)}; + + auto MIBExtractS32 = B.buildExtract(S32, Copies[1], 32); + auto MIBExtractS16 = B.buildExtract(S16, Copies[1], 0); + + AInfo Info(MF->getSubtarget()); + DummyGISelObserver Observer; + LegalizerHelper Helper(*MF, Info, Observer, B); + + EXPECT_EQ(LegalizerHelper::LegalizeResult::Legalized, + Helper.narrowScalar(*MIBExtractS32, 1, S32)); + + EXPECT_EQ(LegalizerHelper::LegalizeResult::Legalized, + Helper.narrowScalar(*MIBExtractS16, 1, S32)); + + const auto *CheckStr = R"( + CHECK: [[UV:%[0-9]+]]:_(s32), [[UV1:%[0-9]+]]:_(s32) = G_UNMERGE_VALUES + CHECK: [[COPY:%[0-9]+]]:_(s32) = COPY [[UV1]] + CHECK: [[UV3:%[0-9]+]]:_(s32), [[UV4:%[0-9]+]]:_(s32) = G_UNMERGE_VALUES + CHECK: [[EXTR:%[0-9]+]]:_(s16) = G_EXTRACT [[UV3]]:_(s32), 0 + CHECK: [[COPY:%[0-9]+]]:_(s16) = COPY [[EXTR]] + )"; + + // Check + EXPECT_TRUE(CheckMachineFunction(*MF, CheckStr)) << *MF; +} } // namespace