Subzero. Removes references to %ah.

src/IceTargetLoweringX86BaseImpl.h

Index: src/IceTargetLoweringX86BaseImpl.h
diff --git a/src/IceTargetLoweringX86BaseImpl.h b/src/IceTargetLoweringX86BaseImpl.h
index 77048b089c1a4c9934568f8dd973429138e36f9c..02b4cd140e7995561a7daa4c91936cea255ff3ff 100644
--- a/src/IceTargetLoweringX86BaseImpl.h
+++ b/src/IceTargetLoweringX86BaseImpl.h
@@ -1850,12 +1850,13 @@ void TargetX86Base<Machine>::lowerArithmetic(const InstArithmetic *Inst) {
     // 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);
@@ -1917,12 +1918,21 @@ void TargetX86Base<Machine>::lowerArithmetic(const InstArithmetic *Inst) {
   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);
@@ -1974,12 +1984,21 @@ void TargetX86Base<Machine>::lowerArithmetic(const InstArithmetic *Inst) {
     }
     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);