Subzero: Mips: Lower some i64 arithmetic instructions

src/IceTargetLoweringMIPS32.cpp

 //===- subzero/src/IceTargetLoweringMIPS32.cpp - MIPS32 lowering ----------===//
 //
 //                        The Subzero Code Generator
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 ///
 /// \file
@@ skipping 553 matching lines @@
   UsesFramePointer = true;
   // Conservatively require the stack to be aligned. Some stack adjustment
   // operations implemented below assume that the stack is aligned before the
   // alloca. All the alloca code ensures that the stack alignment is preserved
   // after the alloca. The stack alignment restriction can be relaxed in some
   // cases.
   NeedsStackAlignment = true;
   UnimplementedLoweringError(this, Inst);
 }
 
+void TargetMIPS32::lowerInt64Arithmetic(const InstArithmetic *Inst,
+                                        Variable *Dest, Operand *Src0,
+                                        Operand *Src1) {
+  InstArithmetic::OpKind Op = Inst->getOp();
+  switch (Op) {
+  case InstArithmetic::Add:
+  case InstArithmetic::And:
+  case InstArithmetic::Or:
+  case InstArithmetic::Sub:
+  case InstArithmetic::Xor:
+    break;
+  default:
+    UnimplementedLoweringError(this, Inst);
+    return;
+  }
+  Src0 = legalizeUndef(Src0);

[Jim Stichnoth, 2016/01/30 17:03:28]

Can this be removed?  It's already being done in the caller, and also it's
strange to see this done for Src0 but not Src1.

[rkotlerimgtec, 2016/01/30 23:48:30]

Done.

+  Operand *Src0Lo = legalize(loOperand(Src0), Legal_Reg);
+  Operand *Src0Hi = legalize(hiOperand(Src0), Legal_Reg);
+  Operand *Src1Lo = legalize(loOperand(Src1), Legal_Reg);
+  Operand *Src1Hi = legalize(hiOperand(Src1), Legal_Reg);
+  auto *DestLo = llvm::cast<Variable>(loOperand(Dest));
+  auto *DestHi = llvm::cast<Variable>(hiOperand(Dest));
+
+  Variable *Src0LoR = legalizeToReg(Src0Lo);

[Jim Stichnoth, 2016/01/30 17:03:28]

This can just be:

  Variable *Src0LoR = legalizeToReg(loOperand(Src0));

and Src0Lo above can be removed entirely.

Same for the next 3 Variables.

[rkotlerimgtec, 2016/01/30 23:48:31]

Done.

+  Variable *Src1LoR = legalizeToReg(Src1Lo);
+  Variable *Src0HiR = legalizeToReg(Src0Hi);
+  Variable *Src1HiR = legalizeToReg(Src1Hi);
+
+  switch (Op) {
+  case InstArithmetic::_num:
+    llvm::report_fatal_error("Unknown arithmetic operator");
+    return;
+  case InstArithmetic::Add: {
+    Variable *T_Lo = makeReg(IceType_i32);

[Jim Stichnoth, 2016/01/30 17:03:28]

This lowering sequence has a problem in that invalid addressing modes might be
generated if DestLo or DestHi are not register-allocated and therefore are
mapped to stack locations.

Something like this might be better:

    Variable *T_Carry = makeReg(IceType_i32);
    Variable *T_Lo = makeReg(IceType_i32);
    Variable *T_Hi = makeReg(IceType_i32);
    _addu(T_Lo, Src0LoR, Src1LoR);
    _mov(DestLo, T_Lo);
    _sltu(T_Carry, T_Lo, Src0LoR);
    _addu(T_Hi, T_Carry, Src0HiR);
    _addu(T_Hi, T_Hi, Src1HiR);
    _mov(DestHi, T_Hi);
    return;

[rkotlerimgtec, 2016/01/30 23:48:30]

Done.

+    Variable *T_Hi = makeReg(IceType_i32);
+    _addu(DestLo, Src0LoR, Src1LoR);
+    _sltu(T_Lo, DestLo, Src0LoR);
+    _addu(T_Hi,  T_Lo, Src0HiR);
+    _addu(DestHi, Src1HiR, T_Hi);
+    return;
+  }
+  case InstArithmetic::And: {
+    _and(DestLo, Src0LoR, Src1LoR);

[Jim Stichnoth, 2016/01/30 17:03:28]

Similar to above, you need to ensure the _and dest is a physical register.

_and(T_Lo, Src0LoR, Src1LoR);
_mov(DestLo, T_Lo);
_and(T_Hi, Src0HiR, Src1HiR);
_mov(DestHi, T_Hi);

[rkotlerimgtec, 2016/01/30 23:48:30]

Done.

[rkotlerimgtec, 2016/01/30 23:48:31]

Done.

+    _and(DestHi, Src0HiR, Src1HiR);
+    return;
+  }
+  case InstArithmetic::Sub: {
+    Variable *T_Lo = makeReg(IceType_i32);

[Jim Stichnoth, 2016/01/30 17:03:29]

Similar comment as for Add, maybe calling it "Borrow" instead of "Carry".

[rkotlerimgtec, 2016/01/30 23:48:30]

Done.

+    Variable *T_Hi = makeReg(IceType_i32);
+    _subu(DestLo, Src0LoR, Src1LoR);
+    _sltu(T_Lo, Src0LoR, Src1LoR);
+    _addu(T_Hi, T_Lo, Src1HiR);
+    _subu(DestHi, Src0HiR, T_Hi);
+    return;
+  }
+  case InstArithmetic::Or: {
+    _or(DestLo, Src0LoR, Src1LoR);
+    _or(DestHi, Src0HiR, Src1HiR);
+    return;
+  }
+  case InstArithmetic::Xor: {
+    _xor(DestLo, Src0LoR, Src1LoR);
+    _xor(DestHi, Src0HiR, Src1HiR);
+    return;
+  }
+  default:
+    UnimplementedLoweringError(this, Inst);
+    return;
+    //}

[Jim Stichnoth, 2016/01/30 17:03:29]

remove this

[rkotlerimgtec, 2016/01/30 23:48:30]

Done.

+  }
+}
+
 void TargetMIPS32::lowerArithmetic(const InstArithmetic *Inst) {
   Variable *Dest = Inst->getDest();
   // We need to signal all the UnimplementedLoweringError errors before any
   // legalization into new variables, otherwise Om1 register allocation may fail
   // when it sees variables that are defined but not used.
-  if (Dest->getType() == IceType_i64) {
-    UnimplementedLoweringError(this, Inst);
+  Type DestTy = Dest->getType();
+  Operand *Src0 = legalizeUndef(Inst->getSrc(0));
+  Operand *Src1 = legalizeUndef(Inst->getSrc(1));
+  if (DestTy == IceType_i64) {
+    lowerInt64Arithmetic(Inst, Inst->getDest(), Src0, Src1);
     return;
   }
   if (isVectorType(Dest->getType())) {
     UnimplementedLoweringError(this, Inst);
     return;
   }
   switch (Inst->getOp()) {
   default:
     break;
   case InstArithmetic::Shl:
   case InstArithmetic::Lshr:
   case InstArithmetic::Ashr:
   case InstArithmetic::Udiv:
   case InstArithmetic::Sdiv:
   case InstArithmetic::Urem:
   case InstArithmetic::Srem:
   case InstArithmetic::Fadd:
   case InstArithmetic::Fsub:
   case InstArithmetic::Fmul:
   case InstArithmetic::Fdiv:
   case InstArithmetic::Frem:
     UnimplementedLoweringError(this, Inst);
     return;
   }
 
   // At this point Dest->getType() is non-i64 scalar
 
   Variable *T = makeReg(Dest->getType());
-  Operand *Src0 = legalizeUndef(Inst->getSrc(0));
-  Operand *Src1 = legalizeUndef(Inst->getSrc(1));
   Variable *Src0R = legalizeToReg(Src0);
   Variable *Src1R = legalizeToReg(Src1);
 
   switch (Inst->getOp()) {
   case InstArithmetic::_num:
     break;
   case InstArithmetic::Add:
     _add(T, Src0R, Src1R);
     _mov(Dest, T);
     return;
@@ skipping 36 matching lines @@
     break;
   case InstArithmetic::Fsub:
     break;
   case InstArithmetic::Fmul:
     break;
   case InstArithmetic::Fdiv:
     break;
   case InstArithmetic::Frem:
     break;
   }
+  UnimplementedLoweringError(this, Inst);
 }
 
 void TargetMIPS32::lowerAssign(const InstAssign *Inst) {
   Variable *Dest = Inst->getDest();
   Operand *Src0 = Inst->getSrc(0);
   assert(Dest->getType() == Src0->getType());
   if (Dest->getType() == IceType_i64) {
     Src0 = legalizeUndef(Src0);
     Operand *Src0Lo = legalize(loOperand(Src0), Legal_Reg);
     Operand *Src0Hi = legalize(hiOperand(Src0), Legal_Reg);
@@ skipping 24 matching lines @@
     } else {
       _mov(Dest, SrcR);
     }
   }
 }
 
 void TargetMIPS32::lowerBr(const InstBr *Inst) {
   UnimplementedLoweringError(this, Inst);
 }
 
