[PATCH] Fix for CodeGenPrepare wrongly updating the map of sunk addresses

[Nick Lewycky]

Andrea_DiBiagio at sn.scee.net wrote:
> Hello,
>
> I'd like to contribute a patch to fix a problem in Pass CodeGen Prepare.
>
> Method OptimizeMemoryInst is not always able to correctly update DenseMap
> SunkAddrs by erasing no longer valid keys associated to Values which have
> been deleted in memory.
>
> When method OptimizeMemoryInst calls function
> `RecursivelyDeleteTriviallyDeadInstructions', potentially not all the
> dead keys are erased from the map SunkAddrs.
> Therefore, there are cases where map SunkAddrs is left in a inconsistent
> state where buckets associated to keys of deleted Values are
> still considered to be valid.
>
> This can be problematic for example in the following scenario where:
> 1. A new Instruction I created to replace the address expression for a
>     load or store instruction;
> 2. address of I is the same as a previously deleted Value
>     pointed to by a key K of map SunkAddrs;
> 3. K is still a valid key in SunkAddrs.
>
> CodeGenPrepare may think that occurrence of address I could be replaced
> with the Value returned by SunkAddrs[I] which is however invalid.
>
> PROPOSED FIX
> ------------
> With this patch, CodeGenPrepare now uses a CallbackVH as the key of
> map SunkAddrs. Each CallbackVH is able to monitor changes in the
> state of a specific key Value. When a key is deleted by
> method RecursivelyDeleteTriviallyDeadInstructions its associated
> CallbackVH is notified and the corresponding entry in SunkAddrs
> is correctly erased.
> I also added an x86 specific test case that exposes the problem
> found in CodeGenPrepare.

Instead of building your own CallbackVH, would using a ValueMap work? If 
not, I'd really like to see a generalized way to create the equivalent 
of DenseMap<WeakVH<...>, ...>.

+    typedef DenseMap<SunkAddressVH, Value*,
+                     SunkAddressVHDenseMapInfo>  SunkAddressMapType;

Two spaces before "SunkAddressMapType;" should be one.

Nick