[PATCH] D62132: [RFC] Intrinsics for Hardware Loops

[Jon Chesterfield via Phabricator via llvm-commits]

JonChesterfield added a comment.

I've implemented this out of tree (for Graphcore), based loosely on the PPC implementation. IR pass based on SCEV inserts intrinsics, SDag patches them up a little, MIR pass picks appropriate instructions or falls back to a decrement and branch loop.

> As the example shows, control-flow is still ultimately performed through the icmp and br pair.

This is interesting. We've also gone with a pair of intrinsics in IR - one in the loop preheader that takes an integer for trip count (backedge + 1), one in the body which returns an i1 that goes to brcond. Ideally I'd have liked to use an intrinsic to represent the control flow. An opaque intrinsic returning a boolean for brcond is approximately the same. An integer is threaded between the two in order to keep a GP register live until the back end where the lowering to ISA may need it, but that's probably unique to our arch.

Folding the icmp behaviour into the intrinsic instead stands a reasonable chance of making the induction variable dead (well, effectively hidden in the intrinsics) with subsequent IR simplification. Is the advantage to keeping it explicit in the interaction with other loop passes?

In SDag for our target, each IR intrinsic turns into two pseudo instructions. One representing the branch, one representing the arithmetic. It seems you've continued with two intrinsics, using a specific register and side effects to represent the combination. For example, the 'decrement arithmetic' pseudo turns into either a subtract one or into no code, depending on the instructions chosen. Keeping it separate from the terminator seems to help position it in a reasonable place in the BB for the decrement. I'm not sure which approach is better.

Would your implementation make sense without the masked load/store support? I'm wondering if that's a reasonable place to split the patch.

Thanks for the diff!


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62132/new/

https://reviews.llvm.org/D62132