Bug 3918 – Parameter use before its use in an AndAnd expression with reals treats NaN as false

Status
RESOLVED
Resolution
FIXED
Severity
normal
Priority
P2
Component
dmd
Product
D
Version
D2
Platform
x86
OS
All
Creation time
2010-03-09T16:41:00Z
Last change time
2014-09-05T23:16:12Z
Keywords
pull, wrong-code
Assigned to
yebblies
Creator
aldonunez1
Blocks
3761

Comments

Comment #0 by aldonunez1 — 2010-03-09T16:41:26Z
NaN is supposed to be treated as true when implicitly converted to bool, because it's not the same as zero. But, in the following case, the NaN is treated as false. - optimized build - a variable of type real with value NaN - the variable is used in an AndAnd (&&) or OrOr (||) expression - the variable is used before the AndAnd (&&) or OrOr (||) expression I assume this happens anywhere a real is turned into a bool, but I haven't checked. Here's a sample program: import std.stdio; void Do( float t, real u ) { writeln( u ); writeln( t && u ); } void Do2( real t, float u ) { writeln( t ); writeln( t && u ); } void main( string[] args ) { Do( float.nan, real.nan ); Do2( real.nan, float.nan ); } In the function calls to both Do and Do2, the result is "false". It should be "true". Commenting out "writeln( u )" and "writeln( t )" also make the results "true". Looking at the assembly code, I noticed that the cause seems to be an instruction that's left out when the program is optimized. Unoptimized (right): 00403C0D fld tbyte ptr [esp+8] 00403C11 fldz 00403C13 fucompp 00403C15 fnstsw ax 00403C17 sahf 00403C18 jne 00403C20 00403C1A jp 00403C20 Optimized (wrong): 00403C1E fld tbyte ptr [esp+8] 00403C22 fldz 00403C24 fucompp 00403C26 fnstsw ax 00403C28 sahf 00403C29 jne 00403C2F The JP instruction is missing. As a result, we'll fall thru to where EAX is cleared, and the result turns into false.
Comment #1 by clugdbug — 2010-03-15T21:07:26Z
>NaN is supposed to be treated as true when implicitly converted to bool, because it's not the same as zero. That's surprising behaviour, I'd have expected that it needs to a non-zero value. I don't think implicit conversion float->bool should be legal at all, for exactly this reason. But obviously its behaviour should be unaffected by -O.
Comment #2 by aldonunez1 — 2010-03-15T21:49:32Z
Actually, I'm fine with NaN going either way, as long as it's consistent... and documented.
Comment #3 by clugdbug — 2010-08-02T00:12:09Z
I investigated this bug because I thought it might be related to the more difficult bug 4443. Unfortunately, they are unrelated. This one happens because the variable 'u' is recognised as a common sub expression (CSE). Then in cod3.c, jmpopcode(), around line 805, we see this code. The two lines marked with * mean that it just does a JNE, instead of a JNE/JP combination. When there's no CSE, the next return is used, "return XP|JNE", which adds the JP in. op = e->Eoper; * if (e->Ecount != e->Ecomsub) // comsubs just get Z bit set * return JNE; if (!OTrel(op)) // not relational operator { tym_t tymx = tybasic(e->Ety); if (tyfloating(tymx) && config.inline8087 && (tymx == TYldouble || tymx == TYildouble || tymx == TYcldouble || tymx == TYcdouble || tymx == TYcfloat || op == OPind)) { return XP|JNE; } return (op >= OPbt && op <= OPbts) ? JC : JNE; } ------------ How to fix this? (1) Duplicate the if(tyfloating) code in the first return, so that floating point CSEs still include a JP. But that penalizes the general case. (2) Don't detect if(x) as a CSE, when x is floating point. One way of doing this would be to change it into x!=0. (3) Entirely disallow if (x) for floating point, generating an error in the front end. Personally I think (3) is the best. I think making 'if (nan)' true leads to subtle bugs, and making it false also leads to subtle bugs.
Comment #4 by yebblies — 2012-10-28T13:30:53Z
(In reply to comment #3) > > Then in cod3.c, jmpopcode(), around line 805, we see this code. The two lines > marked with * mean that it just does a JNE, instead of a JNE/JP combination. > When there's no CSE, the next return is used, "return XP|JNE", which adds the > JP in. > I hit this same bit of code when investigating the xmm codegen bugs earlier this year and tried to fix it by moving the CSE if to under the fp block, and Walter told me that "floating point expressions are not common subexpressioned." Nasty.
Comment #5 by github-bugzilla — 2014-07-25T04:14:46Z
Commit pushed to master at https://github.com/D-Programming-Language/dmd https://github.com/D-Programming-Language/dmd/commit/031c94f25a0a25125730e4033a3e19e8b20bc861 Merge pull request #3811 from yebblies/issue3918 Issue 12820 - DMD can inline calls to functions that use alloca, allocating the memory in the caller function instead.
Comment #6 by yebblies — 2014-07-25T06:38:41Z
Whoops, I gave the branch the wrong number. This issue is unchanged.
Comment #7 by yebblies — 2014-07-25T08:54:52Z
Comment #8 by github-bugzilla — 2014-07-31T02:35:15Z
Commit pushed to 2.066 at https://github.com/D-Programming-Language/dmd https://github.com/D-Programming-Language/dmd/commit/aa2b31e10f5a5970e899de200b4b21d915a45ef4 Merge pull request #3811 from yebblies/issue3918 Issue 12820 - DMD can inline calls to functions that use alloca, allocating the memory in the caller function instead.
Comment #9 by github-bugzilla — 2014-08-22T08:05:01Z
Comment #10 by github-bugzilla — 2014-09-05T23:16:12Z
Commits pushed to master at https://github.com/D-Programming-Language/dmd https://github.com/D-Programming-Language/dmd/commit/d089a3a059915d84170b549de1fc2215eada3334 Fix Issue 3918 - Parameter use before its use in an AndAnd expression with reals treats NaN as false https://github.com/D-Programming-Language/dmd/commit/1ea942a60971472a09e6764a28ccfbfea1bdb501 Merge pull request #3812 from yebblies/issue3918 Issue 3918 - Parameter use before its use in an AndAnd expression with reals treats NaN as false