From a649e0a6e80f5ae26657b640de469e8a53244ad2 Mon Sep 17 00:00:00 2001 From: Ian Anderson Date: Wed, 13 Mar 2024 11:15:41 -0700 Subject: [PATCH 01/25] [clang][modules] giving the __stddef_ headers their own modules can cause redeclaration errors with -fbuiltin-headers-in-system-modules (#84127) On Apple platforms, some of the stddef.h types are also declared in system headers. In particular NULL has a conflicting declaration in . When that's in a different module from <__stddef_null.h>, redeclaration errors can occur. Make the \_\_stddef_ headers be non-modular in -fbuiltin-headers-in-system-modules and restore them back to not respecting their header guards. Still define the header guards though. __stddef_max_align_t.h was in _Builtin_stddef_max_align_t prior to the addition of _Builtin_stddef, and it needs to stay in a module because struct's can't be type merged. __stddef_wint_t.h didn't used to have a module, but leave it in it current module since it doesn't really belong to stddef.h. (cherry picked from commit f50d3582b4844b86ad86372028e44b52c560ec7d) --- clang/lib/Basic/Module.cpp | 7 ++-- clang/lib/Headers/__stddef_null.h | 2 +- clang/lib/Headers/__stddef_nullptr_t.h | 7 +++- clang/lib/Headers/__stddef_offsetof.h | 7 +++- clang/lib/Headers/__stddef_ptrdiff_t.h | 7 +++- clang/lib/Headers/__stddef_rsize_t.h | 7 +++- clang/lib/Headers/__stddef_size_t.h | 7 +++- clang/lib/Headers/__stddef_unreachable.h | 7 +++- clang/lib/Headers/__stddef_wchar_t.h | 7 +++- clang/lib/Headers/module.modulemap | 20 ++++++------ clang/lib/Lex/ModuleMap.cpp | 9 ++++-- .../no-undeclared-includes-builtins.cpp | 2 +- clang/test/Modules/stddef.c | 32 +++++++++++-------- 13 files changed, 80 insertions(+), 41 deletions(-) diff --git a/clang/lib/Basic/Module.cpp b/clang/lib/Basic/Module.cpp index 925217431d4d0..0dac8748a98af 100644 --- a/clang/lib/Basic/Module.cpp +++ b/clang/lib/Basic/Module.cpp @@ -301,10 +301,9 @@ bool Module::directlyUses(const Module *Requested) { if (Requested->isSubModuleOf(Use)) return true; - // Anyone is allowed to use our builtin stdarg.h and stddef.h and their - // accompanying modules. - if (Requested->getTopLevelModuleName() == "_Builtin_stdarg" || - Requested->getTopLevelModuleName() == "_Builtin_stddef") + // Anyone is allowed to use our builtin stddef.h and its accompanying modules. + if (Requested->fullModuleNameIs({"_Builtin_stddef", "max_align_t"}) || + Requested->fullModuleNameIs({"_Builtin_stddef_wint_t"})) return true; if (NoUndeclaredIncludes) diff --git a/clang/lib/Headers/__stddef_null.h b/clang/lib/Headers/__stddef_null.h index 7336fdab38972..c10bd2d7d9887 100644 --- a/clang/lib/Headers/__stddef_null.h +++ b/clang/lib/Headers/__stddef_null.h @@ -7,7 +7,7 @@ *===-----------------------------------------------------------------------=== */ -#if !defined(NULL) || !__has_feature(modules) +#if !defined(NULL) || !__building_module(_Builtin_stddef) /* linux/stddef.h will define NULL to 0. glibc (and other) headers then define * __need_NULL and rely on stddef.h to redefine NULL to the correct value again. diff --git a/clang/lib/Headers/__stddef_nullptr_t.h b/clang/lib/Headers/__stddef_nullptr_t.h index 183d394d56c1b..7f3fbe6fe0d3a 100644 --- a/clang/lib/Headers/__stddef_nullptr_t.h +++ b/clang/lib/Headers/__stddef_nullptr_t.h @@ -7,7 +7,12 @@ *===-----------------------------------------------------------------------=== */ -#ifndef _NULLPTR_T +/* + * When -fbuiltin-headers-in-system-modules is set this is a non-modular header + * and needs to behave as if it was textual. + */ +#if !defined(_NULLPTR_T) || \ + (__has_feature(modules) && !__building_module(_Builtin_stddef)) #define _NULLPTR_T #ifdef __cplusplus diff --git a/clang/lib/Headers/__stddef_offsetof.h b/clang/lib/Headers/__stddef_offsetof.h index 3b347b3b92f62..84172c6cd2735 100644 --- a/clang/lib/Headers/__stddef_offsetof.h +++ b/clang/lib/Headers/__stddef_offsetof.h @@ -7,6 +7,11 @@ *===-----------------------------------------------------------------------=== */ -#ifndef offsetof +/* + * When -fbuiltin-headers-in-system-modules is set this is a non-modular header + * and needs to behave as if it was textual. + */ +#if !defined(offsetof) || \ + (__has_feature(modules) && !__building_module(_Builtin_stddef)) #define offsetof(t, d) __builtin_offsetof(t, d) #endif diff --git a/clang/lib/Headers/__stddef_ptrdiff_t.h b/clang/lib/Headers/__stddef_ptrdiff_t.h index 3ea6d7d2852e1..fd3c893c66c97 100644 --- a/clang/lib/Headers/__stddef_ptrdiff_t.h +++ b/clang/lib/Headers/__stddef_ptrdiff_t.h @@ -7,7 +7,12 @@ *===-----------------------------------------------------------------------=== */ -#ifndef _PTRDIFF_T +/* + * When -fbuiltin-headers-in-system-modules is set this is a non-modular header + * and needs to behave as if it was textual. + */ +#if !defined(_PTRDIFF_T) || \ + (__has_feature(modules) && !__building_module(_Builtin_stddef)) #define _PTRDIFF_T typedef __PTRDIFF_TYPE__ ptrdiff_t; diff --git a/clang/lib/Headers/__stddef_rsize_t.h b/clang/lib/Headers/__stddef_rsize_t.h index b6428d0c12b62..dd433d40d9733 100644 --- a/clang/lib/Headers/__stddef_rsize_t.h +++ b/clang/lib/Headers/__stddef_rsize_t.h @@ -7,7 +7,12 @@ *===-----------------------------------------------------------------------=== */ -#ifndef _RSIZE_T +/* + * When -fbuiltin-headers-in-system-modules is set this is a non-modular header + * and needs to behave as if it was textual. + */ +#if !defined(_RSIZE_T) || \ + (__has_feature(modules) && !__building_module(_Builtin_stddef)) #define _RSIZE_T typedef __SIZE_TYPE__ rsize_t; diff --git a/clang/lib/Headers/__stddef_size_t.h b/clang/lib/Headers/__stddef_size_t.h index e4a389510bcdb..3dd7b1f379294 100644 --- a/clang/lib/Headers/__stddef_size_t.h +++ b/clang/lib/Headers/__stddef_size_t.h @@ -7,7 +7,12 @@ *===-----------------------------------------------------------------------=== */ -#ifndef _SIZE_T +/* + * When -fbuiltin-headers-in-system-modules is set this is a non-modular header + * and needs to behave as if it was textual. + */ +#if !defined(_SIZE_T) || \ + (__has_feature(modules) && !__building_module(_Builtin_stddef)) #define _SIZE_T typedef __SIZE_TYPE__ size_t; diff --git a/clang/lib/Headers/__stddef_unreachable.h b/clang/lib/Headers/__stddef_unreachable.h index 3e7fe01979662..518580c92d3f5 100644 --- a/clang/lib/Headers/__stddef_unreachable.h +++ b/clang/lib/Headers/__stddef_unreachable.h @@ -7,6 +7,11 @@ *===-----------------------------------------------------------------------=== */ -#ifndef unreachable +/* + * When -fbuiltin-headers-in-system-modules is set this is a non-modular header + * and needs to behave as if it was textual. + */ +#if !defined(unreachable) || \ + (__has_feature(modules) && !__building_module(_Builtin_stddef)) #define unreachable() __builtin_unreachable() #endif diff --git a/clang/lib/Headers/__stddef_wchar_t.h b/clang/lib/Headers/__stddef_wchar_t.h index 16a6186512c0c..bd69f63225416 100644 --- a/clang/lib/Headers/__stddef_wchar_t.h +++ b/clang/lib/Headers/__stddef_wchar_t.h @@ -9,7 +9,12 @@ #if !defined(__cplusplus) || (defined(_MSC_VER) && !_NATIVE_WCHAR_T_DEFINED) -#ifndef _WCHAR_T +/* + * When -fbuiltin-headers-in-system-modules is set this is a non-modular header + * and needs to behave as if it was textual. + */ +#if !defined(_WCHAR_T) || \ + (__has_feature(modules) && !__building_module(_Builtin_stddef)) #define _WCHAR_T #ifdef _MSC_EXTENSIONS diff --git a/clang/lib/Headers/module.modulemap b/clang/lib/Headers/module.modulemap index a786689d39177..56a13f69bc055 100644 --- a/clang/lib/Headers/module.modulemap +++ b/clang/lib/Headers/module.modulemap @@ -155,9 +155,9 @@ module _Builtin_intrinsics [system] [extern_c] { // Start -fbuiltin-headers-in-system-modules affected modules -// The following modules all ignore their top level headers -// when -fbuiltin-headers-in-system-modules is passed, and -// most of those headers join system modules when present. +// The following modules all ignore their headers when +// -fbuiltin-headers-in-system-modules is passed, and many of +// those headers join system modules when present. // e.g. if -fbuiltin-headers-in-system-modules is passed, then // float.h will not be in the _Builtin_float module (that module @@ -190,11 +190,6 @@ module _Builtin_stdalign [system] { export * } -// When -fbuiltin-headers-in-system-modules is passed, only -// the top level headers are removed, the implementation headers -// will always be in their submodules. That means when stdarg.h -// is included, it will still import this module and make the -// appropriate submodules visible. module _Builtin_stdarg [system] { textual header "stdarg.h" @@ -237,6 +232,8 @@ module _Builtin_stdbool [system] { module _Builtin_stddef [system] { textual header "stddef.h" + // __stddef_max_align_t.h is always in this module, even if + // -fbuiltin-headers-in-system-modules is passed. explicit module max_align_t { header "__stddef_max_align_t.h" export * @@ -283,9 +280,10 @@ module _Builtin_stddef [system] { } } -/* wint_t is provided by and not . It's here - * for compatibility, but must be explicitly requested. Therefore - * __stddef_wint_t.h is not part of _Builtin_stddef. */ +// wint_t is provided by and not . It's here +// for compatibility, but must be explicitly requested. Therefore +// __stddef_wint_t.h is not part of _Builtin_stddef. It is always in +// this module even if -fbuiltin-headers-in-system-modules is passed. module _Builtin_stddef_wint_t [system] { header "__stddef_wint_t.h" export * diff --git a/clang/lib/Lex/ModuleMap.cpp b/clang/lib/Lex/ModuleMap.cpp index afb2948f05ae5..10c475f617d48 100644 --- a/clang/lib/Lex/ModuleMap.cpp +++ b/clang/lib/Lex/ModuleMap.cpp @@ -2498,9 +2498,12 @@ void ModuleMapParser::parseHeaderDecl(MMToken::TokenKind LeadingToken, } bool NeedsFramework = false; - // Don't add the top level headers to the builtin modules if the builtin headers - // belong to the system modules. - if (!Map.LangOpts.BuiltinHeadersInSystemModules || ActiveModule->isSubModule() || !isBuiltInModuleName(ActiveModule->Name)) + // Don't add headers to the builtin modules if the builtin headers belong to + // the system modules, with the exception of __stddef_max_align_t.h which + // always had its own module. + if (!Map.LangOpts.BuiltinHeadersInSystemModules || + !isBuiltInModuleName(ActiveModule->getTopLevelModuleName()) || + ActiveModule->fullModuleNameIs({"_Builtin_stddef", "max_align_t"})) Map.addUnresolvedHeader(ActiveModule, std::move(Header), NeedsFramework); if (NeedsFramework) diff --git a/clang/test/Modules/no-undeclared-includes-builtins.cpp b/clang/test/Modules/no-undeclared-includes-builtins.cpp index c9bffc5561990..f9eefd24a33c7 100644 --- a/clang/test/Modules/no-undeclared-includes-builtins.cpp +++ b/clang/test/Modules/no-undeclared-includes-builtins.cpp @@ -8,7 +8,7 @@ // headers. // RUN: rm -rf %t -// RUN: %clang_cc1 -fmodules-cache-path=%t -fmodules -fimplicit-module-maps -I %S/Inputs/no-undeclared-includes-builtins/libcxx -I %S/Inputs/no-undeclared-includes-builtins/glibc %s +// RUN: %clang_cc1 -fmodules-cache-path=%t -fmodules -fbuiltin-headers-in-system-modules -fimplicit-module-maps -I %S/Inputs/no-undeclared-includes-builtins/libcxx -I %S/Inputs/no-undeclared-includes-builtins/glibc %s // expected-no-diagnostics #include diff --git a/clang/test/Modules/stddef.c b/clang/test/Modules/stddef.c index 5bc0d1e44c856..7623982614681 100644 --- a/clang/test/Modules/stddef.c +++ b/clang/test/Modules/stddef.c @@ -1,29 +1,33 @@ // RUN: rm -rf %t -// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fbuiltin-headers-in-system-modules -fmodules-cache-path=%t -I%S/Inputs/StdDef %s -verify -fno-modules-error-recovery +// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fbuiltin-headers-in-system-modules -fmodules-cache-path=%t -I%S/Inputs/StdDef %s -verify=builtin-headers-in-system-modules -fno-modules-error-recovery // RUN: rm -rf %t -// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -I%S/Inputs/StdDef %s -verify -fno-modules-error-recovery +// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t -I%S/Inputs/StdDef %s -verify=no-builtin-headers-in-system-modules -fno-modules-error-recovery #include "ptrdiff_t.h" ptrdiff_t pdt; -// size_t is declared in both size_t.h and __stddef_size_t.h, both of which are -// modular headers. Regardless of whether stddef.h joins the StdDef test module -// or is in its _Builtin_stddef module, __stddef_size_t.h will be in -// _Builtin_stddef.size_t. It's not defined which module will win as the expected -// provider of size_t. For the purposes of this test it doesn't matter which header -// gets reported, just as long as it isn't other.h or include_again.h. -size_t st; // expected-error-re {{missing '#include "{{size_t|__stddef_size_t}}.h"'; 'size_t' must be declared before it is used}} -// expected-note@size_t.h:* 0+ {{here}} -// expected-note@__stddef_size_t.h:* 0+ {{here}} +// size_t is declared in both size_t.h and __stddef_size_t.h. If +// -fbuiltin-headers-in-system-modules is set, then __stddef_size_t.h is a +// non-modular header that will be transitively pulled in the StdDef test module +// by include_again.h. Otherwise it will be in the _Builtin_stddef module. In +// any case it's not defined which module will win as the expected provider of +// size_t. For the purposes of this test it doesn't matter which of the two +// providing headers get reported. +size_t st; // builtin-headers-in-system-modules-error-re {{missing '#include "{{size_t|include_again}}.h"'; 'size_t' must be declared before it is used}} \ + no-builtin-headers-in-system-modules-error-re {{missing '#include "{{size_t|__stddef_size_t}}.h"'; 'size_t' must be declared before it is used}} +// builtin-headers-in-system-modules-note@size_t.h:* 0+ {{here}} \ + no-builtin-headers-in-system-modules-note@size_t.h:* 0+ {{here}} +// builtin-headers-in-system-modules-note@__stddef_size_t.h:* 0+ {{here}} \ + no-builtin-headers-in-system-modules-note@__stddef_size_t.h:* 0+ {{here}} #include "include_again.h" -// Includes which includes <__stddef_size_t.h> which imports the -// _Builtin_stddef.size_t module. +// Includes which includes <__stddef_size_t.h>. size_t st2; #include "size_t.h" -// Redeclares size_t, but the type merger should figure it out. +// Redeclares size_t when -fbuiltin-headers-in-system-modules is not passed, but +// the type merger should figure it out. size_t st3; From 0c1dcd675291f058d530078f9304162ea9d27cd1 Mon Sep 17 00:00:00 2001 From: Nathan Ridge Date: Mon, 4 Mar 2024 00:12:56 -0500 Subject: [PATCH 02/25] [clangd] [HeuristicResolver] Protect against infinite recursion on DependentNameTypes (#83542) When resolving names inside templates that implement recursive compile-time functions (e.g. waldo::type is defined in terms of waldo::type), HeuristicResolver could get into an infinite recursion, specifically one where resolveDependentNameType() can be called recursively with the same DependentNameType*. To guard against this, HeuristicResolver tracks, for each external call into a HeuristicResolver function, the set of DependentNameTypes that it has seen, and bails if it sees the same DependentNameType again. To implement this, a helper class HeuristicResolverImpl is introduced to store state that persists for the duration of an external call into HeuristicResolver (but does not persist between such calls). Fixes https://github.com/clangd/clangd/issues/1951 (cherry picked from commit e6e53ca8470d719882539359ebe3ad8b442a8cb0) --- .../clangd/HeuristicResolver.cpp | 170 ++++++++++++++---- clang-tools-extra/clangd/HeuristicResolver.h | 37 ---- .../clangd/unittests/FindTargetTests.cpp | 27 +++ 3 files changed, 165 insertions(+), 69 deletions(-) diff --git a/clang-tools-extra/clangd/HeuristicResolver.cpp b/clang-tools-extra/clangd/HeuristicResolver.cpp index 3c147b6b582bf..26d54200eeffd 100644 --- a/clang-tools-extra/clangd/HeuristicResolver.cpp +++ b/clang-tools-extra/clangd/HeuristicResolver.cpp @@ -16,6 +16,80 @@ namespace clang { namespace clangd { +namespace { + +// Helper class for implementing HeuristicResolver. +// Unlike HeuristicResolver which is a long-lived class, +// a new instance of this class is created for every external +// call into a HeuristicResolver operation. That allows this +// class to store state that's local to such a top-level call, +// particularly "recursion protection sets" that keep track of +// nodes that have already been seen to avoid infinite recursion. +class HeuristicResolverImpl { +public: + HeuristicResolverImpl(ASTContext &Ctx) : Ctx(Ctx) {} + + // These functions match the public interface of HeuristicResolver + // (but aren't const since they may modify the recursion protection sets). + std::vector + resolveMemberExpr(const CXXDependentScopeMemberExpr *ME); + std::vector + resolveDeclRefExpr(const DependentScopeDeclRefExpr *RE); + std::vector resolveTypeOfCallExpr(const CallExpr *CE); + std::vector resolveCalleeOfCallExpr(const CallExpr *CE); + std::vector + resolveUsingValueDecl(const UnresolvedUsingValueDecl *UUVD); + std::vector + resolveDependentNameType(const DependentNameType *DNT); + std::vector resolveTemplateSpecializationType( + const DependentTemplateSpecializationType *DTST); + const Type *resolveNestedNameSpecifierToType(const NestedNameSpecifier *NNS); + const Type *getPointeeType(const Type *T); + +private: + ASTContext &Ctx; + + // Recursion protection sets + llvm::SmallSet SeenDependentNameTypes; + + // Given a tag-decl type and a member name, heuristically resolve the + // name to one or more declarations. + // The current heuristic is simply to look up the name in the primary + // template. This is a heuristic because the template could potentially + // have specializations that declare different members. + // Multiple declarations could be returned if the name is overloaded + // (e.g. an overloaded method in the primary template). + // This heuristic will give the desired answer in many cases, e.g. + // for a call to vector::size(). + std::vector + resolveDependentMember(const Type *T, DeclarationName Name, + llvm::function_ref Filter); + + // Try to heuristically resolve the type of a possibly-dependent expression + // `E`. + const Type *resolveExprToType(const Expr *E); + std::vector resolveExprToDecls(const Expr *E); + + // Helper function for HeuristicResolver::resolveDependentMember() + // which takes a possibly-dependent type `T` and heuristically + // resolves it to a CXXRecordDecl in which we can try name lookup. + CXXRecordDecl *resolveTypeToRecordDecl(const Type *T); + + // This is a reimplementation of CXXRecordDecl::lookupDependentName() + // so that the implementation can call into other HeuristicResolver helpers. + // FIXME: Once HeuristicResolver is upstreamed to the clang libraries + // (https://github.com/clangd/clangd/discussions/1662), + // CXXRecordDecl::lookupDepenedentName() can be removed, and its call sites + // can be modified to benefit from the more comprehensive heuristics offered + // by HeuristicResolver instead. + std::vector + lookupDependentName(CXXRecordDecl *RD, DeclarationName Name, + llvm::function_ref Filter); + bool findOrdinaryMemberInDependentClasses(const CXXBaseSpecifier *Specifier, + CXXBasePath &Path, + DeclarationName Name); +}; + // Convenience lambdas for use as the 'Filter' parameter of // HeuristicResolver::resolveDependentMember(). const auto NoFilter = [](const NamedDecl *D) { return true; }; @@ -31,8 +105,6 @@ const auto TemplateFilter = [](const NamedDecl *D) { return isa(D); }; -namespace { - const Type *resolveDeclsToType(const std::vector &Decls, ASTContext &Ctx) { if (Decls.size() != 1) // Names an overload set -- just bail. @@ -46,12 +118,10 @@ const Type *resolveDeclsToType(const std::vector &Decls, return nullptr; } -} // namespace - // Helper function for HeuristicResolver::resolveDependentMember() // which takes a possibly-dependent type `T` and heuristically // resolves it to a CXXRecordDecl in which we can try name lookup. -CXXRecordDecl *HeuristicResolver::resolveTypeToRecordDecl(const Type *T) const { +CXXRecordDecl *HeuristicResolverImpl::resolveTypeToRecordDecl(const Type *T) { assert(T); // Unwrap type sugar such as type aliases. @@ -84,7 +154,7 @@ CXXRecordDecl *HeuristicResolver::resolveTypeToRecordDecl(const Type *T) const { return TD->getTemplatedDecl(); } -const Type *HeuristicResolver::getPointeeType(const Type *T) const { +const Type *HeuristicResolverImpl::getPointeeType(const Type *T) { if (!T) return nullptr; @@ -117,8 +187,8 @@ const Type *HeuristicResolver::getPointeeType(const Type *T) const { return FirstArg.getAsType().getTypePtrOrNull(); } -std::vector HeuristicResolver::resolveMemberExpr( - const CXXDependentScopeMemberExpr *ME) const { +std::vector HeuristicResolverImpl::resolveMemberExpr( + const CXXDependentScopeMemberExpr *ME) { // If the expression has a qualifier, try resolving the member inside the // qualifier's type. // Note that we cannot use a NonStaticFilter in either case, for a couple @@ -164,14 +234,14 @@ std::vector HeuristicResolver::resolveMemberExpr( return resolveDependentMember(BaseType, ME->getMember(), NoFilter); } -std::vector HeuristicResolver::resolveDeclRefExpr( - const DependentScopeDeclRefExpr *RE) const { +std::vector +HeuristicResolverImpl::resolveDeclRefExpr(const DependentScopeDeclRefExpr *RE) { return resolveDependentMember(RE->getQualifier()->getAsType(), RE->getDeclName(), StaticFilter); } std::vector -HeuristicResolver::resolveTypeOfCallExpr(const CallExpr *CE) const { +HeuristicResolverImpl::resolveTypeOfCallExpr(const CallExpr *CE) { const auto *CalleeType = resolveExprToType(CE->getCallee()); if (!CalleeType) return {}; @@ -187,7 +257,7 @@ HeuristicResolver::resolveTypeOfCallExpr(const CallExpr *CE) const { } std::vector -HeuristicResolver::resolveCalleeOfCallExpr(const CallExpr *CE) const { +HeuristicResolverImpl::resolveCalleeOfCallExpr(const CallExpr *CE) { if (const auto *ND = dyn_cast_or_null(CE->getCalleeDecl())) { return {ND}; } @@ -195,29 +265,31 @@ HeuristicResolver::resolveCalleeOfCallExpr(const CallExpr *CE) const { return resolveExprToDecls(CE->getCallee()); } -std::vector HeuristicResolver::resolveUsingValueDecl( - const UnresolvedUsingValueDecl *UUVD) const { +std::vector HeuristicResolverImpl::resolveUsingValueDecl( + const UnresolvedUsingValueDecl *UUVD) { return resolveDependentMember(UUVD->getQualifier()->getAsType(), UUVD->getNameInfo().getName(), ValueFilter); } -std::vector HeuristicResolver::resolveDependentNameType( - const DependentNameType *DNT) const { +std::vector +HeuristicResolverImpl::resolveDependentNameType(const DependentNameType *DNT) { + if (auto [_, inserted] = SeenDependentNameTypes.insert(DNT); !inserted) + return {}; return resolveDependentMember( resolveNestedNameSpecifierToType(DNT->getQualifier()), DNT->getIdentifier(), TypeFilter); } std::vector -HeuristicResolver::resolveTemplateSpecializationType( - const DependentTemplateSpecializationType *DTST) const { +HeuristicResolverImpl::resolveTemplateSpecializationType( + const DependentTemplateSpecializationType *DTST) { return resolveDependentMember( resolveNestedNameSpecifierToType(DTST->getQualifier()), DTST->getIdentifier(), TemplateFilter); } std::vector -HeuristicResolver::resolveExprToDecls(const Expr *E) const { +HeuristicResolverImpl::resolveExprToDecls(const Expr *E) { if (const auto *ME = dyn_cast(E)) { return resolveMemberExpr(ME); } @@ -236,7 +308,7 @@ HeuristicResolver::resolveExprToDecls(const Expr *E) const { return {}; } -const Type *HeuristicResolver::resolveExprToType(const Expr *E) const { +const Type *HeuristicResolverImpl::resolveExprToType(const Expr *E) { std::vector Decls = resolveExprToDecls(E); if (!Decls.empty()) return resolveDeclsToType(Decls, Ctx); @@ -244,8 +316,8 @@ const Type *HeuristicResolver::resolveExprToType(const Expr *E) const { return E->getType().getTypePtr(); } -const Type *HeuristicResolver::resolveNestedNameSpecifierToType( - const NestedNameSpecifier *NNS) const { +const Type *HeuristicResolverImpl::resolveNestedNameSpecifierToType( + const NestedNameSpecifier *NNS) { if (!NNS) return nullptr; @@ -270,8 +342,6 @@ const Type *HeuristicResolver::resolveNestedNameSpecifierToType( return nullptr; } -namespace { - bool isOrdinaryMember(const NamedDecl *ND) { return ND->isInIdentifierNamespace(Decl::IDNS_Ordinary | Decl::IDNS_Tag | Decl::IDNS_Member); @@ -287,11 +357,9 @@ bool findOrdinaryMember(const CXXRecordDecl *RD, CXXBasePath &Path, return false; } -} // namespace - -bool HeuristicResolver::findOrdinaryMemberInDependentClasses( +bool HeuristicResolverImpl::findOrdinaryMemberInDependentClasses( const CXXBaseSpecifier *Specifier, CXXBasePath &Path, - DeclarationName Name) const { + DeclarationName Name) { CXXRecordDecl *RD = resolveTypeToRecordDecl(Specifier->getType().getTypePtr()); if (!RD) @@ -299,9 +367,9 @@ bool HeuristicResolver::findOrdinaryMemberInDependentClasses( return findOrdinaryMember(RD, Path, Name); } -std::vector HeuristicResolver::lookupDependentName( +std::vector HeuristicResolverImpl::lookupDependentName( CXXRecordDecl *RD, DeclarationName Name, - llvm::function_ref Filter) const { + llvm::function_ref Filter) { std::vector Results; // Lookup in the class. @@ -332,9 +400,9 @@ std::vector HeuristicResolver::lookupDependentName( return Results; } -std::vector HeuristicResolver::resolveDependentMember( +std::vector HeuristicResolverImpl::resolveDependentMember( const Type *T, DeclarationName Name, - llvm::function_ref Filter) const { + llvm::function_ref Filter) { if (!T) return {}; if (auto *ET = T->getAs()) { @@ -349,6 +417,44 @@ std::vector HeuristicResolver::resolveDependentMember( } return {}; } +} // namespace + +std::vector HeuristicResolver::resolveMemberExpr( + const CXXDependentScopeMemberExpr *ME) const { + return HeuristicResolverImpl(Ctx).resolveMemberExpr(ME); +} +std::vector HeuristicResolver::resolveDeclRefExpr( + const DependentScopeDeclRefExpr *RE) const { + return HeuristicResolverImpl(Ctx).resolveDeclRefExpr(RE); +} +std::vector +HeuristicResolver::resolveTypeOfCallExpr(const CallExpr *CE) const { + return HeuristicResolverImpl(Ctx).resolveTypeOfCallExpr(CE); +} +std::vector +HeuristicResolver::resolveCalleeOfCallExpr(const CallExpr *CE) const { + return HeuristicResolverImpl(Ctx).resolveCalleeOfCallExpr(CE); +} +std::vector HeuristicResolver::resolveUsingValueDecl( + const UnresolvedUsingValueDecl *UUVD) const { + return HeuristicResolverImpl(Ctx).resolveUsingValueDecl(UUVD); +} +std::vector HeuristicResolver::resolveDependentNameType( + const DependentNameType *DNT) const { + return HeuristicResolverImpl(Ctx).resolveDependentNameType(DNT); +} +std::vector +HeuristicResolver::resolveTemplateSpecializationType( + const DependentTemplateSpecializationType *DTST) const { + return HeuristicResolverImpl(Ctx).resolveTemplateSpecializationType(DTST); +} +const Type *HeuristicResolver::resolveNestedNameSpecifierToType( + const NestedNameSpecifier *NNS) const { + return HeuristicResolverImpl(Ctx).resolveNestedNameSpecifierToType(NNS); +} +const Type *HeuristicResolver::getPointeeType(const Type *T) const { + return HeuristicResolverImpl(Ctx).getPointeeType(T); +} } // namespace clangd } // namespace clang diff --git a/clang-tools-extra/clangd/HeuristicResolver.h b/clang-tools-extra/clangd/HeuristicResolver.h index dc04123d37593..dcc063bbc4adc 100644 --- a/clang-tools-extra/clangd/HeuristicResolver.h +++ b/clang-tools-extra/clangd/HeuristicResolver.h @@ -77,43 +77,6 @@ class HeuristicResolver { private: ASTContext &Ctx; - - // Given a tag-decl type and a member name, heuristically resolve the - // name to one or more declarations. - // The current heuristic is simply to look up the name in the primary - // template. This is a heuristic because the template could potentially - // have specializations that declare different members. - // Multiple declarations could be returned if the name is overloaded - // (e.g. an overloaded method in the primary template). - // This heuristic will give the desired answer in many cases, e.g. - // for a call to vector::size(). - std::vector resolveDependentMember( - const Type *T, DeclarationName Name, - llvm::function_ref Filter) const; - - // Try to heuristically resolve the type of a possibly-dependent expression - // `E`. - const Type *resolveExprToType(const Expr *E) const; - std::vector resolveExprToDecls(const Expr *E) const; - - // Helper function for HeuristicResolver::resolveDependentMember() - // which takes a possibly-dependent type `T` and heuristically - // resolves it to a CXXRecordDecl in which we can try name lookup. - CXXRecordDecl *resolveTypeToRecordDecl(const Type *T) const; - - // This is a reimplementation of CXXRecordDecl::lookupDependentName() - // so that the implementation can call into other HeuristicResolver helpers. - // FIXME: Once HeuristicResolver is upstreamed to the clang libraries - // (https://github.com/clangd/clangd/discussions/1662), - // CXXRecordDecl::lookupDepenedentName() can be removed, and its call sites - // can be modified to benefit from the more comprehensive heuristics offered - // by HeuristicResolver instead. - std::vector lookupDependentName( - CXXRecordDecl *RD, DeclarationName Name, - llvm::function_ref Filter) const; - bool findOrdinaryMemberInDependentClasses(const CXXBaseSpecifier *Specifier, - CXXBasePath &Path, - DeclarationName Name) const; }; } // namespace clangd diff --git a/clang-tools-extra/clangd/unittests/FindTargetTests.cpp b/clang-tools-extra/clangd/unittests/FindTargetTests.cpp index 29cff68cf03b2..0af6036734ba5 100644 --- a/clang-tools-extra/clangd/unittests/FindTargetTests.cpp +++ b/clang-tools-extra/clangd/unittests/FindTargetTests.cpp @@ -1009,6 +1009,33 @@ TEST_F(TargetDeclTest, DependentTypes) { )cpp"; EXPECT_DECLS("DependentTemplateSpecializationTypeLoc", "template struct B"); + + // Dependent name with recursive definition. We don't expect a + // result, but we shouldn't get into a stack overflow either. + Code = R"cpp( + template + struct waldo { + typedef typename waldo::type::[[next]] type; + }; + )cpp"; + EXPECT_DECLS("DependentNameTypeLoc"); + + // Similar to above but using mutually recursive templates. + Code = R"cpp( + template + struct odd; + + template + struct even { + using type = typename odd::type::next; + }; + + template + struct odd { + using type = typename even::type::[[next]]; + }; + )cpp"; + EXPECT_DECLS("DependentNameTypeLoc"); } TEST_F(TargetDeclTest, TypedefCascade) { From 3a06861272d8a0710d1880e8e3d4954a76b7f256 Mon Sep 17 00:00:00 2001 From: Louis Dionne Date: Sat, 16 Mar 2024 18:32:44 -0400 Subject: [PATCH 03/25] [libc++] Use clang-tidy version that matches the compiler we use in the CI (#85305) This works around ODR violations in the clang-tidy plugin we use to perform the modules tests. Fixes #85242 --- libcxx/test/tools/clang_tidy_checks/CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/libcxx/test/tools/clang_tidy_checks/CMakeLists.txt b/libcxx/test/tools/clang_tidy_checks/CMakeLists.txt index 978e709521652..a52140e2b9938 100644 --- a/libcxx/test/tools/clang_tidy_checks/CMakeLists.txt +++ b/libcxx/test/tools/clang_tidy_checks/CMakeLists.txt @@ -5,7 +5,7 @@ set(LLVM_DIR_SAVE ${LLVM_DIR}) set(Clang_DIR_SAVE ${Clang_DIR}) -find_package(Clang 18) +find_package(Clang 18.1) if (NOT Clang_FOUND) find_package(Clang 17) endif() From 77e1992e89d3734f20c97ad47b4675219f5b9163 Mon Sep 17 00:00:00 2001 From: Jacek Caban Date: Tue, 6 Feb 2024 13:47:58 +0100 Subject: [PATCH 04/25] [llvm-readobj][Object][COFF] Print COFF import library symbol export name. (#78769) getExportName implementation is based on lld-link. In its current form, it's mostly about convenience, but it will be more useful for EXPORTAS support, for which export name is not possible to deduce from other printed properties. --- lld/test/COFF/def-export-cpp.s | 1 + lld/test/COFF/def-export-stdcall.s | 13 ++++++++++ lld/test/COFF/dllexport.s | 4 +++ llvm/include/llvm/Object/COFFImportFile.h | 1 + llvm/lib/Object/COFFImportFile.cpp | 26 +++++++++++++++++++ .../tools/llvm-dlltool/coff-decorated.def | 7 +++++ llvm/test/tools/llvm-dlltool/coff-exports.def | 3 +++ llvm/test/tools/llvm-dlltool/coff-noname.def | 1 + .../llvm-dlltool/no-leading-underscore.def | 2 ++ llvm/test/tools/llvm-lib/arm64ec-implib.test | 2 ++ .../tools/llvm-readobj/COFF/file-headers.test | 1 + llvm/tools/llvm-readobj/COFFImportDumper.cpp | 3 +++ 12 files changed, 64 insertions(+) diff --git a/lld/test/COFF/def-export-cpp.s b/lld/test/COFF/def-export-cpp.s index e00b35b1c5b39..370b8ddba4104 100644 --- a/lld/test/COFF/def-export-cpp.s +++ b/lld/test/COFF/def-export-cpp.s @@ -10,6 +10,7 @@ # IMPLIB: File: foo.dll # IMPLIB: Name type: undecorate +# IMPLIB-NEXT: Export name: GetPathOnDisk # IMPLIB-NEXT: Symbol: __imp_?GetPathOnDisk@@YA_NPEA_W@Z # IMPLIB-NEXT: Symbol: ?GetPathOnDisk@@YA_NPEA_W@Z diff --git a/lld/test/COFF/def-export-stdcall.s b/lld/test/COFF/def-export-stdcall.s index f015e205c74a3..7e4e04c77cbe7 100644 --- a/lld/test/COFF/def-export-stdcall.s +++ b/lld/test/COFF/def-export-stdcall.s @@ -6,15 +6,19 @@ # RUN: llvm-readobj --coff-exports %t.dll | FileCheck -check-prefix UNDECORATED-EXPORTS %s # UNDECORATED-IMPLIB: Name type: noprefix +# UNDECORATED-IMPLIB-NEXT: Export name: _underscored # UNDECORATED-IMPLIB-NEXT: __imp___underscored # UNDECORATED-IMPLIB-NEXT: __underscored # UNDECORATED-IMPLIB: Name type: undecorate +# UNDECORATED-IMPLIB-NEXT: Export name: fastcall # UNDECORATED-IMPLIB-NEXT: __imp_@fastcall@8 # UNDECORATED-IMPLIB-NEXT: fastcall@8 # UNDECORATED-IMPLIB: Name type: undecorate +# UNDECORATED-IMPLIB-NEXT: Export name: stdcall # UNDECORATED-IMPLIB-NEXT: __imp__stdcall@8 # UNDECORATED-IMPLIB-NEXT: _stdcall@8 # UNDECORATED-IMPLIB: Name type: undecorate +# UNDECORATED-IMPLIB-NEXT: Export name: vectorcall # UNDECORATED-IMPLIB-NEXT: __imp_vectorcall@@8 # UNDECORATED-IMPLIB-NEXT: vectorcall@@8 @@ -30,12 +34,15 @@ # RUN: llvm-readobj --coff-exports %t.dll | FileCheck -check-prefix DECORATED-EXPORTS %s # DECORATED-IMPLIB: Name type: name +# DECORATED-IMPLIB-NEXT: Export name: @fastcall@8 # DECORATED-IMPLIB-NEXT: __imp_@fastcall@8 # DECORATED-IMPLIB-NEXT: @fastcall@8 # DECORATED-IMPLIB: Name type: name +# DECORATED-IMPLIB-NEXT: Export name: _stdcall@8 # DECORATED-IMPLIB-NEXT: __imp__stdcall@8 # DECORATED-IMPLIB-NEXT: _stdcall@8 # DECORATED-IMPLIB: Name type: name +# DECORATED-IMPLIB-NEXT: Export name: vectorcall@@8 # DECORATED-IMPLIB-NEXT: __imp_vectorcall@@8 # DECORATED-IMPLIB-NEXT: vectorcall@@8 @@ -51,14 +58,17 @@ # RUN: llvm-readobj --coff-exports %t.dll | FileCheck -check-prefix DECORATED-MINGW-EXPORTS %s # DECORATED-MINGW-IMPLIB: Name type: name +# DECORATED-MINGW-IMPLIB-NEXT: Export name: @fastcall@8 # DECORATED-MINGW-IMPLIB-NEXT: __imp_@fastcall@8 # DECORATED-MINGW-IMPLIB-NEXT: fastcall@8 # DECORATED-MINGW-IMPLIB: Name type: noprefix +# DECORATED-MINGW-IMPLIB-NEXT: Export name: stdcall@8 # DECORATED-MINGW-IMPLIB-NEXT: __imp__stdcall@8 # DECORATED-MINGW-IMPLIB-NEXT: _stdcall@8 # GNU tools don't support vectorcall, but this test is just to track that # lld's behaviour remains consistent over time. # DECORATED-MINGW-IMPLIB: Name type: name +# DECORATED-MINGW-IMPLIB-NEXT: Export name: vectorcall@@8 # DECORATED-MINGW-IMPLIB-NEXT: __imp_vectorcall@@8 # DECORATED-MINGW-IMPLIB-NEXT: vectorcall@@8 @@ -75,14 +85,17 @@ # RUN: llvm-readobj --coff-exports %t.dll | FileCheck -check-prefix MINGW-KILL-AT-EXPORTS %s # MINGW-KILL-AT-IMPLIB: Name type: noprefix +# MINGW-KILL-AT-IMPLIB: Export name: fastcall # MINGW-KILL-AT-IMPLIB: __imp__fastcall # MINGW-KILL-AT-IMPLIB-NEXT: _fastcall # MINGW-KILL-AT-IMPLIB: Name type: noprefix +# MINGW-KILL-AT-IMPLIB-NEXT: Export name: stdcall # MINGW-KILL-AT-IMPLIB-NEXT: __imp__stdcall # MINGW-KILL-AT-IMPLIB-NEXT: _stdcall # GNU tools don't support vectorcall, but this test is just to track that # lld's behaviour remains consistent over time. # MINGW-KILL-AT-IMPLIB: Name type: noprefix +# MINGW-KILL-AT-IMPLIB-NEXT: Export name: vectorcall # MINGW-KILL-AT-IMPLIB-NEXT: __imp__vectorcall # MINGW-KILL-AT-IMPLIB-NEXT: _vectorcall diff --git a/lld/test/COFF/dllexport.s b/lld/test/COFF/dllexport.s index a238b70ce1b4f..b04ebc3a33c3e 100644 --- a/lld/test/COFF/dllexport.s +++ b/lld/test/COFF/dllexport.s @@ -6,15 +6,19 @@ # RUN: llvm-readobj --coff-exports %t.dll | FileCheck -check-prefix DECORATED-EXPORTS %s # DECORATED-IMPLIB: Name type: name +# DECORATED-IMPLIB-NEXT: Export name: @fastcall@8 # DECORATED-IMPLIB-NEXT: __imp_@fastcall@8 # DECORATED-IMPLIB-NEXT: @fastcall@8 # DECORATED-IMPLIB: Name type: name +# DECORATED-IMPLIB-NEXT: Export name: _stdcall@8 # DECORATED-IMPLIB-NEXT: __imp__stdcall@8 # DECORATED-IMPLIB-NEXT: _stdcall@8 # DECORATED-IMPLIB: Name type: noprefix +# DECORATED-IMPLIB-NEXT: Export name: _underscored # DECORATED-IMPLIB-NEXT: __imp___underscored # DECORATED-IMPLIB-NEXT: __underscored # DECORATED-IMPLIB: Name type: name +# DECORATED-IMPLIB-NEXT: Export name: vectorcall@@8 # DECORATED-IMPLIB-NEXT: __imp_vectorcall@@8 # DECORATED-IMPLIB-NEXT: vectorcall@@8 diff --git a/llvm/include/llvm/Object/COFFImportFile.h b/llvm/include/llvm/Object/COFFImportFile.h index edc836ff0348c..45a4a795fd190 100644 --- a/llvm/include/llvm/Object/COFFImportFile.h +++ b/llvm/include/llvm/Object/COFFImportFile.h @@ -66,6 +66,7 @@ class COFFImportFile : public SymbolicFile { uint16_t getMachine() const { return getCOFFImportHeader()->Machine; } StringRef getFileFormatName() const; + StringRef getExportName() const; private: bool isData() const { diff --git a/llvm/lib/Object/COFFImportFile.cpp b/llvm/lib/Object/COFFImportFile.cpp index 60556c149bf73..d7d26f4f4180c 100644 --- a/llvm/lib/Object/COFFImportFile.cpp +++ b/llvm/lib/Object/COFFImportFile.cpp @@ -52,6 +52,32 @@ StringRef COFFImportFile::getFileFormatName() const { } } +StringRef COFFImportFile::getExportName() const { + const coff_import_header *hdr = getCOFFImportHeader(); + StringRef name = Data.getBuffer().substr(sizeof(*hdr)).split('\0').first; + + auto ltrim1 = [](StringRef s, StringRef chars) { + return !s.empty() && chars.contains(s[0]) ? s.substr(1) : s; + }; + + switch (hdr->getNameType()) { + case IMPORT_ORDINAL: + name = ""; + break; + case IMPORT_NAME_NOPREFIX: + name = ltrim1(name, "?@_"); + break; + case IMPORT_NAME_UNDECORATE: + name = ltrim1(name, "?@_"); + name = name.substr(0, name.find('@')); + break; + default: + break; + } + + return name; +} + static uint16_t getImgRelRelocation(MachineTypes Machine) { switch (Machine) { default: diff --git a/llvm/test/tools/llvm-dlltool/coff-decorated.def b/llvm/test/tools/llvm-dlltool/coff-decorated.def index 856804686168b..fc81f23d09d6c 100644 --- a/llvm/test/tools/llvm-dlltool/coff-decorated.def +++ b/llvm/test/tools/llvm-dlltool/coff-decorated.def @@ -14,25 +14,32 @@ OtherStdcallExportName@4=CdeclInternalFunction CdeclExportName=StdcallInternalFunction@4 ; CHECK: Name type: noprefix +; CHECK-NEXT: Export name: CdeclFunction ; CHECK-NEXT: Symbol: __imp__CdeclFunction ; CHECK-NEXT: Symbol: _CdeclFunction ; CHECK: Name type: undecorate +; CHECK-NEXT: Export name: StdcallFunction ; CHECK-NEXT: Symbol: __imp__StdcallFunction@4 ; CHECK-NEXT: Symbol: _StdcallFunction@4 ; CHECK: Name type: undecorate +; CHECK-NEXT: Export name: FastcallFunction ; CHECK-NEXT: Symbol: __imp_@FastcallFunction@4 ; CHECK-NEXT: Symbol: @FastcallFunction@4 ; CHECK: Name type: name +; CHECK-NEXT: Export name: ??_7exception@@6B@ ; CHECK-NEXT: Symbol: __imp_??_7exception@@6B@ ; CHECK-NEXT: Symbol: ??_7exception@@6B@ ; CHECK-NM: W _StdcallAlias@4 ; CHECK-NM: U _StdcallFunction@4 ; CHECK: Name type: undecorate +; CHECK-NEXT: Export name: StdcallExportName ; CHECK-NEXT: Symbol: __imp__StdcallExportName@4{{$}} ; CHECK-NEXT: Symbol: _StdcallExportName@4{{$}} ; CHECK: Name type: undecorate +; CHECK-NEXT: Export name: OtherStdcallExportName ; CHECK-NEXT: Symbol: __imp__OtherStdcallExportName@4{{$}} ; CHECK-NEXT: Symbol: _OtherStdcallExportName@4{{$}} ; CHECK: Name type: noprefix +; CHECK-NEXT: Export name: CdeclExportName ; CHECK-NEXT: Symbol: __imp__CdeclExportName ; CHECK-NEXT: Symbol: _CdeclExportName diff --git a/llvm/test/tools/llvm-dlltool/coff-exports.def b/llvm/test/tools/llvm-dlltool/coff-exports.def index 57c5574460215..267424db1b8c1 100644 --- a/llvm/test/tools/llvm-dlltool/coff-exports.def +++ b/llvm/test/tools/llvm-dlltool/coff-exports.def @@ -17,12 +17,15 @@ AnotherFunction ; CHECK-ARM64: Format: COFF-import-file-ARM64 ; CHECK: Type: code ; CHECK: Name type: name +; CHECK-NEXT: Export name: TestFunction1 ; CHECK-NEXT: Symbol: __imp_TestFunction1 ; CHECK-NEXT: Symbol: TestFunction1 ; CHECK: Name type: name +; CHECK-NEXT: Export name: TestFunction2 ; CHECK-NEXT: Symbol: __imp_TestFunction2{{$}} ; CHECK-NEXT: Symbol: TestFunction2{{$}} ; CHECK: Name type: name +; CHECK-NEXT: Export name: TestFunction3 ; CHECK-NEXT: Symbol: __imp_TestFunction3{{$}} ; CHECK-NEXT: Symbol: TestFunction3{{$}} diff --git a/llvm/test/tools/llvm-dlltool/coff-noname.def b/llvm/test/tools/llvm-dlltool/coff-noname.def index 27e60efbd2d80..7cb05846ce28a 100644 --- a/llvm/test/tools/llvm-dlltool/coff-noname.def +++ b/llvm/test/tools/llvm-dlltool/coff-noname.def @@ -12,5 +12,6 @@ ByNameFunction ; CHECK-NEXT: Symbol: __imp__ByOrdinalFunction ; CHECK-NEXT: Symbol: _ByOrdinalFunction ; CHECK: Name type: noprefix +; CHECK-NEXT: Export name: ByNameFunction ; CHECK-NEXT: Symbol: __imp__ByNameFunction ; CHECK-NEXT: Symbol: _ByNameFunction diff --git a/llvm/test/tools/llvm-dlltool/no-leading-underscore.def b/llvm/test/tools/llvm-dlltool/no-leading-underscore.def index 6b78e15d2b5f6..9c5e77ca29a82 100644 --- a/llvm/test/tools/llvm-dlltool/no-leading-underscore.def +++ b/llvm/test/tools/llvm-dlltool/no-leading-underscore.def @@ -9,9 +9,11 @@ alias == func DecoratedFunction@4 ; CHECK: Name type: name +; CHECK-NEXT: Export name: func ; CHECK-NEXT: Symbol: __imp_func ; CHECK-NEXT: Symbol: func ; CHECK: Name type: undecorate +; CHECK-NEXT: Export name: DecoratedFunction ; CHECK-NEXT: Symbol: __imp_DecoratedFunction@4 ; CHECK-NEXT: Symbol: DecoratedFunction@4 diff --git a/llvm/test/tools/llvm-lib/arm64ec-implib.test b/llvm/test/tools/llvm-lib/arm64ec-implib.test index 2672f8d38b7f7..4250c775daa60 100644 --- a/llvm/test/tools/llvm-lib/arm64ec-implib.test +++ b/llvm/test/tools/llvm-lib/arm64ec-implib.test @@ -36,6 +36,7 @@ READOBJ-NEXT: File: test.dll READOBJ-NEXT: Format: COFF-import-file-ARM64EC READOBJ-NEXT: Type: code READOBJ-NEXT: Name type: name +READOBJ-NEXT: Export name: funcexp READOBJ-NEXT: Symbol: __imp_funcexp READOBJ-NEXT: Symbol: funcexp READOBJ-EMPTY: @@ -43,6 +44,7 @@ READOBJ-NEXT: File: test.dll READOBJ-NEXT: Format: COFF-import-file-ARM64EC READOBJ-NEXT: Type: data READOBJ-NEXT: Name type: name +READOBJ-NEXT: Export name: dataexp READOBJ-NEXT: Symbol: __imp_dataexp Creating a new lib containing the existing lib: diff --git a/llvm/test/tools/llvm-readobj/COFF/file-headers.test b/llvm/test/tools/llvm-readobj/COFF/file-headers.test index b83a6cf5b972b..32f39e196b000 100644 --- a/llvm/test/tools/llvm-readobj/COFF/file-headers.test +++ b/llvm/test/tools/llvm-readobj/COFF/file-headers.test @@ -323,6 +323,7 @@ symbols: # IMPORTLIB:Format: COFF-import-file-i386 # IMPORTLIB-NEXT:Type: code # IMPORTLIB-NEXT:Name type: noprefix +# IMPORTLIB-NEXT:Export name: func # IMPORTLIB-NEXT:Symbol: __imp__func # IMPORTLIB-NEXT:Symbol: _func # IMPORTLIB-NOT:{{.}} diff --git a/llvm/tools/llvm-readobj/COFFImportDumper.cpp b/llvm/tools/llvm-readobj/COFFImportDumper.cpp index 8aedc310ae3a9..656ca32f03a77 100644 --- a/llvm/tools/llvm-readobj/COFFImportDumper.cpp +++ b/llvm/tools/llvm-readobj/COFFImportDumper.cpp @@ -47,6 +47,9 @@ void dumpCOFFImportFile(const COFFImportFile *File, ScopedPrinter &Writer) { break; } + if (H->getNameType() != COFF::IMPORT_ORDINAL) + Writer.printString("Export name", File->getExportName()); + for (const object::BasicSymbolRef &Sym : File->symbols()) { raw_ostream &OS = Writer.startLine(); OS << "Symbol: "; From 76e1800f356513c491ac3bcebdf952842e7b5a3f Mon Sep 17 00:00:00 2001 From: Jacek Caban Date: Sat, 10 Feb 2024 01:00:14 +0100 Subject: [PATCH 05/25] [llvm-lib][llvm-dlltool][Object] Add support for EXPORTAS name types. (#78772) EXPORTAS is a new name type in import libraries. It's used by default on ARM64EC, but it's allowed on other platforms as well. --- llvm/include/llvm/BinaryFormat/COFF.h | 5 +- llvm/include/llvm/Object/COFFImportFile.h | 4 + llvm/lib/Object/COFFImportFile.cpp | 66 +++++++++----- llvm/lib/Object/COFFModuleDefinition.cpp | 13 ++- llvm/test/tools/llvm-lib/exportas.test | 94 ++++++++++++++++++++ llvm/tools/llvm-readobj/COFFImportDumper.cpp | 3 + 6 files changed, 162 insertions(+), 23 deletions(-) create mode 100644 llvm/test/tools/llvm-lib/exportas.test diff --git a/llvm/include/llvm/BinaryFormat/COFF.h b/llvm/include/llvm/BinaryFormat/COFF.h index 522ee37da6e83..72461d0d9c316 100644 --- a/llvm/include/llvm/BinaryFormat/COFF.h +++ b/llvm/include/llvm/BinaryFormat/COFF.h @@ -716,7 +716,10 @@ enum ImportNameType : unsigned { IMPORT_NAME_NOPREFIX = 2, /// The import name is the public symbol name, but skipping the leading ?, /// @, or optionally _, and truncating at the first @. - IMPORT_NAME_UNDECORATE = 3 + IMPORT_NAME_UNDECORATE = 3, + /// The import name is specified as a separate string in the import library + /// object file. + IMPORT_NAME_EXPORTAS = 4 }; enum class GuardFlags : uint32_t { diff --git a/llvm/include/llvm/Object/COFFImportFile.h b/llvm/include/llvm/Object/COFFImportFile.h index 45a4a795fd190..7c5846e9c044e 100644 --- a/llvm/include/llvm/Object/COFFImportFile.h +++ b/llvm/include/llvm/Object/COFFImportFile.h @@ -92,6 +92,10 @@ struct COFFShortExport { /// file, this is "baz" in "EXPORTS\nfoo = bar == baz". std::string AliasTarget; + /// Specifies EXPORTAS name. In a .def file, this is "bar" in + /// "EXPORTS\nfoo EXPORTAS bar". + std::string ExportAs; + uint16_t Ordinal = 0; bool Noname = false; bool Data = false; diff --git a/llvm/lib/Object/COFFImportFile.cpp b/llvm/lib/Object/COFFImportFile.cpp index d7d26f4f4180c..51e6274dcf235 100644 --- a/llvm/lib/Object/COFFImportFile.cpp +++ b/llvm/lib/Object/COFFImportFile.cpp @@ -71,6 +71,12 @@ StringRef COFFImportFile::getExportName() const { name = ltrim1(name, "?@_"); name = name.substr(0, name.find('@')); break; + case IMPORT_NAME_EXPORTAS: { + // Skip DLL name + name = Data.getBuffer().substr(sizeof(*hdr) + name.size() + 1); + name = name.split('\0').second.split('\0').first; + break; + } default: break; } @@ -209,6 +215,7 @@ class ObjectFactory { // Library Format. NewArchiveMember createShortImport(StringRef Sym, uint16_t Ordinal, ImportType Type, ImportNameType NameType, + StringRef ExportName, MachineTypes Machine); // Create a weak external file which is described in PE/COFF Aux Format 3. @@ -500,12 +507,13 @@ NewArchiveMember ObjectFactory::createNullThunk(std::vector &Buffer) { return {MemoryBufferRef{F, ImportName}}; } -NewArchiveMember ObjectFactory::createShortImport(StringRef Sym, - uint16_t Ordinal, - ImportType ImportType, - ImportNameType NameType, - MachineTypes Machine) { +NewArchiveMember +ObjectFactory::createShortImport(StringRef Sym, uint16_t Ordinal, + ImportType ImportType, ImportNameType NameType, + StringRef ExportName, MachineTypes Machine) { size_t ImpSize = ImportName.size() + Sym.size() + 2; // +2 for NULs + if (!ExportName.empty()) + ImpSize += ExportName.size() + 1; size_t Size = sizeof(coff_import_header) + ImpSize; char *Buf = Alloc.Allocate(Size); memset(Buf, 0, Size); @@ -525,6 +533,10 @@ NewArchiveMember ObjectFactory::createShortImport(StringRef Sym, memcpy(P, Sym.data(), Sym.size()); P += Sym.size() + 1; memcpy(P, ImportName.data(), ImportName.size()); + if (!ExportName.empty()) { + P += ImportName.size() + 1; + memcpy(P, ExportName.data(), ExportName.size()); + } return {MemoryBufferRef(StringRef(Buf, Size), ImportName)}; } @@ -641,27 +653,39 @@ Error writeImportLibrary(StringRef ImportName, StringRef Path, ImportType = IMPORT_CONST; StringRef SymbolName = E.SymbolName.empty() ? E.Name : E.SymbolName; - ImportNameType NameType = E.Noname - ? IMPORT_ORDINAL - : getNameType(SymbolName, E.Name, - Machine, MinGW); - Expected Name = E.ExtName.empty() - ? std::string(SymbolName) - : replace(SymbolName, E.Name, E.ExtName); - - if (!Name) - return Name.takeError(); - - if (!E.AliasTarget.empty() && *Name != E.AliasTarget) { + std::string Name; + + if (E.ExtName.empty()) { + Name = std::string(SymbolName); + } else { + Expected ReplacedName = + replace(SymbolName, E.Name, E.ExtName); + if (!ReplacedName) + return ReplacedName.takeError(); + Name.swap(*ReplacedName); + } + + if (!E.AliasTarget.empty() && Name != E.AliasTarget) { Members.push_back( - OF.createWeakExternal(E.AliasTarget, *Name, false, Machine)); + OF.createWeakExternal(E.AliasTarget, Name, false, Machine)); Members.push_back( - OF.createWeakExternal(E.AliasTarget, *Name, true, Machine)); + OF.createWeakExternal(E.AliasTarget, Name, true, Machine)); continue; } - Members.push_back( - OF.createShortImport(*Name, E.Ordinal, ImportType, NameType, Machine)); + ImportNameType NameType; + std::string ExportName; + if (E.Noname) { + NameType = IMPORT_ORDINAL; + } else if (!E.ExportAs.empty()) { + NameType = IMPORT_NAME_EXPORTAS; + ExportName = E.ExportAs; + } else { + NameType = getNameType(SymbolName, E.Name, Machine, MinGW); + } + + Members.push_back(OF.createShortImport(Name, E.Ordinal, ImportType, + NameType, ExportName, Machine)); } return writeArchive(Path, Members, SymtabWritingMode::NormalSymtab, diff --git a/llvm/lib/Object/COFFModuleDefinition.cpp b/llvm/lib/Object/COFFModuleDefinition.cpp index 648f01f823d00..f60dd49793685 100644 --- a/llvm/lib/Object/COFFModuleDefinition.cpp +++ b/llvm/lib/Object/COFFModuleDefinition.cpp @@ -39,6 +39,7 @@ enum Kind { KwConstant, KwData, KwExports, + KwExportAs, KwHeapsize, KwLibrary, KwName, @@ -118,6 +119,7 @@ class Lexer { .Case("CONSTANT", KwConstant) .Case("DATA", KwData) .Case("EXPORTS", KwExports) + .Case("EXPORTAS", KwExportAs) .Case("HEAPSIZE", KwHeapsize) .Case("LIBRARY", KwLibrary) .Case("NAME", KwName) @@ -286,7 +288,16 @@ class Parser { E.AliasTarget = std::string("_").append(E.AliasTarget); continue; } - unget(); + // EXPORTAS must be at the end of export definition + if (Tok.K == KwExportAs) { + read(); + if (Tok.K == Eof) + return createError( + "unexpected end of file, EXPORTAS identifier expected"); + E.ExportAs = std::string(Tok.Value); + } else { + unget(); + } Info.Exports.push_back(E); return Error::success(); } diff --git a/llvm/test/tools/llvm-lib/exportas.test b/llvm/test/tools/llvm-lib/exportas.test new file mode 100644 index 0000000000000..f6e845ca17466 --- /dev/null +++ b/llvm/test/tools/llvm-lib/exportas.test @@ -0,0 +1,94 @@ +Test EXPORTAS in importlibs. + +RUN: split-file %s %t.dir && cd %t.dir +RUN: llvm-lib -machine:amd64 -def:test.def -out:test.lib + +RUN: llvm-nm --print-armap test.lib | FileCheck --check-prefix=ARMAP %s + +ARMAP: Archive map +ARMAP-NEXT: __IMPORT_DESCRIPTOR_test in test.dll +ARMAP-NEXT: __NULL_IMPORT_DESCRIPTOR in test.dll +ARMAP-NEXT: __imp_func in test.dll +ARMAP-NEXT: __imp_func2 in test.dll +ARMAP-NEXT: __imp_func3 in test.dll +ARMAP-NEXT: __imp_mydata in test.dll +ARMAP-NEXT: func in test.dll +ARMAP-NEXT: func2 in test.dll +ARMAP-NEXT: func3 in test.dll +ARMAP-NEXT: test_NULL_THUNK_DATA in test.dll + +RUN: llvm-readobj test.lib | FileCheck --check-prefix=READOBJ %s + +READOBJ: File: test.lib(test.dll) +READOBJ-NEXT: Format: COFF-x86-64 +READOBJ-NEXT: Arch: x86_64 +READOBJ-NEXT: AddressSize: 64bit +READOBJ-EMPTY: +READOBJ-NEXT: File: test.lib(test.dll) +READOBJ-NEXT: Format: COFF-x86-64 +READOBJ-NEXT: Arch: x86_64 +READOBJ-NEXT: AddressSize: 64bit +READOBJ-EMPTY: +READOBJ-NEXT: File: test.lib(test.dll) +READOBJ-NEXT: Format: COFF-x86-64 +READOBJ-NEXT: Arch: x86_64 +READOBJ-NEXT: AddressSize: 64bit +READOBJ-EMPTY: +READOBJ-NEXT: File: test.dll +READOBJ-NEXT: Format: COFF-import-file-x86-64 +READOBJ-NEXT: Type: code +READOBJ-NEXT: Name type: export as +READOBJ-NEXT: Export name: expfunc +READOBJ-NEXT: Symbol: __imp_func +READOBJ-NEXT: Symbol: func +READOBJ-EMPTY: +READOBJ-NEXT: File: test.dll +READOBJ-NEXT: Format: COFF-import-file-x86-64 +READOBJ-NEXT: Type: data +READOBJ-NEXT: Name type: export as +READOBJ-NEXT: Export name: expdata +READOBJ-NEXT: Symbol: __imp_mydata +READOBJ-EMPTY: +READOBJ-NEXT: File: test.dll +READOBJ-NEXT: Format: COFF-import-file-x86-64 +READOBJ-NEXT: Type: code +READOBJ-NEXT: Name type: export as +READOBJ-NEXT: Export name: expfunc2 +READOBJ-NEXT: Symbol: __imp_func2 +READOBJ-NEXT: Symbol: func2 +READOBJ-EMPTY: +READOBJ-NEXT: File: test.dll +READOBJ-NEXT: Format: COFF-import-file-x86-64 +READOBJ-NEXT: Type: code +READOBJ-NEXT: Name type: export as +READOBJ-NEXT: Export name: expfunc3 +READOBJ-NEXT: Symbol: __imp_func3 +READOBJ-NEXT: Symbol: func3 + + +EXPORTAS must be at the end of entry declaration. +RUN: not llvm-lib -machine:amd64 -def:test2.def -out:test2.lib 2>&1 \ +RUN: | FileCheck --check-prefix=ERROR %s +RUN: not llvm-lib -machine:amd64 -def:test3.def -out:test3.lib 2>&1 \ +RUN: | FileCheck --check-prefix=ERROR %s +ERROR: Invalid data was encountered while parsing the file + + +#--- test.def +LIBRARY test.dll +EXPORTS + func EXPORTAS expfunc + mydata DATA EXPORTAS expdata + func2 = myfunc2 EXPORTAS expfunc2 + func3 = otherdll.otherfunc3 EXPORTAS expfunc3 + +#--- test2.def +LIBRARY test.dll +EXPORTS + func EXPORTAS expfunc + mydata EXPORTAS expdata DATA + +#--- test3.def +LIBRARY test.dll +EXPORTS + mydata EXPORTAS diff --git a/llvm/tools/llvm-readobj/COFFImportDumper.cpp b/llvm/tools/llvm-readobj/COFFImportDumper.cpp index 656ca32f03a77..0ab2a17655653 100644 --- a/llvm/tools/llvm-readobj/COFFImportDumper.cpp +++ b/llvm/tools/llvm-readobj/COFFImportDumper.cpp @@ -45,6 +45,9 @@ void dumpCOFFImportFile(const COFFImportFile *File, ScopedPrinter &Writer) { case COFF::IMPORT_NAME_UNDECORATE: Writer.printString("Name type", "undecorate"); break; + case COFF::IMPORT_NAME_EXPORTAS: + Writer.printString("Name type", "export as"); + break; } if (H->getNameType() != COFF::IMPORT_ORDINAL) From 79bc8b32c6ed4fc6e002d6426c04d4de874b07cc Mon Sep 17 00:00:00 2001 From: Jacek Caban Date: Sat, 10 Feb 2024 12:46:42 +0100 Subject: [PATCH 06/25] [llvm-lib][Object] Add support for EC importlib symbols. (#81059) ARM64EC import libraries expose two additional symbols: mangled thunk symbol (like `#func`) and auxiliary import symbol (like`__imp_aux_func`). The main functional change with this patch is that those symbols are properly added to static library ECSYMBOLS. --- llvm/include/llvm/Object/COFF.h | 41 +++++ llvm/include/llvm/Object/COFFImportFile.h | 28 +++- llvm/lib/Object/COFFImportFile.cpp | 15 ++ .../AArch64/AArch64Arm64ECCallLowering.cpp | 2 + .../lib/Target/AArch64/AArch64MCInstLower.cpp | 2 + .../Target/AArch64/Utils/AArch64BaseInfo.h | 28 ---- llvm/test/tools/llvm-lib/arm64ec-implib.test | 141 +++++++++++++++++- 7 files changed, 225 insertions(+), 32 deletions(-) diff --git a/llvm/include/llvm/Object/COFF.h b/llvm/include/llvm/Object/COFF.h index a548b2c15c5fd..2a5c3d8913b15 100644 --- a/llvm/include/llvm/Object/COFF.h +++ b/llvm/include/llvm/Object/COFF.h @@ -1362,6 +1362,47 @@ class SectionStrippedError SectionStrippedError() { setErrorCode(object_error::section_stripped); } }; +inline std::optional +getArm64ECMangledFunctionName(StringRef Name) { + bool IsCppFn = Name[0] == '?'; + if (IsCppFn && Name.find("$$h") != std::string::npos) + return std::nullopt; + if (!IsCppFn && Name[0] == '#') + return std::nullopt; + + StringRef Prefix = "$$h"; + size_t InsertIdx = 0; + if (IsCppFn) { + InsertIdx = Name.find("@@"); + size_t ThreeAtSignsIdx = Name.find("@@@"); + if (InsertIdx != std::string::npos && InsertIdx != ThreeAtSignsIdx) { + InsertIdx += 2; + } else { + InsertIdx = Name.find("@"); + if (InsertIdx != std::string::npos) + InsertIdx++; + } + } else { + Prefix = "#"; + } + + return std::optional( + (Name.substr(0, InsertIdx) + Prefix + Name.substr(InsertIdx)).str()); +} + +inline std::optional +getArm64ECDemangledFunctionName(StringRef Name) { + if (Name[0] == '#') + return std::string(Name.substr(1)); + if (Name[0] != '?') + return std::nullopt; + + std::pair Pair = Name.split("$$h"); + if (Pair.second.empty()) + return std::nullopt; + return (Pair.first + Pair.second).str(); +} + } // end namespace object } // end namespace llvm diff --git a/llvm/include/llvm/Object/COFFImportFile.h b/llvm/include/llvm/Object/COFFImportFile.h index 7c5846e9c044e..46a982ddb7eee 100644 --- a/llvm/include/llvm/Object/COFFImportFile.h +++ b/llvm/include/llvm/Object/COFFImportFile.h @@ -27,6 +27,9 @@ namespace llvm { namespace object { class COFFImportFile : public SymbolicFile { +private: + enum SymbolIndex { ImpSymbol, ThunkSymbol, ECAuxSymbol, ECThunkSymbol }; + public: COFFImportFile(MemoryBufferRef Source) : SymbolicFile(ID_COFFImportFile, Source) {} @@ -36,9 +39,23 @@ class COFFImportFile : public SymbolicFile { void moveSymbolNext(DataRefImpl &Symb) const override { ++Symb.p; } Error printSymbolName(raw_ostream &OS, DataRefImpl Symb) const override { - if (Symb.p == 0) + switch (Symb.p) { + case ImpSymbol: OS << "__imp_"; - OS << StringRef(Data.getBufferStart() + sizeof(coff_import_header)); + break; + case ECAuxSymbol: + OS << "__imp_aux_"; + break; + } + const char *Name = Data.getBufferStart() + sizeof(coff_import_header); + if (Symb.p != ECThunkSymbol && COFF::isArm64EC(getMachine())) { + if (std::optional DemangledName = + getArm64ECDemangledFunctionName(Name)) { + OS << StringRef(*DemangledName); + return Error::success(); + } + } + OS << StringRef(Name); return Error::success(); } @@ -52,7 +69,12 @@ class COFFImportFile : public SymbolicFile { basic_symbol_iterator symbol_end() const override { DataRefImpl Symb; - Symb.p = isData() ? 1 : 2; + if (isData()) + Symb.p = ImpSymbol + 1; + else if (COFF::isArm64EC(getMachine())) + Symb.p = ECThunkSymbol + 1; + else + Symb.p = ThunkSymbol + 1; return BasicSymbolRef(Symb, this); } diff --git a/llvm/lib/Object/COFFImportFile.cpp b/llvm/lib/Object/COFFImportFile.cpp index 51e6274dcf235..a3e5e78952c16 100644 --- a/llvm/lib/Object/COFFImportFile.cpp +++ b/llvm/lib/Object/COFFImportFile.cpp @@ -684,6 +684,21 @@ Error writeImportLibrary(StringRef ImportName, StringRef Path, NameType = getNameType(SymbolName, E.Name, Machine, MinGW); } + // On ARM64EC, use EXPORTAS to import demangled name for mangled symbols. + if (ImportType == IMPORT_CODE && isArm64EC(Machine)) { + if (std::optional MangledName = + getArm64ECMangledFunctionName(Name)) { + if (ExportName.empty()) { + NameType = IMPORT_NAME_EXPORTAS; + ExportName.swap(Name); + } + Name = std::move(*MangledName); + } else if (ExportName.empty()) { + NameType = IMPORT_NAME_EXPORTAS; + ExportName = std::move(*getArm64ECDemangledFunctionName(Name)); + } + } + Members.push_back(OF.createShortImport(Name, E.Ordinal, ImportType, NameType, ExportName, Machine)); } diff --git a/llvm/lib/Target/AArch64/AArch64Arm64ECCallLowering.cpp b/llvm/lib/Target/AArch64/AArch64Arm64ECCallLowering.cpp index 03d641d04413e..55c5bbc66a3f4 100644 --- a/llvm/lib/Target/AArch64/AArch64Arm64ECCallLowering.cpp +++ b/llvm/lib/Target/AArch64/AArch64Arm64ECCallLowering.cpp @@ -24,11 +24,13 @@ #include "llvm/IR/IRBuilder.h" #include "llvm/IR/Instruction.h" #include "llvm/InitializePasses.h" +#include "llvm/Object/COFF.h" #include "llvm/Pass.h" #include "llvm/Support/CommandLine.h" #include "llvm/TargetParser/Triple.h" using namespace llvm; +using namespace llvm::object; using OperandBundleDef = OperandBundleDefT; diff --git a/llvm/lib/Target/AArch64/AArch64MCInstLower.cpp b/llvm/lib/Target/AArch64/AArch64MCInstLower.cpp index 1e12cf545fa77..37d621cd2f658 100644 --- a/llvm/lib/Target/AArch64/AArch64MCInstLower.cpp +++ b/llvm/lib/Target/AArch64/AArch64MCInstLower.cpp @@ -23,11 +23,13 @@ #include "llvm/MC/MCExpr.h" #include "llvm/MC/MCInst.h" #include "llvm/MC/MCStreamer.h" +#include "llvm/Object/COFF.h" #include "llvm/Support/CodeGen.h" #include "llvm/Support/CommandLine.h" #include "llvm/Target/TargetLoweringObjectFile.h" #include "llvm/Target/TargetMachine.h" using namespace llvm; +using namespace llvm::object; extern cl::opt EnableAArch64ELFLocalDynamicTLSGeneration; diff --git a/llvm/lib/Target/AArch64/Utils/AArch64BaseInfo.h b/llvm/lib/Target/AArch64/Utils/AArch64BaseInfo.h index 10e69655f77e1..8b32d593d2a81 100644 --- a/llvm/lib/Target/AArch64/Utils/AArch64BaseInfo.h +++ b/llvm/lib/Target/AArch64/Utils/AArch64BaseInfo.h @@ -248,34 +248,6 @@ static inline bool atomicBarrierDroppedOnZero(unsigned Opcode) { return false; } -static inline std::optional -getArm64ECMangledFunctionName(std::string Name) { - bool IsCppFn = Name[0] == '?'; - if (IsCppFn && Name.find("$$h") != std::string::npos) - return std::nullopt; - if (!IsCppFn && Name[0] == '#') - return std::nullopt; - - StringRef Prefix = "$$h"; - size_t InsertIdx = 0; - if (IsCppFn) { - InsertIdx = Name.find("@@"); - size_t ThreeAtSignsIdx = Name.find("@@@"); - if (InsertIdx != std::string::npos && InsertIdx != ThreeAtSignsIdx) { - InsertIdx += 2; - } else { - InsertIdx = Name.find("@"); - if (InsertIdx != std::string::npos) - InsertIdx++; - } - } else { - Prefix = "#"; - } - - Name.insert(Name.begin() + InsertIdx, Prefix.begin(), Prefix.end()); - return std::optional(Name); -} - namespace AArch64CC { // The CondCodes constants map directly to the 4-bit encoding of the condition diff --git a/llvm/test/tools/llvm-lib/arm64ec-implib.test b/llvm/test/tools/llvm-lib/arm64ec-implib.test index 4250c775daa60..c583ef75b2b0a 100644 --- a/llvm/test/tools/llvm-lib/arm64ec-implib.test +++ b/llvm/test/tools/llvm-lib/arm64ec-implib.test @@ -11,9 +11,23 @@ ARMAP-NEXT: __NULL_IMPORT_DESCRIPTOR in test.dll ARMAP-NEXT: test_NULL_THUNK_DATA in test.dll ARMAP-EMPTY: ARMAP-NEXT: Archive EC map +ARMAP-NEXT: #expname in test.dll +ARMAP-NEXT: #funcexp in test.dll +ARMAP-NEXT: #mangledfunc in test.dll +ARMAP-NEXT: ?test_cpp_func@@$$hYAHPEAX@Z in test.dll +ARMAP-NEXT: ?test_cpp_func@@YAHPEAX@Z in test.dll +ARMAP-NEXT: __imp_?test_cpp_func@@YAHPEAX@Z in test.dll +ARMAP-NEXT: __imp_aux_?test_cpp_func@@YAHPEAX@Z in test.dll +ARMAP-NEXT: __imp_aux_expname in test.dll +ARMAP-NEXT: __imp_aux_funcexp in test.dll +ARMAP-NEXT: __imp_aux_mangledfunc in test.dll ARMAP-NEXT: __imp_dataexp in test.dll +ARMAP-NEXT: __imp_expname in test.dll ARMAP-NEXT: __imp_funcexp in test.dll +ARMAP-NEXT: __imp_mangledfunc in test.dll +ARMAP-NEXT: expname in test.dll ARMAP-NEXT: funcexp in test.dll +ARMAP-NEXT: mangledfunc in test.dll RUN: llvm-readobj test.lib | FileCheck -check-prefix=READOBJ %s @@ -35,10 +49,42 @@ READOBJ-EMPTY: READOBJ-NEXT: File: test.dll READOBJ-NEXT: Format: COFF-import-file-ARM64EC READOBJ-NEXT: Type: code -READOBJ-NEXT: Name type: name +READOBJ-NEXT: Name type: export as READOBJ-NEXT: Export name: funcexp READOBJ-NEXT: Symbol: __imp_funcexp READOBJ-NEXT: Symbol: funcexp +READOBJ-NEXT: Symbol: __imp_aux_funcexp +READOBJ-NEXT: Symbol: #funcexp +READOBJ-EMPTY: +READOBJ-NEXT: File: test.dll +READOBJ-NEXT: Format: COFF-import-file-ARM64EC +READOBJ-NEXT: Type: code +READOBJ-NEXT: Name type: export as +READOBJ-NEXT: Export name: mangledfunc +READOBJ-NEXT: Symbol: __imp_mangledfunc +READOBJ-NEXT: Symbol: mangledfunc +READOBJ-NEXT: Symbol: __imp_aux_mangledfunc +READOBJ-NEXT: Symbol: #mangledfunc +READOBJ-EMPTY: +READOBJ-NEXT: File: test.dll +READOBJ-NEXT: Format: COFF-import-file-ARM64EC +READOBJ-NEXT: Type: code +READOBJ-NEXT: Name type: export as +READOBJ-NEXT: Export name: ?test_cpp_func@@YAHPEAX@Z +READOBJ-NEXT: Symbol: __imp_?test_cpp_func@@YAHPEAX@Z +READOBJ-NEXT: Symbol: ?test_cpp_func@@YAHPEAX@Z +READOBJ-NEXT: Symbol: __imp_aux_?test_cpp_func@@YAHPEAX@Z +READOBJ-NEXT: Symbol: ?test_cpp_func@@$$hYAHPEAX@Z +READOBJ-EMPTY: +READOBJ-NEXT: File: test.dll +READOBJ-NEXT: Format: COFF-import-file-ARM64EC +READOBJ-NEXT: Type: code +READOBJ-NEXT: Name type: export as +READOBJ-NEXT: Export name: expname +READOBJ-NEXT: Symbol: __imp_expname +READOBJ-NEXT: Symbol: expname +READOBJ-NEXT: Symbol: __imp_aux_expname +READOBJ-NEXT: Symbol: #expname READOBJ-EMPTY: READOBJ-NEXT: File: test.dll READOBJ-NEXT: Format: COFF-import-file-ARM64EC @@ -51,8 +97,101 @@ Creating a new lib containing the existing lib: RUN: llvm-lib -machine:arm64ec test.lib -out:test2.lib RUN: llvm-nm --print-armap test2.lib | FileCheck -check-prefix=ARMAP %s + +RUN: llvm-lib -machine:arm64ec -def:exportas.def -out:exportas.lib +RUN: llvm-nm --print-armap exportas.lib | FileCheck -check-prefix=EXPAS-ARMAP %s +RUN: llvm-readobj exportas.lib | FileCheck -check-prefix=EXPAS-READOBJ %s + +EXPAS-ARMAP: Archive EC map +EXPAS-ARMAP-NEXT: #func1 in test.dll +EXPAS-ARMAP-NEXT: #func2 in test.dll +EXPAS-ARMAP-NEXT: #func3 in test.dll +EXPAS-ARMAP-NEXT: #func4 in test.dll +EXPAS-ARMAP-NEXT: __imp_aux_func1 in test.dll +EXPAS-ARMAP-NEXT: __imp_aux_func2 in test.dll +EXPAS-ARMAP-NEXT: __imp_aux_func3 in test.dll +EXPAS-ARMAP-NEXT: __imp_aux_func4 in test.dll +EXPAS-ARMAP-NEXT: __imp_data1 in test.dll +EXPAS-ARMAP-NEXT: __imp_data2 in test.dll +EXPAS-ARMAP-NEXT: __imp_func1 in test.dll +EXPAS-ARMAP-NEXT: __imp_func2 in test.dll +EXPAS-ARMAP-NEXT: __imp_func3 in test.dll +EXPAS-ARMAP-NEXT: __imp_func4 in test.dll +EXPAS-ARMAP-NEXT: func1 in test.dll +EXPAS-ARMAP-NEXT: func2 in test.dll +EXPAS-ARMAP-NEXT: func3 in test.dll +EXPAS-ARMAP-NEXT: func4 in test.dll + +EXPAS-READOBJ: File: test.dll +EXPAS-READOBJ-NEXT: Format: COFF-import-file-ARM64EC +EXPAS-READOBJ-NEXT: Type: code +EXPAS-READOBJ-NEXT: Name type: export as +EXPAS-READOBJ-NEXT: Export name: func1 +EXPAS-READOBJ-NEXT: Symbol: __imp_func1 +EXPAS-READOBJ-NEXT: Symbol: func1 +EXPAS-READOBJ-NEXT: Symbol: __imp_aux_func1 +EXPAS-READOBJ-NEXT: Symbol: #func1 +EXPAS-READOBJ-EMPTY: +EXPAS-READOBJ-NEXT: File: test.dll +EXPAS-READOBJ-NEXT: Format: COFF-import-file-ARM64EC +EXPAS-READOBJ-NEXT: Type: code +EXPAS-READOBJ-NEXT: Name type: export as +EXPAS-READOBJ-NEXT: Export name: func2 +EXPAS-READOBJ-NEXT: Symbol: __imp_func2 +EXPAS-READOBJ-NEXT: Symbol: func2 +EXPAS-READOBJ-NEXT: Symbol: __imp_aux_func2 +EXPAS-READOBJ-NEXT: Symbol: #func2 +EXPAS-READOBJ-EMPTY: +EXPAS-READOBJ-NEXT: File: test.dll +EXPAS-READOBJ-NEXT: Format: COFF-import-file-ARM64EC +EXPAS-READOBJ-NEXT: Type: code +EXPAS-READOBJ-NEXT: Name type: export as +EXPAS-READOBJ-NEXT: Export name: #func3 +EXPAS-READOBJ-NEXT: Symbol: __imp_func3 +EXPAS-READOBJ-NEXT: Symbol: func3 +EXPAS-READOBJ-NEXT: Symbol: __imp_aux_func3 +EXPAS-READOBJ-NEXT: Symbol: #func3 +EXPAS-READOBJ-EMPTY: +EXPAS-READOBJ-NEXT: File: test.dll +EXPAS-READOBJ-NEXT: Format: COFF-import-file-ARM64EC +EXPAS-READOBJ-NEXT: Type: code +EXPAS-READOBJ-NEXT: Name type: export as +EXPAS-READOBJ-NEXT: Export name: #func4 +EXPAS-READOBJ-NEXT: Symbol: __imp_func4 +EXPAS-READOBJ-NEXT: Symbol: func4 +EXPAS-READOBJ-NEXT: Symbol: __imp_aux_func4 +EXPAS-READOBJ-NEXT: Symbol: #func4 +EXPAS-READOBJ-EMPTY: +EXPAS-READOBJ-NEXT: File: test.dll +EXPAS-READOBJ-NEXT: Format: COFF-import-file-ARM64EC +EXPAS-READOBJ-NEXT: Type: data +EXPAS-READOBJ-NEXT: Name type: export as +EXPAS-READOBJ-NEXT: Export name: #data1 +EXPAS-READOBJ-NEXT: Symbol: __imp_data1 +EXPAS-READOBJ-EMPTY: +EXPAS-READOBJ-NEXT: File: test.dll +EXPAS-READOBJ-NEXT: Format: COFF-import-file-ARM64EC +EXPAS-READOBJ-NEXT: Type: data +EXPAS-READOBJ-NEXT: Name type: export as +EXPAS-READOBJ-NEXT: Export name: data2 +EXPAS-READOBJ-NEXT: Symbol: __imp_data2 + + #--- test.def LIBRARY test.dll EXPORTS funcexp + #mangledfunc + ?test_cpp_func@@YAHPEAX@Z + expname=impname dataexp DATA + +#--- exportas.def +LIBRARY test.dll +EXPORTS + #func1 EXPORTAS func1 + func2 EXPORTAS func2 + func3 EXPORTAS #func3 + #func4 EXPORTAS #func4 + data1 DATA EXPORTAS #data1 + #data2 DATA EXPORTAS data2 From 207ecd684cc6731266e12d8d2df0e3d1d9e1ff8d Mon Sep 17 00:00:00 2001 From: Daniel Paoliello Date: Tue, 12 Mar 2024 14:10:49 -0700 Subject: [PATCH 07/25] [Arm64EC] Copy import descriptors to the EC Map (#84834) As noted in , MSVC places import descriptors in both the EC and regular map - that PR moved the descriptors to ONLY the regular map, however this causes linking errors when linking as Arm64EC: ``` bcryptprimitives.lib(bcryptprimitives.dll) : error LNK2001: unresolved external symbol __IMPORT_DESCRIPTOR_bcryptprimitives (EC Symbol) ``` This change copies import descriptors from the regular map to the EC map, which fixes this linking error. --- llvm/include/llvm/Object/COFFImportFile.h | 6 ++++++ llvm/lib/Object/ArchiveWriter.cpp | 11 +++++++++++ llvm/lib/Object/COFFImportFile.cpp | 10 ++++------ llvm/test/tools/llvm-lib/arm64ec-implib.test | 6 ++++++ 4 files changed, 27 insertions(+), 6 deletions(-) diff --git a/llvm/include/llvm/Object/COFFImportFile.h b/llvm/include/llvm/Object/COFFImportFile.h index 46a982ddb7eee..c79ff18d3616e 100644 --- a/llvm/include/llvm/Object/COFFImportFile.h +++ b/llvm/include/llvm/Object/COFFImportFile.h @@ -26,6 +26,12 @@ namespace llvm { namespace object { +constexpr std::string_view ImportDescriptorPrefix = "__IMPORT_DESCRIPTOR_"; +constexpr std::string_view NullImportDescriptorSymbolName = + "__NULL_IMPORT_DESCRIPTOR"; +constexpr std::string_view NullThunkDataPrefix = "\x7f"; +constexpr std::string_view NullThunkDataSuffix = "_NULL_THUNK_DATA"; + class COFFImportFile : public SymbolicFile { private: enum SymbolIndex { ImpSymbol, ThunkSymbol, ECAuxSymbol, ECThunkSymbol }; diff --git a/llvm/lib/Object/ArchiveWriter.cpp b/llvm/lib/Object/ArchiveWriter.cpp index 155926a8c5949..1a7ed2db54396 100644 --- a/llvm/lib/Object/ArchiveWriter.cpp +++ b/llvm/lib/Object/ArchiveWriter.cpp @@ -677,6 +677,13 @@ static bool isECObject(object::SymbolicFile &Obj) { return false; } +bool isImportDescriptor(StringRef Name) { + return Name.starts_with(ImportDescriptorPrefix) || + Name == StringRef{NullImportDescriptorSymbolName} || + (Name.starts_with(NullThunkDataPrefix) && + Name.ends_with(NullThunkDataSuffix)); +} + static Expected> getSymbols(SymbolicFile *Obj, uint16_t Index, raw_ostream &SymNames, @@ -704,6 +711,10 @@ static Expected> getSymbols(SymbolicFile *Obj, if (Map == &SymMap->Map) { Ret.push_back(SymNames.tell()); SymNames << Name << '\0'; + // If EC is enabled, then the import descriptors are NOT put into EC + // objects so we need to copy them to the EC map manually. + if (SymMap->UseECMap && isImportDescriptor(Name)) + SymMap->ECMap[Name] = Index; } } else { Ret.push_back(SymNames.tell()); diff --git a/llvm/lib/Object/COFFImportFile.cpp b/llvm/lib/Object/COFFImportFile.cpp index a3e5e78952c16..99a42d8c4e06a 100644 --- a/llvm/lib/Object/COFFImportFile.cpp +++ b/llvm/lib/Object/COFFImportFile.cpp @@ -108,7 +108,7 @@ template static void append(std::vector &B, const T &Data) { } static void writeStringTable(std::vector &B, - ArrayRef Strings) { + ArrayRef Strings) { // The COFF string table consists of a 4-byte value which is the size of the // table, including the length field itself. This value is followed by the // string content itself, which is an array of null-terminated C-style @@ -171,9 +171,6 @@ static Expected replace(StringRef S, StringRef From, return (Twine(S.substr(0, Pos)) + To + S.substr(Pos + From.size())).str(); } -static const std::string NullImportDescriptorSymbolName = - "__NULL_IMPORT_DESCRIPTOR"; - namespace { // This class constructs various small object files necessary to support linking // symbols imported from a DLL. The contents are pretty strictly defined and @@ -192,8 +189,9 @@ class ObjectFactory { public: ObjectFactory(StringRef S, MachineTypes M) : NativeMachine(M), ImportName(S), Library(llvm::sys::path::stem(S)), - ImportDescriptorSymbolName(("__IMPORT_DESCRIPTOR_" + Library).str()), - NullThunkSymbolName(("\x7f" + Library + "_NULL_THUNK_DATA").str()) {} + ImportDescriptorSymbolName((ImportDescriptorPrefix + Library).str()), + NullThunkSymbolName( + (NullThunkDataPrefix + Library + NullThunkDataSuffix).str()) {} // Creates an Import Descriptor. This is a small object file which contains a // reference to the terminators and contains the library name (entry) for the diff --git a/llvm/test/tools/llvm-lib/arm64ec-implib.test b/llvm/test/tools/llvm-lib/arm64ec-implib.test index c583ef75b2b0a..52ac486da9e29 100644 --- a/llvm/test/tools/llvm-lib/arm64ec-implib.test +++ b/llvm/test/tools/llvm-lib/arm64ec-implib.test @@ -16,6 +16,8 @@ ARMAP-NEXT: #funcexp in test.dll ARMAP-NEXT: #mangledfunc in test.dll ARMAP-NEXT: ?test_cpp_func@@$$hYAHPEAX@Z in test.dll ARMAP-NEXT: ?test_cpp_func@@YAHPEAX@Z in test.dll +ARMAP-NEXT: __IMPORT_DESCRIPTOR_test in test.dll +ARMAP-NEXT: __NULL_IMPORT_DESCRIPTOR in test.dll ARMAP-NEXT: __imp_?test_cpp_func@@YAHPEAX@Z in test.dll ARMAP-NEXT: __imp_aux_?test_cpp_func@@YAHPEAX@Z in test.dll ARMAP-NEXT: __imp_aux_expname in test.dll @@ -28,6 +30,7 @@ ARMAP-NEXT: __imp_mangledfunc in test.dll ARMAP-NEXT: expname in test.dll ARMAP-NEXT: funcexp in test.dll ARMAP-NEXT: mangledfunc in test.dll +ARMAP-NEXT: test_NULL_THUNK_DATA in test.dll RUN: llvm-readobj test.lib | FileCheck -check-prefix=READOBJ %s @@ -107,6 +110,8 @@ EXPAS-ARMAP-NEXT: #func1 in test.dll EXPAS-ARMAP-NEXT: #func2 in test.dll EXPAS-ARMAP-NEXT: #func3 in test.dll EXPAS-ARMAP-NEXT: #func4 in test.dll +EXPAS-ARMAP-NEXT: __IMPORT_DESCRIPTOR_test in test.dll +EXPAS-ARMAP-NEXT: __NULL_IMPORT_DESCRIPTOR in test.dll EXPAS-ARMAP-NEXT: __imp_aux_func1 in test.dll EXPAS-ARMAP-NEXT: __imp_aux_func2 in test.dll EXPAS-ARMAP-NEXT: __imp_aux_func3 in test.dll @@ -121,6 +126,7 @@ EXPAS-ARMAP-NEXT: func1 in test.dll EXPAS-ARMAP-NEXT: func2 in test.dll EXPAS-ARMAP-NEXT: func3 in test.dll EXPAS-ARMAP-NEXT: func4 in test.dll +EXPAS-ARMAP-NEXT: test_NULL_THUNK_DATA in test.dll EXPAS-READOBJ: File: test.dll EXPAS-READOBJ-NEXT: Format: COFF-import-file-ARM64EC From 2f640ad26d176a70c5e7a77c92a899e525e366d9 Mon Sep 17 00:00:00 2001 From: Daniel Paoliello Date: Wed, 13 Mar 2024 16:18:10 -0700 Subject: [PATCH 08/25] Remove support for EXPORTAS in def files to maintain ABI compatibility for COFFShortExport --- llvm/include/llvm/Object/COFFImportFile.h | 4 - llvm/lib/Object/COFFImportFile.cpp | 3 - llvm/lib/Object/COFFModuleDefinition.cpp | 13 +-- llvm/test/tools/llvm-lib/arm64ec-implib.test | 93 ------------------- llvm/test/tools/llvm-lib/exportas.test | 94 -------------------- 5 files changed, 1 insertion(+), 206 deletions(-) delete mode 100644 llvm/test/tools/llvm-lib/exportas.test diff --git a/llvm/include/llvm/Object/COFFImportFile.h b/llvm/include/llvm/Object/COFFImportFile.h index c79ff18d3616e..8358197309f00 100644 --- a/llvm/include/llvm/Object/COFFImportFile.h +++ b/llvm/include/llvm/Object/COFFImportFile.h @@ -120,10 +120,6 @@ struct COFFShortExport { /// file, this is "baz" in "EXPORTS\nfoo = bar == baz". std::string AliasTarget; - /// Specifies EXPORTAS name. In a .def file, this is "bar" in - /// "EXPORTS\nfoo EXPORTAS bar". - std::string ExportAs; - uint16_t Ordinal = 0; bool Noname = false; bool Data = false; diff --git a/llvm/lib/Object/COFFImportFile.cpp b/llvm/lib/Object/COFFImportFile.cpp index 99a42d8c4e06a..d3b5cf2d9f7b5 100644 --- a/llvm/lib/Object/COFFImportFile.cpp +++ b/llvm/lib/Object/COFFImportFile.cpp @@ -675,9 +675,6 @@ Error writeImportLibrary(StringRef ImportName, StringRef Path, std::string ExportName; if (E.Noname) { NameType = IMPORT_ORDINAL; - } else if (!E.ExportAs.empty()) { - NameType = IMPORT_NAME_EXPORTAS; - ExportName = E.ExportAs; } else { NameType = getNameType(SymbolName, E.Name, Machine, MinGW); } diff --git a/llvm/lib/Object/COFFModuleDefinition.cpp b/llvm/lib/Object/COFFModuleDefinition.cpp index f60dd49793685..648f01f823d00 100644 --- a/llvm/lib/Object/COFFModuleDefinition.cpp +++ b/llvm/lib/Object/COFFModuleDefinition.cpp @@ -39,7 +39,6 @@ enum Kind { KwConstant, KwData, KwExports, - KwExportAs, KwHeapsize, KwLibrary, KwName, @@ -119,7 +118,6 @@ class Lexer { .Case("CONSTANT", KwConstant) .Case("DATA", KwData) .Case("EXPORTS", KwExports) - .Case("EXPORTAS", KwExportAs) .Case("HEAPSIZE", KwHeapsize) .Case("LIBRARY", KwLibrary) .Case("NAME", KwName) @@ -288,16 +286,7 @@ class Parser { E.AliasTarget = std::string("_").append(E.AliasTarget); continue; } - // EXPORTAS must be at the end of export definition - if (Tok.K == KwExportAs) { - read(); - if (Tok.K == Eof) - return createError( - "unexpected end of file, EXPORTAS identifier expected"); - E.ExportAs = std::string(Tok.Value); - } else { - unget(); - } + unget(); Info.Exports.push_back(E); return Error::success(); } diff --git a/llvm/test/tools/llvm-lib/arm64ec-implib.test b/llvm/test/tools/llvm-lib/arm64ec-implib.test index 52ac486da9e29..ebc1b166ee4ea 100644 --- a/llvm/test/tools/llvm-lib/arm64ec-implib.test +++ b/llvm/test/tools/llvm-lib/arm64ec-implib.test @@ -100,89 +100,6 @@ Creating a new lib containing the existing lib: RUN: llvm-lib -machine:arm64ec test.lib -out:test2.lib RUN: llvm-nm --print-armap test2.lib | FileCheck -check-prefix=ARMAP %s - -RUN: llvm-lib -machine:arm64ec -def:exportas.def -out:exportas.lib -RUN: llvm-nm --print-armap exportas.lib | FileCheck -check-prefix=EXPAS-ARMAP %s -RUN: llvm-readobj exportas.lib | FileCheck -check-prefix=EXPAS-READOBJ %s - -EXPAS-ARMAP: Archive EC map -EXPAS-ARMAP-NEXT: #func1 in test.dll -EXPAS-ARMAP-NEXT: #func2 in test.dll -EXPAS-ARMAP-NEXT: #func3 in test.dll -EXPAS-ARMAP-NEXT: #func4 in test.dll -EXPAS-ARMAP-NEXT: __IMPORT_DESCRIPTOR_test in test.dll -EXPAS-ARMAP-NEXT: __NULL_IMPORT_DESCRIPTOR in test.dll -EXPAS-ARMAP-NEXT: __imp_aux_func1 in test.dll -EXPAS-ARMAP-NEXT: __imp_aux_func2 in test.dll -EXPAS-ARMAP-NEXT: __imp_aux_func3 in test.dll -EXPAS-ARMAP-NEXT: __imp_aux_func4 in test.dll -EXPAS-ARMAP-NEXT: __imp_data1 in test.dll -EXPAS-ARMAP-NEXT: __imp_data2 in test.dll -EXPAS-ARMAP-NEXT: __imp_func1 in test.dll -EXPAS-ARMAP-NEXT: __imp_func2 in test.dll -EXPAS-ARMAP-NEXT: __imp_func3 in test.dll -EXPAS-ARMAP-NEXT: __imp_func4 in test.dll -EXPAS-ARMAP-NEXT: func1 in test.dll -EXPAS-ARMAP-NEXT: func2 in test.dll -EXPAS-ARMAP-NEXT: func3 in test.dll -EXPAS-ARMAP-NEXT: func4 in test.dll -EXPAS-ARMAP-NEXT: test_NULL_THUNK_DATA in test.dll - -EXPAS-READOBJ: File: test.dll -EXPAS-READOBJ-NEXT: Format: COFF-import-file-ARM64EC -EXPAS-READOBJ-NEXT: Type: code -EXPAS-READOBJ-NEXT: Name type: export as -EXPAS-READOBJ-NEXT: Export name: func1 -EXPAS-READOBJ-NEXT: Symbol: __imp_func1 -EXPAS-READOBJ-NEXT: Symbol: func1 -EXPAS-READOBJ-NEXT: Symbol: __imp_aux_func1 -EXPAS-READOBJ-NEXT: Symbol: #func1 -EXPAS-READOBJ-EMPTY: -EXPAS-READOBJ-NEXT: File: test.dll -EXPAS-READOBJ-NEXT: Format: COFF-import-file-ARM64EC -EXPAS-READOBJ-NEXT: Type: code -EXPAS-READOBJ-NEXT: Name type: export as -EXPAS-READOBJ-NEXT: Export name: func2 -EXPAS-READOBJ-NEXT: Symbol: __imp_func2 -EXPAS-READOBJ-NEXT: Symbol: func2 -EXPAS-READOBJ-NEXT: Symbol: __imp_aux_func2 -EXPAS-READOBJ-NEXT: Symbol: #func2 -EXPAS-READOBJ-EMPTY: -EXPAS-READOBJ-NEXT: File: test.dll -EXPAS-READOBJ-NEXT: Format: COFF-import-file-ARM64EC -EXPAS-READOBJ-NEXT: Type: code -EXPAS-READOBJ-NEXT: Name type: export as -EXPAS-READOBJ-NEXT: Export name: #func3 -EXPAS-READOBJ-NEXT: Symbol: __imp_func3 -EXPAS-READOBJ-NEXT: Symbol: func3 -EXPAS-READOBJ-NEXT: Symbol: __imp_aux_func3 -EXPAS-READOBJ-NEXT: Symbol: #func3 -EXPAS-READOBJ-EMPTY: -EXPAS-READOBJ-NEXT: File: test.dll -EXPAS-READOBJ-NEXT: Format: COFF-import-file-ARM64EC -EXPAS-READOBJ-NEXT: Type: code -EXPAS-READOBJ-NEXT: Name type: export as -EXPAS-READOBJ-NEXT: Export name: #func4 -EXPAS-READOBJ-NEXT: Symbol: __imp_func4 -EXPAS-READOBJ-NEXT: Symbol: func4 -EXPAS-READOBJ-NEXT: Symbol: __imp_aux_func4 -EXPAS-READOBJ-NEXT: Symbol: #func4 -EXPAS-READOBJ-EMPTY: -EXPAS-READOBJ-NEXT: File: test.dll -EXPAS-READOBJ-NEXT: Format: COFF-import-file-ARM64EC -EXPAS-READOBJ-NEXT: Type: data -EXPAS-READOBJ-NEXT: Name type: export as -EXPAS-READOBJ-NEXT: Export name: #data1 -EXPAS-READOBJ-NEXT: Symbol: __imp_data1 -EXPAS-READOBJ-EMPTY: -EXPAS-READOBJ-NEXT: File: test.dll -EXPAS-READOBJ-NEXT: Format: COFF-import-file-ARM64EC -EXPAS-READOBJ-NEXT: Type: data -EXPAS-READOBJ-NEXT: Name type: export as -EXPAS-READOBJ-NEXT: Export name: data2 -EXPAS-READOBJ-NEXT: Symbol: __imp_data2 - - #--- test.def LIBRARY test.dll EXPORTS @@ -191,13 +108,3 @@ EXPORTS ?test_cpp_func@@YAHPEAX@Z expname=impname dataexp DATA - -#--- exportas.def -LIBRARY test.dll -EXPORTS - #func1 EXPORTAS func1 - func2 EXPORTAS func2 - func3 EXPORTAS #func3 - #func4 EXPORTAS #func4 - data1 DATA EXPORTAS #data1 - #data2 DATA EXPORTAS data2 diff --git a/llvm/test/tools/llvm-lib/exportas.test b/llvm/test/tools/llvm-lib/exportas.test deleted file mode 100644 index f6e845ca17466..0000000000000 --- a/llvm/test/tools/llvm-lib/exportas.test +++ /dev/null @@ -1,94 +0,0 @@ -Test EXPORTAS in importlibs. - -RUN: split-file %s %t.dir && cd %t.dir -RUN: llvm-lib -machine:amd64 -def:test.def -out:test.lib - -RUN: llvm-nm --print-armap test.lib | FileCheck --check-prefix=ARMAP %s - -ARMAP: Archive map -ARMAP-NEXT: __IMPORT_DESCRIPTOR_test in test.dll -ARMAP-NEXT: __NULL_IMPORT_DESCRIPTOR in test.dll -ARMAP-NEXT: __imp_func in test.dll -ARMAP-NEXT: __imp_func2 in test.dll -ARMAP-NEXT: __imp_func3 in test.dll -ARMAP-NEXT: __imp_mydata in test.dll -ARMAP-NEXT: func in test.dll -ARMAP-NEXT: func2 in test.dll -ARMAP-NEXT: func3 in test.dll -ARMAP-NEXT: test_NULL_THUNK_DATA in test.dll - -RUN: llvm-readobj test.lib | FileCheck --check-prefix=READOBJ %s - -READOBJ: File: test.lib(test.dll) -READOBJ-NEXT: Format: COFF-x86-64 -READOBJ-NEXT: Arch: x86_64 -READOBJ-NEXT: AddressSize: 64bit -READOBJ-EMPTY: -READOBJ-NEXT: File: test.lib(test.dll) -READOBJ-NEXT: Format: COFF-x86-64 -READOBJ-NEXT: Arch: x86_64 -READOBJ-NEXT: AddressSize: 64bit -READOBJ-EMPTY: -READOBJ-NEXT: File: test.lib(test.dll) -READOBJ-NEXT: Format: COFF-x86-64 -READOBJ-NEXT: Arch: x86_64 -READOBJ-NEXT: AddressSize: 64bit -READOBJ-EMPTY: -READOBJ-NEXT: File: test.dll -READOBJ-NEXT: Format: COFF-import-file-x86-64 -READOBJ-NEXT: Type: code -READOBJ-NEXT: Name type: export as -READOBJ-NEXT: Export name: expfunc -READOBJ-NEXT: Symbol: __imp_func -READOBJ-NEXT: Symbol: func -READOBJ-EMPTY: -READOBJ-NEXT: File: test.dll -READOBJ-NEXT: Format: COFF-import-file-x86-64 -READOBJ-NEXT: Type: data -READOBJ-NEXT: Name type: export as -READOBJ-NEXT: Export name: expdata -READOBJ-NEXT: Symbol: __imp_mydata -READOBJ-EMPTY: -READOBJ-NEXT: File: test.dll -READOBJ-NEXT: Format: COFF-import-file-x86-64 -READOBJ-NEXT: Type: code -READOBJ-NEXT: Name type: export as -READOBJ-NEXT: Export name: expfunc2 -READOBJ-NEXT: Symbol: __imp_func2 -READOBJ-NEXT: Symbol: func2 -READOBJ-EMPTY: -READOBJ-NEXT: File: test.dll -READOBJ-NEXT: Format: COFF-import-file-x86-64 -READOBJ-NEXT: Type: code -READOBJ-NEXT: Name type: export as -READOBJ-NEXT: Export name: expfunc3 -READOBJ-NEXT: Symbol: __imp_func3 -READOBJ-NEXT: Symbol: func3 - - -EXPORTAS must be at the end of entry declaration. -RUN: not llvm-lib -machine:amd64 -def:test2.def -out:test2.lib 2>&1 \ -RUN: | FileCheck --check-prefix=ERROR %s -RUN: not llvm-lib -machine:amd64 -def:test3.def -out:test3.lib 2>&1 \ -RUN: | FileCheck --check-prefix=ERROR %s -ERROR: Invalid data was encountered while parsing the file - - -#--- test.def -LIBRARY test.dll -EXPORTS - func EXPORTAS expfunc - mydata DATA EXPORTAS expdata - func2 = myfunc2 EXPORTAS expfunc2 - func3 = otherdll.otherfunc3 EXPORTAS expfunc3 - -#--- test2.def -LIBRARY test.dll -EXPORTS - func EXPORTAS expfunc - mydata EXPORTAS expdata DATA - -#--- test3.def -LIBRARY test.dll -EXPORTS - mydata EXPORTAS From b95ea2e51bdfe20c9db6a0675ab068fa59a0b9fe Mon Sep 17 00:00:00 2001 From: Weining Lu Date: Tue, 5 Mar 2024 22:01:07 +0800 Subject: [PATCH 09/25] [lld][test] Fix sanitizer buildbot failure Buildbot failure: https://lab.llvm.org/buildbot/#/builders/5/builds/41530/steps/9/logs/stdio (cherry picked from commit d9b435c24ddddcc8148fd97b42f6bb1124e52307) --- lld/test/ELF/loongarch-reloc-leb128.s | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lld/test/ELF/loongarch-reloc-leb128.s b/lld/test/ELF/loongarch-reloc-leb128.s index 9e6f221e62b63..2dd327d1564eb 100644 --- a/lld/test/ELF/loongarch-reloc-leb128.s +++ b/lld/test/ELF/loongarch-reloc-leb128.s @@ -99,4 +99,4 @@ w2: .reloc ., R_LARCH_ADD_ULEB128, w2 .reloc ., R_LARCH_SUB_ULEB128, w1 .fill 10, 1, 0x80 -.byte 0 +.byte 1 From edbe7fa5fef93bb747cb58a589dc25901793774b Mon Sep 17 00:00:00 2001 From: Weining Lu Date: Tue, 5 Mar 2024 23:19:16 +0800 Subject: [PATCH 10/25] [lld][LoongArch] Fix handleUleb128 (cherry picked from commit a41bcb3930534ef1525b4fc30e53e818b39e2b60) --- lld/ELF/Arch/LoongArch.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lld/ELF/Arch/LoongArch.cpp b/lld/ELF/Arch/LoongArch.cpp index 8a6f6db68f290..464f5dfb320cc 100644 --- a/lld/ELF/Arch/LoongArch.cpp +++ b/lld/ELF/Arch/LoongArch.cpp @@ -159,8 +159,9 @@ static bool isJirl(uint32_t insn) { static void handleUleb128(uint8_t *loc, uint64_t val) { const uint32_t maxcount = 1 + 64 / 7; uint32_t count; - uint64_t orig = decodeULEB128(loc, &count); - if (count > maxcount) + const char *error = nullptr; + uint64_t orig = decodeULEB128(loc, &count, nullptr, &error); + if (count > maxcount || (count == maxcount && error)) errorOrWarn(getErrorLocation(loc) + "extra space for uleb128"); uint64_t mask = count < maxcount ? (1ULL << 7 * count) - 1 : -1ULL; encodeULEB128((orig + val) & mask, loc, count); From 7fd9979eb9cf9d93d76e12e85aa1ad847794ebbc Mon Sep 17 00:00:00 2001 From: Yingwei Zheng Date: Sun, 17 Mar 2024 14:15:27 +0800 Subject: [PATCH 11/25] [InstCombine] Drop UB-implying attrs/metadata after speculating an instruction (#85542) When speculating an instruction in `InstCombinerImpl::FoldOpIntoSelect`, the call may result in undefined behavior. This patch drops all UB-implying attrs/metadata to fix this. Fixes #85536. (cherry picked from commit 252d01952c087cf0d141f7f281cf60efeb98be41) --- .../InstCombine/InstructionCombining.cpp | 1 + .../InstCombine/intrinsic-select.ll | 40 +++++++++++++++++++ 2 files changed, 41 insertions(+) diff --git a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp index 5d207dcfd18dd..6f0cf9d9c8f18 100644 --- a/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp +++ b/llvm/lib/Transforms/InstCombine/InstructionCombining.cpp @@ -1455,6 +1455,7 @@ static Value *foldOperationIntoSelectOperand(Instruction &I, SelectInst *SI, Value *NewOp, InstCombiner &IC) { Instruction *Clone = I.clone(); Clone->replaceUsesOfWith(SI, NewOp); + Clone->dropUBImplyingAttrsAndMetadata(); IC.InsertNewInstBefore(Clone, SI->getIterator()); return Clone; } diff --git a/llvm/test/Transforms/InstCombine/intrinsic-select.ll b/llvm/test/Transforms/InstCombine/intrinsic-select.ll index a203b28bcb82a..f37226bbd5b09 100644 --- a/llvm/test/Transforms/InstCombine/intrinsic-select.ll +++ b/llvm/test/Transforms/InstCombine/intrinsic-select.ll @@ -240,3 +240,43 @@ define i32 @vec_to_scalar_select_vector(<2 x i1> %b) { %c = call i32 @llvm.vector.reduce.add.v2i32(<2 x i32> %s) ret i32 %c } + +define i8 @test_drop_noundef(i1 %cond, i8 %val) { +; CHECK-LABEL: @test_drop_noundef( +; CHECK-NEXT: entry: +; CHECK-NEXT: [[TMP0:%.*]] = call i8 @llvm.smin.i8(i8 [[VAL:%.*]], i8 0) +; CHECK-NEXT: [[RET:%.*]] = select i1 [[COND:%.*]], i8 -1, i8 [[TMP0]] +; CHECK-NEXT: ret i8 [[RET]] +; +entry: + %sel = select i1 %cond, i8 -1, i8 %val + %ret = call noundef i8 @llvm.smin.i8(i8 %sel, i8 0) + ret i8 %ret +} + +define i1 @pr85536(i32 %a) { +; CHECK-LABEL: @pr85536( +; CHECK-NEXT: entry: +; CHECK-NEXT: [[CMP1:%.*]] = icmp ult i32 [[A:%.*]], 31 +; CHECK-NEXT: [[SHL1:%.*]] = shl nsw i32 -1, [[A]] +; CHECK-NEXT: [[ZEXT:%.*]] = zext i32 [[SHL1]] to i64 +; CHECK-NEXT: [[SHL2:%.*]] = shl i64 [[ZEXT]], 48 +; CHECK-NEXT: [[SHR:%.*]] = ashr exact i64 [[SHL2]], 48 +; CHECK-NEXT: [[TMP0:%.*]] = call i64 @llvm.smin.i64(i64 [[SHR]], i64 0) +; CHECK-NEXT: [[TMP1:%.*]] = and i64 [[TMP0]], 65535 +; CHECK-NEXT: [[RET1:%.*]] = icmp eq i64 [[TMP1]], 0 +; CHECK-NEXT: [[RET:%.*]] = select i1 [[CMP1]], i1 [[RET1]], i1 false +; CHECK-NEXT: ret i1 [[RET]] +; +entry: + %cmp1 = icmp ugt i32 %a, 30 + %shl1 = shl nsw i32 -1, %a + %zext = zext i32 %shl1 to i64 + %shl2 = shl i64 %zext, 48 + %shr = ashr exact i64 %shl2, 48 + %sel = select i1 %cmp1, i64 -1, i64 %shr + %smin = call noundef i64 @llvm.smin.i64(i64 %sel, i64 0) + %masked = and i64 %smin, 65535 + %ret = icmp eq i64 %masked, 0 + ret i1 %ret +} From ecc22d27e5b81f87f9e3edbf8981c67d6b154843 Mon Sep 17 00:00:00 2001 From: Tom Stellard Date: Tue, 19 Mar 2024 06:55:10 -0700 Subject: [PATCH 12/25] workflows: Fix baseline version for llvm abi checks (#85166) The baseline version calculations was assuming the minor release would always be 0. (cherry picked from commit d93363a0e803a9fb2889ff03237f4e93aacf0108) --- .github/workflows/llvm-tests.yml | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/.github/workflows/llvm-tests.yml b/.github/workflows/llvm-tests.yml index 127628d76f191..64d60bc3da45e 100644 --- a/.github/workflows/llvm-tests.yml +++ b/.github/workflows/llvm-tests.yml @@ -42,6 +42,7 @@ jobs: BASELINE_REF: ${{ steps.vars.outputs.BASELINE_REF }} ABI_HEADERS: ${{ steps.vars.outputs.ABI_HEADERS }} BASELINE_VERSION_MAJOR: ${{ steps.vars.outputs.BASELINE_VERSION_MAJOR }} + BASELINE_VERSION_MINOR: ${{ steps.vars.outputs.BASELINE_VERSION_MINOR }} LLVM_VERSION_MAJOR: ${{ steps.version.outputs.LLVM_VERSION_MAJOR }} LLVM_VERSION_MINOR: ${{ steps.version.outputs.LLVM_VERSION_MINOR }} LLVM_VERSION_PATCH: ${{ steps.version.outputs.LLVM_VERSION_PATCH }} @@ -58,7 +59,14 @@ jobs: - name: Setup Variables id: vars run: | - if [ ${{ steps.version.outputs.LLVM_VERSION_MINOR }} -ne 0 ] || [ ${{ steps.version.outputs.LLVM_VERSION_PATCH }} -eq 0 ]; then + # C++ ABI: + # 18.1.0 we aren't doing ABI checks. + # 18.1.1 We want to check 18.1.0. + # C ABI: + # 18.1.0 We want to check 17.0.x + # 18.1.1 We want to check 18.1.0 + echo "BASELINE_VERSION_MINOR=1" >> "$GITHUB_OUTPUT" + if [ ${{ steps.version.outputs.LLVM_VERSION_PATCH }} -eq 0 ]; then { echo "BASELINE_VERSION_MAJOR=$(( ${{ steps.version.outputs.LLVM_VERSION_MAJOR }} - 1))" echo "ABI_HEADERS=llvm-c" @@ -82,7 +90,7 @@ jobs: include: - name: build-baseline llvm_version_major: ${{ needs.abi-dump-setup.outputs.BASELINE_VERSION_MAJOR }} - ref: llvmorg-${{ needs.abi-dump-setup.outputs.BASELINE_VERSION_MAJOR }}.0.0 + ref: llvmorg-${{ needs.abi-dump-setup.outputs.BASELINE_VERSION_MAJOR }}.${{ needs.abi-dump-setup.outputs.BASELINE_VERSION_MINOR }}.0 repo: llvm/llvm-project - name: build-latest llvm_version_major: ${{ needs.abi-dump-setup.outputs.LLVM_VERSION_MAJOR }} From b1f1effacf32150021e1ae93afcd3933a66f8260 Mon Sep 17 00:00:00 2001 From: Alex Crichton Date: Mon, 18 Mar 2024 19:55:34 -0500 Subject: [PATCH 13/25] [WebAssembly] Change the default linker for `wasm32-wasip2` (#84569) This commit changes the default linker in the WebAssembly toolchain for the `wasm32-wasip2` target. This target is being added to the WebAssembly/wasi-sdk and WebAssembly/wasi-libc projects to target the Component Model by default, in contrast with the preexisting `wasm32-wasi` target (in the process of being renamed to `wasm32-wasip1`) which outputs a core WebAssembly module by default. The `wasm-component-ld` project currently lives in my GitHub account at https://github.com/alexcrichton/wasm-component-ld and isn't necessarily "official" yet, but it's expected to continue to evolve as the `wasm32-wasip2` target continues to shape up and evolve. (cherry picked from commit d66121d74a458e098511b9de920d815440acaa1b) --- clang/lib/Driver/ToolChains/WebAssembly.cpp | 27 +++++++++++++++++++-- clang/lib/Driver/ToolChains/WebAssembly.h | 2 +- clang/test/Driver/wasm-toolchain.c | 24 ++++++++++++++++++ 3 files changed, 50 insertions(+), 3 deletions(-) diff --git a/clang/lib/Driver/ToolChains/WebAssembly.cpp b/clang/lib/Driver/ToolChains/WebAssembly.cpp index 57f4600727ec8..0b16b660364f0 100644 --- a/clang/lib/Driver/ToolChains/WebAssembly.cpp +++ b/clang/lib/Driver/ToolChains/WebAssembly.cpp @@ -44,8 +44,15 @@ std::string wasm::Linker::getLinkerPath(const ArgList &Args) const { llvm::sys::fs::can_execute(UseLinker)) return std::string(UseLinker); - // Accept 'lld', and 'ld' as aliases for the default linker - if (UseLinker != "lld" && UseLinker != "ld") + // Interpret 'lld' as explicitly requesting `wasm-ld`, so look for that + // linker. Note that for `wasm32-wasip2` this overrides the default linker + // of `wasm-component-ld`. + if (UseLinker == "lld") { + return ToolChain.GetProgramPath("wasm-ld"); + } + + // Allow 'ld' as an alias for the default linker + if (UseLinker != "ld") ToolChain.getDriver().Diag(diag::err_drv_invalid_linker_name) << A->getAsString(Args); } @@ -73,6 +80,16 @@ void wasm::Linker::ConstructJob(Compilation &C, const JobAction &JA, if (Args.hasArg(options::OPT_s)) CmdArgs.push_back("--strip-all"); + // On `wasip2` the default linker is `wasm-component-ld` which wraps the + // execution of `wasm-ld`. Find `wasm-ld` and pass it as an argument of where + // to find it to avoid it needing to hunt and rediscover or search `PATH` for + // where it is. + if (llvm::sys::path::stem(Linker).ends_with_insensitive( + "wasm-component-ld")) { + CmdArgs.push_back("--wasm-ld-path"); + CmdArgs.push_back(Args.MakeArgString(ToolChain.GetProgramPath("wasm-ld"))); + } + Args.addAllArgs(CmdArgs, {options::OPT_L, options::OPT_u}); ToolChain.AddFilePathLibArgs(Args, CmdArgs); @@ -221,6 +238,12 @@ WebAssembly::WebAssembly(const Driver &D, const llvm::Triple &Triple, } } +const char *WebAssembly::getDefaultLinker() const { + if (getOS() == "wasip2") + return "wasm-component-ld"; + return "wasm-ld"; +} + bool WebAssembly::IsMathErrnoDefault() const { return false; } bool WebAssembly::IsObjCNonFragileABIDefault() const { return true; } diff --git a/clang/lib/Driver/ToolChains/WebAssembly.h b/clang/lib/Driver/ToolChains/WebAssembly.h index ae60f464c1081..76e0ca39bd748 100644 --- a/clang/lib/Driver/ToolChains/WebAssembly.h +++ b/clang/lib/Driver/ToolChains/WebAssembly.h @@ -67,7 +67,7 @@ class LLVM_LIBRARY_VISIBILITY WebAssembly final : public ToolChain { llvm::opt::ArgStringList &CmdArgs) const override; SanitizerMask getSupportedSanitizers() const override; - const char *getDefaultLinker() const override { return "wasm-ld"; } + const char *getDefaultLinker() const override; CXXStdlibType GetDefaultCXXStdlibType() const override { return ToolChain::CST_Libcxx; diff --git a/clang/test/Driver/wasm-toolchain.c b/clang/test/Driver/wasm-toolchain.c index f950283ec42aa..88590a3ba4c45 100644 --- a/clang/test/Driver/wasm-toolchain.c +++ b/clang/test/Driver/wasm-toolchain.c @@ -197,3 +197,27 @@ // RUN: not %clang -### %s --target=wasm32-unknown-unknown --sysroot=%s/no-sysroot-there -fPIC -mno-mutable-globals %s 2>&1 \ // RUN: | FileCheck -check-prefix=PIC_NO_MUTABLE_GLOBALS %s // PIC_NO_MUTABLE_GLOBALS: error: invalid argument '-fPIC' not allowed with '-mno-mutable-globals' + +// Test that `wasm32-wasip2` invokes the `wasm-component-ld` linker by default +// instead of `wasm-ld`. + +// RUN: %clang -### -O2 --target=wasm32-wasip2 %s --sysroot /foo 2>&1 \ +// RUN: | FileCheck -check-prefix=LINK_WASIP2 %s +// LINK_WASIP2: "-cc1" {{.*}} "-o" "[[temp:[^"]*]]" +// LINK_WASIP2: wasm-component-ld{{.*}}" "-L/foo/lib/wasm32-wasip2" "crt1.o" "[[temp]]" "-lc" "{{.*[/\\]}}libclang_rt.builtins-wasm32.a" "-o" "a.out" + +// Test that on `wasm32-wasip2` the `wasm-component-ld` programs is told where +// to find `wasm-ld` by default. + +// RUN: %clang -### -O2 --target=wasm32-wasip2 %s --sysroot /foo 2>&1 \ +// RUN: | FileCheck -check-prefix=LINK_WASIP2_FIND_WASMLD %s +// LINK_WASIP2_FIND_WASMLD: "-cc1" {{.*}} "-o" "[[temp:[^"]*]]" +// LINK_WASIP2_FIND_WASMLD: wasm-component-ld{{.*}}" {{.*}} "--wasm-ld-path" "{{.*}}wasm-ld{{.*}}" {{.*}} "[[temp]]" {{.*}} + +// If `wasm32-wasip2` is configured with `wasm-ld` as a linker then don't pass +// the `--wasm-ld-path` flag. + +// RUN: %clang -### -O2 --target=wasm32-wasip2 -fuse-ld=lld %s --sysroot /foo 2>&1 \ +// RUN: | FileCheck -check-prefix=LINK_WASIP2_USE_WASMLD %s +// LINK_WASIP2_USE_WASMLD: "-cc1" {{.*}} "-o" "[[temp:[^"]*]]" +// LINK_WASIP2_USE_WASMLD: wasm-ld{{.*}}" "-m" "wasm32" {{.*}} "[[temp]]" {{.*}} From bcf98bd8419997f006811b6f40703e4e791972e6 Mon Sep 17 00:00:00 2001 From: Tom Stellard Date: Fri, 15 Mar 2024 08:16:46 -0700 Subject: [PATCH 14/25] llvm-shlib: Fix libLLVM-${MAJOR}.so symlink on MacOS (#85163) This is a partial revert of 10c48a772742b7afe665a815b7eba2047f17dc4b with a fix for the symlink target name on MacOS See #84637 (cherry picked from commit ec2b7522dbee1cb91111d6ade6e1768462247dcf) --- llvm/cmake/modules/AddLLVM.cmake | 6 +++--- llvm/tools/llvm-shlib/CMakeLists.txt | 3 +-- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/llvm/cmake/modules/AddLLVM.cmake b/llvm/cmake/modules/AddLLVM.cmake index 3bc78b0dc9355..ceec15b611140 100644 --- a/llvm/cmake/modules/AddLLVM.cmake +++ b/llvm/cmake/modules/AddLLVM.cmake @@ -2074,7 +2074,7 @@ function(add_lit_testsuites project directory) endfunction() function(llvm_install_library_symlink name dest type) - cmake_parse_arguments(ARG "" "COMPONENT;SOVERSION" "" ${ARGN}) + cmake_parse_arguments(ARG "FULL_DEST" "COMPONENT" "" ${ARGN}) foreach(path ${CMAKE_MODULE_PATH}) if(EXISTS ${path}/LLVMInstallSymlink.cmake) set(INSTALL_SYMLINK ${path}/LLVMInstallSymlink.cmake) @@ -2088,8 +2088,8 @@ function(llvm_install_library_symlink name dest type) endif() set(full_name ${CMAKE_${type}_LIBRARY_PREFIX}${name}${CMAKE_${type}_LIBRARY_SUFFIX}) - if (ARG_SOVERSION) - set(full_dest ${CMAKE_${type}_LIBRARY_PREFIX}${dest}${CMAKE_${type}_LIBRARY_SUFFIX}.${ARG_SOVERSION}) + if (ARG_FULL_DEST) + set(full_dest ${dest}) else() set(full_dest ${CMAKE_${type}_LIBRARY_PREFIX}${dest}${CMAKE_${type}_LIBRARY_SUFFIX}) endif() diff --git a/llvm/tools/llvm-shlib/CMakeLists.txt b/llvm/tools/llvm-shlib/CMakeLists.txt index eba1672faee7f..9adce0617ff73 100644 --- a/llvm/tools/llvm-shlib/CMakeLists.txt +++ b/llvm/tools/llvm-shlib/CMakeLists.txt @@ -35,8 +35,7 @@ if(LLVM_BUILD_LLVM_DYLIB) endif() add_llvm_library(LLVM SHARED DISABLE_LLVM_LINK_LLVM_DYLIB OUTPUT_NAME LLVM ${INSTALL_WITH_TOOLCHAIN} ${SOURCES}) # Add symlink for backwards compatibility with old library name - get_target_property(LLVM_DYLIB_SOVERSION LLVM SOVERSION) - llvm_install_library_symlink(LLVM-${LLVM_VERSION_MAJOR}${LLVM_VERSION_SUFFIX} LLVM SHARED COMPONENT LLVM SOVERSION ${LLVM_DYLIB_SOVERSION}) + llvm_install_library_symlink(LLVM-${LLVM_VERSION_MAJOR}${LLVM_VERSION_SUFFIX} $ SHARED FULL_DEST COMPONENT LLVM) list(REMOVE_DUPLICATES LIB_NAMES) if("${CMAKE_SYSTEM_NAME}" STREQUAL "Darwin") From cba6ebf0d0abd226c207a0043dee201ffb805466 Mon Sep 17 00:00:00 2001 From: Tom Stellard Date: Sat, 16 Mar 2024 22:47:27 -0700 Subject: [PATCH 15/25] [llvm-shlib] Fix libLLVM-18 symlink on mingw (#85554) The TARGET_SONAME_FILE_NAME generator expression is not available on dll target platforms. (cherry picked from commit f84980570d3f85bdf5c9432647c05bae04a735a0) --- llvm/tools/llvm-shlib/CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/llvm/tools/llvm-shlib/CMakeLists.txt b/llvm/tools/llvm-shlib/CMakeLists.txt index 9adce0617ff73..0ad350bcbe0d7 100644 --- a/llvm/tools/llvm-shlib/CMakeLists.txt +++ b/llvm/tools/llvm-shlib/CMakeLists.txt @@ -35,7 +35,7 @@ if(LLVM_BUILD_LLVM_DYLIB) endif() add_llvm_library(LLVM SHARED DISABLE_LLVM_LINK_LLVM_DYLIB OUTPUT_NAME LLVM ${INSTALL_WITH_TOOLCHAIN} ${SOURCES}) # Add symlink for backwards compatibility with old library name - llvm_install_library_symlink(LLVM-${LLVM_VERSION_MAJOR}${LLVM_VERSION_SUFFIX} $ SHARED FULL_DEST COMPONENT LLVM) + llvm_install_library_symlink(LLVM-${LLVM_VERSION_MAJOR}${LLVM_VERSION_SUFFIX} $ SHARED FULL_DEST COMPONENT LLVM) list(REMOVE_DUPLICATES LIB_NAMES) if("${CMAKE_SYSTEM_NAME}" STREQUAL "Darwin") From c09f6f6f43418b8c67ef8822fbbf5e04296fdf2d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Martin=20Storsj=C3=B6?= Date: Tue, 19 Mar 2024 08:48:57 +0200 Subject: [PATCH 16/25] [llvm-shlib] Fix the version naming style of libLLVM for Windows (#85710) This reverts the changes from 91a384621e5b762d9c173ffd247cfeadd5f436a2 for Windows targets. The changes in that commit don't work as expected for Windows targets (those parts of llvm_add_library don't quite behave the same for Windows), while the previous status quo (producing a library named "libLLVM-.dll") is the defacto standard way of doing versioned library names there, contrary to on Unix. After that commit, the library always ended up named "libLLVM.dll", executables linking against it would reference "libLLVM.dll", and "libLLVM-.dll" was provided as a symlink. Thus revert this bit back to as it were, so that executables actually link against a versioned libLLVM, and no separate symlink is needed. The only thing that might be improved compared to the status quo as it was before these changes, is that the import library is named "lib/libLLVM-.dll.a", while the common style would be to name it plainly "lib/libLLVM.dll.a" (even while it produces references to "libLLVM-.dll", but none of these had that effect for Windows targets. (As a side note, the llvm-shlib library can be built for MinGW, but not currently in MSVC configurations.) (cherry picked from commit cb2ca23345d3d9bde027a18d301949e8bdf606a6) --- llvm/tools/llvm-shlib/CMakeLists.txt | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/llvm/tools/llvm-shlib/CMakeLists.txt b/llvm/tools/llvm-shlib/CMakeLists.txt index 0ad350bcbe0d7..b20ac318e768d 100644 --- a/llvm/tools/llvm-shlib/CMakeLists.txt +++ b/llvm/tools/llvm-shlib/CMakeLists.txt @@ -33,9 +33,13 @@ if(LLVM_BUILD_LLVM_DYLIB) if (LLVM_LINK_LLVM_DYLIB) set(INSTALL_WITH_TOOLCHAIN INSTALL_WITH_TOOLCHAIN) endif() - add_llvm_library(LLVM SHARED DISABLE_LLVM_LINK_LLVM_DYLIB OUTPUT_NAME LLVM ${INSTALL_WITH_TOOLCHAIN} ${SOURCES}) - # Add symlink for backwards compatibility with old library name - llvm_install_library_symlink(LLVM-${LLVM_VERSION_MAJOR}${LLVM_VERSION_SUFFIX} $ SHARED FULL_DEST COMPONENT LLVM) + if (WIN32) + add_llvm_library(LLVM SHARED DISABLE_LLVM_LINK_LLVM_DYLIB SONAME ${INSTALL_WITH_TOOLCHAIN} ${SOURCES}) + else() + add_llvm_library(LLVM SHARED DISABLE_LLVM_LINK_LLVM_DYLIB OUTPUT_NAME LLVM ${INSTALL_WITH_TOOLCHAIN} ${SOURCES}) + # Add symlink for backwards compatibility with old library name + llvm_install_library_symlink(LLVM-${LLVM_VERSION_MAJOR}${LLVM_VERSION_SUFFIX} $ SHARED FULL_DEST COMPONENT LLVM) + endif() list(REMOVE_DUPLICATES LIB_NAMES) if("${CMAKE_SYSTEM_NAME}" STREQUAL "Darwin") From df20f2fc99986e07886efe9ab279ee0846f5e911 Mon Sep 17 00:00:00 2001 From: Qiu Chaofan Date: Mon, 18 Mar 2024 22:31:05 +0800 Subject: [PATCH 17/25] [PowerPC] Update chain uses when emitting lxsizx (#84892) (cherry picked from commit e5b20c83e5ba25e6e0650df30352ce54c2f6ea2f) --- llvm/lib/Target/PowerPC/PPCISelLowering.cpp | 1 + .../CodeGen/PowerPC/scalar-double-ldst.ll | 58 +++++++++++++++++++ .../test/CodeGen/PowerPC/scalar-float-ldst.ll | 58 +++++++++++++++++++ 3 files changed, 117 insertions(+) diff --git a/llvm/lib/Target/PowerPC/PPCISelLowering.cpp b/llvm/lib/Target/PowerPC/PPCISelLowering.cpp index aec58d1c0dcb9..85f1e670045b9 100644 --- a/llvm/lib/Target/PowerPC/PPCISelLowering.cpp +++ b/llvm/lib/Target/PowerPC/PPCISelLowering.cpp @@ -14942,6 +14942,7 @@ SDValue PPCTargetLowering::combineFPToIntToFP(SDNode *N, SDValue Ld = DAG.getMemIntrinsicNode(PPCISD::LXSIZX, dl, DAG.getVTList(MVT::f64, MVT::Other), Ops, MVT::i8, LDN->getMemOperand()); + DAG.makeEquivalentMemoryOrdering(LDN, Ld); // For signed conversion, we need to sign-extend the value in the VSR if (Signed) { diff --git a/llvm/test/CodeGen/PowerPC/scalar-double-ldst.ll b/llvm/test/CodeGen/PowerPC/scalar-double-ldst.ll index 6f68679325c57..798637b6840f1 100644 --- a/llvm/test/CodeGen/PowerPC/scalar-double-ldst.ll +++ b/llvm/test/CodeGen/PowerPC/scalar-double-ldst.ll @@ -7281,3 +7281,61 @@ entry: store double %str, ptr inttoptr (i64 1000000000000 to ptr), align 4096 ret void } + +define dso_local void @st_reversed_double_from_i8(ptr %ptr) { +; CHECK-P10-LABEL: st_reversed_double_from_i8: +; CHECK-P10: # %bb.0: # %entry +; CHECK-P10-NEXT: li r4, 8 +; CHECK-P10-NEXT: lxsibzx f0, 0, r3 +; CHECK-P10-NEXT: xxspltidp vs2, -1023410176 +; CHECK-P10-NEXT: lxsibzx f1, r3, r4 +; CHECK-P10-NEXT: xscvuxddp f0, f0 +; CHECK-P10-NEXT: xscvuxddp f1, f1 +; CHECK-P10-NEXT: xsadddp f0, f0, f2 +; CHECK-P10-NEXT: xsadddp f1, f1, f2 +; CHECK-P10-NEXT: stfd f1, 0(r3) +; CHECK-P10-NEXT: stfd f0, 8(r3) +; CHECK-P10-NEXT: blr +; +; CHECK-P9-LABEL: st_reversed_double_from_i8: +; CHECK-P9: # %bb.0: # %entry +; CHECK-P9-NEXT: li r4, 8 +; CHECK-P9-NEXT: lxsibzx f0, 0, r3 +; CHECK-P9-NEXT: lxsibzx f1, r3, r4 +; CHECK-P9-NEXT: addis r4, r2, .LCPI300_0@toc@ha +; CHECK-P9-NEXT: lfs f2, .LCPI300_0@toc@l(r4) +; CHECK-P9-NEXT: xscvuxddp f0, f0 +; CHECK-P9-NEXT: xscvuxddp f1, f1 +; CHECK-P9-NEXT: xsadddp f0, f0, f2 +; CHECK-P9-NEXT: xsadddp f1, f1, f2 +; CHECK-P9-NEXT: stfd f0, 8(r3) +; CHECK-P9-NEXT: stfd f1, 0(r3) +; CHECK-P9-NEXT: blr +; +; CHECK-P8-LABEL: st_reversed_double_from_i8: +; CHECK-P8: # %bb.0: # %entry +; CHECK-P8-NEXT: lbz r4, 0(r3) +; CHECK-P8-NEXT: lbz r5, 8(r3) +; CHECK-P8-NEXT: mtfprwz f0, r4 +; CHECK-P8-NEXT: mtfprwz f1, r5 +; CHECK-P8-NEXT: addis r4, r2, .LCPI300_0@toc@ha +; CHECK-P8-NEXT: lfs f2, .LCPI300_0@toc@l(r4) +; CHECK-P8-NEXT: xscvuxddp f0, f0 +; CHECK-P8-NEXT: xscvuxddp f1, f1 +; CHECK-P8-NEXT: xsadddp f0, f0, f2 +; CHECK-P8-NEXT: xsadddp f1, f1, f2 +; CHECK-P8-NEXT: stfd f1, 0(r3) +; CHECK-P8-NEXT: stfd f0, 8(r3) +; CHECK-P8-NEXT: blr +entry: + %idx = getelementptr inbounds i8, ptr %ptr, i64 8 + %i0 = load i8, ptr %ptr, align 1 + %i1 = load i8, ptr %idx, align 1 + %f0 = uitofp i8 %i0 to double + %f1 = uitofp i8 %i1 to double + %a0 = fadd double %f0, -1.280000e+02 + %a1 = fadd double %f1, -1.280000e+02 + store double %a1, ptr %ptr, align 8 + store double %a0, ptr %idx, align 8 + ret void +} diff --git a/llvm/test/CodeGen/PowerPC/scalar-float-ldst.ll b/llvm/test/CodeGen/PowerPC/scalar-float-ldst.ll index 824dd4c4db6cb..f396057342129 100644 --- a/llvm/test/CodeGen/PowerPC/scalar-float-ldst.ll +++ b/llvm/test/CodeGen/PowerPC/scalar-float-ldst.ll @@ -7271,3 +7271,61 @@ entry: store double %conv, ptr inttoptr (i64 1000000000000 to ptr), align 4096 ret void } + +define dso_local void @st_reversed_float_from_i8(ptr %ptr) { +; CHECK-P10-LABEL: st_reversed_float_from_i8: +; CHECK-P10: # %bb.0: # %entry +; CHECK-P10-NEXT: li r4, 8 +; CHECK-P10-NEXT: lxsibzx f0, 0, r3 +; CHECK-P10-NEXT: xxspltidp vs2, -1023410176 +; CHECK-P10-NEXT: lxsibzx f1, r3, r4 +; CHECK-P10-NEXT: xscvuxdsp f0, f0 +; CHECK-P10-NEXT: xscvuxdsp f1, f1 +; CHECK-P10-NEXT: xsaddsp f0, f0, f2 +; CHECK-P10-NEXT: xsaddsp f1, f1, f2 +; CHECK-P10-NEXT: stfs f0, 8(r3) +; CHECK-P10-NEXT: stfs f1, 0(r3) +; CHECK-P10-NEXT: blr +; +; CHECK-P9-LABEL: st_reversed_float_from_i8: +; CHECK-P9: # %bb.0: # %entry +; CHECK-P9-NEXT: li r4, 8 +; CHECK-P9-NEXT: lxsibzx f0, 0, r3 +; CHECK-P9-NEXT: lxsibzx f1, r3, r4 +; CHECK-P9-NEXT: addis r4, r2, .LCPI300_0@toc@ha +; CHECK-P9-NEXT: lfs f2, .LCPI300_0@toc@l(r4) +; CHECK-P9-NEXT: xscvuxdsp f0, f0 +; CHECK-P9-NEXT: xscvuxdsp f1, f1 +; CHECK-P9-NEXT: xsaddsp f0, f0, f2 +; CHECK-P9-NEXT: xsaddsp f1, f1, f2 +; CHECK-P9-NEXT: stfs f0, 8(r3) +; CHECK-P9-NEXT: stfs f1, 0(r3) +; CHECK-P9-NEXT: blr +; +; CHECK-P8-LABEL: st_reversed_float_from_i8: +; CHECK-P8: # %bb.0: # %entry +; CHECK-P8-NEXT: lbz r4, 0(r3) +; CHECK-P8-NEXT: lbz r5, 8(r3) +; CHECK-P8-NEXT: mtfprwz f0, r4 +; CHECK-P8-NEXT: mtfprwz f1, r5 +; CHECK-P8-NEXT: addis r4, r2, .LCPI300_0@toc@ha +; CHECK-P8-NEXT: lfs f2, .LCPI300_0@toc@l(r4) +; CHECK-P8-NEXT: xscvuxdsp f0, f0 +; CHECK-P8-NEXT: xscvuxdsp f1, f1 +; CHECK-P8-NEXT: xsaddsp f0, f0, f2 +; CHECK-P8-NEXT: xsaddsp f1, f1, f2 +; CHECK-P8-NEXT: stfs f1, 0(r3) +; CHECK-P8-NEXT: stfs f0, 8(r3) +; CHECK-P8-NEXT: blr +entry: + %idx = getelementptr inbounds i8, ptr %ptr, i64 8 + %i0 = load i8, ptr %ptr, align 1 + %i1 = load i8, ptr %idx, align 1 + %f0 = uitofp i8 %i0 to float + %f1 = uitofp i8 %i1 to float + %a0 = fadd float %f0, -1.280000e+02 + %a1 = fadd float %f1, -1.280000e+02 + store float %a1, ptr %ptr, align 8 + store float %a0, ptr %idx, align 8 + ret void +} From 2d35ba4a85774074bd17186aa44451772d3a9a27 Mon Sep 17 00:00:00 2001 From: David CARLIER Date: Sat, 16 Mar 2024 13:41:33 +0000 Subject: [PATCH 18/25] =?UTF-8?q?Revert=20"release/18.x:=20[openmp]=20=5F?= =?UTF-8?q?=5Fkmp=5Fx86=5Fcpuid=20fix=20for=20i386/PIC=20builds.=20(#846?= =?UTF-8?q?=E2=80=A6"?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reverts commit 9b3edb592debc00a5c3fbf7a71f63e07d6af44be. --- openmp/runtime/src/kmp.h | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/openmp/runtime/src/kmp.h b/openmp/runtime/src/kmp.h index d51ec886cfe55..e3a1e20731bbe 100644 --- a/openmp/runtime/src/kmp.h +++ b/openmp/runtime/src/kmp.h @@ -1403,19 +1403,9 @@ extern void __kmp_query_cpuid(kmp_cpuinfo_t *p); // subleaf is only needed for cache and topology discovery and can be set to // zero in most cases static inline void __kmp_x86_cpuid(int leaf, int subleaf, struct kmp_cpuid *p) { -#if KMP_ARCH_X86 && (defined(__pic__) || defined(__PIC__)) - // on i386 arch, the ebx reg. is used by pic, thus we need to preserve from - // being trashed beforehand - __asm__ __volatile__("mov %%ebx, %%edi\n" - "cpuid\n" - "xchg %%edi, %%ebx\n" - : "=a"(p->eax), "=b"(p->ebx), "=c"(p->ecx), "=d"(p->edx) - : "a"(leaf), "c"(subleaf)); -#else __asm__ __volatile__("cpuid" : "=a"(p->eax), "=b"(p->ebx), "=c"(p->ecx), "=d"(p->edx) : "a"(leaf), "c"(subleaf)); -#endif } // Load p into FPU control word static inline void __kmp_load_x87_fpu_control_word(const kmp_int16 *p) { From fd9f1faf8fcb0308b369f9acec3a56e42381a025 Mon Sep 17 00:00:00 2001 From: Patryk Wychowaniec Date: Fri, 15 Mar 2024 12:07:54 +0100 Subject: [PATCH 19/25] [AVR] Remove earlyclobber from LDDRdPtrQ (#85277) LDDRdPtrQ was marked as `earlyclobber`, which doesn't play well with GreedyRA (which can generate this instruction through `loadRegFromStackSlot()`). This seems to be the same case as: https://github.com/llvm/llvm-project/blob/a99b912c9b74f6ef91786b4dfbc25160c27d3b41/llvm/lib/Target/AVR/AVRInstrInfo.td#L1421 Closes https://github.com/llvm/llvm-project/issues/81911. --- llvm/lib/Target/AVR/AVRInstrInfo.td | 2 +- llvm/test/CodeGen/AVR/bug-81911.ll | 163 ++++++++++++++++++++++++++++ 2 files changed, 164 insertions(+), 1 deletion(-) create mode 100644 llvm/test/CodeGen/AVR/bug-81911.ll diff --git a/llvm/lib/Target/AVR/AVRInstrInfo.td b/llvm/lib/Target/AVR/AVRInstrInfo.td index efaaec32ee6bb..0a77c7c1d418a 100644 --- a/llvm/lib/Target/AVR/AVRInstrInfo.td +++ b/llvm/lib/Target/AVR/AVRInstrInfo.td @@ -1398,7 +1398,7 @@ let mayLoad = 1, hasSideEffects = 0, // Load indirect with displacement operations. let canFoldAsLoad = 1, isReMaterializable = 1 in { - let Constraints = "@earlyclobber $reg" in def LDDRdPtrQ + def LDDRdPtrQ : FSTDLDD<0, (outs GPR8 : $reg), diff --git a/llvm/test/CodeGen/AVR/bug-81911.ll b/llvm/test/CodeGen/AVR/bug-81911.ll new file mode 100644 index 0000000000000..2a22666a1ff92 --- /dev/null +++ b/llvm/test/CodeGen/AVR/bug-81911.ll @@ -0,0 +1,163 @@ +; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py UTC_ARGS: --version 4 +; RUN: llc < %s -mtriple=avr -mcpu=atmega328 -O1 -verify-machineinstrs | FileCheck %s + +define internal i8 @main() { +; CHECK-LABEL: main: +; CHECK: ; %bb.0: ; %bb0 +; CHECK-NEXT: push r2 +; CHECK-NEXT: push r3 +; CHECK-NEXT: push r4 +; CHECK-NEXT: push r5 +; CHECK-NEXT: push r6 +; CHECK-NEXT: push r7 +; CHECK-NEXT: push r8 +; CHECK-NEXT: push r9 +; CHECK-NEXT: push r10 +; CHECK-NEXT: push r11 +; CHECK-NEXT: push r12 +; CHECK-NEXT: push r13 +; CHECK-NEXT: push r14 +; CHECK-NEXT: push r15 +; CHECK-NEXT: push r16 +; CHECK-NEXT: push r17 +; CHECK-NEXT: push r28 +; CHECK-NEXT: push r29 +; CHECK-NEXT: in r28, 61 +; CHECK-NEXT: in r29, 62 +; CHECK-NEXT: sbiw r28, 13 +; CHECK-NEXT: in r0, 63 +; CHECK-NEXT: cli +; CHECK-NEXT: out 62, r29 +; CHECK-NEXT: out 63, r0 +; CHECK-NEXT: out 61, r28 +; CHECK-NEXT: ldi r16, 0 +; CHECK-NEXT: ldi r17, 0 +; CHECK-NEXT: ldi r18, -1 +; CHECK-NEXT: ;APP +; CHECK-NEXT: ldi r24, 123 +; CHECK-NEXT: ;NO_APP +; CHECK-NEXT: std Y+1, r24 ; 1-byte Folded Spill +; CHECK-NEXT: movw r24, r28 +; CHECK-NEXT: adiw r24, 6 +; CHECK-NEXT: std Y+3, r25 ; 2-byte Folded Spill +; CHECK-NEXT: std Y+2, r24 ; 2-byte Folded Spill +; CHECK-NEXT: movw r8, r16 +; CHECK-NEXT: movw r6, r16 +; CHECK-NEXT: movw r4, r16 +; CHECK-NEXT: movw r2, r16 +; CHECK-NEXT: rjmp .LBB0_2 +; CHECK-NEXT: .LBB0_1: ; %bb1 +; CHECK-NEXT: ; in Loop: Header=BB0_2 Depth=1 +; CHECK-NEXT: andi r30, 1 +; CHECK-NEXT: ldd r31, Y+4 ; 1-byte Folded Reload +; CHECK-NEXT: dec r31 +; CHECK-NEXT: cpi r30, 0 +; CHECK-NEXT: movw r8, r18 +; CHECK-NEXT: movw r6, r20 +; CHECK-NEXT: movw r4, r22 +; CHECK-NEXT: movw r2, r24 +; CHECK-NEXT: mov r18, r31 +; CHECK-NEXT: brne .LBB0_2 +; CHECK-NEXT: rjmp .LBB0_4 +; CHECK-NEXT: .LBB0_2: ; %bb1 +; CHECK-NEXT: ; =>This Inner Loop Header: Depth=1 +; CHECK-NEXT: std Y+4, r18 ; 1-byte Folded Spill +; CHECK-NEXT: movw r18, r8 +; CHECK-NEXT: movw r20, r6 +; CHECK-NEXT: movw r22, r4 +; CHECK-NEXT: movw r24, r2 +; CHECK-NEXT: ldi r26, 10 +; CHECK-NEXT: ldi r27, 0 +; CHECK-NEXT: movw r10, r26 +; CHECK-NEXT: movw r12, r16 +; CHECK-NEXT: movw r14, r16 +; CHECK-NEXT: call __udivdi3 +; CHECK-NEXT: std Y+13, r25 +; CHECK-NEXT: std Y+12, r24 +; CHECK-NEXT: std Y+11, r23 +; CHECK-NEXT: std Y+10, r22 +; CHECK-NEXT: std Y+9, r21 +; CHECK-NEXT: std Y+8, r20 +; CHECK-NEXT: std Y+7, r19 +; CHECK-NEXT: std Y+6, r18 +; CHECK-NEXT: ldd r30, Y+2 ; 2-byte Folded Reload +; CHECK-NEXT: ldd r31, Y+3 ; 2-byte Folded Reload +; CHECK-NEXT: ;APP +; CHECK-NEXT: ;NO_APP +; CHECK-NEXT: ldi r30, 1 +; CHECK-NEXT: cp r8, r1 +; CHECK-NEXT: cpc r9, r1 +; CHECK-NEXT: cpc r6, r16 +; CHECK-NEXT: cpc r7, r17 +; CHECK-NEXT: cpc r4, r16 +; CHECK-NEXT: cpc r5, r17 +; CHECK-NEXT: cpc r2, r16 +; CHECK-NEXT: cpc r3, r17 +; CHECK-NEXT: breq .LBB0_3 +; CHECK-NEXT: rjmp .LBB0_1 +; CHECK-NEXT: .LBB0_3: ; %bb1 +; CHECK-NEXT: ; in Loop: Header=BB0_2 Depth=1 +; CHECK-NEXT: mov r30, r1 +; CHECK-NEXT: rjmp .LBB0_1 +; CHECK-NEXT: .LBB0_4: ; %bb3 +; CHECK-NEXT: ldd r24, Y+1 ; 1-byte Folded Reload +; CHECK-NEXT: std Y+5, r24 +; CHECK-NEXT: movw r24, r28 +; CHECK-NEXT: adiw r24, 5 +; CHECK-NEXT: ;APP +; CHECK-NEXT: ;NO_APP +; CHECK-NEXT: ldd r24, Y+5 +; CHECK-NEXT: adiw r28, 13 +; CHECK-NEXT: in r0, 63 +; CHECK-NEXT: cli +; CHECK-NEXT: out 62, r29 +; CHECK-NEXT: out 63, r0 +; CHECK-NEXT: out 61, r28 +; CHECK-NEXT: pop r29 +; CHECK-NEXT: pop r28 +; CHECK-NEXT: pop r17 +; CHECK-NEXT: pop r16 +; CHECK-NEXT: pop r15 +; CHECK-NEXT: pop r14 +; CHECK-NEXT: pop r13 +; CHECK-NEXT: pop r12 +; CHECK-NEXT: pop r11 +; CHECK-NEXT: pop r10 +; CHECK-NEXT: pop r9 +; CHECK-NEXT: pop r8 +; CHECK-NEXT: pop r7 +; CHECK-NEXT: pop r6 +; CHECK-NEXT: pop r5 +; CHECK-NEXT: pop r4 +; CHECK-NEXT: pop r3 +; CHECK-NEXT: pop r2 +; CHECK-NEXT: ret +bb0: + %0 = alloca i64 + %1 = alloca i8 + %2 = tail call i8 asm sideeffect "ldi ${0}, 123", "=&r,~{sreg},~{memory}"() + + br label %bb1 + +bb1: + %3 = phi i64 [ %5, %bb1 ], [ 0, %bb0 ] + %4 = phi i8 [ %6, %bb1 ], [ 0, %bb0 ] + %5 = udiv i64 %3, 10 + %6 = add i8 %4, 1 + + store i64 %5, ptr %0 + call void asm sideeffect "", "r,~{memory}"(ptr %0) + + %7 = icmp eq i64 %3, 0 + %8 = icmp eq i8 %6, 0 + + br i1 %7, label %bb3, label %bb1 + +bb3: + store i8 %2, ptr %1 + call void asm sideeffect "", "r,~{memory}"(ptr %1) + + %9 = load i8, ptr %1 + + ret i8 %9 +} From 53ea0de61dcdeed075b81e4986b61054b7336f18 Mon Sep 17 00:00:00 2001 From: Nikita Popov Date: Fri, 15 Mar 2024 09:02:10 +0100 Subject: [PATCH 20/25] [TSan] Fix atomicrmw xchg with pointer and floats (#85228) atomicrmw xchg also accepts pointer and floating-point values. To handle those, insert necessary casts to and from integer. This is what we do for cmpxchg as well. Fixes https://github.com/llvm/llvm-project/issues/85226. (cherry picked from commit ff2fb2a1d78585944dcdb9061c8487fe1476dfa4) --- .../Instrumentation/ThreadSanitizer.cpp | 9 +++++---- .../Instrumentation/ThreadSanitizer/atomic.ll | 20 +++++++++++++++++++ 2 files changed, 25 insertions(+), 4 deletions(-) diff --git a/llvm/lib/Transforms/Instrumentation/ThreadSanitizer.cpp b/llvm/lib/Transforms/Instrumentation/ThreadSanitizer.cpp index 8ee0bca7e354f..0f42ff7908699 100644 --- a/llvm/lib/Transforms/Instrumentation/ThreadSanitizer.cpp +++ b/llvm/lib/Transforms/Instrumentation/ThreadSanitizer.cpp @@ -752,11 +752,12 @@ bool ThreadSanitizer::instrumentAtomic(Instruction *I, const DataLayout &DL) { const unsigned ByteSize = 1U << Idx; const unsigned BitSize = ByteSize * 8; Type *Ty = Type::getIntNTy(IRB.getContext(), BitSize); - Value *Args[] = {Addr, - IRB.CreateIntCast(RMWI->getValOperand(), Ty, false), + Value *Val = RMWI->getValOperand(); + Value *Args[] = {Addr, IRB.CreateBitOrPointerCast(Val, Ty), createOrdering(&IRB, RMWI->getOrdering())}; - CallInst *C = CallInst::Create(F, Args); - ReplaceInstWithInst(I, C); + Value *C = IRB.CreateCall(F, Args); + I->replaceAllUsesWith(IRB.CreateBitOrPointerCast(C, Val->getType())); + I->eraseFromParent(); } else if (AtomicCmpXchgInst *CASI = dyn_cast(I)) { Value *Addr = CASI->getPointerOperand(); Type *OrigOldValTy = CASI->getNewValOperand()->getType(); diff --git a/llvm/test/Instrumentation/ThreadSanitizer/atomic.ll b/llvm/test/Instrumentation/ThreadSanitizer/atomic.ll index 76afc4bf007c2..8b387cd496297 100644 --- a/llvm/test/Instrumentation/ThreadSanitizer/atomic.ll +++ b/llvm/test/Instrumentation/ThreadSanitizer/atomic.ll @@ -78,6 +78,26 @@ entry: ; CHECK-LABEL: atomic8_xchg_monotonic ; CHECK: call i8 @__tsan_atomic8_exchange(ptr %a, i8 0, i32 0), !dbg +define void @atomic8_xchg_monotonic_ptr(ptr %a, ptr %b) nounwind uwtable { +entry: + atomicrmw xchg ptr %a, ptr %b monotonic, !dbg !7 + ret void, !dbg !7 +} +; CHECK-LABEL: atomic8_xchg_monotonic_ptr +; CHECK: [[ARG:%.*]] = ptrtoint ptr %b to i64, !dbg +; CHECK: [[RES:%.*]] = call i64 @__tsan_atomic64_exchange(ptr %a, i64 [[ARG]], i32 0), !dbg +; CHECK: [[CAST:%.*]] = inttoptr i64 [[RES]] to ptr, !dbg + +define void @atomic8_xchg_monotonic_float(ptr %a, float %b) nounwind uwtable { +entry: + atomicrmw xchg ptr %a, float %b monotonic, !dbg !7 + ret void, !dbg !7 +} +; CHECK-LABEL: atomic8_xchg_monotonic_float +; CHECK: [[ARG:%.*]] = bitcast float %b to i32, !dbg +; CHECK: [[RES:%.*]] = call i32 @__tsan_atomic32_exchange(ptr %a, i32 [[ARG]], i32 0), !dbg +; CHECK: [[CAST:%.*]] = bitcast i32 [[RES]] to float, !dbg + define void @atomic8_add_monotonic(ptr %a) nounwind uwtable { entry: atomicrmw add ptr %a, i8 0 monotonic, !dbg !7 From 42f511c95c6f58a2ed8d6fe35af1cde14c750342 Mon Sep 17 00:00:00 2001 From: Luke Lau Date: Wed, 21 Feb 2024 18:05:04 +0800 Subject: [PATCH 21/25] [RISCV] Add test case for miscompile in gather -> strided load combine. NFC This shows the issue in #82430, but triggers it via the widening SEW combine rather than a GEP that RISCVGatherScatterLowering doesn't detect. (cherry picked from commit 2cd59bdc891ab59a1abfe5205feb45791a530a47) --- .../RISCV/rvv/fixed-vectors-masked-gather.ll | 47 +++++++++++++++++++ 1 file changed, 47 insertions(+) diff --git a/llvm/test/CodeGen/RISCV/rvv/fixed-vectors-masked-gather.ll b/llvm/test/CodeGen/RISCV/rvv/fixed-vectors-masked-gather.ll index 890707c6337fa..1724b48dd6be9 100644 --- a/llvm/test/CodeGen/RISCV/rvv/fixed-vectors-masked-gather.ll +++ b/llvm/test/CodeGen/RISCV/rvv/fixed-vectors-masked-gather.ll @@ -15086,5 +15086,52 @@ define <32 x i64> @mgather_strided_split(ptr %base) { ret <32 x i64> %x } +; FIXME: This is a miscompile triggered by the mgather -> +; riscv.masked.strided.load combine. In order for it to trigger we need either a +; strided gather that RISCVGatherScatterLowering doesn't pick up, or a new +; strided gather generated by the widening sew combine. +define <4 x i32> @masked_gather_widen_sew_negative_stride(ptr %base) { +; RV32V-LABEL: masked_gather_widen_sew_negative_stride: +; RV32V: # %bb.0: +; RV32V-NEXT: addi a0, a0, -128 +; RV32V-NEXT: li a1, -128 +; RV32V-NEXT: vsetivli zero, 2, e64, m1, ta, ma +; RV32V-NEXT: vlse64.v v8, (a0), a1 +; RV32V-NEXT: ret +; +; RV64V-LABEL: masked_gather_widen_sew_negative_stride: +; RV64V: # %bb.0: +; RV64V-NEXT: addi a0, a0, -128 +; RV64V-NEXT: li a1, -128 +; RV64V-NEXT: vsetivli zero, 2, e64, m1, ta, ma +; RV64V-NEXT: vlse64.v v8, (a0), a1 +; RV64V-NEXT: ret +; +; RV32ZVE32F-LABEL: masked_gather_widen_sew_negative_stride: +; RV32ZVE32F: # %bb.0: +; RV32ZVE32F-NEXT: lui a1, 16392 +; RV32ZVE32F-NEXT: addi a1, a1, 1152 +; RV32ZVE32F-NEXT: vsetivli zero, 4, e32, m1, ta, ma +; RV32ZVE32F-NEXT: vmv.s.x v9, a1 +; RV32ZVE32F-NEXT: vluxei8.v v8, (a0), v9 +; RV32ZVE32F-NEXT: ret +; +; RV64ZVE32F-LABEL: masked_gather_widen_sew_negative_stride: +; RV64ZVE32F: # %bb.0: +; RV64ZVE32F-NEXT: addi a1, a0, 128 +; RV64ZVE32F-NEXT: lw a2, 132(a0) +; RV64ZVE32F-NEXT: lw a3, 0(a0) +; RV64ZVE32F-NEXT: lw a0, 4(a0) +; RV64ZVE32F-NEXT: vsetivli zero, 4, e32, m1, ta, ma +; RV64ZVE32F-NEXT: vlse32.v v8, (a1), zero +; RV64ZVE32F-NEXT: vslide1down.vx v8, v8, a2 +; RV64ZVE32F-NEXT: vslide1down.vx v8, v8, a3 +; RV64ZVE32F-NEXT: vslide1down.vx v8, v8, a0 +; RV64ZVE32F-NEXT: ret + %ptrs = getelementptr i32, ptr %base, <4 x i64> + %x = call <4 x i32> @llvm.masked.gather.v4i32.v32p0(<4 x ptr> %ptrs, i32 8, <4 x i1> shufflevector(<4 x i1> insertelement(<4 x i1> poison, i1 true, i32 0), <4 x i1> poison, <4 x i32> zeroinitializer), <4 x i32> poison) + ret <4 x i32> %x +} + ;; NOTE: These prefixes are unused and the list is autogenerated. Do not add tests below this line: ; RV64: {{.*}} From a9d4ed71707d36bc554bfe38408c74c285b11e6b Mon Sep 17 00:00:00 2001 From: Luke Lau Date: Thu, 22 Feb 2024 11:05:06 +0800 Subject: [PATCH 22/25] [RISCV] Adjust test case to show wrong stride. NFC See https://github.com/llvm/llvm-project/pull/82506#discussion_r1498080785 (cherry picked from commit 11d115d0569b212dfeb7fe6485be48070e068e19) --- .../RISCV/rvv/fixed-vectors-masked-gather.ll | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/llvm/test/CodeGen/RISCV/rvv/fixed-vectors-masked-gather.ll b/llvm/test/CodeGen/RISCV/rvv/fixed-vectors-masked-gather.ll index 1724b48dd6be9..60eec356773bf 100644 --- a/llvm/test/CodeGen/RISCV/rvv/fixed-vectors-masked-gather.ll +++ b/llvm/test/CodeGen/RISCV/rvv/fixed-vectors-masked-gather.ll @@ -15093,24 +15093,24 @@ define <32 x i64> @mgather_strided_split(ptr %base) { define <4 x i32> @masked_gather_widen_sew_negative_stride(ptr %base) { ; RV32V-LABEL: masked_gather_widen_sew_negative_stride: ; RV32V: # %bb.0: -; RV32V-NEXT: addi a0, a0, -128 -; RV32V-NEXT: li a1, -128 +; RV32V-NEXT: addi a0, a0, -120 +; RV32V-NEXT: li a1, 120 ; RV32V-NEXT: vsetivli zero, 2, e64, m1, ta, ma ; RV32V-NEXT: vlse64.v v8, (a0), a1 ; RV32V-NEXT: ret ; ; RV64V-LABEL: masked_gather_widen_sew_negative_stride: ; RV64V: # %bb.0: -; RV64V-NEXT: addi a0, a0, -128 -; RV64V-NEXT: li a1, -128 +; RV64V-NEXT: addi a0, a0, -120 +; RV64V-NEXT: li a1, 120 ; RV64V-NEXT: vsetivli zero, 2, e64, m1, ta, ma ; RV64V-NEXT: vlse64.v v8, (a0), a1 ; RV64V-NEXT: ret ; ; RV32ZVE32F-LABEL: masked_gather_widen_sew_negative_stride: ; RV32ZVE32F: # %bb.0: -; RV32ZVE32F-NEXT: lui a1, 16392 -; RV32ZVE32F-NEXT: addi a1, a1, 1152 +; RV32ZVE32F-NEXT: lui a1, 16393 +; RV32ZVE32F-NEXT: addi a1, a1, -888 ; RV32ZVE32F-NEXT: vsetivli zero, 4, e32, m1, ta, ma ; RV32ZVE32F-NEXT: vmv.s.x v9, a1 ; RV32ZVE32F-NEXT: vluxei8.v v8, (a0), v9 @@ -15118,8 +15118,8 @@ define <4 x i32> @masked_gather_widen_sew_negative_stride(ptr %base) { ; ; RV64ZVE32F-LABEL: masked_gather_widen_sew_negative_stride: ; RV64ZVE32F: # %bb.0: -; RV64ZVE32F-NEXT: addi a1, a0, 128 -; RV64ZVE32F-NEXT: lw a2, 132(a0) +; RV64ZVE32F-NEXT: addi a1, a0, 136 +; RV64ZVE32F-NEXT: lw a2, 140(a0) ; RV64ZVE32F-NEXT: lw a3, 0(a0) ; RV64ZVE32F-NEXT: lw a0, 4(a0) ; RV64ZVE32F-NEXT: vsetivli zero, 4, e32, m1, ta, ma @@ -15128,7 +15128,7 @@ define <4 x i32> @masked_gather_widen_sew_negative_stride(ptr %base) { ; RV64ZVE32F-NEXT: vslide1down.vx v8, v8, a3 ; RV64ZVE32F-NEXT: vslide1down.vx v8, v8, a0 ; RV64ZVE32F-NEXT: ret - %ptrs = getelementptr i32, ptr %base, <4 x i64> + %ptrs = getelementptr i32, ptr %base, <4 x i64> %x = call <4 x i32> @llvm.masked.gather.v4i32.v32p0(<4 x ptr> %ptrs, i32 8, <4 x i1> shufflevector(<4 x i1> insertelement(<4 x i1> poison, i1 true, i32 0), <4 x i1> poison, <4 x i32> zeroinitializer), <4 x i32> poison) ret <4 x i32> %x } From a2c93b34dfdf6b2e5d16a5068e92f30bbc5d0ba7 Mon Sep 17 00:00:00 2001 From: Luke Lau Date: Thu, 22 Feb 2024 11:50:27 +0800 Subject: [PATCH 23/25] [RISCV] Fix mgather -> riscv.masked.strided.load combine not extending indices (#82506) This fixes the miscompile reported in #82430 by telling isSimpleVIDSequence to sign extend to XLen instead of the width of the indices, since the "sequence" of indices generated by a strided load will be at XLen. This was the simplest way I could think of getting isSimpleVIDSequence to treat the indexes as if they were zero extended to XLenVT. Another way we could do this is by refactoring out the "get constant integers" part from isSimpleVIDSequence and handle them as APInts so we can separately zero extend it. Fixes #82430 (cherry picked from commit 815644b4dd882ade2e5649d4f97c3dd6f7aea200) --- llvm/lib/Target/RISCV/RISCVISelLowering.cpp | 20 +++++++++++-------- .../RISCV/rvv/fixed-vectors-masked-gather.ll | 12 ++++------- 2 files changed, 16 insertions(+), 16 deletions(-) diff --git a/llvm/lib/Target/RISCV/RISCVISelLowering.cpp b/llvm/lib/Target/RISCV/RISCVISelLowering.cpp index 80447d03c000b..a0cec426002b6 100644 --- a/llvm/lib/Target/RISCV/RISCVISelLowering.cpp +++ b/llvm/lib/Target/RISCV/RISCVISelLowering.cpp @@ -3192,7 +3192,8 @@ static std::optional getExactInteger(const APFloat &APF, // Note that this method will also match potentially unappealing index // sequences, like , however it is left to the caller to // determine whether this is worth generating code for. -static std::optional isSimpleVIDSequence(SDValue Op) { +static std::optional isSimpleVIDSequence(SDValue Op, + unsigned EltSizeInBits) { unsigned NumElts = Op.getNumOperands(); assert(Op.getOpcode() == ISD::BUILD_VECTOR && "Unexpected BUILD_VECTOR"); bool IsInteger = Op.getValueType().isInteger(); @@ -3200,7 +3201,7 @@ static std::optional isSimpleVIDSequence(SDValue Op) { std::optional SeqStepDenom; std::optional SeqStepNum, SeqAddend; std::optional> PrevElt; - unsigned EltSizeInBits = Op.getValueType().getScalarSizeInBits(); + assert(EltSizeInBits >= Op.getValueType().getScalarSizeInBits()); for (unsigned Idx = 0; Idx < NumElts; Idx++) { // Assume undef elements match the sequence; we just have to be careful // when interpolating across them. @@ -3213,14 +3214,14 @@ static std::optional isSimpleVIDSequence(SDValue Op) { if (!isa(Op.getOperand(Idx))) return std::nullopt; Val = Op.getConstantOperandVal(Idx) & - maskTrailingOnes(EltSizeInBits); + maskTrailingOnes(Op.getScalarValueSizeInBits()); } else { // The BUILD_VECTOR must be all constants. if (!isa(Op.getOperand(Idx))) return std::nullopt; if (auto ExactInteger = getExactInteger( cast(Op.getOperand(Idx))->getValueAPF(), - EltSizeInBits)) + Op.getScalarValueSizeInBits())) Val = *ExactInteger; else return std::nullopt; @@ -3276,11 +3277,11 @@ static std::optional isSimpleVIDSequence(SDValue Op) { uint64_t Val; if (IsInteger) { Val = Op.getConstantOperandVal(Idx) & - maskTrailingOnes(EltSizeInBits); + maskTrailingOnes(Op.getScalarValueSizeInBits()); } else { Val = *getExactInteger( cast(Op.getOperand(Idx))->getValueAPF(), - EltSizeInBits); + Op.getScalarValueSizeInBits()); } uint64_t ExpectedVal = (int64_t)(Idx * (uint64_t)*SeqStepNum) / *SeqStepDenom; @@ -3550,7 +3551,7 @@ static SDValue lowerBuildVectorOfConstants(SDValue Op, SelectionDAG &DAG, // Try and match index sequences, which we can lower to the vid instruction // with optional modifications. An all-undef vector is matched by // getSplatValue, above. - if (auto SimpleVID = isSimpleVIDSequence(Op)) { + if (auto SimpleVID = isSimpleVIDSequence(Op, Op.getScalarValueSizeInBits())) { int64_t StepNumerator = SimpleVID->StepNumerator; unsigned StepDenominator = SimpleVID->StepDenominator; int64_t Addend = SimpleVID->Addend; @@ -15562,7 +15563,10 @@ SDValue RISCVTargetLowering::PerformDAGCombine(SDNode *N, if (Index.getOpcode() == ISD::BUILD_VECTOR && MGN->getExtensionType() == ISD::NON_EXTLOAD && isTypeLegal(VT)) { - if (std::optional SimpleVID = isSimpleVIDSequence(Index); + // The sequence will be XLenVT, not the type of Index. Tell + // isSimpleVIDSequence this so we avoid overflow. + if (std::optional SimpleVID = + isSimpleVIDSequence(Index, Subtarget.getXLen()); SimpleVID && SimpleVID->StepDenominator == 1) { const int64_t StepNumerator = SimpleVID->StepNumerator; const int64_t Addend = SimpleVID->Addend; diff --git a/llvm/test/CodeGen/RISCV/rvv/fixed-vectors-masked-gather.ll b/llvm/test/CodeGen/RISCV/rvv/fixed-vectors-masked-gather.ll index 60eec356773bf..88c299a19fb4e 100644 --- a/llvm/test/CodeGen/RISCV/rvv/fixed-vectors-masked-gather.ll +++ b/llvm/test/CodeGen/RISCV/rvv/fixed-vectors-masked-gather.ll @@ -15086,23 +15086,19 @@ define <32 x i64> @mgather_strided_split(ptr %base) { ret <32 x i64> %x } -; FIXME: This is a miscompile triggered by the mgather -> -; riscv.masked.strided.load combine. In order for it to trigger we need either a -; strided gather that RISCVGatherScatterLowering doesn't pick up, or a new -; strided gather generated by the widening sew combine. define <4 x i32> @masked_gather_widen_sew_negative_stride(ptr %base) { ; RV32V-LABEL: masked_gather_widen_sew_negative_stride: ; RV32V: # %bb.0: -; RV32V-NEXT: addi a0, a0, -120 -; RV32V-NEXT: li a1, 120 +; RV32V-NEXT: addi a0, a0, 136 +; RV32V-NEXT: li a1, -136 ; RV32V-NEXT: vsetivli zero, 2, e64, m1, ta, ma ; RV32V-NEXT: vlse64.v v8, (a0), a1 ; RV32V-NEXT: ret ; ; RV64V-LABEL: masked_gather_widen_sew_negative_stride: ; RV64V: # %bb.0: -; RV64V-NEXT: addi a0, a0, -120 -; RV64V-NEXT: li a1, 120 +; RV64V-NEXT: addi a0, a0, 136 +; RV64V-NEXT: li a1, -136 ; RV64V-NEXT: vsetivli zero, 2, e64, m1, ta, ma ; RV64V-NEXT: vlse64.v v8, (a0), a1 ; RV64V-NEXT: ret From 0bf7ff1028fb7cc81324e9c38c585e6533b754e4 Mon Sep 17 00:00:00 2001 From: Chuanqi Xu Date: Mon, 11 Mar 2024 11:14:40 +0800 Subject: [PATCH 24/25] [C++20] [Moduls] Avoid computing odr hash for functions from comparing constraint expression Previously we disabled to compute ODR hash for declarations from the global module fragment. However, we missed the case that the functions lives in the concept requiments (see the attached the test files for example). And the mismatch causes the potential crashment. Due to we will set the function body as lazy after we deserialize it and we will only take its body when needed. However, we don't allow to take the body during deserializing. So it is actually potentially problematic if we set the body as lazy first and computing the hash value of the function, which requires to deserialize its body. So we will meet a crash here. This patch tries to solve the issue by not taking the body of the function from GMF. Note that we can't skip comparing the constraint expression from the GMF directly since it is an key part of the function selecting and it may be the reason why we can't return 0 directly for `FunctionDecl::getODRHash()` from the GMF. --- clang/include/clang/AST/DeclBase.h | 10 +++ clang/include/clang/Serialization/ASTReader.h | 7 -- clang/lib/AST/Decl.cpp | 2 +- clang/lib/AST/DeclBase.cpp | 5 ++ clang/lib/Serialization/ASTReader.cpp | 2 +- clang/lib/Serialization/ASTReaderDecl.cpp | 8 +-- clang/lib/Serialization/ASTWriter.cpp | 2 +- clang/lib/Serialization/ASTWriterDecl.cpp | 8 +-- .../hashing-decls-in-exprs-from-gmf.cppm | 67 +++++++++++++++++++ 9 files changed, 93 insertions(+), 18 deletions(-) create mode 100644 clang/test/Modules/hashing-decls-in-exprs-from-gmf.cppm diff --git a/clang/include/clang/AST/DeclBase.h b/clang/include/clang/AST/DeclBase.h index 9a4736019d1b1..eb7a1a3206007 100644 --- a/clang/include/clang/AST/DeclBase.h +++ b/clang/include/clang/AST/DeclBase.h @@ -673,6 +673,16 @@ class alignas(8) Decl { /// fragment. See [module.global.frag]p3,4 for details. bool isDiscardedInGlobalModuleFragment() const { return false; } + /// Check if we should skip checking ODRHash for declaration \param D. + /// + /// The existing ODRHash mechanism seems to be not stable enough and + /// the false positive ODR violation reports are annoying and we rarely see + /// true ODR violation reports. Also we learned that MSVC disabled ODR checks + /// for declarations in GMF. So we try to disable ODR checks in the GMF to + /// get better user experiences before we make the ODR violation checks stable + /// enough. + bool shouldSkipCheckingODR() const; + /// Return true if this declaration has an attribute which acts as /// definition of the entity, such as 'alias' or 'ifunc'. bool hasDefiningAttr() const; diff --git a/clang/include/clang/Serialization/ASTReader.h b/clang/include/clang/Serialization/ASTReader.h index cd28226c295b3..62c25f5b7a0df 100644 --- a/clang/include/clang/Serialization/ASTReader.h +++ b/clang/include/clang/Serialization/ASTReader.h @@ -2451,13 +2451,6 @@ class BitsUnpacker { uint32_t Value; uint32_t CurrentBitsIndex = ~0; }; - -inline bool shouldSkipCheckingODR(const Decl *D) { - return D->getOwningModule() && - D->getASTContext().getLangOpts().SkipODRCheckInGMF && - D->getOwningModule()->isExplicitGlobalModule(); -} - } // namespace clang #endif // LLVM_CLANG_SERIALIZATION_ASTREADER_H diff --git a/clang/lib/AST/Decl.cpp b/clang/lib/AST/Decl.cpp index 26fdfa040796e..1ee33fd7576d7 100644 --- a/clang/lib/AST/Decl.cpp +++ b/clang/lib/AST/Decl.cpp @@ -4476,7 +4476,7 @@ unsigned FunctionDecl::getODRHash() { } class ODRHash Hash; - Hash.AddFunctionDecl(this); + Hash.AddFunctionDecl(this, /*SkipBody=*/shouldSkipCheckingODR()); setHasODRHash(true); ODRHash = Hash.CalculateHash(); return ODRHash; diff --git a/clang/lib/AST/DeclBase.cpp b/clang/lib/AST/DeclBase.cpp index 8163f9bdaf8d9..6b3c13ff206d2 100644 --- a/clang/lib/AST/DeclBase.cpp +++ b/clang/lib/AST/DeclBase.cpp @@ -1102,6 +1102,11 @@ bool Decl::isInAnotherModuleUnit() const { return M != getASTContext().getCurrentNamedModule(); } +bool Decl::shouldSkipCheckingODR() const { + return getASTContext().getLangOpts().SkipODRCheckInGMF && getOwningModule() && + getOwningModule()->isExplicitGlobalModule(); +} + static Decl::Kind getKind(const Decl *D) { return D->getKind(); } static Decl::Kind getKind(const DeclContext *DC) { return DC->getDeclKind(); } diff --git a/clang/lib/Serialization/ASTReader.cpp b/clang/lib/Serialization/ASTReader.cpp index 028610deb3001..490b8cb10a484 100644 --- a/clang/lib/Serialization/ASTReader.cpp +++ b/clang/lib/Serialization/ASTReader.cpp @@ -9745,7 +9745,7 @@ void ASTReader::finishPendingActions() { !NonConstDefn->isLateTemplateParsed() && // We only perform ODR checks for decls not in the explicit // global module fragment. - !shouldSkipCheckingODR(FD) && + !FD->shouldSkipCheckingODR() && FD->getODRHash() != NonConstDefn->getODRHash()) { if (!isa(FD)) { PendingFunctionOdrMergeFailures[FD].push_back(NonConstDefn); diff --git a/clang/lib/Serialization/ASTReaderDecl.cpp b/clang/lib/Serialization/ASTReaderDecl.cpp index 321c11e55c14e..110f55f8c0f49 100644 --- a/clang/lib/Serialization/ASTReaderDecl.cpp +++ b/clang/lib/Serialization/ASTReaderDecl.cpp @@ -832,7 +832,7 @@ void ASTDeclReader::VisitEnumDecl(EnumDecl *ED) { Reader.mergeDefinitionVisibility(OldDef, ED); // We don't want to check the ODR hash value for declarations from global // module fragment. - if (!shouldSkipCheckingODR(ED) && + if (!ED->shouldSkipCheckingODR() && OldDef->getODRHash() != ED->getODRHash()) Reader.PendingEnumOdrMergeFailures[OldDef].push_back(ED); } else { @@ -874,7 +874,7 @@ void ASTDeclReader::VisitRecordDecl(RecordDecl *RD) { VisitRecordDeclImpl(RD); // We should only reach here if we're in C/Objective-C. There is no // global module fragment. - assert(!shouldSkipCheckingODR(RD)); + assert(!RD->shouldSkipCheckingODR()); RD->setODRHash(Record.readInt()); // Maintain the invariant of a redeclaration chain containing only @@ -2152,7 +2152,7 @@ void ASTDeclReader::MergeDefinitionData( } // We don't want to check ODR for decls in the global module fragment. - if (shouldSkipCheckingODR(MergeDD.Definition)) + if (MergeDD.Definition->shouldSkipCheckingODR()) return; if (D->getODRHash() != MergeDD.ODRHash) { @@ -3526,7 +3526,7 @@ ASTDeclReader::FindExistingResult ASTDeclReader::findExisting(NamedDecl *D) { // same template specialization into the same CXXRecordDecl. auto MergedDCIt = Reader.MergedDeclContexts.find(D->getLexicalDeclContext()); if (MergedDCIt != Reader.MergedDeclContexts.end() && - !shouldSkipCheckingODR(D) && MergedDCIt->second == D->getDeclContext()) + !D->shouldSkipCheckingODR() && MergedDCIt->second == D->getDeclContext()) Reader.PendingOdrMergeChecks.push_back(D); return FindExistingResult(Reader, D, /*Existing=*/nullptr, diff --git a/clang/lib/Serialization/ASTWriter.cpp b/clang/lib/Serialization/ASTWriter.cpp index 73018c1170d8f..378a1f86bd534 100644 --- a/clang/lib/Serialization/ASTWriter.cpp +++ b/clang/lib/Serialization/ASTWriter.cpp @@ -6010,7 +6010,7 @@ void ASTRecordWriter::AddCXXDefinitionData(const CXXRecordDecl *D) { BitsPacker DefinitionBits; - bool ShouldSkipCheckingODR = shouldSkipCheckingODR(D); + bool ShouldSkipCheckingODR = D->shouldSkipCheckingODR(); DefinitionBits.addBit(ShouldSkipCheckingODR); #define FIELD(Name, Width, Merge) \ diff --git a/clang/lib/Serialization/ASTWriterDecl.cpp b/clang/lib/Serialization/ASTWriterDecl.cpp index e73800100e3cc..42583c09f009e 100644 --- a/clang/lib/Serialization/ASTWriterDecl.cpp +++ b/clang/lib/Serialization/ASTWriterDecl.cpp @@ -488,7 +488,7 @@ void ASTDeclWriter::VisitEnumDecl(EnumDecl *D) { BitsPacker EnumDeclBits; EnumDeclBits.addBits(D->getNumPositiveBits(), /*BitWidth=*/8); EnumDeclBits.addBits(D->getNumNegativeBits(), /*BitWidth=*/8); - bool ShouldSkipCheckingODR = shouldSkipCheckingODR(D); + bool ShouldSkipCheckingODR = D->shouldSkipCheckingODR(); EnumDeclBits.addBit(ShouldSkipCheckingODR); EnumDeclBits.addBit(D->isScoped()); EnumDeclBits.addBit(D->isScopedUsingClassTag()); @@ -514,7 +514,7 @@ void ASTDeclWriter::VisitEnumDecl(EnumDecl *D) { !D->isTopLevelDeclInObjCContainer() && !CXXRecordDecl::classofKind(D->getKind()) && !D->getIntegerTypeSourceInfo() && !D->getMemberSpecializationInfo() && - !needsAnonymousDeclarationNumber(D) && !shouldSkipCheckingODR(D) && + !needsAnonymousDeclarationNumber(D) && !D->shouldSkipCheckingODR() && D->getDeclName().getNameKind() == DeclarationName::Identifier) AbbrevToUse = Writer.getDeclEnumAbbrev(); @@ -680,7 +680,7 @@ void ASTDeclWriter::VisitFunctionDecl(FunctionDecl *D) { // FIXME: stable encoding FunctionDeclBits.addBits(llvm::to_underlying(D->getLinkageInternal()), 3); FunctionDeclBits.addBits((uint32_t)D->getStorageClass(), /*BitWidth=*/3); - bool ShouldSkipCheckingODR = shouldSkipCheckingODR(D); + bool ShouldSkipCheckingODR = D->shouldSkipCheckingODR(); FunctionDeclBits.addBit(ShouldSkipCheckingODR); FunctionDeclBits.addBit(D->isInlineSpecified()); FunctionDeclBits.addBit(D->isInlined()); @@ -1514,7 +1514,7 @@ void ASTDeclWriter::VisitCXXMethodDecl(CXXMethodDecl *D) { D->getFirstDecl() == D->getMostRecentDecl() && !D->isInvalidDecl() && !D->hasAttrs() && !D->isTopLevelDeclInObjCContainer() && D->getDeclName().getNameKind() == DeclarationName::Identifier && - !shouldSkipCheckingODR(D) && !D->hasExtInfo() && + !D->shouldSkipCheckingODR() && !D->hasExtInfo() && !D->isExplicitlyDefaulted()) { if (D->getTemplatedKind() == FunctionDecl::TK_NonTemplate || D->getTemplatedKind() == FunctionDecl::TK_FunctionTemplate || diff --git a/clang/test/Modules/hashing-decls-in-exprs-from-gmf.cppm b/clang/test/Modules/hashing-decls-in-exprs-from-gmf.cppm new file mode 100644 index 0000000000000..8db53c0ace879 --- /dev/null +++ b/clang/test/Modules/hashing-decls-in-exprs-from-gmf.cppm @@ -0,0 +1,67 @@ +// RUN: rm -rf %t +// RUN: mkdir -p %t +// RUN: split-file %s %t +// +// RUN: %clang_cc1 -std=c++20 -fskip-odr-check-in-gmf %t/A.cppm -emit-module-interface -o %t/A.pcm +// RUN: %clang_cc1 -std=c++20 -fskip-odr-check-in-gmf %t/B.cppm -emit-module-interface -o %t/B.pcm +// RUN: %clang_cc1 -std=c++20 -fskip-odr-check-in-gmf %t/test.cpp -fprebuilt-module-path=%t -fsyntax-only -verify + +//--- header.h +#pragma once +template +class Optional {}; + +template +concept C = requires(const _Tp& __t) { + [](const Optional<_Up>&) {}(__t); +}; + +//--- func.h +#include "header.h" +template +void func() {} + +//--- duplicated_func.h +#include "header.h" +template +void duplicated_func() {} + +//--- test_func.h +#include "func.h" + +void test_func() { + func>(); +} + +//--- test_duplicated_func.h +#include "duplicated_func.h" + +void test_duplicated_func() { + duplicated_func>(); +} + +//--- A.cppm +module; +#include "header.h" +#include "test_duplicated_func.h" +export module A; +export using ::test_duplicated_func; + +//--- B.cppm +module; +#include "header.h" +#include "test_func.h" +#include "test_duplicated_func.h" +export module B; +export using ::test_func; +export using ::test_duplicated_func; + +//--- test.cpp +// expected-no-diagnostics +import A; +import B; + +void test() { + test_func(); + test_duplicated_func(); +} From 26a1d6601d727a96f4301d0d8647b5a42760ae0c Mon Sep 17 00:00:00 2001 From: Phoebe Wang Date: Mon, 4 Mar 2024 18:09:41 +0800 Subject: [PATCH 25/25] [X86] Add missing subvector_subreg_lowering for BF16 (#83720) --- llvm/lib/Target/X86/X86InstrVecCompiler.td | 3 +++ .../CodeGen/X86/avx512bf16-vl-intrinsics.ll | 22 +++++++++++++++++++ llvm/test/CodeGen/X86/bfloat.ll | 1 - 3 files changed, 25 insertions(+), 1 deletion(-) diff --git a/llvm/lib/Target/X86/X86InstrVecCompiler.td b/llvm/lib/Target/X86/X86InstrVecCompiler.td index bbd19cf8d5b25..461b2badc1313 100644 --- a/llvm/lib/Target/X86/X86InstrVecCompiler.td +++ b/llvm/lib/Target/X86/X86InstrVecCompiler.td @@ -83,6 +83,7 @@ defm : subvector_subreg_lowering; defm : subvector_subreg_lowering; defm : subvector_subreg_lowering; defm : subvector_subreg_lowering; +defm : subvector_subreg_lowering; // A 128-bit subvector extract from the first 512-bit vector position is a // subregister copy that needs no instruction. Likewise, a 128-bit subvector @@ -95,6 +96,7 @@ defm : subvector_subreg_lowering; defm : subvector_subreg_lowering; defm : subvector_subreg_lowering; defm : subvector_subreg_lowering; +defm : subvector_subreg_lowering; // A 128-bit subvector extract from the first 512-bit vector position is a // subregister copy that needs no instruction. Likewise, a 128-bit subvector @@ -107,6 +109,7 @@ defm : subvector_subreg_lowering; defm : subvector_subreg_lowering; defm : subvector_subreg_lowering; defm : subvector_subreg_lowering; +defm : subvector_subreg_lowering; // If we're inserting into an all zeros vector, just use a plain move which diff --git a/llvm/test/CodeGen/X86/avx512bf16-vl-intrinsics.ll b/llvm/test/CodeGen/X86/avx512bf16-vl-intrinsics.ll index 0826faa1071b0..482713e12d15c 100644 --- a/llvm/test/CodeGen/X86/avx512bf16-vl-intrinsics.ll +++ b/llvm/test/CodeGen/X86/avx512bf16-vl-intrinsics.ll @@ -381,3 +381,25 @@ entry: %1 = shufflevector <8 x bfloat> %0, <8 x bfloat> undef, <16 x i32> zeroinitializer ret <16 x bfloat> %1 } + +define <16 x i32> @pr83358() { +; X86-LABEL: pr83358: +; X86: # %bb.0: +; X86-NEXT: vcvtneps2bf16y {{\.?LCPI[0-9]+_[0-9]+}}, %xmm0 # encoding: [0x62,0xf2,0x7e,0x28,0x72,0x05,A,A,A,A] +; X86-NEXT: # fixup A - offset: 6, value: {{\.?LCPI[0-9]+_[0-9]+}}, kind: FK_Data_4 +; X86-NEXT: vshufi64x2 $0, %zmm0, %zmm0, %zmm0 # encoding: [0x62,0xf3,0xfd,0x48,0x43,0xc0,0x00] +; X86-NEXT: # zmm0 = zmm0[0,1,0,1,0,1,0,1] +; X86-NEXT: retl # encoding: [0xc3] +; +; X64-LABEL: pr83358: +; X64: # %bb.0: +; X64-NEXT: vcvtneps2bf16y {{\.?LCPI[0-9]+_[0-9]+}}(%rip), %xmm0 # encoding: [0x62,0xf2,0x7e,0x28,0x72,0x05,A,A,A,A] +; X64-NEXT: # fixup A - offset: 6, value: {{\.?LCPI[0-9]+_[0-9]+}}-4, kind: reloc_riprel_4byte +; X64-NEXT: vshufi64x2 $0, %zmm0, %zmm0, %zmm0 # encoding: [0x62,0xf3,0xfd,0x48,0x43,0xc0,0x00] +; X64-NEXT: # zmm0 = zmm0[0,1,0,1,0,1,0,1] +; X64-NEXT: retq # encoding: [0xc3] + %1 = call <8 x bfloat> @llvm.x86.avx512bf16.cvtneps2bf16.256(<8 x float> ) + %2 = bitcast <8 x bfloat> %1 to <4 x i32> + %3 = shufflevector <4 x i32> %2, <4 x i32> undef, <16 x i32> + ret <16 x i32> %3 +} diff --git a/llvm/test/CodeGen/X86/bfloat.ll b/llvm/test/CodeGen/X86/bfloat.ll index f2d3c4fb34199..cd1dba1761162 100644 --- a/llvm/test/CodeGen/X86/bfloat.ll +++ b/llvm/test/CodeGen/X86/bfloat.ll @@ -2423,7 +2423,6 @@ define <16 x bfloat> @fptrunc_v16f32(<16 x float> %a) nounwind { ; AVXNC-LABEL: fptrunc_v16f32: ; AVXNC: # %bb.0: ; AVXNC-NEXT: {vex} vcvtneps2bf16 %ymm0, %xmm0 -; AVXNC-NEXT: vinsertf128 $0, %xmm0, %ymm0, %ymm0 ; AVXNC-NEXT: {vex} vcvtneps2bf16 %ymm1, %xmm1 ; AVXNC-NEXT: vinsertf128 $1, %xmm1, %ymm0, %ymm0 ; AVXNC-NEXT: retq