]> granicus.if.org Git - llvm/commit
Fix a bug in SCEV's isSafeToExpand around speculation safety
authorPhilip Reames <listmail@philipreames.com>
Thu, 18 Apr 2019 16:10:21 +0000 (16:10 +0000)
committerPhilip Reames <listmail@philipreames.com>
Thu, 18 Apr 2019 16:10:21 +0000 (16:10 +0000)
commit5cf89ede07cf10b6173f1406bb55eb46490da335
treed6baed081c4fb8439f00aa73a37372f76056cf95
parent93d23822187bbd144802f3ab9cf75a4e967e4c0c
Fix a bug in SCEV's isSafeToExpand around speculation safety

isSafeToExpand was making a common, but dangerously wrong, mistake in assuming that if any instruction within a basic block executes, that all instructions within that block must execute.  This can be trivially shown to be false by considering the following small example:
bb:
  add x, y  <-- InsertionPoint
  call @throws()
  udiv x, y <-- SCEV* S
  br ...

It's clearly not legal to expand S above the throwing call, but the previous logic would do so since S dominates (but not properlyDominates) the block containing the InsertionPoint.

Since iterating instructions w/in a block is expensive, this change special cases two cases: 1) S is an operand of InsertionPoint, and 2) InsertionPoint is the terminator of it's block.  These two together are enough to keep all current optimizations triggering while fixing the latent correctness issue.

As best I can tell, this is a silent bug in current ToT.  Given that, there's no tests with this change.  It was noticed in an upcoming optimization change (D60093), and was reviewed as part of that.  That change will include the test which caused me to notice the issue.  I'm submitting this seperately so that anyone bisecting a problem gets a clear explanation.

git-svn-id: https://llvm.org/svn/llvm-project/llvm/trunk@358680 91177308-0d34-0410-b5e6-96231b3b80d8
lib/Analysis/ScalarEvolutionExpander.cpp