[PATCH] D112175: [NFC] Add llvm::StaticVector ADT
David Blaikie via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Thu Oct 21 14:57:24 PDT 2021
dblaikie added a comment.
In D112175#3079442 <https://reviews.llvm.org/D112175#3079442>, @jplayer-nv wrote:
> I don't see too many simple substitutions in target independent code, although this class has a SmallVector which never exceeds the inline storage (drop-in type substitution + with YAML binding works):
>
> include\llvm\DebugInfo\CodeView\TypeRecord.h:
>
> // LF_BUILDINFO
> class BuildInfoRecord : public TypeRecord {
> public:
> BuildInfoRecord() = default;
> explicit BuildInfoRecord(TypeRecordKind Kind) : TypeRecord(Kind) {}
> BuildInfoRecord(ArrayRef<TypeIndex> ArgIndices)
> : TypeRecord(TypeRecordKind::BuildInfo),
> ArgIndices(ArgIndices.begin(), ArgIndices.end()) {}
>
> ArrayRef<TypeIndex> getArgs() const { return ArgIndices; }
>
> /// Indices of known build info arguments.
> enum BuildInfoArg {
> CurrentDirectory, ///< Absolute CWD path
> BuildTool, ///< Absolute compiler path
> SourceFile, ///< Path to main source file, relative or absolute
> TypeServerPDB, ///< Absolute path of type server PDB (/Fd)
> CommandLine, ///< Full canonical command line (maybe -cc1)
> MaxArgs
> };
>
> StaticVector<TypeIndex, MaxArgs> ArgIndices;
> };
>
> Is the above substitution sufficient to move forward with a review? Or would you like to see more wide-spread usage?
Not sure - what's your take, @craig.topper / @tschuett
It's still quite a bit of code, not sure what the tipping point in adding all that compared to what benefits would justify it? (whether there's a "this much code size" or "this much performance", etc, that would justify this)
> I suspect this container is going to be more attractive in target-specific code (as is my interest). Is it appropriate for me to modify target-specific code to roll out a container like this?
>
>> could fallible operations ala push_back return an llvm::error?
>
> I implemented a subset of `checked_XXX` versions of fallible member functions this morning. If this is an attractive feature, I can work on a complete set of them and add them to the patch.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D112175/new/
https://reviews.llvm.org/D112175
More information about the llvm-commits
mailing list