[turbofan] Byte and word memory operands in x64 cmp/test. Fixes arithmetic_op_8 in assembler-x64.cc

Currently, if the size of two cmp or test operands is a byte or a word, we sign-extend or zero-extend each of them into a 32-bit register before doing the comparison, even when the conditions for the use of a memory operand are met.

This CL makes it possible to load only one of them into a register and address the other as a memory operand.

Meanwhile, comparisons between Uint8 values in the string relational comparison stubs are done with Uint32LessThan (previously we were always zero-extending the byte to a 32-bit value, so signed comparison was alright).

Found that Assembler::arithmetic_op_8(byte, Register, const Operand&) wasn't taking the Operand's rex_ field into account, so I fixed that too.

BUG=

Review URL: https://codereview.chromium.org/1780193003

Cr-Commit-Position: refs/heads/master@{#34862}

src/code-stubs.cc

@@ -1853,8 +1853,8 @@ void GenerateStringRelationalComparison(compiler::CodeStubAssembler* assembler,
           assembler->Goto(&loop);
 
           assembler->Bind(&if_valueisnotsame);
-          assembler->BranchIfInt32LessThan(lhs_value, rhs_value, &if_less,
-                                           &if_greater);
+          assembler->BranchIf(assembler->Uint32LessThan(lhs_value, rhs_value),
+                              &if_less, &if_greater);
         }
 
         assembler->Bind(&if_done);

src/compiler/code-stub-assembler.cc

@@ -464,6 +464,16 @@ Node* CodeStubAssembler::BitFieldDecode(Node* word32, uint32_t shift,
       raw_assembler_->Int32Constant(shift));
 }
 
