Skip to content

Commit

Permalink
[JSC] ARM64 Imm should accept -UInt12
Browse files Browse the repository at this point in the history
https://bugs.webkit.org/show_bug.cgi?id=257074
rdar://109596645

Reviewed by Keith Miller.

If ARM64 instructions is listed in AirOpcodes.opcode with Imm, this instruction supports
-UInt12 and UInt12 forms. We should correctly reflect it in Air::Arg::isValidImmForm.
This avoids unnecessary constant materialization for negative values, and it reduces register
pressures. Also ARM64 instruction selection can select cmn (cmp with negative UInt12) for example
with this change.

* Source/JavaScriptCore/assembler/MacroAssemblerARM64.h:
(JSC::MacroAssemblerARM64::branchAdd64):
(JSC::MacroAssemblerARM64::branchSub64):
* Source/JavaScriptCore/b3/B3MoveConstants.cpp:
* Source/JavaScriptCore/b3/air/AirArg.h:
(JSC::B3::Air::Arg::isValidImmForm):

Canonical link: https://commits.webkit.org/264296@main
  • Loading branch information
Constellation committed May 20, 2023
1 parent 26e31e6 commit 09468ef
Show file tree
Hide file tree
Showing 3 changed files with 9 additions and 5 deletions.
3 changes: 2 additions & 1 deletion Source/JavaScriptCore/assembler/MacroAssemblerARM64.h
Expand Up @@ -4023,6 +4023,7 @@ class MacroAssemblerARM64 : public AbstractMacroAssembler<Assembler> {

Jump branchAdd64(RelationalCondition cond, TrustedImm32 imm, RegisterID dest)
{
// This is not supporting -imm.m_value UInt12. Thus we are not listing BranchAdd64 RelCond, Imm, Tmp in AirOpcode.opcodes.
ASSERT(isUInt12(imm.m_value));
m_assembler.add<64, S>(dest, dest, UInt12(imm.m_value));
return Jump(makeBranch(cond));
Expand Down Expand Up @@ -4175,12 +4176,12 @@ class MacroAssemblerARM64 : public AbstractMacroAssembler<Assembler> {

Jump branchSub64(RelationalCondition cond, TrustedImm32 imm, RegisterID dest)
{
// This is not supporting -imm.m_value UInt12. Thus we are not listing BranchSub64 RelCond, Imm, Tmp in AirOpcode.opcodes.
ASSERT(isUInt12(imm.m_value));
m_assembler.sub<64, S>(dest, dest, UInt12(imm.m_value));
return Jump(makeBranch(cond));
}


// Jumps, calls, returns

// duplicate MacroAssembler's loadPtr for loading call targets.
Expand Down
2 changes: 0 additions & 2 deletions Source/JavaScriptCore/b3/B3MoveConstants.cpp
Expand Up @@ -217,8 +217,6 @@ class MoveConstants {
int64_t addendConst = addend->asInt();
if (Air::Arg::isValidImmForm(addendConst))
break;
if (addendConst != INT64_MIN && Air::Arg::isValidImmForm(-addendConst))
break;
Value* bestAddend = findBestConstant(
[&] (Value* candidateAddend) -> bool {
if (candidateAddend->type() != addend->type())
Expand Down
9 changes: 7 additions & 2 deletions Source/JavaScriptCore/b3/air/AirArg.h
Expand Up @@ -1269,8 +1269,13 @@ class Arg {
{
if (isX86())
return B3::isRepresentableAs<int32_t>(value);
if (isARM64())
return isUInt12(value);
if (isARM64()) {
if (isUInt12(value))
return true;
if (value == INT64_MIN)
return isUInt12(INT64_MIN);
return isUInt12(-value);
}
if (isARM_THUMB2())
return isValidARMThumb2Immediate(value);
return false;
Expand Down

0 comments on commit 09468ef

Please sign in to comment.