On 07/29/2016 04:50 PM, Joseph Myers wrote:
On Mon, 18 Jul 2016, Martin Sebor wrote:+ /* Number of exponent digits or -1 when unknown. */ + int expdigs = -1; + /* 1 when ARG < 0, 0 when ARG >= 0, -1 when unknown. */ + int negative = -1; + /* Log10 of EXPDIGS. */ + int logexpdigs = 2; + + const double log10_2 = .30102999566398119521; + + if (arg && TREE_CODE (arg) == REAL_CST) + { + expdigs = real_exponent (TREE_REAL_CST_PTR (arg)) * log10_2; + negative = real_isneg (TREE_REAL_CST_PTR (arg)); + logexpdigs = ilog (expdigs, 10);
Thanks for looking at it! The floating conversion wasn't 100% complete and the bits that were there I had the least confidence in so I especially appreciate a second pair of eyes on it.
This looks wrong for the case of a constant with a negative exponent.
You're right -- great catch! I've fixed it.
Also, what if the constant has a decimal floating-point type?
Decimal floating point isn't handled in this initial implementation. The checker gives up after the first directive it doesn't recognize and stops checking the rest of the format string. Eventually, I'd like to enhance it to handle decimal floats and other directives and types.
+ } + else if (REAL_MODE_FORMAT (TYPE_MODE (type))->b == 2) + { + /* Compute T_MAX_EXP for base 2. */ + expdigs = REAL_MODE_FORMAT (TYPE_MODE (type))->emax * log10_2;Shouldn't you also compute logexpdigs here? The comment on logexpdigs implies it's always meant to have a given relation to expdigs. Or maybe those variables need to be split into min/max cases, since you're computing both min/max values below.
I have reworked this so logexpdigs is computed here as well.
+ case 'E': + case 'e': + /* The minimum output is "[-+]1.234567e+00" for an IEEE double + regardless of the value of the actual argument. */ + res.min = ((0 < negative || spec.get_flag ('+') || spec.get_flag (' ')) + + 1 /* unit */ + (prec < 0 ? 7 : prec ? prec + 1 : 0) + + 2 /* e+ */ + (logexpdigs < 2 ? 2 : logexpdigs));As I understand your logic, for the case of a constant expdigs and so logexpdigs may sometimes be 1 bigger than the correct value for that constant, say for 9.999e+99, so this may not actually be the minimum.
Yes, %e is an approximation though I hadn't considered this case. I reworked this part of the implementation to compute an accurate result.
+ case 'G': + case 'g': + /* Treat this the same as '%F' for now even though that's + inaccurate. */ + res.min = 2 + (prec < 0 ? 6 : prec); + res.max = ((spec.get_flag ('L') ? 4934 : 310) + + (prec < 0 ? 6 : prec ? prec + 1 : 0));Again, avoid embedding properties of given formats.
I've removed the IEEE 754 assumptions from both the code and the comments. I will post the latest version of the patch shortly. Martin
