[sh4] Fix floating point comparisons in baseline JIT.
authorcommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 31 May 2013 18:22:05 +0000 (18:22 +0000)
committercommit-queue@webkit.org <commit-queue@webkit.org@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Fri, 31 May 2013 18:22:05 +0000 (18:22 +0000)
https://bugs.webkit.org/show_bug.cgi?id=117066.

Patch by Julien Brianceau <jbrianceau@nds.com> on 2013-05-31
Reviewed by Oliver Hunt.

Current implementation of branchDouble function in baseline JIT is wrong
for some conditions and overkill for others. For instance:
- With DoubleGreaterThanOrEqual condition, branch will be taken if either
  operand is NaN with current implementation whereras it should not.
- With DoubleNotEqualOrUnordered condition, performed NaN checks are
  useless (because comparison result is false if either operand is NaN).

* assembler/MacroAssemblerSH4.h:
(JSC::MacroAssemblerSH4::branchDouble):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@151032 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/JavaScriptCore/ChangeLog
Source/JavaScriptCore/assembler/MacroAssemblerSH4.h

index 7f5b97b..6aff436 100644 (file)
@@ -1,5 +1,22 @@
 2013-05-31  Julien Brianceau  <jbrianceau@nds.com>
 
+        [sh4] Fix floating point comparisons in baseline JIT.
+        https://bugs.webkit.org/show_bug.cgi?id=117066.
+
+        Reviewed by Oliver Hunt.
+
+        Current implementation of branchDouble function in baseline JIT is wrong
+        for some conditions and overkill for others. For instance:
+        - With DoubleGreaterThanOrEqual condition, branch will be taken if either
+          operand is NaN with current implementation whereras it should not.
+        - With DoubleNotEqualOrUnordered condition, performed NaN checks are
+          useless (because comparison result is false if either operand is NaN).
+
+        * assembler/MacroAssemblerSH4.h:
+        (JSC::MacroAssemblerSH4::branchDouble):
+
+2013-05-31  Julien Brianceau  <jbrianceau@nds.com>
+
         [sh4] Fix double floating point transfer in baseline JIT.
         https://bugs.webkit.org/show_bug.cgi?id=117054
 
