This is an archive of the discontinued LLVM Phabricator instance.

[test] Pre-commit test case for PR51981. NFC
ClosedPublic

Authored by bjope on Sep 30 2021, 4:14 AM.

Diff Detail

Event Timeline

bjope requested review of this revision.Sep 30 2021, 4:14 AM
bjope created this revision.
Herald added a project: Restricted Project. · View Herald TranscriptSep 30 2021, 4:14 AM
fhahn added inline comments.Sep 30 2021, 10:01 AM
llvm/test/Transforms/LoopRotate/pr51981-scev-problem.ll
3

Just do double check, running loop(canon-freeze) is required for the crash? Is that only so the SCEV cache is populated?

bjope added inline comments.Sep 30 2021, 10:38 AM
llvm/test/Transforms/LoopRotate/pr51981-scev-problem.ll
3

Yes, that is my understanding. I tried to populate it using print<scalar-evolution> instead, or require<scalar-evolution>, but then I do not see the crash for some reason.
Btw, I noticed right now that I can replace bounds-checking by print<scalar-evolution> and then I still get the crash (but in the printing pass instead).

Not quite sure why the symptom is a bit different depending on how I populate the cache. Afaict canon-freeze isn't doing any modifications to the IR (and neither should the printing).

fhahn added inline comments.Oct 5 2021, 2:33 AM
llvm/test/Transforms/LoopRotate/pr51981-scev-problem.ll
3

Btw, I noticed right now that I can replace bounds-checking by print<scalar-evolution> and then I still get the crash (but in the printing pass instead).

Using print<scalar-evolution> instead of bounds-checking would be better, as print<scalar-evolution> should be more stable.

36

it might be good to rename the blocks to make it easier to identify that there is a loop nest with 2 loops (maybe something like loop.1.header, loop.2, loop.1.latch)

37

does this have to be a load?

bjope updated this revision to Diff 377496.Oct 6 2021, 4:02 AM

Cleaned up the test case a bit as suggested in the review.

bjope added inline comments.Oct 6 2021, 4:06 AM
llvm/test/Transforms/LoopRotate/pr51981-scev-problem.ll
37

Haven't found anything else that still reproduce the problem (such as using an add instruction instead).

fhahn accepted this revision.Oct 7 2021, 4:43 AM

LGTM, thanks!

This revision is now accepted and ready to land.Oct 7 2021, 4:43 AM
This revision was automatically updated to reflect the committed changes.