[X86] Avoid unecessary opsize byte in segment move to memory

[niravd]

Summary

Segment moves to memory are always 16-bit. Remove invalid 32 and 64 bit variants.

Fixes PR34478.

[craig.topper]

gas seems to error on movl %fs,(%rsi) and movl (%rsi), %fs

I wonder if we shouldn't just delete MOV32ms and MOV64ms from InstrSystem.td and change MOV16ms to not have OpSize16. They're declared with mem32 and mem64 anyway which is clearly wrong. Same for MOV32sm and MOV64sm.

[niravd]

Add OpSizeIgnore modifier to allow consumption of optional 0x66 bytes and remove invalid instructions.

We now no longer accept movl/movq versions of these instructions but as Craig points out gas doesn't either.

llvm/lib/Target/X86/MCTargetDesc/X86BaseInfo.h

enum : uint64_t {
     FormMask       = 127,
 
     //===------------------------------------------------------------------===//
     // Actual flags...
 
     // OpSize - OpSizeFixed implies instruction never needs a 0x66 prefix.
     // OpSize16 means this is a 16-bit instruction and needs 0x66 prefix in
     // 32-bit mode. OpSize32 means this is a 32-bit instruction needs a 0x66
-    // prefix in 16-bit mode.
+    // prefix in 16-bit mode. OpSizeIgnore means that the instruction may
+    // take a optional 0x66 byte but should not emit with one.
     OpSizeShift = 7,
     OpSizeMask = 0x3 << OpSizeShift,
 
-    OpSizeFixed = 0 << OpSizeShift,
-    OpSize16    = 1 << OpSizeShift,
-    OpSize32    = 2 << OpSizeShift,
+    OpSizeFixed  = 0 << OpSizeShift,
+    OpSize16     = 1 << OpSizeShift,
+    OpSize32     = 2 << OpSizeShift,
+    OpSizeIgnore = 3 << OpSizeShift,
 
     // AsSize - AdSizeX implies this instruction determines its need of 0x67
     // prefix from a normal ModRM memory operand. The other types indicate that
     // an operand is encoded with a specific width and a prefix is needed if
     // it differs from the current mode.
     AdSizeShift = OpSizeShift + 2,
     AdSizeMask  = 0x3 << AdSizeShift,
 

llvm/lib/Target/X86/X86InstrFormats.td

 def EncVEX    : Encoding<1>;
 def EncXOP    : Encoding<2>;
 def EncEVEX   : Encoding<3>;
 
 // Operand size for encodings that change based on mode.
 class OperandSize<bits<2> val> {
   bits<2> Value = val;
 }
-def OpSizeFixed : OperandSize<0>; // Never needs a 0x66 prefix.
-def OpSize16    : OperandSize<1>; // Needs 0x66 prefix in 32-bit mode.
-def OpSize32    : OperandSize<2>; // Needs 0x66 prefix in 16-bit mode.
+def OpSizeFixed  : OperandSize<0>; // Never needs a 0x66 prefix.
+def OpSize16     : OperandSize<1>; // Needs 0x66 prefix in 32-bit mode.
+def OpSize32     : OperandSize<2>; // Needs 0x66 prefix in 16-bit mode.
+def OpSizeIgnore : OperandSize<3>; // Takes 0x66 prefix, never emits.
 
 // Address size for encodings that change based on mode.
 class AddressSize<bits<2> val> {
   bits<2> Value = val;
 }
 def AdSizeX  : AddressSize<0>; // Address size determined using addr operand.
 def AdSize16 : AddressSize<1>; // Encodes a 16-bit address.
 def AdSize32 : AddressSize<2>; // Encodes a 32-bit address.
 def AdSize64 : AddressSize<3>; // Encodes a 64-bit address.
 
