<div dir="ltr">i just reached the same conclusion and recommitted it. Thanks for the confirmation though :). <div><br></div><div>-Nirav</div></div><div class="gmail_extra"><br><div class="gmail_quote">2017-11-21 14:27 GMT-05:00 Craig Topper <span dir="ltr"><<a href="mailto:craig.topper@gmail.com" target="_blank">craig.topper@gmail.com</a>></span>:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">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.</div><div class="gmail_extra"><span class="HOEnZb"><font color="#888888"><br clear="all"><div><div class="m_-7347345847061338668gmail_signature" data-smartmail="gmail_signature">~Craig</div></div></font></span><div><div class="h5">
<br><div class="gmail_quote">2017-11-20 16:16 GMT-08:00 Richard Trieu via llvm-commits <span dir="ltr"><<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>></span>:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div dir="ltr">This patch has been reverted in r318710 since it was causing a Clang test failure.<div><br></div><div><div>llvm/tools/clang/test/CodeGen/<wbr>ms-inline-asm.c:580:11: error: expected string not found in input</div><div>// CHECK: mov cs, word ptr $0</div><div> ^</div><div><stdin>:402:34: note: scanning from here</div><div>define void @t41(i16 zeroext %a) #0 {</div><div> ^</div><div><stdin>:406:41: note: possible intended match here</div><div> 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},<wbr>~{fpsr},~{flags}"(i16* %a.addr, i16* %a.addr, i16* %a.addr, i16* %a.addr, i16* %a.addr, i16* %a.addr) #3, !srcloc !70</div><div> ^</div><div><br></div><div class="gmail_extra"><br><div class="gmail_quote">On Mon, Nov 20, 2017 at 10:38 AM, Nirav Dave via llvm-commits <span dir="ltr"><<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">Author: niravd<br>
Date: Mon Nov 20 10:38:55 2017<br>
New Revision: 318678<br>
<br>
URL: <a href="http://llvm.org/viewvc/llvm-project?rev=318678&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-pr<wbr>oject?rev=318678&view=rev</a><br>
Log:<br>
[X86] Avoid unecessary opsize byte in segment move to memory<br>
<br>
Summary:<br>
<br>
Segment moves to memory are always 16-bit. Remove invalid 32 and 64<br>
bit variants.<br>
<br>
Fixes PR34478.<br>
<br>
Reviewers: rnk, craig.topper<br>
<br>
Subscribers: llvm-commits, hiraditya<br>
<br>
Differential Revision: <a href="https://reviews.llvm.org/D39847" rel="noreferrer" target="_blank">https://reviews.llvm.org/D3984<wbr>7</a><br>
<br>
Modified:<br>
llvm/trunk/lib/Target/X86/MCTa<wbr>rgetDesc/X86BaseInfo.h<br>
llvm/trunk/lib/Target/X86/X86I<wbr>nstrFormats.td<br>
llvm/trunk/lib/Target/X86/X86I<wbr>nstrInfo.td<br>
llvm/trunk/lib/Target/X86/X86I<wbr>nstrSystem.td<br>
llvm/trunk/lib/Target/X86/X86S<wbr>chedSandyBridge.td<br>
llvm/trunk/test/MC/Disassemble<wbr>r/X86/x86-16.txt<br>
llvm/trunk/test/MC/X86/x86-16.<wbr>s<br>
llvm/trunk/test/MC/X86/x86-32.<wbr>s<br>
llvm/trunk/test/MC/X86/x86-64.<wbr>s<br>
<br>
Modified: llvm/trunk/lib/Target/X86/MCTa<wbr>rgetDesc/X86BaseInfo.h<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/MCTargetDesc/X86BaseInfo.h?rev=318678&r1=318677&r2=318678&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-pr<wbr>oject/llvm/trunk/lib/Target/X8<wbr>6/MCTargetDesc/X86BaseInfo.h?r<wbr>ev=318678&r1=318677&r2=318678&<wbr>view=diff</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- llvm/trunk/lib/Target/X86/MCTa<wbr>rgetDesc/X86BaseInfo.h (original)<br>
+++ llvm/trunk/lib/Target/X86/MCTa<wbr>rgetDesc/X86BaseInfo.h Mon Nov 20 10:38:55 2017<br>
@@ -366,13 +366,15 @@ namespace X86II {<br>
// OpSize - OpSizeFixed implies instruction never needs a 0x66 prefix.<br>
// OpSize16 means this is a 16-bit instruction and needs 0x66 prefix in<br>
// 32-bit mode. OpSize32 means this is a 32-bit instruction needs a 0x66<br>
- // prefix in 16-bit mode.<br>
+ // prefix in 16-bit mode. OpSizeIgnore means that the instruction may<br>
+ // take a optional 0x66 byte but should not emit with one.<br>
OpSizeShift = 7,<br>
OpSizeMask = 0x3 << OpSizeShift,<br>
<br>
- OpSizeFixed = 0 << OpSizeShift,<br>
- OpSize16 = 1 << OpSizeShift,<br>
- OpSize32 = 2 << OpSizeShift,<br>
+ OpSizeFixed = 0 << OpSizeShift,<br>
+ OpSize16 = 1 << OpSizeShift,<br>
+ OpSize32 = 2 << OpSizeShift,<br>
+ OpSizeIgnore = 3 << OpSizeShift,<br>
<br>
// AsSize - AdSizeX implies this instruction determines its need of 0x67<br>
// prefix from a normal ModRM memory operand. The other types indicate that<br>
<br>
Modified: llvm/trunk/lib/Target/X86/X86I<wbr>nstrFormats.td<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/X86InstrFormats.td?rev=318678&r1=318677&r2=318678&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-pr<wbr>oject/llvm/trunk/lib/Target/X8<wbr>6/X86InstrFormats.td?rev=31867<wbr>8&r1=318677&r2=318678&view=dif<wbr>f</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- llvm/trunk/lib/Target/X86/X86I<wbr>nstrFormats.td (original)<br>
+++ llvm/trunk/lib/Target/X86/X86I<wbr>nstrFormats.td Mon Nov 20 10:38:55 2017<br>
@@ -157,9 +157,10 @@ def EncEVEX : Encoding<3>;<br>
class OperandSize<bits<2> val> {<br>
bits<2> Value = val;<br>
}<br>
-def OpSizeFixed : OperandSize<0>; // Never needs a 0x66 prefix.<br>
-def OpSize16 : OperandSize<1>; // Needs 0x66 prefix in 32-bit mode.<br>
-def OpSize32 : OperandSize<2>; // Needs 0x66 prefix in 16-bit mode.<br>
+def OpSizeFixed : OperandSize<0>; // Never needs a 0x66 prefix.<br>
+def OpSize16 : OperandSize<1>; // Needs 0x66 prefix in 32-bit mode.<br>
+def OpSize32 : OperandSize<2>; // Needs 0x66 prefix in 16-bit mode.<br>
+def OpSizeIgnore : OperandSize<3>; // Takes 0x66 prefix, never emits.<br>
<br>
// Address size for encodings that change based on mode.<br>
class AddressSize<bits<2> val> {<br>
@@ -174,6 +175,7 @@ def AdSize64 : AddressSize<3>; // Encode<br>
// emitter that various prefix bytes are required.<br>
class OpSize16 { OperandSize OpSize = OpSize16; }<br>
class OpSize32 { OperandSize OpSize = OpSize32; }<br>
+class OpSizeIgnore { OperandSize OpSize = OpSizeIgnore; }<br>
class AdSize16 { AddressSize AdSize = AdSize16; }<br>
class AdSize32 { AddressSize AdSize = AdSize32; }<br>
class AdSize64 { AddressSize AdSize = AdSize64; }<br>
<br>
Modified: llvm/trunk/lib/Target/X86/X86I<wbr>nstrInfo.td<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/X86InstrInfo.td?rev=318678&r1=318677&r2=318678&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-pr<wbr>oject/llvm/trunk/lib/Target/X8<wbr>6/X86InstrInfo.td?rev=318678&r<wbr>1=318677&r2=318678&view=diff</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- llvm/trunk/lib/Target/X86/X86I<wbr>nstrInfo.td (original)<br>
+++ llvm/trunk/lib/Target/X86/X86I<wbr>nstrInfo.td Mon Nov 20 10:38:55 2017<br>
@@ -3157,8 +3157,8 @@ def : InstAlias<"jmpl\t$seg, $off", (FA<br>
// Force mov without a suffix with a segment and mem to prefer the 'l' form of<br>
// the move. All segment/mem forms are equivalent, this has the shortest<br>
// encoding.<br>
-def : InstAlias<"mov\t{$mem, $seg|$seg, $mem}", (MOV32sm SEGMENT_REG:$seg, i32mem:$mem), 0>;<br>
-def : InstAlias<"mov\t{$seg, $mem|$mem, $seg}", (MOV32ms i32mem:$mem, SEGMENT_REG:$seg), 0>;<br>
+def : InstAlias<"mov\t{$mem, $seg|$seg, $mem}", (MOV16sm SEGMENT_REG:$seg, i16mem:$mem), 0>;<br>
+def : InstAlias<"mov\t{$seg, $mem|$mem, $seg}", (MOV16ms i16mem:$mem, SEGMENT_REG:$seg), 0>;<br>
<br>
// Match 'movq <largeimm>, <reg>' as an alias for movabsq.<br>
def : InstAlias<"mov{q}\t{$imm, $reg|$reg, $imm}", (MOV64ri GR64:$reg, i64imm:$imm), 0>;<br>
<br>
Modified: llvm/trunk/lib/Target/X86/X86I<wbr>nstrSystem.td<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/X86InstrSystem.td?rev=318678&r1=318677&r2=318678&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-pr<wbr>oject/llvm/trunk/lib/Target/X8<wbr>6/X86InstrSystem.td?rev=318678<wbr>&r1=318677&r2=318678&view=diff</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- llvm/trunk/lib/Target/X86/X86I<wbr>nstrSystem.td (original)<br>
+++ llvm/trunk/lib/Target/X86/X86I<wbr>nstrSystem.td Mon Nov 20 10:38:55 2017<br>
@@ -175,11 +175,7 @@ def MOV64rs : RI<0x8C, MRMDestReg, (outs<br>
"mov{q}\t{$src, $dst|$dst, $src}", [], IIC_MOV_REG_SR>;<br>
let mayStore = 1 in {<br>
def MOV16ms : I<0x8C, MRMDestMem, (outs), (ins i16mem:$dst, SEGMENT_REG:$src),<br>
- "mov{w}\t{$src, $dst|$dst, $src}", [], IIC_MOV_MEM_SR>, OpSize16;<br>
-def MOV32ms : I<0x8C, MRMDestMem, (outs), (ins i32mem:$dst, SEGMENT_REG:$src),<br>
- "mov{l}\t{$src, $dst|$dst, $src}", [], IIC_MOV_MEM_SR>, OpSize32;<br>
-def MOV64ms : RI<0x8C, MRMDestMem, (outs), (ins i64mem:$dst, SEGMENT_REG:$src),<br>
- "mov{q}\t{$src, $dst|$dst, $src}", [], IIC_MOV_MEM_SR>;<br>
+ "mov{w}\t{$src, $dst|$dst, $src}", [], IIC_MOV_MEM_SR>, OpSizeIgnore;<br>
}<br>
def MOV16sr : I<0x8E, MRMSrcReg, (outs SEGMENT_REG:$dst), (ins GR16:$src),<br>
"mov{w}\t{$src, $dst|$dst, $src}", [], IIC_MOV_SR_REG>, OpSize16;<br>
@@ -189,11 +185,7 @@ def MOV64sr : RI<0x8E, MRMSrcReg, (outs<br>
"mov{q}\t{$src, $dst|$dst, $src}", [], IIC_MOV_SR_REG>;<br>
let mayLoad = 1 in {<br>
def MOV16sm : I<0x8E, MRMSrcMem, (outs SEGMENT_REG:$dst), (ins i16mem:$src),<br>
- "mov{w}\t{$src, $dst|$dst, $src}", [], IIC_MOV_SR_MEM>, OpSize16;<br>
-def MOV32sm : I<0x8E, MRMSrcMem, (outs SEGMENT_REG:$dst), (ins i32mem:$src),<br>
- "mov{l}\t{$src, $dst|$dst, $src}", [], IIC_MOV_SR_MEM>, OpSize32;<br>
-def MOV64sm : RI<0x8E, MRMSrcMem, (outs SEGMENT_REG:$dst), (ins i64mem:$src),<br>
- "mov{q}\t{$src, $dst|$dst, $src}", [], IIC_MOV_SR_MEM>;<br>
+ "mov{w}\t{$src, $dst|$dst, $src}", [], IIC_MOV_SR_MEM>, OpSizeIgnore;<br>
}<br>
} // SchedRW<br>
<br>
<br>
Modified: llvm/trunk/lib/Target/X86/X86S<wbr>chedSandyBridge.td<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/X86SchedSandyBridge.td?rev=318678&r1=318677&r2=318678&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-pr<wbr>oject/llvm/trunk/lib/Target/X8<wbr>6/X86SchedSandyBridge.td?rev=3<wbr>18678&r1=318677&r2=318678&view<wbr>=diff</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- llvm/trunk/lib/Target/X86/X86S<wbr>chedSandyBridge.td (original)<br>
+++ llvm/trunk/lib/Target/X86/X86S<wbr>chedSandyBridge.td Mon Nov 20 10:38:55 2017<br>
@@ -1550,7 +1550,7 @@ def SBWriteResGroup49 : SchedWriteRes<[S<br>
let ResourceCycles = [1,1];<br>
}<br>
def: InstRW<[SBWriteResGroup49], (instregex "JMP(16|32|64)m")>;<br>
-def: InstRW<[SBWriteResGroup49], (instregex "MOV64sm")>;<br>
+def: InstRW<[SBWriteResGroup49], (instregex "MOV16sm")>;<br>
<br>
def SBWriteResGroup50 : SchedWriteRes<[SBPort23,SBPort<wbr>05]> {<br>
let Latency = 6;<br>
<br>
Modified: llvm/trunk/test/MC/Disassemble<wbr>r/X86/x86-16.txt<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/MC/Disassembler/X86/x86-16.txt?rev=318678&r1=318677&r2=318678&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-pr<wbr>oject/llvm/trunk/test/MC/Disas<wbr>sembler/X86/x86-16.txt?rev=318<wbr>678&r1=318677&r2=318678&view=d<wbr>iff</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- llvm/trunk/test/MC/Disassemble<wbr>r/X86/x86-16.txt (original)<br>
+++ llvm/trunk/test/MC/Disassemble<wbr>r/X86/x86-16.txt Mon Nov 20 10:38:55 2017<br>
@@ -207,7 +207,7 @@<br>
# CHECK: movw %cs, %ax<br>
0x8c 0xc8<br>
<br>
-# CHECK: movl %cs, (%eax)<br>
+# CHECK: movw %cs, (%eax)<br>
0x67 0x66 0x8c 0x08<br>
<br>
# CHECK: movw %cs, (%eax)<br>
@@ -216,7 +216,7 @@<br>
# CHECK: movl %eax, %cs<br>
0x66 0x8e 0xc8<br>
<br>
-# CHECK: movl (%eax), %cs<br>
+# CHECK: movw (%eax), %cs<br>
0x67 0x66 0x8e 0x08<br>
<br>
# CHECK: movw (%eax), %cs<br>
<br>
Modified: llvm/trunk/test/MC/X86/x86-16.<wbr>s<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/MC/X86/x86-16.s?rev=318678&r1=318677&r2=318678&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-pr<wbr>oject/llvm/trunk/test/MC/X86/x<wbr>86-16.s?rev=318678&r1=318677&r<wbr>2=318678&view=diff</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- llvm/trunk/test/MC/X86/x86-16.<wbr>s (original)<br>
+++ llvm/trunk/test/MC/X86/x86-16.<wbr>s Mon Nov 20 10:38:55 2017<br>
@@ -248,9 +248,9 @@ cmovnae %bx,%bx<br>
// CHECK: encoding: [0x8c,0xc8]<br>
movw %cs, %ax<br>
<br>
-// CHECK: movl %cs, (%eax)<br>
-// CHECK: encoding: [0x67,0x66,0x8c,0x08]<br>
- movl %cs, (%eax)<br>
+// CHECK: movw %cs, (%eax)<br>
+// CHECK: encoding: [0x67,0x8c,0x08]<br>
+ mov %cs, (%eax)<br>
<br>
// CHECK: movw %cs, (%eax)<br>
// CHECK: encoding: [0x67,0x8c,0x08]<br>
@@ -272,9 +272,9 @@ cmovnae %bx,%bx<br>
// CHECK: encoding: [0x8e,0xc8]<br>
mov %ax, %cs<br>
<br>
-// CHECK: movl (%eax), %cs<br>
-// CHECK: encoding: [0x67,0x66,0x8e,0x08]<br>
- movl (%eax), %cs<br>
+// CHECK: movw (%eax), %cs<br>
+// CHECK: encoding: [0x67,0x8e,0x08]<br>
+ mov (%eax), %cs<br>
<br>
// CHECK: movw (%eax), %cs<br>
// CHECK: encoding: [0x67,0x8e,0x08]<br>
<br>
Modified: llvm/trunk/test/MC/X86/x86-32.<wbr>s<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/MC/X86/x86-32.s?rev=318678&r1=318677&r2=318678&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-pr<wbr>oject/llvm/trunk/test/MC/X86/x<wbr>86-32.s?rev=318678&r1=318677&r<wbr>2=318678&view=diff</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- llvm/trunk/test/MC/X86/x86-32.<wbr>s (original)<br>
+++ llvm/trunk/test/MC/X86/x86-32.<wbr>s Mon Nov 20 10:38:55 2017<br>
@@ -355,12 +355,12 @@ cmovnae %bx,%bx<br>
// CHECK: encoding: [0x66,0x8c,0xc8]<br>
movw %cs, %ax<br>
<br>
-// CHECK: movl %cs, (%eax)<br>
+// CHECK: movw %cs, (%eax)<br>
// CHECK: encoding: [0x8c,0x08]<br>
- movl %cs, (%eax)<br>
+ mov %cs, (%eax)<br>
<br>
// CHECK: movw %cs, (%eax)<br>
-// CHECK: encoding: [0x66,0x8c,0x08]<br>
+// CHECK: encoding: [0x8c,0x08]<br>
movw %cs, (%eax)<br>
<br>
// CHECK: movl %eax, %cs<br>
@@ -379,12 +379,12 @@ cmovnae %bx,%bx<br>
// CHECK: encoding: [0x8e,0xc8]<br>
mov %ax, %cs<br>
<br>
-// CHECK: movl (%eax), %cs<br>
+// CHECK: movw (%eax), %cs<br>
// CHECK: encoding: [0x8e,0x08]<br>
- movl (%eax), %cs<br>
+ mov (%eax), %cs<br>
<br>
// CHECK: movw (%eax), %cs<br>
-// CHECK: encoding: [0x66,0x8e,0x08]<br>
+// CHECK: encoding: [0x8e,0x08]<br>
movw (%eax), %cs<br>
<br>
// radr://8033374<br>
<br>
Modified: llvm/trunk/test/MC/X86/x86-64.<wbr>s<br>
URL: <a href="http://llvm.org/viewvc/llvm-project/llvm/trunk/test/MC/X86/x86-64.s?rev=318678&r1=318677&r2=318678&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-pr<wbr>oject/llvm/trunk/test/MC/X86/x<wbr>86-64.s?rev=318678&r1=318677&r<wbr>2=318678&view=diff</a><br>
==============================<wbr>==============================<wbr>==================<br>
--- llvm/trunk/test/MC/X86/x86-64.<wbr>s (original)<br>
+++ llvm/trunk/test/MC/X86/x86-64.<wbr>s Mon Nov 20 10:38:55 2017<br>
@@ -1082,8 +1082,8 @@ decl %eax // CHECK: decl %eax # encoding<br>
<br>
<br>
// rdar://8208615<br>
-mov (%rsi), %gs // CHECK: movl (%rsi), %gs # encoding: [0x8e,0x2e]<br>
-mov %gs, (%rsi) // CHECK: movl %gs, (%rsi) # encoding: [0x8c,0x2e]<br>
+mov (%rsi), %gs // CHECK: movw (%rsi), %gs # encoding: [0x8e,0x2e]<br>
+mov %gs, (%rsi) // CHECK: movw %gs, (%rsi) # encoding: [0x8c,0x2e]<br>
<br>
<br>
// rdar://8431864<br>
<br>
<br>
______________________________<wbr>_________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/<wbr>mailman/listinfo/llvm-commits</a><br>
</blockquote></div><br></div></div></div>
<br>______________________________<wbr>_________________<br>
llvm-commits mailing list<br>
<a href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a><br>
<a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/<wbr>mailman/listinfo/llvm-commits</a><br>
<br></blockquote></div><br></div></div></div>
</blockquote></div><br></div>