Re: [PATCH] D11279: Initial patch for PS4 toolchain
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 are not found, the compiler
complains and the test fails.
.keep file is needed to keep these directories in place.
Comment at: lib/Driver/Tools.cpp:10025-10029
@@ +10024,7 @@
+ const char *Exec =
+#ifdef LLVM_ON_WIN32
+ Args.MakeArgString(ToolChain.GetProgramPath("ps4-ld.gold.exe"));
+#else
+ Args.MakeArgString(ToolChain.GetProgramPath("ps4-ld"));
+#endif
+
echristo wrote:
> How does this work with the things below that check for linker name?
>
> Also seems like we'd really want to be able to check our host using the
> normal ways rather than an ifdef. This can be done as a followup, but do
> please check into this.
We will check for the host without the ifdef, but we will do it in a follow-up
commit.
http://reviews.llvm.org/D11279
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D11279: Initial patch for PS4 toolchain
kromanova added a comment. Hi Eric, I don't mind changing .keep into README. However, originally we did it for consistency purpose. If you grep for .keep in /llvm/tools/clang/test/Driver/Inputs directory, you will find hundreds of instances of these files that were created for similar reasons (checking SDK layout), but you won't find a single README file there. Do you still want us to make this change? http://reviews.llvm.org/D11279 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D11279: Initial patch for PS4 toolchain
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&id=35681#toc Repository: rL LLVM http://reviews.llvm.org/D11279 Files: cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td cfe/trunk/include/clang/Basic/DiagnosticGroups.td cfe/trunk/lib/Driver/Driver.cpp cfe/trunk/lib/Driver/ToolChains.cpp cfe/trunk/lib/Driver/ToolChains.h cfe/trunk/lib/Driver/Tools.cpp cfe/trunk/lib/Driver/Tools.h cfe/trunk/lib/Frontend/InitHeaderSearch.cpp cfe/trunk/test/Driver/Inputs/scei-ps4_tree/target/include/.keep cfe/trunk/test/Driver/Inputs/scei-ps4_tree/target/include_common/.keep cfe/trunk/test/Driver/debug-options.c cfe/trunk/test/Driver/ps4-header-search.c cfe/trunk/test/Driver/ps4-linker-non-win.c cfe/trunk/test/Driver/ps4-linker-win.c cfe/trunk/test/Driver/ps4-pic.c cfe/trunk/test/Driver/ps4-sdk-root.c cfe/trunk/test/Driver/rtti-options.cpp cfe/trunk/test/Driver/stack-protector.c Index: cfe/trunk/include/clang/Basic/DiagnosticGroups.td === --- cfe/trunk/include/clang/Basic/DiagnosticGroups.td +++ cfe/trunk/include/clang/Basic/DiagnosticGroups.td @@ -820,3 +820,7 @@ // A warning group for things that will change semantics in the future. def FutureCompat : DiagGroup<"future-compat">; + +def InvalidOrNonExistentDirectory : DiagGroup<"invalid-or-nonexistent-directory">; + +def OptionIgnored : DiagGroup<"option-ignored">; Index: cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td === --- cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td +++ cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td @@ -192,4 +192,18 @@ def warn_target_unsupported_nanlegacy : Warning< "ignoring '-mnan=legacy' option because the '%0' architecture does not support it">, InGroup; + +def warn_drv_unable_to_find_directory_expected : Warning< + "unable to find %0 directory, expected to be in '%1'">, + InGroup; + +def warn_drv_ps4_force_pic : Warning< + "option '%0' was ignored by the PS4 toolchain, using '-fPIC'">, + InGroup; + +def warn_drv_ps4_sdk_dir : Warning< + "environment variable SCE_PS4_SDK_DIR is set, but points to invalid or nonexistent directory '%0'">, + InGroup; + +def err_drv_unsupported_linker : Error<"unsupported value '%0' for -linker option">; } Index: cfe/trunk/test/Driver/ps4-linker-non-win.c === --- cfe/trunk/test/Driver/ps4-linker-non-win.c +++ cfe/trunk/test/Driver/ps4-linker-non-win.c @@ -0,0 +1,18 @@ +// UNSUPPORTED: system-windows +// REQUIRES: x86-registered-target + +// RUN: touch %T/ps4-ld + +// RUN: env "PATH=%T" %clang -### -target x86_64-scei-ps4 %s -linker=gold 2>&1 \ +// RUN: | FileCheck --check-prefix=CHECK-PS4-LINKER %s +// RUN: env "PATH=%T" %clang -### -target x86_64-scei-ps4 %s -shared 2>&1 \ +// RUN: | FileCheck --check-prefix=CHECK-PS4-LINKER %s + +// RUN: env "PATH=%T" %clang -### -target x86_64-scei-ps4 %s 2>&1 \ +// RUN: | FileCheck --check-prefix=CHECK-PS4-LINKER %s +// RUN: env "PATH=%T" %clang -### -target x86_64-scei-ps4 %s -linker=ps4 2>&1 \ +// RUN: | FileCheck --check-prefix=CHECK-PS4-LINKER %s +// RUN: env "PATH=%T" %clang -### -target x86_64-scei-ps4 %s -shared \ +// RUN: -linker=ps4 2>&1 | FileCheck --check-prefix=CHECK-PS4-LINKER %s + +// CHECK-PS4-LINKER: ps4-ld Index: cfe/trunk/test/Driver/ps4-linker-win.c === --- cfe/trunk/test/Driver/ps4-linker-win.c +++ cfe/trunk/test/Driver/ps4-linker-win.c @@ -0,0 +1,26 @@ +// The full path to the gold linker was not found on Windows because the +// driver fails to add an .exe extension to the name. +// We check that gold linker's full name (with an extension) is specified +// on the command line if -linker=gold, or -shared with no -linker option +// are passed. Otherwise, we check that the PS4's linker's full name is +// specified. + +// REQUIRES: system-windows, x86-registered-target + +// RUN: touch %T/ps4-ld.exe +// RUN: touch %T/ps4-ld.gold.exe + +// RUN: env "PATH=%T" %clang -target x86_64-scei-ps4 %s -linker=gold -### 2>&1 \ +// RUN: | FileCheck --check-prefix=CHECK-PS4-GOLD %s +// RUN: env "PATH=%T" %clang -target x86_64-scei-ps4 %s -shared -### 2>&1 \ +// RUN: | FileCheck --check-prefix=CHECK-PS4-GOLD %s + +// RUN: env "PATH=%T" %clang -target x86_64-scei-ps4 %s -### 2>&1 \ +// RUN: | FileCheck --check-prefix=CHECK-PS4-LINKER %s +// RUN: env "PATH=%T" %clang -target x86_64-scei-ps4 %s -linker=ps4 -### 2>&1 \ +// RUN: | FileCheck --check-prefix=CHECK-PS4-LINKER %s +// RUN: env "PATH=%T" %clang -target x86_64-scei-ps4 %s -shared \ +// RUN: -linker=ps4 -### 2>&1 | FileCheck --check-prefix=CHE
[PATCH] D13482: Revised Initial patch for PS4 toolchain
kromanova created this revision. kromanova added reviewers: filcab, echristo, alexr, probinson. kromanova added subscribers: silvas, cfe-commits, chapuni, jroelofs, asl, pgousseau, gbedwell. kromanova set the repository for this revision to rL LLVM. Initial patch for PS4 toolchain was created here: http://reviews.llvm.org/D11279 When this patch was committed in r248546, it caused 2 distinct problems. (1) Many failures were reported on recently set up PS4 bot. The PS4 driver intentionally reported warnings if PS4 SDK was missing. The new buildbot didn't have PS4 SDK directory installed. This "combination" caused failures for all the tests that had -Werror option. (2) ps4-linker-win.c failed on all the Windows bots. This commit was reverted in r248548 to make the bots green again and to decide on how to deal with missing PS4 SDK directory issue. The new patch should take care of both problems mentioned above. Since review for http://reviews.llvm.org/D11279 was owned by Filipe Cabecinhas and already closed, it's probably easier for me to open a new code review. Feel free to compare this new patch to the latest patch in D11279. Here is the summary of the changes: 1. include/clang/Basic/DiagnosticDriverKinds.td Added DefaultIgnore attribute to InvalidOrNonExistentDirectory. By default, the PS4 driver won't report a warning about missing PS4 SDK. This behavior could be changed if -Weverything or -Winvalid-or-nonexistent-directory options are passed. 2. Changed test/Driver/ps4-linker-non-win.c to use -fuse-ld=gold instead of -linker=gold test/Driver/ps4-linker-win.c to use -fuse-ld=gold instead of -linker=gold We do not support "-linker" option anymore. Use "-fuse-ld" instead. 3. test/Driver/ps4-sdk-root.c Added -Winvalid-or-nonexistent-directory to all the RUN lines to force the warning (this is the only test where we want to have the warnings about missing PS4 SDK directory enabled). 4. test/Driver/rtti-options.cpp Removed all the changes. No changes are needed, since the warning is not reported by default anymore. Origianal patch extracted by Filipe Cabecinhas, me (Katya Romanova), and Pierre Gousseau. Repository: rL LLVM http://reviews.llvm.org/D13482 Files: include/clang/Basic/DiagnosticDriverKinds.td include/clang/Basic/DiagnosticGroups.td lib/Driver/Driver.cpp lib/Driver/ToolChains.cpp lib/Driver/ToolChains.h lib/Driver/Tools.cpp lib/Driver/Tools.h lib/Frontend/InitHeaderSearch.cpp test/Driver/Inputs/scei-ps4_tree/target/include/.keep test/Driver/Inputs/scei-ps4_tree/target/include_common/.keep test/Driver/debug-options.c test/Driver/ps4-header-search.c test/Driver/ps4-linker-non-win.c test/Driver/ps4-linker-win.c test/Driver/ps4-pic.c test/Driver/ps4-sdk-root.c test/Driver/stack-protector.c Index: test/Driver/stack-protector.c === --- test/Driver/stack-protector.c +++ test/Driver/stack-protector.c @@ -23,3 +23,12 @@ // RUN: %clang -fstack-protector-all -### %s 2>&1 | FileCheck %s -check-prefix=SSP-ALL // SSP-ALL: "-stack-protector" "3" // SSP-ALL-NOT: "-stack-protector-buffer-size" + +// RUN: %clang -target x86_64-scei-ps4 -### %s 2>&1 | FileCheck %s -check-prefix=SSP-PS4 +// RUN: %clang -target x86_64-scei-ps4 -fstack-protector -### %s 2>&1 | FileCheck %s -check-prefix=SSP-PS4 +// SSP-PS4: "-stack-protector" "2" +// SSP-PS4-NOT: "-stack-protector-buffer-size" + +// RUN: %clang -target x86_64-scei-ps4 -fstack-protector --param ssp-buffer-size=16 -### %s 2>&1 | FileCheck %s -check-prefix=SSP-PS4-BUF +// SSP-PS4-BUF: "-stack-protector" "2" +// SSP-PS4-BUF: "-stack-protector-buffer-size" "16" Index: test/Driver/ps4-sdk-root.c === --- test/Driver/ps4-sdk-root.c +++ test/Driver/ps4-sdk-root.c @@ -0,0 +1,48 @@ +// REQUIRES: x86-registered-target + +// Check that ps4-clang doesn't report a warning message when locating +// system header files (either by looking at the value of SCE_PS4_SDK_DIR +// or relative to the location of the compiler driver), if "-nostdinc", +// "--sysroot" or "-isysroot" option is specified on the command line. +// Otherwise, check that ps4-clang reports a warning. + +// Check that clang doesn't report a warning message when locating +// system libraries (either by looking at the value of SCE_PS4_SDK_DIR +// or relative to the location of the compiler driver), if "-c", "-S", "-E", +// "--sysroot", "-nostdlib" or "-nodefaultlibs" option is specified on +// the command line. +// Otherwise, check that ps4-clang reports a warning. + +// setting up SCE_PS4_SDK_DIR to existing location, which is not a PS4 SDK. +// RUN: env SCE_PS4_SDK_DIR=.. %clang -Winvalid-or-nonexistent-directory -### -target x86_64-scei-ps4 %s 2>&1 | FileCheck -check-prefix=WARN-SYS-HEADERS -check-prefix=WARN-SYS-LIBS -check-prefix=NO-WARN %s + +// RUN: env SCE_PS4_SDK_DIR=.. %clang -Winvalid-or-nonexiste
Re: [PATCH] D11279: Initial patch for PS4 toolchain
kromanova added inline comments.
Comment at: lib/Frontend/InitHeaderSearch.cpp:325-344
@@ +324,22 @@
+ case llvm::Triple::PS4: {
+// gets prepended later in AddPath().
+std::string BaseSDKPath = "";
+if (!HasSysroot) {
+ const char *envValue = getenv("SCE_PS4_SDK_DIR");
+ if (envValue)
+BaseSDKPath = envValue;
+ else {
+// HSOpts.ResourceDir variable contains the location of Clang's
+// resource files.
+// Assuming that Clang is configured for PS4 without
+// --with-clang-resource-dir option, the location of Clang's resource
+// files is /host_tools/lib/clang
+SmallString<128> P = StringRef(HSOpts.ResourceDir);
+llvm::sys::path::append(P, "../../..");
+BaseSDKPath = P.str();
+ }
+}
+AddPath(BaseSDKPath + "/target/include", System, false);
+if (triple.isPS4CPU())
+ AddPath(BaseSDKPath + "/target/include_common", System, false);
+ }
echristo wrote:
> This all seems odd, what's going on here?
Our SDK layout structure is different from other platforms. Some of the PS4
specific headers (e.g. graphic headers, etc) are placed into
target/include_common directory and we want to have them in the search path.
http://reviews.llvm.org/D11279
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D11737: Add -linker (and -linker=) alias for -fuse-ld=
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 : BooleanFFlag<"align-functions">, Group; We will remove -linker alias. We had to confirm with the interested parties that they are OK with this option being removed. http://reviews.llvm.org/D11737 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D11279: Initial patch for PS4 toolchain
kromanova added inline comments.
Comment at: lib/Frontend/InitHeaderSearch.cpp:325-344
@@ +324,22 @@
+ case llvm::Triple::PS4: {
+// gets prepended later in AddPath().
+std::string BaseSDKPath = "";
+if (!HasSysroot) {
+ const char *envValue = getenv("SCE_PS4_SDK_DIR");
+ if (envValue)
+BaseSDKPath = envValue;
+ else {
+// HSOpts.ResourceDir variable contains the location of Clang's
+// resource files.
+// Assuming that Clang is configured for PS4 without
+// --with-clang-resource-dir option, the location of Clang's resource
+// files is /host_tools/lib/clang
+SmallString<128> P = StringRef(HSOpts.ResourceDir);
+llvm::sys::path::append(P, "../../..");
+BaseSDKPath = P.str();
+ }
+}
+AddPath(BaseSDKPath + "/target/include", System, false);
+if (triple.isPS4CPU())
+ AddPath(BaseSDKPath + "/target/include_common", System, false);
+ }
echristo wrote:
> kromanova wrote:
> > echristo wrote:
> > > This all seems odd, what's going on here?
> > Our SDK layout structure is different from other platforms. Some of the PS4
> > specific headers (e.g. graphic headers, etc) are placed into
> > target/include_common directory and we want to have them in the search path.
> You also check for ps4cpu in here?
I don't think this is needed since we are under Triple::PS4 switch now. Sorry,
this is an oversight on our part, caused by refactoring of old code that didn't
has PS4 switch case before. We will remove 'if' part.
http://reviews.llvm.org/D11279
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D11279: Initial patch for PS4 toolchain
kromanova added inline comments. Comment at: lib/Frontend/InitHeaderSearch.cpp:340 @@ +339,3 @@ +BaseSDKPath = P.str(); + } +} jroelofs wrote: > The lifetime of `P` ends here, yet a reference to it lives on because of the > `= P.str()`. Using it later on line 344 is UB. Good catch. Thank you! We will fix it. http://reviews.llvm.org/D11279 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D11279: Initial patch for PS4 toolchain
kromanova added inline comments.
Comment at: lib/Driver/Tools.cpp:3590-3591
@@ -3580,4 +3589,4 @@
Args.ClaimAllArgs(options::OPT_g_flags_Group);
if (Args.hasFlag(options::OPT_gcolumn_info, options::OPT_gno_column_info,
- /*Default*/ true))
+ /*Default*/ !Triple.isPS4CPU()))
CmdArgs.push_back("-dwarf-column-info");
echristo wrote:
> filcab wrote:
> > echristo wrote:
> > > Hmm?
> > We have different defaults from other platforms.
> *nod* disabling column info yes? I have "opinions" on this, but it's not my
> platform. Needs a comment.
Our debugger doesn't use column information. Though it's not too much, unused
column information adds to the total size of DI.
We will add a comment.
Comment at: lib/Driver/Tools.cpp:3613
@@ -3603,3 +3612,3 @@
// backend.
- if (Args.hasArg(options::OPT_gdwarf_aranges)) {
+ if (Args.hasArg(options::OPT_gdwarf_aranges) || Triple.isPS4CPU()) {
CmdArgs.push_back("-backend-option");
echristo wrote:
> filcab wrote:
> > echristo wrote:
> > > Ditto.
> > Ditto, different defaults.
> > But I guess I can hoist out the Triple.isPS4CPU() on both cases, if you
> > prefer, like it's done for other toolchains like:
> > bool IsWindowsMSVC =
> > getToolChain().getTriple().isWindowsMSVCEnvironment();
> Probably better to just handle it the same way as the column info I'd think.
> Also, this is all terrible and needs to change, that said, not your problem.
> :)
We could do exactly the same for aranges as it we did for the column info.
Though it seems that we will have to add 'gno_dwarf_aranges' if we want to use
hasFlag function here too. Is it worth doing it?
http://reviews.llvm.org/D11279
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D12624: Top-level anonymous namespaces are missing import DW_TAG_imported_module and nested anonymous namespaces are not
kromanova created this revision.
kromanova added reviewers: rsmith, dblaikie, echristo, probinson, aprantl.
kromanova added a subscriber: cfe-commits.
I had a review opened for anonymous namespaces bug in llvm-commits (see
http://reviews.llvm.org/D7895). It was opened in llvm-commits is because the
first patch that I submitted was in the backend. However it seems that the
frontend patch will be more appropriate in this case. David Blaikie advised to
get cfe developers opinion, specifically Richard Smith's.
Rather than just adding cfe-dev to the reviewers list, I thought it will be
better to open a brand new review.
There was quite a long discussion in llvm-commits list about this bug and
several re-implementations of a fix, so it gets confusing after a while.
I will write a short summary of what was discussed in
http://reviews.llvm.org/D7895
Problem #1:
DW_TAG_imported_module is missing from the compile unit's root scope. Without
this tag, anonymous namespace is not imported into the root scope by PS4
debugger.
Consider the following testcase. Lack of DW_TAG_imported_module prevents our
debugger from displaying the value of ‘a’.
#include
namespace
{
int a = 5;
}
int main()
{
printf("%d\n", a);
return 0;
}
Problem #2:
Another (more general) problem is that while the clang compiler doesn't
generate DW_TAG_imported_module for the top-level anonymous namespace, it
generates this tag for nested anonymous namespaces. So, we have an asymmetrical
situation.
namespace
{
namespace {
int a1 = 5;
}
int a2 = 7;
}
int *b1 = &a1;
int *b2 = &a2;
DWARF:
<1><54>: Abbrev Number: 5 (DW_TAG_namespace)
<55> DW_AT_decl_file : 1
<56> DW_AT_decl_line : 2
<2><57>: Abbrev Number: 5 (DW_TAG_namespace)
<58> DW_AT_decl_file : 1
<59> DW_AT_decl_line : 3
<2><8d>: Abbrev Number: 7 (DW_TAG_imported_module)
<8e> DW_AT_import : <0x57> [Abbrev Number: 5 (DW_TAG_namespace)]
Note that DW_TAG_import is not generated for the top-level anonymous namespace.
There was a long discussion if we want to generate explicit imports marked by
DW_AT_artificial attribute for anonymous namespaces in Clang or not. Both GCC
and ICC compilers generate explicit imports in this case. However, it's an
arguable gray area between the compiler and the debugger. The final decision
was *not* to generate DW_TAG_imported_module for all the platforms except PS4.
On PS4 we definitely want it, since our debugger doesn't import the anonymous
namespace automatically.
There were several different patches for this fix. For this review, I'm
providing the latest FE patch only. It takes care of both problem #1 and
problem #2.
See http://reviews.llvm.org/D7895 for more details/earlier patches.
Richard, could you please give your opinion on how that problem needs to get
taken care of in the FE. I don't mind re-writing patch based on your comments,
if necessary.
Thank you!
http://reviews.llvm.org/D12624
Files:
include/clang/Parse/Parser.h
include/clang/Sema/Sema.h
lib/CodeGen/CGDebugInfo.cpp
lib/Parse/ParseDecl.cpp
lib/Parse/ParseDeclCXX.cpp
lib/Sema/SemaDeclCXX.cpp
test/CodeGenCXX/debug-info-anon-namespace.cpp
Index: test/CodeGenCXX/debug-info-anon-namespace.cpp
===
--- test/CodeGenCXX/debug-info-anon-namespace.cpp
+++ test/CodeGenCXX/debug-info-anon-namespace.cpp
@@ -0,0 +1,26 @@
+// RUN: %clang_cc1 -emit-llvm -g -triple x86_64-scei-ps4 -O0 %s -o - | FileCheck --check-prefix=PS4 %s
+// RUN: %clang_cc1 -emit-llvm -g -triple x86_64-unknown-linux-gnu -O0 %s -o - | FileCheck --check-prefix=NON-PS4 %s
+
+namespace
+{
+ int a = 5;
+}
+int *b = &a;
+
+namespace
+{
+ namespace {
+int a1 = 5;
+ }
+ int a2 = 7;
+}
+int *b1 = &a1;
+int *b2 = &a2;
+
+
+// PS4: [[NS:![0-9]+]] = !DINamespace
+// PS4: [[NS2:![0-9]+]] = !DINamespace
+// PS4: !DIImportedEntity(tag: DW_TAG_imported_module, scope: !0, entity: [[NS]])
+// PS4: !DIImportedEntity(tag: DW_TAG_imported_module, scope: [[NS]], entity: [[NS2]], line: {{[0-9]+}})
+// NON-PS4-NOT: !DIImportedEntity
+
Index: lib/Sema/SemaDeclCXX.cpp
===
--- lib/Sema/SemaDeclCXX.cpp
+++ lib/Sema/SemaDeclCXX.cpp
@@ -7185,7 +7185,8 @@
SourceLocation IdentLoc,
IdentifierInfo *II,
SourceLocation LBrace,
- AttributeList *AttrList) {
+ AttributeList *AttrList,
+ UsingDirectiveDecl *& UD) {
SourceLocation StartLoc = InlineLoc.isValid() ? InlineLoc : NamespaceLoc;
// For anonymous namespace, take the location of the left brace.
SourceLocation Loc = II ? IdentLoc : LBrace;
@@ -7309,14 +7310,13 @@
// namespace internal linkage.
if (!PrevNS) {
- UsingDirectiveDecl* UD
Re: [PATCH] D13482: Revised Initial patch for PS4 toolchain
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 diff that is available for download doesn't mention these files. Most likely this happens because .keep files are empty. 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
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 the problem described in (1). (3) A comment is added in ToolChains.cpp to emphasize that the waning about missing PS4 SDK headers and libs is ignored by default. Repository: rL LLVM http://reviews.llvm.org/D13482 Files: include/clang/Basic/DiagnosticDriverKinds.td include/clang/Basic/DiagnosticGroups.td lib/Driver/Driver.cpp lib/Driver/ToolChains.cpp lib/Driver/ToolChains.h lib/Driver/Tools.cpp lib/Driver/Tools.h lib/Frontend/InitHeaderSearch.cpp test/Driver/Inputs/scei-ps4_tree/target/include/.keep test/Driver/Inputs/scei-ps4_tree/target/include_common/.keep test/Driver/debug-options.c test/Driver/no-integrated-as.s test/Driver/ps4-header-search.c test/Driver/ps4-linker-non-win.c test/Driver/ps4-linker-win.c test/Driver/ps4-pic.c test/Driver/ps4-sdk-root.c test/Driver/stack-protector.c Index: test/Driver/stack-protector.c === --- test/Driver/stack-protector.c +++ test/Driver/stack-protector.c @@ -23,3 +23,12 @@ // RUN: %clang -fstack-protector-all -### %s 2>&1 | FileCheck %s -check-prefix=SSP-ALL // SSP-ALL: "-stack-protector" "3" // SSP-ALL-NOT: "-stack-protector-buffer-size" + +// RUN: %clang -target x86_64-scei-ps4 -### %s 2>&1 | FileCheck %s -check-prefix=SSP-PS4 +// RUN: %clang -target x86_64-scei-ps4 -fstack-protector -### %s 2>&1 | FileCheck %s -check-prefix=SSP-PS4 +// SSP-PS4: "-stack-protector" "2" +// SSP-PS4-NOT: "-stack-protector-buffer-size" + +// RUN: %clang -target x86_64-scei-ps4 -fstack-protector --param ssp-buffer-size=16 -### %s 2>&1 | FileCheck %s -check-prefix=SSP-PS4-BUF +// SSP-PS4-BUF: "-stack-protector" "2" +// SSP-PS4-BUF: "-stack-protector-buffer-size" "16" Index: test/Driver/ps4-sdk-root.c === --- test/Driver/ps4-sdk-root.c +++ test/Driver/ps4-sdk-root.c @@ -0,0 +1,48 @@ +// REQUIRES: x86-registered-target + +// Check that ps4-clang doesn't report a warning message when locating +// system header files (either by looking at the value of SCE_PS4_SDK_DIR +// or relative to the location of the compiler driver), if "-nostdinc", +// "--sysroot" or "-isysroot" option is specified on the command line. +// Otherwise, check that ps4-clang reports a warning. + +// Check that clang doesn't report a warning message when locating +// system libraries (either by looking at the value of SCE_PS4_SDK_DIR +// or relative to the location of the compiler driver), if "-c", "-S", "-E", +// "--sysroot", "-nostdlib" or "-nodefaultlibs" option is specified on +// the command line. +// Otherwise, check that ps4-clang reports a warning. + +// setting up SCE_PS4_SDK_DIR to existing location, which is not a PS4 SDK. +// RUN: env SCE_PS4_SDK_DIR=.. %clang -Winvalid-or-nonexistent-directory -### -target x86_64-scei-ps4 %s 2>&1 | FileCheck -check-prefix=WARN-SYS-HEADERS -check-prefix=WARN-SYS-LIBS -check-prefix=NO-WARN %s + +// RUN: env SCE_PS4_SDK_DIR=.. %clang -Winvalid-or-nonexistent-directory -### -c -target x86_64-scei-ps4 %s 2>&1 | FileCheck -check-prefix=WARN-SYS-HEADERS -check-prefix=NO-WARN %s +// RUN: env SCE_PS4_SDK_DIR=.. %clang -Winvalid-or-nonexistent-directory -### -S -target x86_64-scei-ps4 %s 2>&1 | FileCheck -check-prefix=WARN-SYS-HEADERS -check-prefix=NO-WARN %s +// RUN: env SCE_PS4_SDK_DIR=.. %clang -Winvalid-or-nonexistent-directory -### -E -target x86_64-scei-ps4 %s 2>&1 | FileCheck -check-prefix=WARN-SYS-HEADERS -check-prefix=NO-WARN %s +// RUN: env SCE_PS4_SDK_DIR=.. %clang -Winvalid-or-nonexistent-directory -### -emit-ast -target x86_64-scei-ps4 %s 2>&1 | FileCheck -check-prefix=WARN-SYS-HEADERS -check-prefix=NO-WARN %s +// RUN: env SCE_PS4_SDK_DIR=.. %clang -Winvalid-or-nonexistent-directory -### -isysroot foo -target x86_64-scei-ps4 %s 2>&1 | FileCheck -check-prefix=WARN-ISYSROOT -check-prefix=WARN-SYS-LIBS -check-prefix=NO-WARN %s + +// RUN: env SCE_PS4_SDK_DIR=.. %clang -Winvalid-or-nonexistent-directory -### -c -nostdinc -target x86_64-scei-ps4 %s 2>&1 | FileCheck -check-prefix=NO-WARN %s +// RUN: env SCE_PS4_SDK_DIR=.. %clang -Winvalid-or-nonexistent-directory -### -S -nostdinc -target x86_64-scei-ps4 %s 2>&1 | FileCheck -check-prefix=NO-WARN %s +// RUN: env SCE_PS4_SDK_DIR=.. %clang -Winvalid-or-nonexistent-directory -### -E -nostdinc -target x86_64-scei-ps4 %s 2>&1 | FileCheck -check-prefix=NO-WARN %s +// RUN: env SCE_PS4_SDK_DIR=.. %clang -Winvalid-or-nonexistent-directory -### -emit-ast -nostdinc -target x86_64-scei-ps4 %s 2>&1 | FileCheck -check-prefix=NO-WARN %s + +// RUN: env SCE_PS4_SDK_DIR=.. %clang -Winvalid-or-nonexistent-directory -### -c --sysroot=foo/ -target x86_64-scei-ps4 %s 2>&1 | FileCheck -
Re: [PATCH] D13482: Revised Initial patch for PS4 toolchain
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
kromanova updated this revision to Diff 37292. kromanova added a comment. Updated the patch based on Jonathan's comments. Jonathan, if you have a minute, please review. Repository: rL LLVM http://reviews.llvm.org/D13482 Files: include/clang/Basic/DiagnosticDriverKinds.td include/clang/Basic/DiagnosticGroups.td lib/Driver/Driver.cpp lib/Driver/ToolChains.cpp lib/Driver/ToolChains.h lib/Driver/Tools.cpp lib/Driver/Tools.h lib/Frontend/InitHeaderSearch.cpp test/Driver/Inputs/scei-ps4_tree/target/include/.keep test/Driver/Inputs/scei-ps4_tree/target/include_common/.keep test/Driver/debug-options.c test/Driver/no-integrated-as.s test/Driver/ps4-header-search.c test/Driver/ps4-linker-non-win.c test/Driver/ps4-linker-win.c test/Driver/ps4-pic.c test/Driver/ps4-sdk-root.c test/Driver/stack-protector.c Index: test/Driver/stack-protector.c === --- test/Driver/stack-protector.c +++ test/Driver/stack-protector.c @@ -23,3 +23,12 @@ // RUN: %clang -fstack-protector-all -### %s 2>&1 | FileCheck %s -check-prefix=SSP-ALL // SSP-ALL: "-stack-protector" "3" // SSP-ALL-NOT: "-stack-protector-buffer-size" + +// RUN: %clang -target x86_64-scei-ps4 -### %s 2>&1 | FileCheck %s -check-prefix=SSP-PS4 +// RUN: %clang -target x86_64-scei-ps4 -fstack-protector -### %s 2>&1 | FileCheck %s -check-prefix=SSP-PS4 +// SSP-PS4: "-stack-protector" "2" +// SSP-PS4-NOT: "-stack-protector-buffer-size" + +// RUN: %clang -target x86_64-scei-ps4 -fstack-protector --param ssp-buffer-size=16 -### %s 2>&1 | FileCheck %s -check-prefix=SSP-PS4-BUF +// SSP-PS4-BUF: "-stack-protector" "2" +// SSP-PS4-BUF: "-stack-protector-buffer-size" "16" Index: test/Driver/ps4-sdk-root.c === --- test/Driver/ps4-sdk-root.c +++ test/Driver/ps4-sdk-root.c @@ -0,0 +1,48 @@ +// REQUIRES: x86-registered-target + +// Check that ps4-clang doesn't report a warning message when locating +// system header files (either by looking at the value of SCE_PS4_SDK_DIR +// or relative to the location of the compiler driver), if "-nostdinc", +// "--sysroot" or "-isysroot" option is specified on the command line. +// Otherwise, check that ps4-clang reports a warning. + +// Check that clang doesn't report a warning message when locating +// system libraries (either by looking at the value of SCE_PS4_SDK_DIR +// or relative to the location of the compiler driver), if "-c", "-S", "-E", +// "--sysroot", "-nostdlib" or "-nodefaultlibs" option is specified on +// the command line. +// Otherwise, check that ps4-clang reports a warning. + +// setting up SCE_PS4_SDK_DIR to existing location, which is not a PS4 SDK. +// RUN: env SCE_PS4_SDK_DIR=.. %clang -Winvalid-or-nonexistent-directory -### -target x86_64-scei-ps4 %s 2>&1 | FileCheck -check-prefix=WARN-SYS-HEADERS -check-prefix=WARN-SYS-LIBS -check-prefix=NO-WARN %s + +// RUN: env SCE_PS4_SDK_DIR=.. %clang -Winvalid-or-nonexistent-directory -### -c -target x86_64-scei-ps4 %s 2>&1 | FileCheck -check-prefix=WARN-SYS-HEADERS -check-prefix=NO-WARN %s +// RUN: env SCE_PS4_SDK_DIR=.. %clang -Winvalid-or-nonexistent-directory -### -S -target x86_64-scei-ps4 %s 2>&1 | FileCheck -check-prefix=WARN-SYS-HEADERS -check-prefix=NO-WARN %s +// RUN: env SCE_PS4_SDK_DIR=.. %clang -Winvalid-or-nonexistent-directory -### -E -target x86_64-scei-ps4 %s 2>&1 | FileCheck -check-prefix=WARN-SYS-HEADERS -check-prefix=NO-WARN %s +// RUN: env SCE_PS4_SDK_DIR=.. %clang -Winvalid-or-nonexistent-directory -### -emit-ast -target x86_64-scei-ps4 %s 2>&1 | FileCheck -check-prefix=WARN-SYS-HEADERS -check-prefix=NO-WARN %s +// RUN: env SCE_PS4_SDK_DIR=.. %clang -Winvalid-or-nonexistent-directory -### -isysroot foo -target x86_64-scei-ps4 %s 2>&1 | FileCheck -check-prefix=WARN-ISYSROOT -check-prefix=WARN-SYS-LIBS -check-prefix=NO-WARN %s + +// RUN: env SCE_PS4_SDK_DIR=.. %clang -Winvalid-or-nonexistent-directory -### -c -nostdinc -target x86_64-scei-ps4 %s 2>&1 | FileCheck -check-prefix=NO-WARN %s +// RUN: env SCE_PS4_SDK_DIR=.. %clang -Winvalid-or-nonexistent-directory -### -S -nostdinc -target x86_64-scei-ps4 %s 2>&1 | FileCheck -check-prefix=NO-WARN %s +// RUN: env SCE_PS4_SDK_DIR=.. %clang -Winvalid-or-nonexistent-directory -### -E -nostdinc -target x86_64-scei-ps4 %s 2>&1 | FileCheck -check-prefix=NO-WARN %s +// RUN: env SCE_PS4_SDK_DIR=.. %clang -Winvalid-or-nonexistent-directory -### -emit-ast -nostdinc -target x86_64-scei-ps4 %s 2>&1 | FileCheck -check-prefix=NO-WARN %s + +// RUN: env SCE_PS4_SDK_DIR=.. %clang -Winvalid-or-nonexistent-directory -### -c --sysroot=foo/ -target x86_64-scei-ps4 %s 2>&1 | FileCheck -check-prefix=NO-WARN %s +// RUN: env SCE_PS4_SDK_DIR=.. %clang -Winvalid-or-nonexistent-directory -### -S --sysroot=foo/ -target x86_64-scei-ps4 %s 2>&1 | FileCheck -check-prefix=NO-WARN %s +// RUN: env SCE_PS4_SDK_DIR=.. %clang -Winvalid-or-nonexistent-directory -### -E --sysroot=foo/ -targe
Re: [PATCH] D13482: Revised Initial patch for PS4 toolchain
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&id=37301#toc
Repository:
rL LLVM
http://reviews.llvm.org/D13482
Files:
cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td
cfe/trunk/include/clang/Basic/DiagnosticGroups.td
cfe/trunk/lib/Driver/Driver.cpp
cfe/trunk/lib/Driver/ToolChains.cpp
cfe/trunk/lib/Driver/ToolChains.h
cfe/trunk/lib/Driver/Tools.cpp
cfe/trunk/lib/Driver/Tools.h
cfe/trunk/lib/Frontend/InitHeaderSearch.cpp
cfe/trunk/test/Driver/Inputs/scei-ps4_tree/target/include/.keep
cfe/trunk/test/Driver/Inputs/scei-ps4_tree/target/include_common/.keep
cfe/trunk/test/Driver/debug-options.c
cfe/trunk/test/Driver/no-integrated-as.s
cfe/trunk/test/Driver/ps4-header-search.c
cfe/trunk/test/Driver/ps4-linker-non-win.c
cfe/trunk/test/Driver/ps4-linker-win.c
cfe/trunk/test/Driver/ps4-pic.c
cfe/trunk/test/Driver/ps4-sdk-root.c
cfe/trunk/test/Driver/stack-protector.c
Index: cfe/trunk/lib/Driver/ToolChains.h
===
--- cfe/trunk/lib/Driver/ToolChains.h
+++ cfe/trunk/lib/Driver/ToolChains.h
@@ -1015,6 +1015,27 @@
llvm::opt::ArgStringList &CC1Args) const override;
};
+class LLVM_LIBRARY_VISIBILITY PS4CPU : public Generic_ELF {
+public:
+ PS4CPU(const Driver &D, const llvm::Triple &Triple,
+ const llvm::opt::ArgList &Args);
+
+ bool IsMathErrnoDefault() const override { return false; }
+ bool IsObjCNonFragileABIDefault() const override { return true; }
+ bool HasNativeLLVMSupport() const override;
+ bool isPICDefault() const override;
+
+ unsigned GetDefaultStackProtectorLevel(bool KernelOrKext) const override {
+return 2; // SSPStrong
+ }
+
+ SanitizerMask getSupportedSanitizers() const override;
+
+protected:
+ Tool *buildAssembler() const override;
+ Tool *buildLinker() const override;
+};
+
} // end namespace toolchains
} // end namespace driver
} // end namespace clang
Index: cfe/trunk/lib/Driver/Tools.h
===
--- cfe/trunk/lib/Driver/Tools.h
+++ cfe/trunk/lib/Driver/Tools.h
@@ -831,6 +831,36 @@
};
} // end namespace Myriad
+namespace PS4cpu {
+class LLVM_LIBRARY_VISIBILITY Assemble : public Tool {
+public:
+ Assemble(const ToolChain &TC)
+ : Tool("PS4cpu::Assemble", "assembler", TC, RF_Full) {}
+
+ virtual bool hasIntegratedCPP() const { return false; }
+
+ virtual void ConstructJob(Compilation &C, const JobAction &JA,
+const InputInfo &Output,
+const InputInfoList &Inputs,
+const llvm::opt::ArgList &TCArgs,
+const char *LinkingOutput) const;
+};
+
+class LLVM_LIBRARY_VISIBILITY Link : public Tool {
+public:
+ Link(const ToolChain &TC) : Tool("PS4cpu::Link", "linker", TC, RF_Full) {}
+
+ virtual bool hasIntegratedCPP() const { return false; }
+ virtual bool isLinkJob() const { return true; }
+
+ virtual void ConstructJob(Compilation &C, const JobAction &JA,
+const InputInfo &Output,
+const InputInfoList &Inputs,
+const llvm::opt::ArgList &TCArgs,
+const char *LinkingOutput) const;
+};
+} // end namespace PS4cpu
+
} // end namespace tools
} // end namespace driver
} // end namespace clang
Index: cfe/trunk/lib/Driver/ToolChains.cpp
===
--- cfe/trunk/lib/Driver/ToolChains.cpp
+++ cfe/trunk/lib/Driver/ToolChains.cpp
@@ -4066,3 +4066,77 @@
options::OPT_fno_use_init_array, true))
CC1Args.push_back("-fuse-init-array");
}
+
+PS4CPU::PS4CPU(const Driver &D, const llvm::Triple &Triple, const ArgList &Args)
+: Generic_ELF(D, Triple, Args) {
+ if (Args.hasArg(options::OPT_static))
+D.Diag(diag::err_drv_unsupported_opt_for_target) << "-static" << "PS4";
+
+ // Determine where to find the PS4 libraries. We use SCE_PS4_SDK_DIR
+ // if it exists; otherwise use the driver's installation path, which
+ // should be /host_tools/bin.
+
+ SmallString<512> PS4SDKDir;
+ if (const char *EnvValue = getenv("SCE_PS4_SDK_DIR"))
+if (!llvm::sys::fs::exists(EnvValue))
+ getDriver().Diag(clang::diag::warn_drv_ps4_sdk_dir) << EnvValue;
+PS4SDKDir = EnvValue;
+ } else {
+PS4SDKDir = getDriver().Dir;
+llvm::sys::path::append(PS4SDKDir, "/../../");
+ }
+
+ // By default, the driver won't report a warning if it can't find
+ // PS4's include or lib directories. This behavior could be changed if
+ // -Weverything or -Winvalid-or-nonexistent-directory options are passed.
+ // If -isysroot was passed, use that as the SDK base path.
+ std::
Re: [PATCH] D13482: Revised Initial patch for PS4 toolchain
kromanova added a comment. Hi, The initial PS4 patch caused a test failure (debug-options.c) on the PS4 bot. I suspect that I know why the problem happens, but I'm not sure what will be the best way to handle it. If someone knows how to fix this test more "elegantly", I would appreciate their advice. FAIL: Clang :: Driver/debug-options.c (3509 of 24708) - TEST 'Clang :: Driver/debug-options.c' FAILED Script: --- /home/buildbot/Buildbot/Slave/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/./bin/clang -### -c -g /home/buildbot/Buildbot/Slave/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.src/tools/clang/test/Driver/debug-options.c -target x86_64-linux-gnu 2>&1 | /home/buildbot/Buildbot/Slave/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/./bin/FileCheck -check-prefix=G /home/buildbot/Buildbot/Slave/llvm-clang-lld-x86_64-scei- /home/buildbot/Buildbot/Slave/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/./bin/clang -### -g /home/buildbot/Buildbot/Slave/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.src/tools/clang/test/Driver/debug-options.c 2>&1 | /home/buildbot/Buildbot/Slave/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/./bin/FileCheck -check-prefix=CI /home/buildbot/Buildbot/Slave/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.src/tools/clang/test/Driver/debug-options.c Exit Code: 1 Command Output (stderr): /home/buildbot/Buildbot/Slave/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.src/tools/clang/test/Driver/debug-options.c:139:8: error: expected string not found in input // CI: "-dwarf-column-info" ^ :1:1: note: scanning from here clang version 3.8.0 (trunk 250262) ^ :5:438: note: possible intended match here "/home/buildbot/Buildbot/Slave/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/bin/clang-3.8" "-cc1" "-triple" "x86_64-scei-ps4" "-emit-obj" "-mrelax-all" "-disable-free" "-main-file-name" "debug-options.c" "-mrelocation-model" "pic" "-pic-level" "2" "-mthread-model" "posix" "-mdisable-fp-elim" "-masm-verbose" "-mconstructor-aliases" "-munwind-tables" "-target-cpu" "btver2" "-momit-leaf-frame-pointer" "-debug-info-kind=limited" "-dwarf-version=4" "-backend-option" "-generate-arange-section" "-resource-dir" "/home/buildbot/Buildbot/Slave/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/bin/../lib/clang/3.8.0" "-fdebug-compilation-dir" "/home/buildbot/Buildbot/Slave/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.obj/tools/clang/test/Driver" "-ferror-limit" "19" "-fmessage-length" "0" "-stack-protector" "2" "-fdeclspec" "-fobjc-runtime=gnustep" "-fdiagnostics-show-option" "-o" "/tmp/debug-options-1505f5.o" "-x" "c" "/home/buildbot/Buildbot/Slave/llvm-clang-lld-x86_64-scei-ps4-ubuntu-fast/llvm.src/tools/clang/test/Driver/debug-options.c" ^ - The latest driver patch introduced a change, causing the PS4 driver *not* to have -gcolumn-info enabled by default. Consequently, this generic line started to fail on the PS4 bot. // RUN: %clang -### -g %s 2>&1 | FileCheck -check-prefix=CI %s Does someone know what will be the best way to run the test line for all the platforms, except PS4? In the patch, we have added a couple of PS4 specific lines to this test, to verify that the behavior on PS4 is correct: // RUN: %clang -### -c %s -g -target x86_64-scei-ps4 2>&1 \ // RUN: | FileCheck -check-prefix=NOCI %s // RUN: %clang -### -c %s -g -gcolumn-info -target x86_64-scei-ps4 2>&1 \ // RUN: | FileCheck -check-prefix=CI %s I do not want to make this test XFAIL for PS4 (though I might do it as an interim solution). I would also prefer to avoid duplicating most of the content of this file into a separate ps4-specific file. Any ideas how to handle this issue "more elegantly"? If nobody objects, I will mark this test as XFAIL for PS4 for a time being. 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
[PATCH] D16913: Adding doxygen comments to the LLVM intrinsics (part 4, pmmintrin.h)
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 in the header
file pmmintrin.h.
The doxygen comments are automatically generated based on SCE internal
intrinsics document using DCG tool that I wrote.
I will submit more doxygen comments for the other intrinsic header files as
soon as this patch is approved.
Here is the link to the general discussion about adding comments to x86
intrinsics headers.
http://permalink.gmane.org/gmane.comp.compilers.clang.devel/42032
Here are the links to the similar code reviews for the doxygen comments other
header files.
http://reviews.llvm.org/D8762 (closed)
http://reviews.llvm.org/D15999 (closed)
http://reviews.llvm.org/D16562 (closed)
Katya.
Repository:
rL LLVM
http://reviews.llvm.org/D16913
Files:
pmmintrin.h
Index: pmmintrin.h
===
--- pmmintrin.h
+++ pmmintrin.h
@@ -27,64 +27,231 @@
#include
/* Define the default attributes for the functions in this file. */
-#define __DEFAULT_FN_ATTRS __attribute__((__always_inline__, __nodebug__, __target__("sse3")))
+#define __DEFAULT_FN_ATTRS \
+ __attribute__((__always_inline__, __nodebug__, __target__("sse3")))
+/// \brief Loads data from an unaligned memory location to elements in a 128-bit
+///vector. If the address of the data is not 16-byte aligned, the
+///instruction may read two adjacent aligned blocks of memory to retrieve
+///the requested data.
+///
+/// \headerfile
+///
+/// This intrinsic corresponds to the \c VLDDQU instruction.
+///
+/// \param __p
+///A pointer to a 128-bit integer vector containing integer values.
+/// \returns A 128-bit vector containing the moved values.
static __inline__ __m128i __DEFAULT_FN_ATTRS
_mm_lddqu_si128(__m128i const *__p)
{
return (__m128i)__builtin_ia32_lddqu((char const *)__p);
}
+/// \brief Adds the even-indexed values and subtracts the odd-indexed values of
+///two 128-bit vectors of [4 x float].
+///
+/// \headerfile
+///
+/// This intrinsic corresponds to the \c VADDSUBPS instruction.
+///
+/// \param __a
+///A 128-bit vector of [4 x float] containing the left source operand.
+/// \param __b
+///A 128-bit vector of [4 x float] containing the right source operand.
+/// \returns A 128-bit vector of [4 x float] containing the alternating sums and
+///differences of both operands.
static __inline__ __m128 __DEFAULT_FN_ATTRS
_mm_addsub_ps(__m128 __a, __m128 __b)
{
return __builtin_ia32_addsubps(__a, __b);
}
+/// \brief Horizontally adds the adjacent pairs of values contained in two
+///128-bit vectors of [4 x float].
+///
+/// \headerfile
+///
+/// This intrinsic corresponds to the \c VHADDPS instruction.
+///
+/// \param __a
+///A 128-bit vector of [4 x float] containing one of the source operands.
+///The horizontal sums of the values are stored in the lower bits of the
+///destination.
+/// \param __b
+///A 128-bit vector of [4 x float] containing one of the source operands.
+///The horizontal sums of the values are stored in the upper bits of the
+///destination.
+/// \returns A 128-bit vector of [4 x float] containing the horizontal sums of
+///both operands.
static __inline__ __m128 __DEFAULT_FN_ATTRS
_mm_hadd_ps(__m128 __a, __m128 __b)
{
return __builtin_ia32_haddps(__a, __b);
}
+/// \brief Horizontally subtracts the adjacent pairs of values contained in two
+///128-bit vectors of [4 x float].
+///
+/// \headerfile
+///
+/// This intrinsic corresponds to the \c VHSUBPS instruction.
+///
+/// \param __a
+///A 128-bit vector of [4 x float] containing one of the source operands.
+///The horizontal differences between the values are stored in the lower
+///bits of the destination.
+/// \param __b
+///A 128-bit vector of [4 x float] containing one of the source operands.
+///The horizontal differences between the values are stored in the upper
+///bits of the destination.
+/// \returns A 128-bit vector of [4 x float] containing the horizontal
+///differences of both operands.
static __inline__ __m128 __DEFAULT_FN_ATTRS
_mm_hsub_ps(__m128 __a, __m128 __b)
{
return __builtin_ia32_hsubps(__a, __b);
}
+/// \brief Moves and duplicates high-order (odd-indexed) values from a 128-bit
+///vector of [4 x float] to float values stored in a 128-bit vector of
+///[4 x float].
+///Bits [127:96] of the source are written to bits [127:96] and [95:64] of
+///the destination.
+///Bits [63:32] of the source are written to bits [63:32] and [31:0] of the
+///destination.
+///
+/// \headerfile
+///
+/// This intrinsic corresponds to the \c VMOVSHDUP instruction.
+///
+/// \param __a
+///A 128-bi
Re: [PATCH] D16913: Adding doxygen comments to the LLVM intrinsics (part 4, pmmintrin.h)
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&id=47250#toc
Repository:
rL LLVM
http://reviews.llvm.org/D16913
Files:
cfe/trunk/lib/Headers/pmmintrin.h
Index: cfe/trunk/lib/Headers/pmmintrin.h
===
--- cfe/trunk/lib/Headers/pmmintrin.h
+++ cfe/trunk/lib/Headers/pmmintrin.h
@@ -27,64 +27,231 @@
#include
/* Define the default attributes for the functions in this file. */
-#define __DEFAULT_FN_ATTRS __attribute__((__always_inline__, __nodebug__, __target__("sse3")))
+#define __DEFAULT_FN_ATTRS \
+ __attribute__((__always_inline__, __nodebug__, __target__("sse3")))
+/// \brief Loads data from an unaligned memory location to elements in a 128-bit
+///vector. If the address of the data is not 16-byte aligned, the
+///instruction may read two adjacent aligned blocks of memory to retrieve
+///the requested data.
+///
+/// \headerfile
+///
+/// This intrinsic corresponds to the \c VLDDQU instruction.
+///
+/// \param __p
+///A pointer to a 128-bit integer vector containing integer values.
+/// \returns A 128-bit vector containing the moved values.
static __inline__ __m128i __DEFAULT_FN_ATTRS
_mm_lddqu_si128(__m128i const *__p)
{
return (__m128i)__builtin_ia32_lddqu((char const *)__p);
}
+/// \brief Adds the even-indexed values and subtracts the odd-indexed values of
+///two 128-bit vectors of [4 x float].
+///
+/// \headerfile
+///
+/// This intrinsic corresponds to the \c VADDSUBPS instruction.
+///
+/// \param __a
+///A 128-bit vector of [4 x float] containing the left source operand.
+/// \param __b
+///A 128-bit vector of [4 x float] containing the right source operand.
+/// \returns A 128-bit vector of [4 x float] containing the alternating sums and
+///differences of both operands.
static __inline__ __m128 __DEFAULT_FN_ATTRS
_mm_addsub_ps(__m128 __a, __m128 __b)
{
return __builtin_ia32_addsubps(__a, __b);
}
+/// \brief Horizontally adds the adjacent pairs of values contained in two
+///128-bit vectors of [4 x float].
+///
+/// \headerfile
+///
+/// This intrinsic corresponds to the \c VHADDPS instruction.
+///
+/// \param __a
+///A 128-bit vector of [4 x float] containing one of the source operands.
+///The horizontal sums of the values are stored in the lower bits of the
+///destination.
+/// \param __b
+///A 128-bit vector of [4 x float] containing one of the source operands.
+///The horizontal sums of the values are stored in the upper bits of the
+///destination.
+/// \returns A 128-bit vector of [4 x float] containing the horizontal sums of
+///both operands.
static __inline__ __m128 __DEFAULT_FN_ATTRS
_mm_hadd_ps(__m128 __a, __m128 __b)
{
return __builtin_ia32_haddps(__a, __b);
}
+/// \brief Horizontally subtracts the adjacent pairs of values contained in two
+///128-bit vectors of [4 x float].
+///
+/// \headerfile
+///
+/// This intrinsic corresponds to the \c VHSUBPS instruction.
+///
+/// \param __a
+///A 128-bit vector of [4 x float] containing one of the source operands.
+///The horizontal differences between the values are stored in the lower
+///bits of the destination.
+/// \param __b
+///A 128-bit vector of [4 x float] containing one of the source operands.
+///The horizontal differences between the values are stored in the upper
+///bits of the destination.
+/// \returns A 128-bit vector of [4 x float] containing the horizontal
+///differences of both operands.
static __inline__ __m128 __DEFAULT_FN_ATTRS
_mm_hsub_ps(__m128 __a, __m128 __b)
{
return __builtin_ia32_hsubps(__a, __b);
}
+/// \brief Moves and duplicates high-order (odd-indexed) values from a 128-bit
+///vector of [4 x float] to float values stored in a 128-bit vector of
+///[4 x float].
+///Bits [127:96] of the source are written to bits [127:96] and [95:64] of
+///the destination.
+///Bits [63:32] of the source are written to bits [63:32] and [31:0] of the
+///destination.
+///
+/// \headerfile
+///
+/// This intrinsic corresponds to the \c VMOVSHDUP instruction.
+///
+/// \param __a
+///A 128-bit vector of [4 x float].
+/// \returns A 128-bit vector of [4 x float] containing the moved and duplicated
+///values.
static __inline__ __m128 __DEFAULT_FN_ATTRS
_mm_movehdup_ps(__m128 __a)
{
return __builtin_shufflevector(__a, __a, 1, 1, 3, 3);
}
+/// \brief Duplicates low-order (even-indexed) values from a 128-bit
+///vector of [4 x float] to float values stored in a 128-bit vector of
+///[4 x float].
+///Bits [95:64] of the source are written to bits [127:96] and [95:64] of
+///the destination.
+///Bits [31:0] of the source are w
[PATCH] D17021: Adding doxygen comments to the LLVM intrinsics (part 5, f16cintrin.h)
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 comments
for x86 intrinsics without formal OK from the open source reviewers.
Post-commit reviews could be done later if needed. It should speed up the
process.
I still think it's worth posting a code review. At first, it's nice for
bookkeeping. At second, I made one tiny non-comment related change by replacing
the name of the parameter in one of the intrinsics. It's done for consistency
purpose. It might be the only thing that needs to be briefly looked at during
pre-commit review.
=
Here is the patch with the doxygen comments for the intrinsincs in the header
file f16cintrin.h.
The doxygen comments are automatically generated based on SCE internal
intrinsics document using DCG tool that I wrote.
I will submit more doxygen comments for the other intrinsic header files as
soon as this patch is approved.
Here is the link to the general discussion about adding comments to x86
intrinsics headers.
http://permalink.gmane.org/gmane.comp.compilers.clang.devel/42032
Here are the links to the similar code reviews for the doxygen comments other
header files.
http://reviews.llvm.org/D8762 (closed)
http://reviews.llvm.org/D15999 (closed)
http://reviews.llvm.org/D16562 (closed)
http://reviews.llvm.org/D16913 (closed)
Repository:
rL LLVM
http://reviews.llvm.org/D17021
Files:
f16cintrin.h
Index: f16cintrin.h
===
--- f16cintrin.h
+++ f16cintrin.h
@@ -29,28 +29,94 @@
#define __F16CINTRIN_H
/* Define the default attributes for the functions in this file. */
-#define __DEFAULT_FN_ATTRS \
+#define __DEFAULT_FN_ATTRS \
__attribute__((__always_inline__, __nodebug__, __target__("f16c")))
+/// \brief Converts a 16-bit half-precision float value into a 32-bit float
+///value.
+///
+/// \headerfile
+///
+/// This intrinsic corresponds to the \c VCVTPH2PS instruction.
+///
+/// \param a
+///A 16-bit half-precision float value.
+/// \returns The converted 32-bit float value.
static __inline float __DEFAULT_FN_ATTRS
_cvtsh_ss(unsigned short a)
{
__v8hi v = {(short)a, 0, 0, 0, 0, 0, 0, 0};
__v4sf r = __builtin_ia32_vcvtph2ps(v);
return r[0];
}
+/// \brief Converts a 32-bit single-precision float value to a 16-bit
+///half-precision float value.
+///
+/// \headerfile
+///
+/// \code
+/// unsigned short _cvtss_sh(float a, int imm);
+/// \endcode
+///
+/// This intrinsic corresponds to the \c VCVTPS2PH instruction.
+///
+/// \param a
+///A 32-bit single-precision float value to be converted to a 16-bit
+///half-precision float value.
+/// \param imm
+///An immediate value controlling rounding using bits [2:0]:
+///000: Nearest
+///001: Down
+///010: Up
+///011: Truncate
+///1XX: Use MXCSR.RC for rounding
+/// \returns The converted 16-bit half-precision float value.
#define _cvtss_sh(a, imm) \
((unsigned short)(((__v8hi)__builtin_ia32_vcvtps2ph((__v4sf){a, 0, 0, 0}, \
(imm)))[0]))
+/// \brief Converts a 128-bit vector containing 32-bit float values into a
+///128-bit vector containing 16-bit half-precision float values.
+///
+/// \headerfile
+///
+/// \code
+/// __m128i _mm_cvtps_ph(__m128 a, int imm);
+/// \endcode
+///
+/// This intrinsic corresponds to the \c VCVTPS2PH instruction.
+///
+/// \param a
+///A 128-bit vector containing 32-bit float values.
+/// \param imm
+///An immediate value controlling rounding using bits [2:0]:
+///000: Nearest
+///001: Down
+///010: Up
+///011: Truncate
+///1XX: Use MXCSR.RC for rounding
+/// \returns A 128-bit vector containing converted 16-bit half-precision float
+///values. The lower 64 bits are used to store the converted 16-bit
+///half-precision floating-point values.
#define _mm_cvtps_ph(a, imm) \
((__m128i)__builtin_ia32_vcvtps2ph((__v4sf)(__m128)(a), (imm)))
+/// \brief Converts a 128-bit vector containing 16-bit half-precision float
+///values into a 128-bit vector containing 32-bit float values.
+///
+/// \headerfile
+///
+/// This intrinsic corresponds to the \c VCVTPH2PS instruction.
+///
+/// \param a
+///A 128-bit vector containing 16-bit half-precision float values. The lower
+///64 bits are used in the conversion.
+/// \returns A 128-bit vector of [4 x float] containing converted float values.
static __inline __m128 __DEFAULT_FN_ATTRS
-_mm_cvtph_ps(__m128i __a)
+_mm_cvtph_ps(__m128i a)
{
- return (__m128)__builtin_ia32_vcvtph2ps((__v8hi)__a);
+ return (__m128)__builtin_ia32_vcvtph2ps((__v8hi)a);
}
#undef __DEFAULT_FN_ATTRS
_
Re: [PATCH] D17021: Adding doxygen comments to the LLVM intrinsics (part 5, f16cintrin.h)
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&id=47394#toc
Repository:
rL LLVM
http://reviews.llvm.org/D17021
Files:
cfe/trunk/lib/Headers/f16cintrin.h
Index: cfe/trunk/lib/Headers/f16cintrin.h
===
--- cfe/trunk/lib/Headers/f16cintrin.h
+++ cfe/trunk/lib/Headers/f16cintrin.h
@@ -29,28 +29,94 @@
#define __F16CINTRIN_H
/* Define the default attributes for the functions in this file. */
-#define __DEFAULT_FN_ATTRS \
+#define __DEFAULT_FN_ATTRS \
__attribute__((__always_inline__, __nodebug__, __target__("f16c")))
+/// \brief Converts a 16-bit half-precision float value into a 32-bit float
+///value.
+///
+/// \headerfile
+///
+/// This intrinsic corresponds to the \c VCVTPH2PS instruction.
+///
+/// \param a
+///A 16-bit half-precision float value.
+/// \returns The converted 32-bit float value.
static __inline float __DEFAULT_FN_ATTRS
_cvtsh_ss(unsigned short a)
{
__v8hi v = {(short)a, 0, 0, 0, 0, 0, 0, 0};
__v4sf r = __builtin_ia32_vcvtph2ps(v);
return r[0];
}
+/// \brief Converts a 32-bit single-precision float value to a 16-bit
+///half-precision float value.
+///
+/// \headerfile
+///
+/// \code
+/// unsigned short _cvtss_sh(float a, const int imm);
+/// \endcode
+///
+/// This intrinsic corresponds to the \c VCVTPS2PH instruction.
+///
+/// \param a
+///A 32-bit single-precision float value to be converted to a 16-bit
+///half-precision float value.
+/// \param imm
+///An immediate value controlling rounding using bits [2:0]:
+///000: Nearest
+///001: Down
+///010: Up
+///011: Truncate
+///1XX: Use MXCSR.RC for rounding
+/// \returns The converted 16-bit half-precision float value.
#define _cvtss_sh(a, imm) \
((unsigned short)(((__v8hi)__builtin_ia32_vcvtps2ph((__v4sf){a, 0, 0, 0}, \
(imm)))[0]))
+/// \brief Converts a 128-bit vector containing 32-bit float values into a
+///128-bit vector containing 16-bit half-precision float values.
+///
+/// \headerfile
+///
+/// \code
+/// __m128i _mm_cvtps_ph(__m128 a, const int imm);
+/// \endcode
+///
+/// This intrinsic corresponds to the \c VCVTPS2PH instruction.
+///
+/// \param a
+///A 128-bit vector containing 32-bit float values.
+/// \param imm
+///An immediate value controlling rounding using bits [2:0]:
+///000: Nearest
+///001: Down
+///010: Up
+///011: Truncate
+///1XX: Use MXCSR.RC for rounding
+/// \returns A 128-bit vector containing converted 16-bit half-precision float
+///values. The lower 64 bits are used to store the converted 16-bit
+///half-precision floating-point values.
#define _mm_cvtps_ph(a, imm) \
((__m128i)__builtin_ia32_vcvtps2ph((__v4sf)(__m128)(a), (imm)))
+/// \brief Converts a 128-bit vector containing 16-bit half-precision float
+///values into a 128-bit vector containing 32-bit float values.
+///
+/// \headerfile
+///
+/// This intrinsic corresponds to the \c VCVTPH2PS instruction.
+///
+/// \param a
+///A 128-bit vector containing 16-bit half-precision float values. The lower
+///64 bits are used in the conversion.
+/// \returns A 128-bit vector of [4 x float] containing converted float values.
static __inline __m128 __DEFAULT_FN_ATTRS
-_mm_cvtph_ps(__m128i __a)
+_mm_cvtph_ps(__m128i a)
{
- return (__m128)__builtin_ia32_vcvtph2ps((__v8hi)__a);
+ return (__m128)__builtin_ia32_vcvtph2ps((__v8hi)a);
}
#undef __DEFAULT_FN_ATTRS
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D17550: Adding doxygen comments to the LLVM intrinsics (part 6, popcntintrin.h)
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 internal
intrinsics document using DCG tool that I wrote.
I will submit more doxygen comments for the other intrinsic header files as
soon as this patch is approved.
Here is the link to the general discussion about adding comments to x86
intrinsics headers.
http://permalink.gmane.org/gmane.comp.compilers.clang.devel/42032
Here are the links to the similar code reviews for the doxygen comments in some
other header files.
http://reviews.llvm.org/D8762 (closed)
http://reviews.llvm.org/D15999 (closed)
http://reviews.llvm.org/D16562 (closed)
http://reviews.llvm.org/D16913 (closed)
http://reviews.llvm.org/D17021 (closed)
Repository:
rL LLVM
http://reviews.llvm.org/D17550
Files:
popcntintrin.h
Index: popcntintrin.h
===
--- popcntintrin.h
+++ popcntintrin.h
@@ -27,25 +27,65 @@
/* Define the default attributes for the functions in this file. */
#define __DEFAULT_FN_ATTRS __attribute__((__always_inline__, __nodebug__,
__target__("popcnt")))
+/// \brief Counts the number of bits in the source operand having a value of 1.
+///
+/// \headerfile
+///
+/// This intrinsic corresponds to the \c POPCNT instruction.
+///
+/// \param __A
+///An unsigned 32-bit integer operand.
+/// \returns A 32-bit integer containing the number of bits with value 1 in the
+///source operand.
static __inline__ int __DEFAULT_FN_ATTRS
_mm_popcnt_u32(unsigned int __A)
{
return __builtin_popcount(__A);
}
+/// \brief Counts the number of bits in the source operand having a value of 1.
+///
+/// \headerfile
+///
+/// This intrinsic corresponds to the \c POPCNT instruction.
+///
+/// \param __A
+///A signed 32-bit integer operand.
+/// \returns A 32-bit integer containing the number of bits with value 1 in the
+///source operand.
static __inline__ int __DEFAULT_FN_ATTRS
_popcnt32(int __A)
{
return __builtin_popcount(__A);
}
#ifdef __x86_64__
+/// \brief Counts the number of bits in the source operand having a value of 1.
+///
+/// \headerfile
+///
+/// This intrinsic corresponds to the \c POPCNT instruction.
+///
+/// \param __A
+///An unsigned 64-bit integer operand.
+/// \returns A 64-bit integer containing the number of bits with value 1 in the
+///source operand.
static __inline__ long long __DEFAULT_FN_ATTRS
_mm_popcnt_u64(unsigned long long __A)
{
return __builtin_popcountll(__A);
}
+/// \brief Counts the number of bits in the source operand having a value of 1.
+///
+/// \headerfile
+///
+/// This intrinsic corresponds to the \c POPCNT instruction.
+///
+/// \param __A
+///A signed 64-bit integer operand.
+/// \returns A 64-bit integer containing the number of bits with value 1 in the
+///source operand.
static __inline__ long long __DEFAULT_FN_ATTRS
_popcnt64(long long __A)
{
Index: popcntintrin.h
===
--- popcntintrin.h
+++ popcntintrin.h
@@ -27,25 +27,65 @@
/* Define the default attributes for the functions in this file. */
#define __DEFAULT_FN_ATTRS __attribute__((__always_inline__, __nodebug__, __target__("popcnt")))
+/// \brief Counts the number of bits in the source operand having a value of 1.
+///
+/// \headerfile
+///
+/// This intrinsic corresponds to the \c POPCNT instruction.
+///
+/// \param __A
+///An unsigned 32-bit integer operand.
+/// \returns A 32-bit integer containing the number of bits with value 1 in the
+///source operand.
static __inline__ int __DEFAULT_FN_ATTRS
_mm_popcnt_u32(unsigned int __A)
{
return __builtin_popcount(__A);
}
+/// \brief Counts the number of bits in the source operand having a value of 1.
+///
+/// \headerfile
+///
+/// This intrinsic corresponds to the \c POPCNT instruction.
+///
+/// \param __A
+///A signed 32-bit integer operand.
+/// \returns A 32-bit integer containing the number of bits with value 1 in the
+///source operand.
static __inline__ int __DEFAULT_FN_ATTRS
_popcnt32(int __A)
{
return __builtin_popcount(__A);
}
#ifdef __x86_64__
+/// \brief Counts the number of bits in the source operand having a value of 1.
+///
+/// \headerfile
+///
+/// This intrinsic corresponds to the \c POPCNT instruction.
+///
+/// \param __A
+///An unsigned 64-bit integer operand.
+/// \returns A 64-bit integer containing the number of bits with value 1 in the
+///source operand.
static __inline__ long long __DEFAULT_FN_ATTRS
_mm_popcnt_u64(unsigned long long __A)
{
return __builtin_popcountll(__A);
}
+/// \brief Counts the number of bits in the source operand having a value of 1.
+///
+/// \headerfile
+///
+/// This intrinsic corresponds to
[PATCH] D20614: Remove trailing spaces in x86 intrinsic headers
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:
__wmmintrin_aes.h
__wmmintrin_pclmul.h
avx512fintrin.h
avx512vldqintrin.h
mwaitxintrin.h
pmmintrin.h
Index: pmmintrin.h
===
--- pmmintrin.h
+++ pmmintrin.h
@@ -31,9 +31,9 @@
__attribute__((__always_inline__, __nodebug__, __target__("sse3")))
/// \brief Loads data from an unaligned memory location to elements in a 128-bit
-///vector. If the address of the data is not 16-byte aligned, the
-///instruction may read two adjacent aligned blocks of memory to retrieve
-///the requested data.
+///vector. If the address of the data is not 16-byte aligned, the
+///instruction may read two adjacent aligned blocks of memory to retrieve
+///the requested data.
///
/// \headerfile
///
@@ -75,14 +75,14 @@
/// This intrinsic corresponds to the \c VHADDPS instruction.
///
/// \param __a
-///A 128-bit vector of [4 x float] containing one of the source operands.
-///The horizontal sums of the values are stored in the lower bits of the
+///A 128-bit vector of [4 x float] containing one of the source operands.
+///The horizontal sums of the values are stored in the lower bits of the
///destination.
/// \param __b
-///A 128-bit vector of [4 x float] containing one of the source operands.
-///The horizontal sums of the values are stored in the upper bits of the
+///A 128-bit vector of [4 x float] containing one of the source operands.
+///The horizontal sums of the values are stored in the upper bits of the
///destination.
-/// \returns A 128-bit vector of [4 x float] containing the horizontal sums of
+/// \returns A 128-bit vector of [4 x float] containing the horizontal sums of
///both operands.
static __inline__ __m128 __DEFAULT_FN_ATTRS
_mm_hadd_ps(__m128 __a, __m128 __b)
@@ -98,14 +98,14 @@
/// This intrinsic corresponds to the \c VHSUBPS instruction.
///
/// \param __a
-///A 128-bit vector of [4 x float] containing one of the source operands.
-///The horizontal differences between the values are stored in the lower
+///A 128-bit vector of [4 x float] containing one of the source operands.
+///The horizontal differences between the values are stored in the lower
///bits of the destination.
/// \param __b
-///A 128-bit vector of [4 x float] containing one of the source operands.
-///The horizontal differences between the values are stored in the upper
+///A 128-bit vector of [4 x float] containing one of the source operands.
+///The horizontal differences between the values are stored in the upper
///bits of the destination.
-/// \returns A 128-bit vector of [4 x float] containing the horizontal
+/// \returns A 128-bit vector of [4 x float] containing the horizontal
///differences of both operands.
static __inline__ __m128 __DEFAULT_FN_ATTRS
_mm_hsub_ps(__m128 __a, __m128 __b)
@@ -168,7 +168,7 @@
///A 128-bit vector of [2 x double] containing the left source operand.
/// \param __b
///A 128-bit vector of [2 x double] containing the right source operand.
-/// \returns A 128-bit vector of [2 x double] containing the alternating sums
+/// \returns A 128-bit vector of [2 x double] containing the alternating sums
///and differences of both operands.
static __inline__ __m128d __DEFAULT_FN_ATTRS
_mm_addsub_pd(__m128d __a, __m128d __b)
@@ -176,7 +176,7 @@
return __builtin_ia32_addsubpd((__v2df)__a, (__v2df)__b);
}
-/// \brief Horizontally adds the pairs of values contained in two 128-bit
+/// \brief Horizontally adds the pairs of values contained in two 128-bit
///vectors of [2 x double].
///
/// \headerfile
@@ -184,12 +184,12 @@
/// This intrinsic corresponds to the \c VHADDPD instruction.
///
/// \param __a
-///A 128-bit vector of [2 x double] containing one of the source operands.
-///The horizontal sum of the values is stored in the lower bits of the
+///A 128-bit vector of [2 x double] containing one of the source operands.
+///The horizontal sum of the values is stored in the lower bits of the
///destination.
/// \param __b
-///A 128-bit vector of [2 x double] containing one of the source operands.
-///The horizontal sum of the values is stored in the upper bits of the
+///A 128-bit vector of [2 x double] containing one of the source operands.
+///The horizontal sum of the values is stored in the upper bits of the
///destination.
/// \returns A 128-bit vector of [2 x double] containing the horizontal sums of
///both operands.
@@ -207,14 +207,14 @@
/// This intrinsic corresponds to the \c VHSUBPD instruction.
///
/// \
Re: [PATCH] D20614: Remove trailing spaces in x86 intrinsic headers
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 header files in the intrinsics header directory. That's why
some of the files that you recently touched were affected.
Repository:
rL LLVM
http://reviews.llvm.org/D20614
Files:
__wmmintrin_aes.h
__wmmintrin_pclmul.h
avx512fintrin.h
avx512vldqintrin.h
mwaitxintrin.h
pmmintrin.h
Index: pmmintrin.h
===
--- pmmintrin.h
+++ pmmintrin.h
@@ -31,9 +31,9 @@
__attribute__((__always_inline__, __nodebug__, __target__("sse3")))
/// \brief Loads data from an unaligned memory location to elements in a 128-bit
-///vector. If the address of the data is not 16-byte aligned, the
-///instruction may read two adjacent aligned blocks of memory to retrieve
-///the requested data.
+///vector. If the address of the data is not 16-byte aligned, the
+///instruction may read two adjacent aligned blocks of memory to retrieve
+///the requested data.
///
/// \headerfile
///
@@ -75,14 +75,14 @@
/// This intrinsic corresponds to the \c VHADDPS instruction.
///
/// \param __a
-///A 128-bit vector of [4 x float] containing one of the source operands.
-///The horizontal sums of the values are stored in the lower bits of the
+///A 128-bit vector of [4 x float] containing one of the source operands.
+///The horizontal sums of the values are stored in the lower bits of the
///destination.
/// \param __b
-///A 128-bit vector of [4 x float] containing one of the source operands.
-///The horizontal sums of the values are stored in the upper bits of the
+///A 128-bit vector of [4 x float] containing one of the source operands.
+///The horizontal sums of the values are stored in the upper bits of the
///destination.
-/// \returns A 128-bit vector of [4 x float] containing the horizontal sums of
+/// \returns A 128-bit vector of [4 x float] containing the horizontal sums of
///both operands.
static __inline__ __m128 __DEFAULT_FN_ATTRS
_mm_hadd_ps(__m128 __a, __m128 __b)
@@ -98,14 +98,14 @@
/// This intrinsic corresponds to the \c VHSUBPS instruction.
///
/// \param __a
-///A 128-bit vector of [4 x float] containing one of the source operands.
-///The horizontal differences between the values are stored in the lower
+///A 128-bit vector of [4 x float] containing one of the source operands.
+///The horizontal differences between the values are stored in the lower
///bits of the destination.
/// \param __b
-///A 128-bit vector of [4 x float] containing one of the source operands.
-///The horizontal differences between the values are stored in the upper
+///A 128-bit vector of [4 x float] containing one of the source operands.
+///The horizontal differences between the values are stored in the upper
///bits of the destination.
-/// \returns A 128-bit vector of [4 x float] containing the horizontal
+/// \returns A 128-bit vector of [4 x float] containing the horizontal
///differences of both operands.
static __inline__ __m128 __DEFAULT_FN_ATTRS
_mm_hsub_ps(__m128 __a, __m128 __b)
@@ -168,28 +168,28 @@
///A 128-bit vector of [2 x double] containing the left source operand.
/// \param __b
///A 128-bit vector of [2 x double] containing the right source operand.
-/// \returns A 128-bit vector of [2 x double] containing the alternating sums
+/// \returns A 128-bit vector of [2 x double] containing the alternating sums
///and differences of both operands.
static __inline__ __m128d __DEFAULT_FN_ATTRS
_mm_addsub_pd(__m128d __a, __m128d __b)
{
return __builtin_ia32_addsubpd((__v2df)__a, (__v2df)__b);
}
-/// \brief Horizontally adds the pairs of values contained in two 128-bit
+/// \brief Horizontally adds the pairs of values contained in two 128-bit
///vectors of [2 x double].
///
/// \headerfile
///
/// This intrinsic corresponds to the \c VHADDPD instruction.
///
/// \param __a
-///A 128-bit vector of [2 x double] containing one of the source operands.
-///The horizontal sum of the values is stored in the lower bits of the
+///A 128-bit vector of [2 x double] containing one of the source operands.
+///The horizontal sum of the values is stored in the lower bits of the
///destination.
/// \param __b
-///A 128-bit vector of [2 x double] containing one of the source operands.
-///The horizontal sum of the values is stored in the upper bits of the
+///A 128-bit vector of [2 x double] containing one of the source operands.
+///The horizontal sum of the values is stored in the upper bits of the
///destination.
/// \returns A 128-bit vector of [2 x double] con
Re: [PATCH] D20614: Remove trailing spaces in x86 intrinsic headers
This revision was automatically updated to reflect the committed changes.
Closed by commit rL271077: Clean up: remove trailing spaces in x86 intrinsic
headers. (authored by kromanova).
Changed prior to commit:
http://reviews.llvm.org/D20614?vs=58472&id=58869#toc
Repository:
rL LLVM
http://reviews.llvm.org/D20614
Files:
cfe/trunk/lib/Headers/__wmmintrin_aes.h
cfe/trunk/lib/Headers/__wmmintrin_pclmul.h
cfe/trunk/lib/Headers/avx512fintrin.h
cfe/trunk/lib/Headers/avx512vldqintrin.h
cfe/trunk/lib/Headers/mwaitxintrin.h
cfe/trunk/lib/Headers/pmmintrin.h
Index: cfe/trunk/lib/Headers/avx512fintrin.h
===
--- cfe/trunk/lib/Headers/avx512fintrin.h
+++ cfe/trunk/lib/Headers/avx512fintrin.h
@@ -949,7 +949,7 @@
(__v2df)_mm_setzero_pd(), \
(__mmask8)(U), (int)(R)); })
-static __inline __m512i
+static __inline __m512i
__DEFAULT_FN_ATTRS
_mm512_max_epi32(__m512i __A, __m512i __B)
{
@@ -1508,16 +1508,16 @@
}
static __inline__ __m128 __DEFAULT_FN_ATTRS
-_mm_mask_rsqrt14_ss (__m128 __W, __mmask8 __U, __m128 __A, __m128 __B)
+_mm_mask_rsqrt14_ss (__m128 __W, __mmask8 __U, __m128 __A, __m128 __B)
{
return (__m128) __builtin_ia32_rsqrt14ss_mask ((__v4sf) __A,
(__v4sf) __B,
(__v4sf) __W,
(__mmask8) __U);
}
static __inline__ __m128 __DEFAULT_FN_ATTRS
-_mm_maskz_rsqrt14_ss (__mmask8 __U, __m128 __A, __m128 __B)
+_mm_maskz_rsqrt14_ss (__mmask8 __U, __m128 __A, __m128 __B)
{
return (__m128) __builtin_ia32_rsqrt14ss_mask ((__v4sf) __A,
(__v4sf) __B,
@@ -1536,16 +1536,16 @@
}
static __inline__ __m128d __DEFAULT_FN_ATTRS
-_mm_mask_rsqrt14_sd (__m128d __W, __mmask8 __U, __m128d __A, __m128d __B)
+_mm_mask_rsqrt14_sd (__m128d __W, __mmask8 __U, __m128d __A, __m128d __B)
{
return (__m128d) __builtin_ia32_rsqrt14sd_mask ( (__v2df) __A,
(__v2df) __B,
(__v2df) __W,
(__mmask8) __U);
}
static __inline__ __m128d __DEFAULT_FN_ATTRS
-_mm_maskz_rsqrt14_sd (__mmask8 __U, __m128d __A, __m128d __B)
+_mm_maskz_rsqrt14_sd (__mmask8 __U, __m128d __A, __m128d __B)
{
return (__m128d) __builtin_ia32_rsqrt14sd_mask ( (__v2df) __A,
(__v2df) __B,
@@ -1616,16 +1616,16 @@
}
static __inline__ __m128 __DEFAULT_FN_ATTRS
-_mm_mask_rcp14_ss (__m128 __W, __mmask8 __U, __m128 __A, __m128 __B)
+_mm_mask_rcp14_ss (__m128 __W, __mmask8 __U, __m128 __A, __m128 __B)
{
return (__m128) __builtin_ia32_rcp14ss_mask ((__v4sf) __A,
(__v4sf) __B,
(__v4sf) __W,
(__mmask8) __U);
}
static __inline__ __m128 __DEFAULT_FN_ATTRS
-_mm_maskz_rcp14_ss (__mmask8 __U, __m128 __A, __m128 __B)
+_mm_maskz_rcp14_ss (__mmask8 __U, __m128 __A, __m128 __B)
{
return (__m128) __builtin_ia32_rcp14ss_mask ((__v4sf) __A,
(__v4sf) __B,
@@ -1644,16 +1644,16 @@
}
static __inline__ __m128d __DEFAULT_FN_ATTRS
-_mm_mask_rcp14_sd (__m128d __W, __mmask8 __U, __m128d __A, __m128d __B)
+_mm_mask_rcp14_sd (__m128d __W, __mmask8 __U, __m128d __A, __m128d __B)
{
return (__m128d) __builtin_ia32_rcp14sd_mask ( (__v2df) __A,
(__v2df) __B,
(__v2df) __W,
(__mmask8) __U);
}
static __inline__ __m128d __DEFAULT_FN_ATTRS
-_mm_maskz_rcp14_sd (__mmask8 __U, __m128d __A, __m128d __B)
+_mm_maskz_rcp14_sd (__mmask8 __U, __m128d __A, __m128d __B)
{
return (__m128d) __builtin_ia32_rcp14sd_mask ( (__v2df) __A,
(__v2df) __B,
@@ -3306,7 +3306,7 @@
(__v16si)(__m512i)(B), (int)(I), \
(__v16si)_mm512_setzero_si512(), \
(__mmask16)-1); })
-
+
#define _mm512_mask_alignr_epi32(W, U, A, B, imm) __extension__ ({\
(__m512i)__builtin_ia32_alignd512_mask((__v16si)(__m512i)(A), \
(__v16si)(__m512i)(B), (int)(imm), \
@@ -3488,7 +3488,7 @@
_mm512_cvtepi32_pd(__m256i __A)
{
return (__m512d) __builtin_ia32_cvtdq2pd512_mask ((__v8si) __A,
-(__v8df)
+(__v8df)
_mm512_setzero_pd (),
(__mmask8) -1);
}
@@ -3613,7 +3613,7 @@
_mm512_cvtph_ps(__m256i __A)
{
return (__m512) __builtin_ia32_vcvtph2ps512_mask ((__v16hi) __A,
-(__v16sf)
+(__v16sf)
_mm512_setzero_ps (),
(__mmask16) -1,
_MM_FROUND_CUR_DIRECTION);
@@ -3733,7 +3733,7 @@
(__mmask16) __U,
_MM_FROUND_CUR_DIRECTION);
}
-
+
#define _mm512_cvt_roundpd_epi32(A, R) __extension__ ({ \
(__m256i)__builtin_ia32_cvtpd2dq512_mask((__v8df)(__m512d)(A), \
(__v8si)_mm256_setzero_si256(), \
@@ -3774,7 +3774,7 @@
Re: [PATCH] D17550: Adding doxygen comments to the LLVM intrinsics (part 6, popcntintrin.h)
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&id=49534#toc
Repository:
rL LLVM
http://reviews.llvm.org/D17550
Files:
cfe/trunk/lib/Headers/popcntintrin.h
Index: cfe/trunk/lib/Headers/popcntintrin.h
===
--- cfe/trunk/lib/Headers/popcntintrin.h
+++ cfe/trunk/lib/Headers/popcntintrin.h
@@ -27,25 +27,65 @@
/* Define the default attributes for the functions in this file. */
#define __DEFAULT_FN_ATTRS __attribute__((__always_inline__, __nodebug__,
__target__("popcnt")))
+/// \brief Counts the number of bits in the source operand having a value of 1.
+///
+/// \headerfile
+///
+/// This intrinsic corresponds to the \c POPCNT instruction.
+///
+/// \param __A
+///An unsigned 32-bit integer operand.
+/// \returns A 32-bit integer containing the number of bits with value 1 in the
+///source operand.
static __inline__ int __DEFAULT_FN_ATTRS
_mm_popcnt_u32(unsigned int __A)
{
return __builtin_popcount(__A);
}
+/// \brief Counts the number of bits in the source operand having a value of 1.
+///
+/// \headerfile
+///
+/// This intrinsic corresponds to the \c POPCNT instruction.
+///
+/// \param __A
+///A signed 32-bit integer operand.
+/// \returns A 32-bit integer containing the number of bits with value 1 in the
+///source operand.
static __inline__ int __DEFAULT_FN_ATTRS
_popcnt32(int __A)
{
return __builtin_popcount(__A);
}
#ifdef __x86_64__
+/// \brief Counts the number of bits in the source operand having a value of 1.
+///
+/// \headerfile
+///
+/// This intrinsic corresponds to the \c POPCNT instruction.
+///
+/// \param __A
+///An unsigned 64-bit integer operand.
+/// \returns A 64-bit integer containing the number of bits with value 1 in the
+///source operand.
static __inline__ long long __DEFAULT_FN_ATTRS
_mm_popcnt_u64(unsigned long long __A)
{
return __builtin_popcountll(__A);
}
+/// \brief Counts the number of bits in the source operand having a value of 1.
+///
+/// \headerfile
+///
+/// This intrinsic corresponds to the \c POPCNT instruction.
+///
+/// \param __A
+///A signed 64-bit integer operand.
+/// \returns A 64-bit integer containing the number of bits with value 1 in the
+///source operand.
static __inline__ long long __DEFAULT_FN_ATTRS
_popcnt64(long long __A)
{
Index: cfe/trunk/lib/Headers/popcntintrin.h
===
--- cfe/trunk/lib/Headers/popcntintrin.h
+++ cfe/trunk/lib/Headers/popcntintrin.h
@@ -27,25 +27,65 @@
/* Define the default attributes for the functions in this file. */
#define __DEFAULT_FN_ATTRS __attribute__((__always_inline__, __nodebug__, __target__("popcnt")))
+/// \brief Counts the number of bits in the source operand having a value of 1.
+///
+/// \headerfile
+///
+/// This intrinsic corresponds to the \c POPCNT instruction.
+///
+/// \param __A
+///An unsigned 32-bit integer operand.
+/// \returns A 32-bit integer containing the number of bits with value 1 in the
+///source operand.
static __inline__ int __DEFAULT_FN_ATTRS
_mm_popcnt_u32(unsigned int __A)
{
return __builtin_popcount(__A);
}
+/// \brief Counts the number of bits in the source operand having a value of 1.
+///
+/// \headerfile
+///
+/// This intrinsic corresponds to the \c POPCNT instruction.
+///
+/// \param __A
+///A signed 32-bit integer operand.
+/// \returns A 32-bit integer containing the number of bits with value 1 in the
+///source operand.
static __inline__ int __DEFAULT_FN_ATTRS
_popcnt32(int __A)
{
return __builtin_popcount(__A);
}
#ifdef __x86_64__
+/// \brief Counts the number of bits in the source operand having a value of 1.
+///
+/// \headerfile
+///
+/// This intrinsic corresponds to the \c POPCNT instruction.
+///
+/// \param __A
+///An unsigned 64-bit integer operand.
+/// \returns A 64-bit integer containing the number of bits with value 1 in the
+///source operand.
static __inline__ long long __DEFAULT_FN_ATTRS
_mm_popcnt_u64(unsigned long long __A)
{
return __builtin_popcountll(__A);
}
+/// \brief Counts the number of bits in the source operand having a value of 1.
+///
+/// \headerfile
+///
+/// This intrinsic corresponds to the \c POPCNT instruction.
+///
+/// \param __A
+///A signed 64-bit integer operand.
+/// \returns A 64-bit integer containing the number of bits with value 1 in the
+///source operand.
static __inline__ long long __DEFAULT_FN_ATTRS
_popcnt64(long long __A)
{
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D15999: Adding doxygen comments to the LLVM intrinsics (part 2, _wmmintrin_pclmul.h)
kromanova created this revision. kromanova added reviewers: gribozavr, jroelofs, ygao. 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 in the header file __wmmintrin_pclmul.h. The doxygen comments are automatically generated based on our internal intrinsics document. I will submit more doxygen comments for the other intrinsic header files as soon as this patch is approved. Here is the link to the general discussion about adding comments to x86 intrinsics headers. http://permalink.gmane.org/gmane.comp.compilers.clang.devel/42032 Here is the link to the similar code review for the doxygen comments for the header file ammintrin.h "Adding doxygen comments to the LLVM intrinsics (part 1, ammintrin.h)" http://reviews.llvm.org/D8762 Please review. Katya. Repository: rL LLVM http://reviews.llvm.org/D15999 Files: lib/Headers/__wmmintrin_pclmul.h Index: lib/Headers/__wmmintrin_pclmul.h === --- lib/Headers/__wmmintrin_pclmul.h +++ lib/Headers/__wmmintrin_pclmul.h @@ -23,6 +23,36 @@ #ifndef _WMMINTRIN_PCLMUL_H #define _WMMINTRIN_PCLMUL_H +/// \brief Multiplies two 64-bit integer values, selected from the operands +///using the immediate value operand. The multiplication is a carry-less +///multiplication, and the 128-bit integer product is stored in the +///destination. +/// +/// \headerfile +/// +/// \code +/// __m128i _mm_clmulepi64_si128(__m128i __X, __m128i __Y, const int __I); +/// \endcode +/// +/// This intrinsic corresponds to \c VPCLMULQDQ instruction. +/// +/// \param __X +///A 128-bit vector of [2 x i64] containing one of the source +///operands. +/// \param __Y +///A 128-bit vector of [2 x i64] containing one of the source +///operands. +/// \param __I +///An immediate value specifying which 64-bit values to select +///from the operands. +///Bit 0 is used to select a value from operand __X, +///and bit 4 is used to select a value from operand __Y: +///Bit[0]=0 indicates that bits[63:0] of operand __X are used. +///Bit[0]=1 indicates that bits[127:64] of operand __X are used. +///Bit[4]=0 indicates that bits[63:0] of operand __Y are used. +///Bit[4]=1 indicates that bits[127:64] of operand __Y are used. +/// \returns The 128-bit integer vector containing the result of the carry-less +///multiplication of the selected 64-bit values. #define _mm_clmulepi64_si128(__X, __Y, __I) \ ((__m128i)__builtin_ia32_pclmulqdq128((__v2di)(__m128i)(__X), \ (__v2di)(__m128i)(__Y), (char)(__I))) Index: lib/Headers/__wmmintrin_pclmul.h === --- lib/Headers/__wmmintrin_pclmul.h +++ lib/Headers/__wmmintrin_pclmul.h @@ -23,6 +23,36 @@ #ifndef _WMMINTRIN_PCLMUL_H #define _WMMINTRIN_PCLMUL_H +/// \brief Multiplies two 64-bit integer values, selected from the operands +///using the immediate value operand. The multiplication is a carry-less +///multiplication, and the 128-bit integer product is stored in the +///destination. +/// +/// \headerfile +/// +/// \code +/// __m128i _mm_clmulepi64_si128(__m128i __X, __m128i __Y, const int __I); +/// \endcode +/// +/// This intrinsic corresponds to \c VPCLMULQDQ instruction. +/// +/// \param __X +///A 128-bit vector of [2 x i64] containing one of the source +///operands. +/// \param __Y +///A 128-bit vector of [2 x i64] containing one of the source +///operands. +/// \param __I +///An immediate value specifying which 64-bit values to select +///from the operands. +///Bit 0 is used to select a value from operand __X, +///and bit 4 is used to select a value from operand __Y: +///Bit[0]=0 indicates that bits[63:0] of operand __X are used. +///Bit[0]=1 indicates that bits[127:64] of operand __X are used. +///Bit[4]=0 indicates that bits[63:0] of operand __Y are used. +///Bit[4]=1 indicates that bits[127:64] of operand __Y are used. +/// \returns The 128-bit integer vector containing the result of the carry-less +///multiplication of the selected 64-bit values. #define _mm_clmulepi64_si128(__X, __Y, __I) \ ((__m128i)__builtin_ia32_pclmulqdq128((__v2di)(__m128i)(__X), \ (__v2di)(__m128i)(__Y), (char)(__I))) ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D16177: Adding missing intrinsics _cvtsh_ss and _cvtss_sh
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/mailman/listinfo/cfe-commits
Re: [PATCH] D15999: Adding doxygen comments to the LLVM intrinsics (part 2, _wmmintrin_pclmul.h)
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 comments to check that all the > extra comment skipping doesn't affect compilation time? I.e. run your script > to add the comments for "all" the intrinsic headers (similar to what you > expect the final state to be after all these patches) and test the build time > of a game (and compare with the unmodified headers). This makes sense. I can do this, though it might take a couple of days, since I don't have a setup for a game build. I suspect that that we shouldn't use an application/game that is using precompiled headers. Any other limitation should be imposed on the beanchmark? > Also, can you post a patch that changes "all" the headers to have doxygen > comments like you intend, so that others can test and verify? Yes, I'll do this. However, there are a couple of things that the people should be aware: (a) *only* intrinsics that are supported on PS4 will have doxygen comments. PS4 documentation is parsed to generate these comments, and it doesn't document the intrinsics that are not supported on PS4. (b) the doxygen comments for the rest (not submitted upstream) headers files might have some problems (typos, incorrect parameter types, bugs in the description, etc). The tool that generates the comments might have some glitches too. Most of this things get fixed after the manual review that I do before I submit the Phabricator code review. So, please don't assume that this is the final version of the header files with doxygen comments. Repository: rL LLVM http://reviews.llvm.org/D15999 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D15999: Adding doxygen comments to the LLVM intrinsics (part 2, _wmmintrin_pclmul.h)
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 comments to check that all the > extra comment skipping doesn't affect compilation time? I.e. run your script > to add the comments for "all" the intrinsic headers (similar to what you > expect the final state to be after all these patches) and test the build time > of a game (and compare with the unmodified headers). > > Also, can you post a patch that changes "all" the headers to have doxygen > comments like you intend, so that others can test and verify? Out of curiosity, do you know if the impact to the build time for Eric's change of starting to use the target attributes instead of conditional inclusion was measured (r239883) on a large scale application. If so, what were the results? I should probably include Eric. Repository: rL LLVM http://reviews.llvm.org/D15999 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D15999: Adding doxygen comments to the LLVM intrinsics (part 2, _wmmintrin_pclmul.h)
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 else pretty much) > include the intrinsic headers in basically every TU so the impact is probably > not a huge concern overall. However, games include these headers in basically > every TU so we at SCE should take a look. Yes, typical PS4 codebase will heavily include x86 intrinsics headers. At the same time, typical PS4 codebase will use precompiled headers or modules for most of the time, so I hope the real world impact of doxygen comment will not be huge in any case. BTW, in my experiment, I'm planning to measure worst case scenario (no modules, no PCH). I think t(ahe benefit of having (1) tooltips and autocomplication in XCode, (2) Intellisense tooltips in MS Visual Studio, (3) internal intrinsics documentation for clang, (4) ability to generate up-to-date internal intrinsic documentation based on the latest Clang headers, etc, should overweigh the drawback of slower compile time for the applications/games that are heavily using intrinsic headers, but opted not to use modules or PCH. Still, I agree that we need to make this measurement. And I'm curious too :) Repository: rL LLVM http://reviews.llvm.org/D15999 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D15999: Adding doxygen comments to the LLVM intrinsics (part 2, _wmmintrin_pclmul.h)
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 > > > comments like you intend, so that others can test and verify? > > > > > > Yes, I'll do this. However, there are a couple of things that the people > > should be aware: > > (a) *only* intrinsics that are supported on PS4 will have doxygen > > comments. PS4 documentation is parsed to generate these comments, and it > > doesn't document the intrinsics that are not supported on PS4. > > > If we don't have coverage of every intrinsic, we will need to add > documentation for Clang developers about the comment format so that future > intrinsics (and intrinsics not covered) can be documented. That was the plan... The consistency of the doxygen documentation will be especially important for the individuals/companies that decide to generate their up-to-date intrinsics guide based on the latest Clang headers. One doesn't want to see differently formatted intrinsics guide. > > > > (b) the doxygen comments for the rest (not submitted upstream) headers > > files might have some problems (typos, incorrect parameter types, bugs in > > the description, etc). The tool that generates the comments might have some > > glitches too. Most of this things get fixed after the manual review that I > > do before I submit the Phabricator code review. So, please don't assume > > that this is the final version of the header files with doxygen comments. > > > Sure. Makes sense. Repository: rL LLVM http://reviews.llvm.org/D15999 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D16177: Adding missing intrinsics _cvtsh_ss and _cvtss_sh
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/f16c-builtins.c
===
--- test/CodeGen/f16c-builtins.c
+++ test/CodeGen/f16c-builtins.c
@@ -5,6 +5,18 @@
#include
+float test_cvtsh_ss(unsigned short a) {
+ // CHECK-LABEL: test_cvtsh_ss
+ // CHECK: @llvm.x86.vcvtph2ps.128
+ return _cvtsh_ss(a);
+}
+
+unsigned short test_cvtss_sh(float a) {
+ // CHECK-LABEL: test_cvtss_sh
+ // CHECK: @llvm.x86.vcvtps2ph.128
+ return _cvtss_sh(a, 0);
+}
+
__m128 test_mm_cvtph_ps(__m128i a) {
// CHECK-LABEL: test_mm_cvtph_ps
// CHECK: @llvm.x86.vcvtph2ps.128
Index: lib/Headers/f16cintrin.h
===
--- lib/Headers/f16cintrin.h
+++ lib/Headers/f16cintrin.h
@@ -31,6 +31,20 @@
/* Define the default attributes for the functions in this file. */
#define __DEFAULT_FN_ATTRS __attribute__((__always_inline__, __nodebug__,
__target__("f16c")))
+static __inline float __DEFAULT_FN_ATTRS
+_cvtsh_ss(unsigned short a)
+{
+ __v8hi v = {(short)a, 0, 0, 0, 0, 0, 0, 0};
+ __v4sf r = __builtin_ia32_vcvtph2ps(v);
+ return r[0];
+}
+
+#define _cvtss_sh(a, imm) __extension__ ({ \
+ __v8hi r = __builtin_ia32_vcvtps2ph((__v4sf){a, 0, 0, 0}, (imm)); \
+ (unsigned short)r[0]; \
+})
+
+
#define _mm_cvtps_ph(a, imm) __extension__ ({ \
(__m128i)__builtin_ia32_vcvtps2ph((__v4sf)(__m128)(a), (imm)); })
Index: test/CodeGen/f16c-builtins.c
===
--- test/CodeGen/f16c-builtins.c
+++ test/CodeGen/f16c-builtins.c
@@ -5,6 +5,18 @@
#include
+float test_cvtsh_ss(unsigned short a) {
+ // CHECK-LABEL: test_cvtsh_ss
+ // CHECK: @llvm.x86.vcvtph2ps.128
+ return _cvtsh_ss(a);
+}
+
+unsigned short test_cvtss_sh(float a) {
+ // CHECK-LABEL: test_cvtss_sh
+ // CHECK: @llvm.x86.vcvtps2ph.128
+ return _cvtss_sh(a, 0);
+}
+
__m128 test_mm_cvtph_ps(__m128i a) {
// CHECK-LABEL: test_mm_cvtph_ps
// CHECK: @llvm.x86.vcvtph2ps.128
Index: lib/Headers/f16cintrin.h
===
--- lib/Headers/f16cintrin.h
+++ lib/Headers/f16cintrin.h
@@ -31,6 +31,20 @@
/* Define the default attributes for the functions in this file. */
#define __DEFAULT_FN_ATTRS __attribute__((__always_inline__, __nodebug__, __target__("f16c")))
+static __inline float __DEFAULT_FN_ATTRS
+_cvtsh_ss(unsigned short a)
+{
+ __v8hi v = {(short)a, 0, 0, 0, 0, 0, 0, 0};
+ __v4sf r = __builtin_ia32_vcvtph2ps(v);
+ return r[0];
+}
+
+#define _cvtss_sh(a, imm) __extension__ ({ \
+ __v8hi r = __builtin_ia32_vcvtps2ph((__v4sf){a, 0, 0, 0}, (imm)); \
+ (unsigned short)r[0]; \
+})
+
+
#define _mm_cvtps_ph(a, imm) __extension__ ({ \
(__m128i)__builtin_ia32_vcvtps2ph((__v4sf)(__m128)(a), (imm)); })
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D16177: Adding missing intrinsics _cvtsh_ss and _cvtss_sh
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/CodeGen/f16c-builtins.c
===
--- test/CodeGen/f16c-builtins.c
+++ test/CodeGen/f16c-builtins.c
@@ -5,6 +5,18 @@
#include
+float test_cvtsh_ss(unsigned short a) {
+ // CHECK-LABEL: test_cvtsh_ss
+ // CHECK: @llvm.x86.vcvtph2ps.128
+ return _cvtsh_ss(a);
+}
+
+unsigned short test_cvtss_sh(float a) {
+ // CHECK-LABEL: test_cvtss_sh
+ // CHECK: @llvm.x86.vcvtps2ph.128
+ return _cvtss_sh(a, 0);
+}
+
__m128 test_mm_cvtph_ps(__m128i a) {
// CHECK-LABEL: test_mm_cvtph_ps
// CHECK: @llvm.x86.vcvtph2ps.128
Index: lib/Headers/f16cintrin.h
===
--- lib/Headers/f16cintrin.h
+++ lib/Headers/f16cintrin.h
@@ -31,6 +31,19 @@
/* Define the default attributes for the functions in this file. */
#define __DEFAULT_FN_ATTRS __attribute__((__always_inline__, __nodebug__,
__target__("f16c")))
+static __inline float __DEFAULT_FN_ATTRS
+_cvtsh_ss(unsigned short a)
+{
+ __v8hi v = {(short)a, 0, 0, 0, 0, 0, 0, 0};
+ __v4sf r = __builtin_ia32_vcvtph2ps(v);
+ return r[0];
+}
+
+#define _cvtss_sh(a, imm) __extension__ ({ \
+ (unsigned short)(((__v8hi)__builtin_ia32_vcvtps2ph((__v4sf){a, 0, 0, 0}, \
+ (imm)))[0]); })
+
+
#define _mm_cvtps_ph(a, imm) __extension__ ({ \
(__m128i)__builtin_ia32_vcvtps2ph((__v4sf)(__m128)(a), (imm)); })
Index: test/CodeGen/f16c-builtins.c
===
--- test/CodeGen/f16c-builtins.c
+++ test/CodeGen/f16c-builtins.c
@@ -5,6 +5,18 @@
#include
+float test_cvtsh_ss(unsigned short a) {
+ // CHECK-LABEL: test_cvtsh_ss
+ // CHECK: @llvm.x86.vcvtph2ps.128
+ return _cvtsh_ss(a);
+}
+
+unsigned short test_cvtss_sh(float a) {
+ // CHECK-LABEL: test_cvtss_sh
+ // CHECK: @llvm.x86.vcvtps2ph.128
+ return _cvtss_sh(a, 0);
+}
+
__m128 test_mm_cvtph_ps(__m128i a) {
// CHECK-LABEL: test_mm_cvtph_ps
// CHECK: @llvm.x86.vcvtph2ps.128
Index: lib/Headers/f16cintrin.h
===
--- lib/Headers/f16cintrin.h
+++ lib/Headers/f16cintrin.h
@@ -31,6 +31,19 @@
/* Define the default attributes for the functions in this file. */
#define __DEFAULT_FN_ATTRS __attribute__((__always_inline__, __nodebug__, __target__("f16c")))
+static __inline float __DEFAULT_FN_ATTRS
+_cvtsh_ss(unsigned short a)
+{
+ __v8hi v = {(short)a, 0, 0, 0, 0, 0, 0, 0};
+ __v4sf r = __builtin_ia32_vcvtph2ps(v);
+ return r[0];
+}
+
+#define _cvtss_sh(a, imm) __extension__ ({ \
+ (unsigned short)(((__v8hi)__builtin_ia32_vcvtps2ph((__v4sf){a, 0, 0, 0}, \
+ (imm)))[0]); })
+
+
#define _mm_cvtps_ph(a, imm) __extension__ ({ \
(__m128i)__builtin_ia32_vcvtps2ph((__v4sf)(__m128)(a), (imm)); })
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D16177: Adding missing intrinsics _cvtsh_ss and _cvtss_sh
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, imm) __extension__ ({ \
> (unsigned short)((__v8hi)__builtin_ia32_vcvtps2ph((__v4sf){a, 0, 0, 0},
> (imm))[0]); \
> })
Hi Craig,
I should have looked how it's done just a few lines below. Sorry.
I had to slightly modify the body of the define that you proposed by adding an
additional pair of round brackets, otherwise I got compilation errors like this:
~/ngh/ToT_commit/build/bin/clang intr.cpp -mf16c intr.cpp:10:7:
error: C-style cast from scalar 'short' to vector '__v8hi'
(vector of 8 'short' values) of different size
a = _cvtss_sh(res, imm);
^~~
~/ngh/ToT_commit/build/bin/../lib/clang/3.8.0/include/f16cintrin.h:43:20:
note:
expanded from macro '_cvtss_sh'
(unsigned short)((__v8hi)__builtin_ia32_vcvtps2ph((__v4sf){a, 0, 0, 0}, \
^~~~
intr.cpp:10:5: error: assigning to 'unsigned short' from incompatible type
'void'
a = _cvtss_sh(res, imm);
^ ~~~
2 errors generated.
Repository:
rL LLVM
http://reviews.llvm.org/D16177
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D16177: Adding missing intrinsics _cvtsh_ss and _cvtss_sh
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_FN_ATTRS macro to comply with 80 characters
limitation.
Repository:
rL LLVM
http://reviews.llvm.org/D16177
Files:
lib/Headers/f16cintrin.h
test/CodeGen/f16c-builtins.c
Index: test/CodeGen/f16c-builtins.c
===
--- test/CodeGen/f16c-builtins.c
+++ test/CodeGen/f16c-builtins.c
@@ -5,6 +5,18 @@
#include
+float test_cvtsh_ss(unsigned short a) {
+ // CHECK-LABEL: test_cvtsh_ss
+ // CHECK: @llvm.x86.vcvtph2ps.128
+ return _cvtsh_ss(a);
+}
+
+unsigned short test_cvtss_sh(float a) {
+ // CHECK-LABEL: test_cvtss_sh
+ // CHECK: @llvm.x86.vcvtps2ph.128
+ return _cvtss_sh(a, 0);
+}
+
__m128 test_mm_cvtph_ps(__m128i a) {
// CHECK-LABEL: test_mm_cvtph_ps
// CHECK: @llvm.x86.vcvtph2ps.128
Index: lib/Headers/f16cintrin.h
===
--- lib/Headers/f16cintrin.h
+++ lib/Headers/f16cintrin.h
@@ -29,10 +29,23 @@
#define __F16CINTRIN_H
/* Define the default attributes for the functions in this file. */
-#define __DEFAULT_FN_ATTRS __attribute__((__always_inline__, __nodebug__,
__target__("f16c")))
+#define __DEFAULT_FN_ATTRS \
+ __attribute__((__always_inline__, __nodebug__, __target__("f16c")))
-#define _mm_cvtps_ph(a, imm) __extension__ ({ \
- (__m128i)__builtin_ia32_vcvtps2ph((__v4sf)(__m128)(a), (imm)); })
+static __inline float __DEFAULT_FN_ATTRS
+_cvtsh_ss(unsigned short a)
+{
+ __v8hi v = {(short)a, 0, 0, 0, 0, 0, 0, 0};
+ __v4sf r = __builtin_ia32_vcvtph2ps(v);
+ return r[0];
+}
+
+#define _cvtss_sh(a, imm) \
+ ((unsigned short)(((__v8hi)__builtin_ia32_vcvtps2ph((__v4sf){a, 0, 0, 0}, \
+ (imm)))[0]))
+
+#define _mm_cvtps_ph(a, imm) \
+ ((__m128i)__builtin_ia32_vcvtps2ph((__v4sf)(__m128)(a), (imm)))
static __inline __m128 __DEFAULT_FN_ATTRS
_mm_cvtph_ps(__m128i __a)
Index: test/CodeGen/f16c-builtins.c
===
--- test/CodeGen/f16c-builtins.c
+++ test/CodeGen/f16c-builtins.c
@@ -5,6 +5,18 @@
#include
+float test_cvtsh_ss(unsigned short a) {
+ // CHECK-LABEL: test_cvtsh_ss
+ // CHECK: @llvm.x86.vcvtph2ps.128
+ return _cvtsh_ss(a);
+}
+
+unsigned short test_cvtss_sh(float a) {
+ // CHECK-LABEL: test_cvtss_sh
+ // CHECK: @llvm.x86.vcvtps2ph.128
+ return _cvtss_sh(a, 0);
+}
+
__m128 test_mm_cvtph_ps(__m128i a) {
// CHECK-LABEL: test_mm_cvtph_ps
// CHECK: @llvm.x86.vcvtph2ps.128
Index: lib/Headers/f16cintrin.h
===
--- lib/Headers/f16cintrin.h
+++ lib/Headers/f16cintrin.h
@@ -29,10 +29,23 @@
#define __F16CINTRIN_H
/* Define the default attributes for the functions in this file. */
-#define __DEFAULT_FN_ATTRS __attribute__((__always_inline__, __nodebug__, __target__("f16c")))
+#define __DEFAULT_FN_ATTRS \
+ __attribute__((__always_inline__, __nodebug__, __target__("f16c")))
-#define _mm_cvtps_ph(a, imm) __extension__ ({ \
- (__m128i)__builtin_ia32_vcvtps2ph((__v4sf)(__m128)(a), (imm)); })
+static __inline float __DEFAULT_FN_ATTRS
+_cvtsh_ss(unsigned short a)
+{
+ __v8hi v = {(short)a, 0, 0, 0, 0, 0, 0, 0};
+ __v4sf r = __builtin_ia32_vcvtph2ps(v);
+ return r[0];
+}
+
+#define _cvtss_sh(a, imm) \
+ ((unsigned short)(((__v8hi)__builtin_ia32_vcvtps2ph((__v4sf){a, 0, 0, 0}, \
+ (imm)))[0]))
+
+#define _mm_cvtps_ph(a, imm) \
+ ((__m128i)__builtin_ia32_vcvtps2ph((__v4sf)(__m128)(a), (imm)))
static __inline __m128 __DEFAULT_FN_ATTRS
_mm_cvtph_ps(__m128i __a)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D16177: Adding missing intrinsics _cvtsh_ss and _cvtss_sh
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 each test. My concern is that these additional lines might change from time to time. Repository: rL LLVM http://reviews.llvm.org/D16177 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D15999: Adding doxygen comments to the LLVM intrinsics (part 2, _wmmintrin_pclmul.h)
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 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D16177: Adding missing intrinsics _cvtsh_ss and _cvtss_sh
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:
rL LLVM
http://reviews.llvm.org/D16177
Files:
cfe/trunk/lib/Headers/f16cintrin.h
cfe/trunk/test/CodeGen/f16c-builtins.c
Index: cfe/trunk/lib/Headers/f16cintrin.h
===
--- cfe/trunk/lib/Headers/f16cintrin.h
+++ cfe/trunk/lib/Headers/f16cintrin.h
@@ -29,10 +29,23 @@
#define __F16CINTRIN_H
/* Define the default attributes for the functions in this file. */
-#define __DEFAULT_FN_ATTRS __attribute__((__always_inline__, __nodebug__,
__target__("f16c")))
+#define __DEFAULT_FN_ATTRS \
+ __attribute__((__always_inline__, __nodebug__, __target__("f16c")))
-#define _mm_cvtps_ph(a, imm) __extension__ ({ \
- (__m128i)__builtin_ia32_vcvtps2ph((__v4sf)(__m128)(a), (imm)); })
+static __inline float __DEFAULT_FN_ATTRS
+_cvtsh_ss(unsigned short a)
+{
+ __v8hi v = {(short)a, 0, 0, 0, 0, 0, 0, 0};
+ __v4sf r = __builtin_ia32_vcvtph2ps(v);
+ return r[0];
+}
+
+#define _cvtss_sh(a, imm) \
+ ((unsigned short)(((__v8hi)__builtin_ia32_vcvtps2ph((__v4sf){a, 0, 0, 0}, \
+ (imm)))[0]))
+
+#define _mm_cvtps_ph(a, imm) \
+ ((__m128i)__builtin_ia32_vcvtps2ph((__v4sf)(__m128)(a), (imm)))
static __inline __m128 __DEFAULT_FN_ATTRS
_mm_cvtph_ps(__m128i __a)
Index: cfe/trunk/test/CodeGen/f16c-builtins.c
===
--- cfe/trunk/test/CodeGen/f16c-builtins.c
+++ cfe/trunk/test/CodeGen/f16c-builtins.c
@@ -5,6 +5,18 @@
#include
+float test_cvtsh_ss(unsigned short a) {
+ // CHECK-LABEL: test_cvtsh_ss
+ // CHECK: @llvm.x86.vcvtph2ps.128
+ return _cvtsh_ss(a);
+}
+
+unsigned short test_cvtss_sh(float a) {
+ // CHECK-LABEL: test_cvtss_sh
+ // CHECK: @llvm.x86.vcvtps2ph.128
+ return _cvtss_sh(a, 0);
+}
+
__m128 test_mm_cvtph_ps(__m128i a) {
// CHECK-LABEL: test_mm_cvtph_ps
// CHECK: @llvm.x86.vcvtph2ps.128
Index: cfe/trunk/lib/Headers/f16cintrin.h
===
--- cfe/trunk/lib/Headers/f16cintrin.h
+++ cfe/trunk/lib/Headers/f16cintrin.h
@@ -29,10 +29,23 @@
#define __F16CINTRIN_H
/* Define the default attributes for the functions in this file. */
-#define __DEFAULT_FN_ATTRS __attribute__((__always_inline__, __nodebug__, __target__("f16c")))
+#define __DEFAULT_FN_ATTRS \
+ __attribute__((__always_inline__, __nodebug__, __target__("f16c")))
-#define _mm_cvtps_ph(a, imm) __extension__ ({ \
- (__m128i)__builtin_ia32_vcvtps2ph((__v4sf)(__m128)(a), (imm)); })
+static __inline float __DEFAULT_FN_ATTRS
+_cvtsh_ss(unsigned short a)
+{
+ __v8hi v = {(short)a, 0, 0, 0, 0, 0, 0, 0};
+ __v4sf r = __builtin_ia32_vcvtph2ps(v);
+ return r[0];
+}
+
+#define _cvtss_sh(a, imm) \
+ ((unsigned short)(((__v8hi)__builtin_ia32_vcvtps2ph((__v4sf){a, 0, 0, 0}, \
+ (imm)))[0]))
+
+#define _mm_cvtps_ph(a, imm) \
+ ((__m128i)__builtin_ia32_vcvtps2ph((__v4sf)(__m128)(a), (imm)))
static __inline __m128 __DEFAULT_FN_ATTRS
_mm_cvtph_ps(__m128i __a)
Index: cfe/trunk/test/CodeGen/f16c-builtins.c
===
--- cfe/trunk/test/CodeGen/f16c-builtins.c
+++ cfe/trunk/test/CodeGen/f16c-builtins.c
@@ -5,6 +5,18 @@
#include
+float test_cvtsh_ss(unsigned short a) {
+ // CHECK-LABEL: test_cvtsh_ss
+ // CHECK: @llvm.x86.vcvtph2ps.128
+ return _cvtsh_ss(a);
+}
+
+unsigned short test_cvtss_sh(float a) {
+ // CHECK-LABEL: test_cvtss_sh
+ // CHECK: @llvm.x86.vcvtps2ph.128
+ return _cvtss_sh(a, 0);
+}
+
__m128 test_mm_cvtph_ps(__m128i a) {
// CHECK-LABEL: test_mm_cvtph_ps
// CHECK: @llvm.x86.vcvtph2ps.128
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D15999: Adding doxygen comments to the LLVM intrinsics (part 2, _wmmintrin_pclmul.h)
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 neither. Repository: rL LLVM http://reviews.llvm.org/D15999 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D15999: Adding doxygen comments to the LLVM intrinsics (part 2, _wmmintrin_pclmul.h)
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: lib/Headers/__wmmintrin_pclmul.h Index: lib/Headers/__wmmintrin_pclmul.h === --- lib/Headers/__wmmintrin_pclmul.h +++ lib/Headers/__wmmintrin_pclmul.h @@ -23,6 +23,36 @@ #ifndef _WMMINTRIN_PCLMUL_H #define _WMMINTRIN_PCLMUL_H +/// \brief Multiplies two 64-bit integer values, which are selected from source +///operands using the immediate-value operand. The multiplication is a +///carry-less multiplication, and the 128-bit integer product is stored in +///the destination. +/// +/// \headerfile +/// +/// \code +/// __m128i _mm_clmulepi64_si128(__m128i __X, __m128i __Y, const int __I); +/// \endcode +/// +/// This intrinsic corresponds to \c VPCLMULQDQ instruction. +/// +/// \param __X +///A 128-bit vector of [2 x i64] containing one of the source +///operands. +/// \param __Y +///A 128-bit vector of [2 x i64] containing one of the source +///operands. +/// \param __I +///An immediate value specifying which 64-bit values to select +///from the operands. +///Bit 0 is used to select a value from operand __X, +///and bit 4 is used to select a value from operand __Y: +///Bit[0]=0 indicates that bits[63:0] of operand __X are used. +///Bit[0]=1 indicates that bits[127:64] of operand __X are used. +///Bit[4]=0 indicates that bits[63:0] of operand __Y are used. +///Bit[4]=1 indicates that bits[127:64] of operand __Y are used. +/// \returns The 128-bit integer vector containing the result of the carry-less +///multiplication of the selected 64-bit values. #define _mm_clmulepi64_si128(__X, __Y, __I) \ ((__m128i)__builtin_ia32_pclmulqdq128((__v2di)(__m128i)(__X), \ (__v2di)(__m128i)(__Y), (char)(__I))) Index: lib/Headers/__wmmintrin_pclmul.h === --- lib/Headers/__wmmintrin_pclmul.h +++ lib/Headers/__wmmintrin_pclmul.h @@ -23,6 +23,36 @@ #ifndef _WMMINTRIN_PCLMUL_H #define _WMMINTRIN_PCLMUL_H +/// \brief Multiplies two 64-bit integer values, which are selected from source +///operands using the immediate-value operand. The multiplication is a +///carry-less multiplication, and the 128-bit integer product is stored in +///the destination. +/// +/// \headerfile +/// +/// \code +/// __m128i _mm_clmulepi64_si128(__m128i __X, __m128i __Y, const int __I); +/// \endcode +/// +/// This intrinsic corresponds to \c VPCLMULQDQ instruction. +/// +/// \param __X +///A 128-bit vector of [2 x i64] containing one of the source +///operands. +/// \param __Y +///A 128-bit vector of [2 x i64] containing one of the source +///operands. +/// \param __I +///An immediate value specifying which 64-bit values to select +///from the operands. +///Bit 0 is used to select a value from operand __X, +///and bit 4 is used to select a value from operand __Y: +///Bit[0]=0 indicates that bits[63:0] of operand __X are used. +///Bit[0]=1 indicates that bits[127:64] of operand __X are used. +///Bit[4]=0 indicates that bits[63:0] of operand __Y are used. +///Bit[4]=1 indicates that bits[127:64] of operand __Y are used. +/// \returns The 128-bit integer vector containing the result of the carry-less +///multiplication of the selected 64-bit values. #define _mm_clmulepi64_si128(__X, __Y, __I) \ ((__m128i)__builtin_ia32_pclmulqdq128((__v2di)(__m128i)(__X), \ (__v2di)(__m128i)(__Y), (char)(__I))) ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D16562: Adding doxygen comments to the LLVM intrinsics (part 3, __wmmintrin_aes.h)
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 intrinsincs in the header
file __wmmintrin_aes.h.
The doxygen comments are automatically generated based on SCE internal
intrinsics document using DCG tool that I wrote.
I will submit more doxygen comments for the other intrinsic header files as
soon as this patch is approved.
Here is the link to the general discussion about adding comments to x86
intrinsics headers.
http://permalink.gmane.org/gmane.comp.compilers.clang.devel/42032
Here are the links to the similar code reviews for the doxygen comments in
ammintrin.h and _wmmintrin_pclmul.h
http://reviews.llvm.org/D8762 (closed)
http://reviews.llvm.org/D15999 (still needs review)
Katya.
Repository:
rL LLVM
http://reviews.llvm.org/D16562
Files:
__wmmintrin_aes.h
Index: __wmmintrin_aes.h
===
--- __wmmintrin_aes.h
+++ __wmmintrin_aes.h
@@ -28,36 +28,121 @@
/* Define the default attributes for the functions in this file. */
#define __DEFAULT_FN_ATTRS __attribute__((__always_inline__, __nodebug__, __target__("aes")))
+/// \brief Performs a single round of AES encryption using the Equivalent
+///Inverse Cipher, transforming the state value from the first source
+///operand using a 128-bit round key value contained in the second source
+///operand, and writes the result to the destination.
+///
+/// \headerfile
+///
+/// This intrinsic corresponds to \c VAESENC instruction.
+///
+/// \param __V
+///A 128-bit integer vector containing the state value.
+/// \param __R
+///A 128-bit integer vector containing the round key value.
+/// \returns A 128-bit integer vector containing the encrypted value.
static __inline__ __m128i __DEFAULT_FN_ATTRS
_mm_aesenc_si128(__m128i __V, __m128i __R)
{
return (__m128i)__builtin_ia32_aesenc128(__V, __R);
}
+/// \brief Performs the final round of AES encryption using the Equivalent
+///Inverse Cipher, transforming the state value from the first source
+///operand using a 128-bit round key value contained in the second source
+///operand, and writes the result to the destination.
+///
+/// \headerfile
+///
+/// This intrinsic corresponds to \c VAESENCLAST instruction.
+///
+/// \param __V
+///A 128-bit integer vector containing the state value.
+/// \param __R
+///A 128-bit integer vector containing the round key value.
+/// \returns A 128-bit integer vector containing the encrypted value.
static __inline__ __m128i __DEFAULT_FN_ATTRS
_mm_aesenclast_si128(__m128i __V, __m128i __R)
{
return (__m128i)__builtin_ia32_aesenclast128(__V, __R);
}
+/// \brief Performs a single round of AES decryption using the Equivalent
+///Inverse Cipher, transforming the state value from the first source
+///operand using a 128-bit round key value contained in the second source
+///operand, and writes the result to the destination.
+///
+/// \headerfile
+///
+/// This intrinsic corresponds to \c VAESDEC instruction.
+///
+/// \param __V
+///A 128-bit integer vector containing the state value.
+/// \param __R
+///A 128-bit integer vector containing the round key value.
+/// \returns A 128-bit integer vector containing the decrypted value.
static __inline__ __m128i __DEFAULT_FN_ATTRS
_mm_aesdec_si128(__m128i __V, __m128i __R)
{
return (__m128i)__builtin_ia32_aesdec128(__V, __R);
}
+/// \brief Performs the final round of AES decryption using the Equivalent
+///Inverse Cipher, transforming the state value from the first source
+///operand using a 128-bit round key value contained in the second source
+///operand, and writes the result to the destination.
+///
+/// \headerfile
+///
+/// This intrinsic corresponds to \c VAESDECLAST instruction.
+///
+/// \param __V
+///A 128-bit integer vector containing the state value.
+/// \param __R
+///A 128-bit integer vector containing the round key value.
+/// \returns A 128-bit integer vector containing the decrypted value.
static __inline__ __m128i __DEFAULT_FN_ATTRS
_mm_aesdeclast_si128(__m128i __V, __m128i __R)
{
return (__m128i)__builtin_ia32_aesdeclast128(__V, __R);
}
+/// \brief Applies the AES InvMixColumns() transformation to an expanded key
+///contained in the source operand, and writes the result to the
+///destination.
+///
+/// \headerfile
+///
+/// This intrinsic corresponds to \c VAESIMC instruction.
+///
+/// \param __V
+///A 128-bit integer vector containing the expanded key.
+/// \returns A 128-bit integer vector containing the transformed value.
static __inline__ __m128i __DEFAULT_FN_ATTRS
_mm_aesimc_si128(__m128i __V)
{
return (__m128i)__builtin_ia32_aesimc128(__V);
}
+/// \brief Generates a ro
Re: [PATCH] D15999: Adding doxygen comments to the LLVM intrinsics (part 2, _wmmintrin_pclmul.h)
kromanova added a reviewer: craig.topper. kromanova added a comment. Hello, I have a lot of intrinsic header files with doxygen documentation that I will be submitting in the near future. We have been reviewing this internally at SCE to make sure the patch it in the best shape before submitting and make the reviews easier. Every patch has been already reviewed by at least three people (me, one of SCE's compiler developer and SCE's technical writer). So, hopefully the quality should be pretty good. Still, there are a lot of intrinsics (~800) to review and I don’t want to put the burden on only 1-2 open source reviewers. Who else (except people on the 'reviewers' and 'subscribers' list) will be a good reviewer for X86 intrinsics? Are there any intrinsics experts from MS, Intel, AMD, Apple on the mailing list who might be willing to help out?” Thanks! Katya. http://reviews.llvm.org/D15999 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D15999: Adding doxygen comments to the LLVM intrinsics (part 2, _wmmintrin_pclmul.h)
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 corresponds to > the instruction" (side note, use the "the"? I commented on a case > inline). This isn't always the case with all of our intrinsics when the > compiler lowers them to a shuffle intrinsic or some such, or it's optimized, > etc. Personally I'd leave that line out, though I understand it exists in a > lot of similar documentation. Hi Eric, I agree. Sometimes the instruction that corresponds to a specific intrinsic is optimized out, sometimes it will get lowered to something else, etc. However, I think keeping the instruction name in the documentation is extremely useful. In general, intrinsic documentation (especially in the form of comments) is not very complete. When I need to know what a specific intrinsic is doing (and I very often have to look up intrinsics!), I find the corresponding instruction name and go dig in AMD's or Intel's Architecture Programmer's manuals, where I could find all the details I need. Programmer's manuals instruction descriptions are much more detailed and complete. However, it's too much information to add to the comments. :) As you know, Intel's and MS's intrinsics guides are also specifying corresponding instruction names for the intrinsics. I suspect they had the same idea that I just described. I briefly chatted with Paul Robinson and he suggested to say "This intrinsic is equivalent to the instruction" instead, because this sentence doesn't give a false impression that one will definitely see this particular instruction in the generated code. Intel's intrinsics documentation says something like that, e.g: "The corresponding Intel® AVX instruction is VBLENDPD" What do you think/prefer? And, yes, I will add "the" before "corresponds to". Thanks! Easy enough with the generator. :) http://reviews.llvm.org/D15999 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D15999: Adding doxygen comments to the LLVM intrinsics (part 2, _wmmintrin_pclmul.h)
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 corresponds to > the instruction" (side note, use the "the"? I commented on a case > inline). This isn't always the > case with all of our intrinsics when the compiler lowers them to a shuffle > intrinsic or some such, or it's optimized, etc. Personally I'd leave that > line out, though I understand it exists > in a lot of similar documentation. BTW, in some cases, our documentation won't be as specific and will say "This intrinsic (e.g. _mm_store_ps1 ) corresponds to the Shuffling + MOVSS instruction" or "No AVX instruction corresponds to this intrinsic (e.g. _mm256_set_pd)" or "Composite SSE2 instruction corresponds to this intrinsic (e.g. _mm_set_sd). Microsoft and Intel's documentation are very similar with this respect. See the description of _mm_set_sd intrinsic that I just mentioned. https://software.intel.com/en-us/node/524261 https://msdn.microsoft.com/en-us/library/dksztbt9%28v=vs.90%29.aspx http://reviews.llvm.org/D15999 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[PATCH] D16697: Updating .debug_line section version information to match DWARF version.
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. DW_LNS_set_prologue_end, DW_LNS_set_epilogue_begin or DW_LNS_set_isa)
and from DWARF 4 could be emitted by the compiler. That's not exactly right.
This patch changes version information of .debug_line to exactly match the
DWARF version (which is 4 by default).
For .debug_line version 4, a new field maximum_operations_per_instruction is
emitted. I'm not exactly sure how to set this field correctly for VLIW
architectures. Hopefully, there are some experts in the community that could
help out (any Hexagon debuginfo developers out there?).
I’ve tried a few tools to check if they could handle .debug_line version 4.
On a simple testcase, I verified that GDB debugger 7.7 understands .debug_line
version 4.
Sony’s proprietary debugger can understand it too.
I don't have LLDB built, so I haven’t tried it.
GCC (older version 4.8.2) still emits version 2 for the .debug_line section,
even though the DWARF version is set to 4. I'm not sure about the latest GCC.
GNU's readelf (version 2.24) can handle .debug_line version 4.
I added 3 new tests debug-line-version2.ll, debug-line-version3.ll and
debug-line-version4.ll. The IR is generated from one source using different
options (-gdwarf-2, -gdwarf-3 and –gdwarf-4). Any idea how to merge these three
tests into one?
Let me know what do you think about the patch.
Repository:
rL LLVM
http://reviews.llvm.org/D16697
Files:
lib/MC/MCDwarf.cpp
test/DebugInfo/AArch64/line-header.ll
test/DebugInfo/Generic/debug-line-version2.ll
test/DebugInfo/Generic/debug-line-version3.ll
test/DebugInfo/Generic/debug-line-version4.ll
test/DebugInfo/Generic/empty.ll
test/DebugInfo/X86/empty.ll
test/DebugInfo/X86/stmt-list-multiple-compile-units.ll
test/MC/ELF/debug-line.s
test/MC/ELF/debug-line2.s
test/MC/ELF/debug-loc.s
test/MC/ELF/discriminator.s
test/MC/ELF/empty-dwarf-lines.s
test/MC/MachO/file.s
test/MC/MachO/gen-dwarf.s
test/MC/MachO/loc.s
Index: test/MC/MachO/loc.s
===
--- test/MC/MachO/loc.s
+++ test/MC/MachO/loc.s
@@ -9,27 +9,27 @@
// CHECK: Name: __debug_line (5F 5F 64 65 62 75 67 5F 6C 69 6E 65 00 00 00 00)
// CHECK: Segment: __DWARF (5F 5F 44 57 41 52 46 00 00 00 00 00 00 00 00 00)
// CHECK: Address: 0x1
-// CHECK: Size: 0x33
+// CHECK: Size: 0x34
// CHECK: Offset: 237
// CHECK: Alignment: 0
-// CHECK: RelocationOffset: 0x120
+// CHECK: RelocationOffset: 0x124
// CHECK: RelocationCount: 1
// CHECK: Type: 0x0
// CHECK: Attributes [ (0x2)
// CHECK: Debug (0x2)
// CHECK: ]
// CHECK: Reserved1: 0x0
// CHECK: Reserved2: 0x0
// CHECK: SectionData (
-// CHECK: : 2F00 02001A00 0101 FB0E0D00 |/...|
-// CHECK: 0010: 01010101 0001 0100 666F6F00 |foo.|
-// CHECK: 0020: 00050200 0003 3F010201 |?...|
-// CHECK: 0030: 000101 |...|
+// CHECK: : 3000 04001B00 0101 01FB0E0D |0...|
+// CHECK: 0010: 00010101 0100 0101 00666F6F |.foo|
+// CHECK: 0020: 0502 033F0102 |.?..|
+// CHECK: 0030: 01000101
// CHECK: )
// CHECK: }
// CHECK: ]
// CHECK: Relocations [
// CHECK: Section __debug_line {
-// CHECK: 0x27 0 2 0 GENERIC_RELOC_VANILLA 0 __text
+// CHECK: 0x28 0 2 0 GENERIC_RELOC_VANILLA 0 __text
// CHECK: }
// CHECK: ]
Index: test/MC/MachO/gen-dwarf.s
===
--- test/MC/MachO/gen-dwarf.s
+++ test/MC/MachO/gen-dwarf.s
@@ -89,7 +89,7 @@
// CHECK: .debug_line contents:
// CHECK: Line table prologue:
// We don't check the total_length as it includes lengths of temp paths
-// CHECK: version: 2
+// CHECK: version: 4
// We don't check the prologue_length as it too includes lengths of temp paths
// CHECK: min_inst_length: 1
// CHECK: default_is_stmt: 1
Index: test/MC/MachO/file.s
===
--- test/MC/MachO/file.s
+++ test/MC/MachO/file.s
@@ -8,7 +8,7 @@
// CHECK-NEXT:Name: __debug_line
// CHECK-NEXT:Segment: __DWARF
// CHECK-NEXT:Address: 0x1
-// CHECK-NEXT:Size: 0x28
+// CHECK-NEXT:Size: 0x29
// CHECK-NEXT:Offset: 237
// CHECK-NEXT:Alignment: 0
// CHECK-NEXT:RelocationOffset: 0x0
@@ -20,8 +20,8 @@
// CHECK-NEXT:Reserved1: 0x0
// CHECK-NEXT:Reserved2: 0x0
// CHECK-NEXT:SectionData (
-// CHECK-NEXT: : 2400 02001E00 0101 FB0E0D00
-// CHECK-NEXT: 0010: 01010101
Re: [PATCH] D15999: Adding doxygen comments to the LLVM intrinsics (part 2, _wmmintrin_pclmul.h)
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&id=46411#toc Repository: rL LLVM http://reviews.llvm.org/D15999 Files: cfe/trunk/lib/Headers/__wmmintrin_pclmul.h Index: cfe/trunk/lib/Headers/__wmmintrin_pclmul.h === --- cfe/trunk/lib/Headers/__wmmintrin_pclmul.h +++ cfe/trunk/lib/Headers/__wmmintrin_pclmul.h @@ -23,6 +23,34 @@ #ifndef _WMMINTRIN_PCLMUL_H #define _WMMINTRIN_PCLMUL_H +/// \brief Multiplies two 64-bit integer values, which are selected from source +///operands using the immediate-value operand. The multiplication is a +///carry-less multiplication, and the 128-bit integer product is stored in +///the destination. +/// +/// \headerfile +/// +/// \code +/// __m128i _mm_clmulepi64_si128(__m128i __X, __m128i __Y, const int __I); +/// \endcode +/// +/// This intrinsic corresponds to the \c VPCLMULQDQ instruction. +/// +/// \param __X +///A 128-bit vector of [2 x i64] containing one of the source operands. +/// \param __Y +///A 128-bit vector of [2 x i64] containing one of the source operands. +/// \param __I +///An immediate value specifying which 64-bit values to select from the +///operands. +///Bit 0 is used to select a value from operand __X, +///and bit 4 is used to select a value from operand __Y: +///Bit[0]=0 indicates that bits[63:0] of operand __X are used. +///Bit[0]=1 indicates that bits[127:64] of operand __X are used. +///Bit[4]=0 indicates that bits[63:0] of operand __Y are used. +///Bit[4]=1 indicates that bits[127:64] of operand __Y are used. +/// \returns The 128-bit integer vector containing the result of the carry-less +///multiplication of the selected 64-bit values. #define _mm_clmulepi64_si128(__X, __Y, __I) \ ((__m128i)__builtin_ia32_pclmulqdq128((__v2di)(__m128i)(__X), \ (__v2di)(__m128i)(__Y), (char)(__I))) Index: cfe/trunk/lib/Headers/__wmmintrin_pclmul.h === --- cfe/trunk/lib/Headers/__wmmintrin_pclmul.h +++ cfe/trunk/lib/Headers/__wmmintrin_pclmul.h @@ -23,6 +23,34 @@ #ifndef _WMMINTRIN_PCLMUL_H #define _WMMINTRIN_PCLMUL_H +/// \brief Multiplies two 64-bit integer values, which are selected from source +///operands using the immediate-value operand. The multiplication is a +///carry-less multiplication, and the 128-bit integer product is stored in +///the destination. +/// +/// \headerfile +/// +/// \code +/// __m128i _mm_clmulepi64_si128(__m128i __X, __m128i __Y, const int __I); +/// \endcode +/// +/// This intrinsic corresponds to the \c VPCLMULQDQ instruction. +/// +/// \param __X +///A 128-bit vector of [2 x i64] containing one of the source operands. +/// \param __Y +///A 128-bit vector of [2 x i64] containing one of the source operands. +/// \param __I +///An immediate value specifying which 64-bit values to select from the +///operands. +///Bit 0 is used to select a value from operand __X, +///and bit 4 is used to select a value from operand __Y: +///Bit[0]=0 indicates that bits[63:0] of operand __X are used. +///Bit[0]=1 indicates that bits[127:64] of operand __X are used. +///Bit[4]=0 indicates that bits[63:0] of operand __Y are used. +///Bit[4]=1 indicates that bits[127:64] of operand __Y are used. +/// \returns The 128-bit integer vector containing the result of the carry-less +///multiplication of the selected 64-bit values. #define _mm_clmulepi64_si128(__X, __Y, __I) \ ((__m128i)__builtin_ia32_pclmulqdq128((__v2di)(__m128i)(__X), \ (__v2di)(__m128i)(__Y), (char)(__I))) ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D16562: Adding doxygen comments to the LLVM intrinsics (part 3, __wmmintrin_aes.h)
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 http://reviews.llvm.org/D16562 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D16562: Adding doxygen comments to the LLVM intrinsics (part 3, __wmmintrin_aes.h)
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&id=46447#toc
Repository:
rL LLVM
http://reviews.llvm.org/D16562
Files:
cfe/trunk/lib/Headers/__wmmintrin_aes.h
Index: cfe/trunk/lib/Headers/__wmmintrin_aes.h
===
--- cfe/trunk/lib/Headers/__wmmintrin_aes.h
+++ cfe/trunk/lib/Headers/__wmmintrin_aes.h
@@ -28,36 +28,121 @@
/* Define the default attributes for the functions in this file. */
#define __DEFAULT_FN_ATTRS __attribute__((__always_inline__, __nodebug__, __target__("aes")))
+/// \brief Performs a single round of AES encryption using the Equivalent
+///Inverse Cipher, transforming the state value from the first source
+///operand using a 128-bit round key value contained in the second source
+///operand, and writes the result to the destination.
+///
+/// \headerfile
+///
+/// This intrinsic corresponds to the \c VAESENC instruction.
+///
+/// \param __V
+///A 128-bit integer vector containing the state value.
+/// \param __R
+///A 128-bit integer vector containing the round key value.
+/// \returns A 128-bit integer vector containing the encrypted value.
static __inline__ __m128i __DEFAULT_FN_ATTRS
_mm_aesenc_si128(__m128i __V, __m128i __R)
{
return (__m128i)__builtin_ia32_aesenc128(__V, __R);
}
+/// \brief Performs the final round of AES encryption using the Equivalent
+///Inverse Cipher, transforming the state value from the first source
+///operand using a 128-bit round key value contained in the second source
+///operand, and writes the result to the destination.
+///
+/// \headerfile
+///
+/// This intrinsic corresponds to the \c VAESENCLAST instruction.
+///
+/// \param __V
+///A 128-bit integer vector containing the state value.
+/// \param __R
+///A 128-bit integer vector containing the round key value.
+/// \returns A 128-bit integer vector containing the encrypted value.
static __inline__ __m128i __DEFAULT_FN_ATTRS
_mm_aesenclast_si128(__m128i __V, __m128i __R)
{
return (__m128i)__builtin_ia32_aesenclast128(__V, __R);
}
+/// \brief Performs a single round of AES decryption using the Equivalent
+///Inverse Cipher, transforming the state value from the first source
+///operand using a 128-bit round key value contained in the second source
+///operand, and writes the result to the destination.
+///
+/// \headerfile
+///
+/// This intrinsic corresponds to the \c VAESDEC instruction.
+///
+/// \param __V
+///A 128-bit integer vector containing the state value.
+/// \param __R
+///A 128-bit integer vector containing the round key value.
+/// \returns A 128-bit integer vector containing the decrypted value.
static __inline__ __m128i __DEFAULT_FN_ATTRS
_mm_aesdec_si128(__m128i __V, __m128i __R)
{
return (__m128i)__builtin_ia32_aesdec128(__V, __R);
}
+/// \brief Performs the final round of AES decryption using the Equivalent
+///Inverse Cipher, transforming the state value from the first source
+///operand using a 128-bit round key value contained in the second source
+///operand, and writes the result to the destination.
+///
+/// \headerfile
+///
+/// This intrinsic corresponds to the \c VAESDECLAST instruction.
+///
+/// \param __V
+///A 128-bit integer vector containing the state value.
+/// \param __R
+///A 128-bit integer vector containing the round key value.
+/// \returns A 128-bit integer vector containing the decrypted value.
static __inline__ __m128i __DEFAULT_FN_ATTRS
_mm_aesdeclast_si128(__m128i __V, __m128i __R)
{
return (__m128i)__builtin_ia32_aesdeclast128(__V, __R);
}
+/// \brief Applies the AES InvMixColumns() transformation to an expanded key
+///contained in the source operand, and writes the result to the
+///destination.
+///
+/// \headerfile
+///
+/// This intrinsic corresponds to the \c VAESIMC instruction.
+///
+/// \param __V
+///A 128-bit integer vector containing the expanded key.
+/// \returns A 128-bit integer vector containing the transformed value.
static __inline__ __m128i __DEFAULT_FN_ATTRS
_mm_aesimc_si128(__m128i __V)
{
return (__m128i)__builtin_ia32_aesimc128(__V);
}
+/// \brief Generates a round key for AES encyption, operating on 128-bit data
+///specified in the first source operand and using an 8-bit round constant
+///specified by the second source operand, and writes the result to the
+///destination.
+///
+/// \headerfile
+///
+/// \code
+/// __m128i _mm_aeskeygenassist_si128(__m128i C, const int R);
+/// \endcode
+///
+/// This intrinsic corresponds to the \c AESKEYGENASSIST instruction.
+///
+/// \param C
+///A 128-bit integer vector that is used to generate the AES encryption key.
+/// \param R
+///An 8-b
Re: [PATCH] D12624: Top-level anonymous namespaces are missing import DW_TAG_imported_module and nested anonymous namespaces are not
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 else I need to change?
Repository:
rL LLVM
http://reviews.llvm.org/D12624
Files:
include/clang/Frontend/CodeGenOptions.def
include/clang/Parse/Parser.h
include/clang/Sema/Sema.h
lib/CodeGen/CGDebugInfo.cpp
lib/Frontend/CompilerInvocation.cpp
lib/Parse/ParseDecl.cpp
lib/Parse/ParseDeclCXX.cpp
lib/Sema/SemaDeclCXX.cpp
test/CodeGenCXX/debug-info-anon-namespace.cpp
Index: test/CodeGenCXX/debug-info-anon-namespace.cpp
===
--- test/CodeGenCXX/debug-info-anon-namespace.cpp
+++ test/CodeGenCXX/debug-info-anon-namespace.cpp
@@ -0,0 +1,26 @@
+// RUN: %clang_cc1 -emit-llvm -debug-info-kind=limited -triple x86_64-scei-ps4 -O0 %s -o - | FileCheck --check-prefix=PS4 %s
+// RUN: %clang_cc1 -emit-llvm -debug-info-kind=limited -triple x86_64-unknown-linux-gnu -O0 %s -o - | FileCheck --check-prefix=NON-PS4 %s
+
+namespace
+{
+ int a = 5;
+}
+int *b = &a;
+
+namespace
+{
+ namespace {
+int a1 = 5;
+ }
+ int a2 = 7;
+}
+int *b1 = &a1;
+int *b2 = &a2;
+
+
+// PS4: [[NS:![0-9]+]] = !DINamespace
+// PS4: [[NS2:![0-9]+]] = !DINamespace
+// PS4: !DIImportedEntity(tag: DW_TAG_imported_module, scope: !0, entity: [[NS]])
+// PS4: !DIImportedEntity(tag: DW_TAG_imported_module, scope: [[NS]], entity: [[NS2]], line: {{[0-9]+}})
+// NON-PS4-NOT: !DIImportedEntity
+
Index: lib/Sema/SemaDeclCXX.cpp
===
--- lib/Sema/SemaDeclCXX.cpp
+++ lib/Sema/SemaDeclCXX.cpp
@@ -7185,7 +7185,8 @@
SourceLocation IdentLoc,
IdentifierInfo *II,
SourceLocation LBrace,
- AttributeList *AttrList) {
+ AttributeList *AttrList,
+ UsingDirectiveDecl *&UD) {
SourceLocation StartLoc = InlineLoc.isValid() ? InlineLoc : NamespaceLoc;
// For anonymous namespace, take the location of the left brace.
SourceLocation Loc = II ? IdentLoc : LBrace;
@@ -7299,14 +7300,13 @@
// namespace internal linkage.
if (!PrevNS) {
- UsingDirectiveDecl* UD
-= UsingDirectiveDecl::Create(Context, Parent,
- /* 'using' */ LBrace,
- /* 'namespace' */ SourceLocation(),
- /* qualifier */ NestedNameSpecifierLoc(),
- /* identifier */ SourceLocation(),
- Namespc,
- /* Ancestor */ Parent);
+ UD = UsingDirectiveDecl::Create(Context, Parent,
+ /* 'using' */ LBrace,
+ /* 'namespace' */ SourceLocation(),
+ /* qualifier */ NestedNameSpecifierLoc(),
+ /* identifier */ SourceLocation(),
+ Namespc,
+ /* Ancestor */ Parent);
UD->setImplicit();
Parent->addDecl(UD);
}
Index: lib/Parse/ParseDeclCXX.cpp
===
--- lib/Parse/ParseDeclCXX.cpp
+++ lib/Parse/ParseDeclCXX.cpp
@@ -55,17 +55,17 @@
/// namespace-alias-definition: [C++ 7.3.2: namespace.alias]
/// 'namespace' identifier '=' qualified-namespace-specifier ';'
///
-Decl *Parser::ParseNamespace(unsigned Context,
- SourceLocation &DeclEnd,
- SourceLocation InlineLoc) {
+Parser::DeclGroupPtrTy Parser::ParseNamespace(unsigned Context,
+ SourceLocation &DeclEnd,
+ SourceLocation InlineLoc) {
assert(Tok.is(tok::kw_namespace) && "Not a namespace!");
SourceLocation NamespaceLoc = ConsumeToken(); // eat the 'namespace'.
ObjCDeclContextSwitch ObjCDC(*this);
if (Tok.is(tok::code_completion)) {
Actions.CodeCompleteNamespaceDecl(getCurScope());
cutOffParsing();
-return nullptr;
+return DeclGroupPtrTy();;
}
SourceLocation IdentLoc;
@@ -109,31 +109,32 @@
Diag(Tok, diag::err_expected) << tok::identifier;
// Skip to end of the definition and eat the ';'.
SkipUntil(tok::semi);
- return nullptr;
+ return DeclGroupPtrTy();
}
if (attrLoc.isValid())
Diag(attrLoc, diag::err_unexpected_namespace_attributes_alias);
if (InlineLoc.isValid())
Diag(InlineLoc, diag::err_inline_namespace_alias)
<< FixItHint::Creat
Re: [PATCH] D12624: Top-level anonymous namespaces are missing import DW_TAG_imported_module and nested anonymous namespaces are not
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. Katya. Repository: rL LLVM http://reviews.llvm.org/D12624 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D8762: Adding doxygen comments to the LLVM intrinsics (part 1, ammintrin.h)
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 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
Re: [PATCH] D12624: Top-level anonymous namespaces are missing import DW_TAG_imported_module and nested anonymous namespaces are not
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&id=42449#toc
Repository:
rL LLVM
http://reviews.llvm.org/D12624
Files:
cfe/trunk/include/clang/Frontend/CodeGenOptions.def
cfe/trunk/include/clang/Parse/Parser.h
cfe/trunk/include/clang/Sema/Sema.h
cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
cfe/trunk/lib/Frontend/CompilerInvocation.cpp
cfe/trunk/lib/Parse/ParseDecl.cpp
cfe/trunk/lib/Parse/ParseDeclCXX.cpp
cfe/trunk/lib/Sema/SemaDeclCXX.cpp
cfe/trunk/test/CodeGenCXX/debug-info-anon-namespace.cpp
Index: cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
===
--- cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
+++ cfe/trunk/lib/CodeGen/CGDebugInfo.cpp
@@ -3408,10 +3408,14 @@
void CGDebugInfo::EmitUsingDirective(const UsingDirectiveDecl &UD) {
if (CGM.getCodeGenOpts().getDebugInfo() < CodeGenOptions::LimitedDebugInfo)
return;
- DBuilder.createImportedModule(
- getCurrentContextDescriptor(cast(UD.getDeclContext())),
- getOrCreateNameSpace(UD.getNominatedNamespace()),
- getLineNumber(UD.getLocation()));
+ const NamespaceDecl *NSDecl = UD.getNominatedNamespace();
+ if (!NSDecl->isAnonymousNamespace() ||
+ CGM.getCodeGenOpts().DebugExplicitImport) {
+DBuilder.createImportedModule(
+getCurrentContextDescriptor(cast(UD.getDeclContext())),
+getOrCreateNameSpace(NSDecl),
+getLineNumber(UD.getLocation()));
+ }
}
void CGDebugInfo::EmitUsingDecl(const UsingDecl &UD) {
Index: cfe/trunk/lib/Sema/SemaDeclCXX.cpp
===
--- cfe/trunk/lib/Sema/SemaDeclCXX.cpp
+++ cfe/trunk/lib/Sema/SemaDeclCXX.cpp
@@ -7185,7 +7185,8 @@
SourceLocation IdentLoc,
IdentifierInfo *II,
SourceLocation LBrace,
- AttributeList *AttrList) {
+ AttributeList *AttrList,
+ UsingDirectiveDecl *&UD) {
SourceLocation StartLoc = InlineLoc.isValid() ? InlineLoc : NamespaceLoc;
// For anonymous namespace, take the location of the left brace.
SourceLocation Loc = II ? IdentLoc : LBrace;
@@ -7299,14 +7300,13 @@
// namespace internal linkage.
if (!PrevNS) {
- UsingDirectiveDecl* UD
-= UsingDirectiveDecl::Create(Context, Parent,
- /* 'using' */ LBrace,
- /* 'namespace' */ SourceLocation(),
- /* qualifier */ NestedNameSpecifierLoc(),
- /* identifier */ SourceLocation(),
- Namespc,
- /* Ancestor */ Parent);
+ UD = UsingDirectiveDecl::Create(Context, Parent,
+ /* 'using' */ LBrace,
+ /* 'namespace' */ SourceLocation(),
+ /* qualifier */ NestedNameSpecifierLoc(),
+ /* identifier */ SourceLocation(),
+ Namespc,
+ /* Ancestor */ Parent);
UD->setImplicit();
Parent->addDecl(UD);
}
Index: cfe/trunk/lib/Frontend/CompilerInvocation.cpp
===
--- cfe/trunk/lib/Frontend/CompilerInvocation.cpp
+++ cfe/trunk/lib/Frontend/CompilerInvocation.cpp
@@ -365,6 +365,7 @@
const TargetOptions &TargetOpts) {
using namespace options;
bool Success = true;
+ llvm::Triple Triple = llvm::Triple(TargetOpts.Triple);
unsigned OptimizationLevel = getOptimizationLevel(Args, IK, Diags);
// TODO: This could be done in Driver
@@ -409,6 +410,8 @@
Opts.EmitCodeView = Args.hasArg(OPT_gcodeview);
Opts.SplitDwarfFile = Args.getLastArgValue(OPT_split_dwarf_file);
Opts.DebugTypeExtRefs = Args.hasArg(OPT_dwarf_ext_refs);
+ if (Triple.isPS4CPU())
+Opts.DebugExplicitImport = true;
for (const auto &Arg : Args.getAllArgValues(OPT_fdebug_prefix_map_EQ))
Opts.DebugPrefixMap.insert(StringRef(Arg).split('='));
Index: cfe/trunk/lib/Parse/ParseDecl.cpp
===
--- cfe/trunk/lib/Parse/ParseDecl.cpp
+++ cfe/trunk/lib/Parse/ParseDecl.cpp
@@ -1465,15 +1465,13 @@
if (getLangOpts().CPlusPlus && NextToken().is(tok::kw_namespace)) {
ProhibitAttributes(attrs);
SourceLocation InlineLoc = ConsumeToken();
- SingleDecl = ParseNamespace(Context, DeclEnd, InlineLoc);
- break;
+ return ParseNamespace(Context, DeclEnd, Inli
[clang] [lld] [llvm] Integrated Distributed ThinLTO (DTLTO): Design Overview (PR #126654)
romanova-ekaterina wrote: > Hi Reviewers! Thanks for the feedback here. I wanted to draw attention to > something that I mentioned briefly in the description - the possibility of a > plugin interface as opposed to invoking an external process that consumes > JSON. > > There are some theoretical advantages with plugins. For example, if > distribution systems exist/arise that allow data to be passed back and forth > between LLVM and a distribution system using memory buffers instead of files, > a plugin could perhaps do that more efficiently. But we haven't done anything > yet to quantify how much better this would be vs implicitly leaning on e.g. > memory mapped files and the OS file cache. The distribution systems we're > motivated to support from customer demand don't have such capabilities at > this time. > > Does anyone have any opinions on this? The “No backend DTLTO” branch is ready. Please have a look and let us know what you think. https://github.com/romanova-ekaterina/llvm-project/pull/new/kromanova/main/integrated-DTLTO-no-backend This comment [https://github.com/llvm/llvm-project/pull/127749#issuecomment-2727266591] has more details about the differences between “Out of process (DTLTO) backend” branch and “No backend” DTLTO branch. https://github.com/llvm/llvm-project/pull/126654 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [lld] [llvm] Integrated Distributed ThinLTO (DTLTO): Design Overview (PR #126654)
romanova-ekaterina wrote: > > Thanks for the heads up, so I should not do a detailed code review for > > PR127749? Is there more info on what you mean by a "no-backend DTLTO"? > > Actually, please review whatever you would like to at this point, Theresa. I > don't want to get in the way of hearing what you think - we're keen to your > input. I just wanted to point out that since another branch is coming, you > may wish to wait until it arrives if you think a side-by-side comparison > would be a good way of doing things. To clarify: that other branch won't be > put up as a pull request, but we can decide how to proceed here if the > general design shown in that other branch is preferred. I also mentioned that > it will appear in a few days, but that's really dependent on the results of > some more internal review. We're working hard on it! The “No backend DTLTO” branch is ready. Please have a look and let us know what you think. https://github.com/romanova-ekaterina/llvm-project/pull/new/kromanova/main/integrated-DTLTO-no-backend This comment [https://github.com/llvm/llvm-project/pull/127749#issuecomment-2727266591] has more details about the differences between “Out of process (DTLTO) backend” branch and “No backend” DTLTO branch. https://github.com/llvm/llvm-project/pull/126654 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [lld] [llvm] Integrated Distributed ThinLTO (DTLTO): Design Overview (PR #126654)
romanova-ekaterina wrote: > > Thanks for the heads up, so I should not do a detailed code review for > > PR127749? Is there more info on what you mean by a "no-backend DTLTO"? > > Actually, please review whatever you would like to at this point, Theresa. I > don't want to get in the way of hearing what you think - we're keen to your > input. I just wanted to point out that since another branch is coming, you > may wish to wait until it arrives if you think a side-by-side comparison > would be a good way of doing things. To clarify: that other branch won't be > put up as a pull request, but we can decide how to proceed here if the > general design shown in that other branch is preferred. I also mentioned that > it will appear in a few days, but that's really dependent on the results of > some more internal review. We're working hard on it! The “No backend DTLTO” branch is ready https://github.com/romanova-ekaterina/llvm-project/pull/new/kromanova/main/integrated-DTLTO-no-backend. Please have a look and let us know what you think. This comment [[DTLTO][LLVM] Integrated Distributed ThinLTO (DTLTO) by bd1976bris · Pull Request #127749 · llvm/llvm-project](https://github.com/llvm/llvm-project/pull/127749#issuecomment-2727266591) has more details about the differences between “Out of process (DTLTO) backend” branch and “No backend” DTLTO branch. https://github.com/llvm/llvm-project/pull/126654 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [lld] [llvm] Integrated Distributed ThinLTO (DTLTO): Initial support (PR #126654)
romanova-ekaterina wrote: > > I don't have a strong opinion on this but I have basically the same > > concerns with completely opposite conclusions. To me, the distributed > > thinLTO makes you think there is a distributed full LTO, while just call it > > distributed LTO will eliminate that confusion. Distributed LTO is by nature > > based on thin LTO infrastructure but that doesn't need to be exposed. > > Accepted. I think it might be worth appealing to authority here. I wonder if > @MaskRay or @teresajohnson have an opinion? > > > Isn't the LTO option to be `Full/Thin/Distributed` cleaner? > > Sorry, I don't entirely understand this bit, could you expand on this a bit. > Are you envisioning an interface like: clang -flto -> FullLTO clang > -flto=thin -> ThinLTO clang -flto=distributed -> DTLTO > > > You can still keep `DTLTO` for `DisTributed LTO` > > :) We prefer to keep DTLTO name. One of the reason is to distinguish our "integrated" distributed appoach from "non-integrated" distributed approach that has been supported for a while. Another reason is to avoid confusion. We have been using this acronim for a while, referring to it in RFCs and in a couple of lightning talks on developer conferences. When we had a round table last LLVM developers conference we brought up this topic about naming one more time and we all agreed to keep DTLTO name. https://github.com/llvm/llvm-project/pull/126654 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [lld] [llvm] Integrated Distributed ThinLTO (DTLTO): Initial support (PR #126654)
romanova-ekaterina wrote: > I'm fine with DTLTO as a shorthand for "integrated distributed ThinLTO". Great! I'm glad that you support this acronim too. :) > BTW thanks for sending the LLVM patch, I will review that tonight or more > likely tomorrow. Teresa, when reviewing, could you please focus on the design/idea rather than doing a full-fledged code review? In a day or two we will submit another PR for "no-backend" DTLTO implementation. We are doing final "touches" to this PR now. No-backend DLTO implementation has some important benefits/advantages. So, I guess, at this time, it will be most important to understand both designs (i.e. current implementation with DTLTO backend that Ben submitted and the alternative "no DTLTO backend" implementation that we submit a couple of from now), rather than focusing on details of implementations/nikpicks of this particular PR. I will try to do my best to explain the differences between both designs at the time of submission. Hopefully, it will help us to choose the best design for using upstream or potentially do a hybrid solution, choosing the best ideas from both designs. https://github.com/llvm/llvm-project/pull/126654 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [lld] [llvm] Integrated Distributed ThinLTO (DTLTO): Initial support (PR #126654)
romanova-ekaterina wrote: > Thanks for the heads up, so I should not do a detailed code review for > PR127749? Is there more info on what you mean by a "no-backend DTLTO"? "Actually, please review whatever you would like to at this point, Theresa. I don't want to get in the way of hearing what you think - we're keen to your input. I just wanted to point out that since another branch is coming, you may wish to wait until it arrives if you think a side-by-side comparison would be a good way of doing things. To clarify: that other branch won't be put up as a pull request, but we can decide how to proceed here if the general design shown in that other branch is preferred. I also mentioned that it will appear in a few days, but that's really dependent on the results of some more internal review. We're working hard on it!" https://github.com/llvm/llvm-project/pull/126654 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [lld] [llvm] Integrated Distributed ThinLTO (DTLTO): Initial support (PR #126654)
romanova-ekaterina wrote: > In a couple of days, we will submit an alternative PR with "no DTLTO backend" > implementation. Though from the first sight it seems logical to use a > separate DTLTO backend, we thought that "no DTLTO backend" implementation > will be simpler, cleaner and more importantly, will leave a solid foundation > for future important performance enhancements that are planning to add in the > future. These performance enhancements will be impossible to implement within > DTLTO backend. Actually, we don't have any data to determine whether or not these ideas for performance enhancements translate into real world gains. So, let's perhaps revisit these in subsequent PRs, assuming we get something landed for starters, here. https://github.com/llvm/llvm-project/pull/126654 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
