From f12cd99c440a83d53a8717a9c8cdc4df41f39f3d Mon Sep 17 00:00:00 2001 From: Sam McCall Date: Fri, 26 Jun 2020 01:49:53 +0200 Subject: [PATCH 01/14] [clangd] Config: compile Fragment -> CompiledFragment -> Config Subscribers: mgorny, ilya-biryukov, MaskRay, jkorous, arphaman, kadircet, usaxena95, cfe-commits Tags: #clang Differential Revision: https://reviews.llvm.org/D82612 --- clang-tools-extra/clangd/CMakeLists.txt | 1 + clang-tools-extra/clangd/ConfigCompile.cpp | 156 ++++++++++++++++++ clang-tools-extra/clangd/ConfigFragment.h | 24 ++- clang-tools-extra/clangd/ConfigProvider.h | 54 ++++++ clang-tools-extra/clangd/ConfigYAML.cpp | 6 +- .../clangd/unittests/CMakeLists.txt | 1 + .../clangd/unittests/ConfigCompileTests.cpp | 97 +++++++++++ .../clangd/unittests/ConfigTesting.h | 77 +++++++++ .../clangd/unittests/ConfigYAMLTests.cpp | 62 +------ 9 files changed, 411 insertions(+), 67 deletions(-) create mode 100644 clang-tools-extra/clangd/ConfigCompile.cpp create mode 100644 clang-tools-extra/clangd/ConfigProvider.h create mode 100644 clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp create mode 100644 clang-tools-extra/clangd/unittests/ConfigTesting.h diff --git a/clang-tools-extra/clangd/CMakeLists.txt b/clang-tools-extra/clangd/CMakeLists.txt index 20a32d28e11904..916826a8679b16 100644 --- a/clang-tools-extra/clangd/CMakeLists.txt +++ b/clang-tools-extra/clangd/CMakeLists.txt @@ -37,6 +37,7 @@ add_clang_library(clangDaemon CompileCommands.cpp Compiler.cpp Config.cpp + ConfigCompile.cpp ConfigYAML.cpp Diagnostics.cpp DraftStore.cpp diff --git a/clang-tools-extra/clangd/ConfigCompile.cpp b/clang-tools-extra/clangd/ConfigCompile.cpp new file mode 100644 index 00000000000000..63c1681ceb0b16 --- /dev/null +++ b/clang-tools-extra/clangd/ConfigCompile.cpp @@ -0,0 +1,156 @@ +//===--- ConfigCompile.cpp - Translating Fragments into Config ------------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// +// +// Fragments are applied to Configs in two steps: +// +// 1. (When the fragment is first loaded) +// FragmentCompiler::compile() traverses the Fragment and creates +// function objects that know how to apply the configuration. +// 2. (Every time a config is required) +// CompiledFragment() executes these functions to populate the Config. +// +// Work could be split between these steps in different ways. We try to +// do as much work as possible in the first step. For example, regexes are +// compiled in stage 1 and captured by the apply function. This is because: +// +// - it's more efficient, as the work done in stage 1 must only be done once +// - problems can be reported in stage 1, in stage 2 we must silently recover +// +//===----------------------------------------------------------------------===// + +#include "Config.h" +#include "ConfigFragment.h" +#include "support/Logger.h" +#include "support/Trace.h" +#include "llvm/ADT/StringRef.h" +#include "llvm/Support/Regex.h" +#include "llvm/Support/SMLoc.h" +#include "llvm/Support/SourceMgr.h" + +namespace clang { +namespace clangd { +namespace config { +namespace { + +struct CompiledFragmentImpl { + // The independent conditions to check before using settings from this config. + // The following fragment has *two* conditions: + // If: { Platform: [mac, linux], PathMatch: foo/.* } + // All of them must be satisfied: the platform and path conditions are ANDed. + // The OR logic for the platform condition is implemented inside the function. + std::vector> Conditions; + // Mutations that this fragment will apply to the configuration. + // These are invoked only if the conditions are satisfied. + std::vector> Apply; + + bool operator()(const Params &P, Config &C) const { + for (const auto &C : Conditions) { + if (!C(P)) { + dlog("Config fragment {0}: condition not met", this); + return false; + } + } + dlog("Config fragment {0}: applying {1} rules", this, Apply.size()); + for (const auto &A : Apply) + A(C); + return true; + } +}; + +// Wrapper around condition compile() functions to reduce arg-passing. +struct FragmentCompiler { + CompiledFragmentImpl &Out; + DiagnosticCallback Diagnostic; + llvm::SourceMgr *SourceMgr; + + llvm::Optional compileRegex(const Located &Text) { + std::string Anchored = "^(" + *Text + ")$"; + llvm::Regex Result(Anchored); + std::string RegexError; + if (!Result.isValid(RegexError)) { + diag(Error, "Invalid regex " + Anchored + ": " + RegexError, Text.Range); + return llvm::None; + } + return Result; + } + + void compile(Fragment &&F) { + compile(std::move(F.If)); + compile(std::move(F.CompileFlags)); + } + + void compile(Fragment::IfBlock &&F) { + if (F.HasUnrecognizedCondition) + Out.Conditions.push_back([&](const Params &) { return false; }); + + auto PathMatch = std::make_unique>(); + for (auto &Entry : F.PathMatch) { + if (auto RE = compileRegex(Entry)) + PathMatch->push_back(std::move(*RE)); + } + if (!PathMatch->empty()) { + Out.Conditions.push_back( + [PathMatch(std::move(PathMatch))](const Params &P) { + if (P.Path.empty()) + return false; + return llvm::any_of(*PathMatch, [&](const llvm::Regex &RE) { + return RE.match(P.Path); + }); + }); + } + } + + void compile(Fragment::CompileFlagsBlock &&F) { + if (!F.Add.empty()) { + std::vector Add; + for (auto &A : F.Add) + Add.push_back(std::move(*A)); + Out.Apply.push_back([Add(std::move(Add))](Config &C) { + C.CompileFlags.Edits.push_back([Add](std::vector &Args) { + Args.insert(Args.end(), Add.begin(), Add.end()); + }); + }); + } + } + + constexpr static llvm::SourceMgr::DiagKind Error = llvm::SourceMgr::DK_Error; + void diag(llvm::SourceMgr::DiagKind Kind, llvm::StringRef Message, + llvm::SMRange Range) { + if (Range.isValid() && SourceMgr != nullptr) + Diagnostic(SourceMgr->GetMessage(Range.Start, Kind, Message, Range)); + else + Diagnostic(llvm::SMDiagnostic("", Kind, Message)); + } +}; + +} // namespace + +CompiledFragment Fragment::compile(DiagnosticCallback D) && { + llvm::StringRef ConfigFile = ""; + std::pair LineCol = {0, 0}; + if (auto *SM = Source.Manager.get()) { + unsigned BufID = SM->getMainFileID(); + LineCol = SM->getLineAndColumn(Source.Location, BufID); + ConfigFile = SM->getBufferInfo(BufID).Buffer->getBufferIdentifier(); + } + trace::Span Tracer("ConfigCompile"); + SPAN_ATTACH(Tracer, "ConfigFile", ConfigFile); + auto Result = std::make_shared(); + vlog("Config fragment: compiling {0}:{1} -> {2}", ConfigFile, LineCol.first, + Result.get()); + + FragmentCompiler{*Result, D, Source.Manager.get()}.compile(std::move(*this)); + // Return as cheaply-copyable wrapper. + return [Result(std::move(Result))](const Params &P, Config &C) { + return (*Result)(P, C); + }; +} + +} // namespace config +} // namespace clangd +} // namespace clang diff --git a/clang-tools-extra/clangd/ConfigFragment.h b/clang-tools-extra/clangd/ConfigFragment.h index 2cb646757e17d7..be5bd5edc188bd 100644 --- a/clang-tools-extra/clangd/ConfigFragment.h +++ b/clang-tools-extra/clangd/ConfigFragment.h @@ -32,6 +32,7 @@ #ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_CONFIGFRAGMENT_H #define LLVM_CLANG_TOOLS_EXTRA_CLANGD_CONFIGFRAGMENT_H +#include "ConfigProvider.h" #include "llvm/ADT/Optional.h" #include "llvm/ADT/STLExtras.h" #include "llvm/Support/Error.h" @@ -59,11 +60,6 @@ template struct Located { T Value; }; -/// Used to report problems in parsing or interpreting a config. -/// Errors reflect structurally invalid config that should be user-visible. -/// Warnings reflect e.g. unknown properties that are recoverable. -using DiagnosticCallback = llvm::function_ref; - /// A chunk of configuration obtained from a config file, LSP, or elsewhere. struct Fragment { /// Parses fragments from a YAML file (one from each --- delimited document). @@ -73,6 +69,17 @@ struct Fragment { llvm::StringRef BufferName, DiagnosticCallback); + /// Analyzes and consumes this fragment, possibly yielding more diagnostics. + /// This always produces a usable result (errors are recovered). + /// + /// Typically, providers will compile a Fragment once when it's first loaded, + /// caching the result for reuse. + /// Like a compiled program, this is good for performance and also encourages + /// errors to be reported early and only once. + /// + /// The returned function is a cheap-copyable wrapper of refcounted internals. + CompiledFragment compile(DiagnosticCallback) &&; + /// These fields are not part of the user-specified configuration, but /// instead are populated by the parser to describe the configuration source. struct SourceInfo { @@ -87,24 +94,25 @@ struct Fragment { }; SourceInfo Source; - /// Conditions restrict when a Fragment applies. + /// Conditions in the If block restrict when a Fragment applies. /// /// Each separate condition must match (combined with AND). /// When one condition has multiple values, any may match (combined with OR). + /// e.g. `PathMatch: [foo/.*, bar/.*]` matches files in either directory. /// /// Conditions based on a file's path use the following form: /// - if the fragment came from a project directory, the path is relative /// - if the fragment is global (e.g. user config), the path is absolute /// - paths always use forward-slashes (UNIX-style) /// If no file is being processed, these conditions will not match. - struct ConditionBlock { + struct IfBlock { /// The file being processed must fully match a regular expression. std::vector> PathMatch; /// An unrecognized key was found while parsing the condition. /// The condition will evaluate to false. bool HasUnrecognizedCondition = false; }; - ConditionBlock Condition; + IfBlock If; struct CompileFlagsBlock { std::vector> Add; diff --git a/clang-tools-extra/clangd/ConfigProvider.h b/clang-tools-extra/clangd/ConfigProvider.h new file mode 100644 index 00000000000000..a81e7eb855b4f9 --- /dev/null +++ b/clang-tools-extra/clangd/ConfigProvider.h @@ -0,0 +1,54 @@ +//===--- ConfigProvider.h - Loading of user configuration --------*- C++-*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// +// +// Various clangd features have configurable behaviour (or can be disabled). +// The configuration system allows users to control this: +// - in a user config file, a project config file, via LSP, or via flags +// - specifying different settings for different files +// This file defines the structures used for this, that produce a Config. +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_CONFIGPROVIDER_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_CONFIGPROVIDER_H + +#include "llvm/ADT/FunctionExtras.h" +#include "llvm/Support/SMLoc.h" +#include "llvm/Support/SourceMgr.h" +#include +#include + +namespace clang { +namespace clangd { +struct Config; +namespace config { + +/// Describes the context used to evaluate configuration fragments. +struct Params { + /// Absolute path to a source file we're applying the config to. Unix slashes. + /// Empty if not configuring a particular file. + llvm::StringRef Path; +}; + +/// Used to report problems in parsing or interpreting a config. +/// Errors reflect structurally invalid config that should be user-visible. +/// Warnings reflect e.g. unknown properties that are recoverable. +using DiagnosticCallback = llvm::function_ref; + +/// A chunk of configuration that has been fully analyzed and is ready to apply. +/// Typically this is obtained from a Fragment by calling Fragment::compile(). +/// +/// Calling it updates the configuration to reflect settings from the fragment. +/// Returns true if the condition was met and the settings were used. +using CompiledFragment = std::function; + +} // namespace config +} // namespace clangd +} // namespace clang + +#endif diff --git a/clang-tools-extra/clangd/ConfigYAML.cpp b/clang-tools-extra/clangd/ConfigYAML.cpp index 8575356132caec..1201c9a5d5232b 100644 --- a/clang-tools-extra/clangd/ConfigYAML.cpp +++ b/clang-tools-extra/clangd/ConfigYAML.cpp @@ -35,15 +35,15 @@ class Parser { // The private parse() helpers follow the same pattern. bool parse(Fragment &F, Node &N) { DictParser Dict("Config", this); - Dict.handle("If", [&](Node &N) { return parse(F.Condition, N); }); + Dict.handle("If", [&](Node &N) { return parse(F.If, N); }); Dict.handle("CompileFlags", [&](Node &N) { return parse(F.CompileFlags, N); }); return Dict.parse(N); } private: - bool parse(Fragment::ConditionBlock &F, Node &N) { - DictParser Dict("Condition", this); + bool parse(Fragment::IfBlock &F, Node &N) { + DictParser Dict("If", this); Dict.unrecognized( [&](llvm::StringRef) { F.HasUnrecognizedCondition = true; }); Dict.handle("PathMatch", [&](Node &N) { diff --git a/clang-tools-extra/clangd/unittests/CMakeLists.txt b/clang-tools-extra/clangd/unittests/CMakeLists.txt index 7cfe3793865431..698e27a86008af 100644 --- a/clang-tools-extra/clangd/unittests/CMakeLists.txt +++ b/clang-tools-extra/clangd/unittests/CMakeLists.txt @@ -41,6 +41,7 @@ add_unittest(ClangdUnitTests ClangdTests CollectMacrosTests.cpp CompileCommandsTests.cpp CompilerTests.cpp + ConfigCompileTests.cpp ConfigYAMLTests.cpp DexTests.cpp DiagnosticsTests.cpp diff --git a/clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp b/clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp new file mode 100644 index 00000000000000..17db87afecfd2f --- /dev/null +++ b/clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp @@ -0,0 +1,97 @@ +//===-- ConfigCompileTests.cpp --------------------------------------------===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#include "Config.h" +#include "ConfigFragment.h" +#include "ConfigTesting.h" +#include "gmock/gmock.h" +#include "gtest/gtest.h" + +namespace clang { +namespace clangd { +namespace config { +namespace { +using ::testing::ElementsAre; +using ::testing::IsEmpty; +using ::testing::SizeIs; +using ::testing::StartsWith; + +class ConfigCompileTests : public ::testing::Test { +protected: + CapturedDiags Diags; + Config Conf; + Fragment Frag; + Params Parm; + + bool compileAndApply() { + Conf = Config(); + Diags.Diagnostics.clear(); + auto Compiled = std::move(Frag).compile(Diags.callback()); + return Compiled(Parm, Conf); + } +}; + +TEST_F(ConfigCompileTests, Condition) { + // No condition. + Frag = {}; + Frag.CompileFlags.Add.emplace_back("X"); + EXPECT_TRUE(compileAndApply()) << "Empty config"; + EXPECT_THAT(Diags.Diagnostics, IsEmpty()); + EXPECT_THAT(Conf.CompileFlags.Edits, SizeIs(1)); + + // Regex with no file. + Frag = {}; + Frag.If.PathMatch.emplace_back("fo*"); + EXPECT_FALSE(compileAndApply()); + EXPECT_THAT(Diags.Diagnostics, IsEmpty()); + EXPECT_THAT(Conf.CompileFlags.Edits, SizeIs(0)); + + // Following tests have a file path set. + Parm.Path = "bar"; + + // Non-matching regex. + Frag = {}; + Frag.If.PathMatch.emplace_back("fo*"); + EXPECT_FALSE(compileAndApply()); + EXPECT_THAT(Diags.Diagnostics, IsEmpty()); + + // Matching regex. + Frag = {}; + Frag.If.PathMatch.emplace_back("fo*"); + Frag.If.PathMatch.emplace_back("ba*r"); + EXPECT_TRUE(compileAndApply()); + EXPECT_THAT(Diags.Diagnostics, IsEmpty()); + + // Invalid regex. + Frag = {}; + Frag.If.PathMatch.emplace_back("**]@theu"); + EXPECT_TRUE(compileAndApply()); + EXPECT_THAT(Diags.Diagnostics, SizeIs(1)); + EXPECT_THAT(Diags.Diagnostics.front().Message, StartsWith("Invalid regex")); + + // Valid regex and unknown key. + Frag = {}; + Frag.If.HasUnrecognizedCondition = true; + Frag.If.PathMatch.emplace_back("ba*r"); + EXPECT_FALSE(compileAndApply()); + EXPECT_THAT(Diags.Diagnostics, IsEmpty()); +} + +TEST_F(ConfigCompileTests, CompileCommands) { + Frag.CompileFlags.Add.emplace_back("-foo"); + std::vector Argv = {"clang", "a.cc"}; + EXPECT_TRUE(compileAndApply()); + EXPECT_THAT(Conf.CompileFlags.Edits, SizeIs(1)); + Conf.CompileFlags.Edits.front()(Argv); + EXPECT_THAT(Argv, ElementsAre("clang", "a.cc", "-foo")); +} + +} // namespace +} // namespace config +} // namespace clangd +} // namespace clang diff --git a/clang-tools-extra/clangd/unittests/ConfigTesting.h b/clang-tools-extra/clangd/unittests/ConfigTesting.h new file mode 100644 index 00000000000000..fd0ecf53f360d4 --- /dev/null +++ b/clang-tools-extra/clangd/unittests/ConfigTesting.h @@ -0,0 +1,77 @@ +//===-- ConfigTesting.h - Helpers for configuration tests -------*- C++ -*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_UNITTESTS_CONFIGTESTING_H +#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_UNITTESTS_CONFIGTESTING_H + +#include "Protocol.h" +#include "llvm/Support/ScopedPrinter.h" +#include "llvm/Support/SourceMgr.h" +#include "gmock/gmock.h" +#include + +namespace clang { +namespace clangd { +namespace config { + +// Provides a DiagnosticsCallback that records diganostics. +// Unlike just pushing them into a vector, underlying storage need not survive. +struct CapturedDiags { + std::function callback() { + return [this](const llvm::SMDiagnostic &D) { + Diagnostics.emplace_back(); + Diag &Out = Diagnostics.back(); + Out.Message = D.getMessage().str(); + Out.Kind = D.getKind(); + Out.Pos.line = D.getLineNo() - 1; + Out.Pos.character = D.getColumnNo(); // Zero-based - bug in SourceMgr? + if (!D.getRanges().empty()) { + const auto &R = D.getRanges().front(); + Out.Range.emplace(); + Out.Range->start.line = Out.Range->end.line = Out.Pos.line; + Out.Range->start.character = R.first; + Out.Range->end.character = R.second; + } + }; + } + struct Diag { + std::string Message; + llvm::SourceMgr::DiagKind Kind; + Position Pos; + llvm::Optional Range; + + friend void PrintTo(const Diag &D, std::ostream *OS) { + *OS << (D.Kind == llvm::SourceMgr::DK_Error ? "error: " : "warning: ") + << D.Message << "@" << llvm::to_string(D.Pos); + } + }; + std::vector Diagnostics; +}; + +MATCHER_P(DiagMessage, M, "") { return arg.Message == M; } +MATCHER_P(DiagKind, K, "") { return arg.Kind == K; } +MATCHER_P(DiagPos, P, "") { return arg.Pos == P; } +MATCHER_P(DiagRange, R, "") { return arg.Range == R; } + +inline Position toPosition(llvm::SMLoc L, const llvm::SourceMgr &SM) { + auto LineCol = SM.getLineAndColumn(L); + Position P; + P.line = LineCol.first - 1; + P.character = LineCol.second - 1; + return P; +} + +inline Range toRange(llvm::SMRange R, const llvm::SourceMgr &SM) { + return Range{toPosition(R.Start, SM), toPosition(R.End, SM)}; +} + +} // namespace config +} // namespace clangd +} // namespace clang + +#endif diff --git a/clang-tools-extra/clangd/unittests/ConfigYAMLTests.cpp b/clang-tools-extra/clangd/unittests/ConfigYAMLTests.cpp index e3b8215670c058..3ad8b2e7c1c8e2 100644 --- a/clang-tools-extra/clangd/unittests/ConfigYAMLTests.cpp +++ b/clang-tools-extra/clangd/unittests/ConfigYAMLTests.cpp @@ -8,14 +8,13 @@ #include "Annotations.h" #include "ConfigFragment.h" -#include "Matchers.h" +#include "ConfigTesting.h" #include "Protocol.h" #include "llvm/Support/SMLoc.h" #include "llvm/Support/ScopedPrinter.h" #include "llvm/Support/SourceMgr.h" #include "gmock/gmock.h" #include "gtest/gtest.h" -#include "gtest/internal/gtest-internal.h" namespace clang { namespace clangd { @@ -36,55 +35,6 @@ MATCHER_P(Val, Value, "") { return false; } -Position toPosition(llvm::SMLoc L, const llvm::SourceMgr &SM) { - auto LineCol = SM.getLineAndColumn(L); - Position P; - P.line = LineCol.first - 1; - P.character = LineCol.second - 1; - return P; -} - -Range toRange(llvm::SMRange R, const llvm::SourceMgr &SM) { - return Range{toPosition(R.Start, SM), toPosition(R.End, SM)}; -} - -struct CapturedDiags { - std::function callback() { - return [this](const llvm::SMDiagnostic &D) { - Diagnostics.emplace_back(); - Diag &Out = Diagnostics.back(); - Out.Message = D.getMessage().str(); - Out.Kind = D.getKind(); - Out.Pos.line = D.getLineNo() - 1; - Out.Pos.character = D.getColumnNo(); // Zero-based - bug in SourceMgr? - if (!D.getRanges().empty()) { - const auto &R = D.getRanges().front(); - Out.Rng.emplace(); - Out.Rng->start.line = Out.Rng->end.line = Out.Pos.line; - Out.Rng->start.character = R.first; - Out.Rng->end.character = R.second; - } - }; - } - struct Diag { - std::string Message; - llvm::SourceMgr::DiagKind Kind; - Position Pos; - llvm::Optional Rng; - - friend void PrintTo(const Diag &D, std::ostream *OS) { - *OS << (D.Kind == llvm::SourceMgr::DK_Error ? "error: " : "warning: ") - << D.Message << "@" << llvm::to_string(D.Pos); - } - }; - std::vector Diagnostics; -}; - -MATCHER_P(DiagMessage, M, "") { return arg.Message == M; } -MATCHER_P(DiagKind, K, "") { return arg.Kind == K; } -MATCHER_P(DiagPos, P, "") { return arg.Pos == P; } -MATCHER_P(DiagRange, R, "") { return arg.Rng == R; } - TEST(ParseYAML, SyntacticForms) { CapturedDiags Diags; const char *YAML = R"yaml( @@ -101,8 +51,8 @@ CompileFlags: { Add: [foo, bar] } auto Results = Fragment::parseYAML(YAML, "config.yaml", Diags.callback()); EXPECT_THAT(Diags.Diagnostics, IsEmpty()); ASSERT_EQ(Results.size(), 2u); - EXPECT_FALSE(Results.front().Condition.HasUnrecognizedCondition); - EXPECT_THAT(Results.front().Condition.PathMatch, ElementsAre(Val("abc"))); + EXPECT_FALSE(Results.front().If.HasUnrecognizedCondition); + EXPECT_THAT(Results.front().If.PathMatch, ElementsAre(Val("abc"))); EXPECT_THAT(Results.front().CompileFlags.Add, ElementsAre(Val("foo"), Val("bar"))); @@ -120,7 +70,7 @@ TEST(ParseYAML, Locations) { EXPECT_THAT(Diags.Diagnostics, IsEmpty()); ASSERT_EQ(Results.size(), 1u); ASSERT_NE(Results.front().Source.Manager, nullptr); - EXPECT_EQ(toRange(Results.front().Condition.PathMatch.front().Range, + EXPECT_EQ(toRange(Results.front().If.PathMatch.front().Range, *Results.front().Source.Manager), YAML.range()); } @@ -140,7 +90,7 @@ CompileFlags: {^ ASSERT_THAT( Diags.Diagnostics, - ElementsAre(AllOf(DiagMessage("Unknown Condition key UnknownCondition"), + ElementsAre(AllOf(DiagMessage("Unknown If key UnknownCondition"), DiagKind(llvm::SourceMgr::DK_Warning), DiagPos(YAML.range().start), DiagRange(YAML.range())), AllOf(DiagMessage("Unexpected token. Expected Key, Flow " @@ -150,7 +100,7 @@ CompileFlags: {^ ASSERT_EQ(Results.size(), 2u); EXPECT_THAT(Results.front().CompileFlags.Add, ElementsAre(Val("first"))); - EXPECT_TRUE(Results.front().Condition.HasUnrecognizedCondition); + EXPECT_TRUE(Results.front().If.HasUnrecognizedCondition); EXPECT_THAT(Results.back().CompileFlags.Add, IsEmpty()); } From 52f65323660051a5d039d475edfd4a3018682dcb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Endre=20F=C3=BCl=C3=B6p?= Date: Thu, 25 Jun 2020 17:10:56 +0200 Subject: [PATCH 02/14] [analyzer][CrossTU] Lower CTUImportThreshold default value Summary: The default value of 100 makes the analysis slow. Projects of considerable size can take more time to finish than it is practical. The new default setting of 8 is based on the analysis of LLVM itself. With the old default value of 100 the analysis time was over a magnitude slower. Thresholding the load of ASTUnits is to be extended in the future with a more fine-tuneable solution that accomodates to the specifics of the project analyzed. Reviewers: martong, balazske, Szelethus Subscribers: whisperity, xazax.hun, baloghadamsoftware, szepet, rnkovacs, a.sidorin, mikhail.ramalho, Szelethus, donat.nagy, dkrupp, Charusso, steakhal, ASDenysPetrov, cfe-commits Tags: #clang Differential Revision: https://reviews.llvm.org/D82561 --- clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def | 2 +- clang/test/Analysis/analyzer-config.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def b/clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def index 8944dfe0f749a6..9ee113c0dcaf11 100644 --- a/clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def +++ b/clang/include/clang/StaticAnalyzer/Core/AnalyzerOptions.def @@ -324,7 +324,7 @@ ANALYZER_OPTION(unsigned, CTUImportThreshold, "ctu-import-threshold", "Lowering this threshold can alleviate the memory burder of " "analysis with many interdependent definitions located in " "various translation units.", - 100u) + 8u) ANALYZER_OPTION( unsigned, AlwaysInlineSize, "ipa-always-inline-size", diff --git a/clang/test/Analysis/analyzer-config.c b/clang/test/Analysis/analyzer-config.c index 7a411a162201bf..e4035cf755b24e 100644 --- a/clang/test/Analysis/analyzer-config.c +++ b/clang/test/Analysis/analyzer-config.c @@ -41,7 +41,7 @@ // CHECK-NEXT: cplusplus.Move:WarnOn = KnownsAndLocals // CHECK-NEXT: crosscheck-with-z3 = false // CHECK-NEXT: ctu-dir = "" -// CHECK-NEXT: ctu-import-threshold = 100 +// CHECK-NEXT: ctu-import-threshold = 8 // CHECK-NEXT: ctu-index-name = externalDefMap.txt // CHECK-NEXT: ctu-invocation-list = invocations.yaml // CHECK-NEXT: deadcode.DeadStores:ShowFixIts = false From 9d347f6efa3018faf2fa159e25830817f4d2f41d Mon Sep 17 00:00:00 2001 From: LLVM GN Syncbot Date: Wed, 1 Jul 2020 08:09:43 +0000 Subject: [PATCH 03/14] [gn build] Port f12cd99c440 --- llvm/utils/gn/secondary/clang-tools-extra/clangd/BUILD.gn | 1 + .../gn/secondary/clang-tools-extra/clangd/unittests/BUILD.gn | 1 + 2 files changed, 2 insertions(+) diff --git a/llvm/utils/gn/secondary/clang-tools-extra/clangd/BUILD.gn b/llvm/utils/gn/secondary/clang-tools-extra/clangd/BUILD.gn index 724a3a23f28c4a..7ed1b60f2fe90c 100644 --- a/llvm/utils/gn/secondary/clang-tools-extra/clangd/BUILD.gn +++ b/llvm/utils/gn/secondary/clang-tools-extra/clangd/BUILD.gn @@ -62,6 +62,7 @@ static_library("clangd") { "CompileCommands.cpp", "Compiler.cpp", "Config.cpp", + "ConfigCompile.cpp", "ConfigYAML.cpp", "Diagnostics.cpp", "DraftStore.cpp", diff --git a/llvm/utils/gn/secondary/clang-tools-extra/clangd/unittests/BUILD.gn b/llvm/utils/gn/secondary/clang-tools-extra/clangd/unittests/BUILD.gn index 7a7fc184a36c2f..a4d5434acb33cb 100644 --- a/llvm/utils/gn/secondary/clang-tools-extra/clangd/unittests/BUILD.gn +++ b/llvm/utils/gn/secondary/clang-tools-extra/clangd/unittests/BUILD.gn @@ -41,6 +41,7 @@ unittest("ClangdTests") { "CollectMacrosTests.cpp", "CompileCommandsTests.cpp", "CompilerTests.cpp", + "ConfigCompileTests.cpp", "ConfigYAMLTests.cpp", "DexTests.cpp", "DiagnosticsTests.cpp", From a1aed80a35f3f775cdb1d68c4388723691abc0dd Mon Sep 17 00:00:00 2001 From: Paul Walker Date: Wed, 1 Jul 2020 08:12:46 +0000 Subject: [PATCH 04/14] [SVE] Relax merge requirement for IR based divides. We currently lower SDIV to SDIV_MERGE_OP1. This forces the value for inactive lanes in a way that can hamper register allocation, however, the lowering has no requirement for inactive lanes. Instead this patch replaces SDIV_MERGE_OP1 with SDIV_PRED thus freeing the register allocator. Once done the only user of SDIV_MERGE_OP1 is intrinsic lowering so I've removed the node and perform ISel on the intrinsic directly. This also allows us to implement MOVPRFX based zeroing in the same manner as SUB. This patch also renames UDIV_MERGE_OP1 and [F]ADD_MERGE_OP1 for the same reason but in the ADD cases the ISel code is already as required. Differential Revision: https://reviews.llvm.org/D82783 --- .../Target/AArch64/AArch64ISelLowering.cpp | 22 ++--- llvm/lib/Target/AArch64/AArch64ISelLowering.h | 8 +- .../lib/Target/AArch64/AArch64SVEInstrInfo.td | 29 ++++--- llvm/lib/Target/AArch64/SVEInstrFormats.td | 84 ++++++++++++------- .../CodeGen/AArch64/llvm-ir-to-intrinsic.ll | 8 +- 5 files changed, 87 insertions(+), 64 deletions(-) diff --git a/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp b/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp index 5c97c1b731908a..27b5659e8f66be 100644 --- a/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp +++ b/llvm/lib/Target/AArch64/AArch64ISelLowering.cpp @@ -1356,9 +1356,9 @@ const char *AArch64TargetLowering::getTargetNodeName(unsigned Opcode) const { MAKE_CASE(AArch64ISD::CSINC) MAKE_CASE(AArch64ISD::THREAD_POINTER) MAKE_CASE(AArch64ISD::TLSDESC_CALLSEQ) - MAKE_CASE(AArch64ISD::ADD_MERGE_OP1) - MAKE_CASE(AArch64ISD::SDIV_MERGE_OP1) - MAKE_CASE(AArch64ISD::UDIV_MERGE_OP1) + MAKE_CASE(AArch64ISD::ADD_PRED) + MAKE_CASE(AArch64ISD::SDIV_PRED) + MAKE_CASE(AArch64ISD::UDIV_PRED) MAKE_CASE(AArch64ISD::SMIN_MERGE_OP1) MAKE_CASE(AArch64ISD::UMIN_MERGE_OP1) MAKE_CASE(AArch64ISD::SMAX_MERGE_OP1) @@ -1450,7 +1450,7 @@ const char *AArch64TargetLowering::getTargetNodeName(unsigned Opcode) const { MAKE_CASE(AArch64ISD::REV) MAKE_CASE(AArch64ISD::REINTERPRET_CAST) MAKE_CASE(AArch64ISD::TBL) - MAKE_CASE(AArch64ISD::FADD_MERGE_OP1) + MAKE_CASE(AArch64ISD::FADD_PRED) MAKE_CASE(AArch64ISD::FADDA_PRED) MAKE_CASE(AArch64ISD::FADDV_PRED) MAKE_CASE(AArch64ISD::FMAXV_PRED) @@ -3424,7 +3424,7 @@ SDValue AArch64TargetLowering::LowerOperation(SDValue Op, return LowerXALUO(Op, DAG); case ISD::FADD: if (useSVEForFixedLengthVectorVT(Op.getValueType())) - return LowerToPredicatedOp(Op, DAG, AArch64ISD::FADD_MERGE_OP1); + return LowerToPredicatedOp(Op, DAG, AArch64ISD::FADD_PRED); return LowerF128Call(Op, DAG, RTLIB::ADD_F128); case ISD::FSUB: return LowerF128Call(Op, DAG, RTLIB::SUB_F128); @@ -3458,9 +3458,9 @@ SDValue AArch64TargetLowering::LowerOperation(SDValue Op, case ISD::EXTRACT_SUBVECTOR: return LowerEXTRACT_SUBVECTOR(Op, DAG); case ISD::SDIV: - return LowerToPredicatedOp(Op, DAG, AArch64ISD::SDIV_MERGE_OP1); + return LowerToPredicatedOp(Op, DAG, AArch64ISD::SDIV_PRED); case ISD::UDIV: - return LowerToPredicatedOp(Op, DAG, AArch64ISD::UDIV_MERGE_OP1); + return LowerToPredicatedOp(Op, DAG, AArch64ISD::UDIV_PRED); case ISD::SMIN: return LowerToPredicatedOp(Op, DAG, AArch64ISD::SMIN_MERGE_OP1); case ISD::UMIN: @@ -3530,7 +3530,7 @@ SDValue AArch64TargetLowering::LowerOperation(SDValue Op, llvm_unreachable("Unexpected request to lower ISD::LOAD"); case ISD::ADD: if (useSVEForFixedLengthVectorVT(Op.getValueType())) - return LowerToPredicatedOp(Op, DAG, AArch64ISD::ADD_MERGE_OP1); + return LowerToPredicatedOp(Op, DAG, AArch64ISD::ADD_PRED); llvm_unreachable("Unexpected request to lower ISD::ADD"); } } @@ -11761,12 +11761,6 @@ static SDValue performIntrinsicCombine(SDNode *N, N->getOperand(1)); case Intrinsic::aarch64_sve_ext: return LowerSVEIntrinsicEXT(N, DAG); - case Intrinsic::aarch64_sve_sdiv: - return DAG.getNode(AArch64ISD::SDIV_MERGE_OP1, SDLoc(N), N->getValueType(0), - N->getOperand(1), N->getOperand(2), N->getOperand(3)); - case Intrinsic::aarch64_sve_udiv: - return DAG.getNode(AArch64ISD::UDIV_MERGE_OP1, SDLoc(N), N->getValueType(0), - N->getOperand(1), N->getOperand(2), N->getOperand(3)); case Intrinsic::aarch64_sve_smin: return DAG.getNode(AArch64ISD::SMIN_MERGE_OP1, SDLoc(N), N->getValueType(0), N->getOperand(1), N->getOperand(2), N->getOperand(3)); diff --git a/llvm/lib/Target/AArch64/AArch64ISelLowering.h b/llvm/lib/Target/AArch64/AArch64ISelLowering.h index 74006b4f92d88b..7ab6c3f0257af1 100644 --- a/llvm/lib/Target/AArch64/AArch64ISelLowering.h +++ b/llvm/lib/Target/AArch64/AArch64ISelLowering.h @@ -73,10 +73,10 @@ enum NodeType : unsigned { SBC, // adc, sbc instructions // Arithmetic instructions - ADD_MERGE_OP1, - FADD_MERGE_OP1, - SDIV_MERGE_OP1, - UDIV_MERGE_OP1, + ADD_PRED, + FADD_PRED, + SDIV_PRED, + UDIV_PRED, SMIN_MERGE_OP1, UMIN_MERGE_OP1, SMAX_MERGE_OP1, diff --git a/llvm/lib/Target/AArch64/AArch64SVEInstrInfo.td b/llvm/lib/Target/AArch64/AArch64SVEInstrInfo.td index 0344aad85030c1..c51e9a24d71811 100644 --- a/llvm/lib/Target/AArch64/AArch64SVEInstrInfo.td +++ b/llvm/lib/Target/AArch64/AArch64SVEInstrInfo.td @@ -167,11 +167,13 @@ def SDT_AArch64Arith : SDTypeProfile<1, 3, [ SDTCVecEltisVT<1,i1>, SDTCisSameAs<2,3> ]>; +// Predicated operations with the result of inactive lanes being unspecified. +def AArch64add_p : SDNode<"AArch64ISD::ADD_PRED", SDT_AArch64Arith>; +def AArch64fadd_p : SDNode<"AArch64ISD::FADD_PRED", SDT_AArch64Arith>; +def AArch64sdiv_p : SDNode<"AArch64ISD::SDIV_PRED", SDT_AArch64Arith>; +def AArch64udiv_p : SDNode<"AArch64ISD::UDIV_PRED", SDT_AArch64Arith>; + // Merging op1 into the inactive lanes. -def AArch64add_m1 : SDNode<"AArch64ISD::ADD_MERGE_OP1", SDT_AArch64Arith>; -def AArch64fadd_m1 : SDNode<"AArch64ISD::FADD_MERGE_OP1", SDT_AArch64Arith>; -def AArch64sdiv_m1 : SDNode<"AArch64ISD::SDIV_MERGE_OP1", SDT_AArch64Arith>; -def AArch64udiv_m1 : SDNode<"AArch64ISD::UDIV_MERGE_OP1", SDT_AArch64Arith>; def AArch64smin_m1 : SDNode<"AArch64ISD::SMIN_MERGE_OP1", SDT_AArch64Arith>; def AArch64umin_m1 : SDNode<"AArch64ISD::UMIN_MERGE_OP1", SDT_AArch64Arith>; def AArch64smax_m1 : SDNode<"AArch64ISD::SMAX_MERGE_OP1", SDT_AArch64Arith>; @@ -222,7 +224,9 @@ let Predicates = [HasSVE] in { defm SUB_ZPmZ : sve_int_bin_pred_arit_0<0b001, "sub", "SUB_ZPZZ", int_aarch64_sve_sub, DestructiveBinaryCommWithRev, "SUBR_ZPmZ", 1>; defm SUBR_ZPmZ : sve_int_bin_pred_arit_0<0b011, "subr", "SUBR_ZPZZ", int_aarch64_sve_subr, DestructiveBinaryCommWithRev, "SUB_ZPmZ", 0>; - defm ADD_ZPZZ : sve_int_bin_pred_zx; + defm ADD_ZPZZ : sve_int_bin_pred_bhsd; + + defm ADD_ZPZZ : sve_int_bin_pred_zx; defm SUB_ZPZZ : sve_int_bin_pred_zx; defm SUBR_ZPZZ : sve_int_bin_pred_zx; @@ -279,10 +283,13 @@ let Predicates = [HasSVE] in { def : Pat<(mul nxv2i64:$Op1, nxv2i64:$Op2), (MUL_ZPmZ_D (PTRUE_D 31), $Op1, $Op2)>; - defm SDIV_ZPmZ : sve_int_bin_pred_arit_2_div<0b100, "sdiv", AArch64sdiv_m1>; - defm UDIV_ZPmZ : sve_int_bin_pred_arit_2_div<0b101, "udiv", AArch64udiv_m1>; - defm SDIVR_ZPmZ : sve_int_bin_pred_arit_2_div<0b110, "sdivr", int_aarch64_sve_sdivr>; - defm UDIVR_ZPmZ : sve_int_bin_pred_arit_2_div<0b111, "udivr", int_aarch64_sve_udivr>; + defm SDIV_ZPmZ : sve_int_bin_pred_arit_2_div<0b100, "sdiv", "SDIV_ZPZZ", int_aarch64_sve_sdiv, DestructiveBinaryCommWithRev, "SDIVR_ZPmZ", 1>; + defm UDIV_ZPmZ : sve_int_bin_pred_arit_2_div<0b101, "udiv", "UDIV_ZPZZ", int_aarch64_sve_udiv, DestructiveBinaryCommWithRev, "UDIVR_ZPmZ", 1>; + defm SDIVR_ZPmZ : sve_int_bin_pred_arit_2_div<0b110, "sdivr", "SDIVR_ZPZZ", int_aarch64_sve_sdivr, DestructiveBinaryCommWithRev, "SDIV_ZPmZ", 0>; + defm UDIVR_ZPmZ : sve_int_bin_pred_arit_2_div<0b111, "udivr", "UDIVR_ZPZZ", int_aarch64_sve_udivr, DestructiveBinaryCommWithRev, "UDIV_ZPmZ", 0>; + + defm SDIV_ZPZZ : sve_int_bin_pred_sd; + defm UDIV_ZPZZ : sve_int_bin_pred_sd; defm SDOT_ZZZ : sve_intx_dot<0b0, "sdot", int_aarch64_sve_sdot>; defm UDOT_ZZZ : sve_intx_dot<0b1, "udot", int_aarch64_sve_udot>; @@ -345,7 +352,9 @@ let Predicates = [HasSVE] in { defm FDIVR_ZPmZ : sve_fp_2op_p_zds<0b1100, "fdivr", "FDIVR_ZPZZ", int_aarch64_sve_fdivr, DestructiveBinaryCommWithRev, "FDIV_ZPmZ", 0>; defm FDIV_ZPmZ : sve_fp_2op_p_zds<0b1101, "fdiv", "FDIV_ZPZZ", int_aarch64_sve_fdiv, DestructiveBinaryCommWithRev, "FDIVR_ZPmZ", 1>; - defm FADD_ZPZZ : sve_fp_2op_p_zds_zx; + defm FADD_ZPZZ : sve_fp_bin_pred_hfd; + + defm FADD_ZPZZ : sve_fp_2op_p_zds_zx; defm FSUB_ZPZZ : sve_fp_2op_p_zds_zx; defm FMUL_ZPZZ : sve_fp_2op_p_zds_zx; defm FSUBR_ZPZZ : sve_fp_2op_p_zds_zx; diff --git a/llvm/lib/Target/AArch64/SVEInstrFormats.td b/llvm/lib/Target/AArch64/SVEInstrFormats.td index 7fc1c416f8a2e3..305cc6915ad97d 100644 --- a/llvm/lib/Target/AArch64/SVEInstrFormats.td +++ b/llvm/lib/Target/AArch64/SVEInstrFormats.td @@ -1596,23 +1596,14 @@ multiclass sve_fp_2op_p_zds_fscale opc, string asm, def : SVE_3_Op_Pat(NAME # _D)>; } -multiclass sve_fp_2op_p_zds_zx { - def _UNDEF_H : PredTwoOpPseudo; - def _UNDEF_S : PredTwoOpPseudo; - def _UNDEF_D : PredTwoOpPseudo; - +multiclass sve_fp_2op_p_zds_zx { def _ZERO_H : PredTwoOpPseudo; def _ZERO_S : PredTwoOpPseudo; def _ZERO_D : PredTwoOpPseudo; - def : SVE_3_Op_Pat(NAME # _UNDEF_H)>; - def : SVE_3_Op_Pat(NAME # _UNDEF_S)>; - def : SVE_3_Op_Pat(NAME # _UNDEF_D)>; - - def : SVE_3_Op_Pat_SelZero(NAME # _ZERO_H)>; - def : SVE_3_Op_Pat_SelZero(NAME # _ZERO_S)>; - def : SVE_3_Op_Pat_SelZero(NAME # _ZERO_D)>; + def : SVE_3_Op_Pat_SelZero(NAME # _ZERO_H)>; + def : SVE_3_Op_Pat_SelZero(NAME # _ZERO_S)>; + def : SVE_3_Op_Pat_SelZero(NAME # _ZERO_D)>; } class sve_fp_ftmad sz, string asm, ZPRRegOp zprty> @@ -2404,9 +2395,16 @@ multiclass sve_int_bin_pred_arit_2 opc, string asm, SDPatternOperator op } // Special case for divides which are not defined for 8b/16b elements. -multiclass sve_int_bin_pred_arit_2_div opc, string asm, SDPatternOperator op> { - def _S : sve_int_bin_pred_arit_log<0b10, 0b10, opc, asm, ZPR32>; - def _D : sve_int_bin_pred_arit_log<0b11, 0b10, opc, asm, ZPR64>; +multiclass sve_int_bin_pred_arit_2_div opc, string asm, string Ps, + SDPatternOperator op, + DestructiveInstTypeEnum flags, + string revname="", bit isOrig=0> { + let DestructiveInstType = flags in { + def _S : sve_int_bin_pred_arit_log<0b10, 0b10, opc, asm, ZPR32>, + SVEPseudo2Instr, SVEInstr2Rev; + def _D : sve_int_bin_pred_arit_log<0b11, 0b10, opc, asm, ZPR64>, + SVEPseudo2Instr, SVEInstr2Rev; + } def : SVE_3_Op_Pat(NAME # _S)>; def : SVE_3_Op_Pat(NAME # _D)>; @@ -4865,27 +4863,16 @@ multiclass sve_int_bin_pred_shift opc, string asm, string Ps, def : SVE_3_Op_Pat(NAME # _D)>; } -multiclass sve_int_bin_pred_zx { - def _UNDEF_B : PredTwoOpPseudo; - def _UNDEF_H : PredTwoOpPseudo; - def _UNDEF_S : PredTwoOpPseudo; - def _UNDEF_D : PredTwoOpPseudo; - +multiclass sve_int_bin_pred_zx { def _ZERO_B : PredTwoOpPseudo; def _ZERO_H : PredTwoOpPseudo; def _ZERO_S : PredTwoOpPseudo; def _ZERO_D : PredTwoOpPseudo; - def : SVE_3_Op_Pat(NAME # _UNDEF_B)>; - def : SVE_3_Op_Pat(NAME # _UNDEF_H)>; - def : SVE_3_Op_Pat(NAME # _UNDEF_S)>; - def : SVE_3_Op_Pat(NAME # _UNDEF_D)>; - - def : SVE_3_Op_Pat_SelZero(NAME # _ZERO_B)>; - def : SVE_3_Op_Pat_SelZero(NAME # _ZERO_H)>; - def : SVE_3_Op_Pat_SelZero(NAME # _ZERO_S)>; - def : SVE_3_Op_Pat_SelZero(NAME # _ZERO_D)>; + def : SVE_3_Op_Pat_SelZero(NAME # _ZERO_B)>; + def : SVE_3_Op_Pat_SelZero(NAME # _ZERO_H)>; + def : SVE_3_Op_Pat_SelZero(NAME # _ZERO_S)>; + def : SVE_3_Op_Pat_SelZero(NAME # _ZERO_D)>; } multiclass sve_int_bin_pred_shift_wide opc, string asm, @@ -7810,3 +7797,36 @@ def am_sve_regreg_lsl0 : ComplexPattern", [] def am_sve_regreg_lsl1 : ComplexPattern", []>; def am_sve_regreg_lsl2 : ComplexPattern", []>; def am_sve_regreg_lsl3 : ComplexPattern", []>; + +// Predicated pseudo floating point two operand instructions. +multiclass sve_fp_bin_pred_hfd { + def _UNDEF_H : PredTwoOpPseudo; + def _UNDEF_S : PredTwoOpPseudo; + def _UNDEF_D : PredTwoOpPseudo; + + def : SVE_3_Op_Pat(NAME # _UNDEF_H)>; + def : SVE_3_Op_Pat(NAME # _UNDEF_S)>; + def : SVE_3_Op_Pat(NAME # _UNDEF_D)>; +} + +// Predicated pseudo integer two operand instructions. +multiclass sve_int_bin_pred_bhsd { + def _UNDEF_B : PredTwoOpPseudo; + def _UNDEF_H : PredTwoOpPseudo; + def _UNDEF_S : PredTwoOpPseudo; + def _UNDEF_D : PredTwoOpPseudo; + + def : SVE_3_Op_Pat(NAME # _UNDEF_B)>; + def : SVE_3_Op_Pat(NAME # _UNDEF_H)>; + def : SVE_3_Op_Pat(NAME # _UNDEF_S)>; + def : SVE_3_Op_Pat(NAME # _UNDEF_D)>; +} + +// As sve_int_bin_pred but when only i32 and i64 vector types are required. +multiclass sve_int_bin_pred_sd { + def _UNDEF_S : PredTwoOpPseudo; + def _UNDEF_D : PredTwoOpPseudo; + + def : SVE_3_Op_Pat(NAME # _UNDEF_S)>; + def : SVE_3_Op_Pat(NAME # _UNDEF_D)>; +} diff --git a/llvm/test/CodeGen/AArch64/llvm-ir-to-intrinsic.ll b/llvm/test/CodeGen/AArch64/llvm-ir-to-intrinsic.ll index bc4778d6600444..816465f9eaa17a 100644 --- a/llvm/test/CodeGen/AArch64/llvm-ir-to-intrinsic.ll +++ b/llvm/test/CodeGen/AArch64/llvm-ir-to-intrinsic.ll @@ -67,7 +67,7 @@ define @srem_i32( %a, %b ; CHECK-LABEL: srem_i32: ; CHECK: // %bb.0: ; CHECK-NEXT: ptrue p0.s -; CHECK-NEXT: mov z2.d, z0.d +; CHECK-NEXT: movprfx z2, z0 ; CHECK-NEXT: sdiv z2.s, p0/m, z2.s, z1.s ; CHECK-NEXT: mul z2.s, p0/m, z2.s, z1.s ; CHECK-NEXT: sub z0.s, z0.s, z2.s @@ -80,7 +80,7 @@ define @srem_i64( %a, %b ; CHECK-LABEL: srem_i64: ; CHECK: // %bb.0: ; CHECK-NEXT: ptrue p0.d -; CHECK-NEXT: mov z2.d, z0.d +; CHECK-NEXT: movprfx z2, z0 ; CHECK-NEXT: sdiv z2.d, p0/m, z2.d, z1.d ; CHECK-NEXT: mul z2.d, p0/m, z2.d, z1.d ; CHECK-NEXT: sub z0.d, z0.d, z2.d @@ -156,7 +156,7 @@ define @urem_i32( %a, %b ; CHECK-LABEL: urem_i32: ; CHECK: // %bb.0: ; CHECK-NEXT: ptrue p0.s -; CHECK-NEXT: mov z2.d, z0.d +; CHECK-NEXT: movprfx z2, z0 ; CHECK-NEXT: udiv z2.s, p0/m, z2.s, z1.s ; CHECK-NEXT: mul z2.s, p0/m, z2.s, z1.s ; CHECK-NEXT: sub z0.s, z0.s, z2.s @@ -169,7 +169,7 @@ define @urem_i64( %a, %b ; CHECK-LABEL: urem_i64: ; CHECK: // %bb.0: ; CHECK-NEXT: ptrue p0.d -; CHECK-NEXT: mov z2.d, z0.d +; CHECK-NEXT: movprfx z2, z0 ; CHECK-NEXT: udiv z2.d, p0/m, z2.d, z1.d ; CHECK-NEXT: mul z2.d, p0/m, z2.d, z1.d ; CHECK-NEXT: sub z0.d, z0.d, z2.d From 76b2d9cbebd227d42e2099a0eb89c800b945997a Mon Sep 17 00:00:00 2001 From: Tony Date: Wed, 1 Jul 2020 08:17:28 +0000 Subject: [PATCH 05/14] [AMDGPU] Correct AMDGPUUsage.rst DW_AT_LLVM_lane_pc example - Correct typo of DW_OP_xaddr to DW_OP_addrx in AMDGPUUsage.rst for DW_AT_LLVM_lane_pc example. Change-Id: I1b0ee2b24362a0240388e4c2f044c1d4883509b9 --- llvm/docs/AMDGPUUsage.rst | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/llvm/docs/AMDGPUUsage.rst b/llvm/docs/AMDGPUUsage.rst index fcc4e996c30c3b..36930fc253ec03 100644 --- a/llvm/docs/AMDGPUUsage.rst +++ b/llvm/docs/AMDGPUUsage.rst @@ -1656,7 +1656,7 @@ The following provides an example using pseudo LLVM MIR. DW_AT_name = "__divergent_lane_pc_1_then"; DW_AT_location = DIExpression[ DW_OP_call_ref %__divergent_lane_pc; - DW_OP_xaddr &lex_1_start; + DW_OP_addrx &lex_1_start; DW_OP_stack_value; DW_OP_LLVM_extend 64, 64; DW_OP_call_ref %__lex_1_save_exec; @@ -1679,7 +1679,7 @@ The following provides an example using pseudo LLVM MIR. DW_AT_name = "__divergent_lane_pc_1_1_then"; DW_AT_location = DIExpression[ DW_OP_call_ref %__divergent_lane_pc_1_then; - DW_OP_xaddr &lex_1_1_start; + DW_OP_addrx &lex_1_1_start; DW_OP_stack_value; DW_OP_LLVM_extend 64, 64; DW_OP_call_ref %__lex_1_1_save_exec; @@ -1698,7 +1698,7 @@ The following provides an example using pseudo LLVM MIR. DW_AT_name = "__divergent_lane_pc_1_1_else"; DW_AT_location = DIExpression[ DW_OP_call_ref %__divergent_lane_pc_1_then; - DW_OP_xaddr &lex_1_1_end; + DW_OP_addrx &lex_1_1_end; DW_OP_stack_value; DW_OP_LLVM_extend 64, 64; DW_OP_call_ref %__lex_1_1_save_exec; @@ -1724,7 +1724,7 @@ The following provides an example using pseudo LLVM MIR. DW_AT_name = "__divergent_lane_pc_1_else"; DW_AT_location = DIExpression[ DW_OP_call_ref %__divergent_lane_pc; - DW_OP_xaddr &lex_1_end; + DW_OP_addrx &lex_1_end; DW_OP_stack_value; DW_OP_LLVM_extend 64, 64; DW_OP_call_ref %__lex_1_save_exec; From f0ecfb789bb2d3de57876927e03a5c26da8419c8 Mon Sep 17 00:00:00 2001 From: Sam Parker Date: Wed, 1 Jul 2020 09:20:25 +0100 Subject: [PATCH 06/14] [NFC][ARM] Add test. --- .../varying-outer-2d-reduction.ll | 173 ++++++++++++++++++ 1 file changed, 173 insertions(+) create mode 100644 llvm/test/CodeGen/Thumb2/LowOverheadLoops/varying-outer-2d-reduction.ll diff --git a/llvm/test/CodeGen/Thumb2/LowOverheadLoops/varying-outer-2d-reduction.ll b/llvm/test/CodeGen/Thumb2/LowOverheadLoops/varying-outer-2d-reduction.ll new file mode 100644 index 00000000000000..f1242db3648512 --- /dev/null +++ b/llvm/test/CodeGen/Thumb2/LowOverheadLoops/varying-outer-2d-reduction.ll @@ -0,0 +1,173 @@ +; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py +; RUN: llc -mtriple=thumbv8.1m.main -mattr=+mve -disable-mve-tail-predication=false %s -o - | FileCheck %s + +define dso_local void @varying_outer_2d_reduction(i16* nocapture readonly %Input, i16* nocapture %Output, i16 signext %Size, i16 signext %N, i16 signext %Scale) local_unnamed_addr { +; CHECK-LABEL: varying_outer_2d_reduction: +; CHECK: @ %bb.0: @ %entry +; CHECK-NEXT: push.w {r4, r5, r6, r7, r8, r9, r10, r11, lr} +; CHECK-NEXT: sub sp, #8 +; CHECK-NEXT: cmp r3, #1 +; CHECK-NEXT: str r0, [sp, #4] @ 4-byte Spill +; CHECK-NEXT: blt .LBB0_8 +; CHECK-NEXT: @ %bb.1: @ %for.body.lr.ph +; CHECK-NEXT: ldr r0, [sp, #44] +; CHECK-NEXT: adr r7, .LCPI0_0 +; CHECK-NEXT: ldr.w r10, [sp, #4] @ 4-byte Reload +; CHECK-NEXT: add.w r9, r2, #3 +; CHECK-NEXT: vldrw.u32 q0, [r7] +; CHECK-NEXT: mov.w r11, #0 +; CHECK-NEXT: uxth r0, r0 +; CHECK-NEXT: rsbs r5, r0, #0 +; CHECK-NEXT: str.w r9, [sp] @ 4-byte Spill +; CHECK-NEXT: b .LBB0_4 +; CHECK-NEXT: .LBB0_2: @ in Loop: Header=BB0_4 Depth=1 +; CHECK-NEXT: movs r0, #0 +; CHECK-NEXT: .LBB0_3: @ %for.end +; CHECK-NEXT: @ in Loop: Header=BB0_4 Depth=1 +; CHECK-NEXT: lsrs r0, r0, #16 +; CHECK-NEXT: sub.w r9, r9, #1 +; CHECK-NEXT: strh.w r0, [r1, r11, lsl #1] +; CHECK-NEXT: add.w r11, r11, #1 +; CHECK-NEXT: add.w r10, r10, #2 +; CHECK-NEXT: cmp r11, r3 +; CHECK-NEXT: beq .LBB0_8 +; CHECK-NEXT: .LBB0_4: @ %for.body +; CHECK-NEXT: @ =>This Loop Header: Depth=1 +; CHECK-NEXT: @ Child Loop BB0_6 Depth 2 +; CHECK-NEXT: cmp r2, r11 +; CHECK-NEXT: ble .LBB0_2 +; CHECK-NEXT: @ %bb.5: @ %vector.ph +; CHECK-NEXT: @ in Loop: Header=BB0_4 Depth=1 +; CHECK-NEXT: bic r7, r9, #3 +; CHECK-NEXT: movs r6, #1 +; CHECK-NEXT: subs r7, #4 +; CHECK-NEXT: sub.w r0, r2, r11 +; CHECK-NEXT: vmov.i32 q2, #0x0 +; CHECK-NEXT: add.w r8, r6, r7, lsr #2 +; CHECK-NEXT: ldr r7, [sp] @ 4-byte Reload +; CHECK-NEXT: sub.w r4, r7, r11 +; CHECK-NEXT: movs r7, #0 +; CHECK-NEXT: bic r4, r4, #3 +; CHECK-NEXT: subs r4, #4 +; CHECK-NEXT: add.w r4, r6, r4, lsr #2 +; CHECK-NEXT: subs r6, r0, #1 +; CHECK-NEXT: dls lr, r4 +; CHECK-NEXT: mov r4, r10 +; CHECK-NEXT: ldr r0, [sp, #4] @ 4-byte Reload +; CHECK-NEXT: .LBB0_6: @ %vector.body +; CHECK-NEXT: @ Parent Loop BB0_4 Depth=1 +; CHECK-NEXT: @ => This Inner Loop Header: Depth=2 +; CHECK-NEXT: vmov q1, q2 +; CHECK-NEXT: vadd.i32 q2, q0, r7 +; CHECK-NEXT: vdup.32 q3, r7 +; CHECK-NEXT: mov lr, r8 +; CHECK-NEXT: vcmp.u32 hi, q3, q2 +; CHECK-NEXT: vdup.32 q3, r6 +; CHECK-NEXT: vpnot +; CHECK-NEXT: sub.w r8, r8, #1 +; CHECK-NEXT: vpsttt +; CHECK-NEXT: vcmpt.u32 cs, q3, q2 +; CHECK-NEXT: vldrht.s32 q2, [r0], #8 +; CHECK-NEXT: vldrht.s32 q3, [r4], #8 +; CHECK-NEXT: adds r7, #4 +; CHECK-NEXT: vmul.i32 q2, q3, q2 +; CHECK-NEXT: vshl.s32 q2, r5 +; CHECK-NEXT: vadd.i32 q2, q2, q1 +; CHECK-NEXT: le lr, .LBB0_6 +; CHECK-NEXT: @ %bb.7: @ %middle.block +; CHECK-NEXT: @ in Loop: Header=BB0_4 Depth=1 +; CHECK-NEXT: vpsel q1, q2, q1 +; CHECK-NEXT: vaddv.u32 r0, q1 +; CHECK-NEXT: b .LBB0_3 +; CHECK-NEXT: .LBB0_8: @ %for.end17 +; CHECK-NEXT: add sp, #8 +; CHECK-NEXT: pop.w {r4, r5, r6, r7, r8, r9, r10, r11, pc} +; CHECK-NEXT: .p2align 4 +; CHECK-NEXT: @ %bb.9: +; CHECK-NEXT: .LCPI0_0: +; CHECK-NEXT: .long 0 @ 0x0 +; CHECK-NEXT: .long 1 @ 0x1 +; CHECK-NEXT: .long 2 @ 0x2 +; CHECK-NEXT: .long 3 @ 0x3 +entry: + %conv = sext i16 %N to i32 + %cmp36 = icmp sgt i16 %N, 0 + br i1 %cmp36, label %for.body.lr.ph, label %for.end17 + +for.body.lr.ph: ; preds = %entry + %conv2 = sext i16 %Size to i32 + %conv1032 = zext i16 %Scale to i32 + %i = add i32 %conv2, 3 + br label %for.body + +for.body: ; preds = %for.end, %for.body.lr.ph + %lsr.iv51 = phi i32 [ %lsr.iv.next, %for.end ], [ %i, %for.body.lr.ph ] + %lsr.iv46 = phi i16* [ %scevgep47, %for.end ], [ %Input, %for.body.lr.ph ] + %i.037 = phi i32 [ 0, %for.body.lr.ph ], [ %inc16, %for.end ] + %i1 = mul nsw i32 %i.037, -1 + %i2 = add i32 %i, %i1 + %i3 = lshr i32 %i2, 2 + %i4 = shl nuw i32 %i3, 2 + %i5 = add i32 %i4, -4 + %i6 = lshr i32 %i5, 2 + %i7 = add nuw nsw i32 %i6, 1 + %i8 = sub i32 %conv2, %i.037 + %cmp433 = icmp slt i32 %i.037, %conv2 + br i1 %cmp433, label %vector.ph, label %for.end + +vector.ph: ; preds = %for.body + %trip.count.minus.1 = add i32 %i8, -1 + call void @llvm.set.loop.iterations.i32(i32 %i7) + br label %vector.body + +vector.body: ; preds = %vector.body, %vector.ph + %lsr.iv48 = phi i16* [ %scevgep49, %vector.body ], [ %lsr.iv46, %vector.ph ] + %lsr.iv = phi i16* [ %scevgep, %vector.body ], [ %Input, %vector.ph ] + %index = phi i32 [ 0, %vector.ph ], [ %index.next, %vector.body ] + %vec.phi = phi <4 x i32> [ zeroinitializer, %vector.ph ], [ %i16, %vector.body ] + %i9 = phi i32 [ %i7, %vector.ph ], [ %i17, %vector.body ] + %lsr.iv4850 = bitcast i16* %lsr.iv48 to <4 x i16>* + %lsr.iv45 = bitcast i16* %lsr.iv to <4 x i16>* + %active.lane.mask = call <4 x i1> @llvm.get.active.lane.mask.v4i1.i32(i32 %index, i32 %trip.count.minus.1) + %wide.masked.load = call <4 x i16> @llvm.masked.load.v4i16.p0v4i16(<4 x i16>* %lsr.iv45, i32 2, <4 x i1> %active.lane.mask, <4 x i16> undef) + %i10 = sext <4 x i16> %wide.masked.load to <4 x i32> + %wide.masked.load42 = call <4 x i16> @llvm.masked.load.v4i16.p0v4i16(<4 x i16>* %lsr.iv4850, i32 2, <4 x i1> %active.lane.mask, <4 x i16> undef) + %i11 = sext <4 x i16> %wide.masked.load42 to <4 x i32> + %i12 = mul nsw <4 x i32> %i11, %i10 + %i13 = insertelement <4 x i32> undef, i32 %conv1032, i32 0 + %i14 = shufflevector <4 x i32> %i13, <4 x i32> undef, <4 x i32> zeroinitializer + %i15 = ashr <4 x i32> %i12, %i14 + %i16 = add <4 x i32> %i15, %vec.phi + %index.next = add i32 %index, 4 + %scevgep = getelementptr i16, i16* %lsr.iv, i32 4 + %scevgep49 = getelementptr i16, i16* %lsr.iv48, i32 4 + %i17 = call i32 @llvm.loop.decrement.reg.i32(i32 %i9, i32 1) + %i18 = icmp ne i32 %i17, 0 + br i1 %i18, label %vector.body, label %middle.block + +middle.block: ; preds = %vector.body + %i19 = select <4 x i1> %active.lane.mask, <4 x i32> %i16, <4 x i32> %vec.phi + %i20 = call i32 @llvm.experimental.vector.reduce.add.v4i32(<4 x i32> %i19) + br label %for.end + +for.end: ; preds = %middle.block, %for.body + %Sum.0.lcssa = phi i32 [ 0, %for.body ], [ %i20, %middle.block ] + %i21 = lshr i32 %Sum.0.lcssa, 16 + %conv13 = trunc i32 %i21 to i16 + %arrayidx14 = getelementptr inbounds i16, i16* %Output, i32 %i.037 + store i16 %conv13, i16* %arrayidx14, align 2 + %inc16 = add nuw nsw i32 %i.037, 1 + %scevgep47 = getelementptr i16, i16* %lsr.iv46, i32 1 + %lsr.iv.next = add i32 %lsr.iv51, -1 + %exitcond39 = icmp eq i32 %inc16, %conv + br i1 %exitcond39, label %for.end17, label %for.body + +for.end17: ; preds = %for.end, %entry + ret void +} + +declare <4 x i1> @llvm.get.active.lane.mask.v4i1.i32(i32, i32) +declare <4 x i16> @llvm.masked.load.v4i16.p0v4i16(<4 x i16>*, i32 immarg, <4 x i1>, <4 x i16>) +declare i32 @llvm.experimental.vector.reduce.add.v4i32(<4 x i32>) +declare i32 @llvm.loop.decrement.reg.i32(i32, i32) +declare void @llvm.set.loop.iterations.i32(i32) From 8270a903baf55122289499ba00a979e9c04dcd44 Mon Sep 17 00:00:00 2001 From: Pavel Labath Date: Wed, 1 Jul 2020 10:18:02 +0200 Subject: [PATCH 07/14] [lldb] Scalar re-fix UB in float->int conversions The refactor in 48ca15592f1 reintroduced UB when converting out-of-bounds floating point numbers to integers -- the behavior for ULongLong() was originally fixed in r341685, but did not survive my refactor because I based my template code on one of the methods which did not have this fix. This time, I apply the fix to all float->int conversions, instead of just the "double->unsigned long long" case. I also use a slightly simpler version of the code, with fewer round-trips (APFloat->APSInt->native_int vs APFloat->native_float->APInt->native_int). I also add some unit tests for the conversions. --- lldb/source/Utility/Scalar.cpp | 76 +++++++-------------------- lldb/unittests/Utility/ScalarTest.cpp | 38 +++++++++----- 2 files changed, 45 insertions(+), 69 deletions(-) diff --git a/lldb/source/Utility/Scalar.cpp b/lldb/source/Utility/Scalar.cpp index d275f6211e5c3c..7d0b7178ddd26f 100644 --- a/lldb/source/Utility/Scalar.cpp +++ b/lldb/source/Utility/Scalar.cpp @@ -14,7 +14,7 @@ #include "lldb/Utility/Stream.h" #include "lldb/Utility/StreamString.h" #include "lldb/lldb-types.h" - +#include "llvm/ADT/APSInt.h" #include "llvm/ADT/SmallString.h" #include @@ -645,60 +645,34 @@ bool Scalar::MakeUnsigned() { } template T Scalar::GetAsSigned(T fail_value) const { - switch (m_type) { - case e_void: + switch (GetCategory(m_type)) { + case Category::Void: break; - case e_sint: - case e_uint: - case e_slong: - case e_ulong: - case e_slonglong: - case e_ulonglong: - case e_sint128: - case e_uint128: - case e_sint256: - case e_uint256: - case e_sint512: - case e_uint512: + case Category::Integral: return m_integer.sextOrTrunc(sizeof(T) * 8).getSExtValue(); - case e_float: - return static_cast(m_float.convertToFloat()); - case e_double: - return static_cast(m_float.convertToDouble()); - case e_long_double: - llvm::APInt ldbl_val = m_float.bitcastToAPInt(); - return static_cast( - (ldbl_val.sextOrTrunc(sizeof(schar_t) * 8)).getSExtValue()); + case Category::Float: { + llvm::APSInt result(sizeof(T) * 8, /*isUnsigned=*/false); + bool isExact; + m_float.convertToInteger(result, llvm::APFloat::rmTowardZero, &isExact); + return result.getSExtValue(); + } } return fail_value; } template T Scalar::GetAsUnsigned(T fail_value) const { - switch (m_type) { - case e_void: + switch (GetCategory(m_type)) { + case Category::Void: break; - case e_sint: - case e_uint: - case e_slong: - case e_ulong: - case e_slonglong: - case e_ulonglong: - case e_sint128: - case e_uint128: - case e_sint256: - case e_uint256: - case e_sint512: - case e_uint512: + case Category::Integral: return m_integer.zextOrTrunc(sizeof(T) * 8).getZExtValue(); - - case e_float: - return static_cast(m_float.convertToFloat()); - case e_double: - return static_cast(m_float.convertToDouble()); - case e_long_double: - llvm::APInt ldbl_val = m_float.bitcastToAPInt(); - return static_cast((ldbl_val.zextOrTrunc(sizeof(T) * 8)).getZExtValue()); + case Category::Float: { + llvm::APSInt result(sizeof(T) * 8, /*isUnsigned=*/true); + bool isExact; + m_float.convertToInteger(result, llvm::APFloat::rmTowardZero, &isExact); + return result.getZExtValue(); + } } return fail_value; } @@ -736,17 +710,7 @@ long long Scalar::SLongLong(long long fail_value) const { } unsigned long long Scalar::ULongLong(unsigned long long fail_value) const { - switch (m_type) { - case e_double: { - double d_val = m_float.convertToDouble(); - llvm::APInt rounded_double = - llvm::APIntOps::RoundDoubleToAPInt(d_val, sizeof(ulonglong_t) * 8); - return static_cast( - (rounded_double.zextOrTrunc(sizeof(ulonglong_t) * 8)).getZExtValue()); - } - default: - return GetAsUnsigned(fail_value); - } + return GetAsUnsigned(fail_value); } llvm::APInt Scalar::SInt128(const llvm::APInt &fail_value) const { diff --git a/lldb/unittests/Utility/ScalarTest.cpp b/lldb/unittests/Utility/ScalarTest.cpp index 960161c01bac90..64b3c9e2bc45fc 100644 --- a/lldb/unittests/Utility/ScalarTest.cpp +++ b/lldb/unittests/Utility/ScalarTest.cpp @@ -119,22 +119,34 @@ TEST(ScalarTest, GetBytes) { TEST(ScalarTest, CastOperations) { long long a = 0xf1f2f3f4f5f6f7f8LL; Scalar a_scalar(a); - ASSERT_EQ((signed char)a, a_scalar.SChar()); - ASSERT_EQ((unsigned char)a, a_scalar.UChar()); - ASSERT_EQ((signed short)a, a_scalar.SShort()); - ASSERT_EQ((unsigned short)a, a_scalar.UShort()); - ASSERT_EQ((signed int)a, a_scalar.SInt()); - ASSERT_EQ((unsigned int)a, a_scalar.UInt()); - ASSERT_EQ((signed long)a, a_scalar.SLong()); - ASSERT_EQ((unsigned long)a, a_scalar.ULong()); - ASSERT_EQ((signed long long)a, a_scalar.SLongLong()); - ASSERT_EQ((unsigned long long)a, a_scalar.ULongLong()); + EXPECT_EQ((signed char)a, a_scalar.SChar()); + EXPECT_EQ((unsigned char)a, a_scalar.UChar()); + EXPECT_EQ((signed short)a, a_scalar.SShort()); + EXPECT_EQ((unsigned short)a, a_scalar.UShort()); + EXPECT_EQ((signed int)a, a_scalar.SInt()); + EXPECT_EQ((unsigned int)a, a_scalar.UInt()); + EXPECT_EQ((signed long)a, a_scalar.SLong()); + EXPECT_EQ((unsigned long)a, a_scalar.ULong()); + EXPECT_EQ((signed long long)a, a_scalar.SLongLong()); + EXPECT_EQ((unsigned long long)a, a_scalar.ULongLong()); int a2 = 23; Scalar a2_scalar(a2); - ASSERT_EQ((float)a2, a2_scalar.Float()); - ASSERT_EQ((double)a2, a2_scalar.Double()); - ASSERT_EQ((long double)a2, a2_scalar.LongDouble()); + EXPECT_EQ((float)a2, a2_scalar.Float()); + EXPECT_EQ((double)a2, a2_scalar.Double()); + EXPECT_EQ((long double)a2, a2_scalar.LongDouble()); + + EXPECT_EQ(std::numeric_limits::min(), Scalar(-1.0f).UInt()); + EXPECT_EQ(std::numeric_limits::max(), Scalar(1e11f).UInt()); + EXPECT_EQ(std::numeric_limits::min(), + Scalar(-1.0).ULongLong()); + EXPECT_EQ(std::numeric_limits::max(), + Scalar(1e22).ULongLong()); + + EXPECT_EQ(std::numeric_limits::min(), Scalar(-1e11f).SInt()); + EXPECT_EQ(std::numeric_limits::max(), Scalar(1e11f).SInt()); + EXPECT_EQ(std::numeric_limits::min(), Scalar(-1e22).SLongLong()); + EXPECT_EQ(std::numeric_limits::max(), Scalar(1e22).SLongLong()); } TEST(ScalarTest, ExtractBitfield) { From 7f37d8830635bf119a5f630dd3958c8f45780805 Mon Sep 17 00:00:00 2001 From: Guillaume Chatelet Date: Wed, 1 Jul 2020 08:49:28 +0000 Subject: [PATCH 08/14] [Alignment][NFC] Migrate MachineFrameInfo::CreateSpillStackObject to Align iThis patch is part of a series to introduce an Alignment type. See this thread for context: http://lists.llvm.org/pipermail/llvm-dev/2019-July/133851.html See this patch for the introduction of the type: https://reviews.llvm.org/D64790 Differential Revision: https://reviews.llvm.org/D82934 --- llvm/include/llvm/CodeGen/MachineFrameInfo.h | 5 +++-- llvm/lib/CodeGen/FixupStatepointCallerSaved.cpp | 2 +- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/llvm/include/llvm/CodeGen/MachineFrameInfo.h b/llvm/include/llvm/CodeGen/MachineFrameInfo.h index d0a8f04f543a04..cd6eafb2b5716f 100644 --- a/llvm/include/llvm/CodeGen/MachineFrameInfo.h +++ b/llvm/include/llvm/CodeGen/MachineFrameInfo.h @@ -768,8 +768,9 @@ class MachineFrameInfo { /// Create a new statically sized stack object that represents a spill slot, /// returning a nonnegative identifier to represent it. int CreateSpillStackObject(uint64_t Size, Align Alignment); - /// FIXME: Remove this function when transition to Align is over. - inline int CreateSpillStackObject(uint64_t Size, unsigned Alignment) { + LLVM_ATTRIBUTE_DEPRECATED( + inline int CreateSpillStackObject(uint64_t Size, unsigned Alignment), + "Use CreateSpillStackObject that takes an Align instead") { return CreateSpillStackObject(Size, assumeAligned(Alignment)); } diff --git a/llvm/lib/CodeGen/FixupStatepointCallerSaved.cpp b/llvm/lib/CodeGen/FixupStatepointCallerSaved.cpp index 0c8d5218bab54f..27319804049de4 100644 --- a/llvm/lib/CodeGen/FixupStatepointCallerSaved.cpp +++ b/llvm/lib/CodeGen/FixupStatepointCallerSaved.cpp @@ -132,7 +132,7 @@ class FrameIndexesCache { } return FI; } - int FI = MFI.CreateSpillStackObject(Size, Size); + int FI = MFI.CreateSpillStackObject(Size, Align(Size)); NumSpillSlotsAllocated++; Line.Slots.push_back(FI); ++Line.Index; From 85460c4ea273784dd45da558ad9a6f13a79b2d91 Mon Sep 17 00:00:00 2001 From: David Stenberg Date: Wed, 1 Jul 2020 09:45:56 +0200 Subject: [PATCH 09/14] [DebugInfo] Do not emit entry values for composite locations Summary: This is a fix for PR45009. When working on D67492 I made DwarfExpression emit a single DW_OP_entry_value operation covering the whole composite location description that is produced if a register does not have a valid DWARF number, and is instead composed of multiple register pieces. Looking closer at the standard, this appears to not be valid DWARF. A DW_OP_entry_value operation's block can only be a DWARF expression or a register location description, so it appears to not be valid for it to hold a composite location description like that. See DWARFv5 sec. 2.5.1.7: "The DW_OP_entry_value operation pushes the value that the described location held upon entering the current subprogram. It has two operands: an unsigned LEB128 length, followed by a block containing a DWARF expression or a register location description (see Section 2.6.1.1.3 on page 39)." Here is a dwarf-discuss mail thread regarding this: http://lists.dwarfstd.org/pipermail/dwarf-discuss-dwarfstd.org/2020-March/004610.html There was not a strong consensus reached there, but people seem to lean towards that operations specified under 2.6 (e.g. DW_OP_piece) may not be part of a DWARF expression, and thus the DW_OP_entry_value operation can't contain those. Perhaps we instead want to emit a entry value operation per each DW_OP_reg* operation, e.g.: - DW_OP_entry_value(DW_OP_regx sub_reg0), DW_OP_stack_value, DW_OP_piece 8, - DW_OP_entry_value(DW_OP_regx sub_reg1), DW_OP_stack_value, DW_OP_piece 8, [...] The question then becomes how the call site should look; should a composite location description be emitted there, and we then leave it up to the debugger to match those two composite location descriptions? Another alternative could be to emit a call site parameter entry for each sub-register, but firstly I'm unsure if that is even valid DWARF, and secondly it seems like that would complicate the collection of call site values quite a bit. As far as I can tell GCC does not emit any entry values / call sites in these cases, so we do not have something to compare with, but the former seems like the more reasonable approach. Currently when trying to emit a call site entry for a parameter composed of multiple DWARF registers a (DwarfRegs.size() == 1) assert is triggered in addMachineRegExpression(). Until the call site representation is figured out, and until there is use for these entry values in practice, this commit simply stops the invalid DWARF from being emitted. Reviewers: djtodoro, vsk, aprantl Reviewed By: djtodoro, vsk Subscribers: jyknight, hiraditya, fedor.sergeev, jrtc27, llvm-commits Tags: #debug-info, #llvm Differential Revision: https://reviews.llvm.org/D75270 --- .../CodeGen/AsmPrinter/DwarfExpression.cpp | 32 +++++++++++++++++-- llvm/lib/CodeGen/AsmPrinter/DwarfExpression.h | 3 ++ .../Sparc/entry-value-complex-reg-expr.ll | 23 ++++++++----- 3 files changed, 47 insertions(+), 11 deletions(-) diff --git a/llvm/lib/CodeGen/AsmPrinter/DwarfExpression.cpp b/llvm/lib/CodeGen/AsmPrinter/DwarfExpression.cpp index 7b64c2238bd6a1..d4762121d1050d 100644 --- a/llvm/lib/CodeGen/AsmPrinter/DwarfExpression.cpp +++ b/llvm/lib/CodeGen/AsmPrinter/DwarfExpression.cpp @@ -237,8 +237,17 @@ bool DwarfExpression::addMachineRegExpression(const TargetRegisterInfo &TRI, // If the register can only be described by a complex expression (i.e., // multiple subregisters) it doesn't safely compose with another complex // expression. For example, it is not possible to apply a DW_OP_deref - // operation to multiple DW_OP_pieces. - if (HasComplexExpression && DwarfRegs.size() > 1) { + // operation to multiple DW_OP_pieces, since composite location descriptions + // do not push anything on the DWARF stack. + // + // DW_OP_entry_value operations can only hold a DWARF expression or a + // register location description, so we can't emit a single entry value + // covering a composite location description. In the future we may want to + // emit entry value operations for each register location in the composite + // location, but until that is supported do not emit anything. + if ((HasComplexExpression || IsEmittingEntryValue) && DwarfRegs.size() > 1) { + if (IsEmittingEntryValue) + cancelEntryValue(); DwarfRegs.clear(); LocationKind = Unknown; return false; @@ -349,7 +358,6 @@ void DwarfExpression::beginEntryValueExpression( assert(Op->getArg(0) == 1 && "Can currently only emit entry values covering a single operation"); - emitOp(CU.getDwarf5OrGNULocationAtom(dwarf::DW_OP_entry_value)); IsEmittingEntryValue = true; enableTemporaryBuffer(); } @@ -358,6 +366,8 @@ void DwarfExpression::finalizeEntryValue() { assert(IsEmittingEntryValue && "Entry value not open?"); disableTemporaryBuffer(); + emitOp(CU.getDwarf5OrGNULocationAtom(dwarf::DW_OP_entry_value)); + // Emit the entry value's size operand. unsigned Size = getTemporaryBufferSize(); emitUnsigned(Size); @@ -368,6 +378,18 @@ void DwarfExpression::finalizeEntryValue() { IsEmittingEntryValue = false; } +void DwarfExpression::cancelEntryValue() { + assert(IsEmittingEntryValue && "Entry value not open?"); + disableTemporaryBuffer(); + + // The temporary buffer can't be emptied, so for now just assert that nothing + // has been emitted to it. + assert(getTemporaryBufferSize() == 0 && + "Began emitting entry value block before cancelling entry value"); + + IsEmittingEntryValue = false; +} + unsigned DwarfExpression::getOrCreateBaseType(unsigned BitSize, dwarf::TypeKind Encoding) { // Reuse the base_type if we already have one in this CU otherwise we @@ -401,6 +423,10 @@ static bool isMemoryLocation(DIExpressionCursor ExprCursor) { void DwarfExpression::addExpression(DIExpressionCursor &&ExprCursor, unsigned FragmentOffsetInBits) { + // Entry values can currently only cover the initial register location, + // and not any other parts of the following DWARF expression. + assert(!IsEmittingEntryValue && "Can't emit entry value around expression"); + // If we need to mask out a subregister, do it now, unless the next // operation would emit an OpPiece anyway. auto N = ExprCursor.peek(); diff --git a/llvm/lib/CodeGen/AsmPrinter/DwarfExpression.h b/llvm/lib/CodeGen/AsmPrinter/DwarfExpression.h index 42be827cd5a091..757b175114535f 100644 --- a/llvm/lib/CodeGen/AsmPrinter/DwarfExpression.h +++ b/llvm/lib/CodeGen/AsmPrinter/DwarfExpression.h @@ -276,6 +276,9 @@ class DwarfExpression { /// DWARF block which has been emitted to the temporary buffer. void finalizeEntryValue(); + /// Cancel the emission of an entry value. + void cancelEntryValue(); + ~DwarfExpression() = default; public: diff --git a/llvm/test/DebugInfo/Sparc/entry-value-complex-reg-expr.ll b/llvm/test/DebugInfo/Sparc/entry-value-complex-reg-expr.ll index 0af5619b75d78a..096df49110c5c9 100644 --- a/llvm/test/DebugInfo/Sparc/entry-value-complex-reg-expr.ll +++ b/llvm/test/DebugInfo/Sparc/entry-value-complex-reg-expr.ll @@ -1,11 +1,14 @@ ; RUN: llc -debug-entry-values -filetype=asm -o - %s | FileCheck %s -; Verify that the entry value covers both of the DW_OP_regx pieces. Previously -; the size operand of the entry value would be hardcoded to one. +; The q0 register does not have a DWARF register number, and is instead emitted +; as a composite location description with two sub-registers. Previously we +; emitted a single DW_OP_entry_value wrapping that whole composite location +; description, but that is not valid DWARF; DW_OP_entry_value operations can +; only hold DWARF expressions and register location descriptions. ; -; XXX: Is this really what should be emitted, or should we instead emit one -; entry value operation per DW_OP_regx? GDB can currently not understand -; entry values containing complex expressions like this. +; In the future we may want to emit a composite location description where each +; DW_OP_regx operation is wrapped in an entry value operation, but for now +; just verify that no invalid DWARF is emitted. target datalayout = "E-m:e-i64:64-n32:64-S128" target triple = "sparc64" @@ -20,8 +23,11 @@ target triple = "sparc64" ; return 123; ; } -; CHECK: .byte 243 ! DW_OP_GNU_entry_value -; CHECK-NEXT: .byte 8 ! 8 +; Verify that we got an entry value in the DIExpression... +; CHECK: DEBUG_VALUE: foo:p <- [DW_OP_LLVM_entry_value 1] $q0 + +; ... but that no entry value location was emitted: +; CHECK: .half 8 ! Loc expr size ; CHECK-NEXT: .byte 144 ! sub-register DW_OP_regx ; CHECK-NEXT: .byte 72 ! 72 ; CHECK-NEXT: .byte 147 ! DW_OP_piece @@ -30,7 +36,8 @@ target triple = "sparc64" ; CHECK-NEXT: .byte 73 ! 73 ; CHECK-NEXT: .byte 147 ! DW_OP_piece ; CHECK-NEXT: .byte 8 ! 8 -; CHECK-NEXT: .byte 159 ! DW_OP_stack_value +; CHECK-NEXT: .xword 0 +; CHECK-NEXT: .xword 0 @global = common global fp128 0xL00000000000000000000000000000000, align 16, !dbg !0 From 917bdfaca6df575f617b0f3aa989183ab187e8ac Mon Sep 17 00:00:00 2001 From: Georgii Rymar Date: Tue, 30 Jun 2020 15:47:05 +0300 Subject: [PATCH 10/14] [llvm-readobj] - Simplify and refine hash table tests Now we are able to have default values for macros in YAML descriptions. I've applied it for hash table tests and also fixed few copy-paste issues in their comments. Differential revision: https://reviews.llvm.org/D82870 --- llvm/test/tools/llvm-readobj/ELF/gnuhash.test | 16 +++++------ .../llvm-readobj/ELF/hash-histogram.test | 24 ++++++++-------- .../tools/llvm-readobj/ELF/hash-symbols.test | 28 +++++++++---------- 3 files changed, 33 insertions(+), 35 deletions(-) diff --git a/llvm/test/tools/llvm-readobj/ELF/gnuhash.test b/llvm/test/tools/llvm-readobj/ELF/gnuhash.test index 7fe2e465c64879..ce10cc3d997d32 100644 --- a/llvm/test/tools/llvm-readobj/ELF/gnuhash.test +++ b/llvm/test/tools/llvm-readobj/ELF/gnuhash.test @@ -1,7 +1,7 @@ ## Check how the GNU Hash section is dumped with --gnu-hash-table. -# RUN: yaml2obj --docnum=1 -DBITS=64 -DMACHINE=EM_X86_64 -D MASKWORDS=2 -D NBUCKETS=3 %s -o %t.x64 -# RUN: yaml2obj --docnum=1 -DBITS=32 -DMACHINE=EM_386 %s -D MASKWORDS=2 -D NBUCKETS=3 -o %t.x32 +# RUN: yaml2obj --docnum=1 -DBITS=64 -DMACHINE=EM_X86_64 %s -o %t.x64 +# RUN: yaml2obj --docnum=1 -DBITS=32 -DMACHINE=EM_386 %s -o %t.x32 # RUN: llvm-readobj --gnu-hash-table %t.x64 | FileCheck %s # RUN: llvm-readelf --gnu-hash-table %t.x64 | FileCheck %s @@ -33,9 +33,9 @@ Sections: SymNdx: 0x1 Shift2: 0x2 ## The number of words in the Bloom filter. The value of 2 is no-op. - MaskWords: [[MASKWORDS]] + MaskWords: [[MASKWORDS=2]] ## The number of hash buckets. The value of 3 is no-op. - NBuckets: [[NBUCKETS]] + NBuckets: [[NBUCKETS=3]] BloomFilter: [0x3, 0x4] HashBuckets: [0x5, 0x6, 0x7] HashValues: [0x8, 0x9, 0xA, 0xB] @@ -290,15 +290,15 @@ ProgramHeaders: ## Check we report a proper warning when a hash table goes past the end of the file. -## Case A: the 'nbuckets' field is set so that the table goes past the end of the file. -# RUN: yaml2obj --docnum=1 -DBITS=64 -DMACHINE=EM_X86_64 -D MASKWORDS=4294967295 -D NBUCKETS=3 %s -o %t.err.maskwords +## Case A: the 'maskwords' field is set so that the table goes past the end of the file. +# RUN: yaml2obj --docnum=1 -DBITS=64 -DMACHINE=EM_X86_64 -D MASKWORDS=4294967295 %s -o %t.err.maskwords # RUN: llvm-readobj --gnu-hash-table %t.err.maskwords 2>&1 | \ # RUN: FileCheck %s -DFILE=%t.err.maskwords -DMASKWORDS=4294967295 -DNBUCKETS=3 --check-prefix=ERR # RUN: llvm-readelf --gnu-hash-table %t.err.maskwords 2>&1 | \ # RUN: FileCheck %s -DFILE=%t.err.maskwords -DMASKWORDS=4294967295 -DNBUCKETS=3 --check-prefix=ERR -## Case B: the 'maskwords' field is set so that the table goes past the end of the file. -# RUN: yaml2obj --docnum=1 -DBITS=64 -DMACHINE=EM_X86_64 -D MASKWORDS=2 -D NBUCKETS=4294967295 %s -o %t.err.nbuckets +## Case B: the 'nbuckets' field is set so that the table goes past the end of the file. +# RUN: yaml2obj --docnum=1 -DBITS=64 -DMACHINE=EM_X86_64 -D NBUCKETS=4294967295 %s -o %t.err.nbuckets # RUN: llvm-readobj --gnu-hash-table %t.err.nbuckets 2>&1 | \ # RUN: FileCheck %s -DFILE=%t.err.nbuckets -DMASKWORDS=2 -DNBUCKETS=4294967295 --check-prefix=ERR # RUN: llvm-readelf --gnu-hash-table %t.err.nbuckets 2>&1 | \ diff --git a/llvm/test/tools/llvm-readobj/ELF/hash-histogram.test b/llvm/test/tools/llvm-readobj/ELF/hash-histogram.test index 525240e7dea754..736f170e4951b4 100644 --- a/llvm/test/tools/llvm-readobj/ELF/hash-histogram.test +++ b/llvm/test/tools/llvm-readobj/ELF/hash-histogram.test @@ -211,8 +211,7 @@ ProgramHeaders: ## Check we dump a histogram for the .gnu.hash table even when the .hash table is skipped. ## Case A: the .hash table has no data to build histogram and it is skipped. -## (NBUCKET == 0x2 is a no-op: it does not change the number of buckets described by the "Bucket" key). -# RUN: yaml2obj --docnum=5 -DNBUCKET=0x2 %s -o %t5.o +# RUN: yaml2obj --docnum=5 %s -o %t5.o # RUN: llvm-readelf --elf-hash-histogram %t5.o 2>&1 | \ # RUN: FileCheck %s --check-prefix=GNU-HASH --implicit-check-not="Histogram" @@ -236,7 +235,8 @@ Sections: Type: SHT_HASH Flags: [ SHF_ALLOC ] Bucket: [ 0 ] - NBucket: [[NBUCKET]] +## 0x2 is a no-op: it does not change the number of buckets described by the "Bucket" key + NBucket: [[NBUCKET=0x2]] Chain: [ 0, 0 ] - Name: .gnu.hash Type: SHT_GNU_HASH @@ -269,17 +269,15 @@ ProgramHeaders: ## Check we report a proper warning when the GNU hash table goes past the end of the file. -## Case A: the 'nbuckets' field is set so that the GNU hash table goes past the end of the file. -## The value of 1 for the NBUCKETS is no-op. -# RUN: yaml2obj --docnum=6 -D MASKWORDS=0x80000000 -D NBUCKETS=1 %s -o %t7 +## Case A: the 'maskwords' field is set so that the GNU hash table goes past the end of the file. +# RUN: yaml2obj --docnum=6 -D MASKWORDS=0x80000000 %s -o %t7 # RUN: llvm-readelf --elf-hash-histogram %t7 2>&1 | \ # RUN: FileCheck %s -DFILE=%t7 --check-prefix=ERR5 --implicit-check-not="Histogram" # ERR5: warning: '[[FILE]]': unable to dump the SHT_GNU_HASH section at 0x78: it goes past the end of the file -## Case B: the 'maskwords' field is set so that the GNU hash table goes past the end of the file. -## The value of 1 for the MASKWORDS is no-op. -# RUN: yaml2obj --docnum=6 -D MASKWORDS=1 -D NBUCKETS=0x80000000 %s -o %t8 +## Case B: the 'nbuckets' field is set so that the GNU hash table goes past the end of the file. +# RUN: yaml2obj --docnum=6 -D NBUCKETS=0x80000000 %s -o %t8 # RUN: llvm-readelf --elf-hash-histogram %t8 2>&1 | \ # RUN: FileCheck %s -DFILE=%t8 --check-prefix=ERR5 --implicit-check-not="Histogram" @@ -296,10 +294,10 @@ Sections: Header: SymNdx: 0x0 Shift2: 0x0 -## The number of words in the Bloom filter. - MaskWords: [[MASKWORDS]] -## The number of hash buckets. - NBuckets: [[NBUCKETS]] +## The number of words in the Bloom filter. The value of 1 is no-op. + MaskWords: [[MASKWORDS=1]] +## The number of hash buckets. The value of 1 is no-op. + NBuckets: [[NBUCKETS=1]] BloomFilter: [ 0x0 ] HashBuckets: [ 0x0 ] HashValues: [ 0x0 ] diff --git a/llvm/test/tools/llvm-readobj/ELF/hash-symbols.test b/llvm/test/tools/llvm-readobj/ELF/hash-symbols.test index edd3c1e9b84c0a..b43f3eb64aec41 100644 --- a/llvm/test/tools/llvm-readobj/ELF/hash-symbols.test +++ b/llvm/test/tools/llvm-readobj/ELF/hash-symbols.test @@ -388,7 +388,7 @@ ProgramHeaders: ## Case A.1: the hash table ends right before the EOF. We have a broken nbucket ## field that has a value larger than the number of buckets. -# RUN: yaml2obj --docnum=7 %s -o %t7.1.o -DNBUCKET=0x5d -DNCHAIN=0x1 +# RUN: yaml2obj --docnum=7 %s -o %t7.1.o -DNBUCKET=0x5d # RUN: llvm-readelf --hash-symbols %t7.1.o 2>&1 | FileCheck %s --check-prefix=NOERR1 # NOERR1: Symbol table of .hash for image: # NOERR1-NEXT: Num Buc: Value Size Type Bind Vis Ndx Name @@ -396,7 +396,7 @@ ProgramHeaders: ## Case A.2: the hash table ends 1 byte past the EOF. We have a broken nbucket ## field that has a value larger than the number of buckets. -# RUN: yaml2obj --docnum=7 %s -o %t7.2.o -DNBUCKET=0x5e -DNCHAIN=0x1 +# RUN: yaml2obj --docnum=7 %s -o %t7.2.o -DNBUCKET=0x5e # RUN: llvm-readelf --hash-symbols %t7.2.o 2>&1 | FileCheck %s --check-prefix=ERR2 -DFILE=%t7.2.o # ERR2: Symbol table of .hash for image: # ERR2-NEXT: warning: '[[FILE]]': the hash table at offset 0x54 goes past the end of the file (0x1d4), nbucket = 94, nchain = 1{{$}} @@ -404,7 +404,7 @@ ProgramHeaders: ## Case B.1: the hash table ends right before the EOF. We have a broken nchain ## field that has a value larger than the number of chains. -# RUN: yaml2obj --docnum=7 %s -o %t7.3.o -DNBUCKET=0x1 -DNCHAIN=0x5d +# RUN: yaml2obj --docnum=7 %s -o %t7.3.o -DNCHAIN=0x5d # RUN: llvm-readelf --hash-symbols %t7.3.o 2>&1 | \ # RUN: FileCheck %s --implicit-check-not="warning:" --check-prefix=NOERR2 -DFILE=%t7.3.o # NOERR2: warning: '[[FILE]]': hash table nchain (93) differs from symbol count derived from SHT_DYNSYM section header (1) @@ -414,7 +414,7 @@ ProgramHeaders: ## Case B.2: the hash table ends 1 byte past the EOF. We have a broken nchain ## field that has a value larger than the number of chains. -# RUN: yaml2obj --docnum=7 %s -o %t7.4.o -DNBUCKET=0x1 -DNCHAIN=0x5e +# RUN: yaml2obj --docnum=7 %s -o %t7.4.o -DNCHAIN=0x5e # RUN: llvm-readelf --hash-symbols %t7.4.o 2>&1 | FileCheck %s --check-prefix=ERR3 -DFILE=%t7.4.o # ERR3: Symbol table of .hash for image: # ERR3-NEXT: warning: '[[FILE]]': the hash table at offset 0x54 goes past the end of the file (0x1d4), nbucket = 1, nchain = 94{{$}} @@ -431,9 +431,9 @@ Sections: Type: SHT_HASH Flags: [ SHF_ALLOC ] Bucket: [ 0 ] - NBucket: [[NBUCKET]] + NBucket: [[NBUCKET=1]] Chain: [ 0 ] - NChain: [[NCHAIN]] + NChain: [[NCHAIN=1]] - Name: .dynamic Type: SHT_DYNAMIC Flags: [ SHF_WRITE, SHF_ALLOC ] @@ -451,19 +451,19 @@ ProgramHeaders: ## Check we report a proper warning when a GNU hash table goes past the end of the file. -## Case A: the 'nbuckets' field is set so that the table goes past the end of the file. -# RUN: yaml2obj --docnum=8 -D MASKWORDS=4294967295 -D NBUCKETS=3 %s -o %t.err.maskwords +## Case A: the 'maskwords' field is set so that the table goes past the end of the file. +# RUN: yaml2obj --docnum=8 -D MASKWORDS=4294967295 %s -o %t.err.maskwords # RUN: llvm-readelf --hash-symbols %t.err.maskwords 2>&1 | \ -# RUN: FileCheck %s -DFILE=%t.err.maskwords -DMASKWORDS=4294967295 -DNBUCKETS=3 --check-prefix=ERR4 +# RUN: FileCheck %s -DFILE=%t.err.maskwords --check-prefix=ERR4 # ERR4: Symbol table of .gnu.hash for image: # ERR4-NEXT: Num Buc: Value Size Type Bind Vis Ndx Name # ERR4-NEXT: warning: '[[FILE]]': unable to dump the SHT_GNU_HASH section at 0x78: it goes past the end of the file -## Case B: the 'maskwords' field is set so that the table goes past the end of the file. -# RUN: yaml2obj --docnum=8 -D MASKWORDS=2 -D NBUCKETS=4294967295 %s -o %t.err.nbuckets +## Case B: the 'nbuckets' field is set so that the table goes past the end of the file. +# RUN: yaml2obj --docnum=8 -D NBUCKETS=4294967295 %s -o %t.err.nbuckets # RUN: llvm-readelf --hash-symbols %t.err.nbuckets 2>&1 | \ -# RUN: FileCheck %s -DFILE=%t.err.nbuckets -DMASKWORDS=2 -DNBUCKETS=4294967295 --check-prefix=ERR4 +# RUN: FileCheck %s -DFILE=%t.err.nbuckets --check-prefix=ERR4 --- !ELF FileHeader: @@ -479,9 +479,9 @@ Sections: SymNdx: 0x1 Shift2: 0x2 ## The number of words in the Bloom filter. The value of 2 is no-op. - MaskWords: [[MASKWORDS]] + MaskWords: [[MASKWORDS=2]] ## The number of hash buckets. The value of 3 is no-op. - NBuckets: [[NBUCKETS]] + NBuckets: [[NBUCKETS=3]] BloomFilter: [0x3, 0x4] HashBuckets: [0x5, 0x6, 0x7] HashValues: [0x8, 0x9, 0xA, 0xB] From 61f967dccabab67f9996a4fb1c6ec4fa4f23f005 Mon Sep 17 00:00:00 2001 From: Georgii Rymar Date: Tue, 30 Jun 2020 17:14:45 +0300 Subject: [PATCH 11/14] [llvm-readobj] - Don't crash when checking the number of dynamic symbols. When we deriving the number of symbols from the DT_HASH table, we can crash when calculate the number of symbols in the symbol table when SHT_DYNSYM has sh_entsize == 0. The patch fixes the issue. Differential revision: https://reviews.llvm.org/D82877 --- .../ELF/dyn-symbols-size-from-hash-table.test | 18 ++++++++++++++++-- llvm/tools/llvm-readobj/ELFDumper.cpp | 18 +++++++++++------- 2 files changed, 27 insertions(+), 9 deletions(-) diff --git a/llvm/test/tools/llvm-readobj/ELF/dyn-symbols-size-from-hash-table.test b/llvm/test/tools/llvm-readobj/ELF/dyn-symbols-size-from-hash-table.test index 80cb8e3e9fa4cf..7da80598ec6e86 100644 --- a/llvm/test/tools/llvm-readobj/ELF/dyn-symbols-size-from-hash-table.test +++ b/llvm/test/tools/llvm-readobj/ELF/dyn-symbols-size-from-hash-table.test @@ -158,6 +158,20 @@ ProgramHeaders: # RUN: FileCheck %s --check-prefixes=GNU2,GNU2-MORE,GNU2-ALL,WARN \ # RUN: --implicit-check-not=warning: -DNCHAIN=4 +# WARN: warning: '{{.*}}2-{{.*}}': hash table nchain ([[NCHAIN]]) differs from symbol count derived from SHT_DYNSYM section header (3) + +## Show we report a warning when the sh_entsize of the SHT_DYNSYM section is zero and therefore we are unable +## to derive the number of dynamic symbols from SHT_DYNSYM section header. +# RUN: yaml2obj --docnum=2 %s -o %t2-zero-entsize -DCHAIN="[1, 2, 3, 4]" -DENTSIZE=0 +# RUN: llvm-readobj --dyn-symbols %t2-zero-entsize 2>&1 | \ +# RUN: FileCheck %s -DFILE=%t2-zero-entsize --check-prefixes=LLVM2,LLVM2-MORE,LLVM2-ALL,WARN-ENTSIZE \ +# RUN: --implicit-check-not=warning: +# RUN: llvm-readelf --dyn-symbols %t2-zero-entsize 2>&1 | \ +# RUN: FileCheck %s -DFILE=%t2-zero-entsize --check-prefixes=GNU2,GNU2-MORE,GNU2-ALL,WARN-ENTSIZE \ +# RUN: --implicit-check-not=warning: -DNCHAIN=4 + +## WARN-ENTSIZE: warning: '[[FILE]]': SHT_DYNSYM section has sh_entsize == 0 + ## Show there's no warning if the sizes match # RUN: yaml2obj --docnum=2 %s -o %t2-same -DCHAIN="[1, 2, 3]" # RUN: llvm-readobj --dyn-symbols %t2-same 2>&1 | \ @@ -166,8 +180,6 @@ ProgramHeaders: # RUN: FileCheck %s --check-prefixes=GNU2,GNU2-MORE \ # RUN: --implicit-check-not=warning: -DNCHAIN=3 -# WARN: warning: '{{.*}}2-{{.*}}': hash table nchain ([[NCHAIN]]) differs from symbol count derived from SHT_DYNSYM section header (3) - # LLVM2: DynamicSymbols [ # LLVM2-NEXT: Symbol { # LLVM2-NEXT: Name: (0) @@ -236,6 +248,8 @@ Sections: ShSize: 0x48 Address: 0x400 AddressAlign: 0x400 +## 0x18 is the standard entsize value. + EntSize: [[ENTSIZE=0x18]] - Name: .hash Type: SHT_HASH Flags: [ SHF_ALLOC ] diff --git a/llvm/tools/llvm-readobj/ELFDumper.cpp b/llvm/tools/llvm-readobj/ELFDumper.cpp index 3f29c99a5c54c8..0fbaf7d98f97d9 100644 --- a/llvm/tools/llvm-readobj/ELFDumper.cpp +++ b/llvm/tools/llvm-readobj/ELFDumper.cpp @@ -2215,13 +2215,17 @@ void ELFDumper::parseDynamicTable(const ELFFile *Obj) { // equal nchain". Check to see if the DT_HASH hash table nchain value // conflicts with the number of symbols in the dynamic symbol table // according to the section header. - if (HashTable && - HashTable->nchain != DynSymRegion->Size / DynSymRegion->EntSize) - reportUniqueWarning(createError( - "hash table nchain (" + Twine(HashTable->nchain) + - ") differs from symbol count derived from SHT_DYNSYM section " - "header (" + - Twine(DynSymRegion->Size / DynSymRegion->EntSize) + ")")); + if (HashTable) { + if (DynSymRegion->EntSize == 0) + reportUniqueWarning( + createError("SHT_DYNSYM section has sh_entsize == 0")); + else if (HashTable->nchain != DynSymRegion->Size / DynSymRegion->EntSize) + reportUniqueWarning(createError( + "hash table nchain (" + Twine(HashTable->nchain) + + ") differs from symbol count derived from SHT_DYNSYM section " + "header (" + + Twine(DynSymRegion->Size / DynSymRegion->EntSize) + ")")); + } } // Delay the creation of the actual dynamic symbol table until now, so that From 7dcc3858e72666dc12240c8a4bd278775cd807ea Mon Sep 17 00:00:00 2001 From: Sam McCall Date: Wed, 1 Jul 2020 11:28:15 +0200 Subject: [PATCH 12/14] [clangd] Fix name conflict again, unbreak GCC. NFC --- clang-tools-extra/clangd/unittests/ConfigTesting.h | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/clang-tools-extra/clangd/unittests/ConfigTesting.h b/clang-tools-extra/clangd/unittests/ConfigTesting.h index fd0ecf53f360d4..5541c6fffe5967 100644 --- a/clang-tools-extra/clangd/unittests/ConfigTesting.h +++ b/clang-tools-extra/clangd/unittests/ConfigTesting.h @@ -32,10 +32,10 @@ struct CapturedDiags { Out.Pos.character = D.getColumnNo(); // Zero-based - bug in SourceMgr? if (!D.getRanges().empty()) { const auto &R = D.getRanges().front(); - Out.Range.emplace(); - Out.Range->start.line = Out.Range->end.line = Out.Pos.line; - Out.Range->start.character = R.first; - Out.Range->end.character = R.second; + Out.Rng.emplace(); + Out.Rng->start.line = Out.Rng->end.line = Out.Pos.line; + Out.Rng->start.character = R.first; + Out.Rng->end.character = R.second; } }; } @@ -43,7 +43,7 @@ struct CapturedDiags { std::string Message; llvm::SourceMgr::DiagKind Kind; Position Pos; - llvm::Optional Range; + llvm::Optional Rng; friend void PrintTo(const Diag &D, std::ostream *OS) { *OS << (D.Kind == llvm::SourceMgr::DK_Error ? "error: " : "warning: ") @@ -56,7 +56,7 @@ struct CapturedDiags { MATCHER_P(DiagMessage, M, "") { return arg.Message == M; } MATCHER_P(DiagKind, K, "") { return arg.Kind == K; } MATCHER_P(DiagPos, P, "") { return arg.Pos == P; } -MATCHER_P(DiagRange, R, "") { return arg.Range == R; } +MATCHER_P(DiagRange, R, "") { return arg.Rng == R; } inline Position toPosition(llvm::SMLoc L, const llvm::SourceMgr &SM) { auto LineCol = SM.getLineAndColumn(L); From 4c6683eafc17b201fc5de17f96230be46d8ff521 Mon Sep 17 00:00:00 2001 From: Kerry McLaughlin Date: Wed, 1 Jul 2020 10:00:35 +0100 Subject: [PATCH 13/14] [AArch64][SVE] Add reg+imm addressing mode for unpredicated loads Reviewers: efriedma, sdesmalen, david-arm Reviewed By: efriedma Subscribers: tschuett, kristof.beyls, hiraditya, rkruppe, psnobl, danielkiss, llvm-commits Tags: #llvm Differential Revision: https://reviews.llvm.org/D82893 --- .../lib/Target/AArch64/AArch64SVEInstrInfo.td | 11 +- .../sve-ld1-addressing-mode-reg-imm.ll | 102 ++++++++++++++++++ 2 files changed, 111 insertions(+), 2 deletions(-) create mode 100644 llvm/test/CodeGen/AArch64/sve-ld1-addressing-mode-reg-imm.ll diff --git a/llvm/lib/Target/AArch64/AArch64SVEInstrInfo.td b/llvm/lib/Target/AArch64/AArch64SVEInstrInfo.td index c51e9a24d71811..537e54f3a8d801 100644 --- a/llvm/lib/Target/AArch64/AArch64SVEInstrInfo.td +++ b/llvm/lib/Target/AArch64/AArch64SVEInstrInfo.td @@ -1719,8 +1719,15 @@ multiclass sve_prefetch { - def : Pat<(Ty (Load (am_sve_fi GPR64sp:$base, simm4s1:$offset))), - (RegImmInst (PTrue 31), GPR64sp:$base, simm4s1:$offset)>; + let AddedComplexity = 1 in { + def _imm: Pat<(Ty (Load (am_sve_indexed_s4 GPR64sp:$base, simm4s1:$offset))), + (RegImmInst (PTrue 31), GPR64sp:$base, simm4s1:$offset)>; + } + + let AddedComplexity = 2 in { + def _fi : Pat<(Ty (Load (am_sve_fi GPR64sp:$base, simm4s1:$offset))), + (RegImmInst (PTrue 31), GPR64sp:$base, simm4s1:$offset)>; + } def : Pat<(Ty (Load GPR64:$base)), (RegImmInst (PTrue 31), GPR64:$base, (i64 0))>; diff --git a/llvm/test/CodeGen/AArch64/sve-ld1-addressing-mode-reg-imm.ll b/llvm/test/CodeGen/AArch64/sve-ld1-addressing-mode-reg-imm.ll new file mode 100644 index 00000000000000..20bcd51e716dff --- /dev/null +++ b/llvm/test/CodeGen/AArch64/sve-ld1-addressing-mode-reg-imm.ll @@ -0,0 +1,102 @@ +; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py +; RUN: llc -mtriple=aarch64-linux-gnu -mattr=+sve < %s | FileCheck %s + +; LD1B + +define @ld1b_lower_bound(* %a) { +; CHECK-LABEL: ld1b_lower_bound: +; CHECK: // %bb.0: +; CHECK-NEXT: ptrue p0.b +; CHECK-NEXT: ld1b { z0.b }, p0/z, [x0, #-8, mul vl] +; CHECK-NEXT: ret + %base = getelementptr , * %a, i64 -8 + %load = load , * %base + ret %load +} + +define @ld1b_inbound(* %a) { +; CHECK-LABEL: ld1b_inbound: +; CHECK: // %bb.0: +; CHECK-NEXT: ptrue p0.b +; CHECK-NEXT: ld1b { z0.b }, p0/z, [x0, #2, mul vl] +; CHECK-NEXT: ret + %base = getelementptr , * %a, i64 2 + %load = load , * %base + ret %load +} + +define @ld1b_upper_bound(* %a) { +; CHECK-LABEL: ld1b_upper_bound: +; CHECK: // %bb.0: +; CHECK-NEXT: ptrue p0.b +; CHECK-NEXT: ld1b { z0.b }, p0/z, [x0, #7, mul vl] +; CHECK-NEXT: ret + %base = getelementptr , * %a, i64 7 + %load = load , * %base + ret %load +} + +define @ld1b_out_of_upper_bound(* %a) { +; CHECK-LABEL: ld1b_out_of_upper_bound: +; CHECK: // %bb.0: +; CHECK-NEXT: rdvl x8, #8 +; CHECK-NEXT: add x8, x0, x8 +; CHECK-NEXT: ptrue p0.b +; CHECK-NEXT: ld1b { z0.b }, p0/z, [x8] +; CHECK-NEXT: ret + %base = getelementptr , * %a, i64 8 + %load = load , * %base + ret %load +} + +define @ld1b_out_of_lower_bound(* %a) { +; CHECK-LABEL: ld1b_out_of_lower_bound: +; CHECK: // %bb.0: +; CHECK-NEXT: rdvl x8, #-9 +; CHECK-NEXT: add x8, x0, x8 +; CHECK-NEXT: ptrue p0.b +; CHECK-NEXT: ld1b { z0.b }, p0/z, [x8] +; CHECK-NEXT: ret + %base = getelementptr , * %a, i64 -9 + %load = load , * %base + ret %load +} + +; LD1H + +define @ld1h_inbound(* %a) { +; CHECK-LABEL: ld1h_inbound: +; CHECK: // %bb.0: +; CHECK-NEXT: ptrue p0.h +; CHECK-NEXT: ld1h { z0.h }, p0/z, [x0, #-2, mul vl] +; CHECK-NEXT: ret + %base = getelementptr , * %a, i64 -2 + %load = load , * %base + ret %load +} + +; LD1W + +define @ld1s_inbound(* %a) { +; CHECK-LABEL: ld1s_inbound: +; CHECK: // %bb.0: +; CHECK-NEXT: ptrue p0.s +; CHECK-NEXT: ld1w { z0.s }, p0/z, [x0, #4, mul vl] +; CHECK-NEXT: ret + %base = getelementptr , * %a, i64 4 + %load = load , * %base + ret %load +} + +; LD1D + +define @ld1d_inbound(* %a) { +; CHECK-LABEL: ld1d_inbound: +; CHECK: // %bb.0: +; CHECK-NEXT: ptrue p0.d +; CHECK-NEXT: ld1d { z0.d }, p0/z, [x0, #6, mul vl] +; CHECK-NEXT: ret + %base = getelementptr , * %a, i64 6 + %load = load , * %base + ret %load +} From 4b9ae1b7e5e052126e1be4c817ff53203d33d9d1 Mon Sep 17 00:00:00 2001 From: Petar Avramovic Date: Wed, 1 Jul 2020 11:50:59 +0200 Subject: [PATCH 14/14] AMDGPU/GlobalISel: Select init_exec intrinsic Change imm with timm in pattern for SI_INIT_EXEC_LO and remove regbank mappings for non register operands. Differential Revision: https://reviews.llvm.org/D82885 --- llvm/lib/Target/AMDGPU/AMDGPURegisterBankInfo.cpp | 4 +--- llvm/lib/Target/AMDGPU/SIInstructions.td | 2 +- llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.init.exec.ll | 4 ++-- 3 files changed, 4 insertions(+), 6 deletions(-) diff --git a/llvm/lib/Target/AMDGPU/AMDGPURegisterBankInfo.cpp b/llvm/lib/Target/AMDGPU/AMDGPURegisterBankInfo.cpp index 7206065660fddb..0d7819bc144dbd 100644 --- a/llvm/lib/Target/AMDGPU/AMDGPURegisterBankInfo.cpp +++ b/llvm/lib/Target/AMDGPU/AMDGPURegisterBankInfo.cpp @@ -4230,8 +4230,7 @@ AMDGPURegisterBankInfo::getInstrMapping(const MachineInstr &MI) const { OpdsMapping[2] = AMDGPU::getValueMapping(Bank, 32); break; } - case Intrinsic::amdgcn_end_cf: - case Intrinsic::amdgcn_init_exec: { + case Intrinsic::amdgcn_end_cf: { unsigned Size = getSizeInBits(MI.getOperand(1).getReg(), MRI, *TRI); OpdsMapping[1] = AMDGPU::getValueMapping(AMDGPU::SGPRRegBankID, Size); break; @@ -4287,7 +4286,6 @@ AMDGPURegisterBankInfo::getInstrMapping(const MachineInstr &MI) const { case Intrinsic::amdgcn_init_exec_from_input: { unsigned Size = getSizeInBits(MI.getOperand(1).getReg(), MRI, *TRI); OpdsMapping[1] = AMDGPU::getValueMapping(AMDGPU::SGPRRegBankID, Size); - OpdsMapping[2] = AMDGPU::getValueMapping(AMDGPU::SGPRRegBankID, Size); break; } case Intrinsic::amdgcn_ds_gws_init: diff --git a/llvm/lib/Target/AMDGPU/SIInstructions.td b/llvm/lib/Target/AMDGPU/SIInstructions.td index 24113cf69f7e6c..2b053f8dc95e19 100644 --- a/llvm/lib/Target/AMDGPU/SIInstructions.td +++ b/llvm/lib/Target/AMDGPU/SIInstructions.td @@ -447,7 +447,7 @@ def SI_INIT_EXEC_FROM_INPUT : SPseudoInstSI < def : GCNPat < (int_amdgcn_init_exec timm:$src), - (SI_INIT_EXEC_LO (as_i32imm imm:$src))> { + (SI_INIT_EXEC_LO (as_i32timm timm:$src))> { let WaveSizePredicate = isWave32; } diff --git a/llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.init.exec.ll b/llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.init.exec.ll index 77c2f80ac02b62..22826a7e993710 100644 --- a/llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.init.exec.ll +++ b/llvm/test/CodeGen/AMDGPU/GlobalISel/llvm.amdgcn.init.exec.ll @@ -1,2 +1,2 @@ -; RUN: llc -march=amdgcn -mcpu=gfx900 -verify-machineinstrs < %S/../llvm.amdgcn.init.exec.ll | FileCheck -check-prefix=GCN %S/../llvm.amdgcn.init.exec.ll -; RUN: llc -march=amdgcn -mcpu=gfx1010 -mattr=-wavefrontsize32,+wavefrontsize64 -verify-machineinstrs < %S/../llvm.amdgcn.init.exec.ll | FileCheck -check-prefix=GCN %S/../llvm.amdgcn.init.exec.ll +; RUN: llc -march=amdgcn -mcpu=gfx900 -global-isel -verify-machineinstrs < %S/../llvm.amdgcn.init.exec.ll | FileCheck -check-prefix=GCN %S/../llvm.amdgcn.init.exec.ll +; RUN: llc -march=amdgcn -mcpu=gfx1010 -global-isel -mattr=-wavefrontsize32,+wavefrontsize64 -verify-machineinstrs < %S/../llvm.amdgcn.init.exec.ll | FileCheck -check-prefix=GCN %S/../llvm.amdgcn.init.exec.ll