Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bitblasting a multiplication by a constant should give a network of full adders #1828

Closed
eddywestbrook opened this issue Mar 2, 2023 · 4 comments · Fixed by #1833
Closed

Comments

@eddywestbrook
Copy link
Contributor

If you bitblast a multiplication of a variable on the left by a constant on the right, write_aig will generate a representation that is a network of full adder circuits, which is useful for methods developed for analyzing circuits with full adders. But if you bitblast a constant multiplication where the variable is on the right, the resulting circuit looks very different, and does not use full adders.

The following demonstrates the problem:

sawscript> write_aig "left.aig" {{ \x -> x * 0x12345678 }}
sawscript> write_aig "right.aig" {{ \x -> 0x12345678 * x }}
sawscript> :q
$ abc -c "&r left.aig; &st; &if -K 3; &ps -n"
ABC command line: "&r left.aig; &st; &if -K 3; &ps -n".

left     : i/o =     32/     32  and =    2485  lev =  100 (47.69)  mem = 0.03 MB
Mapping (K=3)  :  lut =    509  edge =    1403  lev =   38 (17.62)  mem = 0.02 MB
NPN CLASS STATISTICS (for 509 LUT4 present in the current mapping):
  1: Class   5 :  Count =      2   (   0.39 %)   F = (!d*!c)
  2: Class  13 :  Count =      9   (   1.76 %)   F = (!d*(c+b))
  3: Class  15 :  Count =      1   (   0.20 %)   F = (!d*!(c*b))
  4: Class  83 :  Count =    187   (  36.67 %)   F = 17(b,c,d)
  5: Class 120 :  Count =      3   (   0.59 %)   F = (d+!(!c*!b))
  6: Class 180 :  Count =    123   (  24.12 %)   F = (d+c)
  7: Class 220 :  Count =    184   (  36.08 %)   F = (d+(c+!b))
Other     :  Count =      0   (   0.00 %)
Approximate number of 4:1 MUX structures: All =      0  (   0.00 %)  MFFC =      0  (   0.00 %)

$ abc -c "&r right.aig; &st; &if -K 3; &ps -n"
ABC command line: "&r right.aig; &st; &if -K 3; &ps -n".

right    : i/o =     32/     32  and =    4256  lev =  162 (96.81)  mem = 0.05 MB
Mapping (K=3)  :  lut =    922  edge =    2429  lev =   51 (28.34)  mem = 0.03 MB
NPN CLASS STATISTICS (for 922 LUT4 present in the current mapping):
  1: Class   2 :  Count =      2   (   0.22 %)   F = (!d*(!c*!b))
  2: Class   5 :  Count =    299   (  32.39 %)   F = (!d*!c)
  3: Class  15 :  Count =      3   (   0.33 %)   F = (!d*!(c*b))
  4: Class  21 :  Count =    253   (  27.41 %)   F = (!d)
  5: Class  81 :  Count =      3   (   0.33 %)   F = 16(b,c,d)
  6: Class  83 :  Count =      1   (   0.11 %)   F = 17(b,c,d)
  7: Class 103 :  Count =      3   (   0.33 %)   F = 18(b,c,d)
  8: Class 105 :  Count =      3   (   0.33 %)   F = 19(b,c,d)
  9: Class 120 :  Count =    327   (  35.43 %)   F = (d+!(!c*!b))
 10: Class 180 :  Count =     28   (   3.03 %)   F = (d+c)
Other     :  Count =      0   (   0.00 %)
Approximate number of 4:1 MUX structures: All =      0  (   0.00 %)  MFFC =      0  (   0.00 %)

In left.aig, the circuit almost exclusively uses xors (d+c and d+c+b) and majority gates (17(b,c,d)), whereas right.aig looks very different.

@RyanGlScott
Copy link
Contributor

Most likely, this issue revolves around the aig library's implementation of bitvector multiplication:

-- | Multiply two bitvectors with the same size, with result
--   of the same size as the arguments.
--   Overflow is silently discarded.
mul :: IsAIG l g => g s -> BV (l s) -> BV (l s) -> IO (BV (l s))
mul g x y = assert (length x == length y) $ do
  -- Create mutable array to store result.
  let n = length y
  -- Function to update bits.
  let updateBits i z | i == n = return z
      updateBits i z = do
        z' <- iteM g (y ! i) (add g z (shlC g x i)) (return z)
        updateBits (i+1) z'
  updateBits 0 $ replicate (length x) (falseLit g)

This implements multiplication using a shift-and-add algorithm that adds multiples of x (i.e., partial products) based on the bits in y. As such, the order of arguments matters.

