[llvm-dev] Improving LLVM string attributes

[Serge Guelton via llvm-dev]

That's all valid points. I submitted a simplified and hopefully easier to
review version in https://reviews.llvm.org/D114394

On Wed, Nov 17, 2021 at 10:11 PM David Blaikie <dblaikie at gmail.com> wrote:

> Might benefit from a separate copy of the review with only the API
> implementation changes & not all the cleanup/fixes (maybe like one file
> of/few examples of fixes) - so it's easier to find the implementation/core
> of the change.
>
> I'm modestly in favor - though wonder if much/most of the benefit might be
> gained by moving some "standard" free form attributes into
> named/non-free-form attributes in the first place?
>
> On Wed, Nov 17, 2021 at 8:10 AM Serge Guelton via llvm-dev <
> llvm-dev at lists.llvm.org> wrote:
>
>> Hi folks,
>>
>> LLVM handles freeform attributes through string attributes ("key"="value"
>> in the
>> textual IR). Although it's freeform, some of these attributes are
>> relatively
>> standard and widely used (e.g. "target-cpu" or "target-features").
>>
>> However, there's no central place where these attributes are either
>> mentionned,
>> normalized etc.
>>
>> In an effort to improve this siutation, I've submitted
>>
>>     https://reviews.llvm.org/D114082
>>
>> Which is still a WIP by some aspects, but it compiles and passes
>> validation on
>> X86. The key aspects of it are that:
>>
>> 1. common attributes get a private symbol to represent them. For instance
>>
>>         OutlinedFn->addFnAttr("omp_target_num_teams",
>> std::to_string(DefaultValTeams));
>>
>>    becomes
>>
>>         OutlinedFn->addFnAttr(llvm::OmpTargetNumTeamsAttr,
>> std::to_string(DefaultValTeams));
>>
>> 2. free forms attributes are still buildable through a specific factory
>>
>>         Fn->addFnAttr(Out.str());
>>
>>    becomes
>>
>>         Fn->addFnAttr(llvm::AttributeKey::Create(Out.str()));
>>
>> I see several advantage in that approach:
>>
>> - it avoids typo in common attributes
>>
>> - it associates a type to Attributes key (currently named AttributeKey),
>> which
>>   unlocks more potential optimization
>>
>> - (NIY) one could document the meaning of these attributes alongside their
>>   definition, instead of existing mess ;-)
>>
>> As a specific optimization, I already made most AttributeKey be constexpr
>> and have
>> them store their hash, which makes this implementation faster than
>> existing
>> implementation: when checking if an attribute is available, no hash
>> recomputation is involved.
>>
>> /extra note 0/ no change to the IR, that's only a higher level
>> abstraction over
>> existing representation. And free-form attributes are still supported,
>> without
>> much overhead.
>>
>> /extra note 1/ as a side effect, I get a ~1% compile time speedup when
>> compiling large
>> file in -O0 with the patch above.
>>
>> So yeah, how do you feel about that?
>>
>> _______________________________________________
>> LLVM Developers mailing list
>> llvm-dev at lists.llvm.org
>> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20211122/92ff54df/attachment.html>