From 7d0b44e1561d66406821d28a87af494405d1a975 Mon Sep 17 00:00:00 2001 From: Tim Northover Date: Fri, 23 Jun 2017 16:15:55 +0000 Subject: [PATCH] GlobalISel: remove G_SEQUENCE instruction. It was trying to do too many things. The basic lumping together of values for legalization purposes is now handled by G_MERGE_VALUES. More complex things involving gaps and odd sizes are handled by G_INSERT sequences. git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@306120 91177308-0d34-0410-b5e6-96231b3b80d8 --- include/llvm/Target/GenericOpcodes.td | 9 --- include/llvm/Target/TargetOpcodes.def | 2 - lib/CodeGen/GlobalISel/Legalizer.cpp | 70 ------------------- lib/CodeGen/GlobalISel/LegalizerInfo.cpp | 3 +- .../AArch64/AArch64RegisterBankInfo.cpp | 4 -- .../AArch64/GlobalISel/legalize-combines.mir | 50 +++---------- .../AArch64/GlobalISel/no-regclass.mir | 4 +- 7 files changed, 14 insertions(+), 128 deletions(-) diff --git a/include/llvm/Target/GenericOpcodes.td b/include/llvm/Target/GenericOpcodes.td index 3ddd3532d3b..9593d8bd7ed 100644 --- a/include/llvm/Target/GenericOpcodes.td +++ b/include/llvm/Target/GenericOpcodes.td @@ -465,15 +465,6 @@ def G_INSERT : Instruction { let hasSideEffects = 0; } -// Combine a sequence of generic vregs into a single larger value (starting at -// bit 0). Essentially a G_INSERT where $src is an IMPLICIT_DEF, but it's so -// important to legalization it probably deserves its own instruction. -def G_SEQUENCE : Instruction { - let OutOperandList = (outs type0:$dst); - let InOperandList = (ins variable_ops); - let hasSideEffects = 0; -} - def G_MERGE_VALUES : Instruction { let OutOperandList = (outs type0:$dst); let InOperandList = (ins variable_ops); diff --git a/include/llvm/Target/TargetOpcodes.def b/include/llvm/Target/TargetOpcodes.def index e2a67a9460d..836b11cf89c 100644 --- a/include/llvm/Target/TargetOpcodes.def +++ b/include/llvm/Target/TargetOpcodes.def @@ -241,8 +241,6 @@ HANDLE_TARGET_OPCODE(G_INSERT) /// Generic instruction to paste a variable number of components together into a /// larger register. -HANDLE_TARGET_OPCODE(G_SEQUENCE) - HANDLE_TARGET_OPCODE(G_MERGE_VALUES) /// Generic pointer to int conversion. diff --git a/lib/CodeGen/GlobalISel/Legalizer.cpp b/lib/CodeGen/GlobalISel/Legalizer.cpp index bfb02bd1d3c..b699156c568 100644 --- a/lib/CodeGen/GlobalISel/Legalizer.cpp +++ b/lib/CodeGen/GlobalISel/Legalizer.cpp @@ -50,70 +50,6 @@ void Legalizer::getAnalysisUsage(AnalysisUsage &AU) const { void Legalizer::init(MachineFunction &MF) { } -bool Legalizer::combineExtracts(MachineInstr &MI, MachineRegisterInfo &MRI, - const TargetInstrInfo &TII) { - bool Changed = false; - if (MI.getOpcode() != TargetOpcode::G_EXTRACT) - return Changed; - - unsigned NumDefs = (MI.getNumOperands() - 1) / 2; - unsigned SrcReg = MI.getOperand(NumDefs).getReg(); - MachineInstr &SeqI = *MRI.def_instr_begin(SrcReg); - if (SeqI.getOpcode() != TargetOpcode::G_SEQUENCE) - return Changed; - - unsigned NumSeqSrcs = (SeqI.getNumOperands() - 1) / 2; - bool AllDefsReplaced = true; - - // Try to match each register extracted with a corresponding insertion formed - // by the G_SEQUENCE. - for (unsigned Idx = 0, SeqIdx = 0; Idx < NumDefs; ++Idx) { - MachineOperand &ExtractMO = MI.getOperand(Idx); - assert(ExtractMO.isReg() && ExtractMO.isDef() && - "unexpected extract operand"); - - unsigned ExtractReg = ExtractMO.getReg(); - unsigned ExtractPos = MI.getOperand(NumDefs + Idx + 1).getImm(); - - while (SeqIdx < NumSeqSrcs && - SeqI.getOperand(2 * SeqIdx + 2).getImm() < ExtractPos) - ++SeqIdx; - - if (SeqIdx == NumSeqSrcs) { - AllDefsReplaced = false; - continue; - } - - unsigned OrigReg = SeqI.getOperand(2 * SeqIdx + 1).getReg(); - if (SeqI.getOperand(2 * SeqIdx + 2).getImm() != ExtractPos || - MRI.getType(OrigReg) != MRI.getType(ExtractReg)) { - AllDefsReplaced = false; - continue; - } - - assert(!TargetRegisterInfo::isPhysicalRegister(OrigReg) && - "unexpected physical register in G_SEQUENCE"); - - // Finally we can replace the uses. - MRI.replaceRegWith(ExtractReg, OrigReg); - } - - if (AllDefsReplaced) { - // If SeqI was the next instruction in the BB and we removed it, we'd break - // the outer iteration. - assert(std::next(MachineBasicBlock::iterator(MI)) != SeqI && - "G_SEQUENCE does not dominate G_EXTRACT"); - - MI.eraseFromParent(); - - if (MRI.use_empty(SrcReg)) - SeqI.eraseFromParent(); - Changed = true; - } - - return Changed; -} - bool Legalizer::combineMerges(MachineInstr &MI, MachineRegisterInfo &MRI, const TargetInstrInfo &TII, MachineIRBuilder &MIRBuilder) { @@ -271,12 +207,6 @@ bool Legalizer::runOnMachineFunction(MachineFunction &MF) { // Get the next Instruction before we try to legalize, because there's a // good chance MI will be deleted. NextMI = std::next(MI); - - // combineExtracts erases MI. - if (combineExtracts(*MI, MRI, TII)) { - Changed = true; - continue; - } Changed |= combineMerges(*MI, MRI, TII, Helper.MIRBuilder); } } diff --git a/lib/CodeGen/GlobalISel/LegalizerInfo.cpp b/lib/CodeGen/GlobalISel/LegalizerInfo.cpp index 4d459104229..595802f2228 100644 --- a/lib/CodeGen/GlobalISel/LegalizerInfo.cpp +++ b/lib/CodeGen/GlobalISel/LegalizerInfo.cpp @@ -75,8 +75,7 @@ LegalizerInfo::getAction(const InstrAspect &Aspect) const { // FIXME: the long-term plan calls for expansion in terms of load/store (if // they're not legal). - if (Aspect.Opcode == TargetOpcode::G_SEQUENCE || - Aspect.Opcode == TargetOpcode::G_EXTRACT || + if (Aspect.Opcode == TargetOpcode::G_EXTRACT || Aspect.Opcode == TargetOpcode::G_MERGE_VALUES || Aspect.Opcode == TargetOpcode::G_UNMERGE_VALUES) return std::make_pair(Legal, Aspect.Type); diff --git a/lib/Target/AArch64/AArch64RegisterBankInfo.cpp b/lib/Target/AArch64/AArch64RegisterBankInfo.cpp index 9b3899e0681..69124dbd0f8 100644 --- a/lib/Target/AArch64/AArch64RegisterBankInfo.cpp +++ b/lib/Target/AArch64/AArch64RegisterBankInfo.cpp @@ -469,10 +469,6 @@ AArch64RegisterBankInfo::getInstrMapping(const MachineInstr &MI) const { getCopyMapping(DstRB.getID(), SrcRB.getID(), Size), /*NumOperands*/ 2); } - case TargetOpcode::G_SEQUENCE: - // FIXME: support this, but the generic code is really not going to do - // anything sane. - return getInvalidInstructionMapping(); default: break; } diff --git a/test/CodeGen/AArch64/GlobalISel/legalize-combines.mir b/test/CodeGen/AArch64/GlobalISel/legalize-combines.mir index fab6dcf4334..e3e0175d39a 100644 --- a/test/CodeGen/AArch64/GlobalISel/legalize-combines.mir +++ b/test/CodeGen/AArch64/GlobalISel/legalize-combines.mir @@ -3,7 +3,6 @@ --- | target datalayout = "e-m:o-i64:64-i128:128-n32:64-S128" target triple = "aarch64--" - define void @test_combines_1() { ret void } define void @test_combines_2() { ret void } define void @test_combines_3() { ret void } define void @test_combines_4() { ret void } @@ -11,31 +10,6 @@ define void @test_combines_6() { ret void } ... ---- -name: test_combines_1 -body: | - bb.0: - liveins: %w0 - - %0:_(s32) = COPY %w0 - %1:_(s8) = G_TRUNC %0 - - ; Only one of these extracts can be eliminated, the offsets don't match - ; properly in the other cases. - ; CHECK-LABEL: name: test_combines_1 - ; CHECK: %2(s32) = G_SEQUENCE %1(s8), 1 - ; CHECK: %3(s8) = G_EXTRACT %2(s32), 0 - ; CHECK-NOT: G_EXTRACT - ; CHECK: %5(s8) = G_EXTRACT %2(s32), 2 - ; CHECK: %6(s32) = G_ZEXT %1(s8) - - %2:_(s32) = G_SEQUENCE %1, 1 - %3:_(s8) = G_EXTRACT %2, 0 - %4:_(s8) = G_EXTRACT %2, 1 - %5:_(s8) = G_EXTRACT %2, 2 - %6:_(s32) = G_ZEXT %4 -... - --- name: test_combines_2 body: | @@ -46,11 +20,11 @@ body: | ; Similarly, here the types don't match. ; CHECK-LABEL: name: test_combines_2 - ; CHECK: %2(s64) = G_SEQUENCE %0(s32), 0, %1(s32), 32 + ; CHECK: %2(s64) = G_MERGE_VALUES %0(s32), %1(s32) ; CHECK: %3(s1) = G_EXTRACT %2(s64), 0 ; CHECK: %4(s64) = G_EXTRACT %2(s64), 0 %1:_(s32) = G_ADD %0, %0 - %2:_(s64) = G_SEQUENCE %0, 0, %1, 32 + %2:_(s64) = G_MERGE_VALUES %0, %1 %3:_(s1) = G_EXTRACT %2, 0 %4:_(s64) = G_EXTRACT %2, 0 ... @@ -69,9 +43,8 @@ body: | ; CHECK-NOT: G_EXTRACT ; CHECK: %5(s32) = G_ADD %0, %1 %1:_(s32) = G_ADD %0, %0 - %2:_(s64) = G_SEQUENCE %0, 0, %1, 32 - %3:_(s32) = G_EXTRACT %2, 0 - %4:_(s32) = G_EXTRACT %2, 32 + %2:_(s64) = G_MERGE_VALUES %0, %1 + %3:_(s32), %4:_(s32) = G_UNMERGE_VALUES %2 %5:_(s32) = G_ADD %3, %4 ... @@ -86,7 +59,7 @@ body: | ; CHECK-LABEL: name: test_combines_4 ; CHECK: %2(<2 x s32>) = G_EXTRACT %1(s128), 0 ; CHECK: %3(<2 x s32>) = G_ADD %2, %2 - %1:_(s128) = G_SEQUENCE %0, 0, %0, 64 + %1:_(s128) = G_MERGE_VALUES %0, %0 %2:_(<2 x s32>) = G_EXTRACT %1, 0 %3:_(<2 x s32>) = G_ADD %2, %2 ... @@ -100,13 +73,12 @@ body: | %0:_(s32) = COPY %w0 ; CHECK-LABEL: name: test_combines_5 - ; CHECK-NOT: G_SEQUENCE + ; CHECK-NOT: G_MERGE_VALUES ; CHECK-NOT: G_EXTRACT ; CHECK: %5(s32) = G_ADD %0, %1 %1:_(s32) = G_ADD %0, %0 - %2:_(s64) = G_SEQUENCE %0, 0, %1, 32 - %3:_(s32) = G_EXTRACT %2, 0 - %4:_(s32) = G_EXTRACT %2, 32 + %2:_(s64) = G_MERGE_VALUES %0, %1 + %3:_(s32), %4:_(s32) = G_UNMERGE_VALUES %2 %5:_(s32) = G_ADD %3, %4 ... @@ -121,12 +93,12 @@ body: | %0:_(s32) = COPY %w0 ; Check that we replace all the uses of a G_EXTRACT. - ; CHECK-NOT: G_SEQUENCE + ; CHECK-NOT: G_MERGE_VALUES ; CHECK-NOT: G_EXTRACT ; CHECK: %3(s32) = G_MUL %0, %0 ; CHECK: %4(s32) = G_ADD %0, %3 - %1:_(s32) = G_SEQUENCE %0, 0 - %2:_(s32) = G_EXTRACT %1, 0 + %1:_(s32) = G_MERGE_VALUES %0 + %2:_(s32) = G_UNMERGE_VALUES %1 %3:_(s32) = G_MUL %2, %2 %4:_(s32) = G_ADD %2, %3 ... diff --git a/test/CodeGen/AArch64/GlobalISel/no-regclass.mir b/test/CodeGen/AArch64/GlobalISel/no-regclass.mir index 6832ce0ee8b..741d76b830c 100644 --- a/test/CodeGen/AArch64/GlobalISel/no-regclass.mir +++ b/test/CodeGen/AArch64/GlobalISel/no-regclass.mir @@ -24,7 +24,7 @@ body: | bb.0: liveins: %w0 %0:gpr(s32) = COPY %w0 - %1:gpr(s32) = G_SEQUENCE %0(s32), 0 - %2:gpr(s32) = G_EXTRACT %1(s32), 0 + %1:gpr(s32) = G_MERGE_VALUES %0(s32) + %2:gpr(s32) = G_UNMERGE_VALUES %1(s32) %w0 = COPY %2(s32) ... -- 2.50.1