[X86] Improve code size on X86 segment moves

[niravd]

Summary

Moves of a value to a segment register from a 16-bit register is
equivalent to one from it's corresponding 32-bit register. Match gas's
behavior and rewrite instructions to the shorter of equivalent forms.

[rnk]

I dono, if someone really writes movl %eax, %cs I would assume they really want the prefix, right? Obviously gas does something else, but that's surprising to me.

[niravd]

Adding Joerg's and my comments below because phabricator dropped them on the floor.

Nirav said:

The ISA doesn't actually have a 32-bit mov instruction of this form (though all the processors I've seen accept the bytes pattern associated with the 32-bit variant). I doubt that anyone
dealing with explicit segment twiddling ever wants the effectless prefix added. AFAIK these instructions are only ever emitted via explicit assembly so they it would be easy enough to
add the byte via an explicit ".byte" directive.

Ideally we'd only have the 16-bit mov instruction which accepts but ignores the OpSize prefix, but we need to accept the text syntax "mov %eax, %fs" to handle what objdump dumps,
hence this workaround.

Joerg Sonnenberger said:

Sometimes people want explicitly sized assembler for later runtime
patching. I'm not sure it matters in this case.

Joerg

[rnk]

lgtm I'm OK doing what gas does here for now.

lib/Target/X86/AsmParser/X86AsmParser.cpp

2346 ↗

(On Diff #66812)

Please make this and the comment below complete sentences.

2352 ↗

(On Diff #66812)

s/reg/Reg/

llvm/trunk/lib/Target/X86/AsmParser/X86AsmParser.cpp

if (IsFp && Operands.size() == 1) {
     const char *Repl = StringSwitch<const char *>(Name)
       .Case("fsub", "fsubp")
       .Case("fdiv", "fdivp")
       .Case("fsubr", "fsubrp")
       .Case("fdivr", "fdivrp");
     static_cast<X86Operand &>(*Operands[0]).setTokenValue(Repl);
   }
 
+  // Moving a 32 or 16 bit value into a segment register has the same
+  // behavior. Modify such instructions to always take shorter form.
+  if ((Name == "mov" || Name == "movw" || Name == "movl") &&
+      (Operands.size() == 3)) {
+    X86Operand &Op1 = (X86Operand &)*Operands[1];
+    X86Operand &Op2 = (X86Operand &)*Operands[2];
+    SMLoc Loc = Op1.getEndLoc();
+    if (Op1.isReg() && Op2.isReg() &&
+        X86MCRegisterClasses[X86::SEGMENT_REGRegClassID].contains(
+            Op2.getReg()) &&
+        (X86MCRegisterClasses[X86::GR16RegClassID].contains(Op1.getReg()) ||
+         X86MCRegisterClasses[X86::GR32RegClassID].contains(Op1.getReg()))) {
+      // Change instruction name to match new instruction.
+      if (Name != "mov" && Name[3] == (is16BitMode() ? 'l' : 'w')) {
+        Name = is16BitMode() ? "movw" : "movl";
+        Operands[0] = X86Operand::CreateToken(Name, NameLoc);
+      }
+      // Select the correct equivalent 16-/32-bit source register.
+      unsigned Reg =
+          getX86SubSuperRegisterOrZero(Op1.getReg(), is16BitMode() ? 16 : 32);
+      Operands[1] = X86Operand::CreateReg(Reg, Loc, Loc);
+    }
+  }
+
   // This is a terrible hack to handle "out[s]?[bwl]? %al, (%dx)" ->
   // "outb %al, %dx".  Out doesn't take a memory form, but this is a widely
   // documented form in various unofficial manuals, so a lot of code uses it.
   if ((Name == "outb" || Name == "outsb" || Name == "outw" || Name == "outsw" ||
        Name == "outl" || Name == "outsl" || Name == "out" || Name == "outs") &&
       Operands.size() == 3) {
     X86Operand &Op = (X86Operand &)*Operands.back();
     if (Op.isMem() && Op.Mem.SegReg == 0 &&

llvm/trunk/test/MC/X86/x86-16.s

 // CHECK: movl  %cs, (%eax)
 // CHECK:  encoding: [0x67,0x66,0x8c,0x08]
         movl %cs, (%eax)
 
 // CHECK: movw  %cs, (%eax)
 // CHECK:  encoding: [0x67,0x8c,0x08]
         movw %cs, (%eax)
 
-// CHECK: movl  %eax, %cs
-// CHECK:  encoding: [0x66,0x8e,0xc8]
+// CHECK: movw  %ax, %cs
+// CHECK:  encoding: [0x8e,0xc8]
         movl %eax, %cs
 
+// CHECK: movw  %ax, %cs
+// CHECK:  encoding: [0x8e,0xc8]
+        mov %eax, %cs 
+
+// CHECK: movw  %ax, %cs
+// CHECK:  encoding: [0x8e,0xc8]
+        movw %ax, %cs
+
+// CHECK: movw  %ax, %cs
+// CHECK:  encoding: [0x8e,0xc8]
+        mov %ax, %cs    
+  
 // CHECK: movl  (%eax), %cs
 // CHECK:  encoding: [0x67,0x66,0x8e,0x08]
         movl (%eax), %cs
 
 // CHECK: movw  (%eax), %cs
 // CHECK:  encoding: [0x67,0x8e,0x08]
         movw (%eax), %cs
 

llvm/trunk/test/MC/X86/x86-32.s

 // CHECK: movw  %cs, (%eax)
 // CHECK:  encoding: [0x66,0x8c,0x08]
         movw %cs, (%eax)
 
 // CHECK: movl  %eax, %cs
 // CHECK:  encoding: [0x8e,0xc8]
         movl %eax, %cs
 
+// CHECK: movl  %eax, %cs
+// CHECK:  encoding: [0x8e,0xc8]
+        movw %ax, %cs
+
+// CHECK: movl  %eax, %cs
+// CHECK:  encoding: [0x8e,0xc8]
+        mov %eax, %cs
+
+// CHECK: movl  %eax, %cs
+// CHECK:  encoding: [0x8e,0xc8]
+        mov %ax, %cs
+
 // CHECK: movl  (%eax), %cs
 // CHECK:  encoding: [0x8e,0x08]
         movl (%eax), %cs
 
 // CHECK: movw  (%eax), %cs
 // CHECK:  encoding: [0x66,0x8e,0x08]
         movw (%eax), %cs