In D128631#3635085, @dmgreen wrote:

Thanks LGTM

The one reg variant of EXTR (both input registers are the same, can’t be modelled)

It may be possible with a SchedulePredicate, but I think it's fairly minor. This patch is looking good.

The one reg variant of EXTR (both input registers are the same, can’t be modelled) was incorrectly modelled as the W form. Use the two regs properties for W form.

In D128631#3622812, @dmgreen wrote:

I think we can use this new schedule for all "Arm-v9" cores in AArch64.td (that are not in-order). It will almost certainly be a better fit than the older A57 model, and be good to get some decent SVE information.

Sounds good, my only concern is we're quite keen to get this into LLVM 15 which branches around the middle of July I believe and making this the default for all v9 cores presumably raises the bar in terms of validating it is better than the A57 for the v9 cores?

I think for Cortex-A710 we can change it, they are very similar microarchitectures. The performance checks I've ran seem just fine for them. I would change the Cortex-X2 and NeoverseV1 as well, although the core is a little different it would be a shame to leave them off. Neoverse512TVB I know less about, but as it doesn't relate to any specific core I would say a model with SVE scheduling is better than one without. We would presumably add all new cores with the new schedule as opposed to the A57 model, and I would prefer not to leave the other SVE cores behind in that regard.

Update SVE test

In D128631#3615625, @tschuett wrote:

Could you add something modern to the basic instruction test, like, e.g., MEMTAG and BTI?

Add tests for MTE and missing info for STGM/IRGstack instructions.

In D128631#3611765, @dmgreen wrote:

Add tests and missing info for EOR(BT|TB)_ZZZ_[BHSD], LDNT1D_ZZR_D_REAL, and STNT1D_ZZR_D instructions.

In D128631#3611765, @dmgreen wrote:

Hello - I've been looking at scheduling a little lately again. I presume this was created by transcribing the values from the Software Optimization guide? It looks nice and clean from what I can see.

Still looks good, better even :)

LGTM

LGTM

I'm not going to comment on the design / implementation since I wrote much of it, but I've left a few minor comments

@DavidTruby thanks for updating, just one last comment otherwise LGTM

LGTM

can the diagnostics improvements be broke out into separate patches? I think it would make this easier to review

LGTM

thanks for the patch!

LGTM, but I'm pretty new around these parts so I'll leave it to another reviewer to accept

In D120057#3332515, @sgrechanik wrote:

We are trying to fix this in a general way by running verification before printing and switching to generic op printing on failure, here: https://reviews.llvm.org/D117834

Does this need a round-trip test in mlir/test/Dialect/Vector/ops.mlir?

• Run clang-format.
• Add asserts for multi-vector register classes.

In D118561#3284551, @CarolineConcatto wrote:

But it looks a reasonable patch. I believe having this approved means fixing the tests in :
https://reviews.llvm.org/D118562

In D118562#3284538, @CarolineConcatto wrote:

Just to check, this change depends on https://reviews.llvm.org/D118561. Correct?
That is the reason your tests are failing for now.
LLVM ERROR: Don't know how to legalize this scalable vector type

In D118560#3286664, @david-arm wrote:

Hi @c-rhodes, could you add a description of why this refactoring is needed please? It's just not really obvious to me at least why we're shuffling code around.

Added some more splat tests (including NEON). Will post a follow-up patch to prevent scaling of num uses by splat factor unless division can be converted into scalar op.

Combine test with existing llvm/test/CodeGen/AArch64/fdiv-combine.ll

LGTM. I think you meant compile-time not runtime in the commit message?

Add further test for RHS

Add test Dave suggested

Revert LangOpt.VScaleMin default to 0.

In D113294#3170007, @paulwalker-arm wrote:

I agree, it's the change to VScaleMin that has caused the issue. If the LangOpts default can remain as 0 and you can still achieve what you're after then that would be perfect.

Revert to previous behaviour where both the min/max Clang flags override SVE.

Address comments

Update interface comment

Address comments

return None for unbounded in getVScaleRangeMax

In D113294#3115354, @sdesmalen wrote:

It would be nice if the LLVM interfaces could be updated as well (could also be done in a follow-up patch)

Address comments