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

[Sander de Smalen via Phabricator via llvm-commits]

sdesmalen added a comment.

Just left a new nits, but otherwise looks fine to me.



================
Comment at: llvm/lib/Target/AArch64/AArch64Subtarget.cpp:214
+                                   const TargetMachine &TM, bool LittleEndian,
+                                   unsigned MinSVEVectorSizeInBitsOverride,
+                                   unsigned MaxSVEVectorSizeInBitsOverride)
----------------
nit: is it worth making these Optional, if the subtarget doesn't have SVE?


================
Comment at: llvm/lib/Target/AArch64/AArch64Subtarget.cpp:226
+                                 : SVEVectorBitsMaxOpt),
       TargetTriple(TT), FrameLowering(),
       InstrInfo(initializeSubtargetDependencies(FS, CPU)), TSInfo(),
----------------
nit: Does this need an assert that Min|MaxSVEVectorSizeInBits is zero when SVE is not enabled in the feature string?


================
Comment at: llvm/test/CodeGen/AArch64/sve-vscale-attr.ll:134-139
+attributes #0 = { "target-features"="+sve" }
+attributes #1 = { "target-features"="+sve" vscale_range(1,1) }
+attributes #2 = { "target-features"="+sve" vscale_range(2,2) }
+attributes #3 = { "target-features"="+sve" vscale_range(2,4) }
+attributes #4 = { "target-features"="+sve" vscale_range(4,4) }
+attributes #5 = { "target-features"="+sve" vscale_range(8,8) }
----------------
nit: I'm not sure if the attributes need to be at the end of the file necessarily, but if not, can you move each of them below the function that uses it?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103702