Re: [PATCH] D16177: Adding missing intrinsics _cvtsh_ss and _cvtss_sh

2016-01-21 Thread Katya Romanova via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL258492: 2 missing intrinsics _cvtss_sh and _mm_cvtps_ph were added to the intrinsics… (authored by kromanova). Changed prior to commit: http://reviews.llvm.org/D16177?vs=45635&id=45650#toc Repository:

Re: [PATCH] D16177: Adding missing intrinsics _cvtsh_ss and _cvtss_sh

2016-01-21 Thread Craig Topper via cfe-commits
craig.topper accepted this revision. craig.topper added a comment. This revision is now accepted and ready to land. I agree that the vector initialization code will be prone to changing. I think what you have is fine. LGTM Repository: rL LLVM http://reviews.llvm.org/D16177 __

Re: [PATCH] D16177: Adding missing intrinsics _cvtsh_ss and _cvtss_sh

2016-01-21 Thread Katya Romanova via cfe-commits
kromanova added a comment. Craig, do you think it's necessary to make the tests more fancy by checking how the vector is initialized before the builtin invocation and/or that one element is extracted from the vector after the builtin returned a value? It will add additional 10-15 check lines to

Re: [PATCH] D16177: Adding missing intrinsics _cvtsh_ss and _cvtss_sh

2016-01-21 Thread Katya Romanova via cfe-commits
kromanova updated this revision to Diff 45635. kromanova marked an inline comment as done. kromanova added a comment. I further simplified the macros by removing the statement for the define that I added (_cvtss_sh) and for the one that was there before (_mm_cvtps_ph). I also formatted __DEFAULT

Re: [PATCH] D16177: Adding missing intrinsics _cvtsh_ss and _cvtss_sh

2016-01-21 Thread Katya Romanova via cfe-commits
kromanova marked an inline comment as done. Comment at: lib/Headers/f16cintrin.h:47 @@ -34,1 +46,3 @@ + + #define _mm_cvtps_ph(a, imm) __extension__ ({ \ craig.topper wrote: > Can we do something like this to remove the last temporary? > > #define _cvtss_sh(a, i

Re: [PATCH] D16177: Adding missing intrinsics _cvtsh_ss and _cvtss_sh

2016-01-21 Thread Katya Romanova via cfe-commits
kromanova updated this revision to Diff 45632. kromanova marked an inline comment as done. kromanova added a comment. Updated patch to address Craig's comments. Repository: rL LLVM http://reviews.llvm.org/D16177 Files: lib/Headers/f16cintrin.h test/CodeGen/f16c-builtins.c Index: test/Co

Re: [PATCH] D16177: Adding missing intrinsics _cvtsh_ss and _cvtss_sh

2016-01-20 Thread Craig Topper via cfe-commits
craig.topper added inline comments. Comment at: lib/Headers/f16cintrin.h:47 @@ -34,1 +46,3 @@ + + #define _mm_cvtps_ph(a, imm) __extension__ ({ \ Can we do something like this to remove the last temporary? #define _cvtss_sh(a, imm) __extension__ ({ \ (unsigned

Re: [PATCH] D16177: Adding missing intrinsics _cvtsh_ss and _cvtss_sh

2016-01-20 Thread Katya Romanova via cfe-commits
kromanova updated this revision to Diff 45458. kromanova added a comment. Craig, thank you for the review. Here are the changes that you requested. Katya. Repository: rL LLVM http://reviews.llvm.org/D16177 Files: lib/Headers/f16cintrin.h test/CodeGen/f16c-builtins.c Index: test/CodeGen/

Re: [PATCH] D16177: Adding missing intrinsics _cvtsh_ss and _cvtss_sh

2016-01-19 Thread Craig Topper via cfe-commits
craig.topper added inline comments. Comment at: lib/Headers/f16cintrin.h:47 @@ +46,3 @@ +}) + + Would it be possible to do this without temporaries? Temporaries in macros can cause -Wshadow warnings if the macro is used multiple times. Repository: rL LLVM ht

Re: [PATCH] D16177: Adding missing intrinsics _cvtsh_ss and _cvtss_sh

2016-01-13 Thread Katya Romanova via cfe-commits
kromanova added a subscriber: cfe-commits. kromanova added a comment. Adding cfe-commits as a subscriber. Repository: rL LLVM http://reviews.llvm.org/D16177 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mai