ZJIT: anytostring to HIR (rubyGH-13658)

Pop two values from the stack, return the first if it is a string,
otherwise return string coercion of the second

Also piggybacks a fix for string subclasses skipping `to_s` for
`objtostring`.

Co-authored-by: composerinteralia <[email protected]>

Author: ywenc

zjit/src/codegen.rs

@@ -283,6 +283,7 @@ fn gen_insn(cb: &mut CodeBlock, jit: &mut JITState, asm: &mut Assembler, functio
         Insn::SetIvar { self_val, id, val, state: _ } => return gen_setivar(asm, opnd!(self_val), *id, opnd!(val)),
         Insn::SideExit { state } => return gen_side_exit(jit, asm, &function.frame_state(*state)),
         Insn::PutSpecialObject { value_type } => gen_putspecialobject(asm, *value_type),
+        Insn::AnyToString { val, str, state } => gen_anytostring(asm, opnd!(val), opnd!(str), &function.frame_state(*state))?,
         _ => {
             debug!("ZJIT: gen_function: unexpected insn {:?}", insn);
             return None;
@@ -814,6 +815,17 @@ fn gen_fixnum_ge(asm: &mut Assembler, left: lir::Opnd, right: lir::Opnd) -> Opti
     Some(asm.csel_ge(Qtrue.into(), Qfalse.into()))
 }
 
+fn gen_anytostring(asm: &mut Assembler, val: lir::Opnd, str: lir::Opnd, state: &FrameState) -> Option<lir::Opnd> {
+    // Save PC
+    gen_save_pc(asm, state);
+    
+    asm_comment!(asm, "call rb_obj_as_string_result");
+    Some(asm.ccall(
+        rb_obj_as_string_result as *const u8,
+        vec![str, val],
+    ))
+}
+
 /// Evaluate if a value is truthy
 /// Produces a CBool type (0 or 1)
 /// In Ruby, only nil and false are falsy

zjit/src/hir.rs

@@ -498,6 +498,7 @@ pub enum Insn {
 
     // Distinct from `SendWithoutBlock` with `mid:to_s` because does not have a patch point for String to_s being redefined
     ObjToString { val: InsnId, call_info: CallInfo, cd: *const rb_call_data, state: InsnId },
+    AnyToString { val: InsnId, str: InsnId, state: InsnId },
 
     /// Side-exit if val doesn't have the expected type.
     GuardType { val: InsnId, guard_type: Type, state: InsnId },
@@ -699,6 +700,7 @@ impl<'a> std::fmt::Display for InsnPrinter<'a> {
             Insn::ArrayExtend { left, right, .. } => write!(f, "ArrayExtend {left}, {right}"),
             Insn::ArrayPush { array, val, .. } => write!(f, "ArrayPush {array}, {val}"),
             Insn::ObjToString { val, .. } => { write!(f, "ObjToString {val}") },
+            Insn::AnyToString { val, str, .. } => { write!(f, "AnyToString {val}, str: {str}") },
             Insn::SideExit { .. } => write!(f, "SideExit"),
             Insn::PutSpecialObject { value_type } => {
                 write!(f, "PutSpecialObject {}", value_type)
@@ -1023,6 +1025,11 @@ impl Function {
                 cd: *cd,
                 state: *state,
             },
+            AnyToString { val, str, state } => AnyToString {
+                val: find!(*val),
+                str: find!(*str),
+                state: *state,
+            },
             SendWithoutBlock { self_val, call_info, cd, args, state } => SendWithoutBlock {
                 self_val: find!(*self_val),
                 call_info: call_info.clone(),
@@ -1154,6 +1161,7 @@ impl Function {
             Insn::ToNewArray { .. } => types::ArrayExact,
             Insn::ToArray { .. } => types::ArrayExact,
             Insn::ObjToString { .. } => types::BasicObject,
+            Insn::AnyToString { .. } => types::String,
         }
     }
 
@@ -1398,14 +1406,21 @@ impl Function {
                         self.make_equal_to(insn_id, replacement);
                     }
                     Insn::ObjToString { val, call_info, cd, state, .. } => {
-                        if self.is_a(val, types::StringExact) {
+                        if self.is_a(val, types::String) {
                             // behaves differently from `SendWithoutBlock` with `mid:to_s` because ObjToString should not have a patch point for String to_s being redefined
                             self.make_equal_to(insn_id, val);
                         } else {
                             let replacement = self.push_insn(block, Insn::SendWithoutBlock { self_val: val, call_info, cd, args: vec![], state });
                             self.make_equal_to(insn_id, replacement)
                         }
                     }
+                    Insn::AnyToString { str, .. } => {
+                        if self.is_a(str, types::String) {
+                            self.make_equal_to(insn_id, str);
+                        } else {
+                            self.push_insn_id(block, insn_id); 
+                        }
+                    }
                     _ => { self.push_insn_id(block, insn_id); }
                 }
             }
@@ -1782,6 +1797,11 @@ impl Function {
                     worklist.push_back(val);
                     worklist.push_back(state);
                 }
+                Insn::AnyToString { val, str, state, .. } => {
+                    worklist.push_back(val);
+                    worklist.push_back(str);
+                    worklist.push_back(state);
+                }
                 Insn::GetGlobal { state, .. } |
                 Insn::SideExit { state } => worklist.push_back(state),
             }
@@ -2783,6 +2803,14 @@ pub fn iseq_to_hir(iseq: *const rb_iseq_t) -> Result<Function, ParseError> {
                     let objtostring = fun.push_insn(block, Insn::ObjToString { val: recv, call_info: CallInfo { method_name }, cd, state: exit_id });
                     state.stack_push(objtostring)
                 }
+                YARVINSN_anytostring => {
+                    let str = state.stack_pop()?;
+                    let val = state.stack_pop()?;
+
+                    let exit_id = fun.push_insn(block, Insn::Snapshot { state: exit_state });
+                    let anytostring = fun.push_insn(block, Insn::AnyToString { val, str, state: exit_id });
+                    state.stack_push(anytostring);
+                }
                 _ => {
                     // Unknown opcode; side-exit into the interpreter
                     let exit_id = fun.push_insn(block, Insn::Snapshot { state: exit_state });
@@ -4452,7 +4480,7 @@ mod tests {
     }
 
     #[test]
-    fn test_objtostring() {
+    fn test_objtostring_anytostring() {
         eval("
             def test = \"#{1}\"
         ");
@@ -4462,6 +4490,7 @@ mod tests {
               v2:StringExact[VALUE(0x1000)] = Const Value(VALUE(0x1000))
               v3:Fixnum[1] = Const Value(1)
               v5:BasicObject = ObjToString v3
+              v7:String = AnyToString v3, str: v5
               SideExit
         "#]]);
     }
@@ -6010,7 +6039,7 @@ mod opt_tests {
     }
 
     #[test]
-    fn test_objtostring_string() {
+    fn test_objtostring_anytostring_string() {
         eval(r##"
             def test = "#{('foo')}"
         "##);
@@ -6025,7 +6054,7 @@ mod opt_tests {
     }
 
     #[test]
-    fn test_objtostring_with_non_string() {
+    fn test_objtostring_anytostring_with_non_string() {
         eval(r##"
             def test = "#{1}"
         "##);
@@ -6034,7 +6063,8 @@ mod opt_tests {
             bb0(v0:BasicObject):
               v2:StringExact[VALUE(0x1000)] = Const Value(VALUE(0x1000))
               v3:Fixnum[1] = Const Value(1)
-              v8:BasicObject = SendWithoutBlock v3, :to_s
+              v10:BasicObject = SendWithoutBlock v3, :to_s
+              v7:String = AnyToString v3, str: v10
               SideExit
         "#]]);
     }