Fix typo.

@sdesmalen - I have moved the check outside the switch statement as discussed in our phone call.

Nit: there is a typo in the commit message: whre -> where.

I have added test coverage for st1[bhwd] for both .s and .d version of the alias for [<Za>.<T>, #0].

In D86065#2235434, @david-arm wrote:

Hi @fpetrogalli, if you don't mind I think I'll stick with Paul's idea for ogt because this matches the IR neatly, i.e. "fcmp ogt". Also, for me personally it's much simpler and more intuitive.

I lifted the implemementation to the target-agnostic part of the scalarize pass.

In D86249#2227281, @efriedma wrote:

ScalarizeMaskedMemIntrin can't scalarize scalable vectors on any target; would it make sense to check there?

In D86120#2228000, @davezarzycki wrote:

Why were static "get" methods preferred over simply marking the constructor explicit? For example, if the constructor is marked explicit, then this is what users see:

    llvm::ElementCount EC = {vector->getNumElements(), /*scalable*/ false};
                       ^    ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
/home/dave/s/n/llvm/include/llvm/Support/TypeSize.h:37:12: note: explicit constructor declared here
  explicit ElementCount(unsigned Min, bool Scalable) : Min(Min), Scalable(Scalable) {}
           ^
1 error generated.

In D86240#2227031, @ormris wrote:

This builds on Windows. Do the comments need to be updated?

Thank you @rengolin, I have addressed your feedback.

I re-enabled the default contructor so that the code actually builds on linux.

I fixed one assertion message that I missed in last patch update.

@ftynse @mehdi_amini @JDevlieghere - apologies for breaking your workflows, and thank you for acting so quickly. I started doing the revert myself as soon as I saw the bombing from buildbot but by the time I was ready to push my revert your fixes were already pouring in.

Thank you for the review @david-arm.

I merged it in the release branch as https://github.com/llvm/llvm-project/commit/7e6bf0bfe6de9e0d0e58764a66f93210f296bbfa

Thank you for the review, @ctetreau.

Fix typo.

Thank you for the code review @ctetreau.

I have updated the changes on top of https://reviews.llvm.org/D86120 (no constructors for ElementCount).

@ctetreau - I had made the constructor of ElementCount private in https://reviews.llvm.org/D86120.

Thank you again for the review. In the last update I have:

Thank you for the review @david-arm.

Fix indentation of the code exmaple.

I have removed the ordering function of ElementCount. It required replacing the uses of llvm::SmallSet<ElementCount,...> with llvm::SmallSetVector<ElementCount, ...>.

Added context to the diff.

In D85794#2213912, @rogfer01 wrote:

Out of curiosity, what is your approach? Carrying around a bool Scalable flag?

We keep some state around that tells us if we're in a scalable mode. But this doesn't work if we want to be able to vectorize both for fixed size or scalable at the same time for the same target. Not sure if you're considering this scenario (e.g. Advanced SIMD + SVE-256)

Thank you for the reviews.

thank you for explaining. Sorry it took so long to realize I needed to come back to you on this.

Thank you @rengolin and @rogfer01 for the review.

In D85794#2212993, @rengolin wrote:

I think this is a good direction, too.

I don't like implicit assumptions or magic constants, especially when the two (bool scalable and int min) are not linearly independent.

In D85794#2212309, @rogfer01 wrote:

Thank you for the review @paulwalker-arm.

I have updated the tests to make sure they are related to the code that has been modified.

Address last feedback on template parameters.

I replaced cast with static_cast to fix build failures.

I have modified the testing of the reg+imm addressing mode to use upper and lower bound instead of strictly in-range values.

I have addressed your feedback. Thank you @sdesmalen.

Adding some text here as phab doesn't allow me to submit empty comments!

Thank you for the review @sdesmalen, I have addressed all your comments.

Thank you for your review @sdesmalen!