[PATCH] D58435: [SCEV] Ensure that isHighCostExpansion takes into account what is being divided
Sanjoy Das via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Fri Mar 1 16:49:41 PST 2019
sanjoy requested changes to this revision.
sanjoy added a comment.
This revision now requires changes to proceed.
The change itself looks fine to me, but I'm a bit worried about the test changes. Is it possible that the tests were checking certain the correctness for certain expressions that we no longer check because these happen to be expensive? If that's the case maybe we should add a debug flag that disregards the "is expensive expansion" check to make sure the the same code paths remain tested?
Can you also add a specific test for this change in behavior? Even if it is incidentally tested by the other tests, it would be nicer to have a dedicated test.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D58435/new/
https://reviews.llvm.org/D58435
More information about the llvm-commits
mailing list