Please take a look,
Chad
Details
Diff Detail
Event Timeline
| lib/Transforms/InstCombine/InstCombineCompares.cpp | ||
|---|---|---|
| 1549 | Why do you need to do this ConstantInt::get call? Shouldn't A and CI2 have the same type? | |
| lib/Transforms/InstCombine/InstCombineCompares.cpp | ||
|---|---|---|
| 1543–1550 | This transform is valid regardless of what value CI2 holds. We should require it to be a ConstantInt. | |
| test/Transforms/InstCombine/compare-udiv.ll | ||
| 3–9 | This transform is valid, regardless of what value the numerator is. We shouldn't require a constant here. | |
| 11–17 | It'd be good to have a version of this test where the numerator is -1. | |
Address David's comments:
-Handle both constant and non-constant numerator.
-Add test with -1 numerator.
Committed in r268960. I did have to move the optimizations after visitICmpInstWithInstAndIntCst() in order to not regress udiv_icmp1 in exact.ll. Also, I added an additional test case, udiv_icmp2, to provide more context and to improve coverage.
Why do you need to do this ConstantInt::get call? Shouldn't A and CI2 have the same type?