Hmm. Somewhere in the back of my head is a worry that we may miss an optimization opportunity in cases such as max(a, abs(b)) where value-range analysis tells us that a is known to be positive already.

I ended up solving this problem a completely different way, in D72518. Instead of controlling the behavior I need based on the target architecture, I made it depend on a type attribute on the vector types, so that the MVE header file adds that attribute but nobody else does. And doing it like that I was also able to make the behavior more subtle, so that it makes the polymorphic MVE intrinsics work without also turning every case of lax vector checking into strict – so users still get to make a choice about how strict they want their vectors to be, and the intrinsics work either way.

While we're on the subject, it's always seemed odd to me in the first place that -mfloat-abi=soft prevents all use of the FP registers! By a literal reading of the option name, it surely ought to only disallow use of the FP registers at function call boundaries. I'd expect it to mean "Make my functions ABI-compatible with code built for a CPU with no FP regs at all, but within that constraint, do whatever will get the job done fastest"; using FP registers and hardware FP operations inside a function ought still to be legitimate, provided you transfer the return value back into the right integer register(s) once you've finished computing it.

LGTM this time. Thanks for the rewrite!

OK, this LGTM from the point of view of what's happening in the tests.

• Renamed the attribute as suggested.
• Made it target-specific.
• Added a diagnostic when it's applied to anything other than a vector type with a vector kind of NeonVector.
• Added some error tests demonstrating the new diagnostic.
• Added code examples to the docs.

@nhaehnle, @hfinkel: anything left I need to do to this?

@nhaehnle, @hfinkel: anything left I need to do to this?

Closed by rGd857e114b5e04f5143485a5aea7ad9b283768692. (I managed to leave off the commit message footer, sorry.)

Closed by rG9704ba652a0062c53ec66b068766df5c0cd5c620. (I managed to leave off the commit message footer, sorry.)

Removed the now redundant IntrNoMem, and moved cls up to before the MVE section. The output of llvm-tblgen -print-records on the whole of ARM.td is unchanged by these fixes.

Moved the llvm_unreachable.

Restructured as suggested.

LGTM, with a request for an extra comment.

Thanks for the helpful advice!

Added documentation and more tests; changed scoping policy so that then and else clauses do make a new scope for defvar; rebased on top of the revised D71407.

Without introducing a scope, the above is presumably valid but in a rather surprising way.

Addressed all review comments (I think).

This is a hasty first draft because I'm about to go away for the holidays, and won't have access to this account until the New Year.

Revised patch which keeps local-variable scopes in a stack, and opens
a temporary scope for each multiclass body, object body, or foreach body.

This also seems consistent with the feature overall. Are there reasons why we might not want to do this?

LGTM. I must admit that I probably didn't manage to take in every single detail of the revised test collection, but the shape of it looks generally nice, and everything I spot-checked looked spot-on. And the diffs in actual source files are definitely an improvement :-)

I've been working around the lack of this for a while, so I thought it was worth at least trying to get it accepted.

D71191 and D71195 are both now committed (thanks!), so I have a choice of two ways to achieve my original objective, and no further need for this third one.

OK, I'll commit it as is. Thanks!

Refactored further to remove the PredicatedImmediateVectorShift
multiclass completely: the amount of useful content remaining in it
now doesn't seem to justify its existence.

I'm sorry, I'm not sure I understand that question.

@hokein , @rdhindsa , @echristo : you all pointed out test failures in the previous version. Any problems I haven't spotted with this one?

Changes from previous version: