Subzero. X8664. Fixes various small bugs.

src/IceTargetLoweringX86BaseImpl.h

 //===- subzero/src/IceTargetLoweringX86BaseImpl.h - x86 lowering -*- C++ -*-==//
 //
 //                        The Subzero Code Generator
 //
 // This file is distributed under the University of Illinois Open Source
 // License. See LICENSE.TXT for details.
 //
 //===----------------------------------------------------------------------===//
 ///
 /// \file
@@ skipping 2470 matching lines @@
   // is passed.
   for (SizeT i = 0, NumArgs = Instr->getNumArgs(); i < NumArgs; ++i) {
     Operand *Arg = Instr->getArg(i);
     const Type Ty = Arg->getType();
     // The PNaCl ABI requires the width of arguments to be at least 32 bits.
     assert(typeWidthInBytes(Ty) >= 4);
     if (isVectorType(Ty) && (Traits::getRegisterForXmmArgNum(XmmArgs.size()) !=
                              Variable::NoRegister)) {
       XmmArgs.push_back(Arg);
     } else if (isScalarFloatingType(Ty) && Traits::X86_PASS_SCALAR_FP_IN_XMM &&
-               (Traits::getRegisterForXmmArgNum(0) != Variable::NoRegister)) {
+               (Traits::getRegisterForXmmArgNum(XmmArgs.size()) !=
+                Variable::NoRegister)) {
       XmmArgs.push_back(Arg);
     } else if (isScalarIntegerType(Ty) &&
                (Traits::getRegisterForGprArgNum(Ty, GprArgs.size()) !=
                 Variable::NoRegister)) {
       GprArgs.emplace_back(Ty, Arg);
     } else {
       // Place on stack.
       StackArgs.push_back(Arg);
       if (isVectorType(Arg->getType())) {
         ParameterAreaSizeBytes =
@@ skipping 2090 matching lines @@
 template <typename TraitsType>
 void TargetX86Base<TraitsType>::lowerCountZeros(bool Cttz, Type Ty,
                                                 Variable *Dest,
                                                 Operand *FirstVal,
                                                 Operand *SecondVal) {
   // TODO(jvoung): Determine if the user CPU supports LZCNT (BMI).
   // Then the instructions will handle the Val == 0 case much more simply
   // and won't require conversion from bit position to number of zeros.
   //
   // Otherwise:
   //   bsr IF_NOT_ZERO, Val

[Jim Stichnoth, 2016/01/26 23:58:55]

These comments should be updated.

More importantly, I'd like to understand why our cross tests didn't catch this,
and to update them so that the new cases are handled/tested.

[John, 2016/01/27 13:03:15]

Look at the subzero output for one of the test_bitmanip tests:

        .text
        .globl  _ZN8Subzero_15test_const_ctlzEj
        .type   _ZN8Subzero_15test_const_ctlzEj,%function
        .p2align 5,0xf4
_ZN8Subzero_15test_const_ctlzEj:
.L_Z15test_const_ctlzj$entry:
        subl    $28, %esp
        movl    $74560, (%esp)
        call    _Z7my_ctlzj  /* wrong! should've been _ZN8Subzero_7my_ctlzEj */
        addl    $28, %esp
        ret


Essentially, for this test, we have

main (pnacl-clang)
test_bitmanip (llc)
test_bitmanip_intrin (llc) [+]
test_bitmanip (sz)  [*]
test_bitmanip_intrin (sz)

when subzero translates [*] the calls to my_cttz (and friends) are external to
the translation unit. Therefore, subzero knows better, and it does not mangle
those names.

Comes link time, and [*] is referring to the my_cttz (and friends) in [+], which
were translated by llc.

Effectively, the crosstest is testing if a sz-translated module can call a
routine an llc-translated routine.

It seems an fix for this would be to remove the test_bitmanip.cpp file and
inline its contents in the driver. The problem with this approach is that the
different tests there (_alloca, _const, and regular) will be reduntant.

Anyway, this is a serious issue (I am sure there are other instances of this
problem in our test suite) so maybe now is a good time to think/rework
crosstests (on another CL, of course)?

   //   mov T_DEST, 63
   //   cmovne T_DEST, IF_NOT_ZERO
   //   xor T_DEST, 31
   //   mov DEST, T_DEST
   //
   // NOTE: T_DEST must be a register because cmov requires its dest to be a
   // register. Also, bsf and bsr require their dest to be a register.
   //
   // The xor DEST, 31 converts a bit position to # of leading zeroes.
   // E.g., for 000... 00001100, bsr will say that the most significant bit
@@ skipping 13 matching lines @@
   Variable *T = makeReg(DestTy);
   Operand *FirstValRM = legalize(FirstVal, Legal_Mem | Legal_Reg);
   if (Cttz) {
     _bsf(T, FirstValRM);
   } else {
     _bsr(T, FirstValRM);
   }
   Variable *T_Dest = makeReg(DestTy);
   Constant *_31 = Ctx->getConstantInt32(31);
   Constant *_32 = Ctx->getConstantInt(DestTy, 32);
+  Constant *_63 = Ctx->getConstantInt(DestTy, 63);
+  Constant *_64 = Ctx->getConstantInt(DestTy, 64);
   if (Cttz) {
-    _mov(T_Dest, _32);
+    if (DestTy == IceType_i64) {
+      _mov(T_Dest, _64);
+    } else {
+      _mov(T_Dest, _32);
+    }
   } else {
-    Constant *_63 = Ctx->getConstantInt(DestTy, 63);
-    _mov(T_Dest, _63);
+    Constant *_127 = Ctx->getConstantInt(DestTy, 127);
+    if (DestTy == IceType_i64) {
+      _mov(T_Dest, _127);
+    } else {
+      _mov(T_Dest, _63);
+    }
   }
   _cmov(T_Dest, T, Traits::Cond::Br_ne);
   if (!Cttz) {
-    _xor(T_Dest, _31);
+    if (DestTy == IceType_i64) {
+      Constant *_63 = Ctx->getConstantInt32(63);
+      _xor(T_Dest, _63);
+    } else {
+      _xor(T_Dest, _31);
+    }
   }
   if (Traits::Is64Bit || Ty == IceType_i32) {
     _mov(Dest, T_Dest);
     return;
   }
   _add(T_Dest, _32);
   auto *DestLo = llvm::cast<Variable>(loOperand(Dest));
   auto *DestHi = llvm::cast<Variable>(hiOperand(Dest));
   // Will be using "test" on this, so we need a registerized variable.
   Variable *SecondVar = legalizeToReg(SecondVal);
@@ skipping 2779 matching lines @@
         emitGlobal(*Var, SectionSuffix);
       }
     }
   } break;
   }
 }
 } // end of namespace X86NAMESPACE
 } // end of namespace Ice
 
 #endif // SUBZERO_SRC_ICETARGETLOWERINGX86BASEIMPL_H