search

nod-ai / iree-amd-aie

IREE plugin repository for the AMD AIE accelerator
Apache License 2.0
69 stars 30 forks source link

Add HW specific vector sizes to device model #870

Closed newling closed 2 weeks ago

newling commented 2 weeks ago

Initial commit. Future work: move all other HW specific constants here, for example https://github.com/nod-ai/iree-amd-aie/blob/9aeeeb8055cd17798d858f3a0daf795cd637548b/compiler/plugins/target/AMD-AIE/iree-amd-aie/Transforms/AMDAIEUtils.cpp#L50

This PR is depended on by https://github.com/nod-ai/iree-amd-aie/pull/867

newling commented 2 weeks ago

Isn't this already included in: #867 ? I don't think it should be a separate PR.

"This struct is meant to be a thin wrapper around aie-rt" is no longer true. It seems like a significant decision to add information about the core instructions to this struct, that is why I made it a separate PR. But I'm fine just including it in #867

makslevental commented 2 weeks ago

"This struct is meant to be a thin wrapper around aie-rt"

i mean this isn't/wasn't a hard rule - nothing i ever do is like that lol. i think putting arch specific stuff in AMDAIEDeviceModel is a perfectly fine use of the "model". i would prefer it wasn't a nested struct (I had a comment to that effect earlier but removed it since I'm now not qualified to nitpick) but that's fine.

newling commented 2 weeks ago

@makslevental thanks for keeping an eye on this, and you have my permission to nitpick my PRs as much as you like