[llvm-commits] [llvm] r101075 - in /llvm/trunk: lib/Target/X86/X86InstrInfo.cpp test/CodeGen/X86/brcond.ll

[Dan Gohman]

On Apr 13, 2010, at 2:01 AM, Bill Wendling wrote:

> On Apr 12, 2010, at 7:20 PM, Evan Cheng wrote:
> 
>>> The problem lies in this bit of code in X86InstrInfo::AnalyzeBranch:
>>> 
>>>     // If they differ, see if they fit one of the known patterns. Theoretically,                                                                                        
>>>     // we could handle more patterns here, but we shouldn't expect to see them                                                                                          
>>>     // if instruction selection has done a reasonable job.                                                                                                              
>>>     if ((OldBranchCode == X86::COND_NP &&
>>>          BranchCode == X86::COND_E) ||
>>>         (OldBranchCode == X86::COND_E &&
>>>          BranchCode == X86::COND_NP))
>>>       BranchCode = X86::COND_NP_OR_E;
>>>     else if ((OldBranchCode == X86::COND_P &&
>>>               BranchCode == X86::COND_NE) ||
>>>              (OldBranchCode == X86::COND_NE &&
>>>               BranchCode == X86::COND_P))
>>>       BranchCode = X86::COND_NE_OR_P;
>>>     else
>>>       return true;
>>> 
>>> When the code in BranchFolding calls "ReverseBranchCondition", the X86 back-end isn't able to handle it. So it doesn't change the branch conditions.
>>> 
>>> If you look for the bits of code that handle COND_NP_OR_E and COND_NE_OR_P there doesn't seem to be any optimizations done with them. X86InstrInfo::InsertBranch looks at them and inserts two branches.
>>> 
>>> What is the rationale behind converting a JE/JNP and JNE/JP into COND_NE_OR_P and COND_NP_OR_E?
>> 
>> I think these are used to merge two conditional branches into one and split them back out again later. It seems like a hack to allow AnalyzeBranch to handle BB with two conditional branches. They correspond to OEQ and UNE.
>> 
>> Is it possible to teach ReverseBranchCondition to do the right thing? Ain't OEQ the reverse of UNE and vice versa?
>> 
> It looks like I can safely teach GetOppositeBranchCondition to return the opposite for the two COND_* above. We do have an interesting case, though. If I change the original code to "une" instead of "oeq" like this:
> 
> define float @func2(float %x, float %y) nounwind readnone optsize ssp {
> entry:
>   %0 = fpext float %x to double                   ; <double> [#uses=1]
>   %1 = fpext float %y to double                   ; <double> [#uses=1]
>   %2 = fmul double %0, %1                         ; <double> [#uses=3]
>   %3 = fcmp une double %2, 0.000000e+00           ; <i1> [#uses=1]
>   br i1 %3, label %bb2, label %bb1
> 
> bb1:                                              ; preds = %entry
>   %4 = fadd double %2, -1.000000e+00              ; <double> [#uses=1]
>   br label %bb2
> 
> bb2:                                              ; preds = %entry, %bb1
>   %.0.in = phi double [ %4, %bb1 ], [ %2, %entry ] ; <double> [#uses=1]
>   %.0 = fptrunc double %.0.in to float            ; <float> [#uses=1]
>   ret float %.0
> }
> 
> Then we generate this for the "fcmp une ..."
> 
> 	%AL<def> = SETNPr %EFLAGS<imp-use>
> 	%CL<def> = SETEr %EFLAGS<imp-use,kill>
> 	TEST8rr %CL<kill>, %AL<kill>, %EFLAGS<imp-def>
> 	JE_4 <BB#2>, %EFLAGS<imp-use>
> 
> Ick. I suppose we should instead be generating?
> 
> 	JNP_4 <BB#2>, %EFLAGS<imp-use>
> 	JE_4 <BB#2>, %EFLAGS<imp-use>

This isn't equivalent. The first version jumps only if both conditions are
true, the second if either one is true.

Dan