Skip to content
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.

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

Add --yjit-no-type-prop so we can test YJIT without type propagation #5135

Merged
merged 3 commits into from Nov 18, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions .github/CODEOWNERS
Validating CODEOWNERS rules …
Expand Up @@ -7,3 +7,4 @@ yjit* @maximecb @xrxr @tenderlove
doc/yjit/* @maximecb @xrxr @tenderlove
bootstraptest/test_yjit* @maximecb @xrxr @tenderlove
test/ruby/test_yjit* @maximecb @xrxr @tenderlove
.github/workflows/yjit* @maximecb @xrxr @tenderlove
4 changes: 1 addition & 3 deletions .github/workflows/yjit-ubuntu.yml
Expand Up @@ -21,9 +21,7 @@ jobs:
# - ubuntu-18.04
yjit_opts: [
"--yjit",
"--yjit --yjit-call-threshold=1 --yjit-max-versions=1",
# "--yjit --yjit-call-threshold=1",
Copy link
Member

Choose a reason for hiding this comment

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

I think @nobu disabled these because we had too many runs and was slowing down CI. I'm not sure if it's okay to add these back?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hm. Ok. I wish they would tell us before doing things like that. I can leave just two enabled.

# "--yjit --yjit-call-threshold=2"
"--yjit --yjit-call-threshold=1",
]
configure: ["", "cppflags=-DRUBY_DEBUG"]
include:
Expand Down
8 changes: 8 additions & 0 deletions doc/yjit/yjit.md
Expand Up @@ -78,6 +78,14 @@ To support disassembly of the generated code, `libcapstone` is also required (`b
make -j16 install
```

On macOS, you may need to specify where to find openssl, libyaml and gdbm:

```
# Configure with debugging/stats options for development, build and install
./configure cppflags="-DRUBY_DEBUG -DYJIT_STATS" --prefix=$HOME/.rubies/ruby-yjit --disable-install-doc --disable--install-rdoc --with-opt-dir=$(brew --prefix openssl):$(brew --prefix readline):$(brew --prefix libyaml):$(brew --prefix gdbm)
make -j16 install
```

Typically configure will choose default C compiler. To specify the C compiler, use
```
# Choosing a specific c compiler
Expand Down
3 changes: 3 additions & 0 deletions ruby.c
Expand Up @@ -1091,6 +1091,9 @@ setup_yjit_options(const char *s, struct rb_yjit_options *yjit_opt)
else if (yjit_opt_match_noarg(s, l, "greedy-versioning")) {
yjit_opt->greedy_versioning = true;
}
else if (yjit_opt_match_noarg(s, l, "no-type-prop")) {
yjit_opt->no_type_prop = true;
}
else if (yjit_opt_match_noarg(s, l, "stats")) {
yjit_opt->gen_stats = true;
}
Expand Down
3 changes: 3 additions & 0 deletions yjit.h
Expand Up @@ -29,6 +29,9 @@ struct rb_yjit_options {
// Generate versions greedily until the limit is hit
bool greedy_versioning;

// Disable the propagation of type information
bool no_type_prop;

// Maximum number of versions per block
// 1 means always create generic versions
unsigned max_versions;
Expand Down
19 changes: 18 additions & 1 deletion yjit_core.c
Expand Up @@ -26,6 +26,11 @@ Return a pointer to the new stack top
static x86opnd_t
ctx_stack_push_mapping(ctx_t *ctx, temp_type_mapping_t mapping)
{
// If type propagation is disabled, store no types
if (rb_yjit_opts.no_type_prop) {
mapping.type = TYPE_UNKNOWN;
}

// Keep track of the type and mapping of the value
if (ctx->stack_size < MAX_TEMP_TYPES) {
ctx->temp_mapping[ctx->stack_size] = mapping.mapping;
Expand Down Expand Up @@ -80,6 +85,7 @@ ctx_stack_push_local(ctx_t *ctx, size_t local_idx)
(temp_mapping_t){ .kind = TEMP_LOCAL, .idx = local_idx },
TYPE_UNKNOWN
};

return ctx_stack_push_mapping(ctx, mapping);
}

Expand Down Expand Up @@ -165,7 +171,6 @@ static int type_diff(val_type_t src, val_type_t dst);
(dest) = (src); \
} while (false)


/**
Upgrade (or "learn") the type of an instruction operand
This value must be compatible and at least as specific as the previously known type.
Expand All @@ -175,6 +180,10 @@ propagated back to its source.
static void
ctx_upgrade_opnd_type(ctx_t *ctx, insn_opnd_t opnd, val_type_t type)
{
// If type propagation is disabled, store no types
if (rb_yjit_opts.no_type_prop)
return;

if (opnd.is_self) {
UPGRADE_TYPE(ctx->self_type, type);
return;
Expand Down Expand Up @@ -249,6 +258,10 @@ ctx_set_opnd_mapping(ctx_t *ctx, insn_opnd_t opnd, temp_type_mapping_t type_mapp
RUBY_ASSERT(opnd.idx < ctx->stack_size);
int stack_idx = ctx->stack_size - 1 - opnd.idx;

// If type propagation is disabled, store no types
if (rb_yjit_opts.no_type_prop)
return;

// If outside of tracked range, do nothing
if (stack_idx >= MAX_TEMP_TYPES)
return;
Expand All @@ -265,6 +278,10 @@ Set the type of a local variable
static void
ctx_set_local_type(ctx_t *ctx, size_t idx, val_type_t type)
{
// If type propagation is disabled, store no types
if (rb_yjit_opts.no_type_prop)
return;

if (idx >= MAX_LOCAL_TYPES)
return;

Expand Down
6 changes: 6 additions & 0 deletions yjit_iface.c
Expand Up @@ -1210,6 +1210,12 @@ rb_yjit_init(struct rb_yjit_options *options)
rb_yjit_opts.max_versions = 4;
}

// If type propagation is disabled, max 1 version per block
if (rb_yjit_opts.no_type_prop)
{
rb_yjit_opts.max_versions = 1;
}

blocks_assuming_stable_global_constant_state = st_init_numtable();
blocks_assuming_single_ractor_mode = st_init_numtable();
blocks_assuming_bops = st_init_numtable();
Expand Down