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

Comments?

> Also, it should first check if the source of extractvalue comes from
> arithmetic with overflow intrinsics. If not, then the code should not
> be looking for SETO / SETC at all.
>
Okay.

-bw