Random06457 created this revision.
Random06457 added a reviewer: atanasyan.
Herald added subscribers: dang, jrtc27, hiraditya, arichardson, mgorny, sdardis.
Random06457 requested review of this revision.
Herald added projects: clang, LLVM.
Herald added subscribers: llvm-commits, cfe-commits.
Early revisions of the VR4300 have a hardware bug where two consecutive
multiplications can produce an incorrect result in the second multiply.
This revision adds the `-mfix4300` flag to llvm (and clang) which, when passed,
provides a software fix for this issue.
**More precise description of the "mulmul" bug:**
1: mul.[s,d] fd,fs,ft
2: mul.[s,d] fd,fs,ft or [D]MULT[U] rs,rt
When the above sequence is executed by the CPU, if at least one of the source
operands of the first mul instruction happens to be `sNaN`, `0` or `Infinity`,
then the second mul instruction may produce an incorrect result.
This can happen both if the two mul instructions are next to each other of if
the first one is in a delay slot and the second is the first instruction of the
branch target.
**Description of the fix:**
This fix adds a backend pass to llvm which scans for mul instructions in each
basic block and happens a nop whenever the following conditions are met:
- The current instruction is a single or double-precision floating-point mul
instruction.
- The next instrution is either a mul instruction (any kind) or a branch
instruction.
**Note:**
I chose `-mfix4300` as a name for the flag to follow the GCC nomenclature but I
don't know if this is a good name.
Repository:
rG LLVM Github Monorepo
https://reviews.llvm.org/D116238
Files:
clang/include/clang/Driver/Options.td
clang/lib/Driver/ToolChains/Clang.cpp
llvm/lib/Target/Mips/CMakeLists.txt
llvm/lib/Target/Mips/Mips.h
llvm/lib/Target/Mips/MipsMulMulBugPass.cpp
llvm/lib/Target/Mips/MipsTargetMachine.cpp
llvm/test/CodeGen/Mips/vr4300mulmul/mulbranch.ll
llvm/test/CodeGen/Mips/vr4300mulmul/mulmul.ll
Index: llvm/test/CodeGen/Mips/vr4300mulmul/mulmul.ll
===================================================================
--- /dev/null
+++ llvm/test/CodeGen/Mips/vr4300mulmul/mulmul.ll
@@ -0,0 +1,12 @@
+; RUN: llc -march=mips -mfix4300 -verify-machineinstrs < %s | FileCheck %s
+
+; Function Attrs: mustprogress nofree norecurse nosync nounwind readnone willreturn
+define dso_local i32 @fun(i32 signext %x, i32 signext %y) local_unnamed_addr #0 {
+; CHECK: mul
+; CHECK-NEXT: nop
+; CHECK-NEXT: mul
+ %mul = mul nsw i32 %x, %x
+ %mul1 = mul nsw i32 %y, %y
+ %add = add nuw nsw i32 %mul1, %mul
+ ret i32 %add
+}
Index: llvm/test/CodeGen/Mips/vr4300mulmul/mulbranch.ll
===================================================================
--- /dev/null
+++ llvm/test/CodeGen/Mips/vr4300mulmul/mulbranch.ll
@@ -0,0 +1,12 @@
+; RUN: llc -march=mips -mfix4300 -verify-machineinstrs < %s | FileCheck %s
+
+; Function Attrs: nounwind
+define dso_local i32 @my_func(i32 signext %a) local_unnamed_addr #0 {
+; CHECK: mul
+; CHECK-NEXT: nop
+ %mul = mul nsw i32 %a, %a
+ %call = tail call i32 @foo(i32 signext %mul) #2
+ ret i32 %call
+}
+
+declare dso_local i32 @foo(i32 signext) local_unnamed_addr #1
Index: llvm/lib/Target/Mips/MipsTargetMachine.cpp
===================================================================
--- llvm/lib/Target/Mips/MipsTargetMachine.cpp
+++ llvm/lib/Target/Mips/MipsTargetMachine.cpp
@@ -292,6 +292,10 @@
// instructions which can be remapped to a 16 bit instruction.
addPass(createMicroMipsSizeReducePass());
+ // This pass inserts a nop instruction between two back-to-back multiplication
+ // instructions when the "mfix4300" flag is passed.
+ addPass(createMipsMulMulBugPass());
+
// The delay slot filler pass can potientially create forbidden slot hazards
// for MIPSR6 and therefore it should go before MipsBranchExpansion pass.
addPass(createMipsDelaySlotFillerPass());
Index: llvm/lib/Target/Mips/MipsMulMulBugPass.cpp
===================================================================
--- /dev/null
+++ llvm/lib/Target/Mips/MipsMulMulBugPass.cpp
@@ -0,0 +1,121 @@
+#include "Mips.h"
+#include "MipsInstrInfo.h"
+#include "MipsSubtarget.h"
+#include "llvm/CodeGen/MachineBasicBlock.h"
+#include "llvm/CodeGen/MachineFunction.h"
+#include "llvm/CodeGen/MachineFunctionPass.h"
+#include "llvm/Support/CommandLine.h"
+#include "llvm/Support/Debug.h"
+#include "llvm/Target/TargetMachine.h"
+
+#define DEBUG_TYPE "mips-r4300-mulmul-fix"
+
+using namespace llvm;
+
+static cl::opt<bool>
+ EnableMulMulFix("mfix4300", cl::init(false),
+ cl::desc("Enable the VR4300 mulmul bug fix."), cl::Hidden);
+
+class MipsMulMulBugFix : public MachineFunctionPass {
+public:
+ MipsMulMulBugFix() : MachineFunctionPass(ID) {}
+
+ StringRef getPassName() const override { return "Mips mulmul bugfix"; }
+
+ MachineFunctionProperties getRequiredProperties() const override {
+ return MachineFunctionProperties().set(
+ MachineFunctionProperties::Property::NoVRegs);
+ }
+
+ bool runOnMachineFunction(MachineFunction &MF) override;
+ bool FixMulMulBB(MachineBasicBlock &MBB);
+
+ static char ID;
+
+private:
+ static const MipsInstrInfo *MipsII;
+ const MipsSubtarget *Subtarget;
+};
+
+char MipsMulMulBugFix::ID = 0;
+const MipsInstrInfo *MipsMulMulBugFix::MipsII;
+
+bool MipsMulMulBugFix::runOnMachineFunction(MachineFunction &MF) {
+
+ if (!EnableMulMulFix)
+ return false;
+
+ Subtarget = &static_cast<const MipsSubtarget &>(MF.getSubtarget());
+ MipsII = static_cast<const MipsInstrInfo *>(Subtarget->getInstrInfo());
+
+ bool Modified = false;
+ MachineFunction::iterator I = MF.begin(), E = MF.end();
+
+ for (; I != E; ++I)
+ Modified |= FixMulMulBB(*I);
+
+ return Modified;
+}
+
+static bool isFirstMul(const MachineInstr *MI) {
+ switch (MI->getOpcode()) {
+ case Mips::MUL:
+ case Mips::FMUL_S:
+ case Mips::FMUL_D:
+ case Mips::FMUL_D32:
+ case Mips::FMUL_D64:
+ return true;
+ default:
+ return false;
+ }
+}
+
+static bool isSecondMulOrBranch(const MachineInstr *MI) {
+ if (MI->isBranch() || MI->isIndirectBranch() || MI->isCall())
+ return true;
+
+ switch (MI->getOpcode()) {
+ case Mips::MUL:
+ case Mips::FMUL_S:
+ case Mips::FMUL_D:
+ case Mips::FMUL_D32:
+ case Mips::FMUL_D64:
+ case Mips::MULT:
+ case Mips::MULTu:
+ case Mips::DMULT:
+ case Mips::DMULTu:
+ return true;
+ default:
+ return false;
+ }
+}
+
+bool MipsMulMulBugFix::FixMulMulBB(MachineBasicBlock &MBB) {
+ bool Modified = false;
+ MachineBasicBlock::instr_iterator MII = MBB.instr_begin(),
+ E = MBB.instr_end();
+ MachineBasicBlock::instr_iterator NextMII;
+
+ // Iterate through the instructions in the basic block
+ for (; MII != E; MII = NextMII) {
+
+ NextMII = std::next(MII);
+ MachineInstr *MI = &*MII;
+
+ // Trigger when the current instruction is a mul and the next instruction
+ // is either a mul or a branch in case the branch target start with a mul
+ if (NextMII != E && isFirstMul(MI) && isSecondMulOrBranch(&*NextMII)) {
+ LLVM_DEBUG(dbgs() << "Found mulmul!");
+
+ MachineBasicBlock &MBB = *MI->getParent();
+ const MCInstrDesc &NewMCID = MipsII->get(Mips::NOP);
+ BuildMI(MBB, NextMII, DebugLoc(), NewMCID);
+ Modified = true;
+ }
+ }
+
+ return Modified;
+}
+
+/// createMipsMulMulBugPass - Returns a pass that fixes the mulmul bug
+FunctionPass *llvm::createMipsMulMulBugPass() { return new MipsMulMulBugFix(); }
Index: llvm/lib/Target/Mips/Mips.h
===================================================================
--- llvm/lib/Target/Mips/Mips.h
+++ llvm/lib/Target/Mips/Mips.h
@@ -38,6 +38,7 @@
FunctionPass *createMicroMipsSizeReducePass();
FunctionPass *createMipsExpandPseudoPass();
FunctionPass *createMipsPreLegalizeCombiner();
+ FunctionPass *createMipsMulMulBugPass();
InstructionSelector *createMipsInstructionSelector(const MipsTargetMachine &,
MipsSubtarget &,
Index: llvm/lib/Target/Mips/CMakeLists.txt
===================================================================
--- llvm/lib/Target/Mips/CMakeLists.txt
+++ llvm/lib/Target/Mips/CMakeLists.txt
@@ -59,6 +59,7 @@
MipsTargetMachine.cpp
MipsTargetObjectFile.cpp
MicroMipsSizeReduction.cpp
+ MipsMulMulBugPass.cpp
LINK_COMPONENTS
Analysis
Index: clang/lib/Driver/ToolChains/Clang.cpp
===================================================================
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -1929,6 +1929,11 @@
}
}
+ if (Arg *A = Args.getLastArg(options::OPT_mfix4300)) {
+ CmdArgs.push_back("-mllvm");
+ CmdArgs.push_back("-mfix4300");
+ }
+
if (Arg *A = Args.getLastArg(options::OPT_G)) {
StringRef v = A->getValue();
CmdArgs.push_back("-mllvm");
Index: clang/include/clang/Driver/Options.td
===================================================================
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -3617,6 +3617,7 @@
Group<m_mips_Features_Group>;
def mno_check_zero_division : Flag<["-"], "mno-check-zero-division">,
Group<m_mips_Features_Group>;
+def mfix4300 : Flag<["-"], "mfix4300">, Group<m_mips_Features_Group>;
def mcompact_branches_EQ : Joined<["-"], "mcompact-branches=">,
Group<m_mips_Features_Group>;
def mbranch_likely : Flag<["-"], "mbranch-likely">, Group<m_Group>,
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
