if/while `(not= nil x)` doesn't always compile to `jmpni`

[primo-ppcg]

This isn't so much an issue, as an observation.

I've noticed that minor changes can make significant differences in execution speed. One of those is coercing the compiler into using jmpni or jmpnn for not= nil and = nil.

A minimal example:

(defmacro- not-nil-helper [a]
  ~(do
     (var res false)
     (while (not= nil ,a)
       (set res true)
       (break))
     res))

(defn not-nil? [a]
  (not-nil-helper a))

(pp ((disasm not-nil?) :bytecode))

@[(ldf 2) (ldn 4) (neq 3 4 0) (jmpno 3 4) (ldt 2) (jmp 2) (jmp -5) (ret 2)]

If we make one minor change, the compiler performs the appropriate optimization:

-     (while (not= nil ,a)
+     (while (,not= nil ,a)

@[(ldf 2) (jmpni 0 4) (ldt 2) (jmp 2) (jmp -3) (ret 2)]

In all variations of:

(while (not= nil ,a)
(while (,not= nil ,a)
(while (= nil ,a)
(while (,= nil ,a)
(if (not= nil ,a)
(if (,not= nil ,a)
(if (= nil ,a)
(if (,= nil ,a)

the only one that appears to optimize to a single op is (while (,not= nil ,a).

An interesting consequence is that the while loop above (with unconditional break) compiles to more efficient code than if:

(defmacro- not-nil-helper [a]
  ~(do
     (var res false)
     (if (,not= nil ,a)
       (set res true))
     res))

(defn not-nil? [a]
  (not-nil-helper a))

(pp ((disasm not-nil?) :bytecode))

@[(ldf 2) (ldn 4) (neq 3 4 0) (jmpno 3 3) (ldt 2) (jmp 1) (ret 2)]

Hand-rolled, this could be written as just:

@[(ldf 1) (jmpni 0 2) (ldt 1) (ret 1)]

[bakpakin]

This makes sense mostly, as I specifically wrote a case for this so that fast loops could be compiled to break on (not= nil x). We could certainly expand this check to include a few more forms fairly easily. What is confusing to me is that (,not= nil x) and (not= nil x) behave differently.

EDIT: Logic for this optimization is in the function janetc_check_notnil_form in specials.c

[primo-ppcg]

I've prepared a branch which appears to work correctly for all 8 cases.

(defn test [a]
  (var res false)
  (if (not= nil a)
    (set res true))
  res)

(pp ((disasm test) :bytecode))

master:

@[(ldf 2) (ldn 4) (neq 3 4 0) (jmpno 3 3) (ldt 2) (jmp 1) (ret 2)]

branch:

@[(ldf 2) (jmpni 0 2) (ldt 2) (ret 2)]

I also added logic to remove (jmp 1) from else-less ifs, although I'm not confident it's correct in all cases.

@@ /src/core/specials.c -606,3 +643,3 @@
     /* Compile jump to done */
     labeljd = janet_v_count(c->buffer);
-    if (!tail) janetc_emit(c, JOP_JUMP);
+    if (!tail && !(drop && janet_checktype(falsebody, JANET_NIL))) janetc_emit(c, JOP_JUMP);

[bakpakin]

Looks like it works to me. The extra (jmp 1) elimination also looks correct :+1:

[sogaiu]

Tried it out with some tests for various repositories and didn't notice any issues :+1: