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@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@lists.llvm.org > https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
