[PATCH][mips] Add more Octeon cnMips instructions

Daniel Sanders Daniel.Sanders at imgtec.com
Wed Apr 2 10:41:39 PDT 2014


Sorry, I see what I've done. I've read the ' [(set GPR64Opnd:$rt, (Op GPR64Opnd:$rs, imm:$pos, imm:$lenm1))]' and taken that as the definition, but Op is null_frag so it's equivalent to '[]'.

It will LGTM with just the issue about BADDu fixed.

> -----Original Message-----
> From: Kai Nacke [mailto:kai.nacke at redstar.de]
> Sent: 02 April 2014 18:05
> To: Daniel Sanders; llvm-commits
> Subject: Re: [PATCH][mips] Add more Octeon cnMips instructions
> 
> Hi Daniel!
> 
> On 02.04.2014 17:18, Daniel Sanders wrote:
> > 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?
> 
> I did not define a pattern to select these instructions - they should not be
> generated yet. Therefore I included no test. (I still think about the best way
> to select these instructions.)
> 
> Regards,
> Kai
> 
> >
> > 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