[PATCH] [MIPS] OperationAction for FP_TO_FP16, FP16_TO_FP

Pirama Arumuga Nainar pirama at google.com
Tue Apr 21 07:59:33 PDT 2015

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.


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.



More information about the llvm-commits mailing list