[PATCH] D156109: [AArch64][SME] Create new interface for isFullSVEAvailable.
Paul Walker via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Thu Aug 31 06:42:38 PDT 2023
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
More information about the llvm-commits
mailing list