Re: [PATCH] D20614: Remove trailing spaces in x86 intrinsic headers

2016-05-25 Thread Katya Romanova via cfe-commits
kromanova updated this revision to Diff 58472. kromanova added a comment. I attached full svn diff. Thank you Michael! I had to clean up the trailing spaces from doxygen comments (because it mess up our post-processing scripts), but since I was doing it, I decided to clean up the rest of the

[PATCH] D20614: Remove trailing spaces in x86 intrinsic headers

2016-05-25 Thread Katya Romanova via cfe-commits
kromanova created this revision. kromanova added a reviewer: m_zuckerman. kromanova added a subscriber: cfe-commits. kromanova set the repository for this revision to rL LLVM. Clean up: remove trailing spaces in x86 intrinsic headers. Repository: rL LLVM http://reviews.llvm.org/D20614 Files:

Re: [PATCH] D17550: Adding doxygen comments to the LLVM intrinsics (part 6, popcntintrin.h)

2016-03-01 Thread Katya Romanova via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL262385: This patch adds doxygen comments for the intrinsincs in the header file… (authored by kromanova). Changed prior to commit: http://reviews.llvm.org/D17550?vs=48844=49534#toc Repository: rL

[PATCH] D17550: Adding doxygen comments to the LLVM intrinsics (part 6, popcntintrin.h)

2016-02-23 Thread Katya Romanova via cfe-commits
kromanova created this revision. kromanova added a subscriber: cfe-commits. kromanova set the repository for this revision to rL LLVM. Here is the patch with the doxygen comments for the intrinsincs in the header file popcntintrin.h. The doxygen comments are automatically generated based on SCE

[PATCH] D17021: Adding doxygen comments to the LLVM intrinsics (part 5, f16cintrin.h)

2016-02-09 Thread Katya Romanova via cfe-commits
kromanova created this revision. kromanova added reviewers: ygao, probinson, echristo, gribozavr, craig.topper, jroelofs. kromanova added a subscriber: cfe-commits. kromanova set the repository for this revision to rL LLVM. Eric Christopher told me that from now on it's OK to commit doxygen

Re: [PATCH] D17021: Adding doxygen comments to the LLVM intrinsics (part 5, f16cintrin.h)

2016-02-09 Thread Katya Romanova via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL260333: This patch adds doxygen comments for all the intrinsincs in the header file… (authored by kromanova). Changed prior to commit: http://reviews.llvm.org/D17021?vs=47299=47394#toc Repository:

Re: [PATCH] D16913: Adding doxygen comments to the LLVM intrinsics (part 4, pmmintrin.h)

2016-02-08 Thread Katya Romanova via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL260160: This patch adds doxygen comments for all the intrinsincs in the header file… (authored by kromanova). Changed prior to commit: http://reviews.llvm.org/D16913?vs=46988=47250#toc Repository:

[PATCH] D16913: Adding doxygen comments to the LLVM intrinsics (part 4, pmmintrin.h)

2016-02-04 Thread Katya Romanova via cfe-commits
kromanova created this revision. kromanova added reviewers: ygao, echristo, jroelofs, gribozavr, craig.topper, probinson. kromanova added a subscriber: cfe-commits. kromanova set the repository for this revision to rL LLVM. Hello, Here is the patch with the doxygen comments for the intrinsincs

Re: [PATCH] D16562: Adding doxygen comments to the LLVM intrinsics (part 3, __wmmintrin_aes.h)

2016-01-29 Thread Katya Romanova via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL259275: This patch adds doxygen comments for the intrinsincs in the header file… (authored by kromanova). Changed prior to commit: http://reviews.llvm.org/D16562?vs=45924=46447#toc Repository: rL

Re: [PATCH] D16562: Adding doxygen comments to the LLVM intrinsics (part 3, __wmmintrin_aes.h)

2016-01-29 Thread Katya Romanova via cfe-commits
kromanova added a comment. Hi Eric, Could you please accept this revision also? I have already added missing article "the" in "This intrinsic corresponds to instruction" (as you requested in the other doxygen comments review). Thank you! Katya. Repository: rL LLVM

Re: [PATCH] D15999: Adding doxygen comments to the LLVM intrinsics (part 2, _wmmintrin_pclmul.h)

2016-01-29 Thread Katya Romanova via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL259239: This patch adds doxygen comments for the intrinsincs in the header file… (authored by kromanova). Changed prior to commit: http://reviews.llvm.org/D15999?vs=45902=46411#toc Repository: rL

[PATCH] D16697: Updating .debug_line section version information to match DWARF version.

