FWIW, I find this method a bit strange. Do we need have a MachineLoopInfo available such that we could just check in the preheader of the loop?
We generally don't directly recurse though the CFG, but use a worklist and a loop. Can you do that here?
Extra space before 'to'.
Period at the end of the comment.
Given that this is off by default anyway, why is it enabled only for the P9?
It is mainly because only P9 has InstrSchedModel , P8 and below are still using Itineraries.
Use worklist instead, and fix comments.
In short, yes.
I'd not give it a 100% guarantee, but if you only check in the preheader, do you miss anything?
(you could check, perhaps, by putting an assert in the current logic to see if you every find the mtctr in some block other than the preheader).
Oh, sorry, did not read carefully, the 'yes' in my last comment was for 'predecessors' not 'preheader' - we assume that mtctr will be in predecessors of Prolog MBBs, no necessary in preheader only.
I guess you question is about whether we may miss some opportunity here - can NOT find mtctr in some case when mtctr is NOT in predecessors?
From the code in MachinePipeliner, I think we should not missing anything here. But I will try your suggestion to double confirm.
Added asserts before return nullptr, and run test with testsuites, no assert found.
I also look into PPCCTRLoops where we generate mtctr, and looks like that we will only Insert the count into the preheader , and PPCCTRLoopsVerify also trying to find MTCTR8loop/MTCTRloop in predecessors only.
I am not aware of any following passes that might move mtctr to other places.
@hfinkel Do you have any cases in mind that I should check again?
Nope. That's what I suspected that you might find. I think that just checking the preheader, should it exist, will be sufficient here in practice. I recommend doing that, and then if we find cases where we need to do more, revisit at that time.
This patch seems to fail under Asan at https://github.com/llvm-mirror/llvm/blob/28bea3dbfef348e53cf48f921e96a35b642b3950/lib/CodeGen/MachinePipeliner.cpp#L3736.
I verified that lower_bound() may return Indices.end() upon running the test CodeGen/PowerPC/sms-simple.ll.