Hi Jeff, I wasn't sure if you saw the updated patch attached to the previous email or if you just hadn't had the time to look at it yet.
Cheers, Tamar ________________________________________ From: Jeff Law <l...@redhat.com> Sent: Monday, December 19, 2016 8:27:33 PM To: Tamar Christina; Joseph Myers Cc: GCC Patches; Wilco Dijkstra; rguent...@suse.de; Michael Meissner; nd Subject: Re: [PATCH][GCC][PATCHv3] Improve fpclassify w.r.t IEEE like numbers in GIMPLE. On 12/15/2016 03:14 AM, Tamar Christina wrote: > >> On a high level, presumably there's no real value in keeping the old >> code to "fold" fpclassify. By exposing those operations as integer >> logicals for the fast path, if the FP value becomes a constant during >> the optimization pipeline we'll see the reinterpreted values flowing >> into the new integer logical tests and they'll simplify just like >> anything else. Right? > > Yes, if it becomes a constant it will be folded away, both in the integer and > the fp case. Thanks for clarifying. > >> The old IBM format is still supported, though they are expected to be >> moveing towards a standard ieee 128 bit format. So my only concern is >> that we preserve correct behavior for those cases -- I don't really care >> about optimizing them. So I think you need to keep them. > > Yes, I re-added them. It's mostly a copy paste from what they were in the > other functions. But I have no way of testing it. Understood. >> > + const HOST_WIDE_INT type_width = TYPE_PRECISION (type); >>> + return (format->is_binary_ieee_compatible >>> + && FLOAT_WORDS_BIG_ENDIAN == WORDS_BIG_ENDIAN >>> + /* We explicitly disable quad float support on 32 bit systems. */ >>> + && !(UNITS_PER_WORD == 4 && type_width == 128) >>> + && targetm.scalar_mode_supported_p (mode)); >>> +} >> Presumably this is why you needed the target.h inclusion. >> >> Note that on some systems we even disable 64bit floating point support. >> I suspect this check needs a little re-thinking as I don't think that >> checking for a specific UNITS_PER_WORD is correct, nor is checking the >> width of the type. I'm not offhand sure what the test should be, just >> that I think we need something better here. > > I think what I really wanted to test here is if there was an integer mode > available > which has the exact width as the floating point one. So I have replaced this > with > just a call to int_mode_for_mode. Which is probably more correct. I'll need to think about it, but would inherently think that int_mode_for_mode is better than an explicit check of UNITS_PER_WORD and typewidth. > >>> + >>> +/* Determines if the given number is a NaN value. >>> + This function is the last in the chain and only has to >>> + check if it's preconditions are true. */ >>> +static tree >>> +is_nan (gimple_seq *seq, tree arg, location_t loc) >> So in the old code we checked UNGT_EXPR, in the new code's slow path you >> check UNORDERED. Was that change intentional? > > The old FP code used UNORDERED and the new one was using ORDERED and negating > the result. > I've replaced it with UNORDERED, but both are correct. OK. Just wanted to make sure. jeff
