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
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
