Parrot: Ticket #1720: fdiv_i_i_i and fdiv_i_i ops don't work correctly.
http://trac.parrot.org/parrot/ticket/1720
<p>
fdiv is documented to be flooring division: "The result is the floor() of the division i.e. the next whole integer towards -inf."
</p>
<p>
The num variants of fdiv correctly perform flooring division.
</p>
<p>
The int variants of fdiv floor the result of C integer division of the arguments, and C integer division truncates towards zero. Therefore, fdiv_i_i_i is currently equivalent to div_i_i_i.
</p>
en-usParrothttp://trac.parrot.org/parrot/chrome/site/parrot_logo.png
http://trac.parrot.org/parrot/ticket/1720
Trac 0.11.7tcurtisTue, 27 Jul 2010 21:33:56 GMTcomment added; attachment set
http://trac.parrot.org/parrot/ticket/1720
http://trac.parrot.org/parrot/ticket/1720
<ul>
<li><strong>attachment</strong>
set to <em>intfdiv.patch</em>
</li>
</ul>
<p>
An as-yet-untested patch that SHOULD fix the problem.
</p>
TicketkurahaupoWed, 28 Jul 2010 05:13:14 GMTcomment added
http://trac.parrot.org/parrot/ticket/1720#comment:1
http://trac.parrot.org/parrot/ticket/1720#comment:1
<pre class="wiki">On Tue, 27 Jul 2010, Parrot wrote:
> #1720: fdiv_i_i_i and fdiv_i_i ops don't work correctly.
> ---------------------+------------------------------------------------------
> Reporter: tcurtis | Owner:
> Type: bug | Status: new
> Priority: normal | Milestone:
> Component: core | Version: 2.6.0
> Severity: low | Keywords:
> Lang: | Patch:
> Platform: |
> ---------------------+------------------------------------------------------
> fdiv is documented to be flooring division: "The result is the floor() of
> the division i.e. the next whole integer towards -inf."
>
> The num variants of fdiv correctly perform flooring division.
>
> The int variants of fdiv floor the result of C integer division of the
> arguments, and C integer division truncates towards zero. Therefore,
> fdiv_i_i_i is currently equivalent to div_i_i_i.
This will work, although it's horribly inefficient:
Index: src/ops/math.ops
===================================================================
--- src/ops/math.ops (revision 47439)
+++ src/ops/math.ops (working copy)
@@ -289,7 +289,7 @@
=cut
inline op fdiv(inout INT, in INT) :base_core {
- INTVAL den = $2;
+ FLOATVAL den = $2;
FLOATVAL f;
if (den == 0) {
@@ -327,7 +327,7 @@
}
inline op fdiv(out INT, in INT, in INT) :base_core {
- INTVAL den = $3;
+ FLOATVAL den = $3;
FLOATVAL f;
if (den == 0) {
</pre>
TicketNotFoundThu, 29 Jul 2010 17:18:36 GMTcomment added; owner set; status changed
http://trac.parrot.org/parrot/ticket/1720#comment:2
http://trac.parrot.org/parrot/ticket/1720#comment:2
<ul>
<li><strong>status</strong>
changed from <em>new</em> to <em>assigned</em>
</li>
<li><strong>owner</strong>
set to <em>NotFound</em>
</li>
</ul>
TicketNotFoundThu, 29 Jul 2010 17:24:05 GMTcomment added; resolution set; status changed
http://trac.parrot.org/parrot/ticket/1720#comment:3
http://trac.parrot.org/parrot/ticket/1720#comment:3
<ul>
<li><strong>status</strong>
changed from <em>assigned</em> to <em>closed</em>
</li>
<li><strong>resolution</strong>
set to <em>fixed</em>
</li>
</ul>
<p>
Fixed and test added in r48214
</p>
TicketkurahaupoThu, 03 Nov 2011 10:26:44 GMTresolution deleted; comment added; status changed
http://trac.parrot.org/parrot/ticket/1720#comment:4
http://trac.parrot.org/parrot/ticket/1720#comment:4
<ul>
<li><strong>status</strong>
changed from <em>closed</em> to <em>reopened</em>
</li>
<li><strong>resolution</strong>
<em>fixed</em> deleted
</li>
</ul>
<p>
The given fix imposes an unreasonable run-time overhead of converting divisors from integer to float, and then converting the FLOATVAL remainder back to an INTVAL.
</p>
<p>
A more efficient approach would be to use the native integer operations, but check that the result has the desired properties:
</p>
<pre class="wiki"> INTVAL dividend = $1;
INTVAL divisor = $2;
INTVAL quotient = dividend / divisor;
if ( ( dividend - quotient * divisor ^ divisor ) < 0 ) {
quotient--;
}
</pre><p>
Of course, neither deals with integer underflow of <tt>((-MAX_INT-1) / -1)</tt>
</p>
TicketkurahaupoThu, 03 Nov 2011 11:10:55 GMTcomment added
http://trac.parrot.org/parrot/ticket/1720#comment:5
http://trac.parrot.org/parrot/ticket/1720#comment:5
<pre class="wiki">
The quick fix is fairly simple: change the declaration of the "den"
temporary from INTVAL to FLOATVAL. However that imposes an unreasonable
run-time overhead of converting divisors from integer to float, and then
converting the FLOATVAL remainder back to an INTVAL.
A better approach would be to use the native integer operations, but
check that the result has the desired properties:
INTVAL dividend = $1;
INTVAL divisor = $2;
INTVAL quotient = dividend / divisor;
if ( ( dividend - quotient * divisor ^ divisor ) < 0 ) {
quotient--;
}
Generally one wants to choose the handling negative numbers in truncating
division based on the definition of the remainder. In all cases the absolute
value of the remainder must be less than the absolute value of the divisor,
and but then one would choose either:
(A) the sign of the remainder matches the sign of the dividend; or
(B) the sign of the remainder matches the sign of the divisor.
Then round the quotient in the direction that ensures that
quotient = (dividend - remainder) รท divisor
Type (A) is the usual choice for implementing in hardware (and thus is
what the "div" operator provides), and I gather the intention is that
type (B) is what the "fdiv" operator should produce.
x y x/y x div y, x mod y
(A) (B)
+7 +5 +1.4 +1,+2 +1,+2
-7 +5 -1.4 -1,-2 -2,+3
+7 -5 -1.4 -1,+2 -2,-3
-7 -5 +1.4 +1,-2 +1,-2
On Tue, 27 Jul 2010, Parrot wrote:
> fdiv is documented to be flooring division: "The result is the floor() of
> the division i.e. the next whole integer towards -inf."
>
> The num variants of fdiv correctly perform flooring division.
>
> The int variants of fdiv floor the result of C integer division of the
> arguments, and C integer division truncates towards zero. Therefore,
> fdiv_i_i_i is currently equivalent to div_i_i_i.
</pre>
TicketbacekSun, 20 Nov 2011 09:58:18 GMTcomment added; resolution set; status changed
http://trac.parrot.org/parrot/ticket/1720#comment:6
http://trac.parrot.org/parrot/ticket/1720#comment:6
<ul>
<li><strong>status</strong>
changed from <em>reopened</em> to <em>closed</em>
</li>
<li><strong>resolution</strong>
set to <em>fixed</em>
</li>
</ul>
<p>
Hello.
</p>
<p>
Parrot is so far-far away in terms of speed to worry about int-to-float-to-int conversion overhead. Mark ticket as fixed.
</p>
<p>
--
Bacek
</p>
Ticket