Subzero: Simplify the constant pools.

src/assembler_ia32.cpp

 //===- subzero/src/assembler_ia32.cpp - Assembler for x86-32  -------------===//
 // Copyright (c) 2013, the Dart project authors.  Please see the AUTHORS file
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 //
 // Modified by the Subzero authors.
 //
 //===----------------------------------------------------------------------===//
 //
 //                        The Subzero Code Generator
@@ skipping 42 matching lines @@
                              const Constant *Imm) {
   // We should make this much lighter-weight. E.g., just record the const pool
   // entry ID.
   std::string Buffer;
   llvm::raw_string_ostream StrBuf(Buffer);
   Type Ty = Imm->getType();
   assert(llvm::isa<ConstantFloat>(Imm) || llvm::isa<ConstantDouble>(Imm));
   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);
 }
 
 AssemblerX86::~AssemblerX86() {
 #ifndef NDEBUG
   for (const Label *Label : CfgNodeLabels) {
     Label->FinalCheck();
   }
@@ skipping 878 matching lines @@
   // Broadcast low lane into other three lanes.
   shufps(dst, dst, Immediate(0x0));
 }
 
 void AssemblerX86::shufps(XmmRegister dst, XmmRegister src,
                           const Immediate &imm) {
   AssemblerBuffer::EnsureCapacity ensured(&buffer_);
   EmitUint8(0x0F);
   EmitUint8(0xC6);
   EmitXmmRegisterOperand(dst, src);
-  assert(imm.is_uint8());
+  assert(imm.is_int8());
   EmitUint8(imm.value());
 }
 
 void AssemblerX86::pshufd(Type /* Ty */, XmmRegister dst, XmmRegister src,
                           const Immediate &imm) {
   AssemblerBuffer::EnsureCapacity ensured(&buffer_);
   EmitUint8(0x66);
   EmitUint8(0x0F);
   EmitUint8(0x70);
   EmitXmmRegisterOperand(dst, src);
-  assert(imm.is_uint8());
+  assert(imm.is_int8());
   EmitUint8(imm.value());
 }
 
 void AssemblerX86::pshufd(Type /* Ty */, XmmRegister dst, const Address &src,
                           const Immediate &imm) {
   AssemblerBuffer::EnsureCapacity ensured(&buffer_);
   EmitUint8(0x66);
   EmitUint8(0x0F);
   EmitUint8(0x70);
   EmitOperand(dst, src);
-  assert(imm.is_uint8());
+  assert(imm.is_int8());
   EmitUint8(imm.value());
 }
 
 void AssemblerX86::shufps(Type /* Ty */, XmmRegister dst, XmmRegister src,
                           const Immediate &imm) {
   AssemblerBuffer::EnsureCapacity ensured(&buffer_);
   EmitUint8(0x0F);
   EmitUint8(0xC6);
   EmitXmmRegisterOperand(dst, src);
-  assert(imm.is_uint8());
+  assert(imm.is_int8());
   EmitUint8(imm.value());
 }
 
 void AssemblerX86::shufps(Type /* Ty */, XmmRegister dst, const Address &src,
                           const Immediate &imm) {
   AssemblerBuffer::EnsureCapacity ensured(&buffer_);
   EmitUint8(0x0F);
   EmitUint8(0xC6);
   EmitOperand(dst, src);
-  assert(imm.is_uint8());
+  assert(imm.is_int8());
   EmitUint8(imm.value());
 }
 
 void AssemblerX86::minpd(XmmRegister dst, XmmRegister src) {
   AssemblerBuffer::EnsureCapacity ensured(&buffer_);
   EmitUint8(0x66);
   EmitUint8(0x0F);
   EmitUint8(0x5D);
   EmitXmmRegisterOperand(dst, src);
 }
