[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