stanford-ppl / spatial

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

Look-Through RegReads in Access Analyzer #104

Open mattfel1 opened 6 years ago

mattfel1 commented 6 years ago

See KMP.

Looking into this now to see if its something stupid

dkoeplin commented 6 years ago

Noticed this one on Friday - it's gotta be something dumb in the way we've defined the recursive Affine unapply where it doesn't terminate in some specific case.

mattfel1 commented 6 years ago

Overflow is because we have

SRAMRead(ofs = RegRead(reg) - 1) And I put something like the friendly transformer to look at most recent write of a register and use that instead because it kept making unnecessary duplication any time a register was inserted somewhere in the address path. This reg has its most recent write as RegWrite(reg, reg + 1)

I'm going to ignore most recent writes if they are part of an accum cycle, which should break up the stack overflow. Really we only care about iterators that are hidden behind RegReads so maybe there is a more general way to do this, but it has worked as expected so far except for this case.

Seeing if this causes any surprising changes in compile times

dkoeplin commented 6 years ago

Ah good catch, yeah a cycle like that would cause this infinite recursion issue.

We should double check that your short circuiting logic between RegRead and RegWrite is generally legal - i.e. if there are multiple potential writes that can reach that read, the read should appear random unless we can say something definite about the intersection of all of the writes

mattfel1 commented 6 years ago

Is there an easy way to check this? Another example would be a RegWrite that may be disabled so the init value shows through. We would need to check the ancestors + enables for that RegWrite and make sure that it is guaranteed to trigger before the banked accesses

dkoeplin commented 6 years ago

There's a reaching write analysis in the memory configurer. It doesn't properly analyze memory spaces yet, but that doesn't matter anyway since these are registers.

mattfel1 commented 6 years ago

Made a special look-thru that doesn't require any isl or metadata that doesn't exist by the time the AccessAnalyzer runs that does basically the same logic as the reachingWrites in access package. Should we keep working on it with better test cases or call this issue closed?

dkoeplin commented 6 years ago

I can take a quick look - which branch is it on?