From: Daniel Sanders Date: Wed, 12 Mar 2014 13:35:43 +0000 (+0000) Subject: [mips][fp64] Add an implicit def to MTHC1 claiming that it reads the lower 32-bits... X-Git-Url: http://demsky.eecs.uci.edu/git/?a=commitdiff_plain;h=58b6bfeb229acfe4ded95af6099024a3411ead74;p=oota-llvm.git [mips][fp64] Add an implicit def to MTHC1 claiming that it reads the lower 32-bits of 64-bit FPR Summary: This is a white lie to workaround a widespread bug in the -mfp64 implementation. The problem is that none of the 32-bit fpu ops mention the fact that they clobber the upper 32-bits of the 64-bit FPR. This allows MTHC1 to be scheduled on the wrong side of most 32-bit FPU ops, particularly MTC1. Fixing that requires a major overhaul of the FPU implementation which can't be done right now due to time constraints. The testcase is SingleSource/Benchmarks/Misc/oourafft.c when given TARGET_CFLAGS='-mips32r2 mfp64 -mmsa'. Also correct the comment added in r203464 to indicate that two instructions were affected. Reviewers: matheusalmeida, jacksprat Reviewed By: matheusalmeida Differential Revision: http://llvm-reviews.chandlerc.com/D3029 git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@203659 91177308-0d34-0410-b5e6-96231b3b80d8 --- diff --git a/lib/Target/Mips/MipsSEInstrInfo.cpp b/lib/Target/Mips/MipsSEInstrInfo.cpp index a7ade1f47b2..195ad8ee1cc 100644 --- a/lib/Target/Mips/MipsSEInstrInfo.cpp +++ b/lib/Target/Mips/MipsSEInstrInfo.cpp @@ -513,11 +513,11 @@ void MipsSEInstrInfo::expandExtractElementF64(MachineBasicBlock &MBB, // that they clobber the upper 32-bits of the 64-bit FPR. Fixing that // requires a major overhaul of the FPU implementation which can't // be done right now due to time constraints. - // MFHC1 is the only instruction that is affected since it is the - // only instruction that doesn't read the lower 32-bits. We therefore - // pretend that it reads the bottom 32-bits to artificially create a - // dependency and prevent the scheduler changing the behaviour of the - // code. + // MFHC1 is one of two instructions that are affected since they are + // the only instructions that don't read the lower 32-bits. + // We therefore pretend that it reads the bottom 32-bits to + // artificially create a dependency and prevent the scheduler + // changing the behaviour of the code. BuildMI(MBB, I, dl, get(Mips::MFHC1), DstReg).addReg(SubReg).addReg( SrcReg, RegState::Implicit); } else @@ -543,10 +543,22 @@ void MipsSEInstrInfo::expandBuildPairF64(MachineBasicBlock &MBB, BuildMI(MBB, I, dl, Mtc1Tdd, TRI.getSubReg(DstReg, Mips::sub_lo)) .addReg(LoReg); - if (FP64) + if (FP64) { + // FIXME: The .addReg(DstReg, RegState::Implicit) is a white lie used to + // temporarily work around a widespread bug in the -mfp64 support. + // The problem is that none of the 32-bit fpu ops mention the fact + // that they clobber the upper 32-bits of the 64-bit FPR. Fixing that + // requires a major overhaul of the FPU implementation which can't + // be done right now due to time constraints. + // MTHC1 is one of two instructions that are affected since they are + // the only instructions that don't read the lower 32-bits. + // We therefore pretend that it reads the bottom 32-bits to + // artificially create a dependency and prevent the scheduler + // changing the behaviour of the code. BuildMI(MBB, I, dl, get(Mips::MTHC1), TRI.getSubReg(DstReg, Mips::sub_hi)) - .addReg(HiReg); - else + .addReg(HiReg) + .addReg(DstReg, RegState::Implicit); + } else BuildMI(MBB, I, dl, Mtc1Tdd, TRI.getSubReg(DstReg, Mips::sub_hi)) .addReg(HiReg); }