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 :)

LGTM!

LGTM, thanks!

In D84044#2172957, @efriedma wrote:

No changes are needed in Clang to support this AFAIK.

(I meant loosely a change to clang and/or LLVM.)

Consider the following testcase:

#include <arm_sve.h>
void g1(svuint8_t);
void g2(svuint8_t);
struct Z { svuint8_t *x; ~Z() { g2(*x); } };
svuint8_t f(svuint8_t a, svuint8_t b) {
  Z z{&a};
  g1(b);
  return b;
}

If unwind doesn't preserve SVE registers, we need a spill here. clang currently doesn't generate one at -O2.

Yes, great spot! The caller should assume that all SVE registers are clobbered by the unwinder if the function _may_ throw an exception. I think I'll create a separate patch to fix this, as it's not necessarily something to do with how LLVM emits the unwind info, but rather a bug where the caller doesn't honour the right CC.

In D84044#2170905, @efriedma wrote:

I asked @rsandifo-arm about this, who suggested that the unwinder restores the callee-saved registers in the base ABI only, i.e. normal returns preserve extra registers, but exceptional returns don't.

I don't think I've ever seen a target do that, but I guess there isn't any reason it can't work like this. Does clang need additional changes to support this, though?

No changes are needed in Clang to support this AFAIK.

rebased patch.

rebased patch.

LGTM, thanks!

In D84044#2165405, @efriedma wrote:

The CFI entries only cover a subset of the SVE callee-saves and only encodes the lower 64-bits, thus implementing the lowest common denominator ABI. Existing unwinders may support VG but only restore the lower 64-bits.

I'm not sure I understand this. If the calling convention requires saving the whole SVE register, unwinding needs to restore the entire SVE register, or else we corrupt the register in exception unwinding.

Yes that confused me as well, you are right that this would corrupt the register in exception unwinding.

Good catch, I didn't realise we were missing those copies. LGTM!

In D84044#2163137, @efriedma wrote:

I'm confused by the stack layout involved here. SVE registers should never be callee-save. The ABI says we're only supposed to save the low 64 bits.

Just in case you were looking at the wrong version of the spec, https://github.com/ARM-software/abi-aa/blob/master/aapcs64/aapcs64.rst#scalable-vector-registers contains the sections for SVE.

Use SmallString instead of SmallVector<uint8_t>.

Use SmallString instead of SmallVector<uint8_t>.

• Changed condition to useFPForScavengingIndex
• Added new test framelayout-scavengingslot.mir to guard that stack realignment (without SVE) allocates the scavenging slot in the right place.
• Simplified patch, no need to refactor estimateSVEStackObjectOffsets
• Renamed misleading/wrong title of this patch.