[PATCH][mips] Add more Octeon cnMips instructions

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


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