Hello. I have added a test case in the testsuite referring to an existing one as "builtin-rounding-1.c" in the /testsuite/gcc.dg/torture directory. As follows :
/* { dg-do link } */ extern int link_error (int); #define TEST(FN, VALUE, RESULT) \ if (__builtin_##FN (VALUE) != RESULT) link_error (__LINE__); int main (void) { TEST(roundeven, 0, 0); TEST(roundeven, 0.5, 0); TEST(roundeven, -0.5, 0); TEST(roundeven, 6, 6); TEST(roundeven, -8, -8); TEST(roundeven, 2.5, 2); TEST(roundeven, 3.5, 4); TEST(roundeven, -1.5, -2); TEST(roundeven, 3.499, 3); TEST(roundeven, 3.501, 4); return 0; } After checking for test case in the build directory using : $ make check-gcc RUNTESTFLAGS="dg-torture.exp=builtin-round-roundeven.c" test does not FAIL. Though, it FAILs for intentionally giving wrong RESULT. Ex : TEST(roundeven, 3.501, 3); Should fail I believe and thus giving output as: FAIL: gcc.dg/torture/builtin-round-roundeven.c -O0 (test for excess errors) FAIL: gcc.dg/torture/builtin-round-roundeven.c -O1 (test for excess errors) FAIL: gcc.dg/torture/builtin-round-roundeven.c -O2 (test for excess errors) FAIL: gcc.dg/torture/builtin-round-roundeven.c -O3 -g (test for excess errors) FAIL: gcc.dg/torture/builtin-round-roundeven.c -Os (test for excess errors) FAIL: gcc.dg/torture/builtin-round-roundeven.c -O2 -flto -fno-use-linker-plugin -flto-partition=none (test for excess errors) FAIL: gcc.dg/torture/builtin-round-roundeven.c -O2 -flto -fuse-linker-plugin -fno-fat-lto-objects (test for excess errors) Is this the way test cases should be added and checked? Thanks. On Wed, 8 May 2019 at 13:05, Tejas Joshi <tejasjoshi9...@gmail.com> wrote: > > Hello. > I can't figure out from the documentation how to add test cases in the > testsuite and inspect the results. How can I do that? Although, Taking > the mentioned conditions under consideration, I have made another > patch, attached. > > Thanks, > -Tejas > > > On Wed, 8 May 2019 at 09:01, Tejas Joshi <tejasjoshi9...@gmail.com> wrote: > > > > I should have taken all the test cases into consideration. Fool of me. I > > will try to make changes taking all the test cases into consideration along > > with the testsuite. > > Thanks. > > > > On Wed, 8 May 2019 at 02:31, Joseph Myers <jos...@codesourcery.com> wrote: > >> > >> On Wed, 8 May 2019, Tejas Joshi wrote: > >> > >> > Hello. > >> > As per my understanding, 3.5 would be represented in GCC as follows : > >> > r->uexp = 2 > >> > and > >> > r->sig[2] = 1110000....00 in binary 64 bit. (first 2 bits being 3 and > >> > following 1000....0 being 0.5, which later only happens for halfway > >> > cases) > >> > So, if we clear out the significand part and the immediate bit to the > >> > right > >> > which represent 0.5, the entire significand would become 0 due to > >> > bit-wise > >> > ANDing. > >> > > >> > > + tempsig[w] &= (((unsigned long)1 << ((n % HOST_BITS_PER_LONG) - 1)) > >> > > - > >> > > 1); > >> > > > >> > > >> > That is what the following line intend to do. The clearing part would > >> > change the significand, that's why significand was copied to a temporary > >> > >> That much is fine. My issues are two other things: > >> > >> * The function would wrongly return true for 3, not just for 3.5, because > >> it never checks the bit representing 0.5. (If you don't care what it > >> returns for 3, see my previous point about every function needing a > >> comment defining its semantics. Without such a comment, I have to guess, > >> and my guess here is that the function should return true for 3.5 but > >> false for 3 and for 3.5000...0001.) > >> > >> * The function would wrongly return true for 3.5000...0001, if there are > >> enough 0s that all those low bits in sig[2] are 0, but some low bits in > >> sig[1] or sig[0] are not 0. > >> > >> And also: > >> > >> * You should never need to modify parts of (a copy of) the significand in > >> place. Compare low parts of the significand (masked as needed) with 0. > >> If not 0, just return false. Likewise for comparing the 0.5 bit with 1. > >> It's not that copying and modifying in place results in incorrect logic, > >> it's simply excessively convoluted compared to things like: > >> > >> if ((something & mask) != 0) > >> return false > >> > >> (the function is probably twice as long as necessary because of that > >> copying). > >> > >> > array for checking. This logic is inspired by the clear_significand_below > >> > function. Or isn't this the way it was meant to be implemented? Also, why > >> > unsigned long sig[SIGSZ] has to be an array? > >> > >> What would it be other than an array? It can't be a single scalar because > >> floating-point significands may be longer than any supported integer type > >> on the host (remember the IEEE binary128 case). And if you made it a > >> sequence of individually named fields, a load of loops would need to be > >> manually unrolled, which would be much more error prone and hard to read. > >> > >> -- > >> Joseph S. Myers > >> jos...@codesourcery.com