This is an archive of the discontinued LLVM Phabricator instance.

[NFC][LSR] Cleanup Cost API
ClosedPublic

Authored by samparker on Feb 19 2019, 8:39 AM.

Details

Summary

Create members for Loop, ScalarEvolution, DominatorTree, TargetTransformInfo and Formula.

Diff Detail

Repository
rL LLVM

Event Timeline

samparker created this revision.Feb 19 2019, 8:39 AM
gilr added inline comments.Feb 21 2019, 1:09 AM
lib/Transforms/Scalar/LoopStrengthReduce.cpp
1018 ↗(On Diff #187381)

TTI et al. persist throughout Cost's life so it makes sense to initialize the object with them and keep them as members. IINM F is limited to rateFormula() - seems more natural to keep F as its argument (and risky to keep it beyond rateFormula()'s scope).

samparker updated this revision to Diff 187742.Feb 21 2019, 2:00 AM

Thanks, I've now removed Formula from the class.

gilr added inline comments.Feb 21 2019, 4:30 AM
lib/Transforms/Scalar/LoopStrengthReduce.cpp
1020 ↗(On Diff #187742)

If Cost objects are always created with all these initialized please explicitly "= delete" the default c'tor.

samparker updated this revision to Diff 190576.Mar 14 2019, 2:06 AM

Sorry, I went on holiday and forgot about this... marked the constructor as deleted.

gilr added a comment.Mar 14 2019, 3:40 AM

No problem :)
LGTM

gilr accepted this revision.Mar 14 2019, 3:40 AM
This revision is now accepted and ready to land.Mar 14 2019, 3:40 AM
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptMar 14 2019, 4:03 AM