This code is odd. This is because for predicates we always use INS_OPTS_SCALABLE_B for a mov, so the passed in opt value doesn't matter ?

Yes, so basically, this code operates just on INS_OPTS_SCALABLE_B (and we added single test for that). For other places, if we have to pass INS_OPTS_SCALABLE_B if register types are predicate feels like an overhead, so instead this code expects it to be NONE, in which case it will set the options to INS_OPTS_SCALABLE_B.

Would something like this work?

if (!insOptsNone(opt))
{
    assert(opt == INS_OPTS_SCALABLE_B);
}

opt = INS_OPTS_SCALABLE_B;

I think that shape makes it clearer the if-statement is debug-only code, and ensures opt is always INS_OPTS_SCALABLE_B even if we don't have asserts on to catch improper handling.

assert(opt == INS_OPTS_SCALABLE_B || insOptsNone(opt));
opt = INS_OPTS_SCALABLE_B;

?

Are there any functions we can remove sopt completely?

Not sure I understand. Can you please elaborate?

I think Alan means are there any emitInsSve_* methods that no longer need the sopt parameter, because it's only checking that sopt is INS_SCALABLE_OPTS_NONE? emitInsSve_R_R seems close to that, though we still check sopt with insScalableOptsWithVectorLength at the end of the method.

Ah ok. Thanks @amanasifkhalid for clarifying. I didn't check, but can do it as a follow-up work.

I was returning true here because I assumed we needed to track the fact there were addresses in Op2. Is it safe to not track that?

Re-reading, the OperIsMemoryLoad() contract is to return true if this intrinsic may throw NullReferenceException of address is "null". Now, for certain non-faulting intrinsics, it should return false, but for Gather*, this should return true.
Fixed it.

the OperIsMemoryLoad() contract is to return true if this intrinsic may throw NullReferenceException of address is "null".

That was the part I wasn't sure of.
Happy with the new version.