From: "zack.ref@... (Zack Deveau) via ruby-core" Date: 2024-07-11T22:21:55+00:00 Subject: [ruby-core:118569] [Ruby master Misc#20630] Potential optimizations for `NODE_CONST` compilation Issue #20630 has been reported by zack.ref@gmail.com (Zack Deveau). ---------------------------------------- Misc #20630: Potential optimizations for `NODE_CONST` compilation https://bugs.ruby-lang.org/issues/20630 * Author: zack.ref@gmail.com (Zack Deveau) * Status: Open ---------------------------------------- ### 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: #@./test.rb:1 (1,0)-(3,1)> 0000 putchilledstring "navigator" ( 1)[Li] 0002 putspecialobject 3 0004 setconstant :NETSCAPE 0006 opt_getconstant_path ( 2)[Li] 0008 pop 0009 opt_getconstant_path 0011 pop 0012 opt_getconstant_path 0014 pop 0015 opt_getconstant_path 0017 pop 0018 putobject_INT2FIX_1_ ( 3)[Li] 0019 leave ``` `with optimizations` ``` ���� ��� ./build/miniruby --dump insn ./test.rb == disasm: #@./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 ``` -- https://bugs.ruby-lang.org/ ______________________________________________ ruby-core mailing list -- ruby-core@ml.ruby-lang.org To unsubscribe send an email to ruby-core-leave@ml.ruby-lang.org ruby-core info -- https://ml.ruby-lang.org/mailman3/lists/ruby-core.ml.ruby-lang.org/