[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