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

[Paul Walker via Phabricator via llvm-commits]

paulwalker-arm added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64Subtarget.cpp:491
+  // as we don't yet support the feature in LLVM.
+  return hasSVEorSME() && !isStreaming() && !isStreamingCompatible();
 }
----------------
Is the presence of SME relevant to this question? Just `hasSVE()` seems more fitting.


================
Comment at: llvm/lib/Target/AArch64/AArch64Subtarget.h:221
+  /// in streaming-SVE mode or when the target has FEAT_FA64 enabled.
+  bool isFullSVEAvailable() const;
+
----------------
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()`?


================
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
 
----------------
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.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D156109