Changeset 39061 in webkit for trunk/JavaScriptCore

Timestamp:

Dec 5, 2008, 7:25:23 PM (16 years ago)

Author:

[email protected]

Message:

2008-12-05 Gavin Barraclough <​[email protected]>

Reviewed by Oliver Hunt.

Ordering of branch operands in MacroAssembler in unnecessarily inconsistent.

je, jg etc take an immediate operand as the second argument, but for the
equality branches (je, jne) the immediate operand was the first argument. This
was unnecessarily inconsistent. Change je, jne methods to take the immediate
as the second argument.

​https://bugs.webkit.org/show_bug.cgi?id=22703

• assembler/MacroAssembler.h: (JSC::MacroAssembler::je32): (JSC::MacroAssembler::jne32):
• jit/JIT.cpp: (JSC::JIT::compileOpStrictEq):
• wrec/WRECGenerator.cpp: (JSC::WREC::Generator::generateEnter): (JSC::WREC::Generator::generateNonGreedyQuantifier): (JSC::WREC::Generator::generateGreedyQuantifier): (JSC::WREC::Generator::generatePatternCharacterPair): (JSC::WREC::Generator::generatePatternCharacter): (JSC::WREC::Generator::generateCharacterClassInvertedRange): (JSC::WREC::Generator::generateCharacterClassInverted): (JSC::WREC::Generator::generateAssertionBOL): (JSC::WREC::Generator::generateAssertionWordBoundary):

Location:

trunk/JavaScriptCore

Files:

• ChangeLog (modified) (1 diff)
• assembler/MacroAssembler.h (modified) (4 diffs)
• jit/JIT.cpp (modified) (2 diffs)
• wrec/WRECGenerator.cpp (modified) (14 diffs)

trunk/JavaScriptCore/ChangeLog

@@ +1 @@
+2008-12-05  Gavin Barraclough  <[email protected]>
+
+        Reviewed by Oliver Hunt.
+
+        Ordering of branch operands in MacroAssembler in unnecessarily  inconsistent.
+
+        je, jg etc take an immediate operand as the second argument, but for the
+        equality branches (je, jne) the immediate operand was the first argument.  This
+        was unnecessarily inconsistent.  Change je, jne methods to take the immediate
+        as the second argument.
+
+        https://bugs.webkit.org/show_bug.cgi?id=22703
+
+        * assembler/MacroAssembler.h:
+        (JSC::MacroAssembler::je32):
+        (JSC::MacroAssembler::jne32):
+        * jit/JIT.cpp:
+        (JSC::JIT::compileOpStrictEq):
+        * wrec/WRECGenerator.cpp:
+        (JSC::WREC::Generator::generateEnter):
+        (JSC::WREC::Generator::generateNonGreedyQuantifier):
+        (JSC::WREC::Generator::generateGreedyQuantifier):
+        (JSC::WREC::Generator::generatePatternCharacterPair):
+        (JSC::WREC::Generator::generatePatternCharacter):
+        (JSC::WREC::Generator::generateCharacterClassInvertedRange):
+        (JSC::WREC::Generator::generateCharacterClassInverted):
+        (JSC::WREC::Generator::generateAssertionBOL):
+        (JSC::WREC::Generator::generateAssertionWordBoundary):
+
 2008-12-05  Gavin Barraclough  <[email protected]>
 

trunk/JavaScriptCore/assembler/MacroAssembler.h

@@ -579 +579 @@
     }
     
-    Jump je32(Imm32 imm, RegisterID reg)
+    Jump je32(RegisterID reg, Imm32 imm)
     {
         compareImm32ForBranchEquality(reg, imm.m_value);
@@ -651 +651 @@
     }
 
