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

-bw