Page MenuHomePhabricator

[NFC][LoopUtils] Added regression test for getEstimatedTripCount.
AbandonedPublic

Authored by ebrevnov on Jan 16 2020, 3:52 AM.

Details

Reviewers
Ayal
Summary

As agreed in D71990 extracted regression test checnges to a separate review.

Event Timeline

ebrevnov created this revision.Jan 16 2020, 3:52 AM
Herald added a project: Restricted Project. · View Herald TranscriptJan 16 2020, 3:52 AM

@Ayal , would be nice if you find time to look at this patch since next two (accepted) patched depends on this one.

fhahn added a subscriber: fhahn.Jan 17 2020, 1:59 AM

Could you describe why the changes to run the tests via the pass managers is required? Wouldn't it be sufficient to use initialize the required analysis for the function and use that with getLoopEstimatedTripCount?

Could you describe why the changes to run the tests via the pass managers is required? Wouldn't it be sufficient to use initialize the required analysis for the function and use that with getLoopEstimatedTripCount?

They are not required. The thing is LoopUtilsTest.cpp didn't exist when I first developed my test. Later during one of the rebases I discovered that somebody else introduced a test with the same name. It didn't look right to me to do loop traversal in one test in two different ways. My personal preference was to use pass manager and I streamlined the code this way. Do you see any problems with that?

Optionally I can move my part to a separate file so they will be completely independent,.

fhahn added a comment.Jan 21 2020, 4:10 PM

Could you describe why the changes to run the tests via the pass managers is required? Wouldn't it be sufficient to use initialize the required analysis for the function and use that with getLoopEstimatedTripCount?

They are not required. The thing is LoopUtilsTest.cpp didn't exist when I first developed my test. Later during one of the rebases I discovered that somebody else introduced a test with the same name. It didn't look right to me to do loop traversal in one test in two different ways. My personal preference was to use pass manager and I streamlined the code this way. Do you see any problems with that?

IMO pulling in the pass manager to run the tests adds quite a bit of additional complexity to the tests, without much benefit, unless I am missing something. I think it would be preferable to just instantiate the required classes directly. Also, I think overall there would be less code.

ebrevnov abandoned this revision.Dec 4 2020, 2:30 AM