-    Jump jne32(Imm32 imm, RegisterID reg)
+    Jump jne32(RegisterID reg, Imm32 imm)
     {
         compareImm32ForBranchEquality(reg, imm.m_value);
@@ -703 +703 @@
     //     jne32(reg1, reg2).linkTo(topOfLoop);
 
-    void je32(Imm32 imm, RegisterID op2, Label target)
-    {
-        je32(imm, op2).linkTo(target, this);
+    void je32(RegisterID op1, Imm32 imm, Label target)
+    {
+        je32(op1, imm).linkTo(target, this);
     }
 
@@ -728 +728 @@
     }
 
-    void jne32(Imm32 imm, RegisterID op2, Label target)
-    {
-        jne32(imm, op2).linkTo(target, this);
+    void jne32(RegisterID op1, Imm32 imm, Label target)
+    {
+        jne32(op1, imm).linkTo(target, this);
     }
 

trunk/JavaScriptCore/jit/JIT.cpp

@@ -236 +236 @@
     firstNotImmediate.link(this);
     emitJumpSlowCaseIfJSCell(X86::edx, i);
-    m_slowCases.append(SlowCaseEntry(je32(Imm32(asInteger(JSImmediate::zeroImmediate())), X86::edx), i));
+    m_slowCases.append(SlowCaseEntry(je32(X86::edx, Imm32(asInteger(JSImmediate::zeroImmediate()))), i));
     Jump firstWasNotImmediate = jump();
 
@@ -242 +242 @@
     // If eax is 0 jump to a slow case, otherwise these values are not equal.
     secondNotImmediate.link(this);
-    m_slowCases.append(SlowCaseEntry(je32(Imm32(asInteger(JSImmediate::zeroImmediate())), X86::eax), i));
+    m_slowCases.append(SlowCaseEntry(je32(X86::eax, Imm32(asInteger(JSImmediate::zeroImmediate()))), i));
 
     // We get here if the two values are different immediates, or one is 0 and the other is a JSCell.

trunk/JavaScriptCore/wrec/WRECGenerator.cpp

@@ -71 +71 @@
 #ifndef NDEBUG
     // ASSERT that the output register is not null.
-    Jump outputNotNull = jne32(Imm32(0), output);
+    Jump outputNotNull = jne32(output, Imm32(0));
     breakpoint();
     outputNotNull.link(this);
@@ -182 +182 @@
     // (3.2) if there is a limit, and we have reached it, game over. 
     if (max != Quantifier::noMaxSpecified) {
-        je32(Imm32(max), repeatCount, quantifierFailed);
+        je32(repeatCount, Imm32(max), quantifierFailed);
     }
 
@@ -236 +236 @@
         doneReadingAtoms.append(jump());
     else {
-        jne32(Imm32(max), repeatCount, readAnAtom);
+        jne32(repeatCount, Imm32(max), readAnAtom);
         doneReadingAtoms.append(jump());
     }
@@ -322 +322 @@
     int pair = ch1 | (ch2 << 16);
 
-    failures.append(jne32(Imm32(pair), character));
+    failures.append(jne32(character, Imm32(pair)));
     return true;
 }
@@ -344 +344 @@
         } else if (!isASCII(ch) && ((lower = Unicode::toLower(ch)) != (upper = Unicode::toUpper(ch)))) {
             // handle unicode case sentitive characters - branch to success on upper
-            isUpper = je32(Imm32(upper), character);
+            isUpper = je32(character, Imm32(upper));
             hasUpper = true;
             ch = lower;
@@ -351 +351 @@
     
     // checks for ch, or lower case version of ch, if insensitive
-    failures.append(jne32(Imm32((unsigned short)ch), character));
+    failures.append(jne32(character, Imm32((unsigned short)ch)));
 
     if (m_parser.ignoreCase() && hasUpper) {
@@ -380 +380 @@
             
             do {
-                matchDest.append(je32(Imm32((unsigned short)matches[*matchIndex]), character));
+                matchDest.append(je32(character, Imm32((unsigned short)matches[*matchIndex])));
                 ++*matchIndex;
             } while ((*matchIndex < matchCount) && (matches[*matchIndex] < lo));
@@ -418 +418 @@
             for (unsigned i = 0; i < charClass.numMatchesUnicode; ++i) {
                 UChar ch = charClass.matchesUnicode[i];
-                matchDest.append(je32(Imm32(ch), character));
+                matchDest.append(je32(character, Imm32(ch)));
             }
         }
@@ -442 +442 @@
         generateCharacterClassInvertedRange(failures, matchDest, charClass.ranges, charClass.numRanges, &matchIndex, charClass.matches, charClass.numMatches);
         while (matchIndex < charClass.numMatches)
-            matchDest.append(je32(Imm32((unsigned short)charClass.matches[matchIndex++]), character));
+            matchDest.append(je32(character, Imm32((unsigned short)charClass.matches[matchIndex++])));
 
         failures.link(this);
@@ -459 +459 @@
                     continue;
             }
-            matchDest.append(je32(Imm32((unsigned short)ch), character));
+            matchDest.append(je32(character, Imm32((unsigned short)ch)));
         }
 
@@ -465 +465 @@
             or32(Imm32(32), character);
             for (unsigned i = 0; i < countAZaz; ++i)
-                matchDest.append(je32(Imm32(matchesAZaz[i]), character));
+                matchDest.append(je32(character, Imm32(matchesAZaz[i])));
         }
     }
@@ -549 +549 @@
 
         // begin of input == success
-        previousIsNewline.append(je32(Imm32(0), index));
+        previousIsNewline.append(je32(index, Imm32(0)));
 
         // now check prev char against newline characters.
@@ -559 +559 @@
         previousIsNewline.link(this);
     } else
-        failures.append(jne32(Imm32(0), index));
+        failures.append(jne32(index, Imm32(0)));
 }
 
@@ -584 +584 @@
 
     // (1.1) check for begin of input
-    Jump atBegin = je32(Imm32(0), index);
+    Jump atBegin = je32(index, Imm32(0));
     // (1.2) load the last char, and chck if is word character
     load16(BaseIndex(input, index, TimesTwo, -2), character);