[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