This is an archive of the discontinued LLVM Phabricator instance.

[SCEV] Do not insert if it is already in cache
ClosedPublic

Authored by skatkov on Dec 19 2017, 1:44 AM.

Details

Summary

This is fix for the crash caused by ScalarEvolution::getTruncateExpr.

It expects that if it checked the condition that SCEV is not in UniqueSCEVs cache in
the beginning that it will not be there inside this method.

However during recursion and transformation/simplification for sub expression,
it is possible that these modifications will end up with the same SCEV as we started from.

So we must always check whether SCEV is in cache and do not insert item if it is already there.

Diff Detail

Event Timeline

skatkov created this revision.Dec 19 2017, 1:44 AM
skatkov updated this revision to Diff 127479.Dec 19 2017, 3:23 AM

Test is simplified a bit, comment in test is provided to explain how we got a crash.

Do you mind giving the blocks in test some reasonable names? E.g. outer_loop, inner_loop etc, just to make it clear what is going on.

test/Analysis/ScalarEvolution/truncate.ll
44

Nit: update -> updated

skatkov updated this revision to Diff 128161.Dec 26 2017, 3:49 AM

Hi Sanjoy, could you please take a look into this as well?

sanjoy accepted this revision.Dec 26 2017, 10:01 PM

LGTM!

lib/Analysis/ScalarEvolution.cpp
1272

Please add comments here and below about what's going on.

test/Analysis/ScalarEvolution/truncate.ll
23

Please run this through -metarenamer. The %1 etc. names aren't really names, and e.g. adding a new instruction to the test will change these (which will make the test difficult to update).

This revision is now accepted and ready to land.Dec 26 2017, 10:01 PM
This revision was automatically updated to reflect the committed changes.