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

[Evan Cheng]

On Dec 9, 2008, at 3:19 PM, Bill Wendling wrote:

> Author: void
> Date: Tue Dec  9 17:19:12 2008
> New Revision: 60807
>
> URL: http://llvm.org/viewvc/llvm-project?rev=60807&view=rev
> Log:
> Implement fast-isel conversion of a branch instruction that's  
> branching on an
> overflow/carry from the "arithmetic with overflow" intrinsics. It  
> searches the
> machine basic block from bottom to top to find the SETO/SETC  
> instruction that is
> its conditional. If an instruction modifies EFLAGS before it reaches  
> the
> SETO/SETC instruction, then it defaults to the normal instruction  
> emission.
>
> Modified:
>    llvm/trunk/lib/Target/X86/X86FastISel.cpp
>    llvm/trunk/test/CodeGen/X86/add-with-overflow.ll
>
> Modified: llvm/trunk/lib/Target/X86/X86FastISel.cpp
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/lib/Target/X86/X86FastISel.cpp?rev=60807&r1=60806&r2=60807&view=diff
>
> = 
> = 
> = 
> = 
> = 
> = 
> = 
> = 
> ======================================================================
> --- llvm/trunk/lib/Target/X86/X86FastISel.cpp (original)
> +++ llvm/trunk/lib/Target/X86/X86FastISel.cpp Tue Dec  9 17:19:12 2008
> @@ -748,6 +748,69 @@
>       MBB->addSuccessor(TrueMBB);
>       return true;
>     }
> +  } else if (ExtractValueInst *EI =
> +             dyn_cast<ExtractValueInst>(BI->getCondition())) {
> +    // Check to see if the branch instruction is from an  
> "arithmetic with
> +    // overflow" intrinsic. The main way these intrinsics are used  
> is:
> +    //
> +    //   %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 %sum and %obit are converted in an ADD and a SETO/SETC  
> before
> +    // reaching the branch. Therefore, we search backwards through  
> the MBB
> +    // looking for the SETO/SETC instruction. If an instruction  
> modifies the
> +    // EFLAGS register before we reach the SETO/SETC instruction,  
> then we can't
> +    // convert the branch into a JO/JC instruction.
> +    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.

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.

Thanks,

Evan
>
> +
> +      if (MI.modifiesRegister(Reg)) {
> +        unsigned Src, Dst;
> +
> +        if (getInstrInfo()->isMoveInstr(MI, Src, Dst)) {
> +          Reg = Src;
> +          continue;
> +        }
> +
> +        SetMI = &MI;
> +        break;
> +      }
> +
> +      const TargetInstrDesc &TID = MI.getDesc();
> +      const unsigned *ImpDefs = TID.getImplicitDefs();
> +
> +      if (TID.hasUnmodeledSideEffects()) break;
> +
> +      bool ModifiesEFlags = false;
> +
> +      if (ImpDefs) {
> +        for (unsigned u = 0; ImpDefs[u]; ++u)
> +          if (ImpDefs[u] == X86::EFLAGS) {
> +            ModifiesEFlags = true;
> +            break;
> +          }
> +      }
> +
> +      if (ModifiesEFlags) break;
> +    }
> +
> +    if (SetMI) {
> +      unsigned OpCode = SetMI->getOpcode();
> +
> +      if (OpCode == X86::SETOr || OpCode == X86::SETCr) {
> +        BuildMI(MBB, TII.get((OpCode == X86::SETOr) ?
> +                             X86::JO : X86::JC)).addMBB(TrueMBB);
> +        FastEmitBranch(FalseMBB);
> +        MBB->addSuccessor(TrueMBB);
> +        return true;
> +      }
> +    }
>   }
>
>   // Otherwise do a clumsy setcc and re-test it.
>
> Modified: llvm/trunk/test/CodeGen/X86/add-with-overflow.ll
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/CodeGen/X86/add-with-overflow.ll?rev=60807&r1=60806&r2=60807&view=diff
>
> = 
> = 
> = 
> = 
> = 
> = 
> = 
> = 
> ======================================================================
> --- llvm/trunk/test/CodeGen/X86/add-with-overflow.ll (original)
> +++ llvm/trunk/test/CodeGen/X86/add-with-overflow.ll Tue Dec  9  
> 17:19:12 2008
> @@ -1,5 +1,7 @@
> ; RUN: llvm-as < %s | llc -march=x86 | grep {jo} | count 1
> ; RUN: llvm-as < %s | llc -march=x86 | grep {jc} | count 1
> +; RUN: llvm-as < %s | llc -march=x86 -fast | grep {jo} | count 1
> +; RUN: llvm-as < %s | llc -march=x86 -fast | grep {jc} | count 1
>
> @ok = internal constant [4 x i8] c"%d\0A\00"
> @no = internal constant [4 x i8] c"no\0A\00"
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/llvm-commits