[llvm-commits] [llvm] r157831 - in /llvm/trunk: lib/Target/X86/X86InstrArithmetic.td lib/Target/X86/X86InstrInfo.cpp lib/Target/X86/X86InstrInfo.h test/CodeGen/X86/jump_sign.ll

[Pete Cooper]

>
>
> > +    for (unsigned IO = 0, EO = Instr.getNumOperands(); IO != EO; ++IO) {
> > +      const MachineOperand &MO = Instr.getOperand(IO);
> > +      if (MO.isRegMask() && MO.clobbersPhysReg(X86::EFLAGS))
> > +        return false;
> > +      if (!MO.isReg()) continue;
> > +
> > +      // This instruction modifies or uses EFLAGS before we find a SUB.
> > +      // We can't do this transformation.
> > +      if (MO.getReg() == X86::EFLAGS)
> > +        return false;
> > +    }
>
> Please hoist this loop into a helper function. Ideally on MachineInstr so
> others can use it. It seems generically useful.
>
>
> Instead of hoisting this loop, please take a look at MI->modifiesRegister
and MI->readsRegister.  I think calling both should match the functionality
here.  I think MI->readsWritesVirtualRegister matches it even better, but
that function specifically mentions virtual registers in its name even
though from its implementation I think it would be safe here on the EFLAGS
physical register.

Thanks,
Pete
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20120602/5a07ffa8/attachment.html>