[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]

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
>> Log:
>> 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.

Right.

> 
> 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.

Evan

> 
> /jakob
>