Page MenuHomePhabricator

[LoopInterchange] Split up interchange.ll test case (NFC).
ClosedPublic

Authored by fhahn on Jul 17 2017, 8:28 AM.

Details

Summary

Currently most tests for the loop interchange pass are in
test/Transforms/LoopInterchange/interchange.ll. This patch splits up the
large test file in smaller pieces, which makes debugging test failures
easier.

Diff Detail

Event Timeline

fhahn created this revision.Jul 17 2017, 8:28 AM

@mssimpso @mkuper @mzolotukhin

It looks like not much has been done for a while on the LoopInterchange pass. I'd like to try to improve it gradually, but not many people touched the code recently and the original author does not seem to respond to reviews (D34879 and D35210 have been hanging around a bit), so finding appropriate reviewers is hard.

I think you reviewed changes related to loop optimizations recently and I thought you might be able to point me to people suitable as reviewers for this changes or advice on how to proceed :)

One concern with breaking up the tests into separate files is that you're now invoking opt several more times. Generally, this isn't preferred, but if others agree with this direction I won't object.

fhahn added a comment.Jul 17 2017, 9:00 AM

Ok sure I wasn't aware of that, just let me know what people prefer. It seemed just easier for debugging to split them in separate files, but that's only a slight advantage

We don't really have a policy on this either way, I think.
I certainly don't have any strong opinions about it.

hfinkel accepted this revision.Jul 17 2017, 5:18 PM
hfinkel added a subscriber: hfinkel.

We don't really have a policy on this either way, I think.

My impression is that we have an effective policy on this, but it won't help: We try to reduce the number of opt invocations, however, only so long as this supports good logical organization and maintainability. This is fine with me.

I certainly don't have any strong opinions about it.

This revision is now accepted and ready to land.Jul 17 2017, 5:18 PM
fhahn closed this revision.Jul 18 2017, 2:47 AM