[PATCH] D114506: [AArch64][SVE]Support for SVE instrinsic in AArch64TargetTransformInfo

Sander de Smalen via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Nov 24 01:26:01 PST 2021


sdesmalen added a comment.

The change itself seems sensible to me. Just left a few comments on the test.



================
Comment at: llvm/test/Transforms/LoopStrengthReduce/AArch64/getTgtMemIntrinsic1.ll:1
+; RUN: opt < %s -loop-reduce
+; ModuleID = 'test.tmp.bc'
----------------
This RUN line doesn't use FileCheck to check the output?


================
Comment at: llvm/test/Transforms/LoopStrengthReduce/AArch64/getTgtMemIntrinsic1.ll:4
+target triple = "aarch64-none-linux-gnu"
+; CHECK: %lsr.iv12 = bitcast i32* %lsr.iv1 to i1*
+
----------------
Can you add some extra CHECK lines, and move the CHECK lines to below `define void @example01_sve`? It's not clear from the CHECK line that loop-reduce is doing the correct thing.

Some other nits:
* Could you please add a comment with context about what this test is supposed to test?
* Maybe remove the name `@example01_sve` to something more descriptive?
* Can you rename the file to something more descriptive, e.g. `sve-load-store-intrinsics.ll` ?


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

https://reviews.llvm.org/D114506



More information about the llvm-commits mailing list