[clang] 6809af1 - Revert "[OpenMP][OMPIRBuilder] Move SIMD alignment calculation to LLVM Frontend"
Roman Lebedev via cfe-commits
cfe-commits at lists.llvm.org
Fri Jan 13 12:42:04 PST 2023
Reminder to please always mention the reason for the revert/recommit
in the commit message.
On Fri, Jan 13, 2023 at 11:40 PM Dominik Adamski via cfe-commits
<cfe-commits at lists.llvm.org> wrote:
>
>
> Author: Dominik Adamski
> Date: 2023-01-13T14:38:17-06:00
> New Revision: 6809af1a232bc5ac71358e4b874759ddaae056a1
>
> URL: https://github.com/llvm/llvm-project/commit/6809af1a232bc5ac71358e4b874759ddaae056a1
> DIFF: https://github.com/llvm/llvm-project/commit/6809af1a232bc5ac71358e4b874759ddaae056a1.diff
>
> LOG: Revert "[OpenMP][OMPIRBuilder] Move SIMD alignment calculation to LLVM Frontend"
>
> This reverts commit ed01de67433174d3157e9d239d59dd465d52c6a5.
>
> Added:
>
>
> Modified:
> clang/include/clang/Basic/TargetInfo.h
> clang/lib/AST/ASTContext.cpp
> clang/lib/Basic/TargetInfo.cpp
> clang/lib/Basic/Targets/PPC.h
> clang/lib/Basic/Targets/WebAssembly.h
> clang/lib/Basic/Targets/X86.cpp
> lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
> llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
> llvm/include/llvm/Target/TargetMachine.h
> llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
> llvm/lib/Target/PowerPC/PPCTargetMachine.cpp
> llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp
> llvm/lib/Target/X86/X86TargetMachine.cpp
>
> Removed:
>
>
>
> ################################################################################
> diff --git a/clang/include/clang/Basic/TargetInfo.h b/clang/include/clang/Basic/TargetInfo.h
> index def708daac8d2..a5aea33d84751 100644
> --- a/clang/include/clang/Basic/TargetInfo.h
> +++ b/clang/include/clang/Basic/TargetInfo.h
> @@ -226,6 +226,7 @@ class TargetInfo : public virtual TransferrableTargetInfo,
> bool HasStrictFP;
>
> unsigned char MaxAtomicPromoteWidth, MaxAtomicInlineWidth;
> + unsigned short SimdDefaultAlign;
> std::string DataLayoutString;
> const char *UserLabelPrefix;
> const char *MCountName;
> @@ -794,6 +795,10 @@ class TargetInfo : public virtual TransferrableTargetInfo,
>
> /// Return the maximum vector alignment supported for the given target.
> unsigned getMaxVectorAlign() const { return MaxVectorAlign; }
> + /// Return default simd alignment for the given target. Generally, this
> + /// value is type-specific, but this alignment can be used for most of the
> + /// types for the given target.
> + unsigned getSimdDefaultAlign() const { return SimdDefaultAlign; }
>
> unsigned getMaxOpenCLWorkGroupSize() const { return MaxOpenCLWorkGroupSize; }
>
>
> diff --git a/clang/lib/AST/ASTContext.cpp b/clang/lib/AST/ASTContext.cpp
> index 6b97407236ca5..15a43807c3603 100644
> --- a/clang/lib/AST/ASTContext.cpp
> +++ b/clang/lib/AST/ASTContext.cpp
> @@ -79,7 +79,6 @@
> #include "llvm/ADT/StringExtras.h"
> #include "llvm/ADT/StringRef.h"
> #include "llvm/ADT/Triple.h"
> -#include "llvm/Frontend/OpenMP/OMPIRBuilder.h"
> #include "llvm/Support/Capacity.h"
> #include "llvm/Support/Casting.h"
> #include "llvm/Support/Compiler.h"
> @@ -94,7 +93,6 @@
> #include <cstdlib>
> #include <map>
> #include <memory>
> -#include <numeric>
> #include <optional>
> #include <string>
> #include <tuple>
> @@ -2465,16 +2463,7 @@ unsigned ASTContext::getTypeUnadjustedAlign(const Type *T) const {
> }
>
> unsigned ASTContext::getOpenMPDefaultSimdAlign(QualType T) const {
> - const std::vector<std::string> &TargetFeatures =
> - Target->getTargetOpts().Features;
> - std::string TargetFeaturesString = std::accumulate(
> - TargetFeatures.cbegin(), TargetFeatures.cend(), std::string(),
> - [](const std::string &s1, const std::string &s2) {
> - return s1.empty() ? s2 : s1 + "," + s2;
> - });
> - unsigned SimdAlign = llvm::OpenMPIRBuilder ::getSimdDefaultAlignment(
> - getTargetInfo().getTriple().str(), Target->getTargetOpts().CPU,
> - TargetFeaturesString);
> + unsigned SimdAlign = getTargetInfo().getSimdDefaultAlign();
> return SimdAlign;
> }
>
>
> diff --git a/clang/lib/Basic/TargetInfo.cpp b/clang/lib/Basic/TargetInfo.cpp
> index fa5e568d599d0..8ee43261fc1d3 100644
> --- a/clang/lib/Basic/TargetInfo.cpp
> +++ b/clang/lib/Basic/TargetInfo.cpp
> @@ -119,6 +119,7 @@ TargetInfo::TargetInfo(const llvm::Triple &T) : Triple(T) {
> MaxAtomicPromoteWidth = MaxAtomicInlineWidth = 0;
> MaxVectorAlign = 0;
> MaxTLSAlign = 0;
> + SimdDefaultAlign = 0;
> SizeType = UnsignedLong;
> PtrDiffType = SignedLong;
> IntMaxType = SignedLongLong;
>
> diff --git a/clang/lib/Basic/Targets/PPC.h b/clang/lib/Basic/Targets/PPC.h
> index 4c02183feb4c1..cc185fdadfcbc 100644
> --- a/clang/lib/Basic/Targets/PPC.h
> +++ b/clang/lib/Basic/Targets/PPC.h
> @@ -87,6 +87,7 @@ class LLVM_LIBRARY_VISIBILITY PPCTargetInfo : public TargetInfo {
> PPCTargetInfo(const llvm::Triple &Triple, const TargetOptions &)
> : TargetInfo(Triple) {
> SuitableAlign = 128;
> + SimdDefaultAlign = 128;
> LongDoubleWidth = LongDoubleAlign = 128;
> LongDoubleFormat = &llvm::APFloat::PPCDoubleDouble();
> HasStrictFP = true;
>
> diff --git a/clang/lib/Basic/Targets/WebAssembly.h b/clang/lib/Basic/Targets/WebAssembly.h
> index 1f0bb08665347..1e73450fdd0c3 100644
> --- a/clang/lib/Basic/Targets/WebAssembly.h
> +++ b/clang/lib/Basic/Targets/WebAssembly.h
> @@ -49,6 +49,7 @@ class LLVM_LIBRARY_VISIBILITY WebAssemblyTargetInfo : public TargetInfo {
> SuitableAlign = 128;
> LargeArrayMinWidth = 128;
> LargeArrayAlign = 128;
> + SimdDefaultAlign = 128;
> SigAtomicType = SignedLong;
> LongDoubleWidth = LongDoubleAlign = 128;
> LongDoubleFormat = &llvm::APFloat::IEEEquad();
>
> diff --git a/clang/lib/Basic/Targets/X86.cpp b/clang/lib/Basic/Targets/X86.cpp
> index d700fb9a4e83b..a6eea30311d3d 100644
> --- a/clang/lib/Basic/Targets/X86.cpp
> +++ b/clang/lib/Basic/Targets/X86.cpp
> @@ -399,6 +399,9 @@ bool X86TargetInfo::handleTargetFeatures(std::vector<std::string> &Features,
> return false;
> }
>
> + SimdDefaultAlign =
> + hasFeature("avx512f") ? 512 : hasFeature("avx") ? 256 : 128;
> +
> // FIXME: We should allow long double type on 32-bits to match with GCC.
> // This requires backend to be able to lower f80 without x87 first.
> if (!HasX87 && LongDoubleFormat == &llvm::APFloat::x87DoubleExtended())
>
> diff --git a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
> index c22d5827ebbef..58e81fb1cd745 100644
> --- a/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
> +++ b/lldb/source/Plugins/ExpressionParser/Clang/ClangExpressionParser.cpp
> @@ -500,6 +500,8 @@ ClangExpressionParser::ClangExpressionParser(
> auto target_info = TargetInfo::CreateTargetInfo(
> m_compiler->getDiagnostics(), m_compiler->getInvocation().TargetOpts);
> if (log) {
> + LLDB_LOGF(log, "Using SIMD alignment: %d",
> + target_info->getSimdDefaultAlign());
> LLDB_LOGF(log, "Target datalayout string: '%s'",
> target_info->getDataLayoutString());
> LLDB_LOGF(log, "Target ABI: '%s'", target_info->getABI().str().c_str());
>
> diff --git a/llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h b/llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
> index ce4a4f24771c6..2c1abe9e2840e 100644
> --- a/llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
> +++ b/llvm/include/llvm/Frontend/OpenMP/OMPIRBuilder.h
> @@ -502,14 +502,6 @@ class OpenMPIRBuilder {
> ArrayRef<CanonicalLoopInfo *> Loops,
> InsertPointTy ComputeIP);
>
> - /// Get the alignment value for given target
> - ///
> - /// \param Triple String which describes target triple
> - /// \param CPU String which describes target CPU
> - /// \param Features String which describes extra CPU features
> - static unsigned getSimdDefaultAlignment(const std::string &Triple,
> - StringRef CPU, StringRef Features);
> -
> private:
> /// Modifies the canonical loop to be a statically-scheduled workshare loop.
> ///
>
> diff --git a/llvm/include/llvm/Target/TargetMachine.h b/llvm/include/llvm/Target/TargetMachine.h
> index c088052e121ab..6361373ba71b8 100644
> --- a/llvm/include/llvm/Target/TargetMachine.h
> +++ b/llvm/include/llvm/Target/TargetMachine.h
> @@ -108,9 +108,6 @@ class TargetMachine {
> std::unique_ptr<const MCInstrInfo> MII;
> std::unique_ptr<const MCSubtargetInfo> STI;
>
> - /// Simd target specific information
> - unsigned SimdDefaultAlignment = 0;
> -
> unsigned RequireStructuredCFG : 1;
> unsigned O0WantsFastISel : 1;
>
> @@ -207,9 +204,6 @@ class TargetMachine {
> return DL.getPointerSize(DL.getAllocaAddrSpace());
> }
>
> - /// Return default SIMD alignment
> - unsigned getSimdDefaultAlignment() const { return SimdDefaultAlignment; }
> -
> /// Reset the target options based on the function's attributes.
> // FIXME: Remove TargetOptions that affect per-function code generation
> // from TargetMachine.
>
> diff --git a/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp b/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
> index 05bea28b53510..ddbcef1593715 100644
> --- a/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
> +++ b/llvm/lib/Frontend/OpenMP/OMPIRBuilder.cpp
> @@ -255,50 +255,6 @@ static void redirectTo(BasicBlock *Source, BasicBlock *Target, DebugLoc DL) {
> NewBr->setDebugLoc(DL);
> }
>
> -/// Create the TargetMachine object to query the backend for optimization
> -/// preferences.
> -///
> -/// Ideally, this would be passed from the front-end to the OpenMPBuilder, but
> -/// e.g. Clang does not pass it to its CodeGen layer and creates it only when
> -/// needed for the LLVM pass pipline. We use some default options to avoid
> -/// having to pass too many settings from the frontend that probably do not
> -/// matter.
> -///
> -/// Currently, TargetMachine is only used sometimes by the unrollLoopPartial
> -/// and getVectorTypeAlignment methods. If we are going to use TargetMachine
> -/// for more purposes, especially those that are sensitive to TargetOptions,
> -/// RelocModel and CodeModel, it might become be worth requiring front-ends to
> -/// pass on their TargetMachine, or at least cache it between methods.
> -/// Note that while fontends such as Clang have just a single main
> -/// TargetMachine per translation unit, "target-cpu" and "target-features"
> -/// that determine the TargetMachine are per-function and can be overrided
> -/// using __attribute__((target("OPTIONS"))).
> -static std::unique_ptr<TargetMachine>
> -createTargetMachine(const std::string &Triple, StringRef CPU,
> - StringRef Features, CodeGenOpt::Level OptLevel) {
> - std::string Error;
> - const llvm::Target *TheTarget = TargetRegistry::lookupTarget(Triple, Error);
> - if (!TheTarget)
> - return {};
> -
> - llvm::TargetOptions Options;
> - return std::unique_ptr<TargetMachine>(TheTarget->createTargetMachine(
> - Triple, CPU, Features, Options, /*RelocModel=*/std::nullopt,
> - /*CodeModel=*/std::nullopt, OptLevel));
> -}
> -
> -/// Create the TargetMachine object to query the backend for optimization
> -/// preferences.
> -static std::unique_ptr<TargetMachine>
> -createTargetMachine(Function *F, CodeGenOpt::Level OptLevel) {
> - Module *M = F->getParent();
> -
> - StringRef CPU = F->getFnAttribute("target-cpu").getValueAsString();
> - StringRef Features = F->getFnAttribute("target-features").getValueAsString();
> - const std::string &Triple = M->getTargetTriple();
> - return createTargetMachine(Triple, CPU, Features, OptLevel);
> -}
> -
> void llvm::spliceBB(IRBuilderBase::InsertPoint IP, BasicBlock *New,
> bool CreateBranch) {
> assert(New->getFirstInsertionPt() == New->begin() &&
> @@ -3083,18 +3039,6 @@ void OpenMPIRBuilder::createIfVersion(CanonicalLoopInfo *CanonicalLoop,
> Builder.CreateBr(NewBlocks.front());
> }
>
> -unsigned OpenMPIRBuilder::getSimdDefaultAlignment(const std::string &Triple,
> - StringRef CPU,
> - StringRef Features) {
> - std::unique_ptr<TargetMachine> TgtInfo(
> - createTargetMachine(Triple, CPU, Features, CodeGenOpt::Default));
> - if (!TgtInfo) {
> - return 0;
> - }
> -
> - return TgtInfo->getSimdDefaultAlignment();
> -}
> -
> void OpenMPIRBuilder::applySimd(CanonicalLoopInfo *CanonicalLoop,
> MapVector<Value *, Value *> AlignedVars,
> Value *IfCond, OrderKind Order,
> @@ -3197,6 +3141,42 @@ void OpenMPIRBuilder::applySimd(CanonicalLoopInfo *CanonicalLoop,
> addLoopMetadata(CanonicalLoop, LoopMDList);
> }
>
> +/// Create the TargetMachine object to query the backend for optimization
> +/// preferences.
> +///
> +/// Ideally, this would be passed from the front-end to the OpenMPBuilder, but
> +/// e.g. Clang does not pass it to its CodeGen layer and creates it only when
> +/// needed for the LLVM pass pipline. We use some default options to avoid
> +/// having to pass too many settings from the frontend that probably do not
> +/// matter.
> +///
> +/// Currently, TargetMachine is only used sometimes by the unrollLoopPartial
> +/// method. If we are going to use TargetMachine for more purposes, especially
> +/// those that are sensitive to TargetOptions, RelocModel and CodeModel, it
> +/// might become be worth requiring front-ends to pass on their TargetMachine,
> +/// or at least cache it between methods. Note that while fontends such as Clang
> +/// have just a single main TargetMachine per translation unit, "target-cpu" and
> +/// "target-features" that determine the TargetMachine are per-function and can
> +/// be overrided using __attribute__((target("OPTIONS"))).
> +static std::unique_ptr<TargetMachine>
> +createTargetMachine(Function *F, CodeGenOpt::Level OptLevel) {
> + Module *M = F->getParent();
> +
> + StringRef CPU = F->getFnAttribute("target-cpu").getValueAsString();
> + StringRef Features = F->getFnAttribute("target-features").getValueAsString();
> + const std::string &Triple = M->getTargetTriple();
> +
> + std::string Error;
> + const llvm::Target *TheTarget = TargetRegistry::lookupTarget(Triple, Error);
> + if (!TheTarget)
> + return {};
> +
> + llvm::TargetOptions Options;
> + return std::unique_ptr<TargetMachine>(TheTarget->createTargetMachine(
> + Triple, CPU, Features, Options, /*RelocModel=*/std::nullopt,
> + /*CodeModel=*/std::nullopt, OptLevel));
> +}
> +
> /// Heuristically determine the best-performant unroll factor for \p CLI. This
> /// depends on the target processor. We are re-using the same heuristics as the
> /// LoopUnrollPass.
>
> diff --git a/llvm/lib/Target/PowerPC/PPCTargetMachine.cpp b/llvm/lib/Target/PowerPC/PPCTargetMachine.cpp
> index df5d120e0e10a..9aea5af8a60a4 100644
> --- a/llvm/lib/Target/PowerPC/PPCTargetMachine.cpp
> +++ b/llvm/lib/Target/PowerPC/PPCTargetMachine.cpp
> @@ -333,7 +333,6 @@ PPCTargetMachine::PPCTargetMachine(const Target &T, const Triple &TT,
> TLOF(createTLOF(getTargetTriple())),
> TargetABI(computeTargetABI(TT, Options)),
> Endianness(isLittleEndianTriple(TT) ? Endian::LITTLE : Endian::BIG) {
> - SimdDefaultAlignment = 128;
> initAsmInfo();
> }
>
>
> diff --git a/llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp b/llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp
> index a76647fb90e7e..630c786a3dc78 100644
> --- a/llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp
> +++ b/llvm/lib/Target/WebAssembly/WebAssemblyTargetMachine.cpp
> @@ -139,7 +139,6 @@ WebAssemblyTargetMachine::WebAssemblyTargetMachine(
> this->Options.FunctionSections = true;
> this->Options.DataSections = true;
> this->Options.UniqueSectionNames = true;
> - SimdDefaultAlignment = 128;
>
> initAsmInfo();
>
>
> diff --git a/llvm/lib/Target/X86/X86TargetMachine.cpp b/llvm/lib/Target/X86/X86TargetMachine.cpp
> index d53554a19f974..3e8e8af7c2cc0 100644
> --- a/llvm/lib/Target/X86/X86TargetMachine.cpp
> +++ b/llvm/lib/Target/X86/X86TargetMachine.cpp
> @@ -245,13 +245,6 @@ X86TargetMachine::X86TargetMachine(const Target &T, const Triple &TT,
> setSupportsDebugEntryValues(true);
>
> initAsmInfo();
> -
> - if (FS.contains("+avx512f"))
> - SimdDefaultAlignment = 512;
> - else if (FS.contains("+avx"))
> - SimdDefaultAlignment = 256;
> - else
> - SimdDefaultAlignment = 128;
> }
>
> X86TargetMachine::~X86TargetMachine() = default;
>
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
More information about the cfe-commits
mailing list