search

stanford-ppl / spatial

Spatial: "Specify Parameterized Accelerators Through Inordinately Abstract Language"
https://spatial.stanford.edu
MIT License
271 stars 32 forks source link

Accum Analyzer Messing Up Cases #259

Open mattfel1 opened 4 years ago

mattfel1 commented 4 years ago

Accum Analyzer seems to be missing cases to optimize an accumulation and is doing the optimization in cases where it shouldn't.

See https://github.com/stanford-ppl/spatial/blob/develop/test/spatial/tests/compiler/AccumReplacements.scala for a variety of cases we expect it to hit or miss.

What needs to be changed is in AccumAnalyzer.scala.

1) Seems like depending on the order of the ops, it is missing the opportunity to specialize 2) Inspect the mux condition and decide if it should be inverted OR if it is not in the form iterator == 0 OR if the mux args seem to be reversed. Should not optimize in the 2nd and 3rd cases

mattfel1 commented 4 years ago

@kelayamatoz Just added a few more test cases

The metadata set on the registers saying they are Folds accumulations is because of the spatial flow rule, which happens when the reg gets created:

 mem.accumType = AccumType.Fold & mem.accumType

And the boolean logic here is:

  case object Fold extends AccumType {
    def &(that: AccumType): AccumType = that match {case Unknown => Fold; case _ => that }

The place to solve this problem is still the AssociateReduce object in AccumAnalyzer. You need to have more/smarter cases for it to match on, and you also need to fix the ens that it passes along to the AccumMarker for those weird cases. You will want to use val iters = accessIterators(access, reg) to collect the seq of iterators between the mem declaration and the access and use this to see if the mux condition is properly formed. You also will want to change the unapplies for the associated nodes (RegAdd/AddReg/MulReg/RegMul objects, or nodes with .isAssociative true) so that it is allowed to find the mux node somewhere higher up in the tree (i.e. acc14 in the test app)