What is interesting about this implementation is that when y is symbolic, this will construct a mux tree where the leaves call add. I'm not sure if this is necessarily an optimal arrangement, as I would imagine that you'd want the AIG to have full adders that push as much symbolic information into the leaves as possible. (This is inspired by the multiplier_encoding algorithm in this paper). With that in mind, here is one possible way to tweak mul to expose more of this full adder structure:

diff --git a/src/Data/AIG/Operations.hs b/src/Data/AIG/Operations.hs
index a48b11a..2c698a8 100644
--- a/src/Data/AIG/Operations.hs
+++ b/src/Data/AIG/Operations.hs
@@ -629,12 +629,14 @@ mul :: IsAIG l g => g s -> BV (l s) -> BV (l s) -> IO (BV (l s))
 mul g x y = assert (length x == length y) $ do
   -- Create mutable array to store result.
   let n = length y
+  let zeroes = replicate (length x) (falseLit g)
   -- Function to update bits.
   let updateBits i z | i == n = return z
       updateBits i z = do
-        z' <- iteM g (y ! i) (add g z (shlC g x i)) (return z)
+        x' <- ite g (y ! i) x zeroes
+        z' <- add g z (shlC g x' i)
         updateBits (i+1) z'
-  updateBits 0 $ replicate (length x) (falseLit g)
+  updateBits 0 zeroes
 
 -- | Unsigned multiply two bitvectors with size @m@ and size @n@,
 --   resulting in a vector of size @m+n@.

Note how the muxing has been pushed into the add, whereas the old code did this the other way around. If you build SAW with this version of aig and repeat the experiment above:

sawscript> write_aig "left.aig" {{ \x -> x * 0x12345678 }}
sawscript> write_aig "right.aig" {{ \x -> 0x12345678 * x }}

Then running abc on left.aig produces identical results. Running abc on right.aig, on the other hand, produces different results:

$ abc -c "&r right.aig; &st; &if -K 3; &ps -n"
ABC command line: "&r right.aig; &st; &if -K 3; &ps -n".

right    : i/o =     32/     32  and =    3546  lev =  162 (70.94)  mem = 0.04 MB
Mapping (K=3)  :  lut =    763  edge =    1918  lev =   50 (21.19)  mem = 0.03 MB
NPN CLASS STATISTICS (for 763 LUT4 present in the current mapping):
  1: Class   2 :  Count =      5   (   0.65 %)   F = (!d*(!c*!b))
  2: Class   5 :  Count =    117   (  15.31 %)   F = (!d*!c)
  3: Class  13 :  Count =      9   (   1.18 %)   F = (!d*(c+b))
  4: Class  15 :  Count =     30   (   3.93 %)   F = (!d*!(c*b))
  5: Class  81 :  Count =      3   (   0.39 %)   F = 16(b,c,d)
  6: Class  83 :  Count =    137   (  17.93 %)   F = 17(b,c,d)
  7: Class 103 :  Count =      3   (   0.39 %)   F = 18(b,c,d)
  8: Class 105 :  Count =      3   (   0.39 %)   F = 19(b,c,d)
  9: Class 120 :  Count =     43   (   5.63 %)   F = (d+!(!c*!b))
 10: Class 180 :  Count =    256   (  33.51 %)   F = (d+c)
 11: Class 220 :  Count =    157   (  20.55 %)   F = (d+(c+!b))
Other     :  Count =      0   (   0.00 %)
Approximate number of 4:1 MUX structures: All =      0  (   0.00 %)  MFFC =      0  (   0.00 %)

I am not an expert on abc's &ps output, so it is somewhat difficult for me to determine if this makes a difference vis-à-vis verification. But I do note that this new output now has far more XORs: there are 256 (d+c) and 157 (d+(c+!b)), whereas the old right.aig output only has 28 (d+c) and 0 (d+(c+!b)). Similarly, the new right.aig ouput now has 137 17(b,c,d) majority gates, whereas the old output only has 1. There are lots of other class numbers that I don't quite understand (for example, what do the 117 (!d*!c) represent?), but this is still a remarkable difference for such a small change.

@RyanGlScott
Copy link
Contributor

Another approach is to observe that mul is biased towards one argument and to "normalise symmetry", i.e., possibly rewrite mul x y to mul y x depending on if one of the arguments is a constant. (The phrase "normalising symmetry" is taken from the same paper above that discusses the multiplier_encoding algorithm.) This might be worth doing independently of the change proposed above, although it is unclear to me if it is even desirable to have non-normalised multiplications when bitblasting.

@eddywestbrook
Copy link
Contributor Author

I think it makes sense to normalize symmetry here, simply because it allows for more partial evaluation. In either approach, the old one or the one proposed above, the algorithm does a mux on y ! i. This can be simplified statically to the left or right branch of the mux when y is a constant, leading to a much smaller circuit, so we might as well take advantage of that whenever possible.

