Skip to content

Commit

Permalink
Split cases for all numeric operations
Browse files Browse the repository at this point in the history 
Summary:
In working on prior diffs, I've observed that the code quality for these
numeric cases is noticeably worse than other fast paths. In particular,
the compiler does not generate a fall-through to the fast path, perhaps
because it is also the target of a computed goto.

In addition to fixing those to now fall-through, this seems to
significantly reduce the amount of generated code for the interpreter
overall. I see ~9% fewer instructions in the interpreter on arm64.

It is difficult to assess whether this reduction is entirely desirable,
since for instance some movs are replaced by load/store. But overall,
this improves performance measurably on the Raspberry Pi.

Reviewed By: avp

Differential Revision: D66275151

fbshipit-source-id: 76e7b47c047deacf4dcdfe4c3f161234f8b06930
  • Loading branch information
@neildhar @facebook-github-bot
neildhar authored and facebook-github-bot committed Nov 23, 2024
1 parent ed6695d commit ba4c854
Showing 1 changed file with 87 additions and 79 deletions.
166 changes: 87 additions & 79 deletions lib/VM/Interpreter.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1243,12 +1243,10 @@ CallResult<HermesValue> Interpreter::interpretFunction(
if (LLVM_LIKELY( \
_sh_ljs_are_both_non_nan_numbers(O2REG(name), O3REG(name)))) { \
/* Fast-path. */ \
CASE(name##N) { \
O1REG(name) = HermesValue::encodeTrustedNumberValue( \
do##name(O2REG(name).getNumber(), O3REG(name).getNumber())); \
ip = NEXTINST(name); \
DISPATCH; \
} \
O1REG(name) = HermesValue::encodeTrustedNumberValue( \
do##name(O2REG(name).getNumber(), O3REG(name).getNumber())); \
ip = NEXTINST(name); \
DISPATCH; \
} \
CAPTURE_IP_ASSIGN( \
ExecutionStatus status, \
Expand All @@ -1258,6 +1256,12 @@ CallResult<HermesValue> Interpreter::interpretFunction(
gcScope.flushToSmallCount(KEEP_HANDLES); \
ip = NEXTINST(name); \
DISPATCH; \
} \
CASE(name##N) { \
O1REG(name) = HermesValue::encodeTrustedNumberValue( \
do##name(O2REG(name).getNumber(), O3REG(name).getNumber())); \
ip = NEXTINST(name); \
DISPATCH; \
}

#define INCDECOP(name) \
Expand Down Expand Up @@ -1360,46 +1364,54 @@ CallResult<HermesValue> Interpreter::interpretFunction(

/// Implement a comparison conditional jump with a fast path where both
/// operands are numbers.
/// \param name the name of the instruction. The fast path case will have a
/// "N" appended to the name.
/// \param name the name of the instruction.
/// \param suffix Optional suffix to be added to the end (e.g. Long)
/// \param oper the C++ operator to use to actually perform the fast arithmetic
/// comparison.
/// \param operFuncName function to call for the slow-path comparison.
/// \param trueDest ip value if the conditional evaluates to true
/// \param falseDest ip value if the conditional evaluates to false
/// \param NUMBER_CASE a macro to use for the number case.
#define JCOND_IMPL( \
name, suffix, oper, operFuncName, trueDest, falseDest, NUMBER_CASE) \
CASE(name##suffix) { \
if (LLVM_LIKELY(_sh_ljs_are_both_non_nan_numbers( \
O2REG(name##suffix), O3REG(name##suffix)))) { \
/* Fast-path. */ \
NUMBER_CASE(name##N##suffix) { \
if (O2REG(name##suffix) \
.getNumber() oper O3REG(name##suffix) \
.getNumber()) { \
ip = trueDest; \
DISPATCH; \
} \
ip = falseDest; \
DISPATCH; \
} \
} \
CAPTURE_IP( \
boolRes = operFuncName( \
runtime, \
Handle<>(&O2REG(name##suffix)), \
Handle<>(&O3REG(name##suffix)))); \
if (boolRes == ExecutionStatus::EXCEPTION) \
goto exception; \
gcScope.flushToSmallCount(KEEP_HANDLES); \
if (boolRes.getValue()) { \
ip = trueDest; \
DISPATCH; \
} \
ip = falseDest; \
DISPATCH; \
#define JCOND_IMPL(name, suffix, oper, operFuncName, trueDest, falseDest) \
CASE(name##suffix) { \
if (LLVM_LIKELY(_sh_ljs_are_both_non_nan_numbers( \
O2REG(name##suffix), O3REG(name##suffix)))) { \
/* Fast-path. */ \
if (O2REG(name##suffix) \
.getNumber() oper O3REG(name##suffix) \
.getNumber()) { \
ip = trueDest; \
DISPATCH; \
} \
ip = falseDest; \
DISPATCH; \
} \
CAPTURE_IP( \
boolRes = operFuncName( \
runtime, \
Handle<>(&O2REG(name##suffix)), \
Handle<>(&O3REG(name##suffix)))); \
if (boolRes == ExecutionStatus::EXCEPTION) \
goto exception; \
gcScope.flushToSmallCount(KEEP_HANDLES); \
if (boolRes.getValue()) { \
ip = trueDest; \
DISPATCH; \
} \
ip = falseDest; \
DISPATCH; \
}

/// Like JCOND_IMPL, but for cases where the operands are known to be numbers.
#define JCOND_N_IMPL(name, suffix, oper, operFuncName, trueDest, falseDest) \
CASE(name##N##suffix) { \
if (O2REG(name##suffix) \
.getNumber() oper O3REG(name##suffix) \
.getNumber()) { \
ip = trueDest; \
DISPATCH; \
} \
ip = falseDest; \
DISPATCH; \
}

/// Implement a strict equality conditional jump
Expand Down Expand Up @@ -1443,39 +1455,35 @@ CallResult<HermesValue> Interpreter::interpretFunction(
}

/// Implement the long and short forms of a conditional jump, and its negation.
#define JCOND(name, oper, operFuncName, NUMBER_CASE) \
JCOND_IMPL( \
J##name, \
, \
oper, \
operFuncName, \
IPADD(ip->iJ##name.op1), \
NEXTINST(J##name), \
NUMBER_CASE); \
JCOND_IMPL( \
J##name, \
Long, \
oper, \
operFuncName, \
IPADD(ip->iJ##name##Long.op1), \
NEXTINST(J##name##Long), \
NUMBER_CASE); \
JCOND_IMPL( \
JNot##name, \
, \
oper, \
operFuncName, \
NEXTINST(JNot##name), \
IPADD(ip->iJNot##name.op1), \
NUMBER_CASE); \
JCOND_IMPL( \
JNot##name, \
Long, \
oper, \
operFuncName, \
NEXTINST(JNot##name##Long), \
IPADD(ip->iJNot##name##Long.op1), \
NUMBER_CASE);
#define JCOND(name, oper, operFuncName, IMPL) \
IMPL( \
J##name, \
, \
oper, \
operFuncName, \
IPADD(ip->iJ##name.op1), \
NEXTINST(J##name)); \
IMPL( \
J##name, \
Long, \
oper, \
operFuncName, \
IPADD(ip->iJ##name##Long.op1), \
NEXTINST(J##name##Long)); \
IMPL( \
JNot##name, \
, \
oper, \
operFuncName, \
NEXTINST(JNot##name), \
IPADD(ip->iJNot##name.op1)); \
IMPL( \
JNot##name, \
Long, \
oper, \
operFuncName, \
NEXTINST(JNot##name##Long), \
IPADD(ip->iJNot##name##Long.op1));

/// Load a constant.
/// \param value is the value to store in the output register.
Expand Down Expand Up @@ -3639,13 +3647,13 @@ CallResult<HermesValue> Interpreter::interpretFunction(
CONDOP(LessEq, <=, lessEqualOp_RJS);
CONDOP(Greater, >, greaterOp_RJS);
CONDOP(GreaterEq, >=, greaterEqualOp_RJS);
JCOND(Less, <, lessOp_RJS, CASE);
JCOND(LessEqual, <=, lessEqualOp_RJS, CASE);
JCOND(Less, <, lessOp_RJS, JCOND_IMPL);
JCOND(Less, <, lessOp_RJS, JCOND_N_IMPL);
JCOND(LessEqual, <=, lessEqualOp_RJS, JCOND_IMPL);
JCOND(LessEqual, <=, lessEqualOp_RJS, JCOND_N_IMPL);

#define NO_NUMBER_CASE(x)
JCOND(Greater, >, greaterOp_RJS, NO_NUMBER_CASE);
JCOND(GreaterEqual, >=, greaterEqualOp_RJS, NO_NUMBER_CASE);
#undef NO_NUMBER_CASE
JCOND(Greater, >, greaterOp_RJS, JCOND_IMPL);
JCOND(GreaterEqual, >=, greaterEqualOp_RJS, JCOND_IMPL);

JCOND_STRICT_EQ_IMPL(
JStrictEqual, , IPADD(ip->iJStrictEqual.op1), NEXTINST(JStrictEqual));
Expand Down

0 comments on commit ba4c854

Please sign in to comment.