That makes sense to me. It's worth adding a new node if it will help improve lowering for other instructions too. LGTM!

Remove FracBits == ScalarBits - 1 check and add more tests

Remove !IsUnsigned check and TODO

Rename tryFMUL, use isa instead of dyn_cast and add more negative factor tests

Check if the VDUP operand is constant and do clang-format.

Rebase and check if FracBits == ScalarBits - 1 when not unsigned.

Looks good to me. It's up to you if you would like to add a comment as mentioned in my reply.

Move to a function, add IsUnsigned check to fp inf, check bitcast element size, remove IEEEdouble(), make sure FracBits <= ScalarBits, add llvm_unreachable, move test file and use arm_aapcs_vfpcc.

Sweet, looks good.

LGTM

Very nice

We're go for committing after fixing the little nitpick in the comment :)

Abandoning this for now as the code and logic this is based on has been improved and updated. I'll revisit it if it ends up still being an issue.

Nice correction, LGTM

Nice idea. LGTM

Sorry for not looking at this earlier. It looks like a really good improvement.

I remember running into issues where analyzeBranch didn't know about LOL instructions. Thanks for updating it.

Nice and clean 👍🏻

Nice 👍🏻

LGTM

In D94691#2499715, @craig.topper wrote:

Should the pass itself call skipFunction in its runOnMachineFunction method? That's what handles skipping for optnone function and optbisect.

In D94691#2498340, @SjoerdMeijer wrote:

Yep, don't think it is useful or necessary to run this at O0.
LGTM.

Merge with if statement below

Punctuation and check for entry block

Fix erroneous format of a comment

Check for new forwards LE branches and address feedback

Thanks for that Sjoerd, I agree with your examples and comments. I have changed the approach back to check for new backwards WLSs as before.

Change the validation approach back to what it was before

LGTM

LGTM. Have you noticed any performance changes with this?

Clean up

Use MachineLoopInfo. I have removed the fallthrough fixing test since it seems to be impossible to create the needed fallthrough and keep it as a valid MachineLoop.

Use INITIALIZE_PASS technique

Fix incorrect branch opcodes

Move pass order and add copyright header

Looks good to me!

Rename and move earlier in the pipeline.

Fix tests

Inline canMoveBasicBlock, update BB offsets and make fixFallthrough a lambda.

Use BBUtils to get BB offsets, loop over terminators instead of instructions and other clean-up.

In D92385#2428006, @SjoerdMeijer wrote:

I have some high-level questions:

• Are we fighting another optimisations here, some sort of loop-rotate or is this just MBP reshulffling blocks in a way that is not good for us?

Yeah MBP moves the loop blocks (preheader, body etc.) closer together but unfortunately moves the WLSs branch target above the WLS. So it does some good things but also some bad things. After looking at MBP I thought it would be simpler to make a target pass rather than juggle things around in MBP that could end up affecting other targets.

• I know we don't nest WLSTPs for profitability reasons, but in theory we could. Not sure we need to check this though. But in general my impression is that some more test can be added, but perhaps you were still working on that.

I wasn't sure if it was possible or not, but thought I'd add the checks in there just in case. I'm OK with removing them if they definitely are unnecessary. I'm also working on testing a nested while loop, can you think of any other tests I should add?

• I am wondering if some cost-modeling is required. For example, if the iteration count is very low, would that change things?

That is a good idea. It would be good to get an idea of the cycles saved by converting the while loop into a LOL compared to those needed by the branches that replace fallthrough.

Very nice change, looks good to me.

Nice, LGTM

Rebase

Fix test ordering. This is an NFC so I will commit with the previous approval.

Thank you @goncharov and @kadircet

Use std::any_of and hasVPRUse.

Add another test case and format

Get VPR def when merging across blocks rather than keeping a pointer to the VCMP.