Curious bug with (accumulate $)

Authoring Dialog
1

This code seems like it should work:

(two puzzles solved)
    (accumulate 1)
        *(puzzle $Obj)
        (solved $Obj)
    (into 2)

Unfortunately, it does not (on Å-machine specifically).

1
dialogc: backend_aa.c:745: encode_dest: Assertion `0' failed.
Aborted (core dumped)

This is the offending code:

static aaoper_t encode_dest(value_t v, struct program *prg, int unify) {
	if(v.tag == OPER_VAR) {
		assert(v.value < 64);
		return (aaoper_t) {unify? AAO_VAR : AAO_STORE_VAR, v.value};
	} else if(v.tag == OPER_ARG) {
		return (aaoper_t) {unify? AAO_REG : AAO_STORE_REG, REG_A + v.value};
	} else if(v.tag == OPER_TEMP) {
		assert(v.value < AA_MAX_TEMP);
		return (aaoper_t) {unify? AAO_REG : AAO_STORE_REG, REG_X + v.value};
	} else if(v.tag == VAL_NIL && unify) {
		return (aaoper_t) {AAO_REG, REG_NIL};
	} else {
		printf("%d\n", v.tag);
		assert(0); exit(1);
	}
}

And a tag of 1 means VAL_NUM—that is, a numeric constant. It looks like the Å-machine compiler isn’t able to unify the results of certain operations with a non-nil constant value (only with a global, local, temp, or constant nil), and crashes.

Unfortunately my knowledge of the Å-machine isn’t good enough to know how to fix this any time soon. For now, this works:

(two puzzles solved)
    (accumulate 1)
        *(puzzle $Obj)
        (solved $Obj)
    (into $N)
    ($N = 2)
1 Like
Dialog Wishlist
2

Does it work on z-machine and in dgdebug?

For what it’s worth, the second form is somewhat clearer anyway, although I did get a brief thrill of Dialogic mastery when I realised what accumulate ... into 2 was doing (and of course the compiler dumping core is never great).

3

That it does! So I’m pretty sure it’s intended behavior.

4

Sample code:

(program entry point)
	(accumulate $N)
		*($N is one of [1 2 3])
	(into 6)
	Six!

IR:

Intermediate code for (program entry point): 0 -1
R0: (group leader) (1 incoming) clause 0
	JUMP                      R1           -            -           
R1: (part of group R0) (1 incoming) clause 0
	ALLOCATE                  1            0            -           
	COLLECT_BEGIN 1           -            -            -           
	PUSH_CHOICE               0            R2           -           
	MAKE_VAR                  A0           -            -           
	ASSIGN                    V0/$N        A0           -           
	COLLECT_BEGIN 0           -            -            -           
	COLLECT_PUSH 0            1            -            -           
	COLLECT_PUSH 0            2            -            -           
	COLLECT_PUSH 0            3            -            -           
	COLLECT_END_R 0           A1           -            -           
	SET_CONT                  R3           -            -           
	INVOKE_MULTI              ($ is one of $)
R2: (group leader) (1 incoming) clause 0
	POP_CHOICE                0            -            -           
	COLLECT_END_V 1           6            -            -           
	PRINT_WORDS 1             "Six!"       -            -           
	DEALLOCATE 1              -            -            -           
	PROCEED 0                 -            -            -           
R3: (group leader) (1 incoming) clause 0
	COLLECT_PUSH 1            V0/$N        -            -           
	JUMP                      <FAIL>       -            -

It looks like the instruction that’s crashing is COLLECT_END_V 1 6. COLLECT_END means the end of a (collect $) block, V means the result is a value (instead of a reference), 1 means it’s an (accumulate $) instead of a (collect $), and 6 is the value we’re storing into.

Here’s the code for compiling COLLECT_END_V:

case I_COLLECT_END_V:
	if(ci->subop) {
		ai = add_instr(AA_AUX_POP_LIST);
		ai->oper[0] = (aaoper_t) {AAO_STORE_REG, REG_TMP};
		ai = add_instr(AA_MAKE_PAIR_D);
		ai->oper[0] = encode_dest(ci->oper[0], prg, 1);
		ai->oper[1] = (aaoper_t) {AAO_REG, REG_NIL};
		ai->oper[2] = (aaoper_t) {AAO_REG, REG_TMP};
	} else {
		ai = add_instr(AA_AUX_POP_LIST);
		if(ci->oper[0].tag == OPER_TEMP
		|| ci->oper[0].tag == OPER_VAR
		|| ci->oper[0].tag == OPER_ARG
		|| ci->oper[0].tag == VAL_NIL) {
			ai->oper[0] = encode_dest(ci->oper[0], prg, 1);
		} else {
			ai->oper[0] = (aaoper_t) {AAO_STORE_REG, REG_TMP};
			ai = add_instr(AA_ASSIGN);
			ai->oper[0] = encode_value(ci->oper[0], prg);
			ai->oper[1] = (aaoper_t) {AAO_REG, REG_TMP};
		}
	}
	break;

Notably, the case when !ci->subop—that is, (collect $) rather than (accumulate $)—specifically checks if the destination is a temporary, variable, argument, or constant nil before storing to it. If it’s not, it instead stores to REG_TMP, then assigns from there.

So, let’s do the same for ci->subop (that is, (accumulate $)).

ai = add_instr(AA_AUX_POP_LIST);
ai->oper[0] = (aaoper_t) {AAO_STORE_REG, REG_TMP};
ai = add_instr(AA_MAKE_PAIR_D);
ai->oper[1] = (aaoper_t) {AAO_REG, REG_NIL};
ai->oper[2] = (aaoper_t) {AAO_REG, REG_TMP};
// If we can store directly into the result, then do it
if(ci->oper[0].tag == OPER_TEMP
|| ci->oper[0].tag == OPER_VAR
|| ci->oper[0].tag == OPER_ARG
|| ci->oper[0].tag == VAL_NIL) {
	ai->oper[0] = encode_dest(ci->oper[0], prg, 1);
} else {
	// Otherwise, store into REG_TMP, then unify
	ai->oper[0] = (aaoper_t) {AAO_STORE_REG, REG_TMP};
	ai = add_instr(AA_ASSIGN);
	ai->oper[0] = encode_value(ci->oper[0], prg);
	ai->oper[1] = (aaoper_t) {AAO_REG, REG_TMP};
}

And voilà! It works. Another bug squashed.

1 Like