LGTM!

Based on what we discussed in the SVE call yesterday, I'm happy with the uses of getFixedSizeInBits. LGTM!

• Rebased patch.

• Rebased patch.
• Added class LinearPolyBaseOperators which provides a set of overloaded operators for derived classes.
• Made ElementCount a deriving class, because it needs 'isScalar' and 'isVector' methods which cannot be part of its parent class.

Please ignore my comments about getScalarSizeInBits, I thought that interface would assert that the type is not a vector (which I thought would be an extra guard that it's okay to ask for the fixed-size).
Instead, it returns the element type if the type is a vector, so in that case I agree it's better to use getFixedSizeInBits().

LGTM!

Thanks @david-arm, I think I'm happy enough with the changes you've made. Also some of these changes will be followed up further in D88982.
Please also give @ctetreau a chance to give his blessing before you commit.

Thanks for working on this @efriedma. The change seem sensible to me and I appreciate the effect it has on SVE predicates.
I can't really speak for the correctness for other targets though.

Rebased patch.

• Renamed PolyBase -> LinearPolyBase, DimIsExclusive -> IsUnivariate, etc.
• Removed verify() method.
• Replaced regular array with std::array.
• Changed name of new StackOffset class in TypeSize.h to NewStackOffset, so not to clash with the one defined in AArch64StackOffset.h. This is renamed to StackOffset in D88983 when it removes AArch64StackOffset.h entirely.
• Moved TestStackOffset.cpp from D88983 to this patch.
• Updated commit message.

In D88982#2317443, @ctetreau wrote:

This isn't really a true polynomial because it doesn't support multiplication of two terms: 2x * 2x = 4x^2

If what we have here is all we need, then I think it's fine (having not actually code reviewed the implementation at the time of this writing), but we should call it like it is, and ideally implement it in such a way that it would be straightforward to implement a true polynomial.

Are you suggesting rename PolyBase or just to remove the term 'polynomial' from the commit message and any of the comments?

Hi @dp, thanks for your detailed reply!

Thanks for working on this @greened!

Please note that MatchInstructionImpl may also return Match_MissingFeature if an operand does not match selected feature set. So we cannot use MatchInstructionImpl to find out if an instruction mnemonic is supported or not - Match_MissingFeature may be returned for other reasons

Did you try out my suggestion though? Because if I make this local change in AMDGPUAsmParser:

-  OperandMatchResultTy ResTy = MatchOperandParserImpl(Operands, Mnemonic);
+  OperandMatchResultTy ResTy = MatchOperandParserImpl(Operands, Mnemonic, /*ParseForAllFeatures=*/true);

then most of the cases you fixed in D88211 are fixed for free and I don't see the case happening that you mention here. If the above change doesn't fix that, can you please point me to the test for which that happens?
(If the above change does fix it, we should probably consider making 'true' the default for MatchOperandParserImpl)

From a fixed-width vector perspective, all the getFixedSize() changes are purely NFC. Changes like getSizeInBits() -> getScalarSizeInBits() are _possibly_ functional if your assumption that the type must be a scalar value, ends up being false. I think the exception here are the cases where it can be trivially seen that the function bails out early when the type is not a scalar.

LGTM as well.

Hi @dp, I believe that I encountered the same issue when adding the assembler support for AArch64 SVE.

I'm happy colour of the bikeshed. Please re-run clang-format on it though.

LGTM, thanks for fixing!

LGTM. I think this is a sensible change. TypeSize is used to indicate that a value could be scalable, but if it is known the type is always fixed-size this change simplifies code that calls getScalarSizeInBits (i.e. no need to call 'getFixedSize`).
It also puts us on the path towards removing the implicit casts to uint64_t.

LGTM, thanks for the changes @david-arm!

LGTM as well, thanks @kmclaughlin!

nit: I don't know if Clang ever really generates a llvm.invariant.start with -1 size in practice, but if it does, you may want to land this patch as two separate patches, one patch that prevents hoisting when size = -1 and another to stop the hoisting for scalable vectors.

Thanks @fpetrogalli, LGTM!

Abandoning this revision because an updated version of this patch (based on LLVM 9) has been posted in D87056.

Abandoning this revision because an updated version of this patch (based on LLVM 9) has been posted in D87056.

Thanks for reviewing the patch!

Hi @efriedma, what do you think of the latest changes to this patch?

[llvm][sve] Make llvm.masked.[gather|scatter] legal for SVE.

The title of the patch seems wrong, as this patch doesn't do any legalization of masked.gather/scatter for SVE?

In D84737#2242900, @efriedma wrote:

Does MIRParser also need to be modified?

You're right, that was missing. Thanks for pointing out!

• MIRParser now correctly marks registers as clobbered and added corresponding .mir test file.
• Updated RUN lines of unwind-preserved.ll to test with/without global isel.

• Updated patch with option 2: Update the function-global state when we construct the landing pad instruction/node.

Thanks for the review!

• Choose unsigned char element type for svbool_t bitpattern.
• Added LLVM test to ensure debug info for the vector type is correctly codegenerated when DISubrange has upper bound instead of count.

Addressed review comments before committing.

In D84737#2216322, @rsandifo-arm wrote:

Thanks the update, the idea looks good in general.

Thanks for the feedback!

• Made getCustomEHPadPreservedMask() return nullptr by default (meaning 'no clobbers')
• Fixed missing callee saves/restores for clobbered registers.
• Updated tests (and also added a RUN line for regalloc=fast)

• Set clobber mask for EH pad block, instead of changing the CC of the call.
• Rebased patch

Nice, LGTM!

Thanks for the reviews!

LGTM, thanks!

Rebased and added comment to .cfi_escape to describe CFI offset DWARF expression (like was added to D84043)

Added comment to .cfi_escape to describe CFA DWARF expression.

Updated CallerCC to AArch64_SVE_VectorCall if the caller has an SVE signature.
The code that follows further on will automatically determine whether TCO can be usedby looking at the callee-saved regs preserved by the callee and caller (condition if (!CCMatch) { on line 4243)

Gentle ping because D84044 can't land without this patch :)