This is an archive of the discontinued LLVM Phabricator instance.

[LoopStrengthReduce] Don't use a DenseSet<int64_t> when we might add any valid int64_t to the set.
ClosedPublic

Authored by jlebar on Nov 4 2016, 11:20 PM.

Details

Summary

SmallSetVector uses DenseSet, but that means we need to reserve some
values for the empty and tombstone keys.

It seems to me we should have a general way to let us store full-range
ints inside of DenseSets, and furthermore that we probably shouldn't
silently let you add ints into DenseSets without explicitly promising
that they're in range. But that's a battle for another day; for now,
just fix this code, since we currently do something Very Bad when
compiling ffmpeg.

Fixes PR30914.

Event Timeline

jlebar updated this revision to Diff 76965.Nov 4 2016, 11:20 PM
jlebar retitled this revision from to [LoopStrengthReduce] Don't use a DenseSet<int64_t> when we might add any valid int64_t to the set..
jlebar updated this object.
jlebar added a reviewer: jeremyhu.
jlebar added a subscriber: llvm-commits.
hfinkel accepted this revision.Nov 5 2016, 8:31 AM
hfinkel added a reviewer: hfinkel.
hfinkel added a subscriber: hfinkel.

LGTM

We should probably also have some template specialization in the DenseMap implementation that causes us to have an assert that will catch this situation more-directly (i.e. with an easy-to-understand assertion message).

This revision is now accepted and ready to land.Nov 5 2016, 8:31 AM
This revision was automatically updated to reflect the committed changes.