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

Mon Jun 24 17:18:59 PDT 2019

sstefan1 added a comment.

In D63379#1555618 <https://reviews.llvm.org/D63379#1555618>, @jdoerfert wrote:

> 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.

Ran clang-format, but it seems it didn't have any effect.

Numbers are the same when both are enabled.

Maybe I should commit this before nosync, since it looks like it may take few more days?

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D63379