Subzero: Simplify the constant pools.

src/assembler_ia32.cpp

Index: src/assembler_ia32.cpp
diff --git a/src/assembler_ia32.cpp b/src/assembler_ia32.cpp
index 8857a57cdce6b9040dc57d6b98d7a8452dbae398..b6601cc2267dd2062f8ef9a67d1075ee4577e2bb 100644
--- a/src/assembler_ia32.cpp
+++ b/src/assembler_ia32.cpp
@@ -60,8 +60,7 @@ Address Address::ofConstPool(GlobalContext *Ctx, Assembler *Asm,
   StrBuf << ".L$" << Ty << "$" << Imm->getPoolEntryID();
   const RelocOffsetT Offset = 0;
   const bool SuppressMangling = true;
-  Constant *Sym =
-      Ctx->getConstantSym(Ty, Offset, StrBuf.str(), SuppressMangling);
+  Constant *Sym = Ctx->getConstantSym(Offset, StrBuf.str(), SuppressMangling);
   AssemblerFixup *Fixup = x86::DisplacementRelocation::create(
       Asm, FK_Abs_4, llvm::cast<ConstantRelocatable>(Sym));
   return x86::Address::Absolute(Fixup);
@@ -960,7 +959,7 @@ void AssemblerX86::shufps(XmmRegister dst, XmmRegister src,
   EmitUint8(0x0F);
   EmitUint8(0xC6);
   EmitXmmRegisterOperand(dst, src);
-  assert(imm.is_uint8());
+  assert(imm.is_int8());
   EmitUint8(imm.value());
 }
 
@@ -971,7 +970,7 @@ void AssemblerX86::pshufd(Type /* Ty */, XmmRegister dst, XmmRegister src,
   EmitUint8(0x0F);
   EmitUint8(0x70);
   EmitXmmRegisterOperand(dst, src);
-  assert(imm.is_uint8());
+  assert(imm.is_int8());
   EmitUint8(imm.value());
 }
 
@@ -982,7 +981,7 @@ void AssemblerX86::pshufd(Type /* Ty */, XmmRegister dst, const Address &src,
   EmitUint8(0x0F);
   EmitUint8(0x70);
   EmitOperand(dst, src);
-  assert(imm.is_uint8());
+  assert(imm.is_int8());
   EmitUint8(imm.value());
 }
 
@@ -992,7 +991,7 @@ void AssemblerX86::shufps(Type /* Ty */, XmmRegister dst, XmmRegister src,
   EmitUint8(0x0F);
   EmitUint8(0xC6);
   EmitXmmRegisterOperand(dst, src);
-  assert(imm.is_uint8());
+  assert(imm.is_int8());
   EmitUint8(imm.value());
 }
 
@@ -1002,7 +1001,7 @@ void AssemblerX86::shufps(Type /* Ty */, XmmRegister dst, const Address &src,
   EmitUint8(0x0F);
   EmitUint8(0xC6);
   EmitOperand(dst, src);
-  assert(imm.is_uint8());
+  assert(imm.is_int8());
   EmitUint8(imm.value());
 }
 
@@ -1037,7 +1036,7 @@ void AssemblerX86::shufpd(XmmRegister dst, XmmRegister src,
   EmitUint8(0x0F);
   EmitUint8(0xC6);
   EmitXmmRegisterOperand(dst, src);
-  assert(imm.is_uint8());
+  assert(imm.is_int8());
   EmitUint8(imm.value());
 }
 
@@ -1232,7 +1231,7 @@ void AssemblerX86::andpd(XmmRegister dst, XmmRegister src) {
 void AssemblerX86::insertps(Type Ty, XmmRegister dst, XmmRegister src,
                             const Immediate &imm) {
   AssemblerBuffer::EnsureCapacity ensured(&buffer_);
-  assert(imm.is_uint8());
+  assert(imm.is_int8());
   assert(isVectorFloatingType(Ty));
   (void)Ty;
   EmitUint8(0x66);
@@ -1246,7 +1245,7 @@ void AssemblerX86::insertps(Type Ty, XmmRegister dst, XmmRegister src,
 void AssemblerX86::insertps(Type Ty, XmmRegister dst, const Address &src,
                             const Immediate &imm) {
   AssemblerBuffer::EnsureCapacity ensured(&buffer_);
-  assert(imm.is_uint8());
+  assert(imm.is_int8());
   assert(isVectorFloatingType(Ty));
   (void)Ty;
   EmitUint8(0x66);
@@ -1260,7 +1259,7 @@ void AssemblerX86::insertps(Type Ty, XmmRegister dst, const Address &src,
 void AssemblerX86::pinsr(Type Ty, XmmRegister dst, GPRRegister src,
                          const Immediate &imm) {
   AssemblerBuffer::EnsureCapacity ensured(&buffer_);
-  assert(imm.is_uint8());
+  assert(imm.is_int8());
   if (Ty == IceType_i16) {
     EmitUint8(0x66);
     EmitUint8(0x0F);
@@ -1280,7 +1279,7 @@ void AssemblerX86::pinsr(Type Ty, XmmRegister dst, GPRRegister src,
 void AssemblerX86::pinsr(Type Ty, XmmRegister dst, const Address &src,
                          const Immediate &imm) {
   AssemblerBuffer::EnsureCapacity ensured(&buffer_);
-  assert(imm.is_uint8());
+  assert(imm.is_int8());
   if (Ty == IceType_i16) {
     EmitUint8(0x66);
     EmitUint8(0x0F);
@@ -1300,7 +1299,7 @@ void AssemblerX86::pinsr(Type Ty, XmmRegister dst, const Address &src,
 void AssemblerX86::pextr(Type Ty, GPRRegister dst, XmmRegister src,
                          const Immediate &imm) {
   AssemblerBuffer::EnsureCapacity ensured(&buffer_);
-  assert(imm.is_uint8());
+  assert(imm.is_int8());
   if (Ty == IceType_i16) {
     EmitUint8(0x66);
     EmitUint8(0x0F);
@@ -1522,7 +1521,7 @@ void AssemblerX86::test(Type Ty, GPRRegister reg, const Immediate &immediate) {
   // This is legal even if the register had high bits set since
   // this only sets flags registers based on the "AND" of the two operands,
   // and the immediate had zeros at those high bits.
-  if (immediate.is_uint8() && reg < 4) {
+  if (immediate.is_int8() && reg < 4) {

[jvoung (off chromium), 2014/11/20 00:57:12]

Hmm after a second look I'm a little wary of this one, since the immediate is
supposed to represent a bit mask, and it's trying to optimize for when the upper
bits are actually 0.

[Jim Stichnoth, 2014/11/20 19:04:58]

OK, I think I got to the bottom of this.

The root cause is that the target lowering is creating 8-bit immediate constants
to represent masks and other unsigned 8-bit immediates, and when the value is
between 128:255, it ends up sign-extended in the internal representation and the
is_uint8() test fails.  The solution (or at least one easy solution) is for the
lowering to create these constants as a wider type like 32-bit so that sign
extension doesn't corrupt the bit pattern.

With that solution, I reverted all the int8 changes in the file, as well as all
the changes in one of the lit tests.

     // Use zero-extended 8-bit immediate.
     if (reg == RegX8632::Encoded_Reg_eax) {
       EmitUint8(0xA8);
@@ -1551,7 +1550,7 @@ void AssemblerX86::test(Type Ty, const Address &addr,
   AssemblerBuffer::EnsureCapacity ensured(&buffer_);
   // If the immediate is short, we only test the byte addr to keep the
   // encoding short.
-  if (immediate.is_uint8()) {
+  if (immediate.is_int8()) {
     // Use zero-extended 8-bit immediate.
     EmitUint8(0xF6);
     EmitOperand(0, addr);