On 16/01/2026 14:51, Alex Coplan wrote:
Hi Alfie,
Thanks for taking the comments on board, a few more notes below.
Hi Alex,
Thank you for the review. I was not to sure about the code structure so
happy to get some more feedback.
Any removed feedback Ive agreed with and changed.
@@ -18,23 +18,7 @@
;; along with GCC; see the file COPYING3. If not see
;; <http://www.gnu.org/licenses/>.
-;; Code organisation:
-;
-;; The lines of is an instruction is aarch64, simd, sve, sve2, or sme are
-;; a little blurry.
-;;
-;; Therefore code is organised by the following rough principles:
-;;
-;; - aarch64.md: For shared parts of the architecture (such as defining
-;; registers and constants) and for instructions that operate on non-SIMD
-;; registers.
-;; - aarch64-simd.md: For instructions that operate on non-scaling SIMD
-;; registers.
-;; - aarch64-sve.md for SVE instructions that are pre SVE2.
-;; - aarch64-sme.md for any scalable SIMD instruction that is incompatible with
-;; non-streaming mode. This usually means it uses the ZA or ZT register.
-;; - aarch64-sve2.md for any scalable SIMD instruction that either is
-;; streaming compatible, or theoretically could be later.
+;; Code organisation: See comment in aarch64.md.
GCC coding style is to use American spelling, so this needs to be
"organization".
Sorry about that. Just migrated machine and didn't install my American
English spell checker :(
Fixed, here and elsewhere.
Perhaps we should be a bit more specific, say:
"See block comment at the top of aarch64.md."
otherwise the reader might not know where to look.
Same applies for the other occurrences in the rest of the patch, of
course.
Agreed, changed.
I think "a little blurry" is perhpas a bit unfair here, to what is some
pretty well-organized code. How about "perhaps a little non-obvious"
instead?
Agreed
- aarch64-sve-builtins-sme.def for any intrinsic that is fundamentally
incompatible with non-streaming mode. This usually means it uses the ZA
Instead of "fundamentally incompatible with non-streaming mode", how
about "only useable in streaming mode"? Either way is fine by me.
I came up with this with Alice. I was trying to stress that the file is
for instructions/intrinsics (resp) for instructions that won't and can't
be made non-streaming compatible later.
I dont love the wording but I think "only useable in streaming mode"
gives the wrong impression as there are some streaming only
intrinsics/instructions in the SVE2 files as they don't use the ZA/ZT
register and so theoretically could be non-streaming later.
;;
;; - aarch64.md: For shared parts of the architecture (such as defining
;; registers and constants) and for instructions that operate on non-SIMD
;; registers.
I think "general-purpose registers" might read better than non-SIMD
registers, if you agree? Sorry for not mentioning that last time
round.
This was me hedging because cases like FP register instructions or FPMR
register were are in aarch64.md mostly and so I wanted to cover that?
-;; - aarch64-simd.md: For instructions that operate on non-scaling SIMD
+;; - aarch64-simd.md: For instructions that operate on Advanced SIMD
;; registers.
;; - aarch64-sve.md for SVE instructions that are pre SVE2.
;; - aarch64-sme.md for any scalable SIMD instruction that is incompatible
with
;; non-streaming mode. This usually means it uses the ZA or ZT register.
-;; - aarch64-sve2.md for any scalable SIMD instruction that either is
-;; streaming compatible, or theoretically could be later.
+;; - aarch64-sve2.md for any scalable SIMD instruction that is enabled by an
+;; SVE2+ extension.
Hope this is constructive.
Very much so, thank you again.
Thanks,
Alex
;; Register numbers
(define_constants
--
2.34.1
