Misc #20630
Updated by zack.ref@gmail.com (Zack Deveau) 6 days ago
### Description
I would like to propose two potential optimizations to the way we currently compile `NODE_CONST` nodes in `compile.c`. I've included commits for each on a related PR.
- PR: https://github.com/ruby/ruby/pull/11154
- `NODE_CONST` compilation: [9b6d613](https://github.com/ruby/ruby/commit/9b6d613e19c1132ace64583ac23c94fcbc77f1df)
- `opt_getconstant_path` peephole optimization: [b8ece8c](https://github.com/ruby/ruby/commit/b8ece8ca6e2d60cb7c627927aa89c1e8980ad0eb)
These commits pass `test-all` locally.
### `NODE_CONST` Compilation
From what I can tell `NODE_CONST` doesn't appear to follow the conventional use of `popped` found in other similar nodes. For example `NODE_IVAR`, when `popped` (return value not used), will avoid adding the YARV instruction altogether:
```C
case NODE_IVAR:{
debugi("nd_vid", RNODE_IVAR(node)->nd_vid);
if (!popped) {
ADD_INSN2(ret, node, getinstancevariable,
ID2SYM(RNODE_IVAR(node)->nd_vid),
get_ivar_ic_value(iseq, RNODE_IVAR(node)->nd_vid));
}
break;
}
```
When compiling a `NODE_CONST` that is `popped`, we instead add either `get_constant` or the optimized `opt_getconstant_path` instruction, followed by adding a `pop`.
I've modified it to follow the convention found elsewhere to avoid adding either instruction(s) in cases where we don't need the return value. `test-all` passes locally and I can't think of meaningful side effects (other than we avoid exercising the cache). Please let me know if I'm missing any!
### `opt_getconstant_path` peephole optimization
`iseq_peephole_optimize` includes an optimization for `insn -> pop` sequences. Most of the `get_x` instructions are included but we don't appear to include `opt_getconstant_path`. (potentially since it was only recently added in 2022 by @jhawthorn ?)
I've included it and attempted to account for any meaningful side effects (here again I can't think of any other than we avoid exercising the cache). Please let me know if I missed any.
### Results
`test.rb`
```Ruby
NETSCAPE = "navigator"
[NETSCAPE, NETSCAPE, NETSCAPE, NETSCAPE]
1
```
`ruby 3.4.0dev (2024-07-11T19:49:14Z master 6fc83118bb) [arm64-darwin23]`
```
💾 ➜ ruby --dump insn ./test.rb
== disasm: #<ISeq:<main>@./test.rb:1 (1,0)-(3,1)>
0000 putchilledstring "navigator" ( 1)[Li]
0002 putspecialobject 3
0004 setconstant :NETSCAPE
0006 opt_getconstant_path <ic:0 NETSCAPE> ( 2)[Li]
0008 pop
0009 opt_getconstant_path <ic:1 NETSCAPE>
0011 pop
0012 opt_getconstant_path <ic:2 NETSCAPE>
0014 pop
0015 opt_getconstant_path <ic:3 NETSCAPE>
0017 pop
0018 putobject_INT2FIX_1_ ( 3)[Li]
0019 leave
```
`with optimizations`
```
💾 ➜ ./build/miniruby --dump insn ./test.rb
== disasm: #<ISeq:<main>@./test.rb:1 (1,0)-(3,1)>
0000 putchilledstring "navigator" ( 1)[Li]
0002 putspecialobject 3
0004 setconstant :NETSCAPE
0006 putobject_INT2FIX_1_ ( 3)[Li]
0007 leave
```