[PATCH] D105673: [SelectionDAG] Fix the representation of ISD::STEP_VECTOR.

Fri Jul 9 01:26:51 PDT 2021

david-arm added a comment.

Hi @efriedma, I've not looked through all the AArch64 pattern changes yet, but on the surface switching from SDValue to APInt for the STEP_VECTOR seems reasonable, although I think @paulwalker-arm may have an interest in this as he left quite a few comments on my original patch regarding the interface and choice of type. I had a couple of comments so far about the vscale changes.

================
Comment at: llvm/lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp:4205

+void DAGTypeLegalizer::ExpandIntRes_VSCALE(SDNode *N, SDValue &Lo,
+                                           SDValue &Hi) {
----------------
Hi @efriedma, the commit message didn't mention anything about changing the legalisation for VSCALE. Is it possible to either 1) add something to commit message describing why we've also had to fix VSCALE legalisation as part of the patch, or 2) is it possible to do this in a separate patch?

================
Comment at: llvm/lib/CodeGen/SelectionDAG/LegalizeIntegerTypes.cpp:4213
+  // We assume VSCALE(1) fits into a legal integer.
+  APInt One(HalfVT.getSizeInBits(), 1);
+  SDValue VScaleBase = DAG.getVScale(dl, HalfVT, One);
----------------
Can you call getFixedSizeInBits() here to avoid relying upon the implicit TypeSize -> uint64_t cast?

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105673