[PATCH] [MIPS] OperationAction for FP_TO_FP16, FP16_TO_FP

[Daniel Sanders]

No worries. It turned out that I agreed with most of your replies anyway :-).

> I'll address the CHECK-DAG concern in a different patch and link it back to
> here.

Sounds good to me. Thanks.

> -----Original Message-----
> From: Pirama Arumuga Nainar [mailto:pirama at google.com]
> Sent: 21 April 2015 16:00
> To: pirama at google.com; srhines at google.com; Reed Kotler; Daniel Sanders
> Cc: llvm-commits at cs.uiuc.edu; ahmed.bougacha at gmail.com
> Subject: Re: [PATCH] [MIPS] OperationAction for FP_TO_FP16, FP16_TO_FP
> 
> In http://reviews.llvm.org/D8804#158957, @dsanders wrote:
> 
> > In future, please don't commit after a conditional LGTM if the conditions
> haven't been met. If you disagree with the conditions, you should discuss
> them further.
> 
> 
> My apologies Daniel.  I assumed an "Accepted" status in Phabricator carries
> over across minor updates, and that my last patch was minor.  I was wrong on
> both counts.  In the future, I'll wait for a LGTM on all patches.
> 
> I'll address the CHECK-DAG concern in a different patch and link it back to
> here.
> 
> 
> REPOSITORY
>   rL LLVM
> 
> ================
> Comment at: test/CodeGen/Mips/fp16-promote.ll:64-71
> @@ +63,10 @@
> +; CHECK-LIBCALL-LABEL: test_vec_fpext_double:
> +; CHECK-LIBCALL: %call16(__gnu_h2f_ieee)
> +; CHECK-LIBCALL: %call16(__gnu_h2f_ieee)
> +; CHECK-LIBCALL: %call16(__gnu_h2f_ieee)
> +; CHECK-LIBCALL: cvt.d.s
> +; CHECK-LIBCALL: cvt.d.s
> +; CHECK-LIBCALL: cvt.d.s
> +; CHECK-LIBCALL: %call16(__gnu_h2f_ieee)
> +; CHECK-LIBCALL: cvt.d.s
> +define <4 x double> @test_vec_fpext_double(<4 x half>* %p) #0 {
> ----------------
> dsanders wrote:
> > pirama wrote:
> > > dsanders wrote:
> > > > Just a comment, no change required: This might be a little fragile (the
> exact order of the output may vary) but I don't see a good way to make it
> more robust at the moment.
> > > I'll switch to CHECK-DAG for this test.  It's still not robust enough, but is
> better than the current state.
> > I didn't suggest CHECK-DAG here because it doesn't do what you'd expect
> when the match strings are the same. All the 'CHECK-LIBCALL-DAG: cvt.d.s'
> will match at the same position in the file (the first instance of 'cvt.d.s'). This
> is because the search for CHECK-DAG matches are all matched (starting from
> the same position in the file) before ordering violations are determined.
> >
> > For example, the following file will pass because both CHECK-DAG's match
> the same 'foo'.
> >   ; CHECK-DAG: fo{{o}}
> >   ; CHECK-DAG: fo{{o}}
> >   foo
> You are right.  In fact, I found out about this in a different context, but missed
> it here.  I think the right thing to do here is to change CHECK-DAG to CHECK
> and add a comment noting the fragility of this test.
> 
> http://reviews.llvm.org/D8804
> 
> EMAIL PREFERENCES
>   http://reviews.llvm.org/settings/panel/emailpreferences/
>