[PATCH][mips] Add more Octeon cnMips instructions

Daniel Sanders Daniel.Sanders at imgtec.com
Wed Apr 2 08:19:18 PDT 2014


Nearly forgot to mention this: Could you mention the extra pop/dpop tests in your commit message?

> -----Original Message-----
> From: Daniel Sanders
> Sent: 02 April 2014 16:17
> To: 'Kai Nacke'; llvm-commits
> Subject: RE: [PATCH][mips] Add more Octeon cnMips instructions
> 
> Hi,
> 
> Thanks. I've got a couple more comments.
> 
> I'd prefer the pattern for BADDu to be part of the BADDu definition rather
> than in a separate part of the file. I don't mind whether this is done with a 'let
> Pattern = ...' or a PatFrag.
> There's no CodeGen testcase covering EXTS, EXTS32, CINS, and CINS32. Could
> you add a one?
> 
> With both of those issues fixed it will LGTM.
> 
> > -----Original Message-----
> > From: Kai Nacke [mailto:kai.nacke at redstar.de]
> > Sent: 01 April 2014 21:21
> > To: Daniel Sanders; llvm-commits
> > Subject: Re: [PATCH][mips] Add more Octeon cnMips instructions
> >
> > Hi Daniel!
> >
> > Yes, it works without a custom node. Somehow I missed this. Updated
> > patch is attached.
> >
> > Regards,
> > Kai
> >
> > On 01.04.2014 11:07, Daniel Sanders wrote:
> > > Hi,
> > >
> > > I see you've added MipsISD::BADDU but you don't use any custom
> > lowering into that node. Instead you are using a MipsPat<> to select
> > the standard nodes into a BADDu instruction. I don't think you need
> > the custom SelectionDAG node. You could use a PatFrag instead like so:
> > >    def baddu : PatFrag<(ops node:$rs, node:$rt), (and (add node:$rs,
> > > $rt),
> > 255)>;
> > >    def BADDu : ArithLogicR<"baddu", GPR64Opnd, 1, II_BADDU, baddu>,
> > > ...;
> > >
> > > Similarly for cins/cins32/ext/ext32, you define a new SelectionDAG
> > > node
> > but don't lower into it (or select an instruction with a MipsPat). I
> > believe these could be done with PatFrag as well.
> > >
> > >> -----Original Message-----
> > >> From: Kai Nacke [mailto:kai.nacke at redstar.de]
> > >> Sent: 27 March 2014 18:07
> > >> To: llvm-commits; Daniel Sanders
> > >> Subject: Re: [PATCH][mips] Add more Octeon cnMips instructions
> > >>
> > >> Somehow the latest test case was not committed in my tree. This
> > >> patch contains the current test case. Sorry.
> > >>
> > >> Regards,
> > >> Kai
> > >>
> > >> On 27.03.2014 17:56, Kai Nacke wrote:
> > >>> Hi all!
> > >>>
> > >>> This patch adds the cnMips instructions ext/ext32/cins/cins32.
> > >>> It also changes baddud/pop/dpop to accept the two operand version
> > >>> and adds a simple pattern to generate baddu. Test cases are included.
> > >>>
> > >>> Please review.
> > >>>
> > >>> Regards,
> > >>> Kai
> > >>>
> > >>>
> > >>> _______________________________________________
> > >>> llvm-commits mailing list
> > >>> llvm-commits at cs.uiuc.edu
> > >>> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> > >>>
> > >
> > >
> > > _______________________________________________
> > > llvm-commits mailing list
> > > llvm-commits at cs.uiuc.edu
> > > http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits
> > >
> > >





More information about the llvm-commits mailing list