[PATCH] D103702: [AArch64][SVE] Wire up vscale_range attribute to SVE min/max vector queries

[Bradley Smith via Phabricator via cfe-commits]

bsmith added inline comments.


================
Comment at: llvm/lib/Target/AArch64/AArch64Subtarget.cpp:226
+                                 : SVEVectorBitsMaxOpt),
       TargetTriple(TT), FrameLowering(),
       InstrInfo(initializeSubtargetDependencies(FS, CPU)), TSInfo(),
----------------
sdesmalen wrote:
> nit: Does this need an assert that Min|MaxSVEVectorSizeInBits is zero when SVE is not enabled in the feature string?
In principal you could, however I'm not sure it adds any value, as the accessors already assert that SVE is enabled. (And in principal this is a generic attribute, not an SVE one).


================
Comment at: llvm/lib/Target/AArch64/AArch64TargetMachine.cpp:357
+  Attribute VScaleRangeAttr = F.getFnAttribute(Attribute::VScaleRange);
+  if (VScaleRangeAttr.isValid())
+    std::tie(MinSVEVectorSize, MaxSVEVectorSize) =
----------------
paulwalker-arm wrote:
> I don't know if this is possible but I feel we need a `HasSVE` like check here?
I'm not sure this is really doable here without picking apart the feature string, I think it makes more sense to just set the values and assert when using the accessors without SVE enabled.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103702