Remove unneed UBOUNDED_MAX checks

[roconnor-blockstream]

For the case of COMP, if type_dag[COMP_B(...)].bitSize exceeds UNBOUNDED_MAX, then the bounded_add and friends functions will already set extraCellsBound[0] and extraCellsBound[1] to UBOUNDED_MAX. Therefore the branch doesn't have any computational effect. It is also not a case worth optimizing for.

The case of DISCONNECT is similar, however possibly only one of extraCellsBound[0] or extraCellsBound[1] will end up being set to UBOUNDED_MAX instead of both.

This is fine, because these values are just intermediate values for the *cellsBound computation at the end of the function. There we take the bounded_max of both the extraCellsBound[0] and extraCellsBound[1] functions. So the result is the same whether or not both extraCellsBound values are maxed out or only one is.

[roconnor-blockstream]

How did we get here

It's worth reviewing how this code came to be before tearing down this "Chesterton's fence".

Originally we didn't compute extraCellsBound and only extraUWORDBound

simplicity/C/eval.c

Lines 711 to 722 in ba3e39f

	if (SIZE_MAX <= type_dag[COMP_B(dag, type_dag, i)].bitSize) {
	/* 'BITSIZE(B)' has exceeded our limits. */
	bound[i].extraUWORDBound[0] = SIZE_MAX;
	bound[i].extraUWORDBound[1] = SIZE_MAX;
	} else {
	size_t scratch = ROUND_UWORD(type_dag[COMP_B(dag, type_dag, i)].bitSize);
	bound[i].extraUWORDBound[0] = max( bounded_add( scratch
	, max( bound[dag[i].child[0]].extraUWORDBound[0]
	, bound[dag[i].child[1]].extraUWORDBound[1] ))
	, bound[dag[i].child[1]].extraUWORDBound[0] );
	bound[i].extraUWORDBound[1] = bounded_add(scratch, bound[dag[i].child[0]].extraUWORDBound[1]);
	}

Back then, for COMP, type_dag[COMP_B(...)].bitSize was passed to ROUND_UWORD and we needed the check to make sure we had not hit the maximum value yet before doing that. (And similarly for DISCONNCECT).

In b075e2c we added the computation for extraCellsBound to the extraUWORDBound, keeping the same if structure and keeping the extraUWORDBound computation the same.

In 6f3a3e0 we decide that it was fine for extraUWORDBound to perform unsigned overflow because it can only overflow if extraCellsBound maxes out, and we already check that case for extraCellsBound. See the comment

simplicity/C/eval.c

Lines 682 to 691 in 5afc82e

	/* Sum up the total costs.
	* The computations for extraCells and cost are clipped at UBOUNDED_MAX,
	* so a result of UBOUNDED_MAX means "UBOUNDED_MAX or larger".
	*
	* The extraUWORD computation may produce unsigned overflow.
	* However the extraUWORD true value is less than the true value of extraCells.
	* As long as extraCells is strictly less than UBOUNDED_MAX, extraUWORD will be too.
	*
	* The extraFrame computation is bounded by DAG_LEN, and cannot overflow.
	*/

With this change, the whole existence of the if branch became redundant.

[apoelstra]

ACK 8fa7899; successfully ran local tests