Page MenuHomePhabricator

[LoopInfo][Refactor] Make SetLoopAlreadyUnrolled a member function of the Loop Pass, NFC.
ClosedPublic

Authored by etherzhhb on Oct 14 2017, 8:20 PM.

Diff Detail

Repository
rL LLVM

Event Timeline

etherzhhb created this revision.Oct 14 2017, 8:20 PM
etherzhhb updated this revision to Diff 119053.Oct 14 2017, 8:22 PM

Update comments

sanjoy accepted this revision.Oct 14 2017, 8:27 PM

LGTM

I think the Loop class has grown quite a bit, and we should pull out some of the utility functions into a separate LoopUtils namespace, but this patch does not need to be blocked on that.

This revision is now accepted and ready to land.Oct 14 2017, 8:27 PM

LGTM

I think the Loop class has grown quite a bit, and we should pull out some of the utility functions into a separate LoopUtils namespace, but this patch does not need to be blocked on that.

Just out of curiosity, what are the principle to tell whether a function should be a member function of the Loop class or a utilities?

LGTM

I think the Loop class has grown quite a bit, and we should pull out some of the utility functions into a separate LoopUtils namespace, but this patch does not need to be blocked on that.

Just out of curiosity, what are the principle to tell whether a function should be a member function of the Loop class or a utilities?

This is fairly subjective, but I tend to draw the line as: actions that can be performed on an object only using its public interface should be free functions. IOW, don't give a function access to private/protected members of a class if you don't have to.

This revision was automatically updated to reflect the committed changes.