Subzero: Avoid unused insts for ARM Om1 lowering for arithmetic

src/IceTargetLoweringARM32.cpp

 //===- subzero/src/IceTargetLoweringARM32.cpp - ARM32 lowering ------------===//
 //
 //                        The Subzero Code Generator
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 //
 // This file implements the TargetLoweringARM32 class, which consists almost
@@ skipping 1085 matching lines @@
   // TODO(jvoung): Should be able to flip Src0 and Src1 if it is easier
   // to legalize Src0 to flex or Src1 to flex and there is a reversible
   // instruction. E.g., reverse subtract with immediate, register vs
   // register, immediate.
   // Or it may be the case that the operands aren't swapped, but the
   // bits can be flipped and a different operation applied.
   // E.g., use BIC (bit clear) instead of AND for some masks.
   Operand *Src0 = Inst->getSrc(0);
   Operand *Src1 = Inst->getSrc(1);
   if (Dest->getType() == IceType_i64) {
+    // These helper-call-involved instructions are lowered in this
+    // separate switch. This is because we would otherwise assume that
+    // we need to legalize Src0 to Src0RLo and Src0Hi. However, those go unused
+    // with helper calls, and such unused/redundant instructions will fail
+    // liveness analysis under -Om1 setting.

[Jim Stichnoth, 2015/07/07 11:50:29]

Hmm, I wonder if that Om1 validation check should be relaxed?  It checks that
any temporaries with a use in this block also have a definition in this block,
which seems sound, but it also checks that any temporary definition also has a
last use, which fails on this situation.  We could just DCE such definitions.

On the other hand, these checks enforce a discipline of not creating unused
assignments, making codegen and analysis more efficient...

+    switch (Inst->getOp()) {
+    default:
+      break;
+    case InstArithmetic::Udiv:
+    case InstArithmetic::Sdiv:
+    case InstArithmetic::Urem:
+    case InstArithmetic::Srem: {
+      // Check for divide by 0 (ARM normally doesn't trap, but we want it
+      // to trap for NaCl). Src1Lo and Src1Hi may have already been legalized
+      // to a register, which will hide a constant source operand.
+      // Instead, check the not-yet-legalized Src1 to optimize-out a divide
+      // by 0 check.
+      if (auto *C64 = llvm::dyn_cast<ConstantInteger64>(Src1)) {
+        if (C64->getValue() == 0) {
+          _trap();
+          return;
+        }
+      } else {
+        Operand *Src1Lo = legalize(loOperand(Src1), Legal_Reg | Legal_Flex);
+        Operand *Src1Hi = legalize(hiOperand(Src1), Legal_Reg | Legal_Flex);
+        div0Check(IceType_i64, Src1Lo, Src1Hi);
+      }
+      // Technically, ARM has their own aeabi routines, but we can use the
+      // non-aeabi routine as well.  LLVM uses __aeabi_ldivmod for div,
+      // but uses the more standard __moddi3 for rem.
+      const char *HelperName = "";
+      switch (Inst->getOp()) {
+      default:
+        llvm_unreachable("Should have only matched div ops.");
+        break;
+      case InstArithmetic::Udiv:
+        HelperName = H_udiv_i64;
+        break;
+      case InstArithmetic::Sdiv:
+        HelperName = H_sdiv_i64;
+        break;
+      case InstArithmetic::Urem:
+        HelperName = H_urem_i64;
+        break;
+      case InstArithmetic::Srem:
+        HelperName = H_srem_i64;
+        break;
+      }
+      constexpr SizeT MaxSrcs = 2;
+      InstCall *Call = makeHelperCall(HelperName, Dest, MaxSrcs);
+      Call->addArg(Src0);
+      Call->addArg(Src1);
+      lowerCall(Call);
+      return;
+    }
+    }
     Variable *DestLo = llvm::cast<Variable>(loOperand(Dest));
     Variable *DestHi = llvm::cast<Variable>(hiOperand(Dest));
     Variable *Src0RLo = legalizeToVar(loOperand(Src0));
     Variable *Src0RHi = legalizeToVar(hiOperand(Src0));
-    Operand *Src1Lo = legalize(loOperand(Src1), Legal_Reg | Legal_Flex);
-    Operand *Src1Hi = legalize(hiOperand(Src1), Legal_Reg | Legal_Flex);
+    Operand *Src1Lo = loOperand(Src1);
+    Operand *Src1Hi = hiOperand(Src1);
     Variable *T_Lo = makeReg(DestLo->getType());
     Variable *T_Hi = makeReg(DestHi->getType());
     switch (Inst->getOp()) {
     case InstArithmetic::_num:
       llvm_unreachable("Unknown arithmetic operator");
-      break;
+      return;
     case InstArithmetic::Add:
+      Src1Lo = legalize(Src1Lo, Legal_Reg | Legal_Flex);
+      Src1Hi = legalize(Src1Hi, Legal_Reg | Legal_Flex);
       _adds(T_Lo, Src0RLo, Src1Lo);
       _mov(DestLo, T_Lo);
       _adc(T_Hi, Src0RHi, Src1Hi);
       _mov(DestHi, T_Hi);
-      break;
+      return;
     case InstArithmetic::And:
+      Src1Lo = legalize(Src1Lo, Legal_Reg | Legal_Flex);
+      Src1Hi = legalize(Src1Hi, Legal_Reg | Legal_Flex);
       _and(T_Lo, Src0RLo, Src1Lo);
       _mov(DestLo, T_Lo);
       _and(T_Hi, Src0RHi, Src1Hi);
       _mov(DestHi, T_Hi);
-      break;
+      return;
     case InstArithmetic::Or:
+      Src1Lo = legalize(Src1Lo, Legal_Reg | Legal_Flex);
+      Src1Hi = legalize(Src1Hi, Legal_Reg | Legal_Flex);
       _orr(T_Lo, Src0RLo, Src1Lo);
       _mov(DestLo, T_Lo);
       _orr(T_Hi, Src0RHi, Src1Hi);
       _mov(DestHi, T_Hi);
-      break;
+      return;
     case InstArithmetic::Xor:
+      Src1Lo = legalize(Src1Lo, Legal_Reg | Legal_Flex);
+      Src1Hi = legalize(Src1Hi, Legal_Reg | Legal_Flex);
       _eor(T_Lo, Src0RLo, Src1Lo);
       _mov(DestLo, T_Lo);
       _eor(T_Hi, Src0RHi, Src1Hi);
       _mov(DestHi, T_Hi);
-      break;
+      return;
     case InstArithmetic::Sub:
+      Src1Lo = legalize(Src1Lo, Legal_Reg | Legal_Flex);
+      Src1Hi = legalize(Src1Hi, Legal_Reg | Legal_Flex);
       _subs(T_Lo, Src0RLo, Src1Lo);
       _mov(DestLo, T_Lo);
       _sbc(T_Hi, Src0RHi, Src1Hi);
       _mov(DestHi, T_Hi);
-      break;
+      return;
     case InstArithmetic::Mul: {
       // GCC 4.8 does:
       // a=b*c ==>
       //   t_acc =(mul) (b.lo * c.hi)
       //   t_acc =(mla) (c.lo * b.hi) + t_acc
       //   t.hi,t.lo =(umull) b.lo * c.lo
       //   t.hi += t_acc
       //   a.lo = t.lo
       //   a.hi = t.hi
       //
@@ skipping 11 matching lines @@
       Variable *T_Acc1 = makeReg(IceType_i32);
       Variable *T_Hi1 = makeReg(IceType_i32);
       Variable *Src1RLo = legalizeToVar(Src1Lo);
       Variable *Src1RHi = legalizeToVar(Src1Hi);
       _mul(T_Acc, Src0RLo, Src1RHi);
       _mla(T_Acc1, Src1RLo, Src0RHi, T_Acc);
       _umull(T_Lo, T_Hi1, Src0RLo, Src1RLo);
       _add(T_Hi, T_Hi1, T_Acc1);
       _mov(DestLo, T_Lo);
       _mov(DestHi, T_Hi);
-    } break;
+      return;
+    }
     case InstArithmetic::Shl: {
       // a=b<<c ==>
       // GCC 4.8 does:
       // sub t_c1, c.lo, #32
       // lsl t_hi, b.hi, c.lo
       // orr t_hi, t_hi, b.lo, lsl t_c1
       // rsb t_c2, c.lo, #32
       // orr t_hi, t_hi, b.lo, lsr t_c2
       // lsl t_lo, b.lo, c.lo
       // a.lo = t_lo
@@ skipping 17 matching lines @@
       _orr(T_Hi, T_Hi, OperandARM32FlexReg::create(Func, IceType_i32, Src0RLo,
                                                    OperandARM32::LSR, T_C2));
       _mov(DestHi, T_Hi);
       Variable *T_Lo = makeReg(IceType_i32);
       // _mov seems to sometimes have better register preferencing than lsl.
       // Otherwise mov w/ lsl shifted register is a pseudo-instruction
       // that maps to lsl.
       _mov(T_Lo, OperandARM32FlexReg::create(Func, IceType_i32, Src0RLo,
                                              OperandARM32::LSL, Src1RLo));
       _mov(DestLo, T_Lo);
-    } break;
+      return;
+    }
     case InstArithmetic::Lshr:
     // a=b>>c (unsigned) ==>
     // GCC 4.8 does:
     // rsb t_c1, c.lo, #32
     // lsr t_lo, b.lo, c.lo
     // orr t_lo, t_lo, b.hi, lsl t_c1
     // sub t_c2, c.lo, #32
     // orr t_lo, t_lo, b.hi, lsr t_c2
     // lsr t_hi, b.hi, c.lo
     // a.lo = t_lo
