[llvm-commits] [llvm] r60807 - in /llvm/trunk: lib/Target/X86/X86FastISel.cpp test/CodeGen/X86/add-with-overflow.ll

[Evan Cheng]

On Dec 10, 2008, at 2:02 PM, Bill Wendling wrote:

> On Wed, Dec 10, 2008 at 1:51 PM, Evan Cheng <echeng at apple.com> wrote:
>>
>> On Dec 10, 2008, at 11:10 AM, Bill Wendling wrote:
>>
>>> On Wed, Dec 10, 2008 at 9:16 AM, Evan Cheng <echeng at apple.com>  
>>> wrote:
>>>>
>>>>> +    const MachineInstr *SetMI = 0;
>>>>> +    unsigned Reg = lookUpRegForValue(EI);
>>>>> +
>>>>> +    for (MachineBasicBlock::const_reverse_iterator
>>>>> +           RI = MBB->rbegin(), RE = MBB->rend(); RI != RE; + 
>>>>> +RI) {
>>>>> +      const MachineInstr &MI = *RI;
>>>>
>>>> Hi Bill,
>>>>
>>>> I don't think it makes sense to traverse machine instructions here.
>>>> We
>>>> want fastisel to be fast and this has the potential to be slow.  
>>>> This
>>>> code is only correct when we are doing top down fastisel, right? If
>>>> that's the case why not just check the end of MBB instruction is a
>>>> SETO / SETC? If there are other instructions in between, then we'll
>>>> do
>>>> dag isel instead. This allows the code to handle the most common
>>>> cases.
>>>>
>>> The last instruction at this point isn't a SETO/SETC instruction,  
>>> but
>>> a (useless) move of the register that SETO/SETC defines into another
>>> register. That's why I check whether it's a move instruction. The  
>>> code
>>> looks something like this:
>>>
>>>       %reg1024<def> = MOV32rm <fi#-1>, 1, %reg0, 0, Mem:LD(4,4)
>>> [FixedStack-1 + 0]
>>>       %reg1025<def> = MOV32rm <fi#-2>, 1, %reg0, 0, Mem:LD(4,4)
>>> [FixedStack-2 + 0]
>>>       %reg1028<def> = ADD32rr %reg1024, %reg1025, %EFLAGS<imp-def>
>>>       %reg1029<def> = SETOr %EFLAGS<imp-use>
>>>       %reg1026<def> = MOV32rr %reg1028
>>>       %reg1027<def> = MOV8rr %reg1029
>>>
>>> with the conditional for the BRCOND being %reg1027. If I only look  
>>> at
>>> the last instruction, then this will never trigger. I *could* look  
>>> at
>>> the last three instructions, but that's pretty restrictive . . . but
>>> fast.
>>
>> Where are the moves coming from?
>>
> The typical way to use these intrinsics is this:
>
>  %t = call {i32, i1} @llvm.sadd.with.overflow.i32(i32 %v1, i32 %v2)
>  %sum = extractvalue {i32, i1} %t, 0
>  %obit = extractvalue {i32, i1} %t, 1
>  br i1 %obit, label %overflow, label %normal
>
> The llvm.sadd.with.overflow call is lowered into
>
>        %reg1024<def> = MOV32rm ...
>        %reg1025<def> = MOV32rm ...
>        %reg1028<def> = ADD32rr %reg1024, %reg1025, %EFLAGS<imp-def>
>        %reg1029<def> = SETOr %EFLAGS<imp-use>
>
> The extractvalue calls are handled in FastISel by calling
>
>        UpdateValueMap(I, lookUpRegForValue(Agg) + *EI->idx_begin());
>
> Assigning them the register value from the ADD or SETO (depending on
> its index). These result in the two move instructions.

I see. I guess you have no choice but to traverse pass these moves  
until you get to the SETO / SETC. But if it runs into any other  
instructions in between, I would just drop out.

Evan

>
>
> -bw
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits