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.

Diff Detail

Repository
rL LLVM

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.