Skip to content

Commit

Permalink
Add CONST_PTR indirection
Browse files Browse the repository at this point in the history
Previously, we'd put large constants directly in the expression nodes
array, which means that each node had to be as large as a
pointer (i.e. 64 bits wide). This is wasteful (we don't need that much
space for most nodes) but it also ensures that we can't compare
pointers accross runs, since address layout randomization will make
them different.

Furthermore, this simplifies tile development, since most x86-64
instructions can't accept 64-bit constants, so they'd have to be
loaded into a register using the 'movabs' instruction and reduce to
register-register instructions instead. Now that the 'large' and small
constants are separated, this edge case handling is no longer
neccessary and this results in simpler tile code.
  • Loading branch information
bdw committed Jul 6, 2018
1 parent e830ae7 commit 89891f3
Show file tree
Hide file tree
Showing 9 changed files with 221 additions and 161 deletions.
128 changes: 80 additions & 48 deletions docs/jit/todo.org
Expand Up @@ -266,6 +266,82 @@ definitions and uses.... but I'm not sure how just yet.

* Expression Tree

** Reduce tree node size to 32 bits

Tree nodes are currently 64 bits wide to allow them to coexist with
constant pointers. This is handy, but not really required, since we
could use a lookup table to get the pointers (as long as we can
declare pointers, for which I think we can still use the '@' sigil,
e.g:

#+BEGIN_EXAMPLE
(template: say
(call (const @MVM_string_say ptr_sz)
(arglist 2
(carg (tc) ptr)
(carg $0 ptr))
#+END_EXAMPLE

The @MVM_string_say pointer can be stashed in an array:

#+BEGIN_SRC C
static const void *MVM_jit_expr_ptrs[] = {
...
MVM_string_say,
...
};
#+END_SRC

And the pointer itself replaced by the index.

We could argue against dealing with 64 bit constants in general, but
unfortunately, const_i64 prevents us from doing that.... Ways of
dealing with that:

+ A 'large constants' table per tree (into which we could copy both the
i64 constants and the function pointer constants)
+ We could store this entire table in the data section, too
+ A 'large constants' op, which could take the space to store the 64
bit constant directly; one of the advantages of that is that we
could specialise tiling to that (e.g. there is no advantage to
including a very large constant in the ADD tile since the underlying
'add' instruction cannot handle it).
+ Or both: have a large_const op and a large_const table, and only
have the large_const op refer to the large_const table (i.e. not the
regular const)

NB - I didn't do the @ notation, because it turns out we need to
specially handle the indirected constants, and because of platform
differences, we also make the distinction between large constants in
general (MVMint64 or MVMnum64) and pointer constants, because pointers
have different sizes per architecture, and general large constants do
not.


** Encode (parts of) info as flags

Even when we'd have 32 bit operands and we'd (wastefully) use 16 bits
for the operator (rather than 8 which would be till 4x more than we
use now), we still have 16 perfectly good bits left. What I want to do
is to make info mostly sparse, so that:

- we can use a hashtable to represent all nodes that do have info
- we can encode all relevant attributes 'inline'

Which would be:
- number-of-args (redundant but nice, especially if it gets rid of the
first-child-node-is-arg-count nonsense)
- we could maximize this to 16? (4 bits) or 32 (5 bits)
- size (1,2,4,8) (2 bits or 3 bits)
- that may be a bit aggressive, we may want to support SIMD?
- type flag (int/num/str/obj) (2 bits or 5 bits if we want to encode
the whole field)

All that would remain of info would be spesh_ins, meaning that this
would become sufficiently sparse to use a hash table, which would mean
we no longer need to resize the number of args.


** REPR-Specialized expression code

Parts needed:
Expand Down Expand Up @@ -308,48 +384,6 @@ definitions and uses.... but I'm not sure how just yet.

We don't actually need this yet, but we don't guard against it
either. (So maybe install an oops in analyze first).
** Reduce tree node size to 32 bits

Tree nodes are currently 64 bits wide to allow them to coexist with
constant pointers. This is handy, but not really required, since we
could use a lookup table to get the pointers (as long as we can
declare pointers, for which I think we can still use the '@' sigil, e.g:

#+BEGIN_EXAMPLE
(template: say
(call (const @MVM_string_say ptr_sz)
(arglist 2
(carg (tc) ptr)
(carg $0 ptr))
#+END_EXAMPLE

The @MVM_string_say pointer can be stashed in an array:

#+BEGIN_SRC C
static const void *MVM_jit_expr_ptrs[] = {
...
MVM_string_say,
...
};
#+END_SRC

And the pointer itself replaced by the index.

We could argue against dealing with 64 bit constants in general, but
unfortunately, const_i64 prevents us from doing that.... Ways of
dealing with that:

+ A 'large constants' table per tree (into which we could copy both the
i64 constants and the function pointer constants)
+ We could store this entire table in the data section, too
+ A 'large constants' op, which could take the space to store the 64
bit constant directly; one of the advantages of that is that we
could specialise tiling to that (e.g. there is no advantage to
including a very large constant in the ADD tile since the underlying
'add' instruction cannot handle it).
+ Or both: have a large_const op and a large_const table, and only
have the large_const op refer to the large_const table (i.e. not the
regular const)

NB - I didn't do the @ notation, because it turns out we need to
specially handle the indirected constants, and because of platform
Expand Down Expand Up @@ -442,6 +476,7 @@ typedef union {
Simple, mechanical transformation. I wonder if we can have a maximum
depth; probably not, if we can allow revisits. More importantly, this
should allow for some control on the iteration order

** Right-to-left evaluation

E.g. (STORE addr value sz) - it usually makes sense to calculate value
Expand Down Expand Up @@ -621,11 +656,8 @@ Walks should be single-visit.
- all children are replaced with equivalent (according to the table)
- then parent is itself 'hashed' to a record, an potentially
replaced
- IDX CONST to ADDR conversion
- Uses one register less, simpler operation
- ADD CONST to ADDR conversion
- only allowed if user is pointerlike (e.g. LOAD)
- COPY insertion
- barriers should clear out the table


*** DONE IDX CONST to ADDR conversion
Uses one register fewer, simpler operation
Expand All @@ -639,7 +671,7 @@ Only allowed if user is pointerlike (e.g. LOAD)
COPY node and replace the refs
- Possibly a pessimization because it requires more registers!

*** TODO COPY elimination
*** DONE COPY elimination
- possibly the first step, removing redundant copies
- especially for CONST nodes

Expand Down
47 changes: 40 additions & 7 deletions src/jit/expr.c
Expand Up @@ -20,7 +20,6 @@ const MVMJitExprOpInfo MVM_JIT_EXPR_OP_INFO_TABLE[] = {

/* macros used in the expression list templates, defined here so they
don't overwrite other definitions */
#define CONST_PTR(x) ((uintptr_t)(x))
#define QUOTE(x) (x)
#define MSG(...) CONST_PTR(#__VA_ARGS__)
#define SIZEOF_MEMBER(type, member) sizeof(((type*)0)->member)
Expand Down Expand Up @@ -130,6 +129,34 @@ static MVMint32 MVM_jit_expr_add_lexaddr(MVMThreadContext *tc, MVMJitExprTree *t
MVM_JIT_ADDR, 3, idx * MVM_JIT_REG_SZ) + 6;
}

/* Manage large constants - by the way, no attempt is being made to unify them */
static MVMint32 MVM_jit_expr_add_const_i64(MVMThreadContext *tc, MVMJitExprTree *tree, MVMint64 const_i64) {
MVM_VECTOR_ENSURE_SPACE(tree->constants, 1);
{
MVMint32 t = tree->constants_num++;
tree->constants[t].i = const_i64;
return t;
}
}

static MVMint32 MVM_jit_expr_add_const_n64(MVMThreadContext *tc, MVMJitExprTree *tree, MVMnum64 const_n64) {
MVM_VECTOR_ENSURE_SPACE(tree->constants, 1);
{
MVMint32 t = tree->constants_num++;
tree->constants[t].n = const_n64;
return t;
}
}

static MVMint32 MVM_jit_expr_add_const_ptr(MVMThreadContext *tc, MVMJitExprTree *tree, const void *const_ptr) {
MVM_VECTOR_ENSURE_SPACE(tree->constants, 1);
{
MVMint32 t = tree->constants_num++;
tree->constants[t].p = const_ptr;
return t;
}

}

static MVMint32 MVM_jit_expr_add_const(MVMThreadContext *tc, MVMJitExprTree *tree,
MVMSpeshOperand opr, MVMuint8 info) {
Expand All @@ -155,18 +182,19 @@ static MVMint32 MVM_jit_expr_add_const(MVMThreadContext *tc, MVMJitExprTree *tre
template[2] = sizeof(MVMint32);
break;
case MVM_operand_int64:
template[1] = opr.lit_i64;
template[2] = sizeof(MVMint64);
template[0] = MVM_JIT_CONST_LARGE;
template[1] = MVM_jit_expr_add_const_i64(tc, tree, opr.lit_i64);
template[2] = MVM_JIT_INT_SZ;
break;
case MVM_operand_num32:
/* possible endianess issue here */
template[1] = opr.lit_i32;
template[2] = sizeof(MVMnum32);
break;
case MVM_operand_num64:
/* use i64 to get the bits */
template[1] = opr.lit_i64;
template[2] = sizeof(MVMnum64);
template[0] = MVM_JIT_CONST_LARGE;
template[1] = MVM_jit_expr_add_const_n64(tc, tree, opr.lit_n64);
template[2] = MVM_JIT_NUM_SZ;
break;
case MVM_operand_str:
/* string index really */
Expand Down Expand Up @@ -331,6 +359,9 @@ static MVMint32 apply_template(MVMThreadContext *tc, MVMJitExprTree *tree, MVMin
/* add operand node into the nodes */
tree->nodes[num+i] = operands[code[i]];
break;
case 'c':
tree->nodes[num+i] = MVM_jit_expr_add_const_ptr(tc, tree, MVM_jit_expr_template_constants[code[i]]);
break;
default:
/* copy from template to nodes (./n) */
tree->nodes[num+i] = code[i];
Expand Down Expand Up @@ -533,9 +564,10 @@ MVMJitExprTree * MVM_jit_expr_tree_build(MVMThreadContext *tc, MVMJitGraph *jg,
tree = MVM_malloc(sizeof(MVMJitExprTree));
MVM_VECTOR_INIT(tree->nodes, 256);
MVM_VECTOR_INIT(tree->info, 256);
MVM_VECTOR_INIT(tree->constants, 16);
MVM_VECTOR_INIT(tree->roots, 16);

/* ensure that all references are nonzero */
/* start with a no-op so every valid reference is nonzero */
MVM_VECTOR_PUSH(tree->nodes, MVM_JIT_NOOP);

tree->graph = jg;
Expand Down Expand Up @@ -799,6 +831,7 @@ void MVM_jit_expr_tree_destroy(MVMThreadContext *tc, MVMJitExprTree *tree) {
MVM_free(tree->info);
MVM_free(tree->nodes);
MVM_free(tree->roots);
MVM_free(tree->constants);
MVM_free(tree);
}

Expand Down
7 changes: 6 additions & 1 deletion src/jit/expr.h
Expand Up @@ -21,7 +21,6 @@ typedef enum { /* value type */
#define MVM_JIT_UNSIGNED 1
#define MVM_JIT_SIGNED 2


#include "expr_ops.h"


Expand Down Expand Up @@ -57,6 +56,12 @@ struct MVMJitExprTree {
MVM_VECTOR_DECL(MVMJitExprNode, nodes);
MVM_VECTOR_DECL(MVMint32, roots);
MVM_VECTOR_DECL(MVMJitExprNodeInfo, info);
MVM_VECTOR_DECL(union {
MVMint64 i;
MVMnum64 n;
const void *p;
uintptr_t u;
}, constants);

MVMuint32 seq_nr;
};
Expand Down
10 changes: 8 additions & 2 deletions src/jit/expr_ops.h
Expand Up @@ -15,13 +15,19 @@
#define MVM_JIT_EXPR_OPS(_) \
/* invalid operator */ \
_(NOOP, 0, 0, VOID, NO_CAST), \
/* wrap value of other operator */ \
_(COPY, 1, 0, REG, NO_CAST), \
/* memory access */ \
_(LOAD, 1, 1, REG, NO_CAST), \
_(STORE, 2, 1, VOID, NO_CAST), \
_(CONST, 0, 2, REG, NO_CAST), \
_(ADDR, 1, 1, REG, UNSIGNED), \
_(IDX, 2, 1, REG, UNSIGNED), \
_(COPY, 1, 0, REG, NO_CAST), \
/* constant up to 4 bytes */ \
_(CONST, 0, 2, REG, NO_CAST), \
/* constant pointer (architecture-dependent size) */ \
_(CONST_PTR, 0, 1, REG, NO_CAST), \
/* large constant (8 bytes) */ \
_(CONST_LARGE, 0, 2, REG, NO_CAST), \
/* integer comparison */ \
_(LT, 2, 0, FLAG, SIGNED), \
_(LE, 2, 0, FLAG, SIGNED), \
Expand Down
3 changes: 2 additions & 1 deletion src/jit/macro.expr
Expand Up @@ -39,14 +39,15 @@

(macro: ^exit () (branch (label branch_exit)))

(macro: ^func (,x) (const_ptr ,x))

(macro: ^p6obody (,a)
(let: (($replace (^getf ,a MVMP6opaque body.replaced)))
(if (nz $replace)
$replace
(addr ,a (&offsetof MVMP6opaque body)))))


(macro: ^func (,a) (const (&CONST_PTR ,a) ptr_sz))
(macro: ^indirect_cu_string (,cu ,a)
(let: (($str (load (idx (^getf ,cu MVMCompUnit body.strings) ,a ptr_sz) ptr_sz)))
(if (nz $str) $str
Expand Down
1 change: 1 addition & 0 deletions src/jit/x64/tile_decl.h
@@ -1,6 +1,7 @@
MVM_JIT_TILE_DECL(addr);
MVM_JIT_TILE_DECL(idx);
MVM_JIT_TILE_DECL(const_reg);
MVM_JIT_TILE_DECL(const_large);

MVM_JIT_TILE_DECL(load_lbl);
MVM_JIT_TILE_DECL(load_reg);
Expand Down
11 changes: 9 additions & 2 deletions src/jit/x64/tile_pattern.tile
Expand Up @@ -40,6 +40,11 @@
(tile: addr (addr reg $ofs) reg 2)
(tile: idx (idx reg reg $scl) reg 2)
(tile: const_reg (const $val $size) reg 2)

(tile: const_large (const_large $value $size) reg 3)
(tile: const_large (const_ptr $ptr) reg 3)


(tile: load_reg (load reg $size) reg 5)
(tile: load_addr (load (addr reg $ofs) $size) reg 5)
(tile: load_idx (load (idx reg reg $scale) $size) reg 5)
Expand Down Expand Up @@ -116,11 +121,13 @@
# placeholder for arglist pseudotile
(define: (arglist (carg reg)) c_args 1)



(tile: call (call reg c_args $size) reg 4)
(tile: call (callv reg c_args) void 4)

(tile: call_func (call (const $ptr $sz) c_args $size) reg 4)
(tile: call_func (callv (const $ptr $sz) c_args) void 4)
(tile: call_func (call (const_ptr $ptr) c_args $size) reg 4)
(tile: call_func (callv (const_ptr $ptr) c_args) void 4)

(tile: call_addr (call (load (addr reg $ofs) $sz) c_args $size) reg 4)
(tile: call_addr (callv (load (addr reg $ofs) $sz) c_args) void 4)

0 comments on commit 89891f3

Please sign in to comment.