+void CodeStubAssembler::BranchIf(Node* condition, Label* if_true,
+                                 Label* if_false) {
+  Label if_condition_true(this), if_condition_false(this);
+  Branch(condition, &if_condition_true, &if_condition_false);
+  Bind(&if_condition_true);
+  Goto(if_true);
+  Bind(&if_condition_false);
+  Goto(if_false);
+}
+
 void CodeStubAssembler::BranchIfInt32LessThan(Node* a, Node* b, Label* if_true,
                                               Label* if_false) {
   Label if_lessthan(this), if_notlessthan(this);

src/compiler/code-stub-assembler.h

@@ -55,6 +55,7 @@ class Schedule;
   V(Int32GreaterThanOrEqual)                  \
   V(Int32LessThan)                            \
   V(Int32LessThanOrEqual)                     \
+  V(Uint32LessThan)                           \
   V(WordEqual)                                \
   V(WordNotEqual)                             \
   V(WordOr)                                   \
@@ -308,6 +309,7 @@ class CodeStubAssembler {
 
   // Branching helpers.
   // TODO(danno): Can we be more cleverish wrt. edge-split?
+  void BranchIf(Node* condition, Label* if_true, Label* if_false);
   void BranchIfInt32LessThan(Node* a, Node* b, Label* if_true, Label* if_false);
   void BranchIfSmiLessThan(Node* a, Node* b, Label* if_true, Label* if_false);
   void BranchIfSmiLessThanOrEqual(Node* a, Node* b, Label* if_true,

src/compiler/x64/code-generator-x64.cc

@@ -839,12 +839,24 @@ void CodeGenerator::AssembleArchInstruction(Instruction* instr) {
     case kX64And:
       ASSEMBLE_BINOP(andq);
       break;
+    case kX64Cmp8:
+      ASSEMBLE_COMPARE(cmpb);
+      break;
+    case kX64Cmp16:
+      ASSEMBLE_COMPARE(cmpw);
+      break;
     case kX64Cmp32:
       ASSEMBLE_COMPARE(cmpl);
       break;
     case kX64Cmp:
       ASSEMBLE_COMPARE(cmpq);
       break;
+    case kX64Test8:
+      ASSEMBLE_COMPARE(testb);
+      break;
+    case kX64Test16:
+      ASSEMBLE_COMPARE(testw);
+      break;
     case kX64Test32:
       ASSEMBLE_COMPARE(testl);
       break;

src/compiler/x64/instruction-codes-x64.h

@@ -18,8 +18,12 @@ namespace compiler {
   V(X64And32)                      \
   V(X64Cmp)                        \
   V(X64Cmp32)                      \
+  V(X64Cmp16)                      \
+  V(X64Cmp8)                       \
   V(X64Test)                       \
   V(X64Test32)                     \
+  V(X64Test16)                     \
+  V(X64Test8)                      \
   V(X64Or)                         \
   V(X64Or32)                       \
   V(X64Xor)                        \
@@ -139,7 +143,6 @@ namespace compiler {
   V(X64Poke)                       \
   V(X64StackCheck)
 
-
 // Addressing modes represent the "shape" of inputs to an instruction.
 // Many instructions support multiple addressing modes. Addressing modes
 // are encoded into the InstructionCode of the instruction and tell the

src/compiler/x64/instruction-scheduler-x64.cc

@@ -20,8 +20,12 @@ int InstructionScheduler::GetTargetInstructionFlags(
     case kX64And32:
     case kX64Cmp:
     case kX64Cmp32:
+    case kX64Cmp16:
+    case kX64Cmp8:
     case kX64Test:
     case kX64Test32:
+    case kX64Test16:
+    case kX64Test8:
     case kX64Or:
     case kX64Or32:
     case kX64Xor:

src/compiler/x64/instruction-selector-x64.cc

@@ -43,11 +43,22 @@ class X64OperandGenerator final : public OperandGenerator {
     }
     MachineRepresentation rep =
         LoadRepresentationOf(input->op()).representation();
-    if (rep == MachineRepresentation::kWord64 ||
-        rep == MachineRepresentation::kTagged) {
-      return opcode == kX64Cmp || opcode == kX64Test;
-    } else if (rep == MachineRepresentation::kWord32) {
-      return opcode == kX64Cmp32 || opcode == kX64Test32;
+    switch (opcode) {
+      case kX64Cmp:
+      case kX64Test:
+        return rep == MachineRepresentation::kWord64 ||
+               rep == MachineRepresentation::kTagged;
+      case kX64Cmp32:
+      case kX64Test32:
+        return rep == MachineRepresentation::kWord32;
+      case kX64Cmp16:
+      case kX64Test16:
+        return rep == MachineRepresentation::kWord16;
+      case kX64Cmp8:
+      case kX64Test8:
+        return rep == MachineRepresentation::kWord8;
+      default:
+        break;
     }
     return false;
   }
@@ -1416,6 +1427,37 @@ void VisitCompare(InstructionSelector* selector, InstructionCode opcode,
   VisitCompare(selector, opcode, g.UseRegister(left), g.Use(right), cont);
 }
 
+// Tries to match the size of the given opcode to that of the operands, if
+// possible.
+InstructionCode TryNarrowOpcodeSize(InstructionCode opcode, Node* left,
+                                    Node* right) {
+  if (opcode != kX64Cmp32 && opcode != kX64Test32) {
+    return opcode;
+  }
+  // Currently, if one of the two operands is not a Load, we don't know what its
+  // machine representation is, so we bail out.
+  // TODO(epertoso): we can probably get some size information out of immediates
+  // and phi nodes.
+  if (left->opcode() != IrOpcode::kLoad || right->opcode() != IrOpcode::kLoad) {
+    return opcode;
+  }
+  // If the load representations don't match, both operands will be
+  // zero/sign-extended to 32bit.
+  LoadRepresentation left_representation = LoadRepresentationOf(left->op());
+  if (left_representation != LoadRepresentationOf(right->op())) {
+    return opcode;
+  }
+  switch (left_representation.representation()) {
+    case MachineRepresentation::kBit:
+    case MachineRepresentation::kWord8:
+      return opcode == kX64Cmp32 ? kX64Cmp8 : kX64Test8;
+    case MachineRepresentation::kWord16:
+      return opcode == kX64Cmp32 ? kX64Cmp16 : kX64Test16;
+    default:
+      return opcode;
+  }
+}
+
 // Shared routine for multiple word compare operations.
 void VisitWordCompare(InstructionSelector* selector, Node* node,
                       InstructionCode opcode, FlagsContinuation* cont) {
@@ -1423,6 +1465,8 @@ void VisitWordCompare(InstructionSelector* selector, Node* node,
   Node* left = node->InputAt(0);
   Node* right = node->InputAt(1);
 
+  opcode = TryNarrowOpcodeSize(opcode, left, right);
+
   // If one of the two inputs is an immediate, make sure it's on the right, or
   // if one of the two inputs is a memory operand, make sure it's on the left.
   if ((!g.CanBeImmediate(right) && g.CanBeImmediate(left)) ||

src/x64/assembler-x64.cc

@@ -523,8 +523,9 @@ void Assembler::arithmetic_op_16(byte opcode,
 void Assembler::arithmetic_op_8(byte opcode, Register reg, const Operand& op) {
   EnsureSpace ensure_space(this);
   if (!reg.is_byte_register()) {
-    // Register is not one of al, bl, cl, dl.  Its encoding needs REX.
-    emit_rex_32(reg);
+    emit_rex_32(reg, op);
+  } else {
+    emit_optional_rex_32(reg, op);
   }
   emit(opcode);
   emit_operand(reg, op);
@@ -2015,6 +2016,50 @@ void Assembler::testb(const Operand& op, Register reg) {
   emit_operand(reg, op);
 }
 
+void Assembler::testw(Register dst, Register src) {
+  EnsureSpace ensure_space(this);
+  emit(0x66);
+  if (src.low_bits() == 4) {
+    emit_rex_32(src, dst);
+  }
+  emit(0x85);
+  emit_modrm(src, dst);
+}
+
+void Assembler::testw(Register reg, Immediate mask) {
+  DCHECK(is_int16(mask.value_) || is_uint16(mask.value_));
+  EnsureSpace ensure_space(this);
+  emit(0x66);
+  if (reg.is(rax)) {
+    emit(0xA9);
+    emit(mask.value_);
+  } else {
+    if (reg.low_bits() == 4) {
+      emit_rex_32(reg);
+    }
+    emit(0xF7);
+    emit_modrm(0x0, reg);
+    emit(mask.value_);
+  }
+}
+
+void Assembler::testw(const Operand& op, Immediate mask) {
+  DCHECK(is_int16(mask.value_) || is_uint16(mask.value_));
+  EnsureSpace ensure_space(this);
+  emit(0x66);
+  emit_optional_rex_32(rax, op);
+  emit(0xF7);
+  emit_operand(rax, op);
+  emit(mask.value_);
+}
+
+void Assembler::testw(const Operand& op, Register reg) {
+  EnsureSpace ensure_space(this);
+  emit(0x66);
+  emit_optional_rex_32(reg, op);
+  emit(0x85);
+  emit_operand(rax, op);
+}
 
 void Assembler::emit_test(Register dst, Register src, int size) {
   EnsureSpace ensure_space(this);

src/x64/assembler-x64.h

@@ -773,6 +773,10 @@ class Assembler : public AssemblerBase {
     arithmetic_op_16(0x39, src, dst);
   }
 
+  void testb(Register reg, const Operand& op) { testb(op, reg); }
+
+  void testw(Register reg, const Operand& op) { testw(op, reg); }
+
   void andb(Register dst, Immediate src) {
     immediate_arithmetic_op_8(0x4, dst, src);
   }
@@ -848,6 +852,11 @@ class Assembler : public AssemblerBase {
   void testb(const Operand& op, Immediate mask);
   void testb(const Operand& op, Register reg);
 
+  void testw(Register dst, Register src);
+  void testw(Register reg, Immediate mask);
+  void testw(const Operand& op, Immediate mask);
+  void testw(const Operand& op, Register reg);
+
   // Bit operations.
   void bt(const Operand& dst, Register src);
   void bts(const Operand& dst, Register src);