[PATCH] D68203: Add support for (expressing) vscale.

[Sander de Smalen via Phabricator via llvm-commits]

sdesmalen marked 6 inline comments as done.
sdesmalen added inline comments.


================
Comment at: llvm/docs/LangRef.rst:17753
+execution, but is unknown at compile time. The ``scaling`` immediate is
+multiplied at runtime with ``vscale``.
+
----------------
efriedma wrote:
> What happens if the multiply overflows?
Fixed by removing the scaling argument and leaving this to an explicit `mul` (with wrapping flags to express (lack of) overflow).


================
Comment at: llvm/include/llvm/IR/PatternMatch.h:2021
+  int &Val;
+  VScaleVal_match(const DataLayout &DL, int &S) : DL(DL), Val(S) {}
+
----------------
SjoerdMeijer wrote:
> Nit: how about renaming `S` to `Scaling` or something along those lines?
Because the scaling argument has been removed from the intrinsic, I've also removed the matching of any scaling from this function.


================
Comment at: llvm/lib/Target/AArch64/AArch64SVEInstrInfo.td:1108
+  // General case that we ideally never want to match.
+  def : Pat<(vscale GPR64:$scale), (MADDXrrr (UBFMXri (RDVLI_XI 1), 4, 63), $scale, XZR)>;
+
----------------
SjoerdMeijer wrote:
> last question: is there a test case for this, can this be triggered?
Yes! `rdvl_3` is the test addresses that case.


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

https://reviews.llvm.org/D68203