index 3a090fc..7b20382 100644 (file)
@@ -1250,20 +1250,13 @@ public:
         }
 
         if (cond == DoubleNotEqual) {
-            RegisterID scr = claimScratch();
             JumpList end;
-            m_assembler.loadConstant(0x7fbfffff, scratchReg3);
-            m_assembler.dcnvds(right);
-            m_assembler.stsfpulReg(scr);
-            m_assembler.cmplRegReg(scratchReg3, scr, SH4Condition(Equal));
+            m_assembler.dcmppeq(left, left);
             m_assembler.ensureSpace(m_assembler.maxInstructionSize + 22, sizeof(uint32_t));
-            end.append(Jump(m_assembler.je(), SH4Assembler::JumpNear));
-            m_assembler.dcnvds(left);
-            m_assembler.stsfpulReg(scr);
-            m_assembler.cmplRegReg(scratchReg3, scr, SH4Condition(Equal));
-            end.append(Jump(m_assembler.je(), SH4Assembler::JumpNear));
+            end.append(Jump(m_assembler.jne(), SH4Assembler::JumpNear));
+            m_assembler.dcmppeq(right, right);
+            end.append(Jump(m_assembler.jne(), SH4Assembler::JumpNear));
             m_assembler.dcmppeq(right, left);
-            releaseScratch(scr);
             Jump m_jump = branchFalse();
             end.link(this);
             return m_jump;
@@ -1275,8 +1268,16 @@ public:
         }
 
         if (cond == DoubleGreaterThanOrEqual) {
+            JumpList end;
+            m_assembler.dcmppeq(left, left);
+            m_assembler.ensureSpace(m_assembler.maxInstructionSize + 22, sizeof(uint32_t));
+            end.append(Jump(m_assembler.jne(), SH4Assembler::JumpNear));
+            m_assembler.dcmppeq(right, right);
+            end.append(Jump(m_assembler.jne(), SH4Assembler::JumpNear));
             m_assembler.dcmppgt(left, right);
-            return branchFalse();
+            Jump m_jump = branchFalse();
+            end.link(this);
+            return m_jump;
         }
 
         if (cond == DoubleLessThan) {
@@ -1285,134 +1286,73 @@ public:
         }
 
         if (cond == DoubleLessThanOrEqual) {
+            JumpList end;
+            m_assembler.dcmppeq(left, left);
+            m_assembler.ensureSpace(m_assembler.maxInstructionSize + 22, sizeof(uint32_t));
+            end.append(Jump(m_assembler.jne(), SH4Assembler::JumpNear));
+            m_assembler.dcmppeq(right, right);
+            end.append(Jump(m_assembler.jne(), SH4Assembler::JumpNear));
             m_assembler.dcmppgt(right, left);
-            return branchFalse();
+            Jump m_jump = branchFalse();
+            end.link(this);
+            return m_jump;
         }
 
         if (cond == DoubleEqualOrUnordered) {
-            RegisterID scr = claimScratch();
-            JumpList end;
-            m_assembler.loadConstant(0x7fbfffff, scratchReg3);
-            m_assembler.dcnvds(right);
-            m_assembler.stsfpulReg(scr);
-            m_assembler.cmplRegReg(scratchReg3, scr, SH4Condition(Equal));
+            JumpList takeBranch;
+            m_assembler.dcmppeq(left, left);
             m_assembler.ensureSpace(m_assembler.maxInstructionSize + 22, sizeof(uint32_t));
-            end.append(Jump(m_assembler.je(), SH4Assembler::JumpNear));
-            m_assembler.dcnvds(left);
-            m_assembler.stsfpulReg(scr);
-            m_assembler.cmplRegReg(scratchReg3, scr, SH4Condition(Equal));
-            end.append(Jump(m_assembler.je(), SH4Assembler::JumpNear));
+            takeBranch.append(Jump(m_assembler.jne(), SH4Assembler::JumpNear));
+            m_assembler.dcmppeq(right, right);
+            takeBranch.append(Jump(m_assembler.jne(), SH4Assembler::JumpNear));
             m_assembler.dcmppeq(left, right);
             Jump m_jump = Jump(m_assembler.je());
-            end.link(this);
-            m_assembler.extraInstrForBranch(scr);
-            releaseScratch(scr);
+            takeBranch.link(this);
+            m_assembler.extraInstrForBranch(scratchReg3);
             return m_jump;
         }
 
         if (cond == DoubleGreaterThanOrUnordered) {
-            RegisterID scr = claimScratch();
-            JumpList end;
-            m_assembler.loadConstant(0x7fbfffff, scratchReg3);
-            m_assembler.dcnvds(right);
-            m_assembler.stsfpulReg(scr);
-            m_assembler.cmplRegReg(scratchReg3, scr, SH4Condition(Equal));
+            JumpList takeBranch;
+            m_assembler.dcmppeq(left, left);
             m_assembler.ensureSpace(m_assembler.maxInstructionSize + 22, sizeof(uint32_t));
-            end.append(Jump(m_assembler.je(), SH4Assembler::JumpNear));
-            m_assembler.dcnvds(left);
-            m_assembler.stsfpulReg(scr);
-            m_assembler.cmplRegReg(scratchReg3, scr, SH4Condition(Equal));
-            end.append(Jump(m_assembler.je(), SH4Assembler::JumpNear));
+            takeBranch.append(Jump(m_assembler.jne(), SH4Assembler::JumpNear));
+            m_assembler.dcmppeq(right, right);
+            takeBranch.append(Jump(m_assembler.jne(), SH4Assembler::JumpNear));
             m_assembler.dcmppgt(right, left);
             Jump m_jump = Jump(m_assembler.je());
-            end.link(this);
-            m_assembler.extraInstrForBranch(scr);
-            releaseScratch(scr);
+            takeBranch.link(this);
+            m_assembler.extraInstrForBranch(scratchReg3);
             return m_jump;
         }
 
         if (cond == DoubleGreaterThanOrEqualOrUnordered) {
-            RegisterID scr = claimScratch();
-            JumpList end;
-            m_assembler.loadConstant(0x7fbfffff, scratchReg3);
-            m_assembler.dcnvds(right);
-            m_assembler.stsfpulReg(scr);
-            m_assembler.cmplRegReg(scratchReg3, scr, SH4Condition(Equal));
-            m_assembler.ensureSpace(m_assembler.maxInstructionSize + 22, sizeof(uint32_t));
-            end.append(Jump(m_assembler.je(), SH4Assembler::JumpNear));
-            m_assembler.dcnvds(left);
-            m_assembler.stsfpulReg(scr);
-            m_assembler.cmplRegReg(scratchReg3, scr, SH4Condition(Equal));
-            end.append(Jump(m_assembler.je(), SH4Assembler::JumpNear));
             m_assembler.dcmppgt(left, right);
-            Jump m_jump = Jump(m_assembler.jne());
-            end.link(this);
-            m_assembler.extraInstrForBranch(scr);
-            releaseScratch(scr);
-            return m_jump;
+            return branchFalse();
         }
 
         if (cond == DoubleLessThanOrUnordered) {
-            RegisterID scr = claimScratch();
-            JumpList end;
-            m_assembler.loadConstant(0x7fbfffff, scratchReg3);
-            m_assembler.dcnvds(right);
-            m_assembler.stsfpulReg(scr);
-            m_assembler.cmplRegReg(scratchReg3, scr, SH4Condition(Equal));
+            JumpList takeBranch;
+            m_assembler.dcmppeq(left, left);
             m_assembler.ensureSpace(m_assembler.maxInstructionSize + 22, sizeof(uint32_t));
-            end.append(Jump(m_assembler.je(), SH4Assembler::JumpNear));
-            m_assembler.dcnvds(left);
-            m_assembler.stsfpulReg(scr);
-            m_assembler.cmplRegReg(scratchReg3, scr, SH4Condition(Equal));
-            end.append(Jump(m_assembler.je(), SH4Assembler::JumpNear));
+            takeBranch.append(Jump(m_assembler.jne(), SH4Assembler::JumpNear));
+            m_assembler.dcmppeq(right, right);
+            takeBranch.append(Jump(m_assembler.jne(), SH4Assembler::JumpNear));
             m_assembler.dcmppgt(left, right);
             Jump m_jump = Jump(m_assembler.je());
-            end.link(this);
-            m_assembler.extraInstrForBranch(scr);
-            releaseScratch(scr);
+            takeBranch.link(this);
+            m_assembler.extraInstrForBranch(scratchReg3);
             return m_jump;
         }
 
         if (cond == DoubleLessThanOrEqualOrUnordered) {
-            RegisterID scr = claimScratch();
-            JumpList end;
-            m_assembler.loadConstant(0x7fbfffff, scratchReg3);
-            m_assembler.dcnvds(right);
-            m_assembler.stsfpulReg(scr);
-            m_assembler.cmplRegReg(scratchReg3, scr, SH4Condition(Equal));
-            m_assembler.ensureSpace(m_assembler.maxInstructionSize + 22, sizeof(uint32_t));
-            end.append(Jump(m_assembler.je(), SH4Assembler::JumpNear));
-            m_assembler.dcnvds(left);
-            m_assembler.stsfpulReg(scr);
-            m_assembler.cmplRegReg(scratchReg3, scr, SH4Condition(Equal));
-            end.append(Jump(m_assembler.je(), SH4Assembler::JumpNear));
             m_assembler.dcmppgt(right, left);
-            Jump m_jump = Jump(m_assembler.jne());
-            end.link(this);
-            m_assembler.extraInstrForBranch(scr);
-            releaseScratch(scr);
-            return m_jump;
+            return branchFalse();
         }
 
         ASSERT(cond == DoubleNotEqualOrUnordered);
-        RegisterID scr = claimScratch();
-        JumpList end;
-        m_assembler.loadConstant(0x7fbfffff, scratchReg3);
-        m_assembler.dcnvds(right);
-        m_assembler.stsfpulReg(scr);
-        m_assembler.cmplRegReg(scratchReg3, scr, SH4Condition(Equal));
-        m_assembler.ensureSpace(m_assembler.maxInstructionSize + 22, sizeof(uint32_t));
-        end.append(Jump(m_assembler.je(), SH4Assembler::JumpNear));
-        m_assembler.dcnvds(left);
-        m_assembler.stsfpulReg(scr);
-        m_assembler.cmplRegReg(scratchReg3, scr, SH4Condition(Equal));
-        end.append(Jump(m_assembler.je(), SH4Assembler::JumpNear));
         m_assembler.dcmppeq(right, left);
-        Jump m_jump = Jump(m_assembler.jne());
-        end.link(this);
-        m_assembler.extraInstrForBranch(scr);
-        releaseScratch(scr);
-        return m_jump;
+        return branchFalse();
     }
 
     Jump branchTrue()