LGTM

LGTM

LGTM

LGTM

Thanks for reviewing all, I'll land this early next week unless there's any objections between then, cheers

Address @sdesmalen's comments

LGTM thanks

Address comments

In D97053#2575791, @craig.topper wrote:

Is this change specific to fixed vectors declared with arm_sve_vector_bits or any of the subclasses of VectorType? If it allows the others, how do we know for sure that there are enough bits in the scalable type for the fixed vector. I ask because RISCV is also using sizeless builtin types for our vectors as of D92715.

Address comments

In D94708#2512706, @fhahn wrote:

In D94708#2509677, @c-rhodes wrote:

In D94708#2509610, @fhahn wrote:

Are there plans to use this for fixed vectors as well? If not, can this be restricted to scalable vectors only? It seems like for fixed vectors, preferring the shufflevector versions would be beneficial, to avoid having to update all transforms lookin at shuffles.

Otherwise, this should probably have some tests for platforms other than AArch64 and also support in GlobalISel, which is the default on AArch64 with -O0 IIRC (or do the transform to shuffles as an IR transform).

The named shufflevector intrinsics accept both fixed and scalable vectors but for fixed they map to existing shufflevector.

As specified here yes, but I was wondering if they actually need to support fixed-width vectors? Is there a reason to use the intrinsics with fixed vectors instead of shuffles?

Changes:

• As proposed by @sdesmalen, the intrinsic now supports two variants based on the sign of the immediate, where a negative immediate is a trailing element count and a positive immediate an index.
• Following the discussion on the reverse intrinsic (D94883) around whether named shufflevector intrinsics should support fixed vectors, I've opted for the same approach to make it explicit in the LangRef that whilst these instructions are experimental shufflevector should be used for fixed-width vectors. The changes to InstCombineCalls to map this intrinsic to shufflevector as an IR transform have been dropped.
• ExpandVectorSpliceThroughStack has been moved to TLI under expandVectorSplice which is reused by DAGTypeLegalizer::SplitVecRes_VECTOR_SPLICE.

LGTM, might be worth leaving a little time before landing incase others have any comments, cheers

Fix renaming of implicit-defs but only allow it for certain opcodes.

In D90687#2512510, @iajbar wrote:

Thanks Cullen! I tried your fix D94869 and my benchmark passes.

In D94869#2533313, @fhahn wrote:

LGTM, thanks! This should probably also go onto the 12.x release branch.

@fhahn I've simplified the test, thanks for the comments!

In D94708#2509610, @fhahn wrote:

Are there plans to use this for fixed vectors as well? If not, can this be restricted to scalable vectors only? It seems like for fixed vectors, preferring the shufflevector versions would be beneficial, to avoid having to update all transforms lookin at shuffles.

Otherwise, this should probably have some tests for platforms other than AArch64 and also support in GlobalISel, which is the default on AArch64 with -O0 IIRC (or do the transform to shuffles as an IR transform).

Changes:

• Remove unused function declaration for LowerVECTOR_SPLICE.
• Clarify trailing.elts must be an integer constant in LangRef.
• Use DefaultAttrs for intrinsic.
• Use setOperationPromotedToType.

In D90687#2501881, @iajbar wrote:

The patch that causes the assert is patch cba4accda08f90. I tried commit c63799fc52ff247 that is before your patch and there is no assert. Thank you.

In D90687#2501292, @iajbar wrote:

I configured and built llvm with:
cmake -G Ninja -DLLVM_TARGETS_TO_BUILD:STRING=Hexagon -DLLVM_DEFAULT_TARGET_TRIPLE:STRING=hexagon-unknown-elf -DLLVM_TARGET_ARCH:STRING=hexagon-unknown-elf -DLLVM_ENABLE_ASSERTIONS:BOOL=ON -DLLVM_PARALLEL_LINK_JOBS=1 '-DLLVM_ENABLE_PROJECTS=llvm;clang' -DBUILD_SHARED_LIBS:BOOL=ON ..

This patch causes an assert with this testcase:
typedef struct {

char a;

} b;
b *c;
int d, e;
int f() {

int g = 0;
for (; d; d++) {
  e = 0;
  for (; e < c[d].a; e++)
    g++;
}
return g;

}
clang -Os -mhvx -fvectorize -mv67 testcase.i -S -o -

llvm::LoopVectorizationCostModel::computeMaxVF(llvm::ElementCount, unsigned int): Assertion `WideningDecisions.empty() && Uniforms.empty() && Scalars.empty() && "No decisions should have been taken at this point"' failed.

In D94504#2494181, @cameron.mcinally wrote:

LGTM

I'm assuming scheduling the new addvls closer to their uses is a register pressure win?

Rebased. Committed update_llc_test_checks.py changes separately to reduce the diff (thanks for suggestion @sdesmalen).

I've left one minor nit but looks otherwise looks fine to me

In D94168#2484564, @frasercrmck wrote:

@c-rhodes, could you possibly check that the changes to the AArch64 tests are okay? The whilelo one is a bit confusing.

@fhahn thanks for reviewing, I think I've addressed all comments. I'll probably land this over the coming days unless there's any objections between then, cheers.

Address @fhahn comments.

Changes:

• Rebased. Target hook for max vscale has been removed from this patch since it landed in D93030.
• Address Sander's comment w.r.t. falling back to fixed vectorization if scalable vectors are supported, but max vscale is undefined.

Address @fhahn's comments, changes:

Handle scalable VF in a single isScalable block in computeFeasibleMaxVF.

Left a couple of nits but mostly LGTM, cheers

• Rather than disable vectorization if scalable VF isn't supported, let the LV pick a VF as it normally does and process with cost-modelling.
• Added a new LV flag -force-target-supports-scalable-vectors to support writing non-target specific tests with scalable VFs.
• Added a temporary workaround to use MaxVF in place of UserVF until cost modelling loop works for scalable VFs.
• Emit opt remark.

Address comments and added test.

Changes:

• Rebase since D88962 and D91077 have now landed.
• Address @sdesmalen's comments. Added a target hook supportsScalableVectors in D93060 which is used in this patch to bail out of vectorization if unsupported. Also changed the behavior when scalable vectorization is unfeasible given a dependency, it now changes the hint to fixed, where existing behavior applies (i.e. clamping).
• Updated tests.