With the fixed implementation of the "remainder" operation, we can now add support to folding calls to it.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
Added a test (copied from the one for "fmod").
Added handling of "remainder" to TargetLibraryInfo.cpp.
llvm/lib/Analysis/ConstantFolding.cpp | ||
---|---|---|
2106 | Wanted to be consistent with the coding style on that particular file. See line 2095. |
llvm/lib/Analysis/ConstantFolding.cpp | ||
---|---|---|
2106 | I would say change this, and leave the unrelated part alone |
llvm/test/Analysis/ConstantFolding/math-2.ll | ||
---|---|---|
31 | I agree that it should be tested for double as well, but the test cases are not important, as we test them under the APFloat unit-test. Otherwise, we will just have duplicates in the implementation code and the using code. |
llvm/test/Analysis/ConstantFolding/math-2.ll | ||
---|---|---|
31 | The code specifically checks for opStatus though, so the point isn't necessarily then folding result |
llvm/test/Analysis/ConstantFolding/math-2.ll | ||
---|---|---|
31 | The APFloat unit test is also testing various combination of operations' results. |
llvm/test/Analysis/ConstantFolding/math-2.ll | ||
---|---|---|
37 | remainderl is not folded. Just wanted to be consistent with other math operations (like fmodl). I am not sure why they are not folded, but it is consistent with other compilers like icc and gcc). |
llvm/test/Analysis/ConstantFolding/math-2.ll | ||
---|---|---|
31 | Right, but wouldn't adding a bunch of other value combinations just test the result of APFloat? For which we already have the unit test. It doesn't only test the status, but also the actual result (bitwise). |
llvm/test/Analysis/ConstantFolding/math-2.ll | ||
---|---|---|
31 | You don't need to add a large number of other tests, but you should have a test for something that shouldn't fold (V.remainder returns something other than opOK). Otherwise, this LGTM. | |
37 | Yeah, this is indeed consistent with the other similar functions. I'm not sure why either, and we probably should do this (at least on systems where we accurately model the semantics of the long double type). |
llvm/test/Analysis/ConstantFolding/math-2.ll | ||
---|---|---|
31 | Sounds good. I added another test for success in folding, and another for a failure. |
Backwards condition