David Blaikie via llvm-dev
2021-Nov-17 21:11 UTC
[llvm-dev] Improving LLVM string attributes
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>
Serge Guelton via llvm-dev
2021-Nov-22 21:36 UTC
[llvm-dev] Improving LLVM string attributes
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>