[PATCH] D77872: [AArch32] Armv8.6-a Matrix Mult Assembly + Intrinsics

[Dave Green via Phabricator via cfe-commits]

dmgreen added inline comments.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.


================
Comment at: llvm/lib/Target/ARM/ARMInstrNEON.td:4846
+      VDOT<op6, op4, op23, RegTy, Asm, AsmTy, AccumTy, InputTy, OpNode> {
+  let hasNoSchedulingInfo = 1;
+
----------------
LukeGeeson wrote:
> dmgreen wrote:
> > I don't think that hasNoSchedulingInfo is necessarily the best way to handle this. That flag is intended for instructions that will never be scheduled, like Pseudo instructions.
> > 
> > If you are running into "Complete Schedule" problems, they might need HasMatMulInt8 added to the list of unsupported features instead.
> Since there are no 8.6a cpus in llvm that support this extension, the default behaviour is to use Cortex-A57 scheduling - this also has no notion of 8.6a matmul.
> 
> This falls back to SchedMachineModel in `llvm/lib/Target/ARM/ARMScheduleA57.td` and in particular UnsupportedFeatures would be a candidate place to put unsupported features like matmul. I took issue with putting all new extensions here because there should be a separation of concerns between a particular scheduling model, and supporting new behaviour (which could go in a generic catch all location that might be slightly more informative ).
> 
> Did you have something in particular in mind?
The ARMSchedule A57schedule is marked with CompleteModel=1. I believe it's the only one ARM-side that does that, but that's why it's running into problems. I'm surprised it hasn't run into the same problems in the past.

On the AArch64 side we tend to mark all Neon instructions with WriteV, so they at least get basic scheduling info. We don't tend to do that for ARM though.

Marking the instruction as hasNoSchedulingInfo will stop _any_ schedule from setting information on them, which whilst technically OK for now would be cause problems in the long run. We should either be adding a Sched[] Write (but I'm not sure what we would add), marking A57 as not complete (which seems like a hack) or adding unsupported features. Or adding scheduling info for these instructions to A57 I guess, but that doesn't sound right for a CPU that doesn't support them!

I can put a patch together to clean it up pretty easily.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77872