Implement comparison of numbers against null and proper strict comparison of numbers.
Created attachment 338605 [details] [diff] [review] Patch against tip. This patch breaks raytrace. Assertion failed: (rmask(rr) & FpRegs) != 0 (nanojit/Nativei386.cpp:770) Looks like a latent bug.
Assignee: general → gal
Attachment #338605 - Flags: review?(brendan)
Comment on attachment 338605 [details] [diff] [review] Patch against tip. hg diff -w should be possible (not if you are using mq, but you aren't ;-). That would be a lot easier to review -- usually people put the review request on the full diff but attach the -w for easier review right after the full patch to be applied. In the early CMP_STRICT then-clause, isn't cond = false wrong if negate? It seems to me you want cond = negate to match the generated code. /be
Wouldn't it be !negate then?
So "cond = !negate", instead of "cond = false" I mean.
Created attachment 338611 [details] [diff] [review] Brendan was right damn it.
Created attachment 338616 [details] [diff] [review] diff -w for patch, jstracer.cpp only Why is hg so broken? I was told to add this to my ~/.hgrc: [extdiff] # diff -w cmd.wdiff = diff opts.wdiff = -U 8 -p -w but `hg wdiff` produced one-line nonsense about directories differing. /be
Comment on attachment 338611 [details] [diff] [review] Brendan was right damn it. A comment in that new CMP_STRICT case about how the operator is === or !== only, negate is true for !==, and different types implies === is false (so cond = negate) would be nice. /be
Attachment #338611 - Flags: review?(brendan) → review+
IIRC mrbkap suggested the [extdiff] lines in comment 6. Cc'ing hg gurus in case hg has grown the right smarts to do diff -w out of the box. /be
Created attachment 338661 [details] [diff] [review] New patch, improved handling for constant conditional guards (which we don't want to emit.) David, please review since the original patch for the constant conditional guards is yours. I am checking that x is not constant instead of checking both sides of the conditional expression since if those are constant x would become constant too. This change is necessary since strict equality sometimes produces a constant false for two non-constant operands if the types differ (and the types are constant on the trace!).
(In reply to comment #6) > opts.wdiff = -U 8 -p -w > > but `hg wdiff` produced one-line nonsense about directories differing. That's not hg producing nonsense, it's your system diff. For my wdiff, I use: opts.wdiff = -wprNU 8 which also deals with new (and deleted, I think) files correctly. Note in particular the -r and -N options.
Status: NEW → ASSIGNED
OS: Mac OS X → All
Priority: -- → P1
Hardware: PC → All
Target Milestone: --- → mozilla1.9.1b1
mrbkap: thanks for the (better, IIRC you gave me the first tip :-P) diff advice. Why oh why doesn't hg support diff -w out of the box? Grumble. Patch looks great to me, marking in a sec; sorry for bugspam. /be
Attachment #338661 - Flags: review?(brendan) → review+
10 years ago
Attachment #338661 - Flags: review?(danderson) → review+
Status: ASSIGNED → RESOLVED
Last Resolved: 10 years ago
Resolution: --- → FIXED
test already in js1_8_1/trace/trace-test.js
Flags: in-testsuite- → in-testsuite+
You need to log in before you can comment on or make changes to this bug.