stanford-ppl / spatial

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

Host vs. Accel DRAM syntax is confusing #138

Open dkoeplin opened 6 years ago

dkoeplin commented 6 years ago

Noticed the new distinction between accel and host DRAM. While this is good, the way of creating these is not at all clear. At the moment, calling DRAM[Int](32) creates a host DRAM while calling DRAM1[Int](32) creates an Accel DRAM.

It would be much better if AccelDRAM was the default (since that's what it previously was), and there was an option to do something like HostDRAM[Int](32) or DRAM.host[Int](32). Something that makes it obvious this is a different DRAM type.

Additionally, what's the contract for using accelerator DRAM now? It looks like everything is dynamically allocated, even things which might have previously been known statically? Does HostDRAM actually live in the host's memory? (I saw that the previous names for these were DRAMStaticNew and DRAMDynNew, which seems like better names for these if that's actually what they are for)

mattvilim commented 6 years ago

Just to clarify, the current API is:

// dram of statically known dimensions that is allocated by the host and passed in with implicit ArgIns to be shared with the accelerator; this hasn't changed at all from the previous design. It's staged as a DRAMHost node
val host = DRAM[Int](dims...)
// dram of fixed rank but variable dimensions; it's inaccessible to the host and is managed by the accelerator; the current implementation of the hardware heap pokes some registers on the host to request an allocation, but eventually it will be managed fully in hardware. It's staged as a DRAMAccel node.
val accel = DRAM1,2,3,4,5[Int]
accel.alloc(dims...)
accel.dealloc

Would you prefer changed the lang api to DRAMAccel1,2,3,4,5[Int] to make it more clear that this DRAM is managed completely by the accelerator?

dkoeplin commented 6 years ago

That clears things up actually. I think in that case yeah, it would be good just to note that it's accelerator managed memory. Something like AccelDRAM or DynamicDRAM would be a nice mnemonic, otherwise it's hard to distinguish from the host-managed DRAM.