-void TargetMIPS32::lowerCall(const InstCall *Inst) {
-  UnimplementedLoweringError(this, Inst);
+void TargetMIPS32::lowerCall(const InstCall *Instr) {
+  // TODO(jvoung): assign arguments to registers and stack. Also reserve stack.

[Jim Stichnoth, 2016/01/30 17:03:28]

Don't add jvoung to a TODO.  :)

[rkotlerimgtec, 2016/01/30 23:48:31]

Done.

+  if (Instr->getNumArgs()) {
+    UnimplementedLoweringError(this, Instr);
+    return;
+  }
+  // Generate the call instruction.  Assign its result to a temporary

[Jim Stichnoth, 2016/01/30 17:03:28]

reflow to 80-col

[rkotlerimgtec, 2016/01/30 23:48:31]

Done.

+  // with high register allocation weight.
+  Variable *Dest = Instr->getDest();
+  // ReturnReg doubles as ReturnRegLo as necessary.
+  Variable *ReturnReg = nullptr;
+  Variable *ReturnRegHi = nullptr;
+  if (Dest) {
+    switch (Dest->getType()) {
+    case IceType_NUM:
+      llvm_unreachable("Invalid Call dest type");
+      return;
+    case IceType_void:
+      break;
+    case IceType_i1:
+    case IceType_i8:
+    case IceType_i16:
+    case IceType_i32:
+      ReturnReg = makeReg(Dest->getType(), RegMIPS32::Reg_V0);
+      break;
+    case IceType_i64:
+      ReturnReg = makeReg(IceType_i32, RegMIPS32::Reg_V0);
+      ReturnRegHi = makeReg(IceType_i32, RegMIPS32::Reg_V1);
+      break;
+    case IceType_f32:
+    case IceType_f64:
+      UnimplementedLoweringError(this, Instr);
+      return;
+    case IceType_v4i1:
+    case IceType_v8i1:
+    case IceType_v16i1:
+    case IceType_v16i8:
+    case IceType_v8i16:
+    case IceType_v4i32:
+    case IceType_v4f32:
+      UnimplementedLoweringError(this, Instr);
+      return;
+    }
+  }
+  Operand *CallTarget = Instr->getCallTarget();
+  // Allow ConstantRelocatable to be left alone as a direct call,
+  // but force other constants like ConstantInteger32 to be in
+  // a register and make it an indirect call.
+  if (!llvm::isa<ConstantRelocatable>(CallTarget)) {
+    CallTarget = legalize(CallTarget, Legal_Reg);
+  }
+  Inst *NewCall = InstMIPS32Call::create(Func, ReturnReg, CallTarget);
+  Context.insert(NewCall);
+  if (ReturnRegHi)
+    Context.insert(InstFakeDef::create(Func, ReturnRegHi));
+  // Insert a register-kill pseudo instruction.
+  Context.insert(InstFakeKill::create(Func, NewCall));
+  // Generate a FakeUse to keep the call live if necessary.
+  if (Instr->hasSideEffects() && ReturnReg) {
+    Inst *FakeUse = InstFakeUse::create(Func, ReturnReg);
+    Context.insert(FakeUse);
+  }
+  if (!Dest)

[Jim Stichnoth, 2016/01/30 17:03:28]

Dest == nullptr

[rkotlerimgtec, 2016/01/30 23:48:31]

Done.

+    return;
+
+  // Assign the result of the call to Dest.
+  if (ReturnReg) {
+    if (ReturnRegHi) {
+      assert(Dest->getType() == IceType_i64);
+      auto *Dest64On32 = llvm::cast<Variable64On32>(Dest);
+      Variable *DestLo = Dest64On32->getLo();
+      Variable *DestHi = Dest64On32->getHi();
+      _mov(DestLo, ReturnReg);
+      _mov(DestHi, ReturnRegHi);
+    } else {
+      assert(Dest->getType() == IceType_i32 || Dest->getType() == IceType_i16 ||
+             Dest->getType() == IceType_i8 || Dest->getType() == IceType_i1 ||
+             isVectorType(Dest->getType()));
+      if (isFloatingType(Dest->getType()) || isVectorType(Dest->getType())) {
+        UnimplementedLoweringError(this, Instr);
+        return;
+      } else {
+        _mov(Dest, ReturnReg);
+      }
+    }
+  }
+  UnimplementedLoweringError(this, Instr);
 }
 
 void TargetMIPS32::lowerCast(const InstCast *Inst) {
   InstCast::OpKind CastKind = Inst->getCastKind();
   switch (CastKind) {
   default:
     Func->setError("Cast type not supported");
     return;
   case InstCast::Sext: {
     UnimplementedLoweringError(this, Inst);
@@ skipping 252 matching lines @@
 // since loOperand() and hiOperand() don't expect Undef input.
 void TargetMIPS32::prelowerPhis() {
   PhiLowering::prelowerPhis32Bit<TargetMIPS32>(this, Context.getNode(), Func);
 }
 
 void TargetMIPS32::postLower() {
   if (Ctx->getFlags().getOptLevel() == Opt_m1)
     return;
   // Find two-address non-SSA instructions where Dest==Src0, and set the
   // IsDestRedefined flag to keep liveness analysis consistent.
   UnimplementedError(Func->getContext()->getFlags());

[Jim Stichnoth, 2016/01/30 17:03:29]

Remove this line, and replace it with "markRedefinitions();"

 }
 
 void TargetMIPS32::makeRandomRegisterPermutation(
     llvm::SmallVectorImpl<int32_t> &Permutation,
     const llvm::SmallBitVector &ExcludeRegisters, uint64_t Salt) const {
   (void)Permutation;
   (void)ExcludeRegisters;
   (void)Salt;
   UnimplementedError(Func->getContext()->getFlags());
 }
@@ skipping 84 matching lines @@
     // if a Flexible second operand is allowed. We need to know the exact
     // value, so that rules out relocatable constants.
     // Also try the inverse and use MVN if possible.
     // Do a movw/movt to a register.
     Variable *Reg;
     if (RegNum == Variable::NoRegister)
       Reg = makeReg(Ty, RegNum);
     else
       Reg = getPhysicalRegister(RegNum);
     if (isInt<16>(int32_t(Value))) {
-      _addiu(Reg, getPhysicalRegister(RegMIPS32::Reg_ZERO, Ty), Value);
+      //_addiu(Reg, getPhysicalRegister(RegMIPS32::Reg_ZERO, Ty), Value);

[Jim Stichnoth, 2016/01/30 17:03:28]

remove this

[rkotlerimgtec, 2016/01/30 23:48:31]

Done.

+      Variable *Zero = getPhysicalRegister(RegMIPS32::Reg_ZERO, Ty);
+      Context.insert<InstFakeDef>(Zero);
+      _addiu(Reg, Zero, Value);
     } else {
       uint32_t UpperBits = (Value >> 16) & 0xFFFF;
       (void)UpperBits;
       uint32_t LowerBits = Value & 0xFFFF;
       Variable *TReg = makeReg(Ty, RegNum);
       _lui(TReg, UpperBits);
       _ori(Reg, TReg, LowerBits);
     }
     return Reg;
   }
@@ skipping 26 matching lines @@
   Str << "\t.set\t"
       << "nomips16\n";
 }
 
 llvm::SmallBitVector TargetMIPS32::TypeToRegisterSet[RCMIPS32_NUM];
 llvm::SmallBitVector TargetMIPS32::TypeToRegisterSetUnfiltered[RCMIPS32_NUM];
 llvm::SmallBitVector TargetMIPS32::RegisterAliases[RegMIPS32::Reg_NUM];
 
 } // end of namespace MIPS32
 } // end of namespace Ice