On 24/07/18 16:12, James Greenhalgh wrote:
On Thu, Jul 19, 2018 at 07:35:22AM -0500, Matthew Malcomson wrote:
> Hi again.
>
> Providing an updated patch to include the formatting suggestions.
Please try not to top-post replies, it makes the conversation thread
harder to follow (reply continues below!).
> On 12/07/18 11:39, Sudakshina Das wrote:
> > Hi Matthew
> >
> > On 12/07/18 11:18, Richard Sandiford wrote:
> >> Looks good to me FWIW (not a maintainer), just a minor formatting thing:
> >>
> >> Matthew Malcomson <matthew.malcom...@arm.com> writes:
> >>> diff --git a/gcc/config/aarch64/aarch64-simd.md
> >>> b/gcc/config/aarch64/aarch64-simd.md
> >>> index
> >>>
aac5fa146ed8dde4507a0eb4ad6a07ce78d2f0cd..67b29cbe2cad91e031ee23be656ec61a403f2cf9
> >>> 100644
> >>> --- a/gcc/config/aarch64/aarch64-simd.md
> >>> +++ b/gcc/config/aarch64/aarch64-simd.md
> >>> @@ -3302,38 +3302,78 @@
> >>> DONE;
> >>> })
> >>> -(define_insn "aarch64_<ANY_EXTEND:su><ADDSUB:optab>w<mode>"
> >>> +(define_insn "aarch64_<ANY_EXTEND:su>subw<mode>"
> >>> [(set (match_operand:<VWIDE> 0 "register_operand" "=w")
> >>> - (ADDSUB:<VWIDE> (match_operand:<VWIDE> 1 "register_operand"
> >>> "w")
> >>> - (ANY_EXTEND:<VWIDE>
> >>> - (match_operand:VD_BHSI 2 "register_operand" "w"))))]
> >>> + (minus:<VWIDE>
> >>> + (match_operand:<VWIDE> 1 "register_operand" "w")
> >>> + (ANY_EXTEND:<VWIDE>
> >>> + (match_operand:VD_BHSI 2 "register_operand" "w"))))]
> >>
> >> The (minus should be under the "(match_operand":
> >>
> >> (define_insn "aarch64_<ANY_EXTEND:su>subw<mode>"
> >> [(set (match_operand:<VWIDE> 0 "register_operand" "=w")
> >> (minus:<VWIDE> (match_operand:<VWIDE> 1 "register_operand" "w")
> >> (ANY_EXTEND:<VWIDE>
> >> (match_operand:VD_BHSI 2 "register_operand" "w"))))]
> >>
> >> Same for the other patterns.
> >>
> >> Thanks,
> >> Richard
> >>
> >
> > You will need a maintainer's approval but this looks good to me.
> > Thanks for doing this. I would only point out one other nit which you
> > can choose to ignore:
> >
> > +/* Ensure
> > + saddw2 and one saddw for the function add()
> > + ssubw2 and one ssubw for the function subtract()
> > + uaddw2 and one uaddw for the function uadd()
> > + usubw2 and one usubw for the function usubtract() */
> > +
> > +/* { dg-final { scan-assembler-times "\[ \t\]ssubw2\[ \t\]+" 1 } } */
> > +/* { dg-final { scan-assembler-times "\[ \t\]ssubw\[ \t\]+" 1 } } */
> > +/* { dg-final { scan-assembler-times "\[ \t\]saddw2\[ \t\]+" 1 } } */
> > +/* { dg-final { scan-assembler-times "\[ \t\]saddw\[ \t\]+" 1 } } */
> > +/* { dg-final { scan-assembler-times "\[ \t\]usubw2\[ \t\]+" 1 } } */
> > +/* { dg-final { scan-assembler-times "\[ \t\]usubw\[ \t\]+" 1 } } */
> > +/* { dg-final { scan-assembler-times "\[ \t\]uaddw2\[ \t\]+" 1 } } */
> > +/* { dg-final { scan-assembler-times "\[ \t\]uaddw\[ \t\]+" 1 } } */
> >
> > The scan-assembly directives for the different
> > functions can be placed right below each of them and that would
> > make it easier to read the expected results in the test and you
> > can get rid of the comments saying the same.
Thanks for the first-line review Sudi.
OK for trunk.
I've committed this on behalf of Matthew as: https://gcc.gnu.org/r262949
Thanks,
Kyrill
Thanks,
James