@@ skipping 14 matching lines @@
   EmitXmmRegisterOperand(dst, dst);
 }
 
 void AssemblerX86::shufpd(XmmRegister dst, XmmRegister src,
                           const Immediate &imm) {
   AssemblerBuffer::EnsureCapacity ensured(&buffer_);
   EmitUint8(0x66);
   EmitUint8(0x0F);
   EmitUint8(0xC6);
   EmitXmmRegisterOperand(dst, src);
-  assert(imm.is_uint8());
+  assert(imm.is_int8());
   EmitUint8(imm.value());
 }
 
 void AssemblerX86::cvtdq2ps(Type /* Ignore */, XmmRegister dst,
                             XmmRegister src) {
   AssemblerBuffer::EnsureCapacity ensured(&buffer_);
   EmitUint8(0x0F);
   EmitUint8(0x5B);
   EmitXmmRegisterOperand(dst, src);
 }
@@ skipping 174 matching lines @@
   AssemblerBuffer::EnsureCapacity ensured(&buffer_);
   EmitUint8(0x66);
   EmitUint8(0x0F);
   EmitUint8(0x54);
   EmitXmmRegisterOperand(dst, 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);
   EmitUint8(0x0F);
   EmitUint8(0x3A);
   EmitUint8(0x21);
   EmitXmmRegisterOperand(dst, src);
   EmitUint8(imm.value());
 }
 
 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);
   EmitUint8(0x0F);
   EmitUint8(0x3A);
   EmitUint8(0x21);
   EmitOperand(dst, src);
   EmitUint8(imm.value());
 }
 
 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);
     EmitUint8(0xC4);
     EmitXmmRegisterOperand(dst, XmmRegister(src));
     EmitUint8(imm.value());
   } else {
     EmitUint8(0x66);
     EmitUint8(0x0F);
     EmitUint8(0x3A);
     EmitUint8(isByteSizedType(Ty) ? 0x20 : 0x22);
     EmitXmmRegisterOperand(dst, XmmRegister(src));
     EmitUint8(imm.value());
   }
 }
 
 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);
     EmitUint8(0xC4);
     EmitOperand(dst, src);
     EmitUint8(imm.value());
   } else {
     EmitUint8(0x66);
     EmitUint8(0x0F);
     EmitUint8(0x3A);
     EmitUint8(isByteSizedType(Ty) ? 0x20 : 0x22);
     EmitOperand(dst, src);
     EmitUint8(imm.value());
   }
 }
 
 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);
     EmitUint8(0xC5);
     EmitXmmRegisterOperand(XmmRegister(dst), src);
     EmitUint8(imm.value());
   } else {
     EmitUint8(0x66);
     EmitUint8(0x0F);
     EmitUint8(0x3A);
@@ skipping 201 matching lines @@
   EmitOperand(reg, addr);
 }
 
 void AssemblerX86::test(Type Ty, GPRRegister reg, const Immediate &immediate) {
   AssemblerBuffer::EnsureCapacity ensured(&buffer_);
   // For registers that have a byte variant (EAX, EBX, ECX, and EDX)
   // we only test the byte register to keep the encoding short.
   // 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);
     } else {
       EmitUint8(0xF6);
       EmitUint8(0xC0 + reg);
     }
     EmitUint8(immediate.value() & 0xFF);
   } else if (reg == RegX8632::Encoded_Reg_eax) {
     // Use short form if the destination is EAX.
     if (Ty == IceType_i16)
       EmitOperandSizeOverride();
     EmitUint8(0xA9);
     EmitImmediate(Ty, immediate);
   } else {
     if (Ty == IceType_i16)
       EmitOperandSizeOverride();
     EmitUint8(0xF7);
     EmitRegisterOperand(0, reg);
     EmitImmediate(Ty, immediate);
   }
 }
 
 void AssemblerX86::test(Type Ty, const Address &addr,
                         const Immediate &immediate) {
   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);
     EmitUint8(immediate.value() & 0xFF);
   } else {
     if (Ty == IceType_i16)
       EmitOperandSizeOverride();
     EmitUint8(0xF7);
     EmitOperand(0, addr);
     EmitImmediate(Ty, immediate);
@@ skipping 956 matching lines @@
   assert(shifter == RegX8632::Encoded_Reg_ecx);
   (void)shifter;
   if (Ty == IceType_i16)
     EmitOperandSizeOverride();
   EmitUint8(isByteSizedArithType(Ty) ? 0xD2 : 0xD3);
   EmitOperand(rm, operand);
 }
 
 } // end of namespace x86
 } // end of namespace Ice