[llvm-commits] [llvm] r114681 - /llvm/trunk/lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp

[Jakob Stoklund Olesen]

On Sep 24, 2010, at 2:05 PM, Evan Cheng wrote:

> 
> On Sep 24, 2010, at 12:07 PM, Evan Cheng wrote:
> 
>> 
>> On Sep 24, 2010, at 11:49 AM, Jakob Stoklund Olesen wrote:
>> 
>>> 
>>> On Sep 24, 2010, at 10:48 AM, Evan Cheng wrote:
>>> 
>>>> There is definitely a codegen bug downstream. A unconditional branch to a fallthrough block is correct the last time I checked. If someone has some time to look into this, I would appreciate a test case.
>>> 
>>> It looks like MachineBasicBlock::canFallThrough affects codegen which could trigger this bug, and MachineBasicBlock::updateTerminator looks like it is going to undo it anyway by removing the unconditional branch.
>>> 
>>> Is any code assuming the presence of an unconditional branch?
>> 
>> I sure hope not. The reason for this change some DAG transformations require the presence of the unconditional branch following a conditional one so it can flip them. With this case, a lot of fcmp_oeq can be optimized to a pair of conditional branches, rather than two setcc and an and.
>> 
>> I've verified machine instruction passes will eliminate the branch to fallthrough. I wonder if we could disable branch folding and see if more miscompilations show up.
> 
> Tried it. No miscompilations on Darwin.

Thanks.

Well, if this build succeeds, we get to blame Rafael: http://google1.osuosl.org:8011/builders/llvm-gcc-i386-linux-selfhost/builds/69 ;-)

I noticed that AnalyzeBranch is doing a whole lot of editing for a function named Analyze*. It will remove the unconditional branch when called with AllowModify=true, as far as I can tell. That means the unconditional branch is probably not going to survive very far downstream.

/jakob