From d61ccda76965ebb9f4aa24e87899a8b0e65b2d54 Mon Sep 17 00:00:00 2001 From: "Paul C. Anagnostopoulos" Date: Fri, 11 Dec 2020 16:19:52 -0500 Subject: [PATCH 01/10] [TableGen] Slim down the data structures in xxxGenInstrInfo.inc, step 1 --- llvm/utils/TableGen/InstrInfoEmitter.cpp | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/llvm/utils/TableGen/InstrInfoEmitter.cpp b/llvm/utils/TableGen/InstrInfoEmitter.cpp index 025c5354514ce0..156fa6d18d2ee8 100644 --- a/llvm/utils/TableGen/InstrInfoEmitter.cpp +++ b/llvm/utils/TableGen/InstrInfoEmitter.cpp @@ -371,7 +371,7 @@ void InstrInfoEmitter::emitOperandTypeMappings( OS << "namespace " << Namespace << " {\n"; OS << "LLVM_READONLY\n"; OS << "static int getOperandType(uint16_t Opcode, uint16_t OpIdx) {\n"; - // TODO: Factor out instructions with same operands to compress the tables. + // TODO: Factor out duplicate operand lists to compress the tables. if (!NumberedInstructions.empty()) { std::vector OperandOffsets; std::vector OperandRecords; @@ -393,16 +393,26 @@ void InstrInfoEmitter::emitOperandTypeMappings( } } - // Emit the table of offsets for the opcode lookup. - OS << " const int Offsets[] = {\n"; + // Emit the table of offsets (indexes) into the operand type table. + // Size the unsigned integer offset to save space. + assert(OperandRecords.size() <= UINT32_MAX && + "Too many operands for offset table"); + OS << ((OperandRecords.size() <= UINT16_MAX) ? " const uint16_t" + : " const uint32_t"); + OS << " Offsets[] = {\n"; for (int I = 0, E = OperandOffsets.size(); I != E; ++I) OS << " " << OperandOffsets[I] << ",\n"; OS << " };\n"; // Add an entry for the end so that we don't need to special case it below. OperandOffsets.push_back(OperandRecords.size()); + // Emit the actual operand types in a flat table. - OS << " const int OpcodeOperandTypes[] = {\n "; + // Size the signed integer operand type to save space. + assert(EnumVal <= INT16_MAX && + "Too many operand types for operand types table"); + OS << ((EnumVal <= INT8_MAX) ? " const int8_t" : " const int16_t"); + OS << " OpcodeOperandTypes[] = {\n "; for (int I = 0, E = OperandRecords.size(), CurOffset = 1; I != E; ++I) { // We print each Opcode's operands in its own row. if (I == OperandOffsets[CurOffset]) { From 95b2dab199100f5a86d3f73a995afea879886d65 Mon Sep 17 00:00:00 2001 From: Erik Pilkington Date: Tue, 15 Dec 2020 17:00:20 -0500 Subject: [PATCH 02/10] [Sema] Fix a miscompile by retaining array qualifiers when folding VLAs to constant arrays rdar://72243125 Differential revision: https://reviews.llvm.org/D93247 --- clang/lib/Sema/SemaDecl.cpp | 5 +++-- clang/test/SemaObjC/arc.m | 12 ++++++++++++ 2 files changed, 15 insertions(+), 2 deletions(-) diff --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp index 0031f874c05aac..6c438f319991e8 100644 --- a/clang/lib/Sema/SemaDecl.cpp +++ b/clang/lib/Sema/SemaDecl.cpp @@ -5965,8 +5965,9 @@ static QualType TryToFixInvalidVariablyModifiedType(QualType T, return QualType(); } - return Context.getConstantArrayType(ElemTy, Res, VLATy->getSizeExpr(), - ArrayType::Normal, 0); + QualType FoldedArrayType = Context.getConstantArrayType( + ElemTy, Res, VLATy->getSizeExpr(), ArrayType::Normal, 0); + return Qs.apply(Context, FoldedArrayType); } static void diff --git a/clang/test/SemaObjC/arc.m b/clang/test/SemaObjC/arc.m index fe5db9ce538408..bcd2f995622c81 100644 --- a/clang/test/SemaObjC/arc.m +++ b/clang/test/SemaObjC/arc.m @@ -839,3 +839,15 @@ void block_capture_autoreleasing(A * __autoreleasing *a, (void)*l; }(); } + +void test_vla_fold_keeps_strong(void) { + const unsigned bounds = 1; + + static id array[bounds]; // expected-warning {{variable length array folded to constant array as an extension}} + typedef __typeof__(array) array_type; + typedef id __strong array_type[1]; + + static id weak_array[bounds] __weak; // expected-warning {{variable length array folded to constant array as an extension}} + typedef __typeof__(weak_array) weak_array_type; + typedef id __weak weak_array_type[1]; +} From 92d6e8001e20d6d0f457ac7cab8b64f3b1a131bf Mon Sep 17 00:00:00 2001 From: Erik Pilkington Date: Tue, 15 Dec 2020 17:01:20 -0500 Subject: [PATCH 03/10] NFC: balance a quote in AttrDocs.td This was confusing my editor. --- clang/include/clang/Basic/AttrDocs.td | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang/include/clang/Basic/AttrDocs.td b/clang/include/clang/Basic/AttrDocs.td index a65964e94bf146..4f8cd8ecd86f3a 100644 --- a/clang/include/clang/Basic/AttrDocs.td +++ b/clang/include/clang/Basic/AttrDocs.td @@ -3346,7 +3346,7 @@ As ``global_device`` and ``global_host`` are a subset of ``__global/opencl_global`` address spaces it is allowed to convert ``global_device`` and ``global_host`` address spaces to ``__global/opencl_global`` address spaces (following ISO/IEC TR 18037 5.1.3 -"Address space nesting and rules for pointers). +"Address space nesting and rules for pointers"). }]; } From 7082de56b7ad4b4eeb75e59e0d4c28bed44b5d23 Mon Sep 17 00:00:00 2001 From: Tim Keith Date: Wed, 16 Dec 2020 07:06:53 -0800 Subject: [PATCH 04/10] [flang] Handle multiple names for same operator Some operators have more than one name, e.g. operator(==), operator(.eq). That was working correctly in generic definitions but they can also appear in other contexts: USE statements and access statements, for example. This changes FindInScope to always look for each of the names for a symbol. So an operator may be use-associated under one name but declared private under another name and it will be the same symbol. This replaces GenericSpecInfo::FindInScope which was only usable in some cases. Add a version of FindInScope() that looks in the current scope to simplify many of the calls. Differential Revision: https://reviews.llvm.org/D93344 --- flang/lib/Semantics/resolve-names-utils.cpp | 74 ++++++++++++--------- flang/lib/Semantics/resolve-names-utils.h | 12 ++-- flang/lib/Semantics/resolve-names.cpp | 48 +++++++------ flang/test/Semantics/modfile07.f90 | 49 ++++++++++++++ 4 files changed, 126 insertions(+), 57 deletions(-) diff --git a/flang/lib/Semantics/resolve-names-utils.cpp b/flang/lib/Semantics/resolve-names-utils.cpp index 8dbd25e163acb5..83bff78f426abc 100644 --- a/flang/lib/Semantics/resolve-names-utils.cpp +++ b/flang/lib/Semantics/resolve-names-utils.cpp @@ -29,6 +29,8 @@ using common::NumericOperator; using common::RelationalOperator; using IntrinsicOperator = parser::DefinedOperator::IntrinsicOperator; +static constexpr const char *operatorPrefix{"operator("}; + static GenericKind MapIntrinsicOperator(IntrinsicOperator); Symbol *Resolve(const parser::Name &name, Symbol *symbol) { @@ -65,6 +67,37 @@ bool IsIntrinsicOperator( return false; } +template +std::forward_list GetOperatorNames( + const SemanticsContext &context, E opr) { + std::forward_list result; + for (const char *name : context.languageFeatures().GetNames(opr)) { + result.emplace_front(std::string{operatorPrefix} + name + ')'); + } + return result; +} + +std::forward_list GetAllNames( + const SemanticsContext &context, const SourceName &name) { + std::string str{name.ToString()}; + if (!name.empty() && name.end()[-1] == ')' && + name.ToString().rfind(std::string{operatorPrefix}, 0) == 0) { + for (int i{0}; i != common::LogicalOperator_enumSize; ++i) { + auto names{GetOperatorNames(context, LogicalOperator{i})}; + if (std::find(names.begin(), names.end(), str) != names.end()) { + return names; + } + } + for (int i{0}; i != common::RelationalOperator_enumSize; ++i) { + auto names{GetOperatorNames(context, RelationalOperator{i})}; + if (std::find(names.begin(), names.end(), str) != names.end()) { + return names; + } + } + } + return {str}; +} + bool IsLogicalConstant( const SemanticsContext &context, const SourceName &name) { std::string str{name.ToString()}; @@ -73,37 +106,6 @@ bool IsLogicalConstant( (str == ".t" || str == ".f.")); } -// The operators <, <=, >, >=, ==, and /= always have the same interpretations -// as the operators .LT., .LE., .GT., .GE., .EQ., and .NE., respectively. -std::forward_list GenericSpecInfo::GetAllNames( - SemanticsContext &context) const { - auto getNames{[&](auto opr) { - std::forward_list result; - for (const char *name : context.languageFeatures().GetNames(opr)) { - result.emplace_front("operator("s + name + ')'); - } - return result; - }}; - return std::visit( - common::visitors{[&](const LogicalOperator &x) { return getNames(x); }, - [&](const RelationalOperator &x) { return getNames(x); }, - [&](const auto &) -> std::forward_list { - return {symbolName_.value().ToString()}; - }}, - kind_.u); -} - -Symbol *GenericSpecInfo::FindInScope( - SemanticsContext &context, const Scope &scope) const { - for (const auto &name : GetAllNames(context)) { - auto iter{scope.find(SourceName{name})}; - if (iter != scope.end()) { - return &*iter->second; - } - } - return nullptr; -} - void GenericSpecInfo::Resolve(Symbol *symbol) const { if (symbol) { if (auto *details{symbol->detailsIf()}) { @@ -162,6 +164,16 @@ void GenericSpecInfo::Analyze(const parser::GenericSpec &x) { x.u); } +llvm::raw_ostream &operator<<( + llvm::raw_ostream &os, const GenericSpecInfo &info) { + os << "GenericSpecInfo: kind=" << info.kind_.ToString(); + os << " parseName=" + << (info.parseName_ ? info.parseName_->ToString() : "null"); + os << " symbolName=" + << (info.symbolName_ ? info.symbolName_->ToString() : "null"); + return os; +} + // parser::DefinedOperator::IntrinsicOperator -> GenericKind static GenericKind MapIntrinsicOperator(IntrinsicOperator op) { switch (op) { diff --git a/flang/lib/Semantics/resolve-names-utils.h b/flang/lib/Semantics/resolve-names-utils.h index 17462d111d970d..89011ff3b95657 100644 --- a/flang/lib/Semantics/resolve-names-utils.h +++ b/flang/lib/Semantics/resolve-names-utils.h @@ -19,6 +19,7 @@ #include "flang/Semantics/semantics.h" #include "flang/Semantics/symbol.h" #include "flang/Semantics/type.h" +#include "llvm/Support/raw_ostream.h" #include namespace Fortran::parser { @@ -50,6 +51,11 @@ parser::MessageFixedText WithIsFatal( bool IsIntrinsicOperator(const SemanticsContext &, const SourceName &); bool IsLogicalConstant(const SemanticsContext &, const SourceName &); +// Some intrinsic operators have more than one name (e.g. `operator(.eq.)` and +// `operator(==)`). GetAllNames() returns them all, including symbolName. +std::forward_list GetAllNames( + const SemanticsContext &, const SourceName &); + template MaybeIntExpr EvaluateIntExpr(SemanticsContext &context, const T &expr) { if (MaybeExpr maybeExpr{ @@ -75,13 +81,11 @@ class GenericSpecInfo { GenericKind kind() const { return kind_; } const SourceName &symbolName() const { return symbolName_.value(); } - // Some intrinsic operators have more than one name (e.g. `operator(.eq.)` and - // `operator(==)`). GetAllNames() returns them all, including symbolName. - std::forward_list GetAllNames(SemanticsContext &) const; // Set the GenericKind in this symbol and resolve the corresponding // name if there is one void Resolve(Symbol *) const; - Symbol *FindInScope(SemanticsContext &, const Scope &) const; + friend llvm::raw_ostream &operator<<( + llvm::raw_ostream &, const GenericSpecInfo &); private: GenericKind kind_; diff --git a/flang/lib/Semantics/resolve-names.cpp b/flang/lib/Semantics/resolve-names.cpp index 5ac787b61d68be..1288b11a7727a4 100644 --- a/flang/lib/Semantics/resolve-names.cpp +++ b/flang/lib/Semantics/resolve-names.cpp @@ -502,6 +502,9 @@ class ScopeHandler : public ImplicitRulesVisitor { // Search for name only in scope, not in enclosing scopes. Symbol *FindInScope(const Scope &, const parser::Name &); Symbol *FindInScope(const Scope &, const SourceName &); + template Symbol *FindInScope(const T &name) { + return FindInScope(currScope(), name); + } // Search for name in a derived type scope and its parents. Symbol *FindInTypeOrParents(const Scope &, const parser::Name &); Symbol *FindInTypeOrParents(const parser::Name &); @@ -533,7 +536,7 @@ class ScopeHandler : public ImplicitRulesVisitor { const SourceName &name, const Attrs &attrs, D &&details) { // Note: don't use FindSymbol here. If this is a derived type scope, // we want to detect whether the name is already declared as a component. - auto *symbol{FindInScope(currScope(), name)}; + auto *symbol{FindInScope(name)}; if (!symbol) { symbol = &MakeSymbol(name, attrs); symbol->set_details(std::move(details)); @@ -2048,7 +2051,7 @@ Symbol &ScopeHandler::MakeHostAssocSymbol( return symbol; } Symbol &ScopeHandler::CopySymbol(const SourceName &name, const Symbol &symbol) { - CHECK(!FindInScope(currScope(), name)); + CHECK(!FindInScope(name)); return MakeSymbol(currScope(), name, symbol.attrs()); } @@ -2058,11 +2061,14 @@ Symbol *ScopeHandler::FindInScope( return Resolve(name, FindInScope(scope, name.source)); } Symbol *ScopeHandler::FindInScope(const Scope &scope, const SourceName &name) { - if (auto it{scope.find(name)}; it != scope.end()) { - return &*it->second; - } else { - return nullptr; + // all variants of names, e.g. "operator(.ne.)" for "operator(/=)" + for (const std::string &n : GetAllNames(context(), name)) { + auto it{scope.find(SourceName{n})}; + if (it != scope.end()) { + return &*it->second; + } } + return nullptr; } // Find a component or type parameter by name in a derived type or its parents. @@ -2318,7 +2324,7 @@ void ModuleVisitor::Post(const parser::UseStmt &x) { !symbol->attrs().test(Attr::INTRINSIC) && !symbol->has() && useNames.count(name) == 0) { SourceName location{x.moduleName.source}; - if (auto *localSymbol{FindInScope(currScope(), name)}) { + if (auto *localSymbol{FindInScope(name)}) { DoAddUse(location, localSymbol->name(), *localSymbol, *symbol); } else { DoAddUse(location, location, CopySymbol(name, *symbol), *symbol); @@ -2397,8 +2403,7 @@ void ModuleVisitor::DoAddUse(const SourceName &location, generic1.CopyFrom(generic2); } EraseSymbol(localSymbol); - MakeSymbol( - localSymbol.name(), localUltimate.attrs(), std::move(generic1)); + MakeSymbol(localSymbol.name(), localSymbol.attrs(), std::move(generic1)); } else { ConvertToUseError(localSymbol, location, *useModuleScope_); } @@ -2435,8 +2440,7 @@ void ModuleVisitor::DoAddUse(const SourceName &location, void ModuleVisitor::AddUse(const GenericSpecInfo &info) { if (useModuleScope_) { const auto &name{info.symbolName()}; - auto rename{ - AddUse(name, name, info.FindInScope(context(), *useModuleScope_))}; + auto rename{AddUse(name, name, FindInScope(*useModuleScope_, name))}; info.Resolve(rename.use); } } @@ -2523,7 +2527,7 @@ void InterfaceVisitor::Post(const parser::EndInterfaceStmt &) { // Create a symbol in genericSymbol_ for this GenericSpec. bool InterfaceVisitor::Pre(const parser::GenericSpec &x) { - if (auto *symbol{GenericSpecInfo{x}.FindInScope(context(), currScope())}) { + if (auto *symbol{FindInScope(GenericSpecInfo{x}.symbolName())}) { SetGenericSymbol(*symbol); } return false; @@ -3402,7 +3406,7 @@ Symbol &DeclarationVisitor::HandleAttributeStmt( if (attr == Attr::INTRINSIC && !IsIntrinsic(name.source, std::nullopt)) { Say(name.source, "'%s' is not a known intrinsic procedure"_err_en_US); } - auto *symbol{FindInScope(currScope(), name)}; + auto *symbol{FindInScope(name)}; if (attr == Attr::ASYNCHRONOUS || attr == Attr::VOLATILE) { // these can be set on a symbol that is host-assoc or use-assoc if (!symbol && @@ -4065,7 +4069,7 @@ void DeclarationVisitor::CheckBindings( CHECK(currScope().IsDerivedType()); for (auto &declaration : tbps.declarations) { auto &bindingName{std::get(declaration.t)}; - if (Symbol * binding{FindInScope(currScope(), bindingName)}) { + if (Symbol * binding{FindInScope(bindingName)}) { if (auto *details{binding->detailsIf()}) { const Symbol *procedure{FindSubprogram(details->symbol())}; if (!CanBeTypeBoundProc(procedure)) { @@ -4134,7 +4138,7 @@ bool DeclarationVisitor::Pre(const parser::TypeBoundGenericStmt &x) { SourceName symbolName{info.symbolName()}; bool isPrivate{accessSpec ? accessSpec->v == parser::AccessSpec::Kind::Private : derivedTypeInfo_.privateBindings}; - auto *genericSymbol{info.FindInScope(context(), currScope())}; + auto *genericSymbol{FindInScope(symbolName)}; if (genericSymbol) { if (!genericSymbol->has()) { genericSymbol = nullptr; // MakeTypeSymbol will report the error below @@ -4142,7 +4146,7 @@ bool DeclarationVisitor::Pre(const parser::TypeBoundGenericStmt &x) { } else { // look in parent types: Symbol *inheritedSymbol{nullptr}; - for (const auto &name : info.GetAllNames(context())) { + for (const auto &name : GetAllNames(context(), symbolName)) { inheritedSymbol = currScope().FindComponent(SourceName{name}); if (inheritedSymbol) { break; @@ -4298,7 +4302,7 @@ bool DeclarationVisitor::Pre(const parser::NamelistStmt::Group &x) { } const auto &groupName{std::get(x.t)}; - auto *groupSymbol{FindInScope(currScope(), groupName)}; + auto *groupSymbol{FindInScope(groupName)}; if (!groupSymbol || !groupSymbol->has()) { groupSymbol = &MakeSymbol(groupName, std::move(details)); groupSymbol->ReplaceName(groupName.source); @@ -4397,7 +4401,7 @@ bool DeclarationVisitor::Pre(const parser::SaveStmt &x) { void DeclarationVisitor::CheckSaveStmts() { for (const SourceName &name : saveInfo_.entities) { - auto *symbol{FindInScope(currScope(), name)}; + auto *symbol{FindInScope(name)}; if (!symbol) { // error was reported } else if (saveInfo_.saveAll) { @@ -5159,7 +5163,7 @@ bool ConstructVisitor::Pre(const parser::ChangeTeamStmt &x) { void ConstructVisitor::Post(const parser::CoarrayAssociation &x) { const auto &decl{std::get(x.t)}; const auto &name{std::get(decl.t)}; - if (auto *symbol{FindInScope(currScope(), name)}) { + if (auto *symbol{FindInScope(name)}) { const auto &selector{std::get(x.t)}; if (auto sel{ResolveSelector(selector)}) { const Symbol *whole{UnwrapWholeSymbolDataRef(sel.expr)}; @@ -5962,7 +5966,7 @@ bool ModuleVisitor::Pre(const parser::AccessStmt &x) { [=](const Indirection &y) { auto info{GenericSpecInfo{y.value()}}; const auto &symbolName{info.symbolName()}; - if (auto *symbol{info.FindInScope(context(), currScope())}) { + if (auto *symbol{FindInScope(symbolName)}) { info.Resolve(&SetAccess(symbolName, accessAttr, symbol)); } else if (info.kind().IsName()) { info.Resolve(&SetAccess(symbolName, accessAttr)); @@ -6084,7 +6088,7 @@ void ResolveNamesVisitor::CreateGeneric(const parser::GenericSpec &x) { return; } GenericDetails genericDetails; - if (Symbol * existing{info.FindInScope(context(), currScope())}) { + if (Symbol * existing{FindInScope(symbolName)}) { if (existing->has()) { info.Resolve(existing); return; // already have generic, add to it @@ -6204,7 +6208,7 @@ void ResolveNamesVisitor::CheckImports() { void ResolveNamesVisitor::CheckImport( const SourceName &location, const SourceName &name) { - if (auto *symbol{FindInScope(currScope(), name)}) { + if (auto *symbol{FindInScope(name)}) { Say(location, "'%s' from host is not accessible"_err_en_US, name) .Attach(symbol->name(), "'%s' is hidden by this entity"_en_US, symbol->name()); diff --git a/flang/test/Semantics/modfile07.f90 b/flang/test/Semantics/modfile07.f90 index 0809da471a602b..f3e98bf195f9f3 100644 --- a/flang/test/Semantics/modfile07.f90 +++ b/flang/test/Semantics/modfile07.f90 @@ -549,3 +549,52 @@ subroutine test() ! end !end +! Verify that equivalent names are used when generic operators are merged + +module m10a + interface operator(.ne.) + end interface +end +!Expect: m10a.mod +!module m10a +! interface operator(.ne.) +! end interface +!end + +module m10b + interface operator(<>) + end interface +end +!Expect: m10b.mod +!module m10b +! interface operator(<>) +! end interface +!end + +module m10c + use m10a + use m10b + interface operator(/=) + end interface +end +!Expect: m10c.mod +!module m10c +! use m10b,only:operator(.ne.) +! use m10a,only:operator(.ne.) +! interface operator(.ne.) +! end interface +!end + +module m10d + use m10a + use m10c + private :: operator(<>) +end +!Expect: m10d.mod +!module m10d +! use m10c,only:operator(.ne.) +! use m10a,only:operator(.ne.) +! interface operator(.ne.) +! end interface +! private::operator(.ne.) +!end From f3e0431b763979c373258f7222668e87bb5d28cb Mon Sep 17 00:00:00 2001 From: Matt Arsenault Date: Wed, 16 Dec 2020 10:25:07 -0500 Subject: [PATCH 05/10] LangRef: Update byval/sret description for required types --- llvm/docs/LangRef.rst | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/llvm/docs/LangRef.rst b/llvm/docs/LangRef.rst index b29eb589e2d727..cde9ed5196262f 100644 --- a/llvm/docs/LangRef.rst +++ b/llvm/docs/LangRef.rst @@ -1045,7 +1045,7 @@ Currently, only the following parameter attributes are defined: opposed to memory, though some targets use it to distinguish between two different kinds of registers). Use of this attribute is target-specific. -``byval`` or ``byval()`` +``byval()`` This indicates that the pointer parameter should really be passed by value to the function. The attribute implies that a hidden copy of the pointee is made between the caller and the callee, so the callee @@ -1057,7 +1057,7 @@ Currently, only the following parameter attributes are defined: ``byval`` parameters). This is not a valid attribute for return values. - The byval attribute also supports an optional type argument, which + The byval type argument indicates the in-memory value type, and must be the same as the pointee type of the argument. The byval attribute also supports specifying an alignment with the @@ -1144,7 +1144,7 @@ Currently, only the following parameter attributes are defined: See :doc:`InAlloca` for more information on how to use this attribute. -``sret`` or ``sret()`` +``sret()`` This indicates that the pointer parameter specifies the address of a structure that is the return value of the function in the source program. This pointer must be guaranteed by the caller to be valid: @@ -1152,9 +1152,8 @@ Currently, only the following parameter attributes are defined: to trap and to be properly aligned. This is not a valid attribute for return values. - The sret attribute also supports an optional type argument, which - must be the same as the pointee type of the argument. In the - future this will be required. + The sret type argument specifies the in memory type, which must be + the same as the pointee type of the argument. .. _attr_align: From 4a6a4e573fe6dc000d717824106515459f2ff432 Mon Sep 17 00:00:00 2001 From: Florian Hahn Date: Wed, 16 Dec 2020 11:30:01 +0000 Subject: [PATCH 06/10] [InstCombine] Precommit tests for !annotation metadata handling. --- .../Transforms/InstCombine/annotations.ll | 153 ++++++++++++++++++ 1 file changed, 153 insertions(+) create mode 100644 llvm/test/Transforms/InstCombine/annotations.ll diff --git a/llvm/test/Transforms/InstCombine/annotations.ll b/llvm/test/Transforms/InstCombine/annotations.ll new file mode 100644 index 00000000000000..1530b867bfb680 --- /dev/null +++ b/llvm/test/Transforms/InstCombine/annotations.ll @@ -0,0 +1,153 @@ +; RUN: opt < %s -instcombine -S | FileCheck --match-full-lines %s + +; Test cases to make sure !annotation metadata is preserved, if possible. +; Currently we fail to preserve !annotation metadata in many cases. + +; Make sure !annotation metadata is added to new instructions, if the source +; instruction has !annotation metadata. +define i1 @fold_to_new_instruction(i8* %a, i8* %b) { +; CHECK-LABEL: define {{.+}} @fold_to_new_instruction({{.+}} +; CHECK-NEXT: [[C:%.*]] = icmp uge i8* [[A:%.*]], [[B:%[a-z]*]] +; CHECK-NEXT: ret i1 [[C]] +; + %a.c = bitcast i8* %a to i32*, !annotation !0 + %b.c = bitcast i8* %b to i32*, !annotation !0 + %c = icmp uge i32* %a.c, %b.c, !annotation !0 + ret i1 %c +} + +; Make sure !annotation is not added to new instructions if the source +; instruction does not have it (even if some folded operands do have +; !annotation). +define i1 @fold_to_new_instruction2(i8* %a, i8* %b) { +; CHECK-LABEL: define {{.+}} @fold_to_new_instruction2({{.+}} +; CHECK-NEXT: [[C:%.*]] = icmp uge i8* [[A:%.*]], [[B:%[a-z]+]] +; CHECK-NEXT: ret i1 [[C]] +; + %a.c = bitcast i8* %a to i32*, !annotation !0 + %b.c = bitcast i8* %b to i32*, !annotation !0 + %c = icmp uge i32* %a.c, %b.c + ret i1 %c +} + +; Make sure !annotation metadata is *not* added if we replace an instruction +; with !annotation with an existing one without. +define i32 @do_not_add_annotation_to_existing_instr(i32 %a, i32 %b) { +; CHECK-LABEL: define {{.+}} @do_not_add_annotation_to_existing_instr({{.+}} +; CHECK-NEXT: [[ADD:%.*]] = add i32 [[A:%.*]], [[B:%[a-z]+]] +; CHECK-NEXT: ret i32 [[ADD]] +; + %add = add i32 %a, %b + %res = add i32 0, %add, !annotation !0 + ret i32 %res +} + +; memcpy can be expanded inline with load/store. Verify that we keep the +; !annotation metadata. + +declare void @llvm.memcpy.p0i8.p0i8.i32(i8* nocapture, i8* nocapture, i32, i1) nounwind + +define void @copy_1_byte(i8* %d, i8* %s) { +; CHECK-LABEL: define {{.+}} @copy_1_byte({{.+}} +; CHECK-NEXT: [[TMP1:%.*]] = load i8, i8* [[S:%.*]], align 1 +; CHECK-NEXT: store i8 [[TMP1]], i8* [[D:%.*]], align 1 +; CHECK-NEXT: ret void +; + call void @llvm.memcpy.p0i8.p0i8.i32(i8* %d, i8* %s, i32 1, i1 false), !annotation !0 + ret void +} + +declare i8* @memcpy(i8* noalias returned, i8* noalias nocapture readonly, i64) nofree nounwind + +define void @libcallcopy_1_byte(i8* %d, i8* %s) { +; CHECK-LABEL: define {{.+}} @libcallcopy_1_byte({{.+}} +; CHECK-NEXT: [[TMP1:%.*]] = load i8, i8* [[S:%.*]], align 1 +; CHECK-NEXT: store i8 [[TMP1]], i8* [[D:%.*]], align 1 +; CHECK-NEXT: ret void +; + call i8* @memcpy(i8* %d, i8* %s, i64 1), !annotation !0 + ret void +} + +declare i8* @__memcpy_chk(i8*, i8*, i64, i64) nofree nounwind + +define void @libcallcopy_1_byte_chk(i8* %d, i8* %s) { +; CHECK-LABEL: define {{.+}} @libcallcopy_1_byte_chk({{.+}} +; CHECK-NEXT: [[TMP1:%.*]] = load i8, i8* [[S:%.*]], align 1 +; CHECK-NEXT: store i8 [[TMP1]], i8* [[D:%.*]], align 1 +; CHECK-NEXT: ret void +; + call i8* @__memcpy_chk(i8* %d, i8* %s, i64 1, i64 1), !annotation !0 + ret void +} + +declare void @llvm.memmove.p0i8.p0i8.i32(i8* nocapture, i8* nocapture readonly, i32, i1) nounwind + +define void @move_1_byte(i8* %d, i8* %s) { +; CHECK-LABEL: define {{.+}} @move_1_byte({{.+}} +; CHECK-NEXT: [[TMP1:%.*]] = load i8, i8* [[S:%.*]], align 1 +; CHECK-NEXT: store i8 [[TMP1]], i8* [[D:%.*]], align 1 +; CHECK-NEXT: ret void +; + call void @llvm.memmove.p0i8.p0i8.i32(i8* %d, i8* %s, i32 1, i1 false), !annotation !0 + ret void +} + +declare i8* @memmove(i8* returned, i8* nocapture readonly, i64) nofree nounwind + +define void @libcallmove_1_byte(i8* %d, i8* %s) { +; CHECK-LABEL: define {{.+}} @libcallmove_1_byte({{.+}} +; CHECK-NEXT: [[TMP1:%.*]] = load i8, i8* [[S:%.*]], align 1 +; CHECK-NEXT: store i8 [[TMP1]], i8* [[D:%.*]], align 1 +; CHECK-NEXT: ret void +; + call i8* @memmove(i8* %d, i8* %s, i64 1), !annotation !0 + ret void +} + +declare i8* @__memmove_chk(i8*, i8*, i64, i64) nofree nounwind + +define void @libcallmove_1_byte_chk(i8* %d, i8* %s) { +; CHECK-LABEL: define {{.+}} @libcallmove_1_byte_chk({{.+}} +; CHECK-NEXT: [[TMP1:%.*]] = load i8, i8* [[S:%.*]], align 1 +; CHECK-NEXT: store i8 [[TMP1]], i8* [[D:%.*]], align 1 +; CHECK-NEXT: ret void +; + call i8* @__memmove_chk(i8* %d, i8* %s, i64 1, i64 1), !annotation !0 + ret void +} + +declare void @llvm.memset.p0i8.i32(i8* nocapture writeonly, i8, i32, i1) argmemonly nounwind + +define void @set_1_byte(i8* %d) { +; CHECK-LABEL: define {{.+}} @set_1_byte({{.+}} +; CHECK-NEXT: store i8 1, i8* [[D:%.*]], align 1 +; CHECK-NEXT: ret void +; + call void @llvm.memset.p0i8.i32(i8* %d, i8 1, i32 1, i1 false), !annotation !0 + ret void +} + +declare i8* @memset(i8*, i32, i64) nofree + +define void @libcall_set_1_byte(i8* %d) { +; CHECK-LABEL: define {{.+}} @libcall_set_1_byte({{.+}} +; CHECK-NEXT: store i8 1, i8* [[D:%.*]], align 1 +; CHECK-NEXT: ret void +; + call i8* @memset(i8* %d, i32 1, i64 1), !annotation !0 + ret void +} + +declare i8* @__memset_chk(i8*, i32, i64, i64) nofree + +define void @libcall_set_1_byte_chk(i8* %d) { +; CHECK-LABEL: define {{.+}} @libcall_set_1_byte_chk({{.+}} +; CHECK-NEXT: store i8 1, i8* [[D:%.*]], align 1 +; CHECK-NEXT: ret void +; + call i8* @__memset_chk(i8* %d, i32 1, i64 1, i64 1), !annotation !0 + ret void +} + +!0 = !{ !"auto-init" } From e2e86f4e77ec2fd79743f4d0e94689e9668600ad Mon Sep 17 00:00:00 2001 From: Kai Nacke Date: Wed, 16 Dec 2020 06:01:08 -0500 Subject: [PATCH 07/10] [Doc][SystemZ] Add Linux/SystemZ to Getting Started guide. The Linux/SystemZ platform is missing in the Getting Started guide as platform on which LLVM is known to work. Reviewed by: uweigand Differential Revision: https://reviews.llvm.org/D93388 --- llvm/docs/GettingStarted.rst | 1 + 1 file changed, 1 insertion(+) diff --git a/llvm/docs/GettingStarted.rst b/llvm/docs/GettingStarted.rst index 70e12b0f877a04..05d2994fed2b1e 100644 --- a/llvm/docs/GettingStarted.rst +++ b/llvm/docs/GettingStarted.rst @@ -121,6 +121,7 @@ Linux amd64 GCC, Clang Linux ARM GCC, Clang Linux Mips GCC, Clang Linux PowerPC GCC, Clang +Linux SystemZ GCC, Clang Solaris V9 (Ultrasparc) GCC FreeBSD x86\ :sup:`1` GCC, Clang FreeBSD amd64 GCC, Clang From 07751310580fa5b7b94b6efa85d7964af0f699a6 Mon Sep 17 00:00:00 2001 From: peter klausler Date: Tue, 15 Dec 2020 10:59:26 -0800 Subject: [PATCH 08/10] [flang] Fix crash in folding (#48437) Elemental intrinsic function folding was not taking the lower bounds of constant array arguments into account; these lower bounds can be distinct from 1 when named constants appear as arguments. LLVM bugzilla #48437. Differential Revision: https://reviews.llvm.org/D93321 --- flang/include/flang/Evaluate/constant.h | 5 ++-- flang/lib/Evaluate/fold-implementation.h | 36 +++++++++--------------- flang/test/Evaluate/folding16.f90 | 8 ++++++ 3 files changed, 25 insertions(+), 24 deletions(-) create mode 100644 flang/test/Evaluate/folding16.f90 diff --git a/flang/include/flang/Evaluate/constant.h b/flang/include/flang/Evaluate/constant.h index a9f6e87c9db03f..89a5867722f72f 100644 --- a/flang/include/flang/Evaluate/constant.h +++ b/flang/include/flang/Evaluate/constant.h @@ -140,7 +140,8 @@ template class Constant : public ConstantBase { } } - // Apply subscripts. + // Apply subscripts. An empty subscript list is allowed for + // a scalar constant. Element At(const ConstantSubscripts &) const; Constant Reshape(ConstantSubscripts &&) const; @@ -177,7 +178,7 @@ class Constant> : public ConstantBounds { } } - // Apply subscripts + // Apply subscripts, if any. Scalar At(const ConstantSubscripts &) const; Constant Reshape(ConstantSubscripts &&) const; diff --git a/flang/lib/Evaluate/fold-implementation.h b/flang/lib/Evaluate/fold-implementation.h index 4fa5f6a4c88379..7232715600fd55 100644 --- a/flang/lib/Evaluate/fold-implementation.h +++ b/flang/lib/Evaluate/fold-implementation.h @@ -443,9 +443,8 @@ Expr FoldElementalIntrinsicHelper(FoldingContext &context, // Compute the shape of the result based on shapes of arguments ConstantSubscripts shape; int rank{0}; - const ConstantSubscripts *shapes[sizeof...(TA)]{ - &std::get(*args)->shape()...}; - const int ranks[sizeof...(TA)]{std::get(*args)->Rank()...}; + const ConstantSubscripts *shapes[]{&std::get(*args)->shape()...}; + const int ranks[]{std::get(*args)->Rank()...}; for (unsigned int i{0}; i < sizeof...(TA); ++i) { if (ranks[i] > 0) { if (rank == 0) { @@ -470,20 +469,19 @@ Expr FoldElementalIntrinsicHelper(FoldingContext &context, std::vector> results; if (TotalElementCount(shape) > 0) { ConstantBounds bounds{shape}; - ConstantSubscripts index(rank, 1); + ConstantSubscripts resultIndex(rank, 1); + ConstantSubscripts argIndex[]{std::get(*args)->lbounds()...}; do { if constexpr (std::is_same_v, ScalarFuncWithContext>) { - results.emplace_back(func(context, - (ranks[I] ? std::get(*args)->At(index) - : std::get(*args)->GetScalarValue().value())...)); + results.emplace_back( + func(context, std::get(*args)->At(argIndex[I])...)); } else if constexpr (std::is_same_v, ScalarFunc>) { - results.emplace_back(func( - (ranks[I] ? std::get(*args)->At(index) - : std::get(*args)->GetScalarValue().value())...)); + results.emplace_back(func(std::get(*args)->At(argIndex[I])...)); } - } while (bounds.IncrementSubscripts(index)); + (std::get(*args)->IncrementSubscripts(argIndex[I]), ...); + } while (bounds.IncrementSubscripts(resultIndex)); } // Build and return constant result if constexpr (TR::category == TypeCategory::Character) { @@ -732,17 +730,11 @@ template class ArrayConstructorFolder { Expr folded{Fold(context_, common::Clone(expr.value()))}; if (const auto *c{UnwrapConstantValue(folded)}) { // Copy elements in Fortran array element order - ConstantSubscripts shape{c->shape()}; - int rank{c->Rank()}; - ConstantSubscripts index(GetRank(shape), 1); - for (std::size_t n{c->size()}; n-- > 0;) { - elements_.emplace_back(c->At(index)); - for (int d{0}; d < rank; ++d) { - if (++index[d] <= shape[d]) { - break; - } - index[d] = 1; - } + if (c->size() > 0) { + ConstantSubscripts index{c->lbounds()}; + do { + elements_.emplace_back(c->At(index)); + } while (c->IncrementSubscripts(index)); } return true; } else { diff --git a/flang/test/Evaluate/folding16.f90 b/flang/test/Evaluate/folding16.f90 new file mode 100644 index 00000000000000..0918381040bfea --- /dev/null +++ b/flang/test/Evaluate/folding16.f90 @@ -0,0 +1,8 @@ +! RUN: %S/test_folding.sh %s %t %f18 +! Ensure that lower bounds are accounted for in intrinsic folding; +! this is a regression test for a bug in which they were not +real, parameter :: a(-1:-1) = 1. +real, parameter :: b(-1:-1) = log(a) +logical, parameter :: test = lbound(a,1)==-1 .and. lbound(b,1)==-1 .and. & + lbound(log(a),1)==1 .and. all(b==0) +end From 6e890ec7beb0874464a0af9f84e41a987f968b23 Mon Sep 17 00:00:00 2001 From: Jonas Hahnfeld Date: Sat, 5 Dec 2020 12:52:38 +0100 Subject: [PATCH 09/10] [CMake] Avoid __FakeVCSRevision.h with no git repository Set the return variable to "" in find_first_existing_vc_file to say that there is a repository, but no file to depend on. This works transparently for all other callers that handle undefinedness and equality to an empty string the same way. Use the knowledge to avoid depending on __FakeVCSRevision.h if there is no git repository at all (for example when building a release) as there is no point in regenerating an empty VCSRevision.h. Differential Revision: https://reviews.llvm.org/D92718 --- llvm/cmake/modules/AddLLVM.cmake | 8 ++++++++ llvm/include/llvm/Support/CMakeLists.txt | 2 +- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/llvm/cmake/modules/AddLLVM.cmake b/llvm/cmake/modules/AddLLVM.cmake index a3e9eaeeb2622d..b86fbdaaa6d839 100644 --- a/llvm/cmake/modules/AddLLVM.cmake +++ b/llvm/cmake/modules/AddLLVM.cmake @@ -2135,6 +2135,13 @@ function(setup_dependency_debugging name) set_target_properties(${name} PROPERTIES RULE_LAUNCH_COMPILE ${sandbox_command}) endfunction() +# If the sources at the given `path` are under version control, set `out_var` +# to the the path of a file which will be modified when the VCS revision +# changes, attempting to create that file if it does not exist; if no such +# file exists and one cannot be created, instead set `out_var` to the +# empty string. +# +# If the sources are not under version control, do not define `out_var`. function(find_first_existing_vc_file path out_var) if(NOT EXISTS "${path}") return() @@ -2156,6 +2163,7 @@ function(find_first_existing_vc_file path out_var) RESULT_VARIABLE touch_head_result ERROR_QUIET) if (NOT touch_head_result EQUAL 0) + set(${out_var} "" PARENT_SCOPE) return() endif() endif() diff --git a/llvm/include/llvm/Support/CMakeLists.txt b/llvm/include/llvm/Support/CMakeLists.txt index aa71b557218109..aeb5866ecbddee 100644 --- a/llvm/include/llvm/Support/CMakeLists.txt +++ b/llvm/include/llvm/Support/CMakeLists.txt @@ -11,7 +11,7 @@ if(LLVM_APPEND_VC_REV) # A fake version file and is not expected to exist. It is being used to # force regeneration of VCSRevision.h for source directory with no write # permission available. - if (NOT llvm_vc) + if (llvm_vc STREQUAL "") set(fake_version_inc "${CMAKE_CURRENT_BINARY_DIR}/__FakeVCSRevision.h") endif() endif() From b607837c75d04cc007dcf855983dfa3b69f63d73 Mon Sep 17 00:00:00 2001 From: Jon Chesterfield Date: Wed, 16 Dec 2020 16:40:21 +0000 Subject: [PATCH 10/10] [libomptarget][nfc] Replace static const with enum [libomptarget][nfc] Replace static const with enum Semantically identical. Replaces 0xff... with ~0 to spare counting the f. Has the advantage that the compiler doesn't need to prove the 4/8 byte value dead before discarding it, and sidesteps the compilation question associated with what static means for a single source language. Reviewed By: jdoerfert Differential Revision: https://reviews.llvm.org/D93328 --- openmp/libomptarget/deviceRTLs/amdgcn/src/target_impl.h | 5 +++-- openmp/libomptarget/deviceRTLs/nvptx/src/target_impl.h | 5 +++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/openmp/libomptarget/deviceRTLs/amdgcn/src/target_impl.h b/openmp/libomptarget/deviceRTLs/amdgcn/src/target_impl.h index 34794587e0fe7d..d25ea8559c05d8 100644 --- a/openmp/libomptarget/deviceRTLs/amdgcn/src/target_impl.h +++ b/openmp/libomptarget/deviceRTLs/amdgcn/src/target_impl.h @@ -74,8 +74,9 @@ INLINE uint64_t __kmpc_impl_pack(uint32_t lo, uint32_t hi) { return (((uint64_t)hi) << 32) | (uint64_t)lo; } -static const __kmpc_impl_lanemask_t __kmpc_impl_all_lanes = - UINT64_C(0xffffffffffffffff); +enum : __kmpc_impl_lanemask_t { + __kmpc_impl_all_lanes = ~(__kmpc_impl_lanemask_t)0 +}; DEVICE __kmpc_impl_lanemask_t __kmpc_impl_lanemask_lt(); diff --git a/openmp/libomptarget/deviceRTLs/nvptx/src/target_impl.h b/openmp/libomptarget/deviceRTLs/nvptx/src/target_impl.h index 46ce751c44c4dc..411e1676b7c7da 100644 --- a/openmp/libomptarget/deviceRTLs/nvptx/src/target_impl.h +++ b/openmp/libomptarget/deviceRTLs/nvptx/src/target_impl.h @@ -91,8 +91,9 @@ INLINE uint64_t __kmpc_impl_pack(uint32_t lo, uint32_t hi) { return val; } -static const __kmpc_impl_lanemask_t __kmpc_impl_all_lanes = - UINT32_C(0xffffffff); +enum : __kmpc_impl_lanemask_t { + __kmpc_impl_all_lanes = ~(__kmpc_impl_lanemask_t)0 +}; INLINE __kmpc_impl_lanemask_t __kmpc_impl_lanemask_lt() { __kmpc_impl_lanemask_t res;