[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.
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/
More information about the llvm-commits
mailing list