[llvm] Add the 'initialized' attribute langref and support (PR #84803)
Nikita Popov via llvm-commits
llvm-commits at lists.llvm.org
Fri Mar 15 14:30:18 PDT 2024
nikic wrote:
> > As a heads up, the inference implementation for this (or at least the available prototype) does not look viable from a compile-time perspective: https://llvm-compile-time-tracker.com/compare.php?from=69b09b43b0a2057918078edb401adab888d1014b&to=0bd68ae2d56783377acc0aa5d7958b47411b8342&stat=instructions:u
>
> Thanks for this heads up! This PR only adds the attribute support. Will further tune the inferring performance and post the updated compile time tracker in [the inference PR](https://github.com/llvm/llvm-project/pull/84869).
The main thing I would be concerned about at this point is whether the entire approach is viable or not -- I'd rather not add the attribute (plus a whole new attribute *kind*) if it later turns out that this is too expensive.
-----
Anyway, a couple of high level comments:
* I'm not sure this is the ideal attribute name. `initialized` to me sounds like this attribute promises that the memory is initialized on entry to the function, not that the function will initialize it. Maybe `will_initialize` is a better name? Or `initializes`?
* How important is it to represent multiple ranges? We have *just* added support for ConstantRange attributes, so a single range can be represented out of the box. This also avoids the tricky question of how to deal with non-zero start offsets in DSE, which we don't really have AA support for right now. My intuition would be that you usually get whole object initializations -- but maybe this is not the case due to padding holes?
* Failing that, I feel like we should still piggy-back more on the new ConstantRange attribute kind. We could convert that into a list of ConstantRange stored as TrailingObjects and make the current single-range case a special case of the general multiple-range case.
https://github.com/llvm/llvm-project/pull/84803
More information about the llvm-commits
mailing list