This patch addresses the BuDDy part of #396,
reported by Florian Renkin and Reed Oei.
* src/kernel.c, src/kernel.h: Declare a bddrecstack and
associated macros. Resize it when new variable are declared.
* src/cache.h: Add a BddCache_index macro.
* src/bddop.c (not_rec, apply_rec, quant_rec, appquant_rec,
support_rec, ite_rec, compose_rec, restrict_rec, satone_rec,
satoneset_rec): Rewrite using this stack to get rid of the recursion.
* src/bddop.c, src/bddx.h, src/cache.c, src/cache.h, src/kernel.c,
src/kernel.h, src/prime.c, src/prime.h, src/reorder.c: Use power of
two for the sizes of all hash tables, in order to reduce the amount of
divisions performed. Also allow bddhash to be smaller than bddnodes.
* src/bddop.c: Empty macro arguments are undefined ISO C90 and
ISO C++98. Use '+' when calling APPLY_SHORTCUTS.
* src/fdd.c, src/kernel.c: Avoid constructs invalid in C90.
* src/bddop.c, src/bddx.h, src/kernel.c, src/kernel.h,
examples/cmilner/cmilner.c: Remove C++ comments.
The bug was found while running Spot's src/tgbatest/randpsl.test
on Debian i386 with gcc-4.9.2. The following call would crash:
./ltl2tgba -R3 -t '(!(F(({{{(p0) |
{[*0]}}:{{{(p1)}[*2]}[:*]}[*]:[*2]}[:*0..3]}[]-> (G(F(p1)))) &
(G((!(p1)) | ((!(p2)) W (G(!(p0)))))))))'
On amd64 the call does not crash, but valgrind nonetheless
report that uninitialized memory is being read by bdd_gbc()
during the second garbage collect.
* src/kernel.h (PUSHREF): Define as a function rather than a macro
to avoid undefined behavior. See comments for details.
The unicity table was mixed with the bddNode table for now
apparent reason. After the hash of some node is computed,
bddnodes[hash] did only contain some random node (not the one
looked for) whose .hash member would point to the actual node with
this hash. So that's a two step lookup. With this patch, we sill
have a two step lookup, but the .hash member have been moved to a
separate array. A consequence is that bddNode is now 16-byte long
(instead of 20) so it will never span across two cache lines.
* src/kernel.h (bddNode): Remove the hash member, and move it...
(bddhash): ... as this new separate table.
* src/kernel.c, src/reorder.c: Adjust all code.
Remove more sanity checks when NDEBUG is set.
* src/kernel.h (CHECKnc): New macro.
* src/kernel.c (bdd_var, bdd_low, bdd_high, bdd_ithvar,
bdd_nithvar): Use it.
Inline bdd_addref() and bdd_delref() to speedup BDD operations.
* src/kernel.c, src/kernel.h (bdd_addref, bdd_delref): Move these
functions and there associated global variables...
* src/bdd.c (bdd_error): ... and this function ...
* src/bdd.h (bdd_addref, bdd_delref, bdd_error): ...here so that
they can be inlined.
