[PATCH] D156109: [AArch64][SME] Create new interface for isFullSVEAvailable.

[Sander de Smalen via Phabricator via llvm-commits]

sdesmalen marked an inline comment as done.
sdesmalen added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64Subtarget.h:221
+  /// in streaming-SVE mode or when the target has FEAT_FA64 enabled.
+  bool isFullSVEAvailable() const;
+
----------------
paulwalker-arm wrote:
> I don't like this name.  FullSVE isn't really a thing, you either have SVE or you don't, much like with NEON.  Can this just be `isSVEAvailable()`?
I'm happy to change it to `isSVEAvailable()`, but it's perhaps a bit confusing in the context of other interfaces such as `useSVEforFixedLengthVectors`, where `SVE` has a meaning where it allows for the streaming-compatible subset of SVE, whereas for `isSVEAvailable` it strictly relates to the full set of SVE instructions.


================
Comment at: llvm/test/CodeGen/AArch64/sve-fp-reduce-fadda.ll:4-5
 
-; FIXME: Streaming-compatible SVE doesn't include FADDA, so this shouldn't compile!
-; RUN: llc -mattr=+sve -force-streaming-compatible-sve < %s | FileCheck %s
+; Streaming-compatible SVE doesn't include FADDA, so this shouldn't compile!
+; RUN: not --crash llc -mattr=+sve -force-streaming-compatible-sve < %s
 
----------------
paulwalker-arm wrote:
> I'm not sure it worth having RUN lines that we know will crash?  If we care about ensuring such IR isn't code generated then we should update the IR Verifier to generate a clean failure rather than require a compiler crash.
I'd rather keep the RUN line to make sure the code I added in the patch is protected by a test and therefore cannot be removed without breaking something.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156109