LGTM

Huh, surprised this was missing. Add i64 before committing?

In D65884#1621103, @dmgreen wrote:

Sure, the ACLE intrinsic needs to return an i16, but does that mean the IR intrinsic needs to? It could be expanded to two instructions, llvm_arm_vctp32 and llvm_arm_vmrs, with the i16 coming from the vmrs. This kind of thing sounds like it would be useful already for things like masked loads. i.e I'm saying can we invert where the conversion happens?

It would work even better for compares that already have predicate that llvm knows about. They whole thing would just become llvm IR and we can let it optimise away.

Some codegen tests would be good.

What an eye opener...

Ah, nice. LGTM

Updated with context.

Why does the llvm_arm_vctp32 not return a <4xi1> directly?

The vctp family are defined like that because the ACLE specifies that they return a mve_pred16_t and I'm assuming this is a scalar - but I can't find a definition! I think that all the user facing predicate generators will produce a scalar and we will need to do the conversion to make it nice and LLVMy.

LGTM.

With my confusion cleared, LGTM.

Yes, I'm definitely up for preventing unrolling and I think checking the instructions would be better - we'll catch vector intrinsics that way too.

Yes, I guess that would be sensible approach! I am worried that one of the (many) passes will trip somewhere, so this gives me another example test case... For performance, I'm still not convinced this is the best approach because (1) we can't depend on metadata and (2) doesn't this also prevent the scalar remainder from being unrolled too?

Rather than for performance, I can't see how we can legally unroll a vectorized loops in the case of tail predication. I also don't know how we'd detect that a loop had been unrolled when we get to the conversion phase, so it seems like this is the only place where we can maintain correctness. And because we need this for legality, we can't rely on metadata...

Anything is allowed to use the value of LR, just not after it has been updated by LoopDec.

Because of the VecLd[0]/LHS typo, I had assumed that there was something wrong with the handling of the 'exchange' instructions. When I added that functionality, the tests weren't very varied so I assumed that I had missed an edge case with operand orderings. What the tests show is that there are some patterns which don't get optimised well, see the TODOs. The tests in overlapping are the kind of accesses that cause the problems and it wasn't something I had thought about. We test for sext loads with multiple users, but not in the case where multiple ones are widened.

So why doesn't sdiv get optimised like udiv..?

LGTM

@efriedma Thanks, added Preserved before committing.

Thanks. Added skipFunction and rebased.

LGTM

Reduced test case and removed change from previous commit.

I made some changes before committing - after noticing I should have attached the def to LoopEnd and not LoopDec.

Not sure why this is still lingering in my phab, this has been closed already.

Any reason why not to use this for thumb2 as well? It looks like it would be useful for T2 codesize.

yarp.

I trust that those massive test files are okay! LGTM.

Cheers! LGTM

Added support for setcc ge/uge, 1

Do you see a way that we can make this more efficient in the future..? If so, could we add a comment somewhere because looking at these code sequences, I hope this doesn't come up often!

• renamed the nodes and the search function.
• added support for other setcc opcodes, other than EQ and NE.

Committed in rL365749.

Modified logic during lowering.

LGTM

Added and modified comments.

Thanks, LGTM

Committed in rL365363.

Thanks. LGTM

@RKSimon Committed in rL365046

Please could you add an opt test for the hardware-loops too? This looks like a corner case that we're not getting generic coverage on.

Thanks for doing this! LGTM.

There's a fair number of sub, load and ptrtoint instructions so narrowing the context of the checks would be good, just checking which basic block would be good. As well as what GEP the load is using. Thanks.

And please could you update the test so that is clear which instructions you are testing.