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

[Bill Wendling]

On Wed, Dec 10, 2008 at 2:17 PM, Evan Cheng <echeng at apple.com> wrote:
>
> 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.
>
Sure. That sounds fine. :-)

-bw