This is an archive of the discontinued LLVM Phabricator instance.

[Transforms][LICM] A test case for the upcoming fix D152281 for the issue with reassociation profitability
ClosedPublic

Authored by pawosm01 on Jun 6 2023, 9:11 AM.

Details

Summary

This commit introduces a test for the upcoming change addressing
the following issue: https://github.com/llvm/llvm-project/issues/62736

Diff Detail

Event Timeline

pawosm01 created this revision.Jun 6 2023, 9:11 AM
Herald added a project: Restricted Project. · View Herald TranscriptJun 6 2023, 9:11 AM
pawosm01 requested review of this revision.Jun 6 2023, 9:11 AM
Herald added a project: Restricted Project. · View Herald TranscriptJun 6 2023, 9:11 AM
qcolombet accepted this revision.Jun 6 2023, 9:44 AM
This revision is now accepted and ready to land.Jun 6 2023, 9:44 AM
fhahn added inline comments.Jun 6 2023, 1:12 PM
llvm/test/Transforms/LICM/expr-reassociate.ll
3

Usually the tests for a specific transform shouldn't depend on other passes; You could add a test to PhaseOrdering that tests the end-to-end flow (e.g. with -O2) and then here have just the ones that run licm on the input IR that requires reassoication to move it out of the loop

mgabka added a subscriber: mgabka.Jun 7 2023, 12:39 AM
pawosm01 marked an inline comment as done.Jun 7 2023, 6:08 AM
pawosm01 added inline comments.
llvm/test/Transforms/LICM/expr-reassociate.ll
3

I was about to move a vital part of this test into the Transforms/PhaseOrdering but I was encouraged not to. The test as it is now captures the interplay between those two passes quite well and is self-contained. Those end-to-end tests are pretty rare.

pawosm01 marked an inline comment as done.Jun 9 2023, 6:29 AM
pawosm01 updated this revision to Diff 531107.Jun 13 2023, 4:16 PM

Extending it with an additional test case exposing shortcomings that have been fixed.

pawosm01 updated this revision to Diff 542460.Jul 20 2023, 5:44 AM

Followed a reviewer's suggestion to limit the applicability of the attempted transformation.

pawosm01 updated this revision to Diff 544422.Jul 26 2023, 10:16 AM

One more test case added.