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

[Evan Cheng]

On Sep 24, 2010, at 2:15 PM, Jakob Stoklund Olesen wrote:

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

Yes, AnalyzeBranch is pretty horrible.

Evan

> 
> /jakob
>