llvm dev - Mar 2015 - [LLVMdev] [PATCH] fix outs/ins of MOV16mr instruction (X86)

Hi,

This patch fixes outs/ins of MOV16mr instruction of X86.

Thanks.


diff --git a/lib/Target/X86/X86InstrInfo.td b/lib/Target/X86/X86InstrInfo.td
index e9a0431..f5b2064 100644
--- a/lib/Target/X86/X86InstrInfo.td
+++ b/lib/Target/X86/X86InstrInfo.td
@@ -1412,7 +1412,7 @@ let SchedRW = [WriteStore] in {
 def MOV8mr  : I<0x88, MRMDestMem, (outs), (ins i8mem :$dst, GR8 :$src),
                 "mov{b}\t{$src, $dst|$dst, $src}",
                 [(store GR8:$src, addr:$dst)], IIC_MOV_MEM>;
-def MOV16mr : I<0x89, MRMDestMem, (outs), (ins i16mem:$dst, GR16:$src),
+def MOV16mr : I<0x89, MRMDestMem, (outs i16mem:$dst), (ins GR16:$src),
                 "mov{w}\t{$src, $dst|$dst, $src}",
                 [(store GR16:$src, addr:$dst)], IIC_MOV_MEM>, OpSize16;
 def MOV32mr : I<0x89, MRMDestMem, (outs), (ins i32mem:$dst, GR32:$src),
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20150324/4e58d04d/attachment.html>

On Tue, Mar 24, 2015 at 7:54 AM, Jun Koi <junkoi2004 at gmail.com>
wrote:
> Hi,
>
> This patch fixes outs/ins of MOV16mr instruction of X86.
>
> Thanks.
>
>
> diff --git a/lib/Target/X86/X86InstrInfo.td
b/lib/Target/X86/X86InstrInfo.td
> index e9a0431..f5b2064 100644
> --- a/lib/Target/X86/X86InstrInfo.td
> +++ b/lib/Target/X86/X86InstrInfo.td
> @@ -1412,7 +1412,7 @@ let SchedRW = [WriteStore] in {
>  def MOV8mr  : I<0x88, MRMDestMem, (outs), (ins i8mem :$dst, GR8 :$src),
>                  "mov{b}\t{$src, $dst|$dst, $src}",
>                  [(store GR8:$src, addr:$dst)], IIC_MOV_MEM>;
> -def MOV16mr : I<0x89, MRMDestMem, (outs), (ins i16mem:$dst, GR16:$src),
> +def MOV16mr : I<0x89, MRMDestMem, (outs i16mem:$dst), (ins GR16:$src),
>                  "mov{w}\t{$src, $dst|$dst, $src}",
>                  [(store GR16:$src, addr:$dst)], IIC_MOV_MEM>, OpSize16;
Why?  i16mem here stands for the pointer, not the actual memory.  A
store doesn't define a pointer, so why would it be in "outs"?
Also, what's special about i16? You'd need to change the various other
*mr instructions, for instance the MOV8mr right above.

-Ahmed
>  def MOV32mr : I<0x89, MRMDestMem, (outs), (ins i32mem:$dst, GR32:$src),
>
>
> _______________________________________________
> LLVM Developers mailing list
> LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
>

Agree with Ahmed. Also all patches are supposed to go to llvm-commits not
llvm-dev.

On Tue, Mar 24, 2015 at 9:56 AM, Ahmed Bougacha <ahmed.bougacha at
gmail.com>
wrote:
> On Tue, Mar 24, 2015 at 7:54 AM, Jun Koi <junkoi2004 at gmail.com>
wrote:
> > Hi,
> >
> > This patch fixes outs/ins of MOV16mr instruction of X86.
> >
> > Thanks.
> >
> >
> > diff --git a/lib/Target/X86/X86InstrInfo.td
> b/lib/Target/X86/X86InstrInfo.td
> > index e9a0431..f5b2064 100644
> > --- a/lib/Target/X86/X86InstrInfo.td
> > +++ b/lib/Target/X86/X86InstrInfo.td
> > @@ -1412,7 +1412,7 @@ let SchedRW = [WriteStore] in {
> >  def MOV8mr  : I<0x88, MRMDestMem, (outs), (ins i8mem :$dst, GR8
:$src),
> >                  "mov{b}\t{$src, $dst|$dst, $src}",
> >                  [(store GR8:$src, addr:$dst)], IIC_MOV_MEM>;
> > -def MOV16mr : I<0x89, MRMDestMem, (outs), (ins i16mem:$dst,
GR16:$src),
> > +def MOV16mr : I<0x89, MRMDestMem, (outs i16mem:$dst), (ins
GR16:$src),
> >                  "mov{w}\t{$src, $dst|$dst, $src}",
> >                  [(store GR16:$src, addr:$dst)], IIC_MOV_MEM>,
OpSize16;
>
> Why?  i16mem here stands for the pointer, not the actual memory.  A
> store doesn't define a pointer, so why would it be in "outs"?
> Also, what's special about i16? You'd need to change the various
other
> *mr instructions, for instance the MOV8mr right above.
>
> -Ahmed
>
> >  def MOV32mr : I<0x89, MRMDestMem, (outs), (ins i32mem:$dst,
GR32:$src),
> >
> >
> > _______________________________________________
> > LLVM Developers mailing list
> > LLVMdev at cs.uiuc.edu         http://llvm.cs.uiuc.edu
> > http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev
> >
>


-- 
~Craig
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20150324/def5a5af/attachment.html>

On Wed, Mar 25, 2015 at 12:56 AM, Ahmed Bougacha <ahmed.bougacha at
gmail.com>
wrote:
> On Tue, Mar 24, 2015 at 7:54 AM, Jun Koi <junkoi2004 at gmail.com>
wrote:
> > Hi,
> >
> > This patch fixes outs/ins of MOV16mr instruction of X86.
> >
> > Thanks.
> >
> >
> > diff --git a/lib/Target/X86/X86InstrInfo.td
> b/lib/Target/X86/X86InstrInfo.td
> > index e9a0431..f5b2064 100644
> > --- a/lib/Target/X86/X86InstrInfo.td
> > +++ b/lib/Target/X86/X86InstrInfo.td
> > @@ -1412,7 +1412,7 @@ let SchedRW = [WriteStore] in {
> >  def MOV8mr  : I<0x88, MRMDestMem, (outs), (ins i8mem :$dst, GR8
:$src),
> >                  "mov{b}\t{$src, $dst|$dst, $src}",
> >                  [(store GR8:$src, addr:$dst)], IIC_MOV_MEM>;
> > -def MOV16mr : I<0x89, MRMDestMem, (outs), (ins i16mem:$dst,
GR16:$src),
> > +def MOV16mr : I<0x89, MRMDestMem, (outs i16mem:$dst), (ins
GR16:$src),
> >                  "mov{w}\t{$src, $dst|$dst, $src}",
> >                  [(store GR16:$src, addr:$dst)], IIC_MOV_MEM>,
OpSize16;
>
> Why?  i16mem here stands for the pointer, not the actual memory.  A
> store doesn't define a pointer, so why would it be in "outs"?
>

Then why does this "i16mem:$dst" belongs to "ins"? Is that
wrong, correct?


Also, what's special about i16? You'd need to change the various
other
> *mr instructions, for instance the MOV8mr right above.
>
>
Yes, I would fix that with another patch, but now I am not sure if this is
needed.


Thanks,
Jun
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20150325/5c58485d/attachment.html>