[PATCH] D63379: [Attributor] Deducing existing nounwind attribute.

[Johannes Doerfert via Phabricator via llvm-commits]

jdoerfert accepted this revision.
jdoerfert added a comment.
This revision is now accepted and ready to land.

In D63379#1550752 <https://reviews.llvm.org/D63379#1550752>, @sstefan1 wrote:

> In D63379#1548872 <https://reviews.llvm.org/D63379#1548872>, @sstefan1 wrote:
>
> > > Is this sufficient to replace the old nounwind deduction? Can you disable the current one to see if there is something we cannot deduce with this one?
> >
> > I already tried it on some of the other tests and it worked fine. I am going to that and test it some more, and update tomorrow morning.
>
>
> Sorry for the late update. I ran all tests with `-disable-nounwind-inference -attributor -attributor-disable=false` and they pass. Is that enough?




In D63379#1554749 <https://reviews.llvm.org/D63379#1554749>, @sstefan1 wrote:

> I have now run the test-suite with -disable-nounwind-inference with attributor and functionattrs with nounwind, with attributor disabled. These are the results:
>
> attributor.NumFnNoUnwind: 935 (-disable-nounwind-inference)
>  functionattrs.NumNoUnwind: 262 (-attributor-disable=true)


That looks good. Do you happen to know if and how many attributes are deduced by the funcattrs pass if we enable both?

Please run clang format on the Attributor.cpp and revert the non-functional changes, e.g., in the comments.



================
Comment at: llvm/include/llvm/Transforms/IPO/Attributor.h:654
+    static constexpr Attribute::AttrKind ID =
+        Attribute::AttrKind(Attribute::NoUnwind);
+
----------------
sstefan1 wrote:
> jdoerfert wrote:
> > Isn't `ID = Attribute::NoUnwind` sufficient?
> Totally, my bad.
No worries, update it though ;)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63379/new/

https://reviews.llvm.org/D63379