-
Notifications
You must be signed in to change notification settings - Fork 15.2k
[LLVM] Change Intrinsic::ID to encode target and intrinsic index #113576
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,53 @@ | ||
| //===- llvm/Support/IntrinsicID.h - Intrinsic ID encoding -------*- C++ -*-===// | ||
| // | ||
| // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. | ||
| // See https://llvm.org/LICENSE.txt for license information. | ||
| // SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception | ||
| // | ||
| //===----------------------------------------------------------------------===// | ||
| // | ||
| // This file contains functions to support intrinsic ID encoding. The | ||
| // Intrinsic::ID enum value is constructed using a target prefix index in bits | ||
| // 23-16 (8-bit) and an intrinsic index (index within the list of intrinsics for | ||
| // tha target) in lower 16 bits. To support Intrinsic::ID 0 being not used, the | ||
| // intrinsic index is encoded as Index + 1 for all targets. | ||
| // | ||
| // This file defines functions that encapsulate this encoding. | ||
| // | ||
| //===----------------------------------------------------------------------===// | ||
|
|
||
| #ifndef LLVM_SUPPORT_INTRINSIC_ID_H | ||
| #define LLVM_SUPPORT_INTRINSIC_ID_H | ||
|
|
||
| #include <limits> | ||
| #include <optional> | ||
| #include <utility> | ||
|
|
||
| namespace llvm::Intrinsic { | ||
| typedef unsigned ID; | ||
|
|
||
| inline ID encodeIntrinsicID(unsigned TargetIndex, unsigned IntrinsicIndex) { | ||
| assert(IntrinsicIndex < std::numeric_limits<uint16_t>::max()); | ||
| assert(TargetIndex <= std::numeric_limits<uint8_t>::max()); | ||
| return (TargetIndex << 16) | (IntrinsicIndex + 1); | ||
| } | ||
|
|
||
| inline std::pair<unsigned, unsigned> decodeIntrinsicID(ID id) { | ||
| unsigned IntrinsicIndex = id & 0xFFFF; | ||
| unsigned TargetIndex = id >> 16; | ||
| assert(IntrinsicIndex != 0); | ||
| return {TargetIndex, IntrinsicIndex - 1}; | ||
| } | ||
|
|
||
| inline std::optional<std::pair<unsigned, unsigned>> | ||
| decodeIntrinsicIDNoFail(ID id) { | ||
| unsigned IntrinsicIndex = id & 0xFFFF; | ||
| unsigned TargetIndex = id >> 16; | ||
| if (IntrinsicIndex == 0) | ||
| return std::nullopt; | ||
| return std::make_pair(TargetIndex, IntrinsicIndex - 1); | ||
| } | ||
|
|
||
| } // end namespace llvm::Intrinsic | ||
|
|
||
| #endif // LLVM_SUPPORT_INTRINSIC_ID_H |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -30,10 +30,55 @@ | |
| #include "llvm/IR/IntrinsicsXCore.h" | ||
| #include "llvm/IR/Module.h" | ||
| #include "llvm/IR/Type.h" | ||
| #include "llvm/Support/IntrinsicID.h" | ||
|
|
||
| using namespace llvm; | ||
| using namespace Intrinsic; | ||
|
|
||
| /// Table of per-target intrinsic name tables. | ||
| #define GET_INTRINSIC_TARGET_DATA | ||
| #include "llvm/IR/IntrinsicImpl.inc" | ||
| #undef GET_INTRINSIC_TARGET_DATA | ||
| size_t constexpr NumTargets = sizeof(TargetInfos) / sizeof(TargetInfos[0]); | ||
|
|
||
| // Returns true if the given intrinsic ID is valid, that is, its value is one | ||
| // of the enum values defined for this intrinsic (including not_intrinsic). | ||
| bool Intrinsic::IsIntrinsicIDValid(ID ID) { | ||
| if (ID == not_intrinsic) | ||
| return true; | ||
| auto Decoded = decodeIntrinsicIDNoFail(ID); | ||
| if (!Decoded) | ||
| return false; | ||
|
Comment on lines
+50
to
+51
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. no fail but then it needs a null check?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, so in some cases we really just want to return 0 or false if decoding failed, and in other cases we want to assert of fail in different ways, so the nofail variant just reports failure without an assert. Maybe we just have one version that return std::optional and if it has failed, dereferencing the std::optional will fail in cases where |
||
| unsigned TargetIdx = Decoded->first; | ||
| unsigned IntrinsicIdx = Decoded->second; | ||
| return TargetIdx < NumTargets && IntrinsicIdx < TargetInfos[TargetIdx].Count; | ||
| } | ||
|
|
||
| // Returns linear index of ID if its valid, else returns 0. | ||
| unsigned getLinearIndex(ID ID) { | ||
| auto Decoded = decodeIntrinsicIDNoFail(ID); | ||
| if (!Decoded) | ||
| return 0; | ||
| unsigned TargetIdx = Decoded->first; | ||
| unsigned IntrinsicIdx = Decoded->second; | ||
| return TargetInfos[TargetIdx].FirstLinearIndex + IntrinsicIdx; | ||
| } | ||
|
|
||
| ID Intrinsic::GetNextValidIntrinsicID(ID ID) { | ||
| if (ID == not_intrinsic) | ||
| return encodeIntrinsicID(0, 0); | ||
| if (ID == last_valid_intrinsic_id) | ||
| return end_id; | ||
| if (ID == end_id) | ||
| llvm_unreachable("Cannot find the next valid intrisnic"); | ||
| auto [TargetIndex, IntrinsicIndex] = decodeIntrinsicID(ID); | ||
| if (IntrinsicIndex + 1 < TargetInfos[TargetIndex].Count) | ||
| return encodeIntrinsicID(TargetIndex, IntrinsicIndex + 1); | ||
| if (TargetIndex + 1 < NumTargets) | ||
| return encodeIntrinsicID(TargetIndex + 1, 0); | ||
| llvm_unreachable("Cannot find the next valid intrisnic"); | ||
| } | ||
|
|
||
| /// Table of string intrinsic names indexed by enum value. | ||
| static constexpr const char *const IntrinsicNameTable[] = { | ||
| "not_intrinsic", | ||
|
|
@@ -43,12 +88,12 @@ static constexpr const char *const IntrinsicNameTable[] = { | |
| }; | ||
|
|
||
| StringRef Intrinsic::getBaseName(ID id) { | ||
| assert(id < num_intrinsics && "Invalid intrinsic ID!"); | ||
| return IntrinsicNameTable[id]; | ||
| assert(IsIntrinsicIDValid(id) && "Invalid intrinsic ID!"); | ||
| return ArrayRef(IntrinsicNameTable)[getLinearIndex(id)]; | ||
| } | ||
|
|
||
| StringRef Intrinsic::getName(ID id) { | ||
| assert(id < num_intrinsics && "Invalid intrinsic ID!"); | ||
| assert(IsIntrinsicIDValid(id) && "Invalid intrinsic ID!"); | ||
| assert(!isOverloaded(id) && | ||
| "This version of getName does not support overloading"); | ||
| return getBaseName(id); | ||
|
|
@@ -155,8 +200,7 @@ static std::string getMangledTypeStr(Type *Ty, bool &HasUnnamedType) { | |
| static std::string getIntrinsicNameImpl(ID Id, ArrayRef<Type *> Tys, Module *M, | ||
| FunctionType *FT, | ||
| bool EarlyModuleCheck) { | ||
|
|
||
| assert(Id < num_intrinsics && "Invalid intrinsic ID!"); | ||
| assert(IsIntrinsicIDValid(Id) && "Invalid intrinsic ID!"); | ||
| assert((Tys.empty() || isOverloaded(Id)) && | ||
| "This version of getName is for overloaded intrinsics only"); | ||
| (void)EarlyModuleCheck; | ||
|
|
@@ -445,11 +489,15 @@ static void DecodeIITType(unsigned &NextElt, ArrayRef<unsigned char> Infos, | |
| #undef GET_INTRINSIC_GENERATOR_GLOBAL | ||
|
|
||
| void Intrinsic::getIntrinsicInfoTableEntries( | ||
| ID id, SmallVectorImpl<IITDescriptor> &T) { | ||
| ID IntrinsicID, SmallVectorImpl<IITDescriptor> &T) { | ||
| static_assert(sizeof(IIT_Table[0]) == 2, | ||
| "Expect 16-bit entries in IIT_Table"); | ||
| assert(IsIntrinsicIDValid(IntrinsicID)); | ||
| unsigned Idx = getLinearIndex(IntrinsicID); | ||
| if (Idx == 0) | ||
| return; | ||
| // Check to see if the intrinsic's type was expressible by the table. | ||
| uint16_t TableVal = IIT_Table[id - 1]; | ||
| uint16_t TableVal = IIT_Table[Idx - 1]; | ||
|
|
||
| // Decode the TableVal into an array of IITValues. | ||
| SmallVector<unsigned char> IITValues; | ||
|
|
@@ -602,18 +650,21 @@ FunctionType *Intrinsic::getType(LLVMContext &Context, ID id, | |
| return FunctionType::get(ResultTy, ArgTys, false); | ||
| } | ||
|
|
||
| bool Intrinsic::isOverloaded(ID id) { | ||
| // Check if an intrinsic is overloaded or not using its linear index. | ||
| static bool isOverloadedUsingLinearIndex(unsigned Idx) { | ||
| #define GET_INTRINSIC_OVERLOAD_TABLE | ||
| #include "llvm/IR/IntrinsicImpl.inc" | ||
| #undef GET_INTRINSIC_OVERLOAD_TABLE | ||
| } | ||
|
|
||
| /// Table of per-target intrinsic name tables. | ||
| #define GET_INTRINSIC_TARGET_DATA | ||
| #include "llvm/IR/IntrinsicImpl.inc" | ||
| #undef GET_INTRINSIC_TARGET_DATA | ||
| bool Intrinsic::isOverloaded(ID id) { | ||
| assert(IsIntrinsicIDValid(id)); | ||
| return isOverloadedUsingLinearIndex(getLinearIndex(id)); | ||
| } | ||
|
|
||
| bool Intrinsic::isTargetIntrinsic(ID IID) { return IID > TargetInfos[0].Count; } | ||
| bool Intrinsic::isTargetIntrinsic(ID IID) { | ||
| return IID != not_intrinsic && decodeIntrinsicID(IID).first != 0; | ||
| } | ||
|
|
||
| int Intrinsic::lookupLLVMIntrinsicByName(ArrayRef<const char *> NameTable, | ||
| StringRef Name, StringRef Target) { | ||
|
|
@@ -673,7 +724,29 @@ findTargetSubtable(StringRef Name) { | |
| // We've either found the target or just fall back to the generic set, which | ||
| // is always first. | ||
| const auto &TI = It != Targets.end() && It->Name == Target ? *It : Targets[0]; | ||
| return {ArrayRef(&IntrinsicNameTable[1] + TI.Offset, TI.Count), TI.Name}; | ||
| unsigned LinearIndex = TI.FirstLinearIndex; | ||
| return {ArrayRef(IntrinsicNameTable + LinearIndex, TI.Count), TI.Name}; | ||
| } | ||
|
|
||
| static Intrinsic::ID getIntrinsicIDFromIndex(unsigned Idx) { | ||
| if (Idx == 0) | ||
| return Intrinsic::not_intrinsic; | ||
|
|
||
| auto It = | ||
| partition_point(TargetInfos, [Idx](const IntrinsicTargetInfo &Info) { | ||
| return Info.FirstLinearIndex + Info.Count < Idx; | ||
| }); | ||
| // Idx, if present, will be in the entry at It or It + 1. | ||
| if (It == std::end(TargetInfos)) | ||
| return Intrinsic::not_intrinsic; | ||
| unsigned TargetIndex = std::distance(std::begin(TargetInfos), It); | ||
| if (It->FirstLinearIndex <= Idx && Idx < It->FirstLinearIndex + It->Count) | ||
| return encodeIntrinsicID(TargetIndex, Idx - It->FirstLinearIndex); | ||
| ++It; | ||
| ++TargetIndex; | ||
| if (It->FirstLinearIndex <= Idx && Idx < It->FirstLinearIndex + It->Count) | ||
| return encodeIntrinsicID(TargetIndex, Idx - It->FirstLinearIndex); | ||
| return Intrinsic::not_intrinsic; | ||
| } | ||
|
|
||
| /// This does the actual lookup of an intrinsic ID which matches the given | ||
|
|
@@ -683,18 +756,18 @@ ID Intrinsic::lookupIntrinsicID(StringRef Name) { | |
| int Idx = lookupLLVMIntrinsicByName(NameTable, Name, Target); | ||
| if (Idx == -1) | ||
| return not_intrinsic; | ||
|
|
||
| // Intrinsic IDs correspond to the location in IntrinsicNameTable, but we have | ||
| // an index into a sub-table. | ||
| const auto MatchSize = strlen(NameTable[Idx]); | ||
| // Adjust the index from sub-table index to index into the global table. | ||
| int Adjust = NameTable.data() - IntrinsicNameTable; | ||
| ID Id = static_cast<ID>(Idx + Adjust); | ||
| Idx += Adjust; | ||
|
|
||
| // If the intrinsic is not overloaded, require an exact match. If it is | ||
| // overloaded, require either exact or prefix match. | ||
| const auto MatchSize = strlen(NameTable[Idx]); | ||
| assert(Name.size() >= MatchSize && "Expected either exact or prefix match"); | ||
| bool IsExactMatch = Name.size() == MatchSize; | ||
| return IsExactMatch || isOverloaded(Id) ? Id : not_intrinsic; | ||
| return IsExactMatch || isOverloadedUsingLinearIndex(Idx) | ||
| ? getIntrinsicIDFromIndex(Idx) | ||
| : not_intrinsic; | ||
| } | ||
|
|
||
| /// This defines the "Intrinsic::getAttributes(ID id)" method. | ||
|
|
@@ -1028,8 +1101,9 @@ bool Intrinsic::matchIntrinsicVarArg(bool isVarArg, | |
|
|
||
| bool Intrinsic::getIntrinsicSignature(ID ID, FunctionType *FT, | ||
| SmallVectorImpl<Type *> &ArgTys) { | ||
| if (!ID) | ||
| if (ID == Intrinsic::not_intrinsic) | ||
| return false; | ||
| assert(IsIntrinsicIDValid(ID)); | ||
|
|
||
| SmallVector<IITDescriptor, 8> Table; | ||
| getIntrinsicInfoTableEntries(ID, Table); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you omit the extra 2 bytes by assuming the current target's index?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wasn't sure. What if a GISel patterns refers to a generic intrinsic or some other targets intrinsics? For instance, AMDGPU likely refers to mix of r600 and amdgcn intrinsics?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
AMDGPU shouldn't mix r600 and amdgcn intrinsics, but there might be a handful remaining that do that mixing. @AlexVlx also wanted to use amdgcn intrinsics in spirv
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We do use them for AMDGCN flavoured SPIR-V, but I admit I was not aware and have not really internalised what this change would imply in that context. I’ll try to parse it now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks. I don't think it will directly affect anything at the moment. A follow on change would need to be aware of which targets use which intrinsic prefixes to make sure we do disable the right ones for that target,
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My concern with making this 2 byte is: (1) won't we have files with patterns that use mix of target independent and target dependent builtins. In such cases, just encoding the 16-bit target index is not sufficient, and (2) premature optimizations? If its really important, I'd prefer doing this as an independent change.