From dd2810924a8b4e8754c67152362890b345a2d174 Mon Sep 17 00:00:00 2001 From: Johannes Doerfert Date: Tue, 7 Jul 2020 01:08:03 -0500 Subject: [PATCH] [OpenMP] Allow traits for the OpenMP context selector `isa` It was unclear what `isa` was supposed to mean so we did not provide any traits for this context selector. With this patch we will allow *any* string or identifier. We use the target attribute and target info to determine if the trait matches. In other words, we will check if the provided value is a target feature that is available (at the call site). Fixes PR46338 Reviewed By: ABataev Differential Revision: https://reviews.llvm.org/D83281 --- include/llvm/Frontend/OpenMP/OMPContext.h | 49 ++++++++++---- include/llvm/Frontend/OpenMP/OMPKinds.def | 6 +- lib/Frontend/OpenMP/OMPContext.cpp | 27 ++++++-- unittests/Frontend/OpenMPContextTest.cpp | 80 ++++++++++++----------- 4 files changed, 105 insertions(+), 57 deletions(-) diff --git a/include/llvm/Frontend/OpenMP/OMPContext.h b/include/llvm/Frontend/OpenMP/OMPContext.h index 1a42d189db4..8a4179167c8 100644 --- a/include/llvm/Frontend/OpenMP/OMPContext.h +++ b/include/llvm/Frontend/OpenMP/OMPContext.h @@ -70,15 +70,20 @@ TraitSelector getOpenMPContextTraitSelectorForProperty(TraitProperty Property); /// Return a textual representation of the trait selector \p Kind. StringRef getOpenMPContextTraitSelectorName(TraitSelector Kind); -/// Parse \p Str and return the trait set it matches or -/// TraitProperty::invalid. -TraitProperty getOpenMPContextTraitPropertyKind(TraitSet Set, StringRef Str); +/// Parse \p Str and return the trait property it matches in the set \p Set and +/// selector \p Selector or TraitProperty::invalid. +TraitProperty getOpenMPContextTraitPropertyKind(TraitSet Set, + TraitSelector Selector, + StringRef Str); /// Return the trait property for a singleton selector \p Selector. TraitProperty getOpenMPContextTraitPropertyForSelector(TraitSelector Selector); -/// Return a textual representation of the trait property \p Kind. -StringRef getOpenMPContextTraitPropertyName(TraitProperty Kind); +/// Return a textual representation of the trait property \p Kind, which might +/// be the raw string we parsed (\p RawString) if we do not translate the +/// property into a (distinct) enum. +StringRef getOpenMPContextTraitPropertyName(TraitProperty Kind, + StringRef RawString); /// Return a textual representation of the trait property \p Kind with selector /// and set name included. @@ -112,24 +117,36 @@ bool isValidTraitPropertyForTraitSetAndSelector(TraitProperty Property, /// scored (via the ScoresMap). In addition, the required consturct nesting is /// decribed as well. struct VariantMatchInfo { - /// Add the trait \p Property to the required trait set. If \p Score is not - /// null, it recorded as well. If \p Property is in the `construct` set it - /// is recorded in-order in the ConstructTraits as well. - void addTrait(TraitProperty Property, APInt *Score = nullptr) { - addTrait(getOpenMPContextTraitSetForProperty(Property), Property, Score); + /// Add the trait \p Property to the required trait set. \p RawString is the + /// string we parsed and derived \p Property from. If \p Score is not null, it + /// recorded as well. If \p Property is in the `construct` set it is recorded + /// in-order in the ConstructTraits as well. + void addTrait(TraitProperty Property, StringRef RawString, + APInt *Score = nullptr) { + addTrait(getOpenMPContextTraitSetForProperty(Property), Property, RawString, + Score); } /// Add the trait \p Property which is in set \p Set to the required trait - /// set. If \p Score is not null, it recorded as well. If \p Set is the - /// `construct` set it is recorded in-order in the ConstructTraits as well. - void addTrait(TraitSet Set, TraitProperty Property, APInt *Score = nullptr) { + /// set. \p RawString is the string we parsed and derived \p Property from. If + /// \p Score is not null, it recorded as well. If \p Set is the `construct` + /// set it is recorded in-order in the ConstructTraits as well. + void addTrait(TraitSet Set, TraitProperty Property, StringRef RawString, + APInt *Score = nullptr) { if (Score) ScoreMap[Property] = *Score; + + // Special handling for `device={isa(...)}` as we do not match the enum but + // the raw string. + if (Property == TraitProperty::device_isa___ANY) + ISATraits.push_back(RawString); + RequiredTraits.set(unsigned(Property)); if (Set == TraitSet::construct) ConstructTraits.push_back(Property); } BitVector RequiredTraits = BitVector(unsigned(TraitProperty::Last) + 1); + SmallVector ISATraits; SmallVector ConstructTraits; SmallDenseMap ScoreMap; }; @@ -139,6 +156,7 @@ struct VariantMatchInfo { /// in OpenMP constructs at the location. struct OMPContext { OMPContext(bool IsDeviceCompilation, Triple TargetTriple); + virtual ~OMPContext() = default; void addTrait(TraitProperty Property) { addTrait(getOpenMPContextTraitSetForProperty(Property), Property); @@ -149,6 +167,11 @@ struct OMPContext { ConstructTraits.push_back(Property); } + /// Hook for users to check if an ISA trait matches. The trait is described as + /// the string that got parsed and it depends on the target and context if + /// this matches or not. + virtual bool matchesISATrait(StringRef) const { return false; } + BitVector ActiveTraits = BitVector(unsigned(TraitProperty::Last) + 1); SmallVector ConstructTraits; }; diff --git a/include/llvm/Frontend/OpenMP/OMPKinds.def b/include/llvm/Frontend/OpenMP/OMPKinds.def index 7771dcd72d6..3fc87dc34cd 100644 --- a/include/llvm/Frontend/OpenMP/OMPKinds.def +++ b/include/llvm/Frontend/OpenMP/OMPKinds.def @@ -1071,7 +1071,11 @@ __OMP_TRAIT_PROPERTY(device, kind, any) __OMP_TRAIT_SELECTOR(device, isa, true) -// TODO: What do we want for ISA? +// We use "__ANY" as a placeholder in the isa property to denote the +// conceptual "any", not the literal `any` used in kind. The string we +// we use is not important except that it will show up in diagnostics. +OMP_TRAIT_PROPERTY(device_isa___ANY, device, device_isa, + "") __OMP_TRAIT_SELECTOR(device, arch, true) diff --git a/lib/Frontend/OpenMP/OMPContext.cpp b/lib/Frontend/OpenMP/OMPContext.cpp index c44e858ab5e..56a6e2b08bd 100644 --- a/lib/Frontend/OpenMP/OMPContext.cpp +++ b/lib/Frontend/OpenMP/OMPContext.cpp @@ -175,11 +175,11 @@ static int isVariantApplicableInContextHelper( LLVM_DEBUG({ if (MK == MK_ALL) dbgs() << "[" << DEBUG_TYPE << "] Property " - << getOpenMPContextTraitPropertyName(Property) + << getOpenMPContextTraitPropertyName(Property, "") << " was not in the OpenMP context but match kind is all.\n"; if (MK == MK_NONE) dbgs() << "[" << DEBUG_TYPE << "] Property " - << getOpenMPContextTraitPropertyName(Property) + << getOpenMPContextTraitPropertyName(Property, "") << " was in the OpenMP context but match kind is none.\n"; }); return false; @@ -198,6 +198,14 @@ static int isVariantApplicableInContextHelper( continue; bool IsActiveTrait = Ctx.ActiveTraits.test(unsigned(Property)); + + // We overwrite the isa trait as it is actually up to the OMPContext hook to + // check the raw string(s). + if (Property == TraitProperty::device_isa___ANY) + IsActiveTrait = llvm::all_of(VMI.ISATraits, [&](StringRef RawString) { + return Ctx.matchesISATrait(RawString); + }); + Optional Result = HandleTrait(Property, IsActiveTrait); if (Result.hasValue()) return Result.getValue(); @@ -225,7 +233,7 @@ static int isVariantApplicableInContextHelper( if (!FoundInOrder) { LLVM_DEBUG(dbgs() << "[" << DEBUG_TYPE << "] Construct property " - << getOpenMPContextTraitPropertyName(Property) + << getOpenMPContextTraitPropertyName(Property, "") << " was not nested properly.\n"); return false; } @@ -425,8 +433,12 @@ StringRef llvm::omp::getOpenMPContextTraitSelectorName(TraitSelector Kind) { llvm_unreachable("Unknown trait selector!"); } -TraitProperty llvm::omp::getOpenMPContextTraitPropertyKind(TraitSet Set, - StringRef S) { +TraitProperty llvm::omp::getOpenMPContextTraitPropertyKind( + TraitSet Set, TraitSelector Selector, StringRef S) { + // Special handling for `device={isa(...)}` as we accept anything here. It is + // up to the target to decide if the feature is available. + if (Set == TraitSet::device && Selector == TraitSelector::device_isa) + return TraitProperty::device_isa___ANY; #define OMP_TRAIT_PROPERTY(Enum, TraitSetEnum, TraitSelectorEnum, Str) \ if (Set == TraitSet::TraitSetEnum && Str == S) \ return TraitProperty::Enum; @@ -444,7 +456,10 @@ llvm::omp::getOpenMPContextTraitPropertyForSelector(TraitSelector Selector) { #include "llvm/Frontend/OpenMP/OMPKinds.def" .Default(TraitProperty::invalid); } -StringRef llvm::omp::getOpenMPContextTraitPropertyName(TraitProperty Kind) { +StringRef llvm::omp::getOpenMPContextTraitPropertyName(TraitProperty Kind, + StringRef RawString) { + if (Kind == TraitProperty::device_isa___ANY) + return RawString; switch (Kind) { #define OMP_TRAIT_PROPERTY(Enum, TraitSetEnum, TraitSelectorEnum, Str) \ case TraitProperty::Enum: \ diff --git a/unittests/Frontend/OpenMPContextTest.cpp b/unittests/Frontend/OpenMPContextTest.cpp index eb505be042c..5938559aacb 100644 --- a/unittests/Frontend/OpenMPContextTest.cpp +++ b/unittests/Frontend/OpenMPContextTest.cpp @@ -38,11 +38,13 @@ TEST_F(OpenMPContextTest, RoundTripAndAssociation) { #define OMP_TRAIT_PROPERTY(Enum, TraitSetEnum, TraitSelectorEnum, Str) \ EXPECT_EQ(TraitProperty::Enum, \ getOpenMPContextTraitPropertyKind( \ - TraitSet::TraitSetEnum, \ - getOpenMPContextTraitPropertyName(TraitProperty::Enum))); \ + TraitSet::TraitSetEnum, TraitSelector::TraitSelectorEnum, \ + getOpenMPContextTraitPropertyName(TraitProperty::Enum, Str))); \ EXPECT_EQ(Str, getOpenMPContextTraitPropertyName( \ - getOpenMPContextTraitPropertyKind(TraitSet::TraitSetEnum, \ - Str))); \ + getOpenMPContextTraitPropertyKind( \ + TraitSet::TraitSetEnum, \ + TraitSelector::TraitSelectorEnum, Str), \ + Str)); \ EXPECT_EQ(TraitSet::TraitSetEnum, \ getOpenMPContextTraitSetForProperty(TraitProperty::Enum)); \ EXPECT_EQ(TraitSelector::TraitSelectorEnum, \ @@ -77,31 +79,31 @@ TEST_F(OpenMPContextTest, ApplicabilityNonConstruct) { EXPECT_TRUE(isVariantApplicableInContext(Empty, DeviceNVPTX)); VariantMatchInfo UserCondFalse; - UserCondFalse.addTrait(TraitProperty::user_condition_false); + UserCondFalse.addTrait(TraitProperty::user_condition_false, ""); EXPECT_FALSE(isVariantApplicableInContext(UserCondFalse, HostLinux)); EXPECT_FALSE(isVariantApplicableInContext(UserCondFalse, DeviceLinux)); EXPECT_FALSE(isVariantApplicableInContext(UserCondFalse, HostNVPTX)); EXPECT_FALSE(isVariantApplicableInContext(UserCondFalse, DeviceNVPTX)); VariantMatchInfo DeviceArchArm; - DeviceArchArm.addTrait(TraitProperty::device_arch_arm); + DeviceArchArm.addTrait(TraitProperty::device_arch_arm, ""); EXPECT_FALSE(isVariantApplicableInContext(DeviceArchArm, HostLinux)); EXPECT_FALSE(isVariantApplicableInContext(DeviceArchArm, DeviceLinux)); EXPECT_FALSE(isVariantApplicableInContext(DeviceArchArm, HostNVPTX)); EXPECT_FALSE(isVariantApplicableInContext(DeviceArchArm, DeviceNVPTX)); VariantMatchInfo LLVMHostUserCondTrue; - LLVMHostUserCondTrue.addTrait(TraitProperty::implementation_vendor_llvm); - LLVMHostUserCondTrue.addTrait(TraitProperty::device_kind_host); - LLVMHostUserCondTrue.addTrait(TraitProperty::device_kind_any); - LLVMHostUserCondTrue.addTrait(TraitProperty::user_condition_true); + LLVMHostUserCondTrue.addTrait(TraitProperty::implementation_vendor_llvm, ""); + LLVMHostUserCondTrue.addTrait(TraitProperty::device_kind_host, ""); + LLVMHostUserCondTrue.addTrait(TraitProperty::device_kind_any, ""); + LLVMHostUserCondTrue.addTrait(TraitProperty::user_condition_true, ""); EXPECT_TRUE(isVariantApplicableInContext(LLVMHostUserCondTrue, HostLinux)); EXPECT_FALSE(isVariantApplicableInContext(LLVMHostUserCondTrue, DeviceLinux)); EXPECT_TRUE(isVariantApplicableInContext(LLVMHostUserCondTrue, HostNVPTX)); EXPECT_FALSE(isVariantApplicableInContext(LLVMHostUserCondTrue, DeviceNVPTX)); VariantMatchInfo LLVMHostUserCondTrueCPU = LLVMHostUserCondTrue; - LLVMHostUserCondTrueCPU.addTrait(TraitProperty::device_kind_cpu); + LLVMHostUserCondTrueCPU.addTrait(TraitProperty::device_kind_cpu, ""); EXPECT_TRUE(isVariantApplicableInContext(LLVMHostUserCondTrueCPU, HostLinux)); EXPECT_FALSE( isVariantApplicableInContext(LLVMHostUserCondTrueCPU, DeviceLinux)); @@ -111,14 +113,14 @@ TEST_F(OpenMPContextTest, ApplicabilityNonConstruct) { isVariantApplicableInContext(LLVMHostUserCondTrueCPU, DeviceNVPTX)); VariantMatchInfo GPU; - GPU.addTrait(TraitProperty::device_kind_gpu); + GPU.addTrait(TraitProperty::device_kind_gpu, ""); EXPECT_FALSE(isVariantApplicableInContext(GPU, HostLinux)); EXPECT_FALSE(isVariantApplicableInContext(GPU, DeviceLinux)); EXPECT_TRUE(isVariantApplicableInContext(GPU, HostNVPTX)); EXPECT_TRUE(isVariantApplicableInContext(GPU, DeviceNVPTX)); VariantMatchInfo NoHost; - NoHost.addTrait(TraitProperty::device_kind_nohost); + NoHost.addTrait(TraitProperty::device_kind_nohost, ""); EXPECT_FALSE(isVariantApplicableInContext(NoHost, HostLinux)); EXPECT_TRUE(isVariantApplicableInContext(NoHost, DeviceLinux)); EXPECT_FALSE(isVariantApplicableInContext(NoHost, HostNVPTX)); @@ -154,7 +156,7 @@ TEST_F(OpenMPContextTest, ApplicabilityAllTraits) { isVariantApplicableInContext(Empty, DeviceNVPTXTargetTeamsParallel)); VariantMatchInfo UserCondFalse; - UserCondFalse.addTrait(TraitProperty::user_condition_false); + UserCondFalse.addTrait(TraitProperty::user_condition_false, ""); EXPECT_FALSE( isVariantApplicableInContext(UserCondFalse, HostLinuxParallelParallel)); EXPECT_FALSE( @@ -164,7 +166,7 @@ TEST_F(OpenMPContextTest, ApplicabilityAllTraits) { DeviceNVPTXTargetTeamsParallel)); VariantMatchInfo DeviceArchArm; - DeviceArchArm.addTrait(TraitProperty::device_arch_arm); + DeviceArchArm.addTrait(TraitProperty::device_arch_arm, ""); EXPECT_FALSE( isVariantApplicableInContext(DeviceArchArm, HostLinuxParallelParallel)); EXPECT_FALSE( @@ -175,10 +177,12 @@ TEST_F(OpenMPContextTest, ApplicabilityAllTraits) { APInt Score(32, 1000); VariantMatchInfo LLVMHostUserCondTrue; - LLVMHostUserCondTrue.addTrait(TraitProperty::implementation_vendor_llvm); - LLVMHostUserCondTrue.addTrait(TraitProperty::device_kind_host); - LLVMHostUserCondTrue.addTrait(TraitProperty::device_kind_any); - LLVMHostUserCondTrue.addTrait(TraitProperty::user_condition_true, &Score); + LLVMHostUserCondTrue.addTrait(TraitProperty::implementation_vendor_llvm, + ""); + LLVMHostUserCondTrue.addTrait(TraitProperty::device_kind_host, ""); + LLVMHostUserCondTrue.addTrait(TraitProperty::device_kind_any, ""); + LLVMHostUserCondTrue.addTrait(TraitProperty::user_condition_true, "", + &Score); EXPECT_TRUE(isVariantApplicableInContext(LLVMHostUserCondTrue, HostLinuxParallelParallel)); EXPECT_FALSE(isVariantApplicableInContext(LLVMHostUserCondTrue, @@ -189,7 +193,7 @@ TEST_F(OpenMPContextTest, ApplicabilityAllTraits) { DeviceNVPTXTargetTeamsParallel)); VariantMatchInfo LLVMHostUserCondTrueCPU = LLVMHostUserCondTrue; - LLVMHostUserCondTrueCPU.addTrait(TraitProperty::device_kind_cpu); + LLVMHostUserCondTrueCPU.addTrait(TraitProperty::device_kind_cpu, ""); EXPECT_TRUE(isVariantApplicableInContext(LLVMHostUserCondTrueCPU, HostLinuxParallelParallel)); EXPECT_FALSE(isVariantApplicableInContext(LLVMHostUserCondTrueCPU, @@ -200,7 +204,7 @@ TEST_F(OpenMPContextTest, ApplicabilityAllTraits) { DeviceNVPTXTargetTeamsParallel)); VariantMatchInfo GPU; - GPU.addTrait(TraitProperty::device_kind_gpu); + GPU.addTrait(TraitProperty::device_kind_gpu, ""); EXPECT_FALSE(isVariantApplicableInContext(GPU, HostLinuxParallelParallel)); EXPECT_FALSE(isVariantApplicableInContext(GPU, DeviceLinuxTargetParallel)); EXPECT_TRUE(isVariantApplicableInContext(GPU, HostNVPTXFor)); @@ -208,7 +212,7 @@ TEST_F(OpenMPContextTest, ApplicabilityAllTraits) { isVariantApplicableInContext(GPU, DeviceNVPTXTargetTeamsParallel)); VariantMatchInfo NoHost; - NoHost.addTrait(TraitProperty::device_kind_nohost); + NoHost.addTrait(TraitProperty::device_kind_nohost, ""); EXPECT_FALSE( isVariantApplicableInContext(NoHost, HostLinuxParallelParallel)); EXPECT_TRUE( @@ -219,8 +223,9 @@ TEST_F(OpenMPContextTest, ApplicabilityAllTraits) { } { // variants with all sets VariantMatchInfo DeviceArchArmParallel; - DeviceArchArmParallel.addTrait(TraitProperty::construct_parallel_parallel); - DeviceArchArmParallel.addTrait(TraitProperty::device_arch_arm); + DeviceArchArmParallel.addTrait(TraitProperty::construct_parallel_parallel, + ""); + DeviceArchArmParallel.addTrait(TraitProperty::device_arch_arm, ""); EXPECT_FALSE(isVariantApplicableInContext(DeviceArchArmParallel, HostLinuxParallelParallel)); EXPECT_FALSE(isVariantApplicableInContext(DeviceArchArmParallel, @@ -232,12 +237,13 @@ TEST_F(OpenMPContextTest, ApplicabilityAllTraits) { VariantMatchInfo LLVMHostUserCondTrueParallel; LLVMHostUserCondTrueParallel.addTrait( - TraitProperty::implementation_vendor_llvm); - LLVMHostUserCondTrueParallel.addTrait(TraitProperty::device_kind_host); - LLVMHostUserCondTrueParallel.addTrait(TraitProperty::device_kind_any); - LLVMHostUserCondTrueParallel.addTrait(TraitProperty::user_condition_true); + TraitProperty::implementation_vendor_llvm, ""); + LLVMHostUserCondTrueParallel.addTrait(TraitProperty::device_kind_host, ""); + LLVMHostUserCondTrueParallel.addTrait(TraitProperty::device_kind_any, ""); + LLVMHostUserCondTrueParallel.addTrait(TraitProperty::user_condition_true, + ""); LLVMHostUserCondTrueParallel.addTrait( - TraitProperty::construct_parallel_parallel); + TraitProperty::construct_parallel_parallel, ""); EXPECT_TRUE(isVariantApplicableInContext(LLVMHostUserCondTrueParallel, HostLinuxParallelParallel)); EXPECT_FALSE(isVariantApplicableInContext(LLVMHostUserCondTrueParallel, @@ -250,7 +256,7 @@ TEST_F(OpenMPContextTest, ApplicabilityAllTraits) { VariantMatchInfo LLVMHostUserCondTrueParallelParallel = LLVMHostUserCondTrueParallel; LLVMHostUserCondTrueParallelParallel.addTrait( - TraitProperty::construct_parallel_parallel); + TraitProperty::construct_parallel_parallel, ""); EXPECT_TRUE(isVariantApplicableInContext( LLVMHostUserCondTrueParallelParallel, HostLinuxParallelParallel)); EXPECT_FALSE(isVariantApplicableInContext( @@ -263,7 +269,7 @@ TEST_F(OpenMPContextTest, ApplicabilityAllTraits) { VariantMatchInfo LLVMHostUserCondTrueParallelParallelParallel = LLVMHostUserCondTrueParallelParallel; LLVMHostUserCondTrueParallelParallelParallel.addTrait( - TraitProperty::construct_parallel_parallel); + TraitProperty::construct_parallel_parallel, ""); EXPECT_FALSE(isVariantApplicableInContext( LLVMHostUserCondTrueParallelParallelParallel, HostLinuxParallelParallel)); @@ -277,9 +283,9 @@ TEST_F(OpenMPContextTest, ApplicabilityAllTraits) { DeviceNVPTXTargetTeamsParallel)); VariantMatchInfo GPUTargetTeams; - GPUTargetTeams.addTrait(TraitProperty::construct_target_target); - GPUTargetTeams.addTrait(TraitProperty::construct_teams_teams); - GPUTargetTeams.addTrait(TraitProperty::device_kind_gpu); + GPUTargetTeams.addTrait(TraitProperty::construct_target_target, ""); + GPUTargetTeams.addTrait(TraitProperty::construct_teams_teams, ""); + GPUTargetTeams.addTrait(TraitProperty::device_kind_gpu, ""); EXPECT_FALSE(isVariantApplicableInContext(GPUTargetTeams, HostLinuxParallelParallel)); EXPECT_FALSE(isVariantApplicableInContext(GPUTargetTeams, @@ -289,9 +295,9 @@ TEST_F(OpenMPContextTest, ApplicabilityAllTraits) { DeviceNVPTXTargetTeamsParallel)); VariantMatchInfo GPUTargetParallel; - GPUTargetParallel.addTrait(TraitProperty::construct_target_target); - GPUTargetParallel.addTrait(TraitProperty::construct_parallel_parallel); - GPUTargetParallel.addTrait(TraitProperty::device_kind_gpu); + GPUTargetParallel.addTrait(TraitProperty::construct_target_target, ""); + GPUTargetParallel.addTrait(TraitProperty::construct_parallel_parallel, ""); + GPUTargetParallel.addTrait(TraitProperty::device_kind_gpu, ""); EXPECT_FALSE(isVariantApplicableInContext(GPUTargetParallel, HostLinuxParallelParallel)); EXPECT_FALSE(isVariantApplicableInContext(GPUTargetParallel,