Page MenuHomePhabricator

[Attributor] AANoRecurse check all call sites for `norecurse`
ClosedPublic

Authored by jdoerfert on Dec 30 2019, 2:37 PM.

Details

Summary

If all call sites are in norecurse functions we can derive norecurse
as the ReversePostOrderFunctionAttrsPass does. This should make
ReversePostOrderFunctionAttrsLegacyPass obsolete once the Attributor is
enabled.

Diff Detail

Event Timeline

jdoerfert created this revision.Dec 30 2019, 2:37 PM
Herald added a project: Restricted Project. · View Herald TranscriptDec 30 2019, 2:37 PM
uenoku added inline comments.Dec 30 2019, 11:44 PM
llvm/lib/Transforms/IPO/Attributor.cpp
1935

Don't we need to return?

jdoerfert marked an inline comment as done.Dec 30 2019, 11:49 PM
jdoerfert added inline comments.
llvm/lib/Transforms/IPO/Attributor.cpp
1935

We return in the next line either way. If the checkForAllCallSites returned true we can *at least* assume noreturn. Or did I misunderstand your question?

uenoku added inline comments.Dec 30 2019, 11:56 PM
llvm/lib/Transforms/IPO/Attributor.cpp
1935

I mean we need to return ChangeStatus::CHANGED here, isn't it?

jdoerfert marked an inline comment as done.Dec 31 2019, 12:02 AM
jdoerfert added inline comments.
llvm/lib/Transforms/IPO/Attributor.cpp
1935

The assumed state didn't change so I returned UNCHANGED. This is what indicateOptimisticFixpoint returns as well. So far I used the changestatus (only) for the assumed not the known state. I'm not convinced it is useful to indicate "changes" in the known state alone, though I'm open to arguments here.

uenoku added inline comments.Dec 31 2019, 12:56 AM
llvm/lib/Transforms/IPO/Attributor.cpp
1935

Oh, I misunderstood that indicateOptimistic returns Changed. Thanks. I agree we shouldn't use the changestatus for known state.

uenoku accepted this revision.Jan 1 2020, 12:29 PM

LGTM

This revision is now accepted and ready to land.Jan 1 2020, 12:29 PM
This revision was automatically updated to reflect the committed changes.