[llvm] r318678 - [X86] Avoid unecessary opsize byte in segment move to memory
Nirav Davé via llvm-commits
llvm-commits at lists.llvm.org
Tue Nov 21 11:33:54 PST 2017
i just reached the same conclusion and recommitted it. Thanks for the
confirmation though :).
-Nirav
2017-11-21 14:27 GMT-05:00 Craig Topper <craig.topper at gmail.com>:
> 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/d21b8ea7/attachment.html>
More information about the llvm-commits
mailing list