Re: [PATCH] Improve gen-libm-test.pl LIT() application

["Paul E. Murphy" <murphyp at linux dot vnet dot ibm dot com>]

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.