 // Prefix byte classes which are used to indicate to the ad-hoc machine code
 // emitter that various prefix bytes are required.
 class OpSize16 { OperandSize OpSize = OpSize16; }
 class OpSize32 { OperandSize OpSize = OpSize32; }
+class OpSizeIgnore { OperandSize OpSize = OpSizeIgnore; }
 class AdSize16 { AddressSize AdSize = AdSize16; }
 class AdSize32 { AddressSize AdSize = AdSize32; }
 class AdSize64 { AddressSize AdSize = AdSize64; }
 class REX_W  { bit hasREX_WPrefix = 1; }
 class LOCK   { bit hasLockPrefix = 1; }
 class REP    { bit hasREPPrefix = 1; }
 class TB     { Map OpMap = TB; }
 class T8     { Map OpMap = T8; }

llvm/lib/Target/X86/X86InstrInfo.td

 def : InstAlias<"callw\t$seg, $off", (FARCALL16i i16imm:$off, i16imm:$seg)>, Requires<[Not64BitMode]>;
 def : InstAlias<"jmpw\t$seg, $off",  (FARJMP16i  i16imm:$off, i16imm:$seg)>, Requires<[Not64BitMode]>;
 def : InstAlias<"calll\t$seg, $off", (FARCALL32i i32imm:$off, i16imm:$seg)>, Requires<[Not64BitMode]>;
 def : InstAlias<"jmpl\t$seg, $off",  (FARJMP32i  i32imm:$off, i16imm:$seg)>, Requires<[Not64BitMode]>;
 
 // Force mov without a suffix with a segment and mem to prefer the 'l' form of
 // the move.  All segment/mem forms are equivalent, this has the shortest
 // encoding.
-def : InstAlias<"mov\t{$mem, $seg|$seg, $mem}", (MOV32sm SEGMENT_REG:$seg, i32mem:$mem), 0>;
-def : InstAlias<"mov\t{$seg, $mem|$mem, $seg}", (MOV32ms i32mem:$mem, SEGMENT_REG:$seg), 0>;
+def : InstAlias<"mov\t{$mem, $seg|$seg, $mem}", (MOV16sm SEGMENT_REG:$seg, i16mem:$mem), 0>;
+def : InstAlias<"mov\t{$seg, $mem|$mem, $seg}", (MOV16ms i16mem:$mem, SEGMENT_REG:$seg), 0>;
 
 // Match 'movq <largeimm>, <reg>' as an alias for movabsq.
 def : InstAlias<"mov{q}\t{$imm, $reg|$reg, $imm}", (MOV64ri GR64:$reg, i64imm:$imm), 0>;
 
 // Match 'movq GR64, MMX' as an alias for movd.
 def : InstAlias<"movq\t{$src, $dst|$dst, $src}",
                 (MMX_MOVD64to64rr VR64:$dst, GR64:$src), 0>;
 def : InstAlias<"movq\t{$src, $dst|$dst, $src}",

llvm/lib/Target/X86/X86InstrSystem.td

