dialects (arm): Add FP/SIMD register type #4034
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #4034 +/- ##
==========================================
+ Coverage 90.20% 90.23% +0.02%
==========================================
Files 458 459 +1
Lines 58353 58422 +69
Branches 5694 5689 -5
==========================================
+ Hits 52637 52716 +79
+ Misses 4325 4321 -4
+ Partials 1391 1385 -6 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| @@ -36,6 +38,27 @@ def infinite_register_prefix(cls): | |||
| return "inf_" | |||
|
|
|||
|
|
|||
| @irdl_attr_definition | |||
| class FPSIMDRegisterType(ARMRegisterType): | |||
There was a problem hiding this comment.
This is Neon right? Maybe a better name for this as is kinda unique for Arm?
Moreover, these registers support both int and FP, so just using FP in the name doesn't provide any protection or restrict any use. Maybe drop?
There was a problem hiding this comment.
I agree it's a bit confusing but the docs are a bit unclear on this. These registers are called SIMD & FP registers in the documentation. (As I understand, FP and SIMD instructions (on both fp and integer values) share the same register set).
Also they are used by these instructions (https://developer.arm.com/documentation/ddi0602/2024-12/SIMD-FP-Instructions?lang=en) and it's unclear to me whether or not they are a part of the NEON instruction set because it's not clearly specified. Happy to change it though. I can see why fpsimd isn't an accurate name but I'm not sure if restricting the register type to neon insns is fully correct either
There was a problem hiding this comment.
Yeah, Arm has done a great job with the naming (from ISA names down to extensions). I don't feel strongly about this.
I mostly look at this reference.
My (flawed?) understanding is that there are SIMD instructions that work on GP registers (e.g. packing 16-bit values to 32-bit registers) and then Neon which the advanced SIMD that operates on vector registers.
There is this great document showing how these registers have "shared views" and it depends on the actual instruction which of their parts are accessed. I haven't been able to find an updated version, so if you have one, I'd love to have it. The website is another mess.
There is some overlap here as well.
There was a problem hiding this comment.
thanks for that @compor. I'm also finding it hard to navigate the docs and there seem to be a lot of versions floating around. I think the last link explains how these registers can be used for either FP ops or SIMD ops on either fp or int values. I guess the FP ops are not strictly Neon but can still use these registers. I am just thinking of what best to change the name to. Maybe @superlopuh @qaco have ideas
There was a problem hiding this comment.
I would maybe err on the side of "explicit IR in xDSL" at the expense of "reads like ARM assembly", and call these NEON32, NEON64, and NEON128 registers? Especially if they can be used in both float and integer operations. The register here would carry the information about the register set and width, and the operation would determine how the registers would be interpreted.
There was a problem hiding this comment.
I agree on adding the width of registers and yes they can operate on integers (e.g., https://gcc.godbolt.org/z/Y5fer6dPY), so I'd drop the FP prefix.
| def get_arm_simd(): | ||
| from xdsl.dialects.arm_simd import ARM_SIMD | ||
|
|
||
| return ARM_SIMD |
There was a problem hiding this comment.
still need to decide on the naming here so this is subject to change
dialects (arm): Add FP/SIMD register type
Add a new register type for ARM FP/SIMD registers, in preparation for adding SIMD instructions to ARM dialect.