[llvm-commits] [llvm] r116665 - in /llvm/trunk: include/llvm/CodeGen/MachineModuleInfo.h lib/CodeGen/MachineModuleInfo.cpp lib/CodeGen/SelectionDAG/SelectionDAGBuilder.cpp lib/Target/X86/X86AsmPrinter.cpp test/CodeGen/X86/fltused.ll

[Michael Spencer]

On Sat, Oct 16, 2010 at 4:42 AM, Anton Korobeynikov
<anton at korobeynikov.info> wrote:
> Hi Michael,
>
>> +      MachineModuleInfo &MMI = DAG.getMachineFunction().getMMI();
>> +      for (unsigned i = 0, e = I.getNumArgOperands(); i != e &&
>> +                  !MMI.callsExternalFunctionWithFloatingPointArguments(); ++i) {
> Why do you need to check stuff on every iteration?

To exit early if this has already been flagged. The definition of
callsExternalFunctionWithFloatingPointArguments is inline in the
header file, so it should be very cheap.

>> +        const Type* T = I.getArgOperand(i)->getType();
>> +        for (po_iterator<const Type*> i = po_begin(T),
>> +                                      e = po_end(T);
>> +                                      i != e; ++i) {
> Definitely can be fit into single line :)

Went over 80 cols I believe. Although I may have lowered the nesting
depth after I originally wrote it.

>
>> +  if (Subtarget->isTargetWindows()
>> +   && !Subtarget->isTargetCygMing()
>> +   && MMI->callsExternalFunctionWithFloatingPointArguments()) {
> LLVM code tends to put && into prev. line in multi-line conditions
>
> --
> With best regards, Anton Korobeynikov
> Faculty of Mathematics and Mechanics, Saint Petersburg State University
>

Thanks for the review!

- Michael Spencer