[DivRemPairs] Avoid RAUW pitfalls (PR42823)
authorRoman Lebedev <lebedev.ri@gmail.com>
Wed, 31 Jul 2019 12:06:38 +0000 (12:06 +0000)
committerRoman Lebedev <lebedev.ri@gmail.com>
Wed, 31 Jul 2019 12:06:38 +0000 (12:06 +0000)
commit5f616901f579062aabe32fee165eacd36de82e8b
tree77bc6b38c04467edbfed3a5c902b8ebb732ddc31
parent005eff04cc117f8b7626248486ec53f87cb8d3c7
[DivRemPairs] Avoid RAUW pitfalls (PR42823)

Summary:
`DivRemPairs` internally creates two maps:
* {sign, divident, divisor} -> div instruction
* {sign, divident, divisor} -> rem instruction
Then it iterates over rem map, and looks if there is an entry
in div map with the same key. Then depending on some internal logic
it may RAUW rem instruction with something else.

But if that rem instruction is an input to other div/rem,
then it was used as a key in these maps, so the old value (used in key)
is now dandling, because RAUW didn't update those maps.
And we can't even RAUW map keys in general, there's `ValueMap`,
but we don't have a single `Value` as key...

The bug was discovered via D65298, and the test there exists.
Now, i'm not sure how to expose this issue in trunk.
The bug is clearly there if i change the map keys to be `AssertingVH`/`PoisoningVH`,
but i guess this didn't miscompiled anything thus far?
I really don't think this is benin without that patch.

The fix is actually rather straight-forward - instead of trying to somehow
shoe-horn `ValueMap` here (doesn't fit, key isn't just `Value`), or writing a new
`ValueMap` with key being a struct of `Value`s, we can just have an intermediate
data structure - a vector, each entry containing matching `Div, Rem` pair,
and pre-filling it before doing any modifications.
This way we won't need to query map after doing RAUW, so no bug is possible.

Reviewers: spatel, bogner, RKSimon, craig.topper

Reviewed By: spatel

Subscribers: hiraditya, hans, llvm-commits

Tags: #llvm

Differential Revision: https://reviews.llvm.org/D65451

llvm-svn: 367417
llvm/include/llvm/Transforms/Utils/BypassSlowDivision.h
llvm/lib/Transforms/Scalar/DivRemPairs.cpp
llvm/test/Transforms/DivRemPairs/PowerPC/div-expanded-rem-pair.ll
llvm/test/Transforms/DivRemPairs/PowerPC/div-rem-pairs.ll
llvm/test/Transforms/DivRemPairs/X86/div-rem-pairs.ll