This is an archive of the discontinued LLVM Phabricator instance.

[IR] Support efficient lookup of AssertingVH/PoisoningVH
ClosedPublic

Authored by nikic on Jun 13 2020, 12:47 PM.

Details

Summary

Currently, there doesn't seem to be any way to look up a Value* in a map/set indexed by AssertingVH/PoisoningVH, without creating a value handle, which is fairly expensive, because it involves adding the value handle to the use list and immediately removing it again. Using find_as(Value *) does not work (and is in fact worse than just using find(Value *)), because it will end up creating multiple value handles during the lookup.

For AssertingVH, address this by simply using DenseMapInfo<T *> instead of manually implementing something. The AssertingVH<T> will now get coerced to T*, rather than the other way around.

For PoisoningVH, add extra overloads of getHashValue() and isEqual() that accept a T* argument.

This allows using find_as(Value *) to perform efficient lookups in assertion-enabled builds.

Diff Detail

Event Timeline

nikic created this revision.Jun 13 2020, 12:47 PM
Herald added a project: Restricted Project. · View Herald TranscriptJun 13 2020, 12:47 PM

Thanks! Seems reasonable. Can you please add a DenseMap test to llvm/unittests/IR/ValueHandleTest.cpp?

llvm/include/llvm/IR/ValueHandle.h
313

public can be omitted.

Reusing DenseMapInfo<T *> is clever and more efficient.

nikic updated this revision to Diff 270619.Jun 14 2020, 7:01 AM
nikic marked an inline comment as done.

Remove unnecessary public, add some basic tests. These essentially just check that AssertingVH/PoisoningVH inside a DenseMap work, because I don't know how I could test that using find_as() does not construct a temporary value handle.

MaskRay accepted this revision.Jun 14 2020, 8:10 AM

Thanks!

llvm/unittests/IR/ValueHandleTest.cpp
496 ↗(On Diff #270619)

{ K, V } -> {K, V}

This revision is now accepted and ready to land.Jun 14 2020, 8:10 AM
This revision was automatically updated to reflect the committed changes.
nikic marked an inline comment as done.