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

Craig Topper via llvm-commits llvm-commits at lists.llvm.org
Tue Nov 21 11:27:54 PST 2017


Nirav, if you want to re-land your patch, I think you just need to remove
the "word ptr" from the clang test. Before your change the matching was
ambiguous due to multiple segment move instructions with different memory
sizes. The frontend parsing process detected the ambiguity and then
inserted "word ptr" based on the the size of the variable being passed to
the instruction to resolve the ambiguity. Now that there is only one
instruction this process doesn't happen anymore.

~Craig

2017-11-20 16:16 GMT-08:00 Richard Trieu via llvm-commits <
llvm-commits at lists.llvm.org>:

> This patch has been reverted in r318710 since it was causing a Clang test
> failure.
>
> llvm/tools/clang/test/CodeGen/ms-inline-asm.c:580:11: error: expected
> string not found in input
> // CHECK: mov cs, word ptr $0
>           ^
> <stdin>:402:34: note: scanning from here
> define void @t41(i16 zeroext %a) #0 {
>                                  ^
> <stdin>:406:41: note: possible intended match here
>  call void asm sideeffect inteldialect "mov cs, $0\0A\09mov ds,
> $1\0A\09mov es, $2\0A\09mov fs, $3\0A\09mov gs, $4\0A\09mov ss, $5",
> "*m,*m,*m,*m,*m,*m,~{dirflag},~{fpsr},~{flags}"(i16* %a.addr, i16*
> %a.addr, i16* %a.addr, i16* %a.addr, i16* %a.addr, i16* %a.addr) #3,
> !srcloc !70
>                                         ^
>
>
> On Mon, Nov 20, 2017 at 10:38 AM, Nirav Dave via llvm-commits <
> llvm-commits at lists.llvm.org> wrote:
>
>> Author: niravd
>> Date: Mon Nov 20 10:38:55 2017
>> New Revision: 318678
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=318678&view=rev
>> Log:
>> [X86] Avoid unecessary opsize byte in segment move to memory
>>
>> Summary:
>>
>> Segment moves to memory are always 16-bit. Remove invalid 32 and 64
>> bit variants.
>>
>> Fixes PR34478.
>>
>> Reviewers: rnk, craig.topper
>>
>> Subscribers: llvm-commits, hiraditya
>>
>> Differential Revision: https://reviews.llvm.org/D39847
>>
>> Modified:
>>     llvm/trunk/lib/Target/X86/MCTargetDesc/X86BaseInfo.h
>>     llvm/trunk/lib/Target/X86/X86InstrFormats.td
>>     llvm/trunk/lib/Target/X86/X86InstrInfo.td
>>     llvm/trunk/lib/Target/X86/X86InstrSystem.td
>>     llvm/trunk/lib/Target/X86/X86SchedSandyBridge.td
>>     llvm/trunk/test/MC/Disassembler/X86/x86-16.txt
>>     llvm/trunk/test/MC/X86/x86-16.s
>>     llvm/trunk/test/MC/X86/x86-32.s
>>     llvm/trunk/test/MC/X86/x86-64.s
>>
>> Modified: llvm/trunk/lib/Target/X86/MCTargetDesc/X86BaseInfo.h
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X8
>> 6/MCTargetDesc/X86BaseInfo.h?rev=318678&r1=318677&r2=318678&view=diff
>> ============================================================
>> ==================
>> --- llvm/trunk/lib/Target/X86/MCTargetDesc/X86BaseInfo.h (original)
>> +++ llvm/trunk/lib/Target/X86/MCTargetDesc/X86BaseInfo.h Mon Nov 20
>> 10:38:55 2017
>> @@ -366,13 +366,15 @@ namespace X86II {
>>      // 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
>>
>> Modified: llvm/trunk/lib/Target/X86/X86InstrFormats.td
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X8
>> 6/X86InstrFormats.td?rev=318678&r1=318677&r2=318678&view=diff
>> ============================================================
>> ==================
>> --- llvm/trunk/lib/Target/X86/X86InstrFormats.td (original)
>> +++ llvm/trunk/lib/Target/X86/X86InstrFormats.td Mon Nov 20 10:38:55 2017
>> @@ -157,9 +157,10 @@ def EncEVEX   : Encoding<3>;
>>  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> {
>> @@ -174,6 +175,7 @@ def AdSize64 : AddressSize<3>; // Encode
>>  // 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; }
>>
>> Modified: llvm/trunk/lib/Target/X86/X86InstrInfo.td
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X8
>> 6/X86InstrInfo.td?rev=318678&r1=318677&r2=318678&view=diff
>> ============================================================
>> ==================
>> --- llvm/trunk/lib/Target/X86/X86InstrInfo.td (original)
>> +++ llvm/trunk/lib/Target/X86/X86InstrInfo.td Mon Nov 20 10:38:55 2017
>> @@ -3157,8 +3157,8 @@ def : InstAlias<"jmpl\t$seg, $off",  (FA
>>  // 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>;
>>
>> Modified: llvm/trunk/lib/Target/X86/X86InstrSystem.td
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X8
>> 6/X86InstrSystem.td?rev=318678&r1=318677&r2=318678&view=diff
>> ============================================================
>> ==================
>> --- llvm/trunk/lib/Target/X86/X86InstrSystem.td (original)
>> +++ llvm/trunk/lib/Target/X86/X86InstrSystem.td Mon Nov 20 10:38:55 2017
>> @@ -175,11 +175,7 @@ def MOV64rs : RI<0x8C, MRMDestReg, (outs
>>                   "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;
>> @@ -189,11 +185,7 @@ def MOV64sr : RI<0x8E, MRMSrcReg, (outs
>>                   "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
>>
>>
>> Modified: llvm/trunk/lib/Target/X86/X86SchedSandyBridge.td
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X8
>> 6/X86SchedSandyBridge.td?rev=318678&r1=318677&r2=318678&view=diff
>> ============================================================
>> ==================
>> --- llvm/trunk/lib/Target/X86/X86SchedSandyBridge.td (original)
>> +++ llvm/trunk/lib/Target/X86/X86SchedSandyBridge.td Mon Nov 20 10:38:55
>> 2017
>> @@ -1550,7 +1550,7 @@ def SBWriteResGroup49 : SchedWriteRes<[S
>>    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;
>>
>> Modified: llvm/trunk/test/MC/Disassembler/X86/x86-16.txt
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/MC/Disas
>> sembler/X86/x86-16.txt?rev=318678&r1=318677&r2=318678&view=diff
>> ============================================================
>> ==================
>> --- llvm/trunk/test/MC/Disassembler/X86/x86-16.txt (original)
>> +++ llvm/trunk/test/MC/Disassembler/X86/x86-16.txt Mon Nov 20 10:38:55
>> 2017
>> @@ -207,7 +207,7 @@
>>  # CHECK: movw %cs, %ax
>>  0x8c 0xc8
>>
>> -# CHECK: movl %cs, (%eax)
>> +# CHECK: movw %cs, (%eax)
>>  0x67 0x66 0x8c 0x08
>>
>>  # CHECK: movw %cs, (%eax)
>> @@ -216,7 +216,7 @@
>>  # CHECK: movl %eax, %cs
>>  0x66 0x8e 0xc8
>>
>> -# CHECK: movl (%eax), %cs
>> +# CHECK: movw (%eax), %cs
>>  0x67 0x66 0x8e 0x08
>>
>>  # CHECK: movw (%eax), %cs
>>
>> Modified: llvm/trunk/test/MC/X86/x86-16.s
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/MC/X86/x
>> 86-16.s?rev=318678&r1=318677&r2=318678&view=diff
>> ============================================================
>> ==================
>> --- llvm/trunk/test/MC/X86/x86-16.s (original)
>> +++ llvm/trunk/test/MC/X86/x86-16.s Mon Nov 20 10:38:55 2017
>> @@ -248,9 +248,9 @@ cmovnae     %bx,%bx
>>  // 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]
>> @@ -272,9 +272,9 @@ cmovnae     %bx,%bx
>>  // 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]
>>
>> Modified: llvm/trunk/test/MC/X86/x86-32.s
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/MC/X86/x
>> 86-32.s?rev=318678&r1=318677&r2=318678&view=diff
>> ============================================================
>> ==================
>> --- llvm/trunk/test/MC/X86/x86-32.s (original)
>> +++ llvm/trunk/test/MC/X86/x86-32.s Mon Nov 20 10:38:55 2017
>> @@ -355,12 +355,12 @@ cmovnae   %bx,%bx
>>  // 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
>> @@ -379,12 +379,12 @@ cmovnae   %bx,%bx
>>  // 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
>>
>> Modified: llvm/trunk/test/MC/X86/x86-64.s
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/MC/X86/x
>> 86-64.s?rev=318678&r1=318677&r2=318678&view=diff
>> ============================================================
>> ==================
>> --- llvm/trunk/test/MC/X86/x86-64.s (original)
>> +++ llvm/trunk/test/MC/X86/x86-64.s Mon Nov 20 10:38:55 2017
>> @@ -1082,8 +1082,8 @@ decl %eax // CHECK:       decl    %eax # encoding
>>
>>
>>  // 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
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20171121/36d6ea07/attachment.html>


More information about the llvm-commits mailing list