Page MenuHomePhabricator

[Attributor] Split the Attributor::run() into multiple functions.
ClosedPublic

Authored by kuter on Jun 2 2020, 11:21 AM.

Details

Summary

This patch splits the Attributor::run() function into multiple functions.

Simple Logic changes to make this possible:

  1. Moved iteration count verification earlier.
  2. NumFinalAAs get set a little bit later.

Diff Detail

Event Timeline

kuter created this revision.Jun 2 2020, 11:21 AM
Herald added a reviewer: sstefan1. · View Herald Transcript
Herald added a reviewer: uenoku. · View Herald Transcript
Herald added a project: Restricted Project. · View Herald Transcript

Nice cleanup. I basically have only comments on the documentation.

llvm/include/llvm/Transforms/IPO/Attributor.h
1220

I find the name of this not convincing. Maybe, runTillFixpoint or something along those lines?
The documentation should also be a bit more descriptive, e.g., in case of a time out of what? At the same time we don't need to talk about the dep graph but could just say, "on attributes that haven't reached a known valid fixpoint yet".

kuter updated this revision to Diff 268617.Jun 4 2020, 4:15 PM
  1. rename ::runScheduler to ::runTillFixpoint
  2. improve comments for ::runTillFixpoint
jdoerfert accepted this revision.Jun 4 2020, 6:48 PM

LGTM, one nit.

llvm/include/llvm/Transforms/IPO/Attributor.h
1230

Nit: Newline.

This revision is now accepted and ready to land.Jun 4 2020, 6:48 PM
kuter updated this revision to Diff 268981.Jun 5 2020, 6:18 PM
kuter marked an inline comment as done.

add newline after function declaration

This revision was automatically updated to reflect the committed changes.

@sstefan1 I saw you committed and reverted this. What happened?

@sstefan1 I saw you committed and reverted this. What happened?

Forgot to change the author...sorry for the mess.

@sstefan1 I saw you committed and reverted this. What happened?

Forgot to change the author...sorry for the mess.

Happens. No worries and thanks for helping out!