Review feedback: remove dependence on (and test for) NOP being the literal byte 0.

Author: cfallin

cranelift/codegen/src/isa/pulley_shared/inst/mod.rs

@@ -536,9 +536,13 @@ where
     }
 
     fn gen_nop_unit() -> SmallVec<[u8; 8]> {
-        // See unit test `nop_is_single_zero_byte` in Pulley crate
-        // where this is validated.
-        smallvec::smallvec![0]
+        let mut bytes = smallvec::smallvec![];
+        let nop = pulley_interpreter::op::Nop {};
+        nop.encode(&mut bytes);
+        // NOP needs to be a 1-byte opcode so it can be used to
+        // overwrite a callsite of any length.
+        assert_eq!(nop.len(), 1);
+        bytes
     }
 
     fn rc_for_type(ty: Type) -> CodegenResult<(&'static [RegClass], &'static [Type])> {

pulley/src/encode.rs

@@ -335,13 +335,11 @@ for_each_extended_op!(impl_extended_encoders);
 
 #[test]
 #[cfg(feature = "std")]
-fn nop_is_single_zero_byte() {
-    // The fact that [0x00] is a nop is load-bearing; the MachInst
-    // impl in the Cranelift backend uses this. To ensure this, we
-    // list Nop first in the opcode list in lib.rs and give it zero
-    // arguments.
+fn nop_is_single_byte() {
+    // NOP needs to be a single byte so that it can be used to NOP out
+    // an instruction sequence of any length.
     let inst = crate::op::Nop {};
     let mut bytes = vec![];
     inst.encode(&mut bytes);
-    assert_eq!(bytes, vec![0]);
+    assert_eq!(bytes.len(), 1);
 }

pulley/src/lib.rs

@@ -87,8 +87,6 @@ macro_rules! for_each_op {
     ( $macro:ident ) => {
         $macro! {
             /// No-operation.
-            ///
-            /// NOTE: must be first; we rely on the opcode being zero.
             nop = Nop;
 
             /// Transfer control the address in the `lr` register.