llvm dev - Nov 2021 - Improving LLVM string attributes

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?

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/20211117/b09914e1/attachment.html>