@@ skipping 25 matching lines @@
         Pred = CondARM32::AL;
       }
       _orr(T_Lo, T_Lo, OperandARM32FlexReg::create(Func, IceType_i32, Src0RHi,
                                                    RShiftKind, T_C2),
            Pred);
       _mov(DestLo, T_Lo);
       Variable *T_Hi = makeReg(IceType_i32);
       _mov(T_Hi, OperandARM32FlexReg::create(Func, IceType_i32, Src0RHi,
                                              RShiftKind, Src1RLo));
       _mov(DestHi, T_Hi);
-    } break;
-    case InstArithmetic::Udiv:
-    case InstArithmetic::Sdiv:
-    case InstArithmetic::Urem:
-    case InstArithmetic::Srem: {
-      // Check for divide by 0 (ARM normally doesn't trap, but we want it
-      // to trap for NaCl). Src1Lo and Src1Hi may have already been legalized
-      // to a register, which will hide a constant source operand.
-      // Instead, check the not-yet-legalized Src1 to optimize-out a divide
-      // by 0 check.
-      if (auto *C64 = llvm::dyn_cast<ConstantInteger64>(Src1)) {
-        if (C64->getValue() == 0) {
-          div0Check(IceType_i64, Src1Lo, Src1Hi);
-        }
-      } else {
-        div0Check(IceType_i64, Src1Lo, Src1Hi);
-      }
-      // Technically, ARM has their own aeabi routines, but we can use the
-      // non-aeabi routine as well.  LLVM uses __aeabi_ldivmod for div,
-      // but uses the more standard __moddi3 for rem.
-      const char *HelperName = "";
-      switch (Inst->getOp()) {
-      case InstArithmetic::Udiv:
-        HelperName = H_udiv_i64;
-        break;
-      case InstArithmetic::Sdiv:
-        HelperName = H_sdiv_i64;
-        break;
-      case InstArithmetic::Urem:
-        HelperName = H_urem_i64;
-        break;
-      case InstArithmetic::Srem:
-        HelperName = H_srem_i64;
-        break;
-      default:
-        llvm_unreachable("Should have only matched div ops.");
-        break;
-      }
-      constexpr SizeT MaxSrcs = 2;
-      InstCall *Call = makeHelperCall(HelperName, Dest, MaxSrcs);
-      Call->addArg(Inst->getSrc(0));
-      Call->addArg(Inst->getSrc(1));
-      lowerCall(Call);
       return;
     }
     case InstArithmetic::Fadd:
     case InstArithmetic::Fsub:
     case InstArithmetic::Fmul:
     case InstArithmetic::Fdiv:
     case InstArithmetic::Frem:
       llvm_unreachable("FP instruction with i64 type");
