Insert denominator zero checks for NaCl

lib/Target/ARM/ARMNaClDivideCheck.cpp

+//===- ARMNaClDivideCheck.cpp - Add divide by zero checks ----------------===//
+//
+//                     The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===----------------------------------------------------------------------===//
+//
+// This pass adds a check for divide by zero after every integer DIV or REM.

[eliben, 2013/05/02 21:47:31]

after?

[sehr, 2013/05/03 20:33:07]

I changed the code to put the branch before.

+//
+//===----------------------------------------------------------------------===//
+
+#define DEBUG_TYPE "add-divide-check"
+#include "ARM.h"
+#include "llvm/ADT/STLExtras.h"
+#include "llvm/IR/BasicBlock.h"
+#include "llvm/IR/Constants.h"
+#include "llvm/IR/Function.h"
+#include "llvm/IR/Instructions.h"
+#include "llvm/IR/Intrinsics.h"
+#include "llvm/IR/LLVMContext.h"
+#include "llvm/Support/CFG.h"
+
+using namespace llvm;
+
+namespace llvm {
+void initializeARMNaClDivideCheckPass(PassRegistry&);
+}
+
+namespace {
+
+  class ARMNaClDivideCheck : public FunctionPass {
+
+  public:
+    static char ID;
+    ARMNaClDivideCheck() : FunctionPass(ID) {
+      initializeARMNaClDivideCheckPass(*PassRegistry::getPassRegistry());
+    }
+
+    bool runOnFunction(Function &F);
+  };
+}
+
+
+bool ARMNaClDivideCheck::runOnFunction(Function &F) {
+  Module *M = F.getParent();
+  BasicBlock *TrapBlock = NULL;
+  // BasicBlock list is being appended to during the optimization.

[eliben, 2013/05/02 21:47:31]

I'd prefer to see a beefier comment explaining that the BB can be split in which
case the next iteration will go to the new block

[sehr, 2013/05/03 20:33:07]

Done.

+  for (Function::iterator I = F.begin(); I != F.end(); I++) {
+    BasicBlock *BB = I;
+
+    for (BasicBlock::iterator BI = BB->begin(), BE = BB->end(); BI != BE;) {
+      BinaryOperator *DivInst = dyn_cast<BinaryOperator>(BI++);
+      if (!DivInst)
+        continue;
+      unsigned Opcode = DivInst->getOpcode();
+      if (Opcode != Instruction::SDiv && Opcode != Instruction::UDiv &&
+          Opcode != Instruction::SRem && Opcode != Instruction::URem)
+        continue;
+      // Create a single trap block.

[Jim Stichnoth, 2013/05/02 21:37:16]

You might want to check whether the denominator is a nonzero constant and avoid
the splitting.  This may be true reasonably often.

[sehr, 2013/05/03 20:33:07]

Done.  I insert a trap before any explicit divide by zero and elide the test
before any divide by non-zero constant.

+      if (TrapBlock == NULL) {

[eliben, 2013/05/02 21:47:31]

Put the trap block creation into a helper method?

[sehr, 2013/05/03 20:33:07]

Done.

+        TrapBlock = BasicBlock::Create(F.getContext(), "divrem.by.zero",
+                                       &F, NULL);

[eliben, 2013/05/02 21:47:31]

No need for explicit NULL here

[sehr, 2013/05/03 20:33:07]

Done.

+        Value *TrapFn = Intrinsic::getDeclaration(M, Intrinsic::trap);
+        CallInst::Create(TrapFn, "", TrapBlock);
+        new UnreachableInst(DivInst->getContext(), TrapBlock);
+      }
+      // Move all of the instructions from BB after DivInst to a new block.
+      BasicBlock *Successor = BB->splitBasicBlock(BI, "");

[eliben, 2013/05/02 21:47:31]

suggested tests: the div/rem is first / last instruction (obviously last before
terminator) in a BB

[sehr, 2013/05/03 20:33:07]

Done.

+      BB->getTerminator()->eraseFromParent();
+      // Compare the denominator with zero.
+      Value *Denominator = DivInst->getOperand(1);
+      Value *Zero = ConstantInt::get(Denominator->getType(), 0);
+      Value *DenomIsZero = new ICmpInst(DivInst, ICmpInst::ICMP_EQ, Denominator,
+                                        Zero, "");
+      // Put in a condbranch to the trap block.
+      BranchInst::Create(TrapBlock, Successor, DenomIsZero, BB);
+      // BI became invalid when we split.  Stop BasicBlock iterator.
+      break;
+    }
+  }
+
+  return false;
+}
+
+char ARMNaClDivideCheck::ID = 0;
+INITIALIZE_PASS(ARMNaClDivideCheck, "arm-nacl-divide-check",
+                "Add divide by zero checks", false, false)
+
+FunctionPass *llvm::createARMNaClDivideCheckPass() {
+  return new ARMNaClDivideCheck();
+}