I also feel like your proposed change above of pushing the add to the leaves is probably a good change too, but it's a little more difficult to assess without a meaningful metric.

RyanGlScott added a commit to GaloisInc/aig that referenced this issue Mar 3, 2023
RyanGlScott added a commit to GaloisInc/aig that referenced this issue Mar 3, 2023
@RyanGlScott
Copy link
Contributor

I've pushed implementations of each of the ideas above as branches in the aig library:

@weaversa, can you try building SAW with each of these commits in the deps/aig submodule in saw-script and evaluating the results that write_aig produces? If you've already built SAW from source locally, then it's fairly easy to rebuild it after changing the aig submodule, as not much depends on it.

RyanGlScott added a commit to GaloisInc/aig that referenced this issue Mar 6, 2023
`mul` will now flip the order of arguments in `mul` if it detects that the
first argument is a constant, as this can produce more desirable AIGs if the
other argument is symbolic. This is part of an eventual fix for
GaloisInc/saw-script#1828.
RyanGlScott added a commit to GaloisInc/aig that referenced this issue Mar 6, 2023
`mul` will now flip the order of arguments in `mul` if it detects that the
first argument is a constant, as this can produce more desirable AIGs if the
other argument is symbolic. This is part of an eventual fix for
GaloisInc/saw-script#1828.
RyanGlScott added a commit that referenced this issue Mar 6, 2023
This bumps the `aig` submodule to bring in the changes from GaloisInc/aig#14,
which changes how `aig` performs multiplication to swap the order of arguments
if the first argument is a constant. This is because the algorithm that `mul`
uses to compute multiplication is biased in its order of arguments, and having
the second argument be a constant produces a network with more full adders,
which is generally more beneficial.  I have added a test case to check to see
if the test case from #1828 now produces identical AIGs regardless of the order
of arguments.

Fixes #1828.
RyanGlScott added a commit to GaloisInc/aig that referenced this issue Mar 6, 2023
…plication

This tends to play better with libraries such as What4, which use a similar
convention. See the Haddocks for `mul`.

This is part of an eventual fix for GaloisInc/saw-script#1828.
RyanGlScott added a commit to GaloisInc/aig that referenced this issue Mar 6, 2023
…plication

This tends to play better with libraries such as What4, which use a similar
convention. See the Haddocks for `mul`.

This is part of an eventual fix for GaloisInc/saw-script#1828.
RyanGlScott added a commit to GaloisInc/aig that referenced this issue Mar 9, 2023
`mul` will now flip the order of arguments in `mul` if it detects that the
second argument is a constant, as this can produce more desirable AIGs if the
other argument is symbolic. This is part of an eventual fix for
GaloisInc/saw-script#1828.
RyanGlScott added a commit that referenced this issue Mar 9, 2023
This bumps the `aig` submodule to bring in the changes from GaloisInc/aig#14,
which changes how `aig` performs multiplication to swap the order of arguments
if the second argument is a constant. This has two benefits:

* This ensures that `write_aig` will produce the same networks for `X * C` and
  `C * X`, where `C` is a constant and `X` is a variable.
* The algorithm that `mul` uses to compute multiplication is biased in its order
  of arguments, and having the first argument be a constant tends to produce
  networks that ABC has an easier time verifying. (The FFS example under
  `doc/tutorial/code/ffs.c` is a notable example of this.)

I have added a test case to check to see if the test case from #1828 now
produces identical AIGs regardless of the order of arguments.

Fixes #1828.
RyanGlScott added a commit that referenced this issue Mar 9, 2023
The fix for #1828 also addresses many of the symptoms of #1788, so let's add
a test case to ensure that those symptoms remain cured.
RyanGlScott added a commit that referenced this issue Mar 9, 2023
This bumps the `aig` submodule to bring in the changes from GaloisInc/aig#14,
which changes how `aig` performs multiplication to swap the order of arguments
if the second argument is a constant. This has two benefits:

* This ensures that `write_aig` will produce the same networks for `X * C` and
  `C * X`, where `C` is a constant and `X` is a variable.
* The algorithm that `mul` uses to compute multiplication is biased in its order
  of arguments, and having the first argument be a constant tends to produce
  networks that ABC has an easier time verifying. (The FFS example under
  `doc/tutorial/code/ffs.c` is a notable example of this.)

I have added a test case to check to see if the test case from #1828 now
produces identical AIGs regardless of the order of arguments.

Fixes #1828.
RyanGlScott added a commit that referenced this issue Mar 9, 2023
The fix for #1828 also addresses many of the symptoms of #1788, so let's add
a test case to ensure that those symptoms remain cured.
@mergify mergify bot closed this as completed in #1833 Mar 9, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants