From 47e2fafbf3d933532f46ef6e8515e7005df52758 Mon Sep 17 00:00:00 2001 From: Harald van Dijk Date: Sat, 28 Nov 2020 17:46:56 +0000 Subject: [PATCH] [X86] Do not allow FixupSetCC to relax constraints The build bots caught two additional pre-existing problems exposed by the test change part of my change https://reviews.llvm.org/D91339, when expensive checks are enabled. https://reviews.llvm.org/D91924 fixes one of them, this fixes the other. FixupSetCC will change code in the form of %setcc = SETCCr ... %ext1 = MOVZX32rr8 %setcc to %zero = MOV32r0 %setcc = SETCCr ... %ext2 = INSERT_SUBREG %zero, %setcc, %subreg.sub_8bit and replace uses of %ext1 with %ext2. The register class for %ext2 did not take into account any constraints on %ext1, which may have been required by its uses. This change ensures that the original constraints are honoured, by instead of creating a new %ext2 register, reusing %ext1 and further constraining it as needed. This requires a slight reorganisation to account for the fact that it is possible for the constraining to fail, in which case no changes should be made. Reviewed By: RKSimon Differential Revision: https://reviews.llvm.org/D91933 --- llvm/lib/Target/X86/X86FixupSetCC.cpp | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/llvm/lib/Target/X86/X86FixupSetCC.cpp b/llvm/lib/Target/X86/X86FixupSetCC.cpp index 09668d7..269f8ce 100644 --- a/llvm/lib/Target/X86/X86FixupSetCC.cpp +++ b/llvm/lib/Target/X86/X86FixupSetCC.cpp @@ -97,28 +97,31 @@ bool X86FixupSetCCPass::runOnMachineFunction(MachineFunction &MF) { if (FlagsDefMI->readsRegister(X86::EFLAGS)) continue; - ++NumSubstZexts; - Changed = true; - // On 32-bit, we need to be careful to force an ABCD register. const TargetRegisterClass *RC = MF.getSubtarget().is64Bit() ? &X86::GR32RegClass : &X86::GR32_ABCDRegClass; - Register ZeroReg = MRI->createVirtualRegister(RC); - Register InsertReg = MRI->createVirtualRegister(RC); + if (!MRI->constrainRegClass(ZExt->getOperand(0).getReg(), RC)) { + // If we cannot constrain the register, we would need an additional copy + // and are better off keeping the MOVZX32rr8 we have now. + continue; + } + + ++NumSubstZexts; + Changed = true; // Initialize a register with 0. This must go before the eflags def + Register ZeroReg = MRI->createVirtualRegister(RC); BuildMI(MBB, FlagsDefMI, MI.getDebugLoc(), TII->get(X86::MOV32r0), ZeroReg); // X86 setcc only takes an output GR8, so fake a GR32 input by inserting // the setcc result into the low byte of the zeroed register. BuildMI(*ZExt->getParent(), ZExt, ZExt->getDebugLoc(), - TII->get(X86::INSERT_SUBREG), InsertReg) + TII->get(X86::INSERT_SUBREG), ZExt->getOperand(0).getReg()) .addReg(ZeroReg) .addReg(MI.getOperand(0).getReg()) .addImm(X86::sub_8bit); - MRI->replaceRegWith(ZExt->getOperand(0).getReg(), InsertReg); ToErase.push_back(ZExt); } } -- 2.7.4