Skip to content

Commit fa3a17e

Browse files
DinoVmeta-codesync[bot]
authored andcommitted
Use sp for addressing locals when we can
Summary: Take 2 of D114808854. The underlying issue here was actually manifested by the addition of D116050159 but the fix is here. For store pair if we can't encode the store in the offset that store pair supports we could emit two stores instead. We actually weren't properly using `ptr_offset` in the existing code path here so technically this could end up with a failure to compile. D114808854 made us use `getPairScratchPtr` which will handle large offsets properly. But that uses the scratch register and postalloc could have introduced the use of the scratch register as well! This causes us to generate code like: ``` sub x13, fp, #520 str x13, [x13] # we just stored the wrong thing here ``` This only gets hit when we actually start hitting more load pairs/store pairs on large stack references. So there's a fix here so that instead of emitting: sub, str, sub, str we now do sub, stp which is an improvement in our code gen. In D116050159 we'll disallow the optimization when the value involves a scratch register. Currently we always use `fp` for addressing variables on the stack. But if we use `sp` we can get to a wider range of variables more efficiently. This switches us over to doing that when it's safe to do so - usually we can track the stack pointer but in the case of generators we replace fp with the generators memory so we can't track it there. Note there's an assumption here that we're not being insane with our stack allocation - e.g. we don't have one block modifying the stack state and transferring control to another block. That's true today and is likely to always be true. It would generally be hard to make arbitrary changes to the stack across multiple control flow paths. But if we ever needed to do that for some reason we could reset the known stack offset and just lose the optimization. This will also let us use stp more in a subsequent diff. Reviewed By: yoney Differential Revision: D116050156 fbshipit-source-id: bf063b84cd62aacc1d9fad94b9bfbfb01b6021bf
1 parent 4b56028 commit fa3a17e

5 files changed

Lines changed: 212 additions & 47 deletions

File tree

cinderx/Jit/codegen/arch.cpp

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ namespace cinderx::jit::codegen::arch {
1010

1111
// Attempt to build a pointer using an offset from a base register. If it is
1212
// not possible to do so, return std::nullopt.
13-
static std::optional<a64::Mem>
13+
std::optional<a64::Mem>
1414
ptr_offset_try(const a64::Gp& base, int32_t offset, AccessSize access_size) {
1515
if (offset >= -256 && offset < 256) {
1616
// Unscaled immediate offset

cinderx/Jit/codegen/arch.h

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
#include <fmt/ostream.h>
99

1010
#include <iosfwd>
11+
#include <optional>
1112

1213
#if defined(CINDER_X86_64)
1314

@@ -147,6 +148,15 @@ namespace cinderx::jit::codegen::arch {
147148

148149
enum class AccessSize : int32_t { k8 = 1, k16 = 2, k32 = 4, k64 = 8 };
149150

151+
// Sentinel for Environ::sp_to_fp_delta meaning SP is not at its frame
152+
// position, so frame slots can only be reached through FP.
153+
constexpr int32_t kSpPositionUnknown = -1;
154+
155+
std::optional<asmjit::a64::Mem> ptr_offset_try(
156+
const asmjit::a64::Gp& base,
157+
int32_t offset,
158+
AccessSize access_size);
159+
150160
asmjit::a64::Mem ptr_offset(
151161
const asmjit::a64::Gp& base,
152162
int32_t offset,

0 commit comments

Comments
 (0)