[llvm-commits] [llvm] r171793 - in /llvm/trunk: lib/Transforms/InstCombine/InstCombine.h lib/Transforms/InstCombine/InstCombineMulDivRem.cpp test/Transforms/InstCombine/fast-math.ll

Shuxin Yang shuxin.llvm at gmail.com
Mon Jan 7 14:17:07 PST 2013


Hi, Eric:

    Thank you for your feedback. See the question/comment inline.

On 1/7/13 2:01 PM, Eric Christopher wrote:
>
>
>     +  Value *foldFMulConst(Instruction *FMulOrDiv, ConstantFP *C,
>     +                       Instruction *InsertBefore);
>
>
> Formatting.

The real code in VI/emacs and the diff sent to the list looks quite 
different from what you pasted here.
In the code the two "Instruction"s are aligned. (I'm sure there is no 
leading tab, I configure my vi such
that all tab is converted to space).

Are you talking about the alignment issue. Or something else?
I cannot fold the 2nd line to the previous line as it exceeds 80 column.

>     +  if (!I || (I->getOpcode() != Instruction::FMul &&
>     +             I->getOpcode() != Instruction::FDiv)) {
>     +    return false;
>     +  }
>     +
>
>
> No need for braces here.
Okay.
>
>     +Value *InstCombiner::foldFMulConst
>     +  (Instruction *FMulOrDiv, ConstantFP *C, Instruction
>     *InsertBefore) {
>
>
> Formatting.
The 2nd line is indented by 2 space in the real-code?
By "formating", do you mean format like this:
   "Value *class::member (type para1,  /*80-th-column here*/
                                        type para2);



>     +        if (C0) {
>     +          std::swap(C0, C1); std::swap(Opnd0, Opnd1); Swap = true;
>     +        }
>     +
>
>
> Was going to say no need for braces here, but I missed the Swap = at 
> the end of the line, how about putting that on the next line so it's 
> more clear?
Okay.

Thank you again!

> -eric

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130107/c3f257b9/attachment.html>


More information about the llvm-commits mailing list