This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
Re: [PATCH] Improve gen-libm-test.pl LIT() application
On 08/04/2016 11:02 AM, Carlos O'Donell wrote:
> On 08/04/2016 11:05 AM, Paul E. Murphy wrote:
>> When bootstrapping float128, this exposed a number of areas where
>> the L suffix is incorrectly applied to simple expressions when it
>> should be applied to each constant in the expression.
>>
>> In order to stave off more macros in libm-test.inc, apply_lit is
>> made slightly more intelligent. It will now split most basic
>> expressions and apply LIT() individually to each token. As noted,
>> it is not a perfect solution, but compromises correctness,
>> readability, and simplicity.
>>
>> The above is problematic when the L real suffix is not the most
>> expressive modifier, and the compiler complains (i.e ppc64) or
>> silently truncates a value (i.e ppc64).
>>
>> * math/gen-libm-test.pl (apply_lit): Rewrite to apply
>> LIT() to individual constants in simple expressions.
>> (_apply_lit): Rename replaced version, and use it to
>> apply to what appears to be a token.
>
> This looks like a good compromise to me.
>
> Question below.
>
>> ---
>> math/gen-libm-test.pl | 40 +++++++++++++++++++++++++++++++++++++++-
>> 1 file changed, 39 insertions(+), 1 deletion(-)
>>
>> diff --git a/math/gen-libm-test.pl b/math/gen-libm-test.pl
>> index aa66e76..cb0a02b 100755
>> --- a/math/gen-libm-test.pl
>> +++ b/math/gen-libm-test.pl
>> @@ -163,7 +163,7 @@ sub show_exceptions {
>>
>> # Apply the LIT(x) macro to a literal floating point constant
>> # and strip any existing suffix.
>> -sub apply_lit {
>> +sub _apply_lit {
>> my ($lit) = @_;
>> my $exp_re = "([+-])?[[:digit:]]+";
>> # Don't wrap something that does not look like a:
>> @@ -180,6 +180,44 @@ sub apply_lit {
>> return "LIT (${lit})";
>> }
>>
>> +
>> +# Apply LIT macro to individual tokens within an
>> +# expression. This is only meant to work with
>> +# the very simple expressions encountered like
>> +# A (op B)*. It will not split all literals
>> +# correctly, but suffices for this usage without
>> +# a substantially more complex tokenizer.
>
> Is there any chance you can reject literals you won't correctly
> split and raise an error?
As I understand it, the only incorrect splitting occurs for
some inputs of the form:
{integer, identifier} op {integer,real}
Which, will ultimately only apply LIT() to the expressions
containing a real value as the second operand. But, LIT is
applied to the entire expression.
So you might end up passing things like "MAX_EXP+1", "0xe+1.0f"
to _apply_lit. The former does happen, the latter is a
constructed example.
If more complicated expressions are used in libm-test.inc, or
this workaround proven insufficient, we should refactor
libm-test.inc to remove the need for this hack.
>> +sub apply_lit {
>> + my ($lit) = @_;
>> + my @toks = ();
>> +
>> + # There are some constant expressions embedded in the
>> + # test cases. To avoid writing a more complex lexer,
>> + # we apply some fixups, and split based on simple
>> + # operators, unfixup, then apply LIT as needed.
>> +
>> + # This behaves poorly on inputs like 0x0e+1.0f,
>> + # or MAX_EXP+1, but ultimately doesn't break
>> + # anything... for now.
>
> What do you mean by 'behaves poorly'? It looks like your
> regexp's below handle it correctly.
They are ultimately handled correctly, but you could still
conceivably end up applying LIT to more than necessary.
I.e LIT(0xe+1.0) instead of 0xe+LIT(1.0). MAX_EXP+1 is
vacuously correct as it never needs modification.
I've yet to convince myself this solution is foolproof,
but it suffices for the current usage in libm-test.inc.