Subzero: Mips: Lower some i64 arithmetic instructions

src/IceTargetLoweringMIPS32.cpp

Index: src/IceTargetLoweringMIPS32.cpp
diff --git a/src/IceTargetLoweringMIPS32.cpp b/src/IceTargetLoweringMIPS32.cpp
index d8cd65c2973ac502ca2cd18f8c99e000754c1f23..55f56631b6088e6b82592b5d8cf954ac4e3745f3 100644
--- a/src/IceTargetLoweringMIPS32.cpp
+++ b/src/IceTargetLoweringMIPS32.cpp
@@ -571,13 +571,88 @@ void TargetMIPS32::lowerAlloca(const InstAlloca *Inst) {
   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())) {
@@ -606,8 +681,6 @@ void TargetMIPS32::lowerArithmetic(const InstArithmetic *Inst) {
   // 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);
 
@@ -664,6 +737,7 @@ void TargetMIPS32::lowerArithmetic(const InstArithmetic *Inst) {
   case InstArithmetic::Frem:
     break;
   }
+  UnimplementedLoweringError(this, Inst);
 }
 
 void TargetMIPS32::lowerAssign(const InstAssign *Inst) {
@@ -708,8 +782,93 @@ 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) {
@@ -1087,7 +1246,10 @@ Operand *TargetMIPS32::legalize(Operand *From, LegalMask Allowed,
     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;