From 75e0b91e597f97ee8f32f56984988ab55fbfa067 Mon Sep 17 00:00:00 2001 From: Tim Northover Date: Mon, 6 Mar 2017 18:23:04 +0000 Subject: [PATCH] GlobalISel: refactor legalization of G_INSERT. Now that G_INSERT instructions can only insert one register, this code was overly general. In another direction it didn't handle registers that crossed split boundaries properly, which needed to be fixed. llvm-svn: 297042 --- .../llvm/CodeGen/GlobalISel/LegalizerHelper.h | 9 --- .../CodeGen/GlobalISel/LegalizerHelper.cpp | 60 +++++++------------ .../AArch64/GlobalISel/arm64-fallback.ll | 4 +- .../AArch64/GlobalISel/legalize-inserts.mir | 22 +++++++ 4 files changed, 47 insertions(+), 48 deletions(-) diff --git a/llvm/include/llvm/CodeGen/GlobalISel/LegalizerHelper.h b/llvm/include/llvm/CodeGen/GlobalISel/LegalizerHelper.h index 1b7d86b2dd4d..56c444ca46be 100644 --- a/llvm/include/llvm/CodeGen/GlobalISel/LegalizerHelper.h +++ b/llvm/include/llvm/CodeGen/GlobalISel/LegalizerHelper.h @@ -95,15 +95,6 @@ private: void extractParts(unsigned Reg, LLT Ty, int NumParts, SmallVectorImpl &Ops); - /// Set \p CurOp and \p EndOp to the range of G_INSERT operands that fall - /// inside the bit-range specified by \DstStart and \p DstEnd. Assumes \p - /// CurOp is initially pointing at one of the (Reg, Offset) pairs in \p MI (or - /// at the end), which should be a G_INSERT instruction. - void findInsertionsForRange(int64_t DstStart, int64_t DstEnd, - MachineInstr::mop_iterator &CurOp, - MachineInstr::mop_iterator &EndOp, - MachineInstr &MI); - MachineIRBuilder MIRBuilder; MachineRegisterInfo &MRI; }; diff --git a/llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp b/llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp index 2d28a42c7979..a8bc694dc17e 100644 --- a/llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp +++ b/llvm/lib/CodeGen/GlobalISel/LegalizerHelper.cpp @@ -127,17 +127,6 @@ LegalizerHelper::libcall(MachineInstr &MI) { } } -void LegalizerHelper::findInsertionsForRange( - int64_t DstStart, int64_t DstEnd, MachineInstr::mop_iterator &CurOp, - MachineInstr::mop_iterator &EndOp, MachineInstr &MI) { - while (CurOp != MI.operands_end() && std::next(CurOp)->getImm() < DstStart) - CurOp += 2; - - EndOp = CurOp; - while (EndOp != MI.operands_end() && std::next(EndOp)->getImm() < DstEnd) - EndOp += 2; -} - LegalizerHelper::LegalizeResult LegalizerHelper::narrowScalar(MachineInstr &MI, unsigned TypeIdx, LLT NarrowTy) { @@ -181,7 +170,7 @@ LegalizerHelper::LegalizeResult LegalizerHelper::narrowScalar(MachineInstr &MI, if (TypeIdx != 0) return UnableToLegalize; - unsigned NarrowSize = NarrowTy.getSizeInBits(); + int64_t NarrowSize = NarrowTy.getSizeInBits(); int NumParts = MRI.getType(MI.getOperand(0).getReg()).getSizeInBits() / NarrowSize; @@ -189,44 +178,41 @@ LegalizerHelper::LegalizeResult LegalizerHelper::narrowScalar(MachineInstr &MI, SmallVector Indexes; extractParts(MI.getOperand(1).getReg(), NarrowTy, NumParts, SrcRegs); - MachineInstr::mop_iterator CurOp = MI.operands_begin() + 2, EndOp; + unsigned OpReg = MI.getOperand(2).getReg(); + int64_t OpStart = MI.getOperand(3).getImm(); + int64_t OpSize = MRI.getType(OpReg).getSizeInBits(); for (int i = 0; i < NumParts; ++i) { unsigned DstStart = i * NarrowSize; - unsigned DstReg = MRI.createGenericVirtualRegister(NarrowTy); - - findInsertionsForRange(DstStart, DstStart + NarrowSize, CurOp, EndOp, MI); - if (CurOp == EndOp) { + if (DstStart + NarrowSize <= OpStart || DstStart >= OpStart + OpSize) { // No part of the insert affects this subregister, forward the original. DstRegs.push_back(SrcRegs[i]); continue; - } else if (MRI.getType(CurOp->getReg()) == NarrowTy && - std::next(CurOp)->getImm() == DstStart) { + } else if (DstStart == OpStart && NarrowTy == MRI.getType(OpReg)) { // The entire subregister is defined by this insert, forward the new // value. - DstRegs.push_back(CurOp->getReg()); + DstRegs.push_back(OpReg); continue; } - auto MIB = MIRBuilder.buildInstr(TargetOpcode::G_INSERT) - .addDef(DstReg) - .addUse(SrcRegs[i]); - - for (; CurOp != EndOp; CurOp += 2) { - unsigned Reg = CurOp->getReg(); - uint64_t Offset = std::next(CurOp)->getImm() - DstStart; - - // Make sure we don't have a cross-register insert. - if (Offset + MRI.getType(Reg).getSizeInBits() > NarrowSize) { - // FIXME: we should handle this case, though it's unlikely to be - // common given ABI-related layout restrictions. - return UnableToLegalize; - } - - MIB.addUse(Reg); - MIB.addImm(Offset); + int64_t OpSegStart = DstStart - OpStart; + int64_t OpSegSize = + std::min(NarrowSize - OpSegStart, OpSegStart + OpSize); + unsigned OpSegReg = OpReg; + if (OpSegSize != OpSize) { + // A genuine extract is needed. + OpSegReg = MRI.createGenericVirtualRegister(LLT::scalar(OpSegSize)); + MIRBuilder.buildExtract(OpSegReg, std::max(OpSegStart, (int64_t)0), + OpReg); } + unsigned DstReg = MRI.createGenericVirtualRegister(NarrowTy); + MIRBuilder.buildInstr(TargetOpcode::G_INSERT) + .addDef(DstReg) + .addUse(SrcRegs[i]) + .addUse(OpSegReg) + .addImm(std::max((int64_t)0, -OpSegStart)); + DstRegs.push_back(DstReg); } diff --git a/llvm/test/CodeGen/AArch64/GlobalISel/arm64-fallback.ll b/llvm/test/CodeGen/AArch64/GlobalISel/arm64-fallback.ll index cb10f52a5004..02f6e3bd4022 100644 --- a/llvm/test/CodeGen/AArch64/GlobalISel/arm64-fallback.ll +++ b/llvm/test/CodeGen/AArch64/GlobalISel/arm64-fallback.ll @@ -73,7 +73,7 @@ define void @odd_type(i42* %addr) { ; RegBankSelect crashed when given invalid mappings, and AArch64's ; implementation produce valid-but-nonsense mappings for G_SEQUENCE. -; FALLBACK-WITH-REPORT-ERR: remark: :0:0: unable to map instruction: %vreg0(s128) = G_SEQUENCE %vreg1, 0, %vreg2, 64; GPR:%vreg1,%vreg2 (in function: sequence_mapping) +; FALLBACK-WITH-REPORT-ERR: remark: :0:0: unable to map instruction ; FALLBACK-WITH-REPORT-ERR: warning: Instruction selection used fallback path for sequence_mapping ; FALLBACK-WITH-REPORT-OUT-LABEL: sequence_mapping: define void @sequence_mapping([2 x i64] %in) { @@ -81,7 +81,7 @@ define void @sequence_mapping([2 x i64] %in) { } ; Legalizer was asserting when it enountered an unexpected default action. -; FALLBACK-WITH-REPORT-ERR: remark: :0:0: unable to legalize instruction: %vreg9(s128) = G_INSERT %vreg10, %vreg0, 32; (in function: legal_default) +; FALLBACK-WITH-REPORT-ERR: remark: :0:0: unable to map instruction ; FALLBACK-WITH-REPORT-ERR: warning: Instruction selection used fallback path for legal_default ; FALLBACK-WITH-REPORT-LABEL: legal_default: define void @legal_default([8 x i8] %in) { diff --git a/llvm/test/CodeGen/AArch64/GlobalISel/legalize-inserts.mir b/llvm/test/CodeGen/AArch64/GlobalISel/legalize-inserts.mir index d7e853e69b32..86729d648638 100644 --- a/llvm/test/CodeGen/AArch64/GlobalISel/legalize-inserts.mir +++ b/llvm/test/CodeGen/AArch64/GlobalISel/legalize-inserts.mir @@ -7,6 +7,7 @@ define void @test_inserts_2() { ret void } define void @test_inserts_3() { ret void } define void @test_inserts_4() { ret void } + define void @test_inserts_5() { ret void } ... --- @@ -98,3 +99,24 @@ body: | G_STORE %3(s8), %2(p0) :: (store 1) RET_ReallyLR ... + +--- +name: test_inserts_5 +body: | + bb.0: + liveins: %x0, %x1, %x2 + + + ; CHECK-LABEL: name: test_inserts_5 + ; CHECK: [[INS_LO:%[0-9]+]](s32) = G_EXTRACT %2(s64), 0 + ; CHECK: [[VAL_LO:%[0-9]+]](s64) = G_INSERT %0, [[INS_LO]](s32), 32 + ; CHECK: [[INS_HI:%[0-9]+]](s32) = G_EXTRACT %2(s64), 32 + ; CHECK: [[VAL_HI:%[0-9]+]](s64) = G_INSERT %1, [[INS_HI]](s32), 0 + ; CHECK: %4(s128) = G_MERGE_VALUES [[VAL_LO]](s64), [[VAL_HI]](s64) + %0:_(s64) = COPY %x0 + %1:_(s64) = COPY %x1 + %2:_(s64) = COPY %x2 + %3:_(s128) = G_MERGE_VALUES %0, %1 + %4:_(s128) = G_INSERT %3, %2, 32 + RET_ReallyLR +... -- GitLab