Front page | perl.perl5.porters |
Postings from September 2010
Re: Another [PATCH] i_opt
Thread Previous
|
Thread Next
From:
Reini Urban
Date:
September 3, 2010 06:47
Subject:
Re: Another [PATCH] i_opt
Message ID:
4C80FC68.4010409@x-ray.at
Nicholas Clark schrieb:
> On Tue, Aug 31, 2010 at 12:44:47AM +0200, Reini Urban wrote:
>
>> But looking at the old code I found another interesting, and similar
>> story: e7311069df54baa6 "The removal of OA_RETINTEGER"
>> - More Nick-picking -
>
> Did you see the horrible hack that we used to have?
> Effectively we were using 33 bits of a 32 bit quantity.
Yes, that's horrible. But it worked for up to 4 op args.
>> There once was a young british, who disabled in 2002 the whole constant
>> folding for integers. All i_ ops which are guaranteed to return ints
>> with constant int args, are not constant folded anymore. intops are also
>
> That's not accurate. Constant folding remains. Demonstration:
>
> $ ./perl -Ilib -MO=Concise -e '$a = 2 + 3';
> 6<@> leave[1 ref] vKP/REFC ->(end)
> 1<0> enter ->2
> 2<;> nextstate(main 1 -e:1) v:{ ->3
> 5<2> sassign vKS/2 ->6
> 3<$> const(IV 5) s ->4
> -<1> ex-rv2sv sKRM*/1 ->5
> 4<$> gvsv(*a) s ->5
> -e syntax OK
> $ ./perl -Ilib -MO=Concise -e 'use integer; $a = 2 + 3';
> 6<@> leave[1 ref] vKP/REFC ->(end)
> 1<0> enter ->2
> 2<;> nextstate(main 6 -e:1) v:{,i ->3
> 5<2> sassign vKS/2 ->6
> 3<$> const(IV 5) s ->4
> -<1> ex-rv2sv sKRM*/1 ->5
> 4<$> gvsv(*a) s ->5
> -e syntax OK
I didn't say that constant folding is broken,
broken is the promotion to intops with int consts.
In my example was an i_add instead of an add.
> You are correct that I removed the conversion of the general arithmetic
> operations to intops, which the "constant folding" routine used to do.
>
>> not promoted anymore. By accident this "i" bit was removed just this
>> week, so I had to rewrite Opodes because the opmask changed, which did
>> not happen before, so I had to introduce compile-time constant lookup
>> from op.h of these opmasks.
>>
>> The complainer was some Windows guy who complained that
>> multiplied int constants will overflow.
>> http://www.xray.mpe.mpg.de/mailing-lists/perl5-porters/2002-02/msg00281.html
>>
>> But instead of doing the obvious, disabling promotion of multiply to
>> i_multiply as already done for divide and mod this young british fellow
>> removed the whole integer constant folding optimizations for all i_ ops.
>> OA_RETINTEGER in opcode.pl was dead.
>> Which was i++, i--, --i, ++i, all i comparisons (<,>, ...), i_arith and
>
> Prefix increment and decrement can all return floating point values, for
> integer input. It would not be valid to assume that they can only return
> integers.
Wrong: i_ops can only return IV with non-constant integer args,
never NV|PV.
See the i_ variants of preinc, predec, postinc, postdec.
Since constants should not be changed in place, sv_inc is called,
which calls "Can't modify constant item in scalar assignment".
And they return IV with an integer arg, not UV by definition, so the
Sisyphus argument is also bogus.
The signedness overflow is well-defined, as all other participants in
that discussion understood well.
If you declare an integer constant, and do certain constant foldable
operations, it can be expected that the result will not be promoted to
an NV or changed to UV on overflow.
Not so if it's no constant, but a calculated value.
> Postfix increment and decrement will all return integers when given integer
> input. However, they can change the value to floating point for certain
> integer input. It would not be valid to assume that they can always be
> implemented as integers.
Sure. My case were only *constants*, not general integer arith.
>> such. This happened 2002.
>>
>> The simple fix would have been:
>> /* Certain arith ops should never be promoted to i_, even with
>> integer constants.
>> */
>> op.c:
>> @@ -2498,30 +2498,6 @@ Perl_fold_constants(pTHX_ register OP *o)
>> if (!(PL_hints& HINT_INTEGER)) {
>> if (type == OP_MODULO
>> || type == OP_DIVIDE
>> + || type == OP_MULTIPLY
>> || !(o->op_flags& OPf_KIDS))
>> {
>
> You're implying that I didn't check *that*. I can make it overflow with add
> too:
>
> $ /export/home/nwc10/Reference/5.005_03/bin/perl5.00503 -e 'my $n = 1127; my $int = 0.0 + 0x7FFFFFFF + ($n % 1126) + 0x7FFFFFFF; warn $int'
> -1 at -e line 1.
> $ /export/home/nwc10/Reference/5.005_03/bin/perl5.00503 -e 'my $n = 1127; my $int = 0.1 + 0x7FFFFFFF + ($n % 1126) + 0x7FFFFFFF; warn $int'
> 4294967295.1 at -e line 1.
>
> Your alternative suggestion is incomplete.
> Can you prove that it's impossible to overflow integers in any way?
I didn't say that. Of course in my languages I work with, more care is
taken with arith of any type (we have much more numeric types than
perl), type promotion, correctness and such.
perl is different that it explicitly says that it trades simplicity and
performance over correctness. It also promises that ints can always
carry native pointers, which makes language implementation harder. But
it's simplier that way.
If one wrote
use constant i => 2**32 + 1;
he got in IV, not a UV, NV or PV.
Only if he declares it as double he will get a double.
Or if he went the way through stringify the type would be
automatically promoted.
Sysiphus argument was signedness vs unsignedness which was confusing,
but you have been the only one in that discussion who understood our
constant folding behaviour as bug. Very interesting.
>> The more natural fix would have been to check the const numbers in
>> question and use i_ with low numbers, else the big ones.
>> I just did the simple fix first.
>> See http://github.com/rurban/perl/commits/i_opt
A line by line explanation for my patch, resp. the original behaviour.
Background:
We want to fold constants for o, which is not necessarily the current
op, the curop. Normally curop is the upper op, because fold_constants is
only used by newUNOP, newBINOP and ck_select.
The argument for newUNOP or BINOP (normally a conditional) maybe folded
to constant integer.
At first it does some simple checks and promotions if necessary
( a useful side-effect for the calling functions ).
...
Backout if the op has no f flag: OA_FOLDCONST - needs constant folding
Check some known ops and optionally backout.
Call the subtree do fold it (G_FAKINGEVAL) and check the returned value.
Here comes my patch to add again the optional intops optimizations.
That means that of the compiler detects a number as constant,
it can use the better i_ counterpart, which is guaranteed to
return an IV (or bool yes/no SPECIAL).
+ if (!(PL_opargs[type] & OA_OTHERINT))
+ return o;
We have no i_ counterpart for o, so back out.
+
+ if (!(PL_hints & HINT_INTEGER)) {
+ if (type == OP_MODULO
+ || type == OP_DIVIDE
+ || type == OP_MULTIPLY
+ || !(o->op_flags & OPf_KIDS))
+ {
+ return o;
+ }
We do not have use integer, so ignore future optimizations for
*, / and %.
/ and % are questionable, but that was the original state.
I still have to investigate why it was broken there.
I added *, because integer overflow for integer constants with * seems
to be confusing for some folks. Maybe users think << is better for
constant integer multiplication, but there are more usecases than just
mult. with 2.
+
+ for (curop = ((UNOP*)o)->op_first; curop; curop = curop->op_sibling) {
curop check:
Now check all arguments (possibly siblings or kids of o) for the
possibility to become intops.
Note that we want to promote the o to the i_ counterpart.
+ if (curop->op_type == OP_CONST) {
Back out if we explictily const the arg and the arg is no IV.
We do not int-ify non-ints.
+ if (SvIOK(((SVOP*)curop)->op_sv))
+ continue;
+ return o;
+ }
Back out if the current op does not return in IV.
OA_RETINTEGER <=> i in opflags.pl, all conditonals e.g. can be optimized.
So with a normal add/mult/... and a not-constant arg no promotion is done.
+ if (PL_opargs[curop->op_type] & OA_RETINTEGER)
+ continue;
+ return o;
+ }
Promote op to its i_ counterpart. This is a hack, but okay, and actually
quite clever. A shame that is was disabled.
Larry wrote that as a whole.
+ o->op_ppaddr = PL_ppaddr[++(o->op_type)];
+ }
+
> I don't agree. I don't think that it's safe. I can't *prove* that it's safe,
> in the general case. And my hunch is that the amount of time making range
> checks, and the end user cost of bugs caused by bad range checks, is going
> to be higher than the speed gains of any optimisations.
We have only constants here, so it's simple. The general case is slow of
course.
>> {
>> # The peephole optimiser is wrong to think that it can substitute intops
>> # in place of regular ops, because i_multiply can overflow.
>> # Bug reported by "Sisyphus"<kalinabears@hdc.com.au>
>> my $n = 1127;
>>
>> my $float = ($n % 1000) * 167772160.0;
>> tryeq 131, $float, 21307064320;
>>
>> # On a 32 bit machine, if the i_multiply op is used, you will
>> probably get
>> # -167772160. It's actually undefined behaviour, so anything may happen.
>> my $int = ($n % 1000) * 167772160;
>> tryeq 132, $int, 21307064320;
>> }
>>
>> My god, if he does integer arith without bigint but explicit int, he
>> will get ints which just overflow. Such is life. Only print promotes to NV.
>
> You seem to be misunderstanding how Perl is defined as working.
> Or I'm misunderstanding you.
Perl was defined that way, until you changed it.
I'm a couple of years older than you and used it a couple of years
longer, so maybe I'm more shocked than you, that int constants are now
promoted to doubles.
Doubles are written with a dot in the world I live in.
In lisp even with d suffix, because there are floats and long doubles
and bigfloats and ratios and bigrats and so on.
Others probably don't care because perl numbers are now magically
behaving like visual basic numbers and that's obviously a good thing.
> You're saying that it it is correct for the * operator to overflow if it's
> given two integers that happen to have particular values?
> (outside of the scope of use integer)
> Rather than generating the result of a floating-point multiply?
I didn't say that. I added that. But I left the others, because add
overflowing with constants is a nice and useful hack, same for shifts.
But most importantly, I didn't change the types for constant ints.
>> Very interesting is the reasoning of the removal of intop optimizations:
>> http://www.xray.mpe.mpg.de/mailing-lists/perl5-porters/2002-02/msg00301.html
>> This is baby constant folding + type promotion of course.
>> Every op which returns only int or which gets a constant integer arg,
>> can be promoted to an intop.
>>
>> When I have more time I check for ranges and put the other mod, divide
>> and multiply back in.
>
> And so addition will become buggy for some inputs, as demonstrated above?
Bugs are matter of definition. All other people in that discussion
considered it not as a bug, and me neither. Still not.
--
Reini Urban
Thread Previous
|
Thread Next