[PATCH] D138683: [AArch64][SME]: Add streaming-compatible testing files.

[Sander de Smalen via Phabricator via llvm-commits]

sdesmalen added inline comments.


================
Comment at: llvm/test/CodeGen/AArch64/sve-streaming-mode-fixed-length-permute-rev.ll:183
+
+define void @test_rev_elts_fail(<4 x i64>* %a) #0 {
+; CHECK-LABEL: test_rev_elts_fail:
----------------
This one could be handled with `revd` which was added in SME and/or SVE2p1. If you change the attributes for this to function to e.g. `attributes #1 = { "target-features"="+sve2p1"  }`, then I would expect better code for this. Can you try that?


================
Comment at: llvm/test/CodeGen/AArch64/sve-streaming-mode-fixed-length-permute-rev.ll:240
+
+define void @test_revv32i8_vl256(<32 x i8>* %a) #0 {
+; CHECK-LABEL: test_revv32i8_vl256:
----------------
Given that the above case isn't handled by any `rev` instruction, I don't see much value in this test.
Same holds for all the other `@test_rev<type>_256` tests below.


================
Comment at: llvm/test/CodeGen/AArch64/sve-streaming-mode-fixed-length-permute-rev.ll:409
+
+define void @test_revv8f32_vl256(<8 x float>* %a) #0 {
+; CHECK-LABEL: test_revv8f32_vl256:
----------------
I don't think this one is very different from @test_revv8i32, so you can probably remove it.


================
Comment at: llvm/test/CodeGen/AArch64/sve-streaming-mode-fixed-length-permute-rev.ll:455
+
+define void @test_revv8i32v8i32(<8 x i32>* %a, <8 x i32>* %b) #0 {
+; CHECK-LABEL: test_revv8i32v8i32:
----------------
I don't think this one is very different from `@test_revv8i32`, so you can probably remove it.


================
Comment at: llvm/test/CodeGen/AArch64/sve-streaming-mode-fixed-length-permute-rev.ll:490
+
+define void @test_rev_fail(<16 x i16>* %a) #0 {
+; CHECK-LABEL: test_rev_fail:
----------------
I'm also not sure what this test and the one below are testing, maybe they can also be removed?


================
Comment at: llvm/test/CodeGen/AArch64/sve-streaming-mode-fixed-length-subvector.ll:3
+; RUN: llc -force-streaming-compatible-sve  < %s | FileCheck %s
+
+target triple = "aarch64-unknown-linux-gnu"
----------------
Can you keep the original comment from `sve-fixed-length-subvector.ll`:

  ; Test we can code generater patterns of the form:
  ;   fixed_length_vector = ISD::EXTRACT_SUBVECTOR scalable_vector, 0
  ;   scalable_vector = ISD::INSERT_SUBVECTOR scalable_vector, fixed_length_vector, 0
  ; 
  ; NOTE: Currently shufflevector does not support scalable vectors so it cannot
  ; be used to model the above operations.  Instead these tests rely on knowing
  ; how fixed length operation are lowered to scalable ones, with multiple blocks
  ; ensuring insert/extract sequences are not folded away.

Without that comment, this test doesn't make much sense since it's just loading and storing data, yet the name of the test suggests something different.


================
Comment at: llvm/test/CodeGen/AArch64/sve-streaming-mode-fixed-length-subvector.ll:7
+; i8
+define void @subvector_v4i8(<4 x i8> *%in, <4 x i8>* %out) #0 {
+; CHECK-LABEL: subvector_v4i8:
----------------
We can just use opaque pointers here (`ptr` instead of `<4 x i8>*`), please change that here and in the rest of this file.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D138683/new/

https://reviews.llvm.org/D138683