This is trying to add support for r334428.
Details
Diff Detail
- Build Status
Buildable 19559 Build 19559: arc lint + arc unit
Event Timeline
llvm/lib/IR/ConstantRange.cpp | ||
---|---|---|
270 | Sorry for being difficult, but these should live on APInt and be individually unit-tested. | |
336 | Can you please given an informal description of how this works? For example, naively I'd think for unsigned overflow just checking [0,Other.getUnsignedMax()) would be sufficient. |
LGTM modulo comments on the test coverage inline.
I didn't check the math too carefully since the test coverage looks good.
llvm/unittests/IR/ConstantRangeTest.cpp | ||
---|---|---|
1024 | All of these tests would have passed without your change right (since earlier we'd return an empty range for multiplication)? If yes, maybe add a couple of tests that check that we return non-trivial results (i.e. not empty sets) for some inputs? | |
1051 | Maybe add an /*isSigned=*/true here? | |
1056 | Minor nit, but it looks like these three loops can be different test cases with more descriptive names. |
llvm/unittests/IR/ConstantRangeTest.cpp | ||
---|---|---|
1024 |
No they don't pass. These tests test the inverse cases too, e.g. "iff" instead of "if". Looking at EXPECT_EQ(!Overflow, Range.contains(...));, if Overflow is false and Range is empty, the test will fail. |
Sorry for being difficult, but these should live on APInt and be individually unit-tested.