 def MOV16rs : I<0x8C, MRMDestReg, (outs GR16:$dst), (ins SEGMENT_REG:$src),
                 "mov{w}\t{$src, $dst|$dst, $src}", [], IIC_MOV_REG_SR>, OpSize16;
 def MOV32rs : I<0x8C, MRMDestReg, (outs GR32:$dst), (ins SEGMENT_REG:$src),
                 "mov{l}\t{$src, $dst|$dst, $src}", [], IIC_MOV_REG_SR>, OpSize32;
 def MOV64rs : RI<0x8C, MRMDestReg, (outs GR64:$dst), (ins SEGMENT_REG:$src),
                  "mov{q}\t{$src, $dst|$dst, $src}", [], IIC_MOV_REG_SR>;
 let mayStore = 1 in {
 def MOV16ms : I<0x8C, MRMDestMem, (outs), (ins i16mem:$dst, SEGMENT_REG:$src),
-                "mov{w}\t{$src, $dst|$dst, $src}", [], IIC_MOV_MEM_SR>, OpSize16;
-def MOV32ms : I<0x8C, MRMDestMem, (outs), (ins i32mem:$dst, SEGMENT_REG:$src),
-                "mov{l}\t{$src, $dst|$dst, $src}", [], IIC_MOV_MEM_SR>, OpSize32;
-def MOV64ms : RI<0x8C, MRMDestMem, (outs), (ins i64mem:$dst, SEGMENT_REG:$src),
-                 "mov{q}\t{$src, $dst|$dst, $src}", [], IIC_MOV_MEM_SR>;
+                "mov{w}\t{$src, $dst|$dst, $src}", [], IIC_MOV_MEM_SR>, OpSizeIgnore;
 }
 def MOV16sr : I<0x8E, MRMSrcReg, (outs SEGMENT_REG:$dst), (ins GR16:$src),
                 "mov{w}\t{$src, $dst|$dst, $src}", [], IIC_MOV_SR_REG>, OpSize16;
 def MOV32sr : I<0x8E, MRMSrcReg, (outs SEGMENT_REG:$dst), (ins GR32:$src),
                 "mov{l}\t{$src, $dst|$dst, $src}", [], IIC_MOV_SR_REG>, OpSize32;
 def MOV64sr : RI<0x8E, MRMSrcReg, (outs SEGMENT_REG:$dst), (ins GR64:$src),
                  "mov{q}\t{$src, $dst|$dst, $src}", [], IIC_MOV_SR_REG>;
 let mayLoad = 1 in {
 def MOV16sm : I<0x8E, MRMSrcMem, (outs SEGMENT_REG:$dst), (ins i16mem:$src),
-                "mov{w}\t{$src, $dst|$dst, $src}", [], IIC_MOV_SR_MEM>, OpSize16;
-def MOV32sm : I<0x8E, MRMSrcMem, (outs SEGMENT_REG:$dst), (ins i32mem:$src),
-                "mov{l}\t{$src, $dst|$dst, $src}", [], IIC_MOV_SR_MEM>, OpSize32;
-def MOV64sm : RI<0x8E, MRMSrcMem, (outs SEGMENT_REG:$dst), (ins i64mem:$src),
-                 "mov{q}\t{$src, $dst|$dst, $src}", [], IIC_MOV_SR_MEM>;
+                "mov{w}\t{$src, $dst|$dst, $src}", [], IIC_MOV_SR_MEM>, OpSizeIgnore;
 }
 } // SchedRW
 
 //===----------------------------------------------------------------------===//
 // Segmentation support instructions.
 
 let SchedRW = [WriteSystem] in {
 def SWAPGS : I<0x01, MRM_F8, (outs), (ins), "swapgs", [], IIC_SWAPGS>, TB;

llvm/lib/Target/X86/X86SchedSandyBridge.td

 def: InstRW<[SBWriteResGroup48], (instregex "VMOVUPSrm")>;
 
 def SBWriteResGroup49 : SchedWriteRes<[SBPort5,SBPort23]> {
   let Latency = 6;
   let NumMicroOps = 2;
   let ResourceCycles = [1,1];
 }
 def: InstRW<[SBWriteResGroup49], (instregex "JMP(16|32|64)m")>;
-def: InstRW<[SBWriteResGroup49], (instregex "MOV64sm")>;
+def: InstRW<[SBWriteResGroup49], (instregex "MOV16sm")>;
 
 def SBWriteResGroup50 : SchedWriteRes<[SBPort23,SBPort05]> {
   let Latency = 6;
   let NumMicroOps = 2;
   let ResourceCycles = [1,1];
 }
 def: InstRW<[SBWriteResGroup50], (instregex "BT(16|32|64)mi8")>;
 

llvm/test/MC/Disassembler/X86/x86-16.txt

 0xd8 0xf0
 
 # CHECK: movl %cs, %eax
 0x66 0x8c 0xc8
 
 # CHECK: movw %cs, %ax
 0x8c 0xc8
 
-# CHECK: movl %cs, (%eax)
+# CHECK: movw %cs, (%eax)
 0x67 0x66 0x8c 0x08
 
 # CHECK: movw %cs, (%eax)
 0x67 0x8c 0x08
 
 # CHECK: movl %eax, %cs
 0x66 0x8e 0xc8
 
-# CHECK: movl (%eax), %cs
+# CHECK: movw (%eax), %cs
 0x67 0x66 0x8e 0x08
 
 # CHECK: movw (%eax), %cs
 0x67 0x8e 0x08
 
 # CHECKX: movl %cr0, %eax
 0x0f 0x20 0xc0
 

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

 // CHECK: movl  %cs, %eax
 // CHECK:  encoding: [0x66,0x8c,0xc8]
         movl %cs, %eax
 
 // CHECK: movw  %cs, %ax
 // CHECK:  encoding: [0x8c,0xc8]
         movw %cs, %ax
 
-// CHECK: movl  %cs, (%eax)
-// CHECK:  encoding: [0x67,0x66,0x8c,0x08]
-        movl %cs, (%eax)
+// CHECK: movw  %cs, (%eax)
+// CHECK:  encoding: [0x67,0x8c,0x08]
+        mov %cs, (%eax)
 
 // CHECK: movw  %cs, (%eax)
 // CHECK:  encoding: [0x67,0x8c,0x08]
         movw %cs, (%eax)
 
 // 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]
+        mov (%eax), %cs
 
 // CHECK: movw  (%eax), %cs
 // CHECK:  encoding: [0x67,0x8e,0x08]
         movw (%eax), %cs
 
 // CHECK: movl  %cr0, %eax
 // CHECK:  encoding: [0x0f,0x20,0xc0]
         movl %cr0,%eax

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

 // CHECK: movl  %cs, %eax
 // CHECK:  encoding: [0x8c,0xc8]
         movl %cs, %eax
 
 // CHECK: movw  %cs, %ax
 // CHECK:  encoding: [0x66,0x8c,0xc8]
         movw %cs, %ax
 
-// CHECK: movl  %cs, (%eax)
+// CHECK: movw  %cs, (%eax)
 // CHECK:  encoding: [0x8c,0x08]
-        movl %cs, (%eax)
+        mov %cs, (%eax)
 
 // CHECK: movw  %cs, (%eax)
-// CHECK:  encoding: [0x66,0x8c,0x08]
+// CHECK:  encoding: [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: movw  (%eax), %cs
 // CHECK:  encoding: [0x8e,0x08]
-        movl (%eax), %cs
+        mov (%eax), %cs
 
 // CHECK: movw  (%eax), %cs
-// CHECK:  encoding: [0x66,0x8e,0x08]
+// CHECK:  encoding: [0x8e,0x08]
         movw (%eax), %cs
 
 // radr://8033374
 // CHECK: movl  %cr0, %eax
 // CHECK:  encoding: [0x0f,0x20,0xc0]
         movl %cr0,%eax
 
 // CHECK: movl  %cr1, %eax

llvm/test/MC/X86/x86-64.s

// CHECK:  encoding: [0x0f,0x01,0x48,0x04]
           sidt  4(%rax)
 
 // CHECK: sidtq 4(%rax)
 // CHECK:  encoding: [0x0f,0x01,0x48,0x04]
           sidtq 4(%rax)
 
 
 // rdar://8208615
-mov (%rsi), %gs  // CHECK: movl (%rsi), %gs # encoding: [0x8e,0x2e]
-mov %gs, (%rsi)  // CHECK: movl  %gs, (%rsi) # encoding: [0x8c,0x2e]
+mov (%rsi), %gs  // CHECK: movw (%rsi), %gs # encoding: [0x8e,0x2e]
+mov %gs, (%rsi)  // CHECK: movw  %gs, (%rsi) # encoding: [0x8c,0x2e]
 
 
 // rdar://8431864
 //CHECK: divb %bl
 //CHECK: divw %bx
 //CHECK: divl %ecx
 //CHECK: divl 3735928559(%ebx,%ecx,8)
 //CHECK: divl 69