[PATCH] D116157: [AArch64] Adding "armv8.8-a" memcpy/memset support.

Wed Jan 5 05:15:35 PST 2022

dmgreen accepted this revision.
dmgreen added a comment.
This revision is now accepted and ready to land.

> Use Sched<[]> rather than hasNoSchedulingInfo. This requried updating the regexes used by various scheduling models to apply InstRW to CPY* instructions so that they do not capture the new MOPS instructions.

Yeah, I got confused as to why this worked differently to the other new instructions like ST64B. Enough to go and have a look at what was going on, and came to the same conclusion.

I think it's worth renaming the old "CPY" instruction just for clarity. It is not anything to do with any CPY instructions after all. I put up D116655 <https://reviews.llvm.org/D116655> for that.

Otherwise that and the other point here, this patch LGTM.

================
Comment at: llvm/lib/Target/AArch64/AArch64InstrFormats.td:11456
+  let hasNoSchedulingInfo = 1;
+  let hasSideEffects = 1;
+  let mayLoad = 1;
----------------
Sorry, I should have been more clear. I'm not sure this actually needs hasSideEffects. I was just trying to explain that it would already have it set (implicitly), so missing mayLoad/mayStore would not technically be a problem, as it already had sideeffects. With mayLoad and mayStore, I don't believe there are any other side effects of these instructions. They just update registers and load/store memory.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D116157