From 96470fe7e99e340ff67454d98c32935a43e530c5 Mon Sep 17 00:00:00 2001 From: Tim Northover Date: Tue, 27 Jun 2017 21:41:40 +0000 Subject: [PATCH] GlobalISel: verify that a COPY is trivial when created. Without this check, COPY instructions can actually be one of the generic casts in disguise. That's confusing and bad. At some point during ISel this restriction has to be relaxed since the fully selected instructions will usually use COPY for those purposes. Right now I think it's possible that relaxation occurs during RegBankSelect (hence the change there). I'm not convinced that's where it belongs long-term though. git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@306470 91177308-0d34-0410-b5e6-96231b3b80d8 --- lib/CodeGen/GlobalISel/MachineIRBuilder.cpp | 5 +++-- lib/CodeGen/GlobalISel/RegBankSelect.cpp | 10 +++++++--- test/CodeGen/AArch64/GlobalISel/legalize-exceptions.ll | 2 +- 3 files changed, 11 insertions(+), 6 deletions(-) diff --git a/lib/CodeGen/GlobalISel/MachineIRBuilder.cpp b/lib/CodeGen/GlobalISel/MachineIRBuilder.cpp index 6fcc3907dd2..ff3e53ad771 100644 --- a/lib/CodeGen/GlobalISel/MachineIRBuilder.cpp +++ b/lib/CodeGen/GlobalISel/MachineIRBuilder.cpp @@ -268,6 +268,8 @@ MachineInstrBuilder MachineIRBuilder::buildBrIndirect(unsigned Tgt) { } MachineInstrBuilder MachineIRBuilder::buildCopy(unsigned Res, unsigned Op) { + assert(MRI->getType(Res) == LLT() || MRI->getType(Op) == LLT() || + MRI->getType(Res) == MRI->getType(Op)); return buildInstr(TargetOpcode::COPY).addDef(Res).addUse(Op); } @@ -384,7 +386,6 @@ MachineInstrBuilder MachineIRBuilder::buildZExtOrTrunc(unsigned Res, return buildInstr(Opcode).addDef(Res).addUse(Op); } - MachineInstrBuilder MachineIRBuilder::buildCast(unsigned Dst, unsigned Src) { LLT SrcTy = MRI->getType(Src); LLT DstTy = MRI->getType(Dst); @@ -483,7 +484,7 @@ MachineInstrBuilder MachineIRBuilder::buildMerge(unsigned Res, #endif if (Ops.size() == 1) - return buildCopy(Res, Ops[0]); + return buildCast(Res, Ops[0]); MachineInstrBuilder MIB = buildInstr(TargetOpcode::G_MERGE_VALUES); MIB.addDef(Res); diff --git a/lib/CodeGen/GlobalISel/RegBankSelect.cpp b/lib/CodeGen/GlobalISel/RegBankSelect.cpp index 6ee69d5274d..677941dbbf6 100644 --- a/lib/CodeGen/GlobalISel/RegBankSelect.cpp +++ b/lib/CodeGen/GlobalISel/RegBankSelect.cpp @@ -154,9 +154,11 @@ bool RegBankSelect::repairReg( TargetRegisterInfo::isPhysicalRegister(Dst)) && "We are about to create several defs for Dst"); - // Build the instruction used to repair, then clone it at the right places. - MachineInstr *MI = MIRBuilder.buildCopy(Dst, Src); - MI->removeFromParent(); + // Build the instruction used to repair, then clone it at the right + // places. Avoiding buildCopy bypasses the check that Src and Dst have the + // same types because the type is a placeholder when this function is called. + MachineInstr *MI = + MIRBuilder.buildInstrNoInsert(TargetOpcode::COPY).addDef(Dst).addUse(Src); DEBUG(dbgs() << "Copy: " << PrintReg(Src) << " to: " << PrintReg(Dst) << '\n'); // TODO: @@ -556,9 +558,11 @@ bool RegBankSelect::applyMapping( llvm_unreachable("Other kind should not happen"); } } + // Second, rewrite the instruction. DEBUG(dbgs() << "Actual mapping of the operands: " << OpdMapper << '\n'); RBI->applyMapping(OpdMapper); + return true; } diff --git a/test/CodeGen/AArch64/GlobalISel/legalize-exceptions.ll b/test/CodeGen/AArch64/GlobalISel/legalize-exceptions.ll index d2452b86170..42ca367e122 100644 --- a/test/CodeGen/AArch64/GlobalISel/legalize-exceptions.ll +++ b/test/CodeGen/AArch64/GlobalISel/legalize-exceptions.ll @@ -22,7 +22,7 @@ declare void @_Unwind_Resume(i8*) ; CHECK: [[SEL:%[0-9]+]](s32) = G_PTRTOINT [[SEL_PTR]] ; CHECK: [[STRUCT_SEL:%[0-9]+]](s64) = G_INSERT {{%[0-9]+}}, [[SEL]](s32), 0 -; CHECK: [[PTR:%[0-9]+]](p0) = COPY [[STRUCT_PTR]](s64) +; CHECK: [[PTR:%[0-9]+]](p0) = G_INTTOPTR [[STRUCT_PTR]](s64) ; CHECK: G_STORE [[PTR]](p0), {{%[0-9]+}}(p0) ; CHECK: [[SEL_TMP:%[0-9]+]](s32) = G_EXTRACT [[STRUCT_SEL]](s64), 0 -- 2.50.1