Subzero. Removes references to %ah.

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 1832 matching lines @@
     if (!llvm::isa<Constant>(Src1))
       Src1 = legalizeToReg(Src1, Traits::RegisterSet::Reg_ecx);
     _sar(T, Src1);
     _mov(Dest, T);
     break;
   case InstArithmetic::Udiv:
     // div and idiv are the few arithmetic operators that do not allow
     // immediates as the operand.
     Src1 = legalize(Src1, Legal_Reg | Legal_Mem);
     if (isByteSizedArithType(Dest->getType())) {
-      Variable *T_ah = nullptr;
-      Constant *Zero = Ctx->getConstantZero(IceType_i8);
+      Variable *T_eax = makeReg(IceType_i16, Traits::RegisterSet::Reg_eax);

[Jim Stichnoth, 2015/07/28 15:58:23]

Add a comment explaining the ah workaround.

[jvoung (off chromium), 2015/07/28 16:17:45]

For T_eax, why not use IceType_i32 instead of IceType_i16, like in the Urem
case?  Should be smaller without the 0x66 prefix?

[John, 2015/07/28 18:21:16]

Regardless of being smaler, using i32 is more efficient and what I intended.
Thanks for catching this.

[John, 2015/07/28 18:21:16]

Done.

+      Context.insert(InstFakeDef::create(Func, T_eax));
+      _xor(T_eax, T_eax);
       _mov(T, Src0, Traits::RegisterSet::Reg_eax);
-      _mov(T_ah, Zero, Traits::RegisterSet::Reg_ah);
-      _div(T, Src1, T_ah);
+      _div(T, Src1, T);
       _mov(Dest, T);
+      Context.insert(InstFakeUse::create(Func, T_eax));
     } else {
       Constant *Zero = Ctx->getConstantZero(IceType_i32);
       _mov(T, Src0, Traits::RegisterSet::Reg_eax);
       _mov(T_edx, Zero, Traits::RegisterSet::Reg_edx);
       _div(T, Src1, T_edx);
       _mov(Dest, T);
     }
     break;
   case InstArithmetic::Sdiv:
     // TODO(stichnot): Enable this after doing better performance
@@ skipping 41 matching lines @@
       T_edx = makeReg(IceType_i32, Traits::RegisterSet::Reg_edx);
       _mov(T, Src0, Traits::RegisterSet::Reg_eax);
       _cbwdq(T_edx, T);
       _idiv(T, Src1, T_edx);
       _mov(Dest, T);
     }
     break;
   case InstArithmetic::Urem:
     Src1 = legalize(Src1, Legal_Reg | Legal_Mem);
     if (isByteSizedArithType(Dest->getType())) {
-      Variable *T_ah = nullptr;
-      Constant *Zero = Ctx->getConstantZero(IceType_i8);
+      Variable *T_eax = makeReg(IceType_i32, Traits::RegisterSet::Reg_eax);
+      Context.insert(InstFakeDef::create(Func, T_eax));
+      _xor(T_eax, T_eax);
+      _set_dest_nonkillable();

[Jim Stichnoth, 2015/07/28 15:58:23]

I don't think any of these new instances of _set_dest_nonkillable() are needed,
can you double-check?

That method is poorly named, sorry about that.  Generally you only need to call
it after inserting an instruction where the dest variable appears in a source
operand of the instruction except for Dest==Srcs[0].  The Dest==Srcs[0] case is
handled as part of postLower().  For any other source operands, or if Srcs[0] is
e.g. a mem operand that contains Dest (such as in some lea cases), calling it is
needed.

[John, 2015/07/28 18:21:16]

Done.

       _mov(T, Src0, Traits::RegisterSet::Reg_eax);
-      _mov(T_ah, Zero, Traits::RegisterSet::Reg_ah);
-      _div(T_ah, Src1, T);
-      _mov(Dest, T_ah);
+      Variable *T_al = makeReg(IceType_i8, Traits::RegisterSet::Reg_eax);
+      _div(T_al, Src1, T);
+      // shr $8, %eax shifts ah (i.e., the 8 bit remainder) into al. We don't
+      // mov %ah, %al because it would make x86-64 codegen more complicated. If
+      // this ever becomes a problem we can introduce a pseudo rem instruction
+      // that returns the remainder in %al directly (and uses a mov for copying
+      // %ah to %al.)
+      _shr(T_eax, Ctx->getConstantInt8(8));

[Jim Stichnoth, 2015/07/28 15:58:23]

Consider using X86_CHAR_BIT instead of 8.

[John, 2015/07/28 18:21:16]

The name X86_CHAR_BIT is somewhat confusing. In C CHAR_BIT represents the number
of bits in a char (which you can't portably trust to always be 8.) I would
rather leave an 8 (or use a named constant, e.g., AlSizeInBytes.) In the future
I would like to rename X86_CHAR_BITS to X86_BYTE_SIZE or something else.

+      _mov(Dest, T_al);
+      Context.insert(InstFakeUse::create(Func, T_eax));
     } else {
       Constant *Zero = Ctx->getConstantZero(IceType_i32);
       _mov(T_edx, Zero, Traits::RegisterSet::Reg_edx);
       _mov(T, Src0, Traits::RegisterSet::Reg_eax);
       _div(T_edx, Src1, T);
       _mov(Dest, T_edx);
     }
     break;
   case InstArithmetic::Srem:
     // TODO(stichnot): Enable this after doing better performance
@@ skipping 31 matching lines @@
           _and(T, Ctx->getConstantInt(Ty, -(1 << LogDiv)));
           _sub(T, Src0);
           _neg(T);
           _mov(Dest, T);
           return;
         }
       }
     }
     Src1 = legalize(Src1, Legal_Reg | Legal_Mem);
     if (isByteSizedArithType(Dest->getType())) {
-      Variable *T_ah = makeReg(IceType_i8, Traits::RegisterSet::Reg_ah);
       _mov(T, Src0, Traits::RegisterSet::Reg_eax);
+      // T is %al.
+      _set_dest_nonkillable();
       _cbwdq(T, T);
-      Context.insert(InstFakeDef::create(Func, T_ah));
-      _idiv(T_ah, Src1, T);
-      _mov(Dest, T_ah);
+      _idiv(T, Src1, T);
+      Variable *T_eax = makeReg(IceType_i32, Traits::RegisterSet::Reg_eax);
+      Context.insert(InstFakeDef::create(Func, T_eax));
+      // shr $8, %eax shifts ah (i.e., the 8 bit remainder) into al. We don't
+      // mov %ah, %al because it would make x86-64 codegen more complicated. If
+      // this ever becomes a problem we can introduce a pseudo rem instruction
+      // that returns the remainder in %al directly (and uses a mov for copying
+      // %ah to %al.)
+      _shr(T_eax, Ctx->getConstantInt8(8));
+      _mov(Dest, T);
+      Context.insert(InstFakeUse::create(Func, T_eax));
     } else {
       T_edx = makeReg(IceType_i32, Traits::RegisterSet::Reg_edx);
       _mov(T, Src0, Traits::RegisterSet::Reg_eax);
       _cbwdq(T_edx, T);
       _idiv(T_edx, Src1, T);
       _mov(Dest, T_edx);
     }
     break;
   case InstArithmetic::Fadd:
     _mov(T, Src0);
@@ skipping 3623 matching lines @@
   }
   // the offset is not eligible for blinding or pooling, return the original
   // mem operand
   return MemOperand;
 }
 
 } // end of namespace X86Internal
 } // end of namespace Ice
 
 #endif // SUBZERO_SRC_ICETARGETLOWERINGX86BASEIMPL_H