[llvm-commits] [llvm] r111221 - in /llvm/trunk: lib/CodeGen/MachineBasicBlock.cpp lib/CodeGen/PHIElimination.cpp test/CodeGen/ARM/code-placement.ll test/CodeGen/X86/lsr-reuse.ll
evan.cheng at apple.com
Tue Aug 17 10:49:06 PDT 2010
On Aug 17, 2010, at 12:11 AM, Jakob Stoklund Olesen wrote:
> On Aug 16, 2010, at 6:20 PM, Evan Cheng wrote:
>> Author: evancheng
>> Date: Mon Aug 16 20:20:36 2010
>> New Revision: 111221
>> URL: http://llvm.org/viewvc/llvm-project?rev=111221&view=rev
>> PHI elimination should not break back edge. It can cause some significant code placement issues. rdar://8263994
>> Modified: llvm/trunk/lib/CodeGen/MachineBasicBlock.cpp
>> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/CodeGen/MachineBasicBlock.cpp?rev=111221&r1=111220&r2=111221&view=diff
>> --- llvm/trunk/lib/CodeGen/MachineBasicBlock.cpp (original)
>> +++ llvm/trunk/lib/CodeGen/MachineBasicBlock.cpp Mon Aug 16 20:20:36 2010
>> @@ -439,6 +439,14 @@
>> if (TII->AnalyzeBranch(*this, TBB, FBB, Cond))
>> return NULL;
>> + // Avoid splitting backedges of loops. It would introduce small out-of-line
>> + // blocks into the loop which is very bad for code placement.
>> + if (this == Succ)
>> + return NULL;
>> + MachineLoopInfo *MLI = P->getAnalysisIfAvailable<MachineLoopInfo>();
>> + if (MLI->isLoopHeader(Succ))
>> + return NULL;
> Careful you don't dereference a NULL pointer. You can't count on MLI being available here.
> Also, I think the decision to not split a back edge should go in PHIElimination as a policy decision. Other callers of SplitCriticalEdge may have really good reasons for splitting a back edge. That should be allowed.
I have mixed feeling about this. In theory code placement optimization passes should be able to fix this. That's why the check in codegenprep's critical edge splitting is called a hack. I thought it a good idea to put the hack in one place only. Anyway, I'm moving the check to phi elimination (and fixed it) and I'll keep an eye on other clients.
More information about the llvm-commits