-      break;
+      return;
+    case InstArithmetic::Udiv:
+    case InstArithmetic::Sdiv:
+    case InstArithmetic::Urem:
+    case InstArithmetic::Srem:
+      llvm_unreachable("Call-helper-involved instruction for i64 type "
+                       "should have already been handled before");
+      return;
     }
+    return;
   } else if (isVectorType(Dest->getType())) {
     UnimplementedError(Func->getContext()->getFlags());
-  } else { // Dest->getType() is non-i64 scalar
-    Variable *Src0R = legalizeToVar(Inst->getSrc(0));
-    Operand *Src1RF = legalize(Inst->getSrc(1), Legal_Reg | Legal_Flex);
-    Variable *T = makeReg(Dest->getType());
-    switch (Inst->getOp()) {
-    case InstArithmetic::_num:
-      llvm_unreachable("Unknown arithmetic operator");
-      break;
-    case InstArithmetic::Add: {
-      _add(T, Src0R, Src1RF);
-      _mov(Dest, T);
-    } break;
-    case InstArithmetic::And: {
-      _and(T, Src0R, Src1RF);
-      _mov(Dest, T);
-    } break;
-    case InstArithmetic::Or: {
-      _orr(T, Src0R, Src1RF);
-      _mov(Dest, T);
-    } break;
-    case InstArithmetic::Xor: {
-      _eor(T, Src0R, Src1RF);
-      _mov(Dest, T);
-    } break;
-    case InstArithmetic::Sub: {
-      _sub(T, Src0R, Src1RF);
-      _mov(Dest, T);
-    } break;
-    case InstArithmetic::Mul: {
-      Variable *Src1R = legalizeToVar(Src1RF);
-      _mul(T, Src0R, Src1R);
-      _mov(Dest, T);
-    } break;
-    case InstArithmetic::Shl:
-      _lsl(T, Src0R, Src1RF);
-      _mov(Dest, T);
-      break;
-    case InstArithmetic::Lshr:
-      _lsr(T, Src0R, Src1RF);
-      _mov(Dest, T);
-      break;
-    case InstArithmetic::Ashr:
-      _asr(T, Src0R, Src1RF);
-      _mov(Dest, T);
-      break;
-    case InstArithmetic::Udiv: {
-      constexpr bool IsRemainder = false;
-      lowerIDivRem(Dest, T, Src0R, Src1, &TargetARM32::_uxt,
-                   &TargetARM32::_udiv, H_udiv_i32, IsRemainder);
-      return;
-    }
-    case InstArithmetic::Sdiv: {
-      constexpr bool IsRemainder = false;
-      lowerIDivRem(Dest, T, Src0R, Src1, &TargetARM32::_sxt,
-                   &TargetARM32::_sdiv, H_sdiv_i32, IsRemainder);
-      return;
-    }
-    case InstArithmetic::Urem: {
-      constexpr bool IsRemainder = true;
-      lowerIDivRem(Dest, T, Src0R, Src1, &TargetARM32::_uxt,
-                   &TargetARM32::_udiv, H_urem_i32, IsRemainder);
-      return;
-    }
-    case InstArithmetic::Srem: {
-      constexpr bool IsRemainder = true;
-      lowerIDivRem(Dest, T, Src0R, Src1, &TargetARM32::_sxt,
-                   &TargetARM32::_sdiv, H_srem_i32, IsRemainder);
-      return;
-    }
-    case InstArithmetic::Fadd:
-      UnimplementedError(Func->getContext()->getFlags());
-      break;
-    case InstArithmetic::Fsub:
-      UnimplementedError(Func->getContext()->getFlags());
-      break;
-    case InstArithmetic::Fmul:
-      UnimplementedError(Func->getContext()->getFlags());
-      break;
-    case InstArithmetic::Fdiv:
-      UnimplementedError(Func->getContext()->getFlags());
-      break;
-    case InstArithmetic::Frem:
-      UnimplementedError(Func->getContext()->getFlags());
-      break;
-    }
+    return;
+  }
+  // Dest->getType() is a non-i64 scalar.
+  Variable *Src0R = legalizeToVar(Src0);
+  Variable *T = makeReg(Dest->getType());
+  // Handle div/rem separately. They require a non-legalized Src1 to inspect
+  // whether or not Src1 is a non-zero constant. Once legalized it is more
+  // difficult to determine (constant may be moved to a register).
+  switch (Inst->getOp()) {
+  default:
+    break;
+  case InstArithmetic::Udiv: {
+    constexpr bool IsRemainder = false;
+    lowerIDivRem(Dest, T, Src0R, Src1, &TargetARM32::_uxt, &TargetARM32::_udiv,
+                 H_udiv_i32, IsRemainder);
+    return;
+  }
+  case InstArithmetic::Sdiv: {
+    constexpr bool IsRemainder = false;
+    lowerIDivRem(Dest, T, Src0R, Src1, &TargetARM32::_sxt, &TargetARM32::_sdiv,
+                 H_sdiv_i32, IsRemainder);
+    return;
+  }
+  case InstArithmetic::Urem: {
+    constexpr bool IsRemainder = true;
+    lowerIDivRem(Dest, T, Src0R, Src1, &TargetARM32::_uxt, &TargetARM32::_udiv,
+                 H_urem_i32, IsRemainder);
+    return;
+  }
+  case InstArithmetic::Srem: {
+    constexpr bool IsRemainder = true;
+    lowerIDivRem(Dest, T, Src0R, Src1, &TargetARM32::_sxt, &TargetARM32::_sdiv,
+                 H_srem_i32, IsRemainder);
+    return;
+  }
+  }
+
+  Operand *Src1RF = legalize(Src1, Legal_Reg | Legal_Flex);
+  switch (Inst->getOp()) {
+  case InstArithmetic::_num:
+    llvm_unreachable("Unknown arithmetic operator");
+    return;
+  case InstArithmetic::Add:
+    _add(T, Src0R, Src1RF);
+    _mov(Dest, T);
+    return;
+  case InstArithmetic::And:
+    _and(T, Src0R, Src1RF);
+    _mov(Dest, T);
+    return;
+  case InstArithmetic::Or:
+    _orr(T, Src0R, Src1RF);
+    _mov(Dest, T);
+    return;
+  case InstArithmetic::Xor:
+    _eor(T, Src0R, Src1RF);
+    _mov(Dest, T);
+    return;
+  case InstArithmetic::Sub:
+    _sub(T, Src0R, Src1RF);
+    _mov(Dest, T);
+    return;
+  case InstArithmetic::Mul: {
+    Variable *Src1R = legalizeToVar(Src1RF);
+    _mul(T, Src0R, Src1R);
+    _mov(Dest, T);
+    return;
+  }
+  case InstArithmetic::Shl:
+    _lsl(T, Src0R, Src1RF);
+    _mov(Dest, T);
+    return;
+  case InstArithmetic::Lshr:
+    _lsr(T, Src0R, Src1RF);
+    _mov(Dest, T);
+    return;
+  case InstArithmetic::Ashr:
+    _asr(T, Src0R, Src1RF);
+    _mov(Dest, T);
+    return;
+  case InstArithmetic::Udiv:
+  case InstArithmetic::Sdiv:
+  case InstArithmetic::Urem:
+  case InstArithmetic::Srem:
+    llvm_unreachable("Integer div/rem should have been handled earlier.");
+    return;
+  case InstArithmetic::Fadd:
+    UnimplementedError(Func->getContext()->getFlags());
+    return;
+  case InstArithmetic::Fsub:
+    UnimplementedError(Func->getContext()->getFlags());
+    return;
+  case InstArithmetic::Fmul:
+    UnimplementedError(Func->getContext()->getFlags());
+    return;
+  case InstArithmetic::Fdiv:
+    UnimplementedError(Func->getContext()->getFlags());
+    return;
+  case InstArithmetic::Frem:
+    UnimplementedError(Func->getContext()->getFlags());
+    return;
   }
 }
 
 void TargetARM32::lowerAssign(const InstAssign *Inst) {
   Variable *Dest = Inst->getDest();
   Operand *Src0 = Inst->getSrc(0);
   assert(Dest->getType() == Src0->getType());
   if (Dest->getType() == IceType_i64) {
     Src0 = legalize(Src0);
     Operand *Src0Lo = loOperand(Src0);
@@ skipping 1177 matching lines @@
       << ".eabi_attribute 68, 1   @ Tag_Virtualization_use\n";
   if (CPUFeatures.hasFeature(TargetARM32Features::HWDivArm)) {
     Str << ".eabi_attribute 44, 2   @ Tag_DIV_use\n";
   }
   // Technically R9 is used for TLS with Sandboxing, and we reserve it.
   // However, for compatibility with current NaCl LLVM, don't claim that.
   Str << ".eabi_attribute 14, 3   @ Tag_ABI_PCS_R9_use: Not used\n";
 }
 
 } // end of namespace Ice