Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Inconsistent integer division behaviors between different targets. #3741

Closed
Atry opened this issue Jan 6, 2015 · 26 comments · Fixed by #11551
Closed

Inconsistent integer division behaviors between different targets. #3741

Atry opened this issue Jan 6, 2015 · 26 comments · Fixed by #11551
Assignees
Milestone

Comments

@Atry
Copy link
Contributor

Atry commented Jan 6, 2015

class Div
{
    public static function main()
    {
        var i = 1;
        var j = 0;
        var k:Int = Std.int(i / j);
        trace(k);
    }
}
$ haxe -x Div
Div.hx:8: 0
$ haxe -java java -main Div && java -jar java/Div.jar
haxelib run hxjava hxjava_build.txt --haxe-version 3200
javac.exe "-sourcepath" "src" "-d" "obj" "-g:none" "@cmd"
Exception in thread "main" java.lang.ArithmeticException: / by zero
        at haxe.root.Div.main(Unknown Source)
        at haxe.root.Div.main(Unknown Source)
@ncannasse
Copy link
Member

I think we should leave integer division by 0 unspecified.

@nadako
Copy link
Member

nadako commented Jan 7, 2015

Yeah, but couldn't neko throw an exception there as well?

@Atry
Copy link
Contributor Author

Atry commented Jan 7, 2015

class Div
{
    public static function main()
    {
        var i = 1;
        var j = 0;
        var k:Int = (function (f:Float):Int return Std.int(f))(i / j);
        trace(k);
    }
}
$ haxe --no-opt --no-inline -java java -main Div && java -jar java/Div.jar
haxelib run hxjava hxjava_build.txt --haxe-version 3200
javac.exe "-sourcepath" "src" "-d" "obj" "-g:none" "@cmd"
Div.hx:8: 2147483647
$ haxe -java java -main Div && java -jar java/Div.jar
haxelib run hxjava hxjava_build.txt --haxe-version 3200
javac.exe "-sourcepath" "src" "-d" "obj" "-g:none" "@cmd"
Exception in thread "main" java.lang.ArithmeticException: / by zero
        at haxe.root.Div.main(Unknown Source)
        at haxe.root.Div.main(Unknown Source)
$ haxe -x Div
Div.hx:8: 0

How about the different behaviors of Std.Int?

@Atry
Copy link
Contributor Author

Atry commented Jan 7, 2015

class StdInt
{
    public static function main()
    {
        trace(Std.int(1.0 / 0.0));
        trace(Std.int(1 / 0));
    }
}
$ haxe -x StdInt
StdInt.hx:5: 0
StdInt.hx:6: 0
$ haxe -java java -main StdInt && java -jar java/StdInt.jar
haxelib run hxjava hxjava_build.txt --haxe-version 3200
javac.exe "-sourcepath" "src" "-d" "obj" "-g:none" "@cmd"
StdInt.hx:5: 2147483647
Exception in thread "main" java.lang.ArithmeticException: / by zero
        at haxe.root.StdInt.main(Unknown Source)
        at haxe.root.StdInt.main(Unknown Source)

@Simn
Copy link
Member

Simn commented Jan 7, 2015

Any operation on undefined yields undefined.

@Atry
Copy link
Contributor Author

Atry commented Jan 7, 2015

Is Std.int(1.0 / 0.0) undefined?

@Simn
Copy link
Member

Simn commented Jan 7, 2015

Yes because 1.0 / 0.0 is undefined.

@Atry
Copy link
Contributor Author

Atry commented Jan 7, 2015

How about Std.int(Math.POSITIVE_INFINITY)?

@Simn
Copy link
Member

Simn commented Jan 7, 2015

Ifxis outside of the signed Int32 range, or is NaN, NEGATIVE_INFINITY or POSITIVE_INFINITY, the result is unspecified.

@ncannasse
Copy link
Member

Uhm actually I was mistaken. In Haxe, there is no integer division since the result of a division is always a Float. So it means that we shouldn't have an error here, only an unspecified result equal to Std.int(+INF). @waneck I guess that comes from an optimization of Std.int(A/B)

@waneck
Copy link
Member

waneck commented Jan 12, 2015

Yes; Both C# and Java optimize Std.int(a/b) to use integer division. I don't really understand why we need to specify that it doesn't throw an exception - if Std.int(someValue / 0) is already unspecified and presents different values - depending on the target.
If you'd rather fail silently, the bad thing is that we won't be able to optimize to use integer division.

@Simn
Copy link
Member

Simn commented Jan 12, 2015

Wasn't Std.int(a/b) greenlighted to be optimized as integer division anyway? I remember a discussion about that, though I think it was in the context of hxcpp.

@waneck
Copy link
Member

waneck commented Jan 12, 2015

I guess we've forgotten about division by 0. It indeed throws an exception in this case.
I still don't understand why we'd specify that it doesn't throw an exception - if the actual value is unspecified anyway.

@ncannasse
Copy link
Member

@waneck the main issue I can see is that will give different results:

trace( Std.int( 1 / 0 ) );
var x : Float = 1 / 0;
trace( Std.int(x) );

Which IMHO is not a good thing. We could still use integer division but we have to check for 0 I guess.
CC @hughsando

@hughsando
Copy link
Member

Let's just add Math.idiv and be done with it.

@waneck
Copy link
Member

waneck commented Jan 13, 2015

Yeah, I guess Math.idiv is the way to go

@Atry
Copy link
Contributor Author

Atry commented Jan 13, 2015

Does every Haxe target need to behave like AVM2 or dynamic-typed platforms?

Haxe is a statically-typed language.

I think we could change operator / as C++/Java/C# behavior in Haxe 4.

@Atry
Copy link
Contributor Author

Atry commented Jan 13, 2015

Flash's operator / is quite odd, to keep ECMAScript compatibility. I don't think Haxe needs ECMAScript compatibility.

@ncannasse
Copy link
Member

We should still optimize Std.int for people that don't know about Math.idiv I think

@hughsando
Copy link
Member

I think you have just shown that this can't be done.

On Wed, Jan 14, 2015 at 4:51 AM, Nicolas Cannasse notifications@github.com
wrote:

We should still optimize Std.int for people that don't know about
Math.idiv I think


Reply to this email directly or view it on GitHub
#3741 (comment)
.

@Simn
Copy link
Member

Simn commented Jan 14, 2015

That's my understanding as well.

@ncannasse
Copy link
Member

Sorry if I was not clear. What I meant was to optimize Std.int but still check for 0.

@ncannasse
Copy link
Member

and in that case return the same result as would have Std.int(+INF)

@hughsando
Copy link
Member

I would like to see some profile data before introducing an optimization
that trades a conditional branch for a floating point op.

On Wed, Jan 14, 2015 at 10:10 PM, Nicolas Cannasse <notifications@github.com

wrote:

and in that case return the same result as would have Std.int(+INF)


Reply to this email directly or view it on GitHub
#3741 (comment)
.

@Simn
Copy link
Member

Simn commented Jan 14, 2015

Yes that sounds like a potential pessimization more than anything else.

@ncannasse
Copy link
Member

@hughsando if there is no allocation then it's a micro optimization that is not worth it I think. If we need to allocate a float for GC boxing that's another story.

@Simn Simn added this to the 3.3 milestone Feb 22, 2015
@Simn Simn modified the milestones: 3.3.0-rc1, 4.0 Feb 23, 2016
@Simn Simn modified the milestones: 4.0, 3.3.0-rc1 Feb 23, 2016
@Simn Simn modified the milestones: Release 4.0, Design Apr 17, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants