This is an archive of the discontinued LLVM Phabricator instance.

[SimpleLoopUnswitch] Add tests to check partially invariant unswitch
ClosedPublic

Authored by jaykang10 on Mar 29 2021, 3:16 AM.

Details

Summary

It is a test file for partially invariant unswitch. It is related to https://reviews.llvm.org/D99354

Diff Detail

Event Timeline

jaykang10 created this revision.Mar 29 2021, 3:16 AM
jaykang10 requested review of this revision.Mar 29 2021, 3:16 AM
Herald added a project: Restricted Project. · View Herald TranscriptMar 29 2021, 3:16 AM
fhahn accepted this revision.Mar 29 2021, 4:30 AM
fhahn added a subscriber: fhahn.

LGTM, thanks!

This revision is now accepted and ready to land.Mar 29 2021, 4:30 AM

LGTM, thanks!

Thanks for review @fhahn. I have a question. Is it ok to push this test now? or I have to wait for https://reviews.llvm.org/D99354?

LGTM, thanks!

Thanks for review @fhahn. I have a question. Is it ok to push this test now? or I have to wait for https://reviews.llvm.org/D99354?

I think you should push it and rebase D99354

LGTM, thanks!

Thanks for review @fhahn. I have a question. Is it ok to push this test now? or I have to wait for https://reviews.llvm.org/D99354?

I think you should push it and rebase D99354

Yep, I will push it. Thanks for letting me know.

This revision was landed with ongoing or failed builds.Mar 29 2021, 5:07 AM
This revision was automatically updated to reflect the committed changes.

Looks like this test fails: http://45.33.8.238/linux/42881/step_12.txt

um... that's what I was concerned about pushing this patch... because this test relies on https://reviews.llvm.org/D99354. Do I have to revert this patch or something like that? @thakis Can you let me know what I have to do please?

fhahn added a comment.Mar 29 2021, 5:49 AM

Looks like this test fails: http://45.33.8.238/linux/42881/step_12.txt

um... that's what I was concerned about pushing this patch... because this test relies on https://reviews.llvm.org/D99354. Do I have to revert this patch or something like that? @thakis Can you let me know what I have to do please?

Sorry for not being clear. You should pre-commit the tests for D99354 with the checks for the IR generated by *current* trunk. Then update D99354 so the diff only shows the impact of the patch. You should never commit a patch with tests that do *not* pass.

Looks like this test fails: http://45.33.8.238/linux/42881/step_12.txt

um... that's what I was concerned about pushing this patch... because this test relies on https://reviews.llvm.org/D99354. Do I have to revert this patch or something like that? @thakis Can you let me know what I have to do please?

You should push this test with current trunk checks, then rebase D99354 and regenerate checks

Sorry for mistake... I will push the change with trunk.