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

Matt Devereau via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Wed Aug 2 07:25:54 PDT 2023


MattDevereau added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64Subtarget.cpp:493
+bool AArch64Subtarget::isFullSVEAvailable() const{
+  // The 'force-streaming-comaptible-sve' flag overrides the streaming
+  // function attributes.
----------------
Typo, comaptible -> compatible.

It might be nice to include the "-" on the front as well.


================
Comment at: llvm/lib/Target/AArch64/AArch64Subtarget.cpp:495-496
+  // function attributes.
+  if (ForceStreamingCompatibleSVE.getNumOccurrences() > 0)
+    return !ForceStreamingCompatibleSVE;
+
----------------
Using just `if (ForceStreamingCompatibleSVE)` is passing check-llvm for me, so either this can be simplified or a test is missing.

If a test is missing, this check is copy/pasted from `isNeonAvailable` above. We can separate this into a new function such as

```
bool AArch64Subtarget::isForceStreamingCompatibleSVE() const{
  return ForceStreamingCompatibleSVE.getNumOccurrences() > 0;
}
```
Then we can simplify things to 
```
bool AArch64Subtarget::isFullSVEAvailable() const{
  return hasSVEorSME() && !StreamingSVEMode && !StreamingCompatibleSVEMode && !isForceStreamingCompatibleSVE();
}
```
And provide a nice check from the subtarget.

If `ForceStreamingCompatibleSVE.getNumOccurrences() > 0;` and `ForceStreamingCompatibleSVE` are equivalent then you can just do

`return hasSVEorSME() && !StreamingSVEMode && !StreamingCompatibleSVEMode && !ForceStreamingCompatibleSVEMode;`


================
Comment at: llvm/lib/Target/AArch64/AArch64Subtarget.h:220-221
+  /// instructions, for example because it knows the function is known not to be
+  /// in streaming-SVE mode, or when the target has FEAT_FA64 enabled (which
+  /// enables the full set of SVE instructions regardless of streaming mode)
+  bool isFullSVEAvailable() const;
----------------
Maybe its best just to mention the feature instead of explaining it here since there will likely be a more accurate description later that diverges from this text?


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