[PATCH] D142122: [SVE][InstrFormats] Explcitly set hasSideEffects for all SVE instructions.

[Peter Waller via Phabricator via llvm-commits]

peterwaller-arm accepted this revision.
peterwaller-arm added a comment.
This revision is now accepted and ready to land.

I've been down the list of MCA output changes and agree those look good, and your reasoning seems good to me. Accepting as-is, since with think represents an improvement.

I believe the following command (adjusting relative paths) counts the number of instructions with `hasSideEffects` unset.

  bin/llvm-tblgen -I ../../llvm/include -I ../../llvm/lib/Target/AArch64 ../../llvm/lib/Target/AArch64/AArch64.td --dump-json --class=AArch64Inst | \
    jq 'with_entries(select(.value | (type == "object" and has("hasSideEffects") and .hasSideEffects == null))) | length'

With this patch applied (on 5a7f47cc021b <https://reviews.llvm.org/rG5a7f47cc021bd7a19cb70c9a30755d6b3cb67431>) I see the number drop from `4667` to `2198`.

(and if you replace `| length` with `| keys` it shows the instructions). I do see a fair number of instructions remaining that don't have it set which you might have intended to capture, but I haven't dug in. These are the top ten, aggregating by three-char prefix:

   46   "ADD",
   48   "FMI",
   51   "FMA",
   64   "SML",
   64   "UML",
   70   "BFM",
   86   "MOV",
   95   "FCV",
   96   "CPY",
  102   "FML",


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D142122