[clang] fe59122 - [clang][Driver] Fix tool path priority test failures
Author: David Spickett Date: 2020-07-15T09:37:09+01:00 New Revision: fe5912249efa1ec5e6aa6e565f722dd4d33d1e54 URL: https://github.com/llvm/llvm-project/commit/fe5912249efa1ec5e6aa6e565f722dd4d33d1e54 DIFF: https://github.com/llvm/llvm-project/commit/fe5912249efa1ec5e6aa6e565f722dd4d33d1e54.diff LOG: [clang][Driver] Fix tool path priority test failures Summary: Failure type 1: This test can fail when the path of the build includes the strings we're checking for. E.g "/gcc" is found in ".../gcc_7.3.0/..." To correct this look for '"' on the end of all matches. So that we only match the end of paths printed by clang -###. (which would be ".../gcc_7.3.0/.../gcc" for the example) Also look for other gcc names like gcc-x.y.z in the first check. This confirms that the copy of clang we made is isolated as expected. Failure type 2: If you use a triple like "powerpc64le-linux-gnu" clang actually reports "powerpc64le-unknown-linux-gnu". Then it searches for the former. That combined with Mac OS adding a version number to cmake's triple means we can't trust cmake or clang to give us the one default triple. To fix the test, write to both names. As they don't overlap with our fake triple, we're still showing that the lookup works. Reviewers: MaskRay, stevewan Reviewed By: stevewan Subscribers: miyuki, JDevlieghere, steven.zhang, stevewan, cfe-commits Tags: #clang Differential Revision: https://reviews.llvm.org/D83055 Added: Modified: clang/test/Driver/program-path-priority.c clang/test/lit.cfg.py Removed: diff --git a/clang/test/Driver/program-path-priority.c b/clang/test/Driver/program-path-priority.c index ba893e7e2e2c..9f1109f530c6 100644 --- a/clang/test/Driver/program-path-priority.c +++ b/clang/test/Driver/program-path-priority.c @@ -13,6 +13,11 @@ /// so only name priority is accounted for, unless we fail to find /// anything at all in the prefix. +/// Note: All matches are expected to be at the end of file paths. +/// So we match " on the end to account for build systems that +/// put the name of the compiler in the build path. +/// E.g. /build/gcc_X.Y.Z/0/... + /// Symlink clang to a new dir which will be its /// "program path" for these tests // RUN: rm -rf %t && mkdir -p %t @@ -21,14 +26,18 @@ /// No gccs at all, nothing is found // RUN: env "PATH=" %t/clang -### -target notreal-none-elf %s 2>&1 | \ // RUN: FileCheck --check-prefix=NO_NOTREAL_GCC %s -// NO_NOTREAL_GCC-NOT: notreal-none-elf-gcc -// NO_NOTREAL_GCC-NOT: /gcc +// NO_NOTREAL_GCC-NOT: notreal-none-elf-gcc" +/// Some systems will have "gcc-x.y.z" so for this first check +/// make sure we don't find "gcc" or "gcc-x.y.z". If we do find either +/// then there is no point continuing as this copy of clang is not +/// isolated as we expected. +// NO_NOTREAL_GCC-NOT: {{/gcc[^/]*"}} /// -gcc in program path is found // RUN: touch %t/notreal-none-elf-gcc && chmod +x %t/notreal-none-elf-gcc // RUN: env "PATH=" %t/clang -### -target notreal-none-elf %s 2>&1 | \ // RUN: FileCheck --check-prefix=PROG_PATH_NOTREAL_GCC %s -// PROG_PATH_NOTREAL_GCC: notreal-none-elf-gcc +// PROG_PATH_NOTREAL_GCC: notreal-none-elf-gcc" /// -gcc on the PATH is found // RUN: mkdir -p %t/env @@ -36,74 +45,89 @@ // RUN: touch %t/env/notreal-none-elf-gcc && chmod +x %t/env/notreal-none-elf-gcc // RUN: env "PATH=%t/env/" %t/clang -### -target notreal-none-elf %s 2>&1 | \ // RUN: FileCheck --check-prefix=ENV_PATH_NOTREAL_GCC %s -// ENV_PATH_NOTREAL_GCC: env/notreal-none-elf-gcc +// ENV_PATH_NOTREAL_GCC: env/notreal-none-elf-gcc" /// -gcc in program path is preferred to one on the PATH // RUN: touch %t/notreal-none-elf-gcc && chmod +x %t/notreal-none-elf-gcc // RUN: env "PATH=%t/env/" %t/clang -### -target notreal-none-elf %s 2>&1 | \ // RUN: FileCheck --check-prefix=BOTH_NOTREAL_GCC %s -// BOTH_NOTREAL_GCC: notreal-none-elf-gcc -// BOTH_NOTREAL_GCC-NOT: env/notreal-none-elf-gcc +// BOTH_NOTREAL_GCC: notreal-none-elf-gcc" +// BOTH_NOTREAL_GCC-NOT: env/notreal-none-elf-gcc" /// On program path, -gcc is preferred to plain gcc // RUN: touch %t/gcc && chmod +x %t/gcc // RUN: env "PATH=" %t/clang -### -target notreal-none-elf %s 2>&1 | \ // RUN: FileCheck --check-prefix=NOTREAL_GCC_PREFERRED %s -// NOTREAL_GCC_PREFERRED: notreal-none-elf-gcc -// NOTREAL_GCC_PREFERRED-NOT: /gcc +// NOTREAL_GCC_PREFERRED: notreal-none-elf-gcc" +// NOTREAL_GCC_PREFERRED-NOT: /gcc" /// -gcc on the PATH is preferred to gcc in program path // RUN: rm %t/notreal-none-elf-gcc // RUN: env "PATH=%t/env/" %t/clang -### -target notreal-none-elf %s 2>&1 | \ // RUN: FileCheck --check-prefix=NOTREAL_PATH_OVER_GCC_PROG %s -// NOTREAL_PATH_OVER_GCC_PROG: env/notreal-none-elf-gcc -// NOTREAL_PATH_OVER_GCC_PROG-NOT: /gcc +// NOTREAL_PATH_OVER_GCC_PROG: env/notreal-none-elf-gcc" +// NOTREAL_PATH_OVER_GCC_PROG-NOT: /gcc" /// -gcc on the PATH is preferred
[clang] 028571d - [clang][Driver] Correct tool search path priority
Author: David Spickett Date: 2020-06-22T09:41:13+01:00 New Revision: 028571d60843cb87e2637ef69ee09090d4526c62 URL: https://github.com/llvm/llvm-project/commit/028571d60843cb87e2637ef69ee09090d4526c62 DIFF: https://github.com/llvm/llvm-project/commit/028571d60843cb87e2637ef69ee09090d4526c62.diff LOG: [clang][Driver] Correct tool search path priority Summary: As seen in: https://bugs.llvm.org/show_bug.cgi?id=45693 When clang looks for a tool it has a set of possible names for it, in priority order. Previously it would look for these names in the program path. Then look for all the names in the PATH. This means that aarch64-none-elf-gcc on the PATH would lose to gcc in the program path. (which was /usr/bin in the bug's case) This changes that logic to search each name in both possible locations, then move to the next name. Which is more what you would expect to happen when using a non default triple. (-B prefixes maybe should follow this logic too, but are not changed in this patch) Subscribers: kristof.beyls, cfe-commits Tags: #clang Differential Revision: https://reviews.llvm.org/D79988 Added: clang/test/Driver/program-path-priority.c Modified: clang/lib/Driver/Driver.cpp clang/test/lit.cfg.py Removed: diff --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp index a48761af400f..27477553963c 100644 --- a/clang/lib/Driver/Driver.cpp +++ b/clang/lib/Driver/Driver.cpp @@ -4717,13 +4717,11 @@ void Driver::generatePrefixedToolNames( } static bool ScanDirForExecutable(SmallString<128> &Dir, - ArrayRef Names) { - for (const auto &Name : Names) { -llvm::sys::path::append(Dir, Name); -if (llvm::sys::fs::can_execute(Twine(Dir))) - return true; -llvm::sys::path::remove_filename(Dir); - } + const std::string &Name) { + llvm::sys::path::append(Dir, Name); + if (llvm::sys::fs::can_execute(Twine(Dir))) +return true; + llvm::sys::path::remove_filename(Dir); return false; } @@ -4736,8 +4734,9 @@ std::string Driver::GetProgramPath(StringRef Name, const ToolChain &TC) const { for (const auto &PrefixDir : PrefixDirs) { if (llvm::sys::fs::is_directory(PrefixDir)) { SmallString<128> P(PrefixDir); - if (ScanDirForExecutable(P, TargetSpecificExecutables)) -return std::string(P.str()); + for (const auto &TargetSpecificExecutable : TargetSpecificExecutables) +if (ScanDirForExecutable(P, TargetSpecificExecutable)) + return std::string(P.str()); } else { SmallString<128> P((PrefixDir + Name).str()); if (llvm::sys::fs::can_execute(Twine(P))) @@ -4746,17 +4745,25 @@ std::string Driver::GetProgramPath(StringRef Name, const ToolChain &TC) const { } const ToolChain::path_list &List = TC.getProgramPaths(); - for (const auto &Path : List) { -SmallString<128> P(Path); -if (ScanDirForExecutable(P, TargetSpecificExecutables)) - return std::string(P.str()); - } + for (const auto &TargetSpecificExecutable : TargetSpecificExecutables) { +// For each possible name of the tool look for it in +// program paths first, then the path. +// Higher priority names will be first, meaning that +// a higher priority name in the path will be found +// instead of a lower priority name in the program path. +// E.g. -gcc on the path will be found instead +// of gcc in the program path +for (const auto &Path : List) { + SmallString<128> P(Path); + if (ScanDirForExecutable(P, TargetSpecificExecutable)) +return std::string(P.str()); +} - // If all else failed, search the path. - for (const auto &TargetSpecificExecutable : TargetSpecificExecutables) +// Fall back to the path if (llvm::ErrorOr P = llvm::sys::findProgramByName(TargetSpecificExecutable)) return *P; + } return std::string(Name); } diff --git a/clang/test/Driver/program-path-priority.c b/clang/test/Driver/program-path-priority.c new file mode 100644 index ..48f23917812e --- /dev/null +++ b/clang/test/Driver/program-path-priority.c @@ -0,0 +1,106 @@ +/// Don't create symlinks on Windows +// UNSUPPORTED: system-windows + +/// Check the priority used when searching for tools +/// Names and locations are usually in this order: +/// -tool, tool, -tool +/// program path, PATH +/// (from highest to lowest priority) +/// A higher priority name found in a lower priority +/// location will win over a lower priority name in a +/// higher priority location. +/// Prefix dirs (added with -B) override the location, +/// so only name priority is accounted for, unless we fail to find +/// anything at all in the prefix. + +/// Copy clang to a new dir which will be its +/// "program path" for these tests +// RUN: rm -rf %t && mkdir -p %t +// RUN: ln -s %clang %t/clang + +/// No gccs at all, nothi
[clang] f570d58 - Revert "[clang][Driver] Correct tool search path priority"
Author: David Spickett Date: 2020-06-22T14:18:54+01:00 New Revision: f570d5810485fa6fb2e1009f795a899d79bd429f URL: https://github.com/llvm/llvm-project/commit/f570d5810485fa6fb2e1009f795a899d79bd429f DIFF: https://github.com/llvm/llvm-project/commit/f570d5810485fa6fb2e1009f795a899d79bd429f.diff LOG: Revert "[clang][Driver] Correct tool search path priority" Revert 028571d60843cb87e2637ef69ee09090d4526c62 to investigate MacOS failure. (also the review link was incorrect, should be https://reviews.llvm.org/D79842) Added: Modified: clang/lib/Driver/Driver.cpp clang/test/lit.cfg.py Removed: clang/test/Driver/program-path-priority.c diff --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp index 27477553963c..a48761af400f 100644 --- a/clang/lib/Driver/Driver.cpp +++ b/clang/lib/Driver/Driver.cpp @@ -4717,11 +4717,13 @@ void Driver::generatePrefixedToolNames( } static bool ScanDirForExecutable(SmallString<128> &Dir, - const std::string &Name) { - llvm::sys::path::append(Dir, Name); - if (llvm::sys::fs::can_execute(Twine(Dir))) -return true; - llvm::sys::path::remove_filename(Dir); + ArrayRef Names) { + for (const auto &Name : Names) { +llvm::sys::path::append(Dir, Name); +if (llvm::sys::fs::can_execute(Twine(Dir))) + return true; +llvm::sys::path::remove_filename(Dir); + } return false; } @@ -4734,9 +4736,8 @@ std::string Driver::GetProgramPath(StringRef Name, const ToolChain &TC) const { for (const auto &PrefixDir : PrefixDirs) { if (llvm::sys::fs::is_directory(PrefixDir)) { SmallString<128> P(PrefixDir); - for (const auto &TargetSpecificExecutable : TargetSpecificExecutables) -if (ScanDirForExecutable(P, TargetSpecificExecutable)) - return std::string(P.str()); + if (ScanDirForExecutable(P, TargetSpecificExecutables)) +return std::string(P.str()); } else { SmallString<128> P((PrefixDir + Name).str()); if (llvm::sys::fs::can_execute(Twine(P))) @@ -4745,25 +4746,17 @@ std::string Driver::GetProgramPath(StringRef Name, const ToolChain &TC) const { } const ToolChain::path_list &List = TC.getProgramPaths(); - for (const auto &TargetSpecificExecutable : TargetSpecificExecutables) { -// For each possible name of the tool look for it in -// program paths first, then the path. -// Higher priority names will be first, meaning that -// a higher priority name in the path will be found -// instead of a lower priority name in the program path. -// E.g. -gcc on the path will be found instead -// of gcc in the program path -for (const auto &Path : List) { - SmallString<128> P(Path); - if (ScanDirForExecutable(P, TargetSpecificExecutable)) -return std::string(P.str()); -} + for (const auto &Path : List) { +SmallString<128> P(Path); +if (ScanDirForExecutable(P, TargetSpecificExecutables)) + return std::string(P.str()); + } -// Fall back to the path + // If all else failed, search the path. + for (const auto &TargetSpecificExecutable : TargetSpecificExecutables) if (llvm::ErrorOr P = llvm::sys::findProgramByName(TargetSpecificExecutable)) return *P; - } return std::string(Name); } diff --git a/clang/test/Driver/program-path-priority.c b/clang/test/Driver/program-path-priority.c deleted file mode 100644 index 48f23917812e.. --- a/clang/test/Driver/program-path-priority.c +++ /dev/null @@ -1,106 +0,0 @@ -/// Don't create symlinks on Windows -// UNSUPPORTED: system-windows - -/// Check the priority used when searching for tools -/// Names and locations are usually in this order: -/// -tool, tool, -tool -/// program path, PATH -/// (from highest to lowest priority) -/// A higher priority name found in a lower priority -/// location will win over a lower priority name in a -/// higher priority location. -/// Prefix dirs (added with -B) override the location, -/// so only name priority is accounted for, unless we fail to find -/// anything at all in the prefix. - -/// Copy clang to a new dir which will be its -/// "program path" for these tests -// RUN: rm -rf %t && mkdir -p %t -// RUN: ln -s %clang %t/clang - -/// No gccs at all, nothing is found -// RUN: env "PATH=" %t/clang -### -target notreal-none-elf %s 2>&1 | \ -// RUN: FileCheck --check-prefix=NO_NOTREAL_GCC %s -// NO_NOTREAL_GCC-NOT: notreal-none-elf-gcc -// NO_NOTREAL_GCC-NOT: /gcc - -/// -gcc in program path is found -// RUN: touch %t/notreal-none-elf-gcc && chmod +x %t/notreal-none-elf-gcc -// RUN: env "PATH=" %t/clang -### -target notreal-none-elf %s 2>&1 | \ -// RUN: FileCheck --check-prefix=PROG_PATH_NOTREAL_GCC %s -// PROG_PATH_NOTREAL_GCC: notreal-none-elf-gcc - -/// -gcc on the PATH is found -// RUN: mkdir -p %t/env -// RUN: rm %t/notreal-no
Re: [clang] 028571d - [clang][Driver] Correct tool search path priority
Thanks for the heads up Nico. Yes that was a mistake, it was reviewed as https://reviews.llvm.org/D79842 but somehow I told arc to land it as the duplicate diff instead. I'll look into the failure. On Mon, 22 Jun 2020 at 13:48, Nico Weber wrote: > > Hi, > > https://reviews.llvm.org/D79988 is apparently a "Restricted Differential > Revision" and I don't have permissions to do that. This being an open source > project, this is not something we do. I'm guessing it's not intentional? > > This also breaks check-clang on macOS: > http://45.33.8.238/mac/15950/step_7.txt Please take a look and revert if it > takes a while to investigate. > > Nico > > On Mon, Jun 22, 2020 at 4:41 AM David Spickett via cfe-commits > wrote: >> >> >> Author: David Spickett >> Date: 2020-06-22T09:41:13+01:00 >> New Revision: 028571d60843cb87e2637ef69ee09090d4526c62 >> >> URL: >> https://github.com/llvm/llvm-project/commit/028571d60843cb87e2637ef69ee09090d4526c62 >> DIFF: >> https://github.com/llvm/llvm-project/commit/028571d60843cb87e2637ef69ee09090d4526c62.diff >> >> LOG: [clang][Driver] Correct tool search path priority >> >> Summary: >> As seen in: >> https://bugs.llvm.org/show_bug.cgi?id=45693 >> >> When clang looks for a tool it has a set of >> possible names for it, in priority order. >> Previously it would look for these names in >> the program path. Then look for all the names >> in the PATH. >> >> This means that aarch64-none-elf-gcc on the PATH >> would lose to gcc in the program path. >> (which was /usr/bin in the bug's case) >> >> This changes that logic to search each name in both >> possible locations, then move to the next name. >> Which is more what you would expect to happen when >> using a non default triple. >> >> (-B prefixes maybe should follow this logic too, >> but are not changed in this patch) >> >> Subscribers: kristof.beyls, cfe-commits >> >> Tags: #clang >> >> Differential Revision: https://reviews.llvm.org/D79988 >> >> Added: >> clang/test/Driver/program-path-priority.c >> >> Modified: >> clang/lib/Driver/Driver.cpp >> clang/test/lit.cfg.py >> >> Removed: >> >> >> >> >> diff --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp >> index a48761af400f..27477553963c 100644 >> --- a/clang/lib/Driver/Driver.cpp >> +++ b/clang/lib/Driver/Driver.cpp >> @@ -4717,13 +4717,11 @@ void Driver::generatePrefixedToolNames( >> } >> >> static bool ScanDirForExecutable(SmallString<128> &Dir, >> - ArrayRef Names) { >> - for (const auto &Name : Names) { >> -llvm::sys::path::append(Dir, Name); >> -if (llvm::sys::fs::can_execute(Twine(Dir))) >> - return true; >> -llvm::sys::path::remove_filename(Dir); >> - } >> + const std::string &Name) { >> + llvm::sys::path::append(Dir, Name); >> + if (llvm::sys::fs::can_execute(Twine(Dir))) >> +return true; >> + llvm::sys::path::remove_filename(Dir); >>return false; >> } >> >> @@ -4736,8 +4734,9 @@ std::string Driver::GetProgramPath(StringRef Name, >> const ToolChain &TC) const { >>for (const auto &PrefixDir : PrefixDirs) { >> if (llvm::sys::fs::is_directory(PrefixDir)) { >>SmallString<128> P(PrefixDir); >> - if (ScanDirForExecutable(P, TargetSpecificExecutables)) >> -return std::string(P.str()); >> + for (const auto &TargetSpecificExecutable : TargetSpecificExecutables) >> +if (ScanDirForExecutable(P, TargetSpecificExecutable)) >> + return std::string(P.str()); >> } else { >>SmallString<128> P((PrefixDir + Name).str()); >>if (llvm::sys::fs::can_execute(Twine(P))) >> @@ -4746,17 +4745,25 @@ std::string Driver::GetProgramPath(StringRef Name, >> const ToolChain &TC) const { >>} >> >>const ToolChain::path_list &List = TC.getProgramPaths(); >> - for (const auto &Path : List) { >> -SmallString<128> P(Path); >> -if (ScanDirForExecutable(P, TargetSpecificExecutables)) >> - return std::string(P.str()); >> - } >> + for (const auto &TargetSpecificExecutable : TargetSpecificExecutables) { >> +// For each possible name of the tool look for it in &
[clang] d6efc98 - Reland "[clang][Driver] Correct tool search path priority"
Author: David Spickett Date: 2020-06-25T09:33:43+01:00 New Revision: d6efc9811646edbfe13f06c2676fb469f1c155b1 URL: https://github.com/llvm/llvm-project/commit/d6efc9811646edbfe13f06c2676fb469f1c155b1 DIFF: https://github.com/llvm/llvm-project/commit/d6efc9811646edbfe13f06c2676fb469f1c155b1.diff LOG: Reland "[clang][Driver] Correct tool search path priority" This reverts commit f570d5810485fa6fb2e1009f795a899d79bd429f. The test was failing on MacOS if you set LLVM_DEFAULT_TARGET_TRIPLE. For example if you set it to "x86_64-apple-darwin" clang actually uses "x86_64-apple-darwin". To fix this get default triple from clang itself during the test instead of substituting it in via lit. Added: clang/test/Driver/program-path-priority.c Modified: clang/lib/Driver/Driver.cpp Removed: diff --git a/clang/lib/Driver/Driver.cpp b/clang/lib/Driver/Driver.cpp index 24436ec4e8d7..1f718b30c551 100644 --- a/clang/lib/Driver/Driver.cpp +++ b/clang/lib/Driver/Driver.cpp @@ -4766,13 +4766,11 @@ void Driver::generatePrefixedToolNames( } static bool ScanDirForExecutable(SmallString<128> &Dir, - ArrayRef Names) { - for (const auto &Name : Names) { -llvm::sys::path::append(Dir, Name); -if (llvm::sys::fs::can_execute(Twine(Dir))) - return true; -llvm::sys::path::remove_filename(Dir); - } + const std::string &Name) { + llvm::sys::path::append(Dir, Name); + if (llvm::sys::fs::can_execute(Twine(Dir))) +return true; + llvm::sys::path::remove_filename(Dir); return false; } @@ -4785,8 +4783,9 @@ std::string Driver::GetProgramPath(StringRef Name, const ToolChain &TC) const { for (const auto &PrefixDir : PrefixDirs) { if (llvm::sys::fs::is_directory(PrefixDir)) { SmallString<128> P(PrefixDir); - if (ScanDirForExecutable(P, TargetSpecificExecutables)) -return std::string(P.str()); + for (const auto &TargetSpecificExecutable : TargetSpecificExecutables) +if (ScanDirForExecutable(P, TargetSpecificExecutable)) + return std::string(P.str()); } else { SmallString<128> P((PrefixDir + Name).str()); if (llvm::sys::fs::can_execute(Twine(P))) @@ -4795,17 +4794,25 @@ std::string Driver::GetProgramPath(StringRef Name, const ToolChain &TC) const { } const ToolChain::path_list &List = TC.getProgramPaths(); - for (const auto &Path : List) { -SmallString<128> P(Path); -if (ScanDirForExecutable(P, TargetSpecificExecutables)) - return std::string(P.str()); - } + for (const auto &TargetSpecificExecutable : TargetSpecificExecutables) { +// For each possible name of the tool look for it in +// program paths first, then the path. +// Higher priority names will be first, meaning that +// a higher priority name in the path will be found +// instead of a lower priority name in the program path. +// E.g. -gcc on the path will be found instead +// of gcc in the program path +for (const auto &Path : List) { + SmallString<128> P(Path); + if (ScanDirForExecutable(P, TargetSpecificExecutable)) +return std::string(P.str()); +} - // If all else failed, search the path. - for (const auto &TargetSpecificExecutable : TargetSpecificExecutables) +// Fall back to the path if (llvm::ErrorOr P = llvm::sys::findProgramByName(TargetSpecificExecutable)) return *P; + } return std::string(Name); } diff --git a/clang/test/Driver/program-path-priority.c b/clang/test/Driver/program-path-priority.c new file mode 100644 index ..ba893e7e2e2c --- /dev/null +++ b/clang/test/Driver/program-path-priority.c @@ -0,0 +1,109 @@ +/// Don't create symlinks on Windows +// UNSUPPORTED: system-windows + +/// Check the priority used when searching for tools +/// Names and locations are usually in this order: +/// -tool, tool, -tool +/// program path, PATH +/// (from highest to lowest priority) +/// A higher priority name found in a lower priority +/// location will win over a lower priority name in a +/// higher priority location. +/// Prefix dirs (added with -B) override the location, +/// so only name priority is accounted for, unless we fail to find +/// anything at all in the prefix. + +/// Symlink clang to a new dir which will be its +/// "program path" for these tests +// RUN: rm -rf %t && mkdir -p %t +// RUN: ln -s %clang %t/clang + +/// No gccs at all, nothing is found +// RUN: env "PATH=" %t/clang -### -target notreal-none-elf %s 2>&1 | \ +// RUN: FileCheck --check-prefix=NO_NOTREAL_GCC %s +// NO_NOTREAL_GCC-NOT: notreal-none-elf-gcc +// NO_NOTREAL_GCC-NOT: /gcc + +/// -gcc in program path is found +// RUN: touch %t/notreal-none-elf-gcc && chmod +x %t/notreal-none-elf-gcc +// RUN: env "PATH=" %t/clang -### -target notreal-none-elf %s 2>&1 | \ +// RUN: FileCheck --check-prefix=PROG_PATH_
[clang] e428baf - [LLVM][ARM] Remove options for armv2, 2A, 3 and 3M
Author: David Spickett Date: 2022-09-08T09:49:48Z New Revision: e428baf0019e5292d943a8e37bf08f1192a1870c URL: https://github.com/llvm/llvm-project/commit/e428baf0019e5292d943a8e37bf08f1192a1870c DIFF: https://github.com/llvm/llvm-project/commit/e428baf0019e5292d943a8e37bf08f1192a1870c.diff LOG: [LLVM][ARM] Remove options for armv2, 2A, 3 and 3M Fixes #57486 These pre v4 architectures are not specifically supported by codegen. As demonstrated in the linked issue. GCC has not supported 3M since GCC 9 and presumably 2 and 2A earlier than that. So we are aligned in that sense. (see https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=2abd6e34fcf3bd9f9ffafcaa47cdc3ed443f9add) This removes the options and associated testing. The Pre_v4 build attribute remains mainly because its absence would be more confusing. It will not be used other than to complete the list of build attributes as shown in the ABI. https://github.com/ARM-software/abi-aa/blob/main/addenda32/addenda32.rst#3352the-target-related-attributes Reviewed By: nickdesaulniers, peter.smith, rengolin Differential Revision: https://reviews.llvm.org/D133109 Added: Modified: clang/docs/ReleaseNotes.rst llvm/docs/ReleaseNotes.rst llvm/include/llvm/Support/ARMTargetParser.def llvm/lib/Support/ARMTargetParser.cpp llvm/lib/Target/ARM/ARM.td llvm/lib/Target/ARM/ARMSubtarget.h llvm/lib/Target/ARM/MCTargetDesc/ARMELFStreamer.cpp llvm/unittests/Support/TargetParserTest.cpp Removed: llvm/test/MC/ARM/directive-arch-armv2.s llvm/test/MC/ARM/directive-arch-armv2a.s llvm/test/MC/ARM/directive-arch-armv3.s llvm/test/MC/ARM/directive-arch-armv3m.s diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 425135d746b99..5bd812aca97f4 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -284,6 +284,10 @@ DWARF Support in Clang Arm and AArch64 Support in Clang +- `-march` values for targeting armv2, armv2A, armv3 and armv3M have been removed. + Their presence gave the impression that Clang can correctly generate code for + them, which it cannot. + Floating Point Support in Clang --- diff --git a/llvm/docs/ReleaseNotes.rst b/llvm/docs/ReleaseNotes.rst index 7660f3059c531..b5592aae0af5f 100644 --- a/llvm/docs/ReleaseNotes.rst +++ b/llvm/docs/ReleaseNotes.rst @@ -82,6 +82,10 @@ Changes to the AMDGPU Backend Changes to the ARM Backend -- +* Support for targeting armv2, armv2A, armv3 and armv3M has been removed. + LLVM did not, and was not ever likely to generate correct code for those + architecture versions so their presence was misleading. + Changes to the AVR Backend -- diff --git a/llvm/include/llvm/Support/ARMTargetParser.def b/llvm/include/llvm/Support/ARMTargetParser.def index 645c3b8963f54..ad498047b0885 100644 --- a/llvm/include/llvm/Support/ARMTargetParser.def +++ b/llvm/include/llvm/Support/ARMTargetParser.def @@ -47,14 +47,6 @@ ARM_FPU("softvfp", FK_SOFTVFP, FPUVersion::NONE, NeonSupportLevel::None, FPURest #endif ARM_ARCH("invalid", INVALID, "", "", ARMBuildAttrs::CPUArch::Pre_v4, FK_NONE, ARM::AEK_NONE) -ARM_ARCH("armv2", ARMV2, "2", "v2", ARMBuildAttrs::CPUArch::Pre_v4, - FK_NONE, ARM::AEK_NONE) -ARM_ARCH("armv2a", ARMV2A, "2A", "v2a", ARMBuildAttrs::CPUArch::Pre_v4, - FK_NONE, ARM::AEK_NONE) -ARM_ARCH("armv3", ARMV3, "3", "v3", ARMBuildAttrs::CPUArch::Pre_v4, - FK_NONE, ARM::AEK_NONE) -ARM_ARCH("armv3m", ARMV3M, "3M", "v3m", ARMBuildAttrs::CPUArch::Pre_v4, - FK_NONE, ARM::AEK_NONE) ARM_ARCH("armv4", ARMV4, "4", "v4", ARMBuildAttrs::CPUArch::v4, FK_NONE, ARM::AEK_NONE) ARM_ARCH("armv4t", ARMV4T, "4T", "v4t", ARMBuildAttrs::CPUArch::v4T, diff --git a/llvm/lib/Support/ARMTargetParser.cpp b/llvm/lib/Support/ARMTargetParser.cpp index d7294b5b10744..e4c6bcb028acb 100644 --- a/llvm/lib/Support/ARMTargetParser.cpp +++ b/llvm/lib/Support/ARMTargetParser.cpp @@ -39,12 +39,6 @@ ARM::ArchKind ARM::parseArch(StringRef Arch) { unsigned ARM::parseArchVersion(StringRef Arch) { Arch = getCanonicalArchName(Arch); switch (parseArch(Arch)) { - case ArchKind::ARMV2: - case ArchKind::ARMV2A: -return 2; - case ArchKind::ARMV3: - case ArchKind::ARMV3M: -return 3; case ArchKind::ARMV4: case ArchKind::ARMV4T: return 4; @@ -125,10 +119,6 @@ ARM::ProfileKind ARM::parseArchProfile(StringRef Arch) { case ArchKind::ARMV9_2A: case ArchKind::ARMV9_3A: return ProfileKind::A; - case ArchKind::ARMV2: - case ArchKind::ARMV2A: - case ArchKind::ARMV3: - case ArchKind::ARMV3M: case ArchKind::ARMV4: case ArchKind::ARMV4T: case ArchKind::ARMV5T: diff --git a/llvm/lib/Target/ARM/ARM.td b/llvm/lib/Target/ARM/ARM.td index 4da91e1166c
[llvm] [clang] [ARM] .fpu equals fpv5-d16 disables floating point MVE which leads to unsupported MVE instructions assembler error for cortex M85/M55. (PR #71545)
DavidSpickett wrote: I think the commit title would make more sense at a glance if it was saying what is being done instead of what's being fixed. How about: ``` [llvm][ARM] Emit MVE .arch_extension after .fpu directive if it does not include MVE features ``` Then the commit message is fine as is, and tells the reader why this was done and what it fixes. https://github.com/llvm/llvm-project/pull/71545 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[llvm] [clang] [ARM] .fpu equals fpv5-d16 disables floating point MVE which leads to unsupported MVE instructions assembler error for cortex M85/M55. (PR #71545)
@@ -0,0 +1,56 @@ +; REQUIRES: arm-registered-target +; RUN: %clang --target=arm-none-eabi -mcpu=cortex-m85 -mfloat-abi=hard -save-temps=obj -S -o - %s | FileCheck %s +; RUN: %clang --target=arm-none-eabi -mcpu=cortex-m55 -mfloat-abi=hard -save-temps=obj -S -o - %s | FileCheck %s +; RUN: %clang --target=arm-none-eabi -mcpu=cortex-m85 -mfloat-abi=hard -O2 -c -mthumb -save-temps=obj %s +; RUN: %clang --target=arm-none-eabi -mcpu=cortex-m55 -mfloat-abi=hard -O2 -c -mthumb -save-temps=obj %s +; CHECK: .fpu fpv5-d16 +; CHECK .arch_extension mve.fp +target datalayout = "e-m:e-p:32:32-Fi8-i64:64-v128:64:128-a:0:32-n32-S64" +target triple = "thumbv8.1m.main-none-unknown-eabihf" + +%struct.dummy_t = type { float, float, float, float } + +define dso_local signext i8 @foo(ptr noundef %handle) #0 { +entry: + %handle.addr = alloca ptr, align 4 + store ptr %handle, ptr %handle.addr, align 4 + %0 = load ptr, ptr %handle.addr, align 4 + %a = getelementptr inbounds %struct.dummy_t, ptr %0, i32 0, i32 0 + %1 = load float, ptr %a, align 4 + %sub = fsub float 0x3F5439DE4000, %1 + %2 = load ptr, ptr %handle.addr, align 4 + %a1 = getelementptr inbounds %struct.dummy_t, ptr %2, i32 0, i32 0 + %3 = load float, ptr %a1, align 4 + %4 = call float @llvm.fmuladd.f32(float 0x3F847AE14000, float %sub, float %3) + store float %4, ptr %a1, align 4 + %5 = load ptr, ptr %handle.addr, align 4 + %b = getelementptr inbounds %struct.dummy_t, ptr %5, i32 0, i32 1 + %6 = load float, ptr %b, align 4 + %sub2 = fsub float 0x3F5439DE4000, %6 + %7 = load ptr, ptr %handle.addr, align 4 + %b3 = getelementptr inbounds %struct.dummy_t, ptr %7, i32 0, i32 1 + %8 = load float, ptr %b3, align 4 + %9 = call float @llvm.fmuladd.f32(float 0x3F947AE14000, float %sub2, float %8) + store float %9, ptr %b3, align 4 + %10 = load ptr, ptr %handle.addr, align 4 + %c = getelementptr inbounds %struct.dummy_t, ptr %10, i32 0, i32 2 + %11 = load float, ptr %c, align 4 + %sub4 = fsub float 0x3F5439DE4000, %11 + %12 = load ptr, ptr %handle.addr, align 4 + %c5 = getelementptr inbounds %struct.dummy_t, ptr %12, i32 0, i32 2 + %13 = load float, ptr %c5, align 4 + %14 = call float @llvm.fmuladd.f32(float 0x3F9EB851E000, float %sub4, float %13) + store float %14, ptr %c5, align 4 + %15 = load ptr, ptr %handle.addr, align 4 + %d = getelementptr inbounds %struct.dummy_t, ptr %15, i32 0, i32 3 + %16 = load float, ptr %d, align 4 + %sub6 = fsub float 0x3F5439DE4000, %16 + %17 = load ptr, ptr %handle.addr, align 4 + %d7 = getelementptr inbounds %struct.dummy_t, ptr %17, i32 0, i32 3 + %18 = load float, ptr %d7, align 4 + %19 = call float @llvm.fmuladd.f32(float 0x3FA47AE14000, float %sub6, float %18) + store float %19, ptr %d7, align 4 + ret i8 0 DavidSpickett wrote: Do you need such a large example just to get the directives emitted? https://github.com/llvm/llvm-project/pull/71545 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[llvm] [clang] [ARM] .fpu equals fpv5-d16 disables floating point MVE which leads to unsupported MVE instructions assembler error for cortex M85/M55. (PR #71545)
@@ -0,0 +1,56 @@ +; REQUIRES: arm-registered-target +; RUN: %clang --target=arm-none-eabi -mcpu=cortex-m85 -mfloat-abi=hard -save-temps=obj -S -o - %s | FileCheck %s +; RUN: %clang --target=arm-none-eabi -mcpu=cortex-m55 -mfloat-abi=hard -save-temps=obj -S -o - %s | FileCheck %s +; RUN: %clang --target=arm-none-eabi -mcpu=cortex-m85 -mfloat-abi=hard -O2 -c -mthumb -save-temps=obj %s +; RUN: %clang --target=arm-none-eabi -mcpu=cortex-m55 -mfloat-abi=hard -O2 -c -mthumb -save-temps=obj %s +; CHECK: .fpu fpv5-d16 +; CHECK .arch_extension mve.fp DavidSpickett wrote: Is arch_extension expected to always be on the very next line? If so, CHECK-NEXT would be bit more robust. https://github.com/llvm/llvm-project/pull/71545 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[llvm] [clang] [ARM] .fpu equals fpv5-d16 disables floating point MVE which leads to unsupported MVE instructions assembler error for cortex M85/M55. (PR #71545)
@@ -0,0 +1,56 @@ +; REQUIRES: arm-registered-target +; RUN: %clang --target=arm-none-eabi -mcpu=cortex-m85 -mfloat-abi=hard -save-temps=obj -S -o - %s | FileCheck %s +; RUN: %clang --target=arm-none-eabi -mcpu=cortex-m55 -mfloat-abi=hard -save-temps=obj -S -o - %s | FileCheck %s +; RUN: %clang --target=arm-none-eabi -mcpu=cortex-m85 -mfloat-abi=hard -O2 -c -mthumb -save-temps=obj %s +; RUN: %clang --target=arm-none-eabi -mcpu=cortex-m55 -mfloat-abi=hard -O2 -c -mthumb -save-temps=obj %s +; CHECK: .fpu fpv5-d16 +; CHECK .arch_extension mve.fp +target datalayout = "e-m:e-p:32:32-Fi8-i64:64-v128:64:128-a:0:32-n32-S64" +target triple = "thumbv8.1m.main-none-unknown-eabihf" DavidSpickett wrote: Not 100% sure but I think you can leave these out and they'll be got from the options given to clang regardless (not very important though). https://github.com/llvm/llvm-project/pull/71545 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[llvm] [clang] [llvm][ARM] Emit MVE .arch_extension after .fpu directive if it does not include MVE features (PR #71545)
@@ -0,0 +1,56 @@ +; REQUIRES: arm-registered-target +; RUN: %clang --target=arm-none-eabi -mcpu=cortex-m85 -mfloat-abi=hard -save-temps=obj -S -o - %s | FileCheck %s +; RUN: %clang --target=arm-none-eabi -mcpu=cortex-m55 -mfloat-abi=hard -save-temps=obj -S -o - %s | FileCheck %s +; RUN: %clang --target=arm-none-eabi -mcpu=cortex-m85 -mfloat-abi=hard -O2 -c -mthumb -save-temps=obj %s +; RUN: %clang --target=arm-none-eabi -mcpu=cortex-m55 -mfloat-abi=hard -O2 -c -mthumb -save-temps=obj %s +; CHECK: .fpu fpv5-d16 +; CHECK .arch_extension mve.fp +target datalayout = "e-m:e-p:32:32-Fi8-i64:64-v128:64:128-a:0:32-n32-S64" +target triple = "thumbv8.1m.main-none-unknown-eabihf" + +%struct.dummy_t = type { float, float, float, float } + +define dso_local signext i8 @foo(ptr noundef %handle) #0 { +entry: + %handle.addr = alloca ptr, align 4 + store ptr %handle, ptr %handle.addr, align 4 + %0 = load ptr, ptr %handle.addr, align 4 + %a = getelementptr inbounds %struct.dummy_t, ptr %0, i32 0, i32 0 + %1 = load float, ptr %a, align 4 + %sub = fsub float 0x3F5439DE4000, %1 + %2 = load ptr, ptr %handle.addr, align 4 + %a1 = getelementptr inbounds %struct.dummy_t, ptr %2, i32 0, i32 0 + %3 = load float, ptr %a1, align 4 + %4 = call float @llvm.fmuladd.f32(float 0x3F847AE14000, float %sub, float %3) + store float %4, ptr %a1, align 4 + %5 = load ptr, ptr %handle.addr, align 4 + %b = getelementptr inbounds %struct.dummy_t, ptr %5, i32 0, i32 1 + %6 = load float, ptr %b, align 4 + %sub2 = fsub float 0x3F5439DE4000, %6 + %7 = load ptr, ptr %handle.addr, align 4 + %b3 = getelementptr inbounds %struct.dummy_t, ptr %7, i32 0, i32 1 + %8 = load float, ptr %b3, align 4 + %9 = call float @llvm.fmuladd.f32(float 0x3F947AE14000, float %sub2, float %8) + store float %9, ptr %b3, align 4 + %10 = load ptr, ptr %handle.addr, align 4 + %c = getelementptr inbounds %struct.dummy_t, ptr %10, i32 0, i32 2 + %11 = load float, ptr %c, align 4 + %sub4 = fsub float 0x3F5439DE4000, %11 + %12 = load ptr, ptr %handle.addr, align 4 + %c5 = getelementptr inbounds %struct.dummy_t, ptr %12, i32 0, i32 2 + %13 = load float, ptr %c5, align 4 + %14 = call float @llvm.fmuladd.f32(float 0x3F9EB851E000, float %sub4, float %13) + store float %14, ptr %c5, align 4 + %15 = load ptr, ptr %handle.addr, align 4 + %d = getelementptr inbounds %struct.dummy_t, ptr %15, i32 0, i32 3 + %16 = load float, ptr %d, align 4 + %sub6 = fsub float 0x3F5439DE4000, %16 + %17 = load ptr, ptr %handle.addr, align 4 + %d7 = getelementptr inbounds %struct.dummy_t, ptr %17, i32 0, i32 3 + %18 = load float, ptr %d7, align 4 + %19 = call float @llvm.fmuladd.f32(float 0x3FA47AE14000, float %sub6, float %18) + store float %19, ptr %d7, align 4 + ret i8 0 DavidSpickett wrote: Ok I see, you need to setup the call to this fmuladd, that you know will emit MVE instructions. In that case, could you add a check line for one of the MVE instructions as well? That would make the intent clearer, and warn us in the (unlikely) case that this code stopped emitting MVE instructions. https://github.com/llvm/llvm-project/pull/71545 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[llvm] [clang] [llvm][ARM] Emit MVE .arch_extension after .fpu directive if it does not include MVE features (PR #71545)
https://github.com/DavidSpickett edited https://github.com/llvm/llvm-project/pull/71545 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [AArch64] Add soft-float ABI (PR #74460)
DavidSpickett wrote: (maybe this is for a later patch but...) Does this patch include the clang side options for the ABI or does `-march=...+no-fp-armv8` already work for that? https://github.com/llvm/llvm-project/pull/74460 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [AArch64] Add soft-float ABI (PR #74460)
DavidSpickett wrote: That makes sense. Didn't realise we had an `fp` flag, I don't see it in `llvm/include/llvm/TargetParser/AArch64TargetParser.h`. https://github.com/llvm/llvm-project/pull/74460 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [ARM][AArch32] Add support for AArch32 Cortex-M52 CPU (PR #74822)
DavidSpickett wrote: I also made a small change in this area the other day and got a mountain of clang-format-diff changes. So in case it's not clear, you can ignore the formatter and it won't block the approval/merge. Makes sense not to fill the change with unrelated formatting changes. https://github.com/llvm/llvm-project/pull/74822 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [ARM][AArch32] Add support for AArch32 Cortex-M52 CPU (PR #74822)
@@ -102,7 +102,7 @@ Changes to the AMDGPU Backend * Implemented :ref:`llvm.get.rounding ` -* Added support for Cortex-A520, Cortex-A720 and Cortex-X4 CPUs. +* Added support for Cortex-A520, Cortex-A720, Cortex-X4 and Cortex-M52 CPUs. DavidSpickett wrote: This line seems to have been in the wrong section to begin with. I think the AArch64 CPUs need to go in that section, then this one in the ARM section below. https://github.com/llvm/llvm-project/pull/74822 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[llvm] [clang] [ARM][AArch32] Add support for AArch32 Cortex-M52 CPU (PR #74822)
DavidSpickett wrote: Going by the page (didn't see a link to a manual, maybe I missed it), MVE and FPU are optional. "Optional Helium technology (M-profile Vector Extension) supporting up to:" "Optional FPU with support for half precision (fp16), single precision (fp32) and double precision (fp64) floating-point operations." Is this following a pattern from previous CPUs where these things are optional, but users are expected to pass `+nomve` etc. to disable them? (I don't disagree with that, just want to keep it consistent) https://github.com/llvm/llvm-project/pull/74822 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [ARM][AArch32] Add support for AArch32 Cortex-M52 CPU (PR #74822)
DavidSpickett wrote: > Yes, that's correct. We enable all mandatory and optional architecture > extensions, with the exception of crypto. Cool. That said then, should CDE be added? ``` Accelerator support Optional coprocessor interface (64-bit) supporting up to 8 coprocessor units for custom compute accelerators Optional [Arm Custom Instructions](https://developer.arm.com/architectures/instruction-sets/custom-instructions) ``` https://github.com/llvm/llvm-project/pull/74822 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [ARM][AArch32] Add support for AArch32 Cortex-M52 CPU (PR #74822)
@@ -896,9 +896,13 @@ Arm and AArch64 Support Support has been added for the following processors (-mcpu identifiers in parenthesis): - * Arm Cortex-A520 (cortex-a520). - * Arm Cortex-A720 (cortex-a720). - * Arm Cortex-X4 (cortex-x4). + --target=arm + * Arm Cortex-M52 (cortex-m52). + + --target=aarch64 + * Arm Cortex-A520 (cortex-a520). + * Arm Cortex-A720 (cortex-a720). + * Arm Cortex-X4 (cortex-x4). DavidSpickett wrote: When rendered into HTML, this may not turn out the way you expect it to. You can add `LLVM_BUILD_DOCS` and `SPHINX_OUTPUT_HTML` to your cmake config then build them with the `docs-clang-html` target (https://llvm.org/docs/CMake.html). Also github can render the file, but I'm not sure if the result is always going to be the same. https://github.com/llvm/llvm-project/pull/74822 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[llvm] [clang] [ARM][AArch32] Add support for AArch32 Cortex-M52 CPU (PR #74822)
@@ -896,9 +896,13 @@ Arm and AArch64 Support Support has been added for the following processors (-mcpu identifiers in parenthesis): - * Arm Cortex-A520 (cortex-a520). - * Arm Cortex-A720 (cortex-a720). - * Arm Cortex-X4 (cortex-x4). + --target=arm + * Arm Cortex-M52 (cortex-m52). + + --target=aarch64 + * Arm Cortex-A520 (cortex-a520). + * Arm Cortex-A720 (cortex-a720). + * Arm Cortex-X4 (cortex-x4). DavidSpickett wrote: And if you checked that already, I'm probably just seeing a weird artifact in Github's rendering of it. https://github.com/llvm/llvm-project/pull/74822 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[llvm] [clang] [ARM][AArch32] Add support for AArch32 Cortex-M52 CPU (PR #74822)
@@ -896,9 +896,13 @@ Arm and AArch64 Support Support has been added for the following processors (-mcpu identifiers in parenthesis): - * Arm Cortex-A520 (cortex-a520). - * Arm Cortex-A720 (cortex-a720). - * Arm Cortex-X4 (cortex-x4). + --target=arm + * Arm Cortex-M52 (cortex-m52). + + --target=aarch64 + * Arm Cortex-A520 (cortex-a520). + * Arm Cortex-A720 (cortex-a720). + * Arm Cortex-X4 (cortex-x4). DavidSpickett wrote: That's certainly easier to format, so go with that. Wouldn't be surprised if `--` looked to RST like a formatting directive. https://github.com/llvm/llvm-project/pull/74822 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[llvm] [clang] [ARM][AArch32] Add support for AArch32 Cortex-M52 CPU (PR #74822)
https://github.com/DavidSpickett approved this pull request. LGTM https://github.com/llvm/llvm-project/pull/74822 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [AArch64] Correctly mark Neoverse N2 as an Armv9.0a core (PR #75055)
@@ -94,6 +94,11 @@ Changes to the AArch64 Backend * Added support for Cortex-A520, Cortex-A720 and Cortex-X4 CPUs. +* Neoverse-N2 was incorrectly marked as an Armv8.5a core. This has been + changed to an Armv9.0a core. However, crypto options are not enabled + by default for Armv9 cores, so `-mcpu=neoverse-n2+crypto` is required DavidSpickett wrote: `is now required` ? https://github.com/llvm/llvm-project/pull/75055 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [AArch64] Correctly mark Neoverse N2 as an Armv9.0a core (PR #75055)
@@ -94,6 +94,11 @@ Changes to the AArch64 Backend * Added support for Cortex-A520, Cortex-A720 and Cortex-X4 CPUs. +* Neoverse-N2 was incorrectly marked as an Armv8.5a core. This has been + changed to an Armv9.0a core. However, crypto options are not enabled + by default for Armv9 cores, so `-mcpu=neoverse-n2+crypto` is required DavidSpickett wrote: It's my understanding that v8.5-a == v9.0-a as far as the compiler is concerned (there are a couple of extra requirements to be v9.0-a that don't concern the compiler). Is that correct? Might be worth stating that they enable the same features, with the exception of crypto as mentioned. (not saying that they are literally the same, just same as far as a compiler is concerned) https://github.com/llvm/llvm-project/pull/75055 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [AArch64] Correctly mark Neoverse N2 as an Armv9.0a core (PR #75055)
@@ -94,6 +94,11 @@ Changes to the AArch64 Backend * Added support for Cortex-A520, Cortex-A720 and Cortex-X4 CPUs. +* Neoverse-N2 was incorrectly marked as an Armv8.5a core. This has been + changed to an Armv9.0a core. However, crypto options are not enabled + by default for Armv9 cores, so `-mcpu=neoverse-n2+crypto` is required DavidSpickett wrote: That's true but my concern as a user would be does v9.0-a add anything implicitly that wasn't there before. I don't think it does here but worth noting just to save the user the effort of checking. https://github.com/llvm/llvm-project/pull/75055 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [AArch64] Correctly mark Neoverse N2 as an Armv9.0a core (PR #75055)
@@ -94,6 +94,11 @@ Changes to the AArch64 Backend * Added support for Cortex-A520, Cortex-A720 and Cortex-X4 CPUs. +* Neoverse-N2 was incorrectly marked as an Armv8.5a core. This has been + changed to an Armv9.0a core. However, crypto options are not enabled + by default for Armv9 cores, so `-mcpu=neoverse-n2+crypto` is required DavidSpickett wrote: Also clang would let you add sve2 to almost anything, but that's because it doesn't model the rules at all :) https://github.com/llvm/llvm-project/pull/75055 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [AArch64] Correctly mark Neoverse N2 as an Armv9.0a core (PR #75055)
https://github.com/DavidSpickett edited https://github.com/llvm/llvm-project/pull/75055 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [AArch64] Correctly mark Neoverse N2 as an Armv9.0a core (PR #75055)
@@ -94,6 +94,11 @@ Changes to the AArch64 Backend * Added support for Cortex-A520, Cortex-A720 and Cortex-X4 CPUs. +* Neoverse-N2 was incorrectly marked as an Armv8.5a core. This has been + changed to an Armv9.0a core. However, crypto options are not enabled + by default for Armv9 cores, so `-mcpu=neoverse-n2+crypto` is required DavidSpickett wrote: That is a problem worth fixing but it's not unique to this change unfortunately. Examples like this succeed: ``` $ ./bin/clang -march=armv8.1-a+sve2 /tmp/test.c ``` Which is a larger issue with clang. At least with this change, `-mcpu=neoverse-n2+sve+sve2` follows the architecture rules where previously it did not. So that's a good improvement. https://github.com/llvm/llvm-project/pull/75055 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[llvm] [clang] [AArch64] Correctly mark Neoverse N2 as an Armv9.0a core (PR #75055)
https://github.com/DavidSpickett approved this pull request. LGTM Once again clang-format rightly complaining about a lot of the surrounding code, but it would obscure the change here so you can ignore it. https://github.com/llvm/llvm-project/pull/75055 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[llvm] [clang] [llvm][ARM] Emit MVE .arch_extension after .fpu directive if it does not include MVE features (PR #71545)
@@ -0,0 +1,35 @@ +// RUN: %clang --target=arm-none-eabi -mcpu=cortex-m85 -mfloat-abi=hard -O2 -save-temps=obj -S -o - %s | FileCheck %s +// RUN: %clang --target=arm-none-eabi -mcpu=cortex-m85 -mfloat-abi=hard -O2 -c -mthumb -save-temps=obj %s +// RUN: %clang --target=arm-none-eabi -mcpu=cortex-m55 -mfloat-abi=hard -O2 -c -mthumb -save-temps=obj %s DavidSpickett wrote: I presume without this fix, this command would fail because it fails to build the object? If so a comment here would be good, otherwise they look like mistakes as they are RUNs without CHECKs. I'd also put them first, then a blank line, then the RUN + FileCheck lines, just to make it even clearer. https://github.com/llvm/llvm-project/pull/71545 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[llvm] [clang] [llvm][ARM] Emit MVE .arch_extension after .fpu directive if it does not include MVE features (PR #71545)
@@ -0,0 +1,35 @@ +// RUN: %clang --target=arm-none-eabi -mcpu=cortex-m85 -mfloat-abi=hard -O2 -save-temps=obj -S -o - %s | FileCheck %s DavidSpickett wrote: Should there be an m55 test for this as well, as you've got 55 and 85 for save-temps. https://github.com/llvm/llvm-project/pull/71545 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[llvm] [clang] [llvm][ARM] Emit MVE .arch_extension after .fpu directive if it does not include MVE features (PR #71545)
@@ -0,0 +1,35 @@ +// RUN: %clang --target=arm-none-eabi -mcpu=cortex-m85 -mfloat-abi=hard -O2 -save-temps=obj -S -o - %s | FileCheck %s +// RUN: %clang --target=arm-none-eabi -mcpu=cortex-m85 -mfloat-abi=hard -O2 -c -mthumb -save-temps=obj %s +// RUN: %clang --target=arm-none-eabi -mcpu=cortex-m55 -mfloat-abi=hard -O2 -c -mthumb -save-temps=obj %s + +// REQUIRES: arm-registered-target + +// CHECK: .fpu fpv5-d16 +// CHECK-NEXT .arch_extension mve.fp + +#define DUMMY_CONST_1 (0.0012345F) +#define DUMMY_CONST_2 (0.01F) +#define DUMMY_CONST_3 (0.02F) +#define DUMMY_CONST_4 (0.03F) +#define DUMMY_CONST_5 (0.04F) DavidSpickett wrote: I get defining DUMMY_CONST_1 but it seems you could just write the values of the others in `foo`. https://github.com/llvm/llvm-project/pull/71545 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[llvm] [clang] [llvm][ARM] Emit MVE .arch_extension after .fpu directive if it does not include MVE features (PR #71545)
DavidSpickett wrote: Just small things on the test case from me. Nominate @davemgreen to give it a final check and approval, since I have not been around this area in a while. https://github.com/llvm/llvm-project/pull/71545 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [llvm][ARM] Emit MVE .arch_extension after .fpu directive if it does not include MVE features (PR #71545)
DavidSpickett wrote: Tests now LGTM, thanks for the updates. https://github.com/llvm/llvm-project/pull/71545 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] Revert "Revert "[SVE2.1][Clang][LLVM]Add BFloat16 builtin in Clang an… (PR #73110)
https://github.com/DavidSpickett approved this pull request. LGTM, thank you for the fix! You'll know within ~15 minutes of landing whether there is still an issue, but I doubt there will be. https://github.com/llvm/llvm-project/pull/73110 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[llvm] [clang] [clang-tools-extra] [Clang][AArch64] Add fix vector types to header into SVE (PR #73258)
DavidSpickett wrote: I think this caused a test suite program to fail to build: ``` cd /home/tcwg-buildbot/worker/clang-aarch64-global-isel/test/sandbox/build/SingleSource/UnitTests && /home/tcwg-buildbot/worker/clang-aarch64-global-isel/stage1.install/bin/llvm-size --format=sysv /home/tcwg-buildbot/worker/clang-aarch64-global-isel/test/sandbox/build/SingleSource/UnitTests/testcase-CGExprConstant > /home/tcwg-buildbot/worker/clang-aarch64-global-isel/test/sandbox/build/SingleSource/UnitTests/testcase-CGExprConstant.size | ~^ 38 | (value)) | /home/tcwg-buildbot/worker/clang-aarch64-global-isel/test/test-suite/SingleSource/UnitTests/Float/test_isfpclass.h:28:11: note: expanded from macro 'CHECK_CLASS' 28 | if (!(cond)) { \ | ^~~~ /home/tcwg-buildbot/worker/clang-aarch64-global-isel/test/test-suite/SingleSource/UnitTests/Float/test_isfpclass.h:58:3: error: floating point classification requires argument of floating point type (passed in 'int') 58 | CHECK(fcFinite, x); | ^~ /home/tcwg-buildbot/worker/clang-aarch64-global-isel/test/test-suite/SingleSource/UnitTests/Float/test_isfpclass.h:37:60: note: expanded from macro 'CHECK' 37 | CHECK_CLASS(!!((flags)&FPCLASS) == !!__builtin_isfpclass((flags), (value)), \ | ~^ 38 | (value)) | /home/tcwg-buildbot/worker/clang-aarch64-global-isel/test/test-suite/SingleSource/UnitTests/Float/test_isfpclass.h:28:11: note: expanded from macro 'CHECK_CLASS' 28 | if (!(cond)) { \ | ^~~~ /home/tcwg-buildbot/worker/clang-aarch64-global-isel/test/test-suite/SingleSource/UnitTests/Float/test_isfpclass.h:60:3: error: floating point classification requires argument of floating point type (passed in 'int') 60 | CHECK(fcQNan | fcInf | fcNormal | fcSubnormal | fcZero, x); | ^~ /home/tcwg-buildbot/worker/clang-aarch64-global-isel/test/test-suite/SingleSource/UnitTests/Float/test_isfpclass.h:37:60: note: expanded from macro 'CHECK' 37 | CHECK_CLASS(!!((flags)&FPCLASS) == !!__builtin_isfpclass((flags), (value)), \ | ~^ 38 | (value)) | /home/tcwg-buildbot/worker/clang-aarch64-global-isel/test/test-suite/SingleSource/UnitTests/Float/test_isfpclass.h:28:11: note: expanded from macro 'CHECK_CLASS' 28 | if (!(cond)) { \ | ^~~~ ``` https://lab.llvm.org/buildbot/#/builders/183/builds/18017 I'm guessing it includes a header effected by this change. https://github.com/llvm/llvm-project/pull/73258 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[llvm] [clang] [clang-tools-extra] [Clang][AArch64] Add fix vector types to header into SVE (PR #73258)
DavidSpickett wrote: Actually this may be https://github.com/llvm/llvm-test-suite/pull/59, which landed in the test suite just now as well, but is not listed as a change there (which is a known issue with buildbot). https://github.com/llvm/llvm-project/pull/73258 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][ARM] support arm target attribute, and warning for bad typo (PR #74812)
DavidSpickett wrote: Not so sure about that: ``` LINK: command "C:\BuildTools\VC\Tools\MSVC\14.29.30133\bin\Hostx64\x64\link.exe /nologo tools\clang\tools\clang-format\CMakeFiles\clang-format.dir\ClangFormat.cpp.obj tools\clang\tools\clang-format\CMakeFiles\clang-format.dir\C_\ws\src\llvm\resources\windows_version_resource.rc.res /out:bin\clang-format.exe /implib:lib\clang-format.lib /pdb:bin\clang-format.pdb /version:0.0 /machine:x64 /STACK:1000 /INCREMENTAL:NO /subsystem:console lib\LLVMSupport.lib lib\clangBasic.lib lib\clangFormat.lib lib\clangRewrite.lib lib\clangToolingCore.lib lib\clangToolingInclusions.lib lib\clangToolingCore.lib lib\clangRewrite.lib lib\clangLex.lib lib\clangBasic.lib lib\LLVMFrontendOpenMP.lib lib\LLVMScalarOpts.lib lib\LLVMAggressiveInstCombine.lib lib\LLVMInstCombine.lib lib\LLVMFrontendOffloading.lib lib\LLVMTransformUtils.lib lib\LLVMAnalysis.lib lib\LLVMProfileData.lib lib\LLVMSymbolize.lib lib\LLVMDebugInfoPDB.lib C:\BuildTools\DIA SDK\lib\amd64\diaguids.lib lib\LLVMDebugInfoMSF.lib lib\LLVMDebugInfoBTF.lib lib\LLVMDebugInfoDWARF.lib lib\LLVMObject.lib lib\LLVMIRReader.lib lib\LLVMBitReader.lib lib\LLVMAsmParser.lib lib\LLVMCore.lib lib\LLVMRemarks.lib lib\LLVMBitstreamReader.lib lib\LLVMMCParser.lib lib\LLVMMC.lib lib\LLVMDebugInfoCodeView.lib lib\LLVMTextAPI.lib lib\LLVMBinaryFormat.lib lib\LLVMTargetParser.lib lib\LLVMSupport.lib psapi.lib shell32.lib ole32.lib uuid.lib advapi32.lib WS2_32.lib delayimp.lib -delayload:shell32.dll -delayload:ole32.dll lib\LLVMDemangle.lib kernel32.lib user32.lib gdi32.lib winspool.lib shell32.lib ole32.lib oleaut32.lib uuid.lib comdlg32.lib advapi32.lib /MANIFEST /MANIFESTFILE:bin\clang-format.exe.manifest" failed (exit code 1120) with the following output: clangBasic.lib(ARM.cpp.obj) : error LNK2019: unresolved external symbol "private: void __cdecl clang::APValue::DestroyDataAndMakeUninit(void)" (?DestroyDataAndMakeUninit@APValue@clang@@AEAAXXZ) referenced in function "public: __cdecl clang::APValue::~APValue(void)" (??1APValue@clang@@QEAA@XZ) bin\clang-format.exe : fatal error LNK1120: 1 unresolved externals ``` ``` clangBasic.lib(ARM.cpp.obj) : error LNK2019: unresolved external symbol "private: void __cdecl clang::APValue::DestroyDataAndMakeUninit(void)" ``` Which is the file you're modifying. I see that you have added: ``` #include "clang/AST/Attr.h" ``` `DestroyDataAndMakeUninit` is in `clang/lib/AST/APValue.cpp`, which is built into the library `clangAST`. `clang/lib/Basic/Targets/ARM.cpp` is built into `clangBasic` which does not depend on `clangAST`, but does `target_link_libraries` for it. I'm not sure of the difference there, why do one or the other. (clang/lib/Basic/CMakeLists.txt) (if you want to know what library a cpp file ends up in, look in the same folder for a CMakeLists.txt, that usually has the name, one folder up if not and so on) So I guess that on Linux, `target_link_libraries` does what's required but on Windows it does not. https://github.com/llvm/llvm-project/pull/74812 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][ARM] support arm target attribute, and warning for bad typo (PR #74812)
DavidSpickett wrote: Also it seems that `clangAST` links to `clangBasic` but it doesn't `DEPENDS` on them? Not sure what that is trying to achieve. https://github.com/llvm/llvm-project/pull/74812 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][ARM] support arm target attribute, and warning for bad typo (PR #74812)
DavidSpickett wrote: I was mistaken about the `target_link_libraries`, that's what `clangBasic` links to not `clangAST`. It's possible that `clangBasic` now needs to depend on `clangAST`, assuming cmake and the linker are ok with that. https://github.com/llvm/llvm-project/pull/74812 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[flang] [clang] [mlir] [lldb] [libcxxabi] [libc] [compiler-rt] [clang-tools-extra] [lld] [llvm] [libcxx] [openmp] [builtins][arm64] Build __init_cpu_features_resolver on Apple platforms (PR #73685)
DavidSpickett wrote: Also seen on Linaro's Windows on Arm 2 stage bot: https://lab.llvm.org/buildbot/#/builders/120/builds/5990 https://github.com/llvm/llvm-project/pull/73685 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] d7c03a1 - [clang][AArch64][NFC] Remove trailing space in SME intriniscs header
Author: David Spickett Date: 2023-11-27T13:31:15Z New Revision: d7c03a196edb1e5f5a45121bb51899e8f5e72ca6 URL: https://github.com/llvm/llvm-project/commit/d7c03a196edb1e5f5a45121bb51899e8f5e72ca6 DIFF: https://github.com/llvm/llvm-project/commit/d7c03a196edb1e5f5a45121bb51899e8f5e72ca6.diff LOG: [clang][AArch64][NFC] Remove trailing space in SME intriniscs header Added: Modified: clang/utils/TableGen/SveEmitter.cpp Removed: diff --git a/clang/utils/TableGen/SveEmitter.cpp b/clang/utils/TableGen/SveEmitter.cpp index d00989ac0f3beb5..b380bd9dfe6643a 100644 --- a/clang/utils/TableGen/SveEmitter.cpp +++ b/clang/utils/TableGen/SveEmitter.cpp @@ -1571,7 +1571,7 @@ void SVEEmitter::createSMEHeader(raw_ostream &OS) { OS << "#error \"Big endian is currently not supported for arm_sme_draft_spec_subject_to_change.h\"\n"; OS << "#endif\n"; - OS << "#include \n\n"; + OS << "#include \n\n"; OS << "/* Function attributes */\n"; OS << "#define __ai static __inline__ __attribute__((__always_inline__, " ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [lldb] [compiler-rt] [llvm] [clang] [flang] [libc] [lldb][NFC] Fix compilation issue on windows (PR #76453)
https://github.com/DavidSpickett closed https://github.com/llvm/llvm-project/pull/76453 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [Clang][SME] Add IsStreamingOrSVE2p1 (PR #76975)
@@ -50,6 +50,7 @@ class LLVM_LIBRARY_VISIBILITY AArch64TargetInfo : public TargetInfo { bool HasMatMul = false; bool HasBFloat16 = false; bool HasSVE2 = false; + bool HasSVE2p1 = false; DavidSpickett wrote: FYI ``` /home/david.spickett/llvm-project/clang/lib/Basic/Targets/AArch64.h:53:8: warning: private field 'HasSVE2p1' is not used [-Wunused-private-field] bool HasSVE2p1 = false; ^ 1 warning generated. ``` https://github.com/llvm/llvm-project/pull/76975 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] f3fbbe1 - [clang][analyzer][NFC] Use value_or instead of ValueOr
Author: David Spickett Date: 2022-07-26T09:16:45Z New Revision: f3fbbe1cf33bf4da8e2620c770997d9ff68a5384 URL: https://github.com/llvm/llvm-project/commit/f3fbbe1cf33bf4da8e2620c770997d9ff68a5384 DIFF: https://github.com/llvm/llvm-project/commit/f3fbbe1cf33bf4da8e2620c770997d9ff68a5384.diff LOG: [clang][analyzer][NFC] Use value_or instead of ValueOr The latter is deprecated. Added: Modified: clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp Removed: diff --git a/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp b/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp index 836311a69309..04e00274b2a7 100644 --- a/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp +++ b/clang/lib/StaticAnalyzer/Core/ExprEngineCXX.cpp @@ -589,7 +589,7 @@ void ExprEngine::handleConstructor(const Expr *E, unsigned Idx = 0; if (CE->getType()->isArrayType() || AILE) { - Idx = getIndexOfElementToConstruct(State, CE, LCtx).getValueOr(0u); + Idx = getIndexOfElementToConstruct(State, CE, LCtx).value_or(0u); State = setIndexOfElementToConstruct(State, CE, LCtx, Idx + 1); } ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] eb5ecbb - [llvm][AArch64] Insert "bti j" after call to setjmp
Author: David Spickett Date: 2022-03-23T09:51:02Z New Revision: eb5ecbbcbb6ce38e29237ab5d17156fcb2e96e74 URL: https://github.com/llvm/llvm-project/commit/eb5ecbbcbb6ce38e29237ab5d17156fcb2e96e74 DIFF: https://github.com/llvm/llvm-project/commit/eb5ecbbcbb6ce38e29237ab5d17156fcb2e96e74.diff LOG: [llvm][AArch64] Insert "bti j" after call to setjmp Some implementations of setjmp will end with a br instead of a ret. This means that the next instruction after a call to setjmp must be a "bti j" (j for jump) to make this work when branch target identification is enabled. The BTI extension was added in armv8.5-a but the bti instruction is in the hint space. This means we can emit it for any architecture version as long as branch target enforcement flags are passed. The starting point for the hint number is 32 then call adds 2, jump adds 4. Hence "hint #36" for a "bti j" (and "hint #34" for the "bti c" you see at the start of functions). The existing Arm command line option -mno-bti-at-return-twice has been applied to AArch64 as well. Support is added to SelectionDAG Isel and GlobalIsel. FastIsel will defer to SelectionDAG. Based on the change done for M profile Arm in https://reviews.llvm.org/D112427 Fixes #4 Reviewed By: danielkiss Differential Revision: https://reviews.llvm.org/D121707 Added: llvm/test/CodeGen/AArch64/setjmp-bti-no-enforcement.ll llvm/test/CodeGen/AArch64/setjmp-bti-outliner.ll llvm/test/CodeGen/AArch64/setjmp-bti.ll Modified: clang/docs/ClangCommandLineReference.rst clang/docs/ReleaseNotes.rst clang/include/clang/Driver/Options.td clang/lib/Driver/ToolChains/Arch/AArch64.cpp llvm/lib/Target/AArch64/AArch64.td llvm/lib/Target/AArch64/AArch64ExpandPseudoInsts.cpp llvm/lib/Target/AArch64/AArch64FastISel.cpp llvm/lib/Target/AArch64/AArch64ISelLowering.cpp llvm/lib/Target/AArch64/AArch64ISelLowering.h llvm/lib/Target/AArch64/AArch64InstrInfo.td llvm/lib/Target/AArch64/GISel/AArch64CallLowering.cpp Removed: diff --git a/clang/docs/ClangCommandLineReference.rst b/clang/docs/ClangCommandLineReference.rst index 6815dca1f1529..9d097ccae6aab 100644 --- a/clang/docs/ClangCommandLineReference.rst +++ b/clang/docs/ClangCommandLineReference.rst @@ -3329,7 +3329,7 @@ Work around VLLDM erratum CVE-2021-35465 (ARM only) .. option:: -mno-bti-at-return-twice -Do not add a BTI instruction after a setjmp or other return-twice construct (Arm only) +Do not add a BTI instruction after a setjmp or other return-twice construct (AArch32/AArch64 only) .. option:: -mno-movt diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index c30512c80e0b7..105d501073174 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -193,6 +193,11 @@ DWARF Support in Clang Arm and AArch64 Support in Clang +- When using ``-mbranch-protection=bti`` with AArch64, calls to setjmp will + now be followed by a BTI instruction. This is done to be compatible with + setjmp implementations that return with a br instead of a ret. You can + disable this behaviour using the ``-mno-bti-at-return-twice`` option. + Floating Point Support in Clang --- diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td index c56578b34641a..41b3ca5a4583e 100644 --- a/clang/include/clang/Driver/Options.td +++ b/clang/include/clang/Driver/Options.td @@ -3414,7 +3414,7 @@ def mmark_bti_property : Flag<["-"], "mmark-bti-property">, def mno_bti_at_return_twice : Flag<["-"], "mno-bti-at-return-twice">, Group, HelpText<"Do not add a BTI instruction after a setjmp or other" - " return-twice construct (Arm only)">; + " return-twice construct (Arm/AArch64 only)">; foreach i = {1-31} in def ffixed_x#i : Flag<["-"], "ffixed-x"#i>, Group, diff --git a/clang/lib/Driver/ToolChains/Arch/AArch64.cpp b/clang/lib/Driver/ToolChains/Arch/AArch64.cpp index f9557bac5fcdc..610c672feb677 100644 --- a/clang/lib/Driver/ToolChains/Arch/AArch64.cpp +++ b/clang/lib/Driver/ToolChains/Arch/AArch64.cpp @@ -588,4 +588,7 @@ void aarch64::getAArch64TargetFeatures(const Driver &D, // Enabled A53 errata (835769) workaround by default on android Features.push_back("+fix-cortex-a53-835769"); } + + if (Args.getLastArg(options::OPT_mno_bti_at_return_twice)) +Features.push_back("+no-bti-at-return-twice"); } diff --git a/llvm/lib/Target/AArch64/AArch64.td b/llvm/lib/Target/AArch64/AArch64.td index 84d388d94e596..82161b162ecdf 100644 --- a/llvm/lib/Target/AArch64/AArch64.td +++ b/llvm/lib/Target/AArch64/AArch64.td @@ -466,6 +466,11 @@ def FeatureEL3 : SubtargetFeature<"el3", "HasEL3", "true", def FeatureFixCortexA53_835769 : SubtargetFeature<"fix-cortex-a53-835769", "FixCortexA53_835769", "true", "Mitigate Cortex-A53 Erratum 83
[clang] edb7ba7 - Revert "[llvm][AArch64] Insert "bti j" after call to setjmp"
Author: David Spickett Date: 2022-03-23T10:43:20Z New Revision: edb7ba714acba1d18a20d9f4986d2e38aee1d109 URL: https://github.com/llvm/llvm-project/commit/edb7ba714acba1d18a20d9f4986d2e38aee1d109 DIFF: https://github.com/llvm/llvm-project/commit/edb7ba714acba1d18a20d9f4986d2e38aee1d109.diff LOG: Revert "[llvm][AArch64] Insert "bti j" after call to setjmp" This reverts commit eb5ecbbcbb6ce38e29237ab5d17156fcb2e96e74 due to failures on buildbots with expensive checks enabled. Added: Modified: clang/docs/ClangCommandLineReference.rst clang/docs/ReleaseNotes.rst clang/include/clang/Driver/Options.td clang/lib/Driver/ToolChains/Arch/AArch64.cpp llvm/lib/Target/AArch64/AArch64.td llvm/lib/Target/AArch64/AArch64ExpandPseudoInsts.cpp llvm/lib/Target/AArch64/AArch64FastISel.cpp llvm/lib/Target/AArch64/AArch64ISelLowering.cpp llvm/lib/Target/AArch64/AArch64ISelLowering.h llvm/lib/Target/AArch64/AArch64InstrInfo.td llvm/lib/Target/AArch64/GISel/AArch64CallLowering.cpp Removed: llvm/test/CodeGen/AArch64/setjmp-bti-no-enforcement.ll llvm/test/CodeGen/AArch64/setjmp-bti-outliner.ll llvm/test/CodeGen/AArch64/setjmp-bti.ll diff --git a/clang/docs/ClangCommandLineReference.rst b/clang/docs/ClangCommandLineReference.rst index 9d097ccae6aab..6815dca1f1529 100644 --- a/clang/docs/ClangCommandLineReference.rst +++ b/clang/docs/ClangCommandLineReference.rst @@ -3329,7 +3329,7 @@ Work around VLLDM erratum CVE-2021-35465 (ARM only) .. option:: -mno-bti-at-return-twice -Do not add a BTI instruction after a setjmp or other return-twice construct (AArch32/AArch64 only) +Do not add a BTI instruction after a setjmp or other return-twice construct (Arm only) .. option:: -mno-movt diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 105d501073174..c30512c80e0b7 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -193,11 +193,6 @@ DWARF Support in Clang Arm and AArch64 Support in Clang -- When using ``-mbranch-protection=bti`` with AArch64, calls to setjmp will - now be followed by a BTI instruction. This is done to be compatible with - setjmp implementations that return with a br instead of a ret. You can - disable this behaviour using the ``-mno-bti-at-return-twice`` option. - Floating Point Support in Clang --- diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td index 41b3ca5a4583e..c56578b34641a 100644 --- a/clang/include/clang/Driver/Options.td +++ b/clang/include/clang/Driver/Options.td @@ -3414,7 +3414,7 @@ def mmark_bti_property : Flag<["-"], "mmark-bti-property">, def mno_bti_at_return_twice : Flag<["-"], "mno-bti-at-return-twice">, Group, HelpText<"Do not add a BTI instruction after a setjmp or other" - " return-twice construct (Arm/AArch64 only)">; + " return-twice construct (Arm only)">; foreach i = {1-31} in def ffixed_x#i : Flag<["-"], "ffixed-x"#i>, Group, diff --git a/clang/lib/Driver/ToolChains/Arch/AArch64.cpp b/clang/lib/Driver/ToolChains/Arch/AArch64.cpp index 610c672feb677..f9557bac5fcdc 100644 --- a/clang/lib/Driver/ToolChains/Arch/AArch64.cpp +++ b/clang/lib/Driver/ToolChains/Arch/AArch64.cpp @@ -588,7 +588,4 @@ void aarch64::getAArch64TargetFeatures(const Driver &D, // Enabled A53 errata (835769) workaround by default on android Features.push_back("+fix-cortex-a53-835769"); } - - if (Args.getLastArg(options::OPT_mno_bti_at_return_twice)) -Features.push_back("+no-bti-at-return-twice"); } diff --git a/llvm/lib/Target/AArch64/AArch64.td b/llvm/lib/Target/AArch64/AArch64.td index 82161b162ecdf..84d388d94e596 100644 --- a/llvm/lib/Target/AArch64/AArch64.td +++ b/llvm/lib/Target/AArch64/AArch64.td @@ -466,11 +466,6 @@ def FeatureEL3 : SubtargetFeature<"el3", "HasEL3", "true", def FeatureFixCortexA53_835769 : SubtargetFeature<"fix-cortex-a53-835769", "FixCortexA53_835769", "true", "Mitigate Cortex-A53 Erratum 835769">; -def FeatureNoBTIAtReturnTwice : SubtargetFeature<"no-bti-at-return-twice", - "NoBTIAtReturnTwice", "true", - "Don't place a BTI instruction " - "after a return-twice">; - //===--===// // Architectures. // diff --git a/llvm/lib/Target/AArch64/AArch64ExpandPseudoInsts.cpp b/llvm/lib/Target/AArch64/AArch64ExpandPseudoInsts.cpp index 910f8cdede753..b0f739cc26e69 100644 --- a/llvm/lib/Target/AArch64/AArch64ExpandPseudoInsts.cpp +++ b/llvm/lib/Target/AArch64/AArch64ExpandPseudoInsts.cpp @@ -86,7 +86,6 @@ class AArch64ExpandPseudo : public MachineFunctionPass { unsigned N);
[clang] c3b9819 - Reland "[llvm][AArch64] Insert "bti j" after call to setjmp"
Author: David Spickett Date: 2022-03-23T11:43:43Z New Revision: c3b98194df5572bc9b33024b48457538a7213b4c URL: https://github.com/llvm/llvm-project/commit/c3b98194df5572bc9b33024b48457538a7213b4c DIFF: https://github.com/llvm/llvm-project/commit/c3b98194df5572bc9b33024b48457538a7213b4c.diff LOG: Reland "[llvm][AArch64] Insert "bti j" after call to setjmp" This reverts commit edb7ba714acba1d18a20d9f4986d2e38aee1d109. This changes BLR_BTI to take variable_ops meaning that we can accept a register or a label. The pattern still expects one argument so we'll never get more than one. Then later we can check the type of the operand to choose BL or BLR to emit. (this is what BLR_RVMARKER does but I missed this detail of it first time around) Also require NoSLSBLRMitigation which I missed in the first version. Added: llvm/test/CodeGen/AArch64/setjmp-bti-no-enforcement.ll llvm/test/CodeGen/AArch64/setjmp-bti-outliner.ll llvm/test/CodeGen/AArch64/setjmp-bti.ll Modified: clang/docs/ClangCommandLineReference.rst clang/docs/ReleaseNotes.rst clang/include/clang/Driver/Options.td clang/lib/Driver/ToolChains/Arch/AArch64.cpp llvm/lib/Target/AArch64/AArch64.td llvm/lib/Target/AArch64/AArch64ExpandPseudoInsts.cpp llvm/lib/Target/AArch64/AArch64FastISel.cpp llvm/lib/Target/AArch64/AArch64ISelLowering.cpp llvm/lib/Target/AArch64/AArch64ISelLowering.h llvm/lib/Target/AArch64/AArch64InstrInfo.td llvm/lib/Target/AArch64/GISel/AArch64CallLowering.cpp Removed: diff --git a/clang/docs/ClangCommandLineReference.rst b/clang/docs/ClangCommandLineReference.rst index 6815dca1f1529..9d097ccae6aab 100644 --- a/clang/docs/ClangCommandLineReference.rst +++ b/clang/docs/ClangCommandLineReference.rst @@ -3329,7 +3329,7 @@ Work around VLLDM erratum CVE-2021-35465 (ARM only) .. option:: -mno-bti-at-return-twice -Do not add a BTI instruction after a setjmp or other return-twice construct (Arm only) +Do not add a BTI instruction after a setjmp or other return-twice construct (AArch32/AArch64 only) .. option:: -mno-movt diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index 680bdbe4dbfa0..403bc08eec9be 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -195,6 +195,11 @@ DWARF Support in Clang Arm and AArch64 Support in Clang +- When using ``-mbranch-protection=bti`` with AArch64, calls to setjmp will + now be followed by a BTI instruction. This is done to be compatible with + setjmp implementations that return with a br instead of a ret. You can + disable this behaviour using the ``-mno-bti-at-return-twice`` option. + Floating Point Support in Clang --- diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td index c56578b34641a..41b3ca5a4583e 100644 --- a/clang/include/clang/Driver/Options.td +++ b/clang/include/clang/Driver/Options.td @@ -3414,7 +3414,7 @@ def mmark_bti_property : Flag<["-"], "mmark-bti-property">, def mno_bti_at_return_twice : Flag<["-"], "mno-bti-at-return-twice">, Group, HelpText<"Do not add a BTI instruction after a setjmp or other" - " return-twice construct (Arm only)">; + " return-twice construct (Arm/AArch64 only)">; foreach i = {1-31} in def ffixed_x#i : Flag<["-"], "ffixed-x"#i>, Group, diff --git a/clang/lib/Driver/ToolChains/Arch/AArch64.cpp b/clang/lib/Driver/ToolChains/Arch/AArch64.cpp index f9557bac5fcdc..610c672feb677 100644 --- a/clang/lib/Driver/ToolChains/Arch/AArch64.cpp +++ b/clang/lib/Driver/ToolChains/Arch/AArch64.cpp @@ -588,4 +588,7 @@ void aarch64::getAArch64TargetFeatures(const Driver &D, // Enabled A53 errata (835769) workaround by default on android Features.push_back("+fix-cortex-a53-835769"); } + + if (Args.getLastArg(options::OPT_mno_bti_at_return_twice)) +Features.push_back("+no-bti-at-return-twice"); } diff --git a/llvm/lib/Target/AArch64/AArch64.td b/llvm/lib/Target/AArch64/AArch64.td index 84d388d94e596..82161b162ecdf 100644 --- a/llvm/lib/Target/AArch64/AArch64.td +++ b/llvm/lib/Target/AArch64/AArch64.td @@ -466,6 +466,11 @@ def FeatureEL3 : SubtargetFeature<"el3", "HasEL3", "true", def FeatureFixCortexA53_835769 : SubtargetFeature<"fix-cortex-a53-835769", "FixCortexA53_835769", "true", "Mitigate Cortex-A53 Erratum 835769">; +def FeatureNoBTIAtReturnTwice : SubtargetFeature<"no-bti-at-return-twice", + "NoBTIAtReturnTwice", "true", + "Don't place a BTI instruction " + "after a return-twice">; + //===--===// // Architectures. // diff --git a/llvm/lib/Target/AArch64/AArch64ExpandPseudoInsts.
[clang] 55b6a31 - [llvm][AArch64] Generate getExtensionFeatures from the list of extensions
Author: David Spickett Date: 2022-04-11T13:42:24Z New Revision: 55b6a3186cfa8b85a1defe05861d73f447e4c990 URL: https://github.com/llvm/llvm-project/commit/55b6a3186cfa8b85a1defe05861d73f447e4c990 DIFF: https://github.com/llvm/llvm-project/commit/55b6a3186cfa8b85a1defe05861d73f447e4c990.diff LOG: [llvm][AArch64] Generate getExtensionFeatures from the list of extensions This takes the AARCH64_ARCH_EXT_NAME in AArch64TargetParser.def and uses it to generate all the "if bit is set add this feature name" code. Which gives us a bunch that we were missing. I've updated testing to include those and reordered them to match the order in the .def. The final part of the test will catch any missing extensions if we somehow manage to not generate an if block for them. This has changed the order of cc1's "-target-feature" output so I've updated some tests in clang to reflect that. Reviewed By: tmatheson Differential Revision: https://reviews.llvm.org/D123296 Added: Modified: clang/test/Preprocessor/aarch64-target-features.c llvm/lib/Support/AArch64TargetParser.cpp llvm/unittests/Support/TargetParserTest.cpp Removed: diff --git a/clang/test/Preprocessor/aarch64-target-features.c b/clang/test/Preprocessor/aarch64-target-features.c index ff7e06284a072..f82e89f4fc78d 100644 --- a/clang/test/Preprocessor/aarch64-target-features.c +++ b/clang/test/Preprocessor/aarch64-target-features.c @@ -274,30 +274,30 @@ // RUN: %clang -target aarch64 -mcpu=thunderx2t99 -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-MCPU-THUNDERX2T99 %s // RUN: %clang -target aarch64 -mcpu=a64fx -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-MCPU-A64FX %s // RUN: %clang -target aarch64 -mcpu=carmel -### -c %s 2>&1 | FileCheck -check-prefix=CHECK-MCPU-CARMEL %s -// CHECK-MCPU-APPLE-A7: "-cc1"{{.*}} "-triple" "aarch64{{.*}}" "-target-feature" "+v8a" "-target-feature" "+fp-armv8" "-target-feature" "+neon" "-target-feature" "+crypto" "-target-feature" "+zcm" "-target-feature" "+zcz" "-target-feature" "+sha2" "-target-feature" "+aes" -// CHECK-MCPU-APPLE-A10: "-cc1"{{.*}} "-triple" "aarch64{{.*}}" "-target-feature" "+fp-armv8" "-target-feature" "+neon" "-target-feature" "+crc" "-target-feature" "+crypto" "-target-feature" "+rdm" "-target-feature" "+zcm" "-target-feature" "+zcz" "-target-feature" "+sha2" "-target-feature" "+aes" -// CHECK-MCPU-APPLE-A11: "-cc1"{{.*}} "-triple" "aarch64{{.*}}" "-target-feature" "+v8.2a" "-target-feature" "+fp-armv8" "-target-feature" "+neon" "-target-feature" "+crc" "-target-feature" "+crypto" "-target-feature" "+fullfp16" "-target-feature" "+ras" "-target-feature" "+lse" "-target-feature" "+rdm" "-target-feature" "+zcm" "-target-feature" "+zcz" "-target-feature" "+sha2" "-target-feature" "+aes" -// CHECK-MCPU-APPLE-A12: "-cc1"{{.*}} "-triple" "aarch64{{.*}}" "-target-feature" "+v8.3a" "-target-feature" "+fp-armv8" "-target-feature" "+neon" "-target-feature" "+crc" "-target-feature" "+crypto" "-target-feature" "+fullfp16" "-target-feature" "+ras" "-target-feature" "+lse" "-target-feature" "+rdm" "-target-feature" "+rcpc" "-target-feature" "+zcm" "-target-feature" "+zcz" "-target-feature" "+sha2" "-target-feature" "+aes" -// CHECK-MCPU-A34: "-cc1"{{.*}} "-triple" "aarch64{{.*}}" "-target-feature" "+neon" "-target-feature" "+crc" -// CHECK-MCPU-APPLE-A13: "-cc1"{{.*}} "-triple" "aarch64{{.*}}" "-target-feature" "+v8.4a" "-target-feature" "+fp-armv8" "-target-feature" "+neon" "-target-feature" "+crc" "-target-feature" "+crypto" "-target-feature" "+dotprod" "-target-feature" "+fullfp16" "-target-feature" "+ras" "-target-feature" "+lse" "-target-feature" "+rdm" "-target-feature" "+rcpc" "-target-feature" "+zcm" "-target-feature" "+zcz" "-target-feature" "+fp16fml" "-target-feature" "+sm4" "-target-feature" "+sha3" "-target-feature" "+sha2" "-target-feature" "+aes" -// CHECK-MCPU-A35: "-cc1"{{.*}} "-triple" "aarch64{{.*}}" "-target-feature" "+v8a" "-target-feature" "+fp-armv8" "-target-feature" "+neon" "-target-feature" "+crc" "-target-feature" "+crypto" -// CHECK-MCPU-A53: "-cc1"{{.*}} "-triple" "aarch64{{.*}}" "-target-feature" "+v8a" "-target-feature" "+fp-armv8" "-target-feature" "+neon" "-target-feature" "+crc" "-target-feature" "+crypto" -// CHECK-MCPU-A57: "-cc1"{{.*}} "-triple" "aarch64{{.*}}" "-target-feature" "+v8a" "-target-feature" "+fp-armv8" "-target-feature" "+neon" "-target-feature" "+crc" "-target-feature" "+crypto" -// CHECK-MCPU-A72: "-cc1"{{.*}} "-triple" "aarch64{{.*}}" "-target-feature" "+v8a" "-target-feature" "+fp-armv8" "-target-feature" "+neon" "-target-feature" "+crc" "-target-feature" "+crypto" -// CHECK-MCPU-CORTEX-A73: "-cc1"{{.*}} "-triple" "aarch64{{.*}}" "-target-feature" "+v8a" "-target-feature" "+fp-armv8" "-target-feature" "+neon" "-target-feature" "+crc" "-target-feature" "+crypto" -// CHECK-MCPU-CORTEX-R82: "-cc1"{{.*}} "-triple" "aarch
[clang] 218b5c8 - [clang][AArch64] Remove BTI after setjmp from release notes
Author: David Spickett Date: 2022-04-19T13:49:55Z New Revision: 218b5c83940d469424564ba6f1ec488c4970a5e5 URL: https://github.com/llvm/llvm-project/commit/218b5c83940d469424564ba6f1ec488c4970a5e5 DIFF: https://github.com/llvm/llvm-project/commit/218b5c83940d469424564ba6f1ec488c4970a5e5.diff LOG: [clang][AArch64] Remove BTI after setjmp from release notes This is now going into 14.0.2 as 571c7d8f6dae1a8797ae3271c0c09fc648b1940b so will not be new in clang-15. Added: Modified: clang/docs/ReleaseNotes.rst Removed: diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index ef4211204a905..4a0e2ff6170a2 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -298,11 +298,6 @@ DWARF Support in Clang Arm and AArch64 Support in Clang -- When using ``-mbranch-protection=bti`` with AArch64, calls to setjmp will - now be followed by a BTI instruction. This is done to be compatible with - setjmp implementations that return with a br instead of a ret. You can - disable this behaviour using the ``-mno-bti-at-return-twice`` option. - Floating Point Support in Clang --- ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] d15e7dd - [clang][Hexagon] Match -lc option more specifically in toolchain test
Author: David Spickett Date: 2022-02-03T10:08:20Z New Revision: d15e7dd1238df20e9c09cc91930f716e0d3d5b05 URL: https://github.com/llvm/llvm-project/commit/d15e7dd1238df20e9c09cc91930f716e0d3d5b05 DIFF: https://github.com/llvm/llvm-project/commit/d15e7dd1238df20e9c09cc91930f716e0d3d5b05.diff LOG: [clang][Hexagon] Match -lc option more specifically in toolchain test In https://lab.llvm.org/buildbot/#/builders/185/builds/1341 our bot happened to generate a temporary file path with -lc in it. Match with "" around the options so this doesn't happen again. Added: Modified: clang/test/Driver/hexagon-toolchain-linux.c Removed: diff --git a/clang/test/Driver/hexagon-toolchain-linux.c b/clang/test/Driver/hexagon-toolchain-linux.c index da59590371b90..05ae1733992d9 100644 --- a/clang/test/Driver/hexagon-toolchain-linux.c +++ b/clang/test/Driver/hexagon-toolchain-linux.c @@ -39,8 +39,8 @@ // CHECK002: "-dynamic-linker={{/|}}lib{{/|}}ld-musl-hexagon.so.1" // CHECK002-NOT: {{.*}}basic_linux_libcxx_tree{{/|}}usr{{/|}}lib{{/|}}crti.o // CHECK002-NOT: {{.*}}basic_linux_libcxx_tree{{/|}}usr{{/|}}lib{{/|}}crt1.o -// CHECK002-NOT: -lclang_rt.builtins-hexagon -// CHECK002-NOT: -lc +// CHECK002-NOT: "-lclang_rt.builtins-hexagon" +// CHECK002-NOT: "-lc" // - // Passing --musl -nostartfiles // - @@ -65,8 +65,8 @@ // RUN: | FileCheck -check-prefix=CHECK004 %s // CHECK004: "-dynamic-linker={{/|}}lib{{/|}}ld-musl-hexagon.so.1" // CHECK004: "{{.*}}basic_linux_libcxx_tree{{/|}}usr{{/|}}lib{{/|}}crt1.o" -// CHECK004-NOT: -lclang_rt.builtins-hexagon -// CHECK004-NOT: -lc +// CHECK004-NOT: "-lclang_rt.builtins-hexagon" +// CHECK004-NOT: "-lc" // - // Not Passing -fno-use-init-array when musl is selected // - ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [flang] [lld] [llvm] [flang][clang] Add Visibility specific help text for options (PR #81869)
@@ -191,6 +191,60 @@ static MarshallingInfo createMarshallingInfo(const Record &R) { return Ret; } +static void EmitHelpTextsForVariants( +raw_ostream &OS, std::vector, StringRef>> + HelpTextsForVariants) { + // OptTable must be constexpr so it uses std::arrays with these capacities. + const unsigned MaxVisibilityPerHelp = 2; + const unsigned MaxVisibilityHelp = 1; + + // This function must initialise any unused elements of those arrays. + for (auto [Visibilities, _] : HelpTextsForVariants) +while (Visibilities.size() < MaxVisibilityPerHelp) + Visibilities.push_back("0"); + + while (HelpTextsForVariants.size() < MaxVisibilityHelp) +HelpTextsForVariants.push_back( +{std::vector(MaxVisibilityPerHelp, "0"), ""}); + + OS << ", (std::array, const char*>, " << MaxVisibilityHelp << ">{{ "; + + assert(HelpTextsForVariants.size() <= MaxVisibilityHelp && + "Too many help text variants to store in " + "OptTable::HelpTextsForVariants"); + + for (auto VisibilityHelp = HelpTextsForVariants.cbegin(); + VisibilityHelp != HelpTextsForVariants.cend(); ++VisibilityHelp) { +auto [Visibilities, Help] = *VisibilityHelp; + +assert(Visibilities.size() <= MaxVisibilityPerHelp && + "Too many visibilities to store in an " + "OptTable::HelpTextsForVariants entry"); +OS << "std::make_pair(std::array{{"; + +for (auto Visibility = Visibilities.cbegin(); + Visibility != Visibilities.cend(); ++Visibility) { + OS << *Visibility; + if (std::next(Visibility) != Visibilities.cend()) +OS << ", "; +} + +OS << "}}, "; + +if (Help.size()) + OS << "\"" << Help << "\""; DavidSpickett wrote: No! Good spot, `HelpText` uses `write_cstring` which does so I'll use that. https://github.com/llvm/llvm-project/pull/81869 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang][Driver] Add -masm option to flang (PR #81490)
https://github.com/DavidSpickett created https://github.com/llvm/llvm-project/pull/81490 The motivation here was a suggestion over in Compiler Explorer. You can use `-mllvm` already to do this but since gfortran supports `-masm`, I figured I'd try to add it. This is done by flang expanding `-masm` into `-mllvm x86-asm-syntax=`, then passing that to fc1. Which then collects all the `-mllvm` options and forwards them on. The code to expand it comes from clang `Clang::AddX86TargetArgs` (there are some other places doing the same thing too). However I've removed the `-inline-asm` that clang adds, as fortran doesn't have inline assembly. So -masm for flang purely changes the style of assembly output. The test is adapted from `clang/test/Driver/masm.c` by removing the clang-cl related lines and changing the 32 bit triples to 64 bit triples since flang doesn't support 32 bit targets. >From 6cc1f2073033c13fa45f888553ea3b3c384dd508 Mon Sep 17 00:00:00 2001 From: David Spickett Date: Mon, 12 Feb 2024 14:56:33 + Subject: [PATCH] [flang][Driver] Add -masm option to flang The motivation here was a suggestion over in Compiler Explorer. You can use `-mllvm` already to do this but since gfortran supports `-masm`, I figured I'd try to add it. This is done by flang expanding `-masm` into `-mllvm x86-asm-syntax=`, then passing that to fc1. Which then collects all the `-mllvm` options and forwards them on. The code to expand it comes from clang `Clang::AddX86TargetArgs` (there are some other places doing the same thing too). However I've removed the `-inline-asm` that clang adds, as fortran doesn't have inline assembly. So -masm for flang purely changes the style of assembly output. The test is adapted from `clang/test/Driver/masm.c` by removing the clang-cl related lines and changing the 32 bit triples to 64 bit triples since flang doesn't support 32 bit targets. --- clang/include/clang/Driver/Options.td | 2 +- clang/lib/Driver/ToolChains/Flang.cpp | 15 +++ clang/lib/Driver/ToolChains/Flang.h | 7 +++ flang/test/Driver/masm.f90| 13 + 4 files changed, 36 insertions(+), 1 deletion(-) create mode 100644 flang/test/Driver/masm.f90 diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td index 31e8571758bfce..d5017b901d2906 100644 --- a/clang/include/clang/Driver/Options.td +++ b/clang/include/clang/Driver/Options.td @@ -4414,7 +4414,7 @@ def mwatchsimulator_version_min_EQ : Joined<["-"], "mwatchsimulator-version-min= def march_EQ : Joined<["-"], "march=">, Group, Flags<[TargetSpecific]>, Visibility<[ClangOption, CLOption, DXCOption, FlangOption]>, HelpText<"For a list of available architectures for the target use '-mcpu=help'">; -def masm_EQ : Joined<["-"], "masm=">, Group; +def masm_EQ : Joined<["-"], "masm=">, Group, Visibility<[ClangOption, FlangOption]>; def inline_asm_EQ : Joined<["-"], "inline-asm=">, Group, Visibility<[ClangOption, CC1Option]>, Values<"att,intel">, diff --git a/clang/lib/Driver/ToolChains/Flang.cpp b/clang/lib/Driver/ToolChains/Flang.cpp index 23da08aa593f2d..6168b42dc78292 100644 --- a/clang/lib/Driver/ToolChains/Flang.cpp +++ b/clang/lib/Driver/ToolChains/Flang.cpp @@ -249,6 +249,20 @@ void Flang::AddRISCVTargetArgs(const ArgList &Args, } } +void Flang::AddX86_64TargetArgs(const ArgList &Args, +ArgStringList &CmdArgs) const { + if (Arg *A = Args.getLastArg(options::OPT_masm_EQ)) { +StringRef Value = A->getValue(); +if (Value == "intel" || Value == "att") { + CmdArgs.push_back(Args.MakeArgString("-mllvm")); + CmdArgs.push_back(Args.MakeArgString("-x86-asm-syntax=" + Value)); +} else { + getToolChain().getDriver().Diag(diag::err_drv_unsupported_option_argument) + << A->getSpelling() << Value; +} + } +} + static void addVSDefines(const ToolChain &TC, const ArgList &Args, ArgStringList &CmdArgs) { @@ -374,6 +388,7 @@ void Flang::addTargetOptions(const ArgList &Args, break; case llvm::Triple::x86_64: getTargetFeatures(D, Triple, Args, CmdArgs, /*ForAs*/ false); +AddX86_64TargetArgs(Args, CmdArgs); break; } diff --git a/clang/lib/Driver/ToolChains/Flang.h b/clang/lib/Driver/ToolChains/Flang.h index ec2e545a1d0b5c..9f5e26b8608324 100644 --- a/clang/lib/Driver/ToolChains/Flang.h +++ b/clang/lib/Driver/ToolChains/Flang.h @@ -77,6 +77,13 @@ class LLVM_LIBRARY_VISIBILITY Flang : public Tool { void AddRISCVTargetArgs(const llvm::opt::ArgList &Args, llvm::opt::ArgStringList &CmdArgs) const; + /// Add specific options for X86_64 target. + /// + /// \param [in] Args The list of input driver arguments + /// \param [out] CmdArgs The list of output command arguments + void AddX86_64TargetArgs(const llvm::opt::ArgList &Args, + llvm::opt::ArgStringList &CmdArgs) const; + /// Extract offload opti
[clang] [flang] [flang][Driver] Add -masm option to flang (PR #81490)
https://github.com/DavidSpickett edited https://github.com/llvm/llvm-project/pull/81490 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang][Driver] Add -masm option to flang (PR #81490)
https://github.com/DavidSpickett edited https://github.com/llvm/llvm-project/pull/81490 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang][Driver] Add -masm option to flang (PR #81490)
https://github.com/DavidSpickett updated https://github.com/llvm/llvm-project/pull/81490 >From 6cc1f2073033c13fa45f888553ea3b3c384dd508 Mon Sep 17 00:00:00 2001 From: David Spickett Date: Mon, 12 Feb 2024 14:56:33 + Subject: [PATCH 1/2] [flang][Driver] Add -masm option to flang The motivation here was a suggestion over in Compiler Explorer. You can use `-mllvm` already to do this but since gfortran supports `-masm`, I figured I'd try to add it. This is done by flang expanding `-masm` into `-mllvm x86-asm-syntax=`, then passing that to fc1. Which then collects all the `-mllvm` options and forwards them on. The code to expand it comes from clang `Clang::AddX86TargetArgs` (there are some other places doing the same thing too). However I've removed the `-inline-asm` that clang adds, as fortran doesn't have inline assembly. So -masm for flang purely changes the style of assembly output. The test is adapted from `clang/test/Driver/masm.c` by removing the clang-cl related lines and changing the 32 bit triples to 64 bit triples since flang doesn't support 32 bit targets. --- clang/include/clang/Driver/Options.td | 2 +- clang/lib/Driver/ToolChains/Flang.cpp | 15 +++ clang/lib/Driver/ToolChains/Flang.h | 7 +++ flang/test/Driver/masm.f90| 13 + 4 files changed, 36 insertions(+), 1 deletion(-) create mode 100644 flang/test/Driver/masm.f90 diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td index 31e8571758bfce..d5017b901d2906 100644 --- a/clang/include/clang/Driver/Options.td +++ b/clang/include/clang/Driver/Options.td @@ -4414,7 +4414,7 @@ def mwatchsimulator_version_min_EQ : Joined<["-"], "mwatchsimulator-version-min= def march_EQ : Joined<["-"], "march=">, Group, Flags<[TargetSpecific]>, Visibility<[ClangOption, CLOption, DXCOption, FlangOption]>, HelpText<"For a list of available architectures for the target use '-mcpu=help'">; -def masm_EQ : Joined<["-"], "masm=">, Group; +def masm_EQ : Joined<["-"], "masm=">, Group, Visibility<[ClangOption, FlangOption]>; def inline_asm_EQ : Joined<["-"], "inline-asm=">, Group, Visibility<[ClangOption, CC1Option]>, Values<"att,intel">, diff --git a/clang/lib/Driver/ToolChains/Flang.cpp b/clang/lib/Driver/ToolChains/Flang.cpp index 23da08aa593f2d..6168b42dc78292 100644 --- a/clang/lib/Driver/ToolChains/Flang.cpp +++ b/clang/lib/Driver/ToolChains/Flang.cpp @@ -249,6 +249,20 @@ void Flang::AddRISCVTargetArgs(const ArgList &Args, } } +void Flang::AddX86_64TargetArgs(const ArgList &Args, +ArgStringList &CmdArgs) const { + if (Arg *A = Args.getLastArg(options::OPT_masm_EQ)) { +StringRef Value = A->getValue(); +if (Value == "intel" || Value == "att") { + CmdArgs.push_back(Args.MakeArgString("-mllvm")); + CmdArgs.push_back(Args.MakeArgString("-x86-asm-syntax=" + Value)); +} else { + getToolChain().getDriver().Diag(diag::err_drv_unsupported_option_argument) + << A->getSpelling() << Value; +} + } +} + static void addVSDefines(const ToolChain &TC, const ArgList &Args, ArgStringList &CmdArgs) { @@ -374,6 +388,7 @@ void Flang::addTargetOptions(const ArgList &Args, break; case llvm::Triple::x86_64: getTargetFeatures(D, Triple, Args, CmdArgs, /*ForAs*/ false); +AddX86_64TargetArgs(Args, CmdArgs); break; } diff --git a/clang/lib/Driver/ToolChains/Flang.h b/clang/lib/Driver/ToolChains/Flang.h index ec2e545a1d0b5c..9f5e26b8608324 100644 --- a/clang/lib/Driver/ToolChains/Flang.h +++ b/clang/lib/Driver/ToolChains/Flang.h @@ -77,6 +77,13 @@ class LLVM_LIBRARY_VISIBILITY Flang : public Tool { void AddRISCVTargetArgs(const llvm::opt::ArgList &Args, llvm::opt::ArgStringList &CmdArgs) const; + /// Add specific options for X86_64 target. + /// + /// \param [in] Args The list of input driver arguments + /// \param [out] CmdArgs The list of output command arguments + void AddX86_64TargetArgs(const llvm::opt::ArgList &Args, + llvm::opt::ArgStringList &CmdArgs) const; + /// Extract offload options from the driver arguments and add them to /// the command arguments. /// \param [in] C The current compilation for the driver invocation diff --git a/flang/test/Driver/masm.f90 b/flang/test/Driver/masm.f90 new file mode 100644 index 00..a119f02787eca9 --- /dev/null +++ b/flang/test/Driver/masm.f90 @@ -0,0 +1,13 @@ +// RUN: %flang -target x86_64-unknown-linux -masm=intel -S %s -### 2>&1 | FileCheck --check-prefix=CHECK-INTEL %s +// RUN: %flang -target x86_64-unknown-linux -masm=att -S %s -### 2>&1 | FileCheck --check-prefix=CHECK-ATT %s +// RUN: not %flang --target=x86_64-unknown-linux -S -masm=somerequired %s -### 2>&1 | FileCheck --check-prefix=CHECK-SOMEREQUIRED %s +// RUN: %flang -target aarch64-unknown-eabi -S -masm=intel %s -### 2>&1 | FileCheck --check-prefix=CHECK-AA
[clang] [flang] [flang][Driver] Add -masm option to flang (PR #81490)
@@ -0,0 +1,13 @@ +// RUN: %flang -target x86_64-unknown-linux -masm=intel -S %s -### 2>&1 | FileCheck --check-prefix=CHECK-INTEL %s DavidSpickett wrote: 1. Done, I forgot this wasn't actually parsing the file. So I've removed the function as well, it's not needed. 2. Don't need requires if all we're doing is using the triple name, I checked that by building without x86 and the test still works. 3. Used --target= for the run lines. https://github.com/llvm/llvm-project/pull/81490 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [flang][Driver] Add -masm option to flang (PR #81490)
https://github.com/DavidSpickett closed https://github.com/llvm/llvm-project/pull/81490 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] 381a00d - [clang][Driver][HLSL] Fix formatting of clang-dxc options group title
Author: David Spickett Date: 2024-02-13T16:05:03Z New Revision: 381a00de4fdcccd904dac6a0856fb44f12ba0abb URL: https://github.com/llvm/llvm-project/commit/381a00de4fdcccd904dac6a0856fb44f12ba0abb DIFF: https://github.com/llvm/llvm-project/commit/381a00de4fdcccd904dac6a0856fb44f12ba0abb.diff LOG: [clang][Driver][HLSL] Fix formatting of clang-dxc options group title Some extra `<>` and a missing full stop. Added: Modified: clang/include/clang/Driver/Options.td Removed: diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td index d5017b901d2906..187b845ddf3c7b 100644 --- a/clang/include/clang/Driver/Options.td +++ b/clang/include/clang/Driver/Options.td @@ -8466,8 +8466,8 @@ def _SLASH_ZW : CLJoined<"ZW">; // clang-dxc Options //===--===// -def dxc_Group : OptionGroup<"">, Visibility<[DXCOption]>, - HelpText<"dxc compatibility options">; +def dxc_Group : OptionGroup<"clang-dxc options">, Visibility<[DXCOption]>, + HelpText<"dxc compatibility options.">; class DXCFlag : Option<["/", "-"], name, KIND_FLAG>, Group, Visibility<[DXCOption]>; class DXCJoinedOrSeparate : Option<["/", "-"], name, ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] 1d84792 - [clang][Driver] Small correction to print-runtime-dir
Author: David Spickett Date: 2024-02-13T16:14:03Z New Revision: 1d8479225a8c1efc8c90511e6c7fe608ff38163c URL: https://github.com/llvm/llvm-project/commit/1d8479225a8c1efc8c90511e6c7fe608ff38163c DIFF: https://github.com/llvm/llvm-project/commit/1d8479225a8c1efc8c90511e6c7fe608ff38163c.diff LOG: [clang][Driver] Small correction to print-runtime-dir Added: Modified: clang/include/clang/Driver/Options.td Removed: diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td index 187b845ddf3c7b..c625d0dd1c0c72 100644 --- a/clang/include/clang/Driver/Options.td +++ b/clang/include/clang/Driver/Options.td @@ -5339,7 +5339,7 @@ def print_rocm_search_dirs : Flag<["-", "--"], "print-rocm-search-dirs">, HelpText<"Print the paths used for finding ROCm installation">, Visibility<[ClangOption, CLOption]>; def print_runtime_dir : Flag<["-", "--"], "print-runtime-dir">, - HelpText<"Print the directory pathname containing clangs runtime libraries">, + HelpText<"Print the directory pathname containing Clang's runtime libraries">, Visibility<[ClangOption, CLOption]>; def print_diagnostic_options : Flag<["-", "--"], "print-diagnostic-options">, HelpText<"Print all of Clang's warning options">, ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] 7a5c1a4 - [clang][docs] Fix warning in LanguageExtensions
Author: David Spickett Date: 2024-02-13T16:39:59Z New Revision: 7a5c1a4abc750fef335c2ee5191d59ebe9e4bf18 URL: https://github.com/llvm/llvm-project/commit/7a5c1a4abc750fef335c2ee5191d59ebe9e4bf18 DIFF: https://github.com/llvm/llvm-project/commit/7a5c1a4abc750fef335c2ee5191d59ebe9e4bf18.diff LOG: [clang][docs] Fix warning in LanguageExtensions build-llvm/tools/clang/docs/LanguageExtensions.rst:2768: WARNING: Title underline too short. Added: Modified: clang/docs/LanguageExtensions.rst Removed: diff --git a/clang/docs/LanguageExtensions.rst b/clang/docs/LanguageExtensions.rst index ca78a5c39cf73..ee1d25396ca86 100644 --- a/clang/docs/LanguageExtensions.rst +++ b/clang/docs/LanguageExtensions.rst @@ -2765,7 +2765,7 @@ that even if present, its use may depend on run-time privilege or other OS controlled state. ``__builtin_readsteadycounter`` --- +--- ``__builtin_readsteadycounter`` is used to access the fixed frequency counter register (or a similar steady-rate clock) on those targets that support it. ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] Add support for Arm Cortex A78AE CPU (PR #84485)
DavidSpickett wrote: Should this be added to the release notes? (so it doesn't get forgotten in a mad scramble in a few months time) https://github.com/llvm/llvm-project/pull/84485 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] Add support for Arm Cortex A78AE CPU (PR #84485)
DavidSpickett wrote: Also does this address the -mcpu=native part of https://github.com/llvm/llvm-project/issues/84450 as well? https://github.com/llvm/llvm-project/pull/84485 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] Add support for Arm Cortex A78AE CPU (PR #84485)
DavidSpickett wrote: > I've added a host id (0xd42) to llvm/lib/TargetParser/Host.cpp, so I think so > (unless there's something else required). Right, of course, somehow I skipped right over that. https://github.com/llvm/llvm-project/pull/84485 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] Add support for Arm Cortex A78AE CPU (PR #84485)
https://github.com/DavidSpickett approved this pull request. Looks good to me. https://github.com/llvm/llvm-project/pull/84485 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [flang] [lld] [llvm] [flang][clang] Add Visibility specific help text for options (PR #81869)
@@ -3382,10 +3382,19 @@ def fopenmp : Flag<["-"], "fopenmp">, Group, HelpText<"Parse OpenMP pragmas and generate parallel code.">; def fno_openmp : Flag<["-"], "fno-openmp">, Group, Flags<[NoArgumentUnused]>; +class OpenMPVersionHelp { + string str = !strconcat( +"Set OpenMP version (e.g. 45 for OpenMP 4.5, 51 for OpenMP 5.1). Default value is ", +default, " for ", program); +} def fopenmp_version_EQ : Joined<["-"], "fopenmp-version=">, Group, Flags<[NoArgumentUnused]>, Visibility<[ClangOption, CC1Option, FlangOption, FC1Option]>, - HelpText<"Set OpenMP version (e.g. 45 for OpenMP 4.5, 51 for OpenMP 5.1). Default value is 51 for Clang">; + HelpText.str>, + HelpTextForVisibilities<[ +HelpTextForVisibility.str>, +HelpTextForVisibility.str>, DavidSpickett wrote: Sure, I can make it `array, char*>>`. https://github.com/llvm/llvm-project/pull/81869 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [flang] [lld] [llvm] [flang][clang] Add Visibility specific help text for options (PR #81869)
https://github.com/DavidSpickett updated https://github.com/llvm/llvm-project/pull/81869 >From 72bbd9d38cb6e292d92391fcf04154cfbc336192 Mon Sep 17 00:00:00 2001 From: David Spickett Date: Thu, 15 Feb 2024 09:52:22 + Subject: [PATCH 1/5] [flang][clang] Add Visibility specific help text for options And use it to print the correct default OpenMP version for flang. This change adds an optional HelpTextForVisibility to options. This allows you to change the help text (shown in documentation and --help) for one specific visibility. It could be generalised to a list, but we only need to pick out flang right now so I've kept it simple but not so simple that it's "if flang else". For example the OpenMP option will show one default value for Clang and another for Flang. Clang will use the existing HelpText string. I have not applied this to group descriptions just because I don't know of one that needs changing. It could easily be enabled for those too if needed. There are minor changes to them just to get it all to compile. This approach of storing many help strings per option in the 1 driver library seemed perferrable to making a whole new library for Flang (even if that would mostly be including stuff from Clang). --- clang/include/clang/Driver/Options.td | 5 ++- clang/lib/Frontend/CompilerInvocation.cpp | 16 .../utils/TableGen/ClangOptionDocEmitter.cpp | 21 +- flang/test/Driver/driver-help-hidden.f90 | 2 +- flang/test/Driver/driver-help.f90 | 2 +- lld/MachO/DriverUtils.cpp | 20 ++--- lld/MinGW/Driver.cpp | 20 ++--- lld/wasm/Driver.cpp | 20 ++--- llvm/include/llvm/Option/OptParser.td | 10 + llvm/include/llvm/Option/OptTable.h | 41 +-- llvm/lib/Option/OptTable.cpp | 14 --- .../Option/OptionMarshallingTest.cpp | 6 +-- llvm/utils/TableGen/OptParserEmitter.cpp | 16 13 files changed, 143 insertions(+), 50 deletions(-) diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td index 3a028fadb25b18..765d5fbc1e692d 100644 --- a/clang/include/clang/Driver/Options.td +++ b/clang/include/clang/Driver/Options.td @@ -3385,7 +3385,10 @@ def fno_openmp : Flag<["-"], "fno-openmp">, Group, def fopenmp_version_EQ : Joined<["-"], "fopenmp-version=">, Group, Flags<[NoArgumentUnused]>, Visibility<[ClangOption, CC1Option, FlangOption, FC1Option]>, - HelpText<"Set OpenMP version (e.g. 45 for OpenMP 4.5, 51 for OpenMP 5.1). Default value is 51 for Clang">; + HelpText<"Set OpenMP version (e.g. 45 for OpenMP 4.5, 51 for OpenMP 5.1). Default value is 51 for Clang">, + HelpForVisibility>; defm openmp_extensions: BoolFOption<"openmp-extensions", LangOpts<"OpenMPExtensions">, DefaultTrue, PosFlag(R->getValueInit("HelpTextForVisibility"))) { +const Record *VisibilityHelp = R->getValueAsDef("HelpTextForVisibility"); +std::string VisibilityStr = +VisibilityHelp->getValue("Visibility")->getValue()->getAsString(); + +for (StringRef Mask : DocInfo->getValueAsListOfStrings("VisibilityMask")) { + if (VisibilityStr == Mask) { +Description = escapeRST(VisibilityHelp->getValueAsString("Text")); +break; + } +} + } + + // If there's not a program specific string, use the default one. + if (Description.empty()) +Description = getRSTStringWithTextFallback(R, "DocBrief", "HelpText"); if (!isa(R->getValueInit("Values"))) { if (!Description.empty() && Description.back() != '.') diff --git a/flang/test/Driver/driver-help-hidden.f90 b/flang/test/Driver/driver-help-hidden.f90 index 44dbac44772b29..94f2f4d1f522ec 100644 --- a/flang/test/Driver/driver-help-hidden.f90 +++ b/flang/test/Driver/driver-help-hidden.f90 @@ -83,7 +83,7 @@ ! CHECK-NEXT: -fopenmp-targets= ! CHECK-NEXT: Specify comma-separated list of triples OpenMP offloading targets to be supported ! CHECK-NEXT: -fopenmp-version= -! CHECK-NEXT: Set OpenMP version (e.g. 45 for OpenMP 4.5, 51 for OpenMP 5.1). Default value is 51 for Clang +! CHECK-NEXT: Set OpenMP version (e.g. 45 for OpenMP 4.5, 51 for OpenMP 5.1). Default value is 11 for Flang ! CHECK-NEXT: -fopenmpParse OpenMP pragmas and generate parallel code. ! CHECK-NEXT: -foptimization-record-file= ! CHECK-NEXT: Specify the output name of the file containing the optimization remarks. Implies -fsave-optimization-record. On Darwin platforms, this cannot be used with multiple -arch options. diff --git a/flang/test/Driver/driver-help.f90 b/flang/test/Driver/driver-help.f90 index b4280a454e3128..31bc67e1742482 100644 --- a/flang/test/Driver/driver-help.f90 +++ b/flang/test/Driver/driver-help.f90 @@ -69,7 +69,7 @@ ! HELP-NEXT: -fopenmp-targets= ! HELP-NEXT:
[clang] [clang-tools-extra] [flang] [lld] [llvm] [flang][clang] Add Visibility specific help text for options (PR #81869)
https://github.com/DavidSpickett edited https://github.com/llvm/llvm-project/pull/81869 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [flang] [lld] [llvm] [flang][clang] Add Visibility specific help text for options (PR #81869)
@@ -3382,10 +3382,19 @@ def fopenmp : Flag<["-"], "fopenmp">, Group, HelpText<"Parse OpenMP pragmas and generate parallel code.">; def fno_openmp : Flag<["-"], "fno-openmp">, Group, Flags<[NoArgumentUnused]>; +class OpenMPVersionHelp { + string str = !strconcat( +"Set OpenMP version (e.g. 45 for OpenMP 4.5, 51 for OpenMP 5.1). Default value is ", +default, " for ", program); +} def fopenmp_version_EQ : Joined<["-"], "fopenmp-version=">, Group, Flags<[NoArgumentUnused]>, Visibility<[ClangOption, CC1Option, FlangOption, FC1Option]>, - HelpText<"Set OpenMP version (e.g. 45 for OpenMP 4.5, 51 for OpenMP 5.1). Default value is 51 for Clang">; + HelpText.str>, + HelpTextForVisibilities<[ +HelpTextForVisibility.str>, +HelpTextForVisibility.str>, DavidSpickett wrote: This has been done. https://github.com/llvm/llvm-project/pull/81869 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [flang] [lld] [llvm] [flang][clang] Add Visibility specific help text for options (PR #81869)
https://github.com/DavidSpickett updated https://github.com/llvm/llvm-project/pull/81869 >From 72bbd9d38cb6e292d92391fcf04154cfbc336192 Mon Sep 17 00:00:00 2001 From: David Spickett Date: Thu, 15 Feb 2024 09:52:22 + Subject: [PATCH 1/6] [flang][clang] Add Visibility specific help text for options And use it to print the correct default OpenMP version for flang. This change adds an optional HelpTextForVisibility to options. This allows you to change the help text (shown in documentation and --help) for one specific visibility. It could be generalised to a list, but we only need to pick out flang right now so I've kept it simple but not so simple that it's "if flang else". For example the OpenMP option will show one default value for Clang and another for Flang. Clang will use the existing HelpText string. I have not applied this to group descriptions just because I don't know of one that needs changing. It could easily be enabled for those too if needed. There are minor changes to them just to get it all to compile. This approach of storing many help strings per option in the 1 driver library seemed perferrable to making a whole new library for Flang (even if that would mostly be including stuff from Clang). --- clang/include/clang/Driver/Options.td | 5 ++- clang/lib/Frontend/CompilerInvocation.cpp | 16 .../utils/TableGen/ClangOptionDocEmitter.cpp | 21 +- flang/test/Driver/driver-help-hidden.f90 | 2 +- flang/test/Driver/driver-help.f90 | 2 +- lld/MachO/DriverUtils.cpp | 20 ++--- lld/MinGW/Driver.cpp | 20 ++--- lld/wasm/Driver.cpp | 20 ++--- llvm/include/llvm/Option/OptParser.td | 10 + llvm/include/llvm/Option/OptTable.h | 41 +-- llvm/lib/Option/OptTable.cpp | 14 --- .../Option/OptionMarshallingTest.cpp | 6 +-- llvm/utils/TableGen/OptParserEmitter.cpp | 16 13 files changed, 143 insertions(+), 50 deletions(-) diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td index 3a028fadb25b18..765d5fbc1e692d 100644 --- a/clang/include/clang/Driver/Options.td +++ b/clang/include/clang/Driver/Options.td @@ -3385,7 +3385,10 @@ def fno_openmp : Flag<["-"], "fno-openmp">, Group, def fopenmp_version_EQ : Joined<["-"], "fopenmp-version=">, Group, Flags<[NoArgumentUnused]>, Visibility<[ClangOption, CC1Option, FlangOption, FC1Option]>, - HelpText<"Set OpenMP version (e.g. 45 for OpenMP 4.5, 51 for OpenMP 5.1). Default value is 51 for Clang">; + HelpText<"Set OpenMP version (e.g. 45 for OpenMP 4.5, 51 for OpenMP 5.1). Default value is 51 for Clang">, + HelpForVisibility>; defm openmp_extensions: BoolFOption<"openmp-extensions", LangOpts<"OpenMPExtensions">, DefaultTrue, PosFlag(R->getValueInit("HelpTextForVisibility"))) { +const Record *VisibilityHelp = R->getValueAsDef("HelpTextForVisibility"); +std::string VisibilityStr = +VisibilityHelp->getValue("Visibility")->getValue()->getAsString(); + +for (StringRef Mask : DocInfo->getValueAsListOfStrings("VisibilityMask")) { + if (VisibilityStr == Mask) { +Description = escapeRST(VisibilityHelp->getValueAsString("Text")); +break; + } +} + } + + // If there's not a program specific string, use the default one. + if (Description.empty()) +Description = getRSTStringWithTextFallback(R, "DocBrief", "HelpText"); if (!isa(R->getValueInit("Values"))) { if (!Description.empty() && Description.back() != '.') diff --git a/flang/test/Driver/driver-help-hidden.f90 b/flang/test/Driver/driver-help-hidden.f90 index 44dbac44772b29..94f2f4d1f522ec 100644 --- a/flang/test/Driver/driver-help-hidden.f90 +++ b/flang/test/Driver/driver-help-hidden.f90 @@ -83,7 +83,7 @@ ! CHECK-NEXT: -fopenmp-targets= ! CHECK-NEXT: Specify comma-separated list of triples OpenMP offloading targets to be supported ! CHECK-NEXT: -fopenmp-version= -! CHECK-NEXT: Set OpenMP version (e.g. 45 for OpenMP 4.5, 51 for OpenMP 5.1). Default value is 51 for Clang +! CHECK-NEXT: Set OpenMP version (e.g. 45 for OpenMP 4.5, 51 for OpenMP 5.1). Default value is 11 for Flang ! CHECK-NEXT: -fopenmpParse OpenMP pragmas and generate parallel code. ! CHECK-NEXT: -foptimization-record-file= ! CHECK-NEXT: Specify the output name of the file containing the optimization remarks. Implies -fsave-optimization-record. On Darwin platforms, this cannot be used with multiple -arch options. diff --git a/flang/test/Driver/driver-help.f90 b/flang/test/Driver/driver-help.f90 index b4280a454e3128..31bc67e1742482 100644 --- a/flang/test/Driver/driver-help.f90 +++ b/flang/test/Driver/driver-help.f90 @@ -69,7 +69,7 @@ ! HELP-NEXT: -fopenmp-targets= ! HELP-NEXT:
[clang] [clang-tools-extra] [flang] [lld] [llvm] [flang][clang] Add Visibility specific help text for options (PR #81869)
https://github.com/DavidSpickett updated https://github.com/llvm/llvm-project/pull/81869 >From 72bbd9d38cb6e292d92391fcf04154cfbc336192 Mon Sep 17 00:00:00 2001 From: David Spickett Date: Thu, 15 Feb 2024 09:52:22 + Subject: [PATCH 1/7] [flang][clang] Add Visibility specific help text for options And use it to print the correct default OpenMP version for flang. This change adds an optional HelpTextForVisibility to options. This allows you to change the help text (shown in documentation and --help) for one specific visibility. It could be generalised to a list, but we only need to pick out flang right now so I've kept it simple but not so simple that it's "if flang else". For example the OpenMP option will show one default value for Clang and another for Flang. Clang will use the existing HelpText string. I have not applied this to group descriptions just because I don't know of one that needs changing. It could easily be enabled for those too if needed. There are minor changes to them just to get it all to compile. This approach of storing many help strings per option in the 1 driver library seemed perferrable to making a whole new library for Flang (even if that would mostly be including stuff from Clang). --- clang/include/clang/Driver/Options.td | 5 ++- clang/lib/Frontend/CompilerInvocation.cpp | 16 .../utils/TableGen/ClangOptionDocEmitter.cpp | 21 +- flang/test/Driver/driver-help-hidden.f90 | 2 +- flang/test/Driver/driver-help.f90 | 2 +- lld/MachO/DriverUtils.cpp | 20 ++--- lld/MinGW/Driver.cpp | 20 ++--- lld/wasm/Driver.cpp | 20 ++--- llvm/include/llvm/Option/OptParser.td | 10 + llvm/include/llvm/Option/OptTable.h | 41 +-- llvm/lib/Option/OptTable.cpp | 14 --- .../Option/OptionMarshallingTest.cpp | 6 +-- llvm/utils/TableGen/OptParserEmitter.cpp | 16 13 files changed, 143 insertions(+), 50 deletions(-) diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td index 3a028fadb25b18..765d5fbc1e692d 100644 --- a/clang/include/clang/Driver/Options.td +++ b/clang/include/clang/Driver/Options.td @@ -3385,7 +3385,10 @@ def fno_openmp : Flag<["-"], "fno-openmp">, Group, def fopenmp_version_EQ : Joined<["-"], "fopenmp-version=">, Group, Flags<[NoArgumentUnused]>, Visibility<[ClangOption, CC1Option, FlangOption, FC1Option]>, - HelpText<"Set OpenMP version (e.g. 45 for OpenMP 4.5, 51 for OpenMP 5.1). Default value is 51 for Clang">; + HelpText<"Set OpenMP version (e.g. 45 for OpenMP 4.5, 51 for OpenMP 5.1). Default value is 51 for Clang">, + HelpForVisibility>; defm openmp_extensions: BoolFOption<"openmp-extensions", LangOpts<"OpenMPExtensions">, DefaultTrue, PosFlag(R->getValueInit("HelpTextForVisibility"))) { +const Record *VisibilityHelp = R->getValueAsDef("HelpTextForVisibility"); +std::string VisibilityStr = +VisibilityHelp->getValue("Visibility")->getValue()->getAsString(); + +for (StringRef Mask : DocInfo->getValueAsListOfStrings("VisibilityMask")) { + if (VisibilityStr == Mask) { +Description = escapeRST(VisibilityHelp->getValueAsString("Text")); +break; + } +} + } + + // If there's not a program specific string, use the default one. + if (Description.empty()) +Description = getRSTStringWithTextFallback(R, "DocBrief", "HelpText"); if (!isa(R->getValueInit("Values"))) { if (!Description.empty() && Description.back() != '.') diff --git a/flang/test/Driver/driver-help-hidden.f90 b/flang/test/Driver/driver-help-hidden.f90 index 44dbac44772b29..94f2f4d1f522ec 100644 --- a/flang/test/Driver/driver-help-hidden.f90 +++ b/flang/test/Driver/driver-help-hidden.f90 @@ -83,7 +83,7 @@ ! CHECK-NEXT: -fopenmp-targets= ! CHECK-NEXT: Specify comma-separated list of triples OpenMP offloading targets to be supported ! CHECK-NEXT: -fopenmp-version= -! CHECK-NEXT: Set OpenMP version (e.g. 45 for OpenMP 4.5, 51 for OpenMP 5.1). Default value is 51 for Clang +! CHECK-NEXT: Set OpenMP version (e.g. 45 for OpenMP 4.5, 51 for OpenMP 5.1). Default value is 11 for Flang ! CHECK-NEXT: -fopenmpParse OpenMP pragmas and generate parallel code. ! CHECK-NEXT: -foptimization-record-file= ! CHECK-NEXT: Specify the output name of the file containing the optimization remarks. Implies -fsave-optimization-record. On Darwin platforms, this cannot be used with multiple -arch options. diff --git a/flang/test/Driver/driver-help.f90 b/flang/test/Driver/driver-help.f90 index b4280a454e3128..31bc67e1742482 100644 --- a/flang/test/Driver/driver-help.f90 +++ b/flang/test/Driver/driver-help.f90 @@ -69,7 +69,7 @@ ! HELP-NEXT: -fopenmp-targets= ! HELP-NEXT:
[clang] [clang-tools-extra] [flang] [lld] [llvm] [flang][clang] Add Visibility specific help text for options (PR #81869)
https://github.com/DavidSpickett updated https://github.com/llvm/llvm-project/pull/81869 >From 72bbd9d38cb6e292d92391fcf04154cfbc336192 Mon Sep 17 00:00:00 2001 From: David Spickett Date: Thu, 15 Feb 2024 09:52:22 + Subject: [PATCH 1/8] [flang][clang] Add Visibility specific help text for options And use it to print the correct default OpenMP version for flang. This change adds an optional HelpTextForVisibility to options. This allows you to change the help text (shown in documentation and --help) for one specific visibility. It could be generalised to a list, but we only need to pick out flang right now so I've kept it simple but not so simple that it's "if flang else". For example the OpenMP option will show one default value for Clang and another for Flang. Clang will use the existing HelpText string. I have not applied this to group descriptions just because I don't know of one that needs changing. It could easily be enabled for those too if needed. There are minor changes to them just to get it all to compile. This approach of storing many help strings per option in the 1 driver library seemed perferrable to making a whole new library for Flang (even if that would mostly be including stuff from Clang). --- clang/include/clang/Driver/Options.td | 5 ++- clang/lib/Frontend/CompilerInvocation.cpp | 16 .../utils/TableGen/ClangOptionDocEmitter.cpp | 21 +- flang/test/Driver/driver-help-hidden.f90 | 2 +- flang/test/Driver/driver-help.f90 | 2 +- lld/MachO/DriverUtils.cpp | 20 ++--- lld/MinGW/Driver.cpp | 20 ++--- lld/wasm/Driver.cpp | 20 ++--- llvm/include/llvm/Option/OptParser.td | 10 + llvm/include/llvm/Option/OptTable.h | 41 +-- llvm/lib/Option/OptTable.cpp | 14 --- .../Option/OptionMarshallingTest.cpp | 6 +-- llvm/utils/TableGen/OptParserEmitter.cpp | 16 13 files changed, 143 insertions(+), 50 deletions(-) diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td index 3a028fadb25b18..765d5fbc1e692d 100644 --- a/clang/include/clang/Driver/Options.td +++ b/clang/include/clang/Driver/Options.td @@ -3385,7 +3385,10 @@ def fno_openmp : Flag<["-"], "fno-openmp">, Group, def fopenmp_version_EQ : Joined<["-"], "fopenmp-version=">, Group, Flags<[NoArgumentUnused]>, Visibility<[ClangOption, CC1Option, FlangOption, FC1Option]>, - HelpText<"Set OpenMP version (e.g. 45 for OpenMP 4.5, 51 for OpenMP 5.1). Default value is 51 for Clang">; + HelpText<"Set OpenMP version (e.g. 45 for OpenMP 4.5, 51 for OpenMP 5.1). Default value is 51 for Clang">, + HelpForVisibility>; defm openmp_extensions: BoolFOption<"openmp-extensions", LangOpts<"OpenMPExtensions">, DefaultTrue, PosFlag(R->getValueInit("HelpTextForVisibility"))) { +const Record *VisibilityHelp = R->getValueAsDef("HelpTextForVisibility"); +std::string VisibilityStr = +VisibilityHelp->getValue("Visibility")->getValue()->getAsString(); + +for (StringRef Mask : DocInfo->getValueAsListOfStrings("VisibilityMask")) { + if (VisibilityStr == Mask) { +Description = escapeRST(VisibilityHelp->getValueAsString("Text")); +break; + } +} + } + + // If there's not a program specific string, use the default one. + if (Description.empty()) +Description = getRSTStringWithTextFallback(R, "DocBrief", "HelpText"); if (!isa(R->getValueInit("Values"))) { if (!Description.empty() && Description.back() != '.') diff --git a/flang/test/Driver/driver-help-hidden.f90 b/flang/test/Driver/driver-help-hidden.f90 index 44dbac44772b29..94f2f4d1f522ec 100644 --- a/flang/test/Driver/driver-help-hidden.f90 +++ b/flang/test/Driver/driver-help-hidden.f90 @@ -83,7 +83,7 @@ ! CHECK-NEXT: -fopenmp-targets= ! CHECK-NEXT: Specify comma-separated list of triples OpenMP offloading targets to be supported ! CHECK-NEXT: -fopenmp-version= -! CHECK-NEXT: Set OpenMP version (e.g. 45 for OpenMP 4.5, 51 for OpenMP 5.1). Default value is 51 for Clang +! CHECK-NEXT: Set OpenMP version (e.g. 45 for OpenMP 4.5, 51 for OpenMP 5.1). Default value is 11 for Flang ! CHECK-NEXT: -fopenmpParse OpenMP pragmas and generate parallel code. ! CHECK-NEXT: -foptimization-record-file= ! CHECK-NEXT: Specify the output name of the file containing the optimization remarks. Implies -fsave-optimization-record. On Darwin platforms, this cannot be used with multiple -arch options. diff --git a/flang/test/Driver/driver-help.f90 b/flang/test/Driver/driver-help.f90 index b4280a454e3128..31bc67e1742482 100644 --- a/flang/test/Driver/driver-help.f90 +++ b/flang/test/Driver/driver-help.f90 @@ -69,7 +69,7 @@ ! HELP-NEXT: -fopenmp-targets= ! HELP-NEXT:
[clang] [clang-tools-extra] [flang] [lld] [llvm] [flang][clang] Add Visibility specific help text for options (PR #81869)
https://github.com/DavidSpickett updated https://github.com/llvm/llvm-project/pull/81869 >From 72bbd9d38cb6e292d92391fcf04154cfbc336192 Mon Sep 17 00:00:00 2001 From: David Spickett Date: Thu, 15 Feb 2024 09:52:22 + Subject: [PATCH 1/9] [flang][clang] Add Visibility specific help text for options And use it to print the correct default OpenMP version for flang. This change adds an optional HelpTextForVisibility to options. This allows you to change the help text (shown in documentation and --help) for one specific visibility. It could be generalised to a list, but we only need to pick out flang right now so I've kept it simple but not so simple that it's "if flang else". For example the OpenMP option will show one default value for Clang and another for Flang. Clang will use the existing HelpText string. I have not applied this to group descriptions just because I don't know of one that needs changing. It could easily be enabled for those too if needed. There are minor changes to them just to get it all to compile. This approach of storing many help strings per option in the 1 driver library seemed perferrable to making a whole new library for Flang (even if that would mostly be including stuff from Clang). --- clang/include/clang/Driver/Options.td | 5 ++- clang/lib/Frontend/CompilerInvocation.cpp | 16 .../utils/TableGen/ClangOptionDocEmitter.cpp | 21 +- flang/test/Driver/driver-help-hidden.f90 | 2 +- flang/test/Driver/driver-help.f90 | 2 +- lld/MachO/DriverUtils.cpp | 20 ++--- lld/MinGW/Driver.cpp | 20 ++--- lld/wasm/Driver.cpp | 20 ++--- llvm/include/llvm/Option/OptParser.td | 10 + llvm/include/llvm/Option/OptTable.h | 41 +-- llvm/lib/Option/OptTable.cpp | 14 --- .../Option/OptionMarshallingTest.cpp | 6 +-- llvm/utils/TableGen/OptParserEmitter.cpp | 16 13 files changed, 143 insertions(+), 50 deletions(-) diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td index 3a028fadb25b18..765d5fbc1e692d 100644 --- a/clang/include/clang/Driver/Options.td +++ b/clang/include/clang/Driver/Options.td @@ -3385,7 +3385,10 @@ def fno_openmp : Flag<["-"], "fno-openmp">, Group, def fopenmp_version_EQ : Joined<["-"], "fopenmp-version=">, Group, Flags<[NoArgumentUnused]>, Visibility<[ClangOption, CC1Option, FlangOption, FC1Option]>, - HelpText<"Set OpenMP version (e.g. 45 for OpenMP 4.5, 51 for OpenMP 5.1). Default value is 51 for Clang">; + HelpText<"Set OpenMP version (e.g. 45 for OpenMP 4.5, 51 for OpenMP 5.1). Default value is 51 for Clang">, + HelpForVisibility>; defm openmp_extensions: BoolFOption<"openmp-extensions", LangOpts<"OpenMPExtensions">, DefaultTrue, PosFlag(R->getValueInit("HelpTextForVisibility"))) { +const Record *VisibilityHelp = R->getValueAsDef("HelpTextForVisibility"); +std::string VisibilityStr = +VisibilityHelp->getValue("Visibility")->getValue()->getAsString(); + +for (StringRef Mask : DocInfo->getValueAsListOfStrings("VisibilityMask")) { + if (VisibilityStr == Mask) { +Description = escapeRST(VisibilityHelp->getValueAsString("Text")); +break; + } +} + } + + // If there's not a program specific string, use the default one. + if (Description.empty()) +Description = getRSTStringWithTextFallback(R, "DocBrief", "HelpText"); if (!isa(R->getValueInit("Values"))) { if (!Description.empty() && Description.back() != '.') diff --git a/flang/test/Driver/driver-help-hidden.f90 b/flang/test/Driver/driver-help-hidden.f90 index 44dbac44772b29..94f2f4d1f522ec 100644 --- a/flang/test/Driver/driver-help-hidden.f90 +++ b/flang/test/Driver/driver-help-hidden.f90 @@ -83,7 +83,7 @@ ! CHECK-NEXT: -fopenmp-targets= ! CHECK-NEXT: Specify comma-separated list of triples OpenMP offloading targets to be supported ! CHECK-NEXT: -fopenmp-version= -! CHECK-NEXT: Set OpenMP version (e.g. 45 for OpenMP 4.5, 51 for OpenMP 5.1). Default value is 51 for Clang +! CHECK-NEXT: Set OpenMP version (e.g. 45 for OpenMP 4.5, 51 for OpenMP 5.1). Default value is 11 for Flang ! CHECK-NEXT: -fopenmpParse OpenMP pragmas and generate parallel code. ! CHECK-NEXT: -foptimization-record-file= ! CHECK-NEXT: Specify the output name of the file containing the optimization remarks. Implies -fsave-optimization-record. On Darwin platforms, this cannot be used with multiple -arch options. diff --git a/flang/test/Driver/driver-help.f90 b/flang/test/Driver/driver-help.f90 index b4280a454e3128..31bc67e1742482 100644 --- a/flang/test/Driver/driver-help.f90 +++ b/flang/test/Driver/driver-help.f90 @@ -69,7 +69,7 @@ ! HELP-NEXT: -fopenmp-targets= ! HELP-NEXT:
[clang] [clang-tools-extra] [flang] [lld] [llvm] [flang][clang] Add Visibility specific help text for options (PR #81869)
https://github.com/DavidSpickett edited https://github.com/llvm/llvm-project/pull/81869 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [flang] [lld] [llvm] [flang][clang] Add Visibility specific help text for options (PR #81869)
@@ -93,6 +93,11 @@ class OptionGroup { // Define the option class. +class HelpTextForVisibility { DavidSpickett wrote: I've changed it to: HelpTextForVariants (for a single entry in the list, which is many variants to one string) Help**Texts**ForVariants (for the list itself, which is many HelpTextForVariants) Which is bound to confuse someone, but it's that or have them be confused why HelpTextForVariant is actually a many to one mapping which seems worse. https://github.com/llvm/llvm-project/pull/81869 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [flang] [lld] [llvm] [flang][clang] Add Visibility specific help text for options (PR #81869)
https://github.com/DavidSpickett updated https://github.com/llvm/llvm-project/pull/81869 >From 72bbd9d38cb6e292d92391fcf04154cfbc336192 Mon Sep 17 00:00:00 2001 From: David Spickett Date: Thu, 15 Feb 2024 09:52:22 + Subject: [PATCH 01/12] [flang][clang] Add Visibility specific help text for options And use it to print the correct default OpenMP version for flang. This change adds an optional HelpTextForVisibility to options. This allows you to change the help text (shown in documentation and --help) for one specific visibility. It could be generalised to a list, but we only need to pick out flang right now so I've kept it simple but not so simple that it's "if flang else". For example the OpenMP option will show one default value for Clang and another for Flang. Clang will use the existing HelpText string. I have not applied this to group descriptions just because I don't know of one that needs changing. It could easily be enabled for those too if needed. There are minor changes to them just to get it all to compile. This approach of storing many help strings per option in the 1 driver library seemed perferrable to making a whole new library for Flang (even if that would mostly be including stuff from Clang). --- clang/include/clang/Driver/Options.td | 5 ++- clang/lib/Frontend/CompilerInvocation.cpp | 16 .../utils/TableGen/ClangOptionDocEmitter.cpp | 21 +- flang/test/Driver/driver-help-hidden.f90 | 2 +- flang/test/Driver/driver-help.f90 | 2 +- lld/MachO/DriverUtils.cpp | 20 ++--- lld/MinGW/Driver.cpp | 20 ++--- lld/wasm/Driver.cpp | 20 ++--- llvm/include/llvm/Option/OptParser.td | 10 + llvm/include/llvm/Option/OptTable.h | 41 +-- llvm/lib/Option/OptTable.cpp | 14 --- .../Option/OptionMarshallingTest.cpp | 6 +-- llvm/utils/TableGen/OptParserEmitter.cpp | 16 13 files changed, 143 insertions(+), 50 deletions(-) diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td index 3a028fadb25b18..765d5fbc1e692d 100644 --- a/clang/include/clang/Driver/Options.td +++ b/clang/include/clang/Driver/Options.td @@ -3385,7 +3385,10 @@ def fno_openmp : Flag<["-"], "fno-openmp">, Group, def fopenmp_version_EQ : Joined<["-"], "fopenmp-version=">, Group, Flags<[NoArgumentUnused]>, Visibility<[ClangOption, CC1Option, FlangOption, FC1Option]>, - HelpText<"Set OpenMP version (e.g. 45 for OpenMP 4.5, 51 for OpenMP 5.1). Default value is 51 for Clang">; + HelpText<"Set OpenMP version (e.g. 45 for OpenMP 4.5, 51 for OpenMP 5.1). Default value is 51 for Clang">, + HelpForVisibility>; defm openmp_extensions: BoolFOption<"openmp-extensions", LangOpts<"OpenMPExtensions">, DefaultTrue, PosFlag(R->getValueInit("HelpTextForVisibility"))) { +const Record *VisibilityHelp = R->getValueAsDef("HelpTextForVisibility"); +std::string VisibilityStr = +VisibilityHelp->getValue("Visibility")->getValue()->getAsString(); + +for (StringRef Mask : DocInfo->getValueAsListOfStrings("VisibilityMask")) { + if (VisibilityStr == Mask) { +Description = escapeRST(VisibilityHelp->getValueAsString("Text")); +break; + } +} + } + + // If there's not a program specific string, use the default one. + if (Description.empty()) +Description = getRSTStringWithTextFallback(R, "DocBrief", "HelpText"); if (!isa(R->getValueInit("Values"))) { if (!Description.empty() && Description.back() != '.') diff --git a/flang/test/Driver/driver-help-hidden.f90 b/flang/test/Driver/driver-help-hidden.f90 index 44dbac44772b29..94f2f4d1f522ec 100644 --- a/flang/test/Driver/driver-help-hidden.f90 +++ b/flang/test/Driver/driver-help-hidden.f90 @@ -83,7 +83,7 @@ ! CHECK-NEXT: -fopenmp-targets= ! CHECK-NEXT: Specify comma-separated list of triples OpenMP offloading targets to be supported ! CHECK-NEXT: -fopenmp-version= -! CHECK-NEXT: Set OpenMP version (e.g. 45 for OpenMP 4.5, 51 for OpenMP 5.1). Default value is 51 for Clang +! CHECK-NEXT: Set OpenMP version (e.g. 45 for OpenMP 4.5, 51 for OpenMP 5.1). Default value is 11 for Flang ! CHECK-NEXT: -fopenmpParse OpenMP pragmas and generate parallel code. ! CHECK-NEXT: -foptimization-record-file= ! CHECK-NEXT: Specify the output name of the file containing the optimization remarks. Implies -fsave-optimization-record. On Darwin platforms, this cannot be used with multiple -arch options. diff --git a/flang/test/Driver/driver-help.f90 b/flang/test/Driver/driver-help.f90 index b4280a454e3128..31bc67e1742482 100644 --- a/flang/test/Driver/driver-help.f90 +++ b/flang/test/Driver/driver-help.f90 @@ -69,7 +69,7 @@ ! HELP-NEXT: -fopenmp-targets= ! HELP-NEXT:
[clang] [clang-tools-extra] [flang] [lld] [llvm] [flang][clang] Add Visibility specific help text for options (PR #81869)
DavidSpickett wrote: I've also tested this with increased array sizes. You can check the .inc files to see that they are intialised properly, `/lib/ToolDrivers/llvm-lib/Options.inc` is one. https://github.com/llvm/llvm-project/pull/81869 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [flang] [lld] [llvm] [flang][clang] Add Visibility specific help text for options (PR #81869)
https://github.com/DavidSpickett updated https://github.com/llvm/llvm-project/pull/81869 >From 72bbd9d38cb6e292d92391fcf04154cfbc336192 Mon Sep 17 00:00:00 2001 From: David Spickett Date: Thu, 15 Feb 2024 09:52:22 + Subject: [PATCH 1/4] [flang][clang] Add Visibility specific help text for options And use it to print the correct default OpenMP version for flang. This change adds an optional HelpTextForVisibility to options. This allows you to change the help text (shown in documentation and --help) for one specific visibility. It could be generalised to a list, but we only need to pick out flang right now so I've kept it simple but not so simple that it's "if flang else". For example the OpenMP option will show one default value for Clang and another for Flang. Clang will use the existing HelpText string. I have not applied this to group descriptions just because I don't know of one that needs changing. It could easily be enabled for those too if needed. There are minor changes to them just to get it all to compile. This approach of storing many help strings per option in the 1 driver library seemed perferrable to making a whole new library for Flang (even if that would mostly be including stuff from Clang). --- clang/include/clang/Driver/Options.td | 5 ++- clang/lib/Frontend/CompilerInvocation.cpp | 16 .../utils/TableGen/ClangOptionDocEmitter.cpp | 21 +- flang/test/Driver/driver-help-hidden.f90 | 2 +- flang/test/Driver/driver-help.f90 | 2 +- lld/MachO/DriverUtils.cpp | 20 ++--- lld/MinGW/Driver.cpp | 20 ++--- lld/wasm/Driver.cpp | 20 ++--- llvm/include/llvm/Option/OptParser.td | 10 + llvm/include/llvm/Option/OptTable.h | 41 +-- llvm/lib/Option/OptTable.cpp | 14 --- .../Option/OptionMarshallingTest.cpp | 6 +-- llvm/utils/TableGen/OptParserEmitter.cpp | 16 13 files changed, 143 insertions(+), 50 deletions(-) diff --git a/clang/include/clang/Driver/Options.td b/clang/include/clang/Driver/Options.td index 3a028fadb25b18..765d5fbc1e692d 100644 --- a/clang/include/clang/Driver/Options.td +++ b/clang/include/clang/Driver/Options.td @@ -3385,7 +3385,10 @@ def fno_openmp : Flag<["-"], "fno-openmp">, Group, def fopenmp_version_EQ : Joined<["-"], "fopenmp-version=">, Group, Flags<[NoArgumentUnused]>, Visibility<[ClangOption, CC1Option, FlangOption, FC1Option]>, - HelpText<"Set OpenMP version (e.g. 45 for OpenMP 4.5, 51 for OpenMP 5.1). Default value is 51 for Clang">; + HelpText<"Set OpenMP version (e.g. 45 for OpenMP 4.5, 51 for OpenMP 5.1). Default value is 51 for Clang">, + HelpForVisibility>; defm openmp_extensions: BoolFOption<"openmp-extensions", LangOpts<"OpenMPExtensions">, DefaultTrue, PosFlag(R->getValueInit("HelpTextForVisibility"))) { +const Record *VisibilityHelp = R->getValueAsDef("HelpTextForVisibility"); +std::string VisibilityStr = +VisibilityHelp->getValue("Visibility")->getValue()->getAsString(); + +for (StringRef Mask : DocInfo->getValueAsListOfStrings("VisibilityMask")) { + if (VisibilityStr == Mask) { +Description = escapeRST(VisibilityHelp->getValueAsString("Text")); +break; + } +} + } + + // If there's not a program specific string, use the default one. + if (Description.empty()) +Description = getRSTStringWithTextFallback(R, "DocBrief", "HelpText"); if (!isa(R->getValueInit("Values"))) { if (!Description.empty() && Description.back() != '.') diff --git a/flang/test/Driver/driver-help-hidden.f90 b/flang/test/Driver/driver-help-hidden.f90 index 44dbac44772b29..94f2f4d1f522ec 100644 --- a/flang/test/Driver/driver-help-hidden.f90 +++ b/flang/test/Driver/driver-help-hidden.f90 @@ -83,7 +83,7 @@ ! CHECK-NEXT: -fopenmp-targets= ! CHECK-NEXT: Specify comma-separated list of triples OpenMP offloading targets to be supported ! CHECK-NEXT: -fopenmp-version= -! CHECK-NEXT: Set OpenMP version (e.g. 45 for OpenMP 4.5, 51 for OpenMP 5.1). Default value is 51 for Clang +! CHECK-NEXT: Set OpenMP version (e.g. 45 for OpenMP 4.5, 51 for OpenMP 5.1). Default value is 11 for Flang ! CHECK-NEXT: -fopenmpParse OpenMP pragmas and generate parallel code. ! CHECK-NEXT: -foptimization-record-file= ! CHECK-NEXT: Specify the output name of the file containing the optimization remarks. Implies -fsave-optimization-record. On Darwin platforms, this cannot be used with multiple -arch options. diff --git a/flang/test/Driver/driver-help.f90 b/flang/test/Driver/driver-help.f90 index b4280a454e3128..31bc67e1742482 100644 --- a/flang/test/Driver/driver-help.f90 +++ b/flang/test/Driver/driver-help.f90 @@ -69,7 +69,7 @@ ! HELP-NEXT: -fopenmp-targets= ! HELP-NEXT:
[clang] [clang-tools-extra] [flang] [lld] [llvm] [flang][clang] Add Visibility specific help text for options (PR #81869)
https://github.com/DavidSpickett edited https://github.com/llvm/llvm-project/pull/81869 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang-tools-extra] [flang] [lld] [llvm] [flang][clang] Add Visibility specific help text for options (PR #81869)
DavidSpickett wrote: @banach-space Your thoughts as the Flang driver owner? https://github.com/llvm/llvm-project/pull/81869 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[llvm] [clang-tools-extra] [clang] [GitHub][workflows] Add buildbot information comment to first merged PR from a new contributor (PR #78292)
https://github.com/DavidSpickett updated https://github.com/llvm/llvm-project/pull/78292 >From 20822b4a2f8e228365c8fa912af18afc9956749e Mon Sep 17 00:00:00 2001 From: David Spickett Date: Tue, 16 Jan 2024 13:36:15 + Subject: [PATCH 01/10] [GitHub][workflows] Add buildbot information comment to first merged PR from a new contributor This change adds a comment to the first PR from a new contributor that is merged, which tells them what to expect post merge from the build bots. How they will be notified, where to ask questions, that you're more likely to be reverted than in other projects, etc. To do this, I have added a hidden HTML comment to the new contributor greeting comment. This workflow will look for that to tell if the author of the PR was a new contributor at the time they opened the merge. It has to be done this way because as soon as the PR is merged, they are by GitHub's definition no longer a new contributor and I suspect that their author assocication will be "contributor" instead. I cannot 100% confirm that without a whole lot of effort and probably breaking GitHub's terms of service, but it's fairly cheap to work around anyway. It seems rare / almost impossible to reopen a PR in llvm at least, but in case it does happen the buildbot info comment has its own hidden HTML comment. If we find this we will not post another copy of the same information. An example PR can be found here: https://github.com/DavidSpickett/llvm-project/pull/84 (exact text content subject to change) --- .github/workflows/merged-prs.yml| 36 +++ llvm/utils/git/github-automation.py | 71 + 2 files changed, 107 insertions(+) create mode 100644 .github/workflows/merged-prs.yml diff --git a/.github/workflows/merged-prs.yml b/.github/workflows/merged-prs.yml new file mode 100644 index 0..1b1503610dac1 --- /dev/null +++ b/.github/workflows/merged-prs.yml @@ -0,0 +1,36 @@ +name: "Add buildbot information to first PRs from new contributors" + +permissions: + contents: read + +on: + # It's safe to use pull_request_target here, because we aren't checking out + # code from the pull request branch. + # See https://securitylab.github.com/research/github-actions-preventing-pwn-requests/ + pull_request_target: +types: + - closed + +jobs: + buildbot_comment: +runs-on: ubuntu-latest +permissions: + pull-requests: write +if: >- + (github.repository == 'llvm/llvm-project') && + (github.event.pull_request.merged == true) +steps: + - name: Setup Automation Script +run: | + curl -O -L --fail https://raw.githubusercontent.com/"$GITHUB_REPOSITORY"/main/llvm/utils/git/github-automation.py + curl -O -L --fail https://raw.githubusercontent.com/"$GITHUB_REPOSITORY"/main/llvm/utils/git/requirements.txt + chmod a+x github-automation.py + pip install -r requirements.txt + + - name: Add Buildbot information comment +run: | + ./github-automation.py \ +--token '${{ secrets.GITHUB_TOKEN }}' \ +pr-buildbot-information \ +--issue-number "${{ github.event.pull_request.number }}" \ +--author "${{ github.event.pull_request.user.login }}" diff --git a/llvm/utils/git/github-automation.py b/llvm/utils/git/github-automation.py index f78d91059ecd3..55659679536f4 100755 --- a/llvm/utils/git/github-automation.py +++ b/llvm/utils/git/github-automation.py @@ -208,6 +208,8 @@ def _get_curent_team(self) -> Optional[github.Team.Team]: class PRGreeter: +COMMENT_TAG = "\n" + def __init__(self, token: str, repo: str, pr_number: int): repo = github.Github(token).get_repo(repo) self.pr = repo.get_issue(pr_number).as_pull_request() @@ -217,7 +219,9 @@ def run(self) -> bool: # by a user new to LLVM and/or GitHub itself. # This text is using Markdown formatting. + comment = f"""\ +{PRGreeter.COMMENT_TAG} Thank you for submitting a Pull Request (PR) to the LLVM Project! This PR will be automatically labeled and the relevant teams will be @@ -240,6 +244,64 @@ def run(self) -> bool: return True +class PRBuildbotInformation: +COMMENT_TAG = "\n" + +def __init__(self, token: str, repo: str, pr_number: int, author: str): +repo = github.Github(token).get_repo(repo) +self.pr = repo.get_issue(pr_number).as_pull_request() +self.author = author + +def should_comment(self) -> bool: +# As soon as a new contributor has a PR merged, they are no longer a new contributor. +# We can tell that they were a new contributor previously because we would have +# added a new contributor greeting comment when they opened the PR. +found_greeting = False +for comment in self.pr.as_issue().get_comments(): +if PRGreeter.COMMENT_TAG in comment.body: +found_greeting = True +elif
[llvm] [clang] [clang-tools-extra] [GitHub][workflows] Add buildbot information comment to first merged PR from a new contributor (PR #78292)
https://github.com/DavidSpickett closed https://github.com/llvm/llvm-project/pull/78292 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [compiler-rt] [Sanitizer] add signed-integer-wrap sanitizer (PR #80089)
DavidSpickett wrote: > I tried running clang-format but the diff was huge and beyond just my changes. https://clang.llvm.org/docs/ClangFormat.html#git-integration tells you how to narrow it down to just your changes. Once that's setup you can use the command from the comment the bot left. Sometimes it will try to format too much, in which case it's fine to ignore it, but looking at what the bot reported here that's not the case. https://github.com/llvm/llvm-project/pull/80089 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] 0217d2e - [clang][AMDGPU] Remove trialing whitespace in doc
Author: David Spickett Date: 2024-01-31T10:29:48Z New Revision: 0217d2e089afba8ca0713787521ba52a1056 URL: https://github.com/llvm/llvm-project/commit/0217d2e089afba8ca0713787521ba52a1056 DIFF: https://github.com/llvm/llvm-project/commit/0217d2e089afba8ca0713787521ba52a1056.diff LOG: [clang][AMDGPU] Remove trialing whitespace in doc Added by f2a78e68eee53646327f71c475c7f18a28b7f576. Wouldn't normally bother but it's showing up in some CI checks, just want to reduce the noise. Added: Modified: clang/docs/HIPSupport.rst Removed: diff --git a/clang/docs/HIPSupport.rst b/clang/docs/HIPSupport.rst index 0c93d722ff14e..543c82cf90244 100644 --- a/clang/docs/HIPSupport.rst +++ b/clang/docs/HIPSupport.rst @@ -176,8 +176,8 @@ Predefined Macros * - ``HIP_API_PER_THREAD_DEFAULT_STREAM`` - Alias to ``__HIP_API_PER_THREAD_DEFAULT_STREAM__``. Deprecated. -Note that some architecture specific AMDGPU macros will have default values when -used from the HIP host compilation. Other :doc:`AMDGPU macros ` +Note that some architecture specific AMDGPU macros will have default values when +used from the HIP host compilation. Other :doc:`AMDGPU macros ` like ``__AMDGCN_WAVEFRONT_SIZE__`` will default to 64 for example. Compilation Modes ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[flang] [clang-tools-extra] [clang] [llvm] [compiler-rt] [llvm][AArch64] intrinsic to generate a ubfx instruction (PR #80103)
https://github.com/DavidSpickett edited https://github.com/llvm/llvm-project/pull/80103 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [AArch64] Make +pauth enabled in Armv8.3-a by default (PR #78027)
DavidSpickett wrote: Just for citations here, the A profile manual says: FEAT_PAuth is mandatory in Armv8.3 implementations. The R Profile manual says: The Armv8-R AArch64 architecture supports FEAT_PAuth2 feature defined in Armv8-A architecture with a modified definition of PAC field as described below... So the overall intent is correct. https://github.com/llvm/llvm-project/pull/78027 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [AArch64] Make +pauth enabled in Armv8.3-a by default (PR #78027)
https://github.com/DavidSpickett edited https://github.com/llvm/llvm-project/pull/78027 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[llvm] [clang] [AArch64] Make +pauth enabled in Armv8.3-a by default (PR #78027)
https://github.com/DavidSpickett approved this pull request. LGTM. I think the reason we didn't do this is because we have so far: * Used nop space encodings * Used a CPU that would enable it * Been adding -mbranch-protection and friends anyway, so adding +pauth didn't seem like a big deal. But it's correct according to the ARMARM, so let's go ahead with this. https://github.com/llvm/llvm-project/pull/78027 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [AArch64] Make +pauth enabled in Armv8.3-a by default (PR #78027)
@@ -1280,7 +1282,7 @@ INSTANTIATE_TEST_SUITE_P( AArch64::AEK_DOTPROD, AArch64::AEK_FP, AArch64::AEK_SIMD, AArch64::AEK_FP16, AArch64::AEK_FP16FML, AArch64::AEK_RAS, AArch64::AEK_RCPC, AArch64::AEK_LSE, AArch64::AEK_SB, - AArch64::AEK_JSCVT, AArch64::AEK_FCMA})), + AArch64::AEK_JSCVT, AArch64::AEK_FCMA, AArch64::AEK_PAUTH})), DavidSpickett wrote: I confirmed this was correct by looking at https://developer.arm.com/documentation/102670/0200/?lang=en. https://github.com/llvm/llvm-project/pull/78027 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [AArch64] Make +pauth enabled in Armv8.3-a by default (PR #78027)
DavidSpickett wrote: And the code formatter is going crazy over some of this code :) One of these days we'll fix it. You can ignore it for now, save yourself and my downstream colleagues a massive headache with the merge. https://github.com/llvm/llvm-project/pull/78027 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [clang][ARM] Change NEON poly type to be unsigned (#56781) (PR #80691)
https://github.com/DavidSpickett edited https://github.com/llvm/llvm-project/pull/80691 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang-tools-extra] [libcxx] [clang] [llvm] [lld] [lldb] [libc] intrinsic to generate a bfi instruction (PR #79655)
DavidSpickett wrote: If you're referring to the CI that runs here, we've been having capacity issues lately. Another way to "resubmit" is to rebase the PR. https://github.com/llvm/llvm-project/pull/79655 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [llvm] [AArch64][TargetParser] Add mcpu alias for Microsoft Azure Cobalt 100. (PR #79614)
https://github.com/DavidSpickett approved this pull request. ``` /var/lib/buildkite-agent/builds/linux-56-59b8f5d88-r6v66-1/llvm-project/github-pull-requests/llvm/unittests/TargetParser/TargetParserTest.cpp:1662: Failure Expected equality of these values: List.size() Which is: 68 NumAArch64CPUArchs Which is: 67 ``` Needs fixing, but otherwise LGTM. https://github.com/llvm/llvm-project/pull/79614 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] 6f55c13 - [clang[test] Require x86 target for new tests
Author: David Spickett Date: 2024-01-12T16:08:52Z New Revision: 6f55c134d4aaa9eab9ef53886c2532d6da72ca47 URL: https://github.com/llvm/llvm-project/commit/6f55c134d4aaa9eab9ef53886c2532d6da72ca47 DIFF: https://github.com/llvm/llvm-project/commit/6f55c134d4aaa9eab9ef53886c2532d6da72ca47.diff LOG: [clang[test] Require x86 target for new tests Fixes d199ab469949b104bc4fbb888251ee184fd53de1. Added: Modified: clang/test/CodeGen/debug-names-compound-type-units.ll clang/test/CodeGen/thinlto-debug-names-tu-reuse.ll Removed: diff --git a/clang/test/CodeGen/debug-names-compound-type-units.ll b/clang/test/CodeGen/debug-names-compound-type-units.ll index 0d85de286772a3..62cecfdeaf7f94 100644 --- a/clang/test/CodeGen/debug-names-compound-type-units.ll +++ b/clang/test/CodeGen/debug-names-compound-type-units.ll @@ -1,4 +1,5 @@ ; REQUIRES: asserts +; REQUIRES: x86-registered-target ;; Tests that we use correct accelerator table when processing nested TUs. ;; Assert is not triggered. diff --git a/clang/test/CodeGen/thinlto-debug-names-tu-reuse.ll b/clang/test/CodeGen/thinlto-debug-names-tu-reuse.ll index 53aec43a050f8b..8719473820053e 100644 --- a/clang/test/CodeGen/thinlto-debug-names-tu-reuse.ll +++ b/clang/test/CodeGen/thinlto-debug-names-tu-reuse.ll @@ -1,4 +1,5 @@ ; REQUIRES: asserts +; REQUIRES: x86-registered-target ;; Tests that accelerator table switches correctly from TU to CU when a top level TU is re-used. ;; Assert is not triggered. ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [NFC][Clang][Headers] Update refs to ACLE in comments (PR #78305)
https://github.com/DavidSpickett approved this pull request. LGTM https://github.com/llvm/llvm-project/pull/78305 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [llvm] [AArch64][Driver] Better handling of target feature dependencies (PR #78270)
https://github.com/DavidSpickett edited https://github.com/llvm/llvm-project/pull/78270 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [llvm] [AArch64][Driver] Better handling of target feature dependencies (PR #78270)
https://github.com/DavidSpickett commented: Happy to see this change, will be a lot easier to reason about this once it's all in one place. https://github.com/llvm/llvm-project/pull/78270 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [llvm] [AArch64][Driver] Better handling of target feature dependencies (PR #78270)
@@ -308,6 +312,94 @@ inline constexpr ExtensionInfo Extensions[] = { }; // clang-format on +struct ExtensionSet { + // Set of extensions which are currently enabled. + ExtensionBitset Enabled; + // Set of extensions which have been enabled or disabled at any point. Used + // to avoid cluttering the cc1 command-line with lots of unneeded features. + ExtensionBitset Touched; + // Base architecture version, which we need to know because some feature + // dependencies change depending on this. + const ArchInfo *BaseArch; + + ExtensionSet() : Enabled(), Touched(), BaseArch(nullptr) {} + + // Enable the given architecture extension, and any other extensions it + // depends on. DavidSpickett wrote: Is it worth saying here whether it updates the base architecture version? I'm guessing that it does not. https://github.com/llvm/llvm-project/pull/78270 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[flang] [llvm] [clang] [AArch64][Driver] Better handling of target feature dependencies (PR #78270)
@@ -1,8 +1,9 @@ // RUN: %clang -### --target=aarch64-none-elf -march=armv8a+predres %s 2>&1 | FileCheck %s +// RUN: %clang -### --target=aarch64-none-elf -mcpu=cortex-a520 %s 2>&1 | FileCheck %s // CHECK: "-target-feature" "+predres" // CHECK-NOT: "-target-feature" "-predres" -// RUN: %clang -### --target=aarch64-none-elf -march=armv8.5a+nopredres %s 2>&1 | FileCheck %s --check-prefix=NOPR +// RUN: %clang -### --target=aarch64-none-elf -mcpu=cortex-a520+nopredres %s 2>&1 | FileCheck %s --check-prefix=NOPR DavidSpickett wrote: Did you mean to remove the armv8.5-a line here? As above you added the cortex-a520 instead of replacing the armv8.5 line. https://github.com/llvm/llvm-project/pull/78270 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
[clang] [flang] [llvm] [AArch64][Driver] Better handling of target feature dependencies (PR #78270)
@@ -308,6 +312,94 @@ inline constexpr ExtensionInfo Extensions[] = { }; // clang-format on +struct ExtensionSet { + // Set of extensions which are currently enabled. + ExtensionBitset Enabled; + // Set of extensions which have been enabled or disabled at any point. Used + // to avoid cluttering the cc1 command-line with lots of unneeded features. + ExtensionBitset Touched; + // Base architecture version, which we need to know because some feature + // dependencies change depending on this. + const ArchInfo *BaseArch; + + ExtensionSet() : Enabled(), Touched(), BaseArch(nullptr) {} + + // Enable the given architecture extension, and any other extensions it + // depends on. + void enable(ArchExtKind E); + // Disable the given architecture extension, and any other extensions which + // depend on it. + void disable(ArchExtKind E); + + // Add default extensions for the given CPU. + void addCPUDefaults(const CpuInfo &CPU); + // Add default extensions for the given architecture version. + void addArchDefaults(const ArchInfo &Arch); + + // Add or remove a feature based on a modifier string. The string must be of + // the form "" to enable a feature or "no" to disable it. This + // will also enable or disable any features as required by the dependencies + // between them. + bool parseModifier(StringRef Modifier); + + // Convert the set of enabled extension to an LLVM feature list, appending + // them to Features. + void toLLVMFeatureList(std::vector &Features) const; DavidSpickett wrote: Is it useful to have this return by ref? Could it be: ``` std::vector toLLVMFeatureList() const; ``` Depends what the majority of call sites do. https://github.com/llvm/llvm-project/pull/78270 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits