Skip to content

[Arm32-JIT] Elixir fixes #25

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
ziopio wants to merge 4 commits into from
Closed

[Arm32-JIT] Elixir fixes #25

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
38a5757
Properly test arm32 modified immediate constants
ziopio Jul 2, 2026
60800d1
Check pending stubs on every segment
ziopio Jul 2, 2026
971d835
Do not use small memory in debug builds
ziopio Jul 2, 2026
1f8a850
Fix suboptimal literal check in arithmetic operations
ziopio Jul 2, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 13 additions & 8 deletions erts/emulator/beam/jit/arm/32/beam_asm.hpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -444,7 +444,7 @@ struct BeamAssembler : public BeamAssemblerCommon {
}

void emit_branch_if_eq(a32::Gp reg, Uint value, Label lbl) {
if (value <= 255) {
if (is_aarch32_imm(value)) {
a.cmp(reg, imm(value));
} else {
mov_imm(TMP, value);
Expand All @@ -454,7 +454,7 @@ struct BeamAssembler : public BeamAssemblerCommon {
}

void emit_branch_if_ne(a32::Gp reg, Uint value, Label lbl) {
if (value <= 255) {
if (is_aarch32_imm(value)) {
a.cmp(reg, imm(value));
} else {
mov_imm(TMP, value);
Expand Down Expand Up @@ -505,19 +505,24 @@ struct BeamAssembler : public BeamAssemblerCommon {
uint16_t upper16 = (value32 >> 16);
a.movt(to, imm(upper16));
}
}

bool is_aarch32_imm(uint32_t value) {
uint32_t encoded;
return arm::Utils::encodeAArch32Imm(value, &encoded);
}

void mov_imm(a32::Gp to, std::nullptr_t value) {
(void)value;
mov_imm(to, 0);
}

void sub(a32::Gp to, a32::Gp src, int64_t val) {
void sub(a32::Gp to, a32::Gp src, int32_t val) {
if (val < 0) {
add(to, src, -val);
} else if (val == 0 && to != src) {
a.mov(to, src);
} else if (val <= 255) {
} else if (is_aarch32_imm(val)) {
a.sub(to, src, imm(val));
} else {
ASSERT(src != TMP);
Expand All @@ -531,7 +536,7 @@ struct BeamAssembler : public BeamAssemblerCommon {
sub(to, src, -val);
} else if (val == 0 && to != src) {
a.mov(to, src);
} else if (val <= 255) {
} else if (is_aarch32_imm(val)) {
a.add(to, src, imm(val));
} else {
ASSERT(src != TMP);
Expand All @@ -540,10 +545,10 @@ struct BeamAssembler : public BeamAssemblerCommon {
}
}

void subs(a32::Gp to, a32::Gp src, int64_t val) {
if (val >= 0 && val <= 255) {
void subs(a32::Gp to, a32::Gp src, int32_t val) {
if (val >= 0 && is_aarch32_imm(val)) {
a.subs(to, src, imm(val));
} else if (val < 0 && -val <= 255) {
} else if (val < 0 && is_aarch32_imm(-val)) {
a.adds(to, src, imm(-val));
} else {
ASSERT(src != TMP);
Expand Down
12 changes: 8 additions & 4 deletions erts/emulator/beam/jit/arm/32/instr_arith.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -148,8 +148,10 @@ void BeamModuleAssembler::emit_i_plus(const ArgLabel &Fail,
const ArgSource &LHS,
const ArgSource &RHS,
const ArgRegister &Dst) {
bool rhs_is_arm_literal =
RHS.isSmall() && Support::isUInt12(RHS.as<ArgSmall>().get());
bool rhs_is_arm_literal = RHS.isSmall() &&
is_aarch32_imm(
RHS.as<ArgSmall>().get() &
~_TAG_IMMED1_MASK);
bool is_small_result = is_sum_small_if_args_are_small(LHS, RHS);

if (always_small(LHS) && always_small(RHS) && is_small_result) {
Expand Down Expand Up @@ -341,8 +343,10 @@ void BeamModuleAssembler::emit_i_minus(const ArgLabel &Fail,
const ArgSource &LHS,
const ArgSource &RHS,
const ArgRegister &Dst) {
bool rhs_is_arm_literal =
RHS.isSmall() && Support::isUInt12(RHS.as<ArgSmall>().get());
bool rhs_is_arm_literal = RHS.isSmall() &&
is_aarch32_imm(
RHS.as<ArgSmall>().get() &
~_TAG_IMMED1_MASK);
bool is_small_result = is_diff_small_if_args_are_small(LHS, RHS);

if (always_small(LHS) && always_small(RHS) && is_small_result) {
Expand Down
8 changes: 8 additions & 0 deletions erts/emulator/beam/jit/arm/32/instr_bs.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1449,6 +1449,11 @@ void BeamModuleAssembler::emit_i_bs_create_bin(const ArgLabel &Fail,
a.str(TMP, TMP_MEM5q);

for (auto seg : segments) {
// This emitter may introduce more code then expected.
// This may overshoot the 4KB displacement limit of few constants.
// To avoid it we check the pending stubs between every segment.
check_pending_stubs();

@ziopio ziopio Jul 2, 2026 •
edited
Loading

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

During loading of module Elixir.Exceptions this emitter was producing more then 2KB of assembly.

This was breaking asmjit code generation as the next stub creation was forced to handle a displacement value that was over 4KB.

The displacement happened to be over 4KB by 100~200 bytes

@ziopio ziopio Jul 3, 2026 •
edited
Loading

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@fhunleth while rebasing on maint-27 I realized that the OTP team fixed THIS same BUG in 27.2, they basically did the same fix for arm64, but added the check_pending_stubs at the end of the loops, which sligthly makes more sense.


if (seg.effectiveSize >= 0) {
continue;
}
Expand Down Expand Up @@ -1698,6 +1703,9 @@ void BeamModuleAssembler::emit_i_bs_create_bin(const ArgLabel &Fail,
a.str(ARG1, TMP_MEM1q);

for (auto seg : segments) {
// Safety check, see above
check_pending_stubs();

switch (seg.type) {
case am_append:
case am_private_append:
Expand Down
2 changes: 1 addition & 1 deletion erts/emulator/beam/jit/arm/32/instr_select.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -424,7 +424,7 @@ void BeamModuleAssembler::emit_i_jump_on_val(const ArgSource &Src,
a.asr(TMP, TMP, imm(_TAG_IMMED1_SIZE));

if (Base.get() != 0) {
if (Support::isUInt12((Sint)Base.get())) {

@ziopio ziopio Jul 2, 2026 •
edited
Loading

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change fixes a bug while loading Elixir.String.Tokenizer.
This caused the next line to emit sub r12 r12 257 which is illegal.

if (is_aarch32_imm(Base.get())) {
a.sub(TMP, TMP, imm(Base.get()));
} else {
mov_imm(VAR, Base.get());
Expand Down
2 changes: 1 addition & 1 deletion xcomp/erl-xcomp-arm-linux-debug-custom.conf
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,7 @@ CC="arm-linux-gnueabihf-gcc -march=armv7-a -marm -mfpu=neon --sysroot=$ARM_SYSRO
# Skipping warinings that are considered neglegible
SKIPPED_WARNINGS="-Wno-unused-variable -Wno-unused-but-set-variable -Wno-unused-function"
# * `CFLAGS' - C compiler flags.
CFLAGS=" -DDEBUG -DJIT_HARD_DEBUG $GDB_FLAGS -O0 -DSMALL_MEMORY --sysroot=$ARM_SYSROOT -Wall $SKIPPED_WARNINGS"
CFLAGS=" -DDEBUG -DJIT_HARD_DEBUG $GDB_FLAGS -O0 --sysroot=$ARM_SYSROOT -Wall $SKIPPED_WARNINGS"

# * `STATIC_CFLAGS' - Static C compiler flags.
#STATIC_CFLAGS=
Expand Down