[PATCH] D66126: [Attributor] Towards a more structured deduction pattern
Johannes Doerfert via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Tue Aug 13 09:14:05 PDT 2019
jdoerfert marked 2 inline comments as done.
jdoerfert added a comment.
In D66126#1626624 <https://reviews.llvm.org/D66126#1626624>, @uenoku wrote:
> Basically, I like the idea.
>
> You have ported `nonnull` and `align` using this pattern. I want to make other attributes(willreturn, dereferenceable, nofree) deduction use this pattern. Should I wait for this patch being developed more?
We do not need to port all attributes at once even if we move to the "on-demand" creation of attributes (I'll put the patch up today).
I wanted to show how it will look like, that it will be "cleaner" and that we get better results afterwards, I mean after on-demand creation.
================
Comment at: llvm/lib/Transforms/IPO/Attributor.cpp:466
+ auto Assumed = S.getAssumed();
+ S ^= R;
+ return Assumed == S.getAssumed() ? ChangeStatus::UNCHANGED
----------------
uenoku wrote:
> Where `^=` and `&=` are defined? Are these operators already defined?
Sry, forgot that one. In D66146.
================
Comment at: llvm/lib/Transforms/IPO/Attributor.cpp:2127
SmallVectorImpl<Attribute> &Attrs) const override {
- Attrs.emplace_back(Attribute::getWithAlignment(Ctx, getAssumedAlign()));
+ if (getAssumedAlign() > 1)
+ Attrs.emplace_back(Attribute::getWithAlignment(Ctx, getAssumedAlign()));
----------------
uenoku wrote:
> Why is this condition necessary?
It is not. I will remove it.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D66126/new/
https://reviews.llvm.org/D66126
More information about the llvm-commits
mailing list