2016-01-28 Thread Katya Romanova via cfe-commits
kromanova created this revision. kromanova added reviewers: dblaikie, echristo, probinson. kromanova added a subscriber: cfe-commits. kromanova set the repository for this revision to rL LLVM. Hello, The compiler always emits .debug_line version 2, though some opcodes from DWARF 3 (e.g.

Re: [PATCH] D15999: Adding doxygen comments to the LLVM intrinsics (part 2, _wmmintrin_pclmul.h)

2016-01-27 Thread Katya Romanova via cfe-commits
kromanova added a comment. In http://reviews.llvm.org/D15999#335653, @echristo wrote: > Honestly if they've been reviewed like that internally I'm ok with you just > committing them - especially if they look like this. > > The only concerns I'd have are in the case of "This intrinsic

Re: [PATCH] D15999: Adding doxygen comments to the LLVM intrinsics (part 2, _wmmintrin_pclmul.h)

2016-01-27 Thread Katya Romanova via cfe-commits
kromanova added a comment. In http://reviews.llvm.org/D15999#335653, @echristo wrote: > Honestly if they've been reviewed like that internally I'm ok with you just > committing them - especially if they look like this. > > The only concerns I'd have are in the case of "This intrinsic

Re: [PATCH] D15999: Adding doxygen comments to the LLVM intrinsics (part 2, _wmmintrin_pclmul.h)

2016-01-25 Thread Katya Romanova via cfe-commits
kromanova removed rL LLVM as the repository for this revision. kromanova updated this revision to Diff 45902. kromanova added a comment. SCE's techinical writer, Craig Flores, did the code review and suggested a few changes to the documentation. http://reviews.llvm.org/D15999 Files:

[PATCH] D16562: Adding doxygen comments to the LLVM intrinsics (part 3, __wmmintrin_aes.h)

2016-01-25 Thread Katya Romanova via cfe-commits
kromanova created this revision. kromanova added reviewers: gribozavr, jroelofs, gaoyunzhong, craig.topper. kromanova added subscribers: silvas, echristo, cfe-commits. kromanova set the repository for this revision to rL LLVM. Hello, Here is the patch with the doxygen comments for the

Re: [PATCH] D15999: Adding doxygen comments to the LLVM intrinsics (part 2, _wmmintrin_pclmul.h)

2016-01-24 Thread Katya Romanova via cfe-commits
kromanova added a comment. I did some build time measurement on a big game code where the the intrisics are heavily used (PHC was enabled). The presence of comments didn't make any noticeable difference. Sean did similar measurements without PCH and didn't see any difference in performance

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:

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,

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

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

Re: [PATCH] D15999: Adding doxygen comments to the LLVM intrinsics (part 2, _wmmintrin_pclmul.h)

2016-01-21 Thread Katya Romanova via cfe-commits
kromanova added a comment. In http://reviews.llvm.org/D15999#333173, @silvas wrote: > For the preview of all the changes, can you please put that in a separate > patch from this one? Done. See http://reviews.llvm.org/D16442 Repository: rL LLVM http://reviews.llvm.org/D15999

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=45650#toc Repository:

Re: [PATCH] D15999: Adding doxygen comments to the LLVM intrinsics (part 2, _wmmintrin_pclmul.h)

2016-01-20 Thread Katya Romanova via cfe-commits
kromanova added a comment. In http://reviews.llvm.org/D15999#330794, @silvas wrote: > This may sound stupid, but: can you benchmark the time it takes to build some > project (that actually uses intrinsics in most translation units, e.g. a > game) with the headers w/ and w/o the doxygen

Re: [PATCH] D15999: Adding doxygen comments to the LLVM intrinsics (part 2, _wmmintrin_pclmul.h)

2016-01-20 Thread Katya Romanova via cfe-commits
kromanova added a comment. In http://reviews.llvm.org/D15999#331639, @silvas wrote: > In http://reviews.llvm.org/D15999#331601, @kromanova wrote: > > > > > > I don't think we did any testing at SCE, but we probably should have. I don't > think Google's primary codebases (nor Apple's, or anybody

Re: [PATCH] D15999: Adding doxygen comments to the LLVM intrinsics (part 2, _wmmintrin_pclmul.h)

2016-01-20 Thread Katya Romanova via cfe-commits
kromanova added a comment. In http://reviews.llvm.org/D15999#330794, @silvas wrote: > This may sound stupid, but: can you benchmark the time it takes to build some > project (that actually uses intrinsics in most translation units, e.g. a > game) with the headers w/ and w/o the doxygen

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:

Re: [PATCH] D15999: Adding doxygen comments to the LLVM intrinsics (part 2, _wmmintrin_pclmul.h)

2016-01-20 Thread Katya Romanova via cfe-commits
kromanova added a comment. In http://reviews.llvm.org/D15999#331649, @silvas wrote: > In http://reviews.llvm.org/D15999#331512, @kromanova wrote: > > > In http://reviews.llvm.org/D15999#330794, @silvas wrote: > > > > > Also, can you post a patch that changes "all" the headers to have doxygen >

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

Re: [PATCH] D12624: Top-level anonymous namespaces are missing import DW_TAG_imported_module and nested anonymous namespaces are not

2015-12-10 Thread Katya Romanova via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL255281: Do not generate DW_TAG_imported_module for anonymous namespaces (even nested)… (authored by kromanova). Changed prior to commit: http://reviews.llvm.org/D12624?vs=42364=42449#toc Repository:

Re: [PATCH] D12624: Top-level anonymous namespaces are missing import DW_TAG_imported_module and nested anonymous namespaces are not

2015-12-09 Thread Katya Romanova via cfe-commits
kromanova updated the summary for this revision. kromanova set the repository for this revision to rL LLVM. kromanova updated this revision to Diff 42364. kromanova added a comment. I have made all the changes that Richard suggested. Sorry for the delay, got distracted by other tasks. Anything

Re: [PATCH] D12624: Top-level anonymous namespaces are missing import DW_TAG_imported_module and nested anonymous namespaces are not

2015-12-09 Thread Katya Romanova via cfe-commits
kromanova added a comment. > > Once we have this in place we can think about factoring the debug-specific > > flags out of CodeGenOpts into some kind of DebugInfoOpts, to be configured > > by whatever debugger tuning mechanism we end up with. > Good idea! Thank you for such prompt review.

Re: [PATCH] D8762: Adding doxygen comments to the LLVM intrinsics (part 1, ammintrin.h)

2015-12-09 Thread Katya Romanova via cfe-commits
kromanova accepted this revision. kromanova added a reviewer: kromanova. kromanova added a comment. This revision is now accepted and ready to land. This was committed in r238386 but I forgot to close the review. http://reviews.llvm.org/D8762 ___

Re: [PATCH] D13482: Revised Initial patch for PS4 toolchain

2015-10-13 Thread Katya Romanova via cfe-commits
kromanova added a comment. Thank you! I will rebase and commit shortly Katya. Repository: rL LLVM http://reviews.llvm.org/D13482 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D13482: Revised Initial patch for PS4 toolchain

2015-10-13 Thread Katya Romanova via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL250252: This patch adds missing pieces to clang, including the PS4 toolchain (authored by kromanova). Changed prior to commit: http://reviews.llvm.org/D13482?vs=37292=37301#toc Repository: rL LLVM

Re: [PATCH] D13482: Revised Initial patch for PS4 toolchain

2015-10-08 Thread Katya Romanova via cfe-commits
kromanova updated this revision to Diff 36829. kromanova added a comment. Few more changes: (1) There was a bug, where the PS4 driver didn't add input filename in the call to external assembler. Filipe fixed this problem in Tools.cpp (2) A new testcase no-integrated-as.s was added for testing

Re: [PATCH] D13482: Revised Initial patch for PS4 toolchain

2015-10-07 Thread Katya Romanova via cfe-commits
kromanova added a comment. Hi Pierre, I noticed the same issue. When I downloaded a patch from http://reviews.llvm.org/D11279, I had to manually add .keep files to make tests to pass. It's a Phabricator problem. The diff file that I uploaded to Phabricator contains .keep files, however the

Re: [PATCH] D11279: Initial patch for PS4 toolchain

2015-09-24 Thread Katya Romanova via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL248546: This patch adds missing pieces to clang, including the PS4 toolchain (authored by kromanova). Changed prior to commit: http://reviews.llvm.org/D11279?vs=34522=35681#toc Repository: rL LLVM

Re: [PATCH] D11279: Initial patch for PS4 toolchain

2015-09-16 Thread Katya Romanova via cfe-commits
kromanova added a comment. test/Driver/Inputs/scei-ps4_tree/target/include_common/ and test/Driver/Inputs/scei-ps4_tree/target/include/ are expected to be present by a test ps4-header-search.c. This test checks that these directories are found in the header search path. If these directories

Re: [PATCH] D11737: Add -linker (and -linker=) alias for -fuse-ld=

2015-09-02 Thread Katya Romanova via cfe-commits
kromanova added inline comments. Comment at: include/clang/Driver/Options.td:1853-1854 @@ -1853,2 +1852,4 @@ +def fuse_ld_EQ : Joined<["-"], "fuse-ld=">, HelpText<"Use linker ">, Group; +def linker_EQ : Joined<["-"], "linker=">, Alias, MetaVarName<"">; defm align_functions :