[PATCH] [InstCombineAddSub.cpp] Corrected Formatting and Indentation
suyog
suyog.sarda at samsung.com
Tue Oct 7 23:41:20 PDT 2014
Few comments inline.
I would suggest to re-direct it to someone who is more familiar with clang-format.
================
Comment at: lib/Transforms/InstCombine/InstCombineAddSub.cpp:1525
@@ -1524,3 +1524,2 @@
-
Instruction *InstCombiner::visitSub(BinaryOperator &I) {
----------------
Removing this extra space is Ok.
================
Comment at: lib/Transforms/InstCombine/InstCombineAddSub.cpp:1598
@@ -1599,1 +1597,3 @@
+ Value *X;
+ ConstantInt *CI;
if (match(Op1, m_LShr(m_Value(X), m_ConstantInt(CI))) &&
----------------
This is Ok.
================
Comment at: lib/Transforms/InstCombine/InstCombineAddSub.cpp:1601
@@ -1600,3 +1600,3 @@
// Verify we are shifting out everything but the sign bit.
- CI->getValue() == I.getType()->getPrimitiveSizeInBits()-1)
+ CI->getValue() == I.getType()->getPrimitiveSizeInBits() - 1)
return BinaryOperator::CreateAShr(X, CI);
----------------
Not sure if this change is ok. Comment in between confuses things.
clang-format may consider to include extra space.
I would suggest to leave it like as it is.
================
Comment at: lib/Transforms/InstCombine/InstCombineAddSub.cpp:1606
@@ -1605,3 +1605,3 @@
// Verify we are shifting out everything but the sign bit.
- CI->getValue() == I.getType()->getPrimitiveSizeInBits()-1)
+ CI->getValue() == I.getType()->getPrimitiveSizeInBits() - 1)
return BinaryOperator::CreateLShr(X, CI);
----------------
Not sure if this change is ok. Comment in between confuses things.
clang-format may consider to include extra space.
I would suggest to leave it like as it is, with space padding for '-1'
================
Comment at: lib/Transforms/InstCombine/InstCombineAddSub.cpp:1612
@@ -1613,1 +1611,3 @@
+ {
+ Value *Y;
// X-(X+Y) == -Y X-(Y+X) == -Y
----------------
This is ok.
================
Comment at: lib/Transforms/InstCombine/InstCombineAddSub.cpp:1631
@@ -1632,1 +1630,3 @@
+ return BinaryOperator::CreateAdd(
+ Op0, Builder->CreateSub(Z, Y, Op1->getName()));
----------------
Not sure if this is Ok.
================
Comment at: lib/Transforms/InstCombine/InstCombineAddSub.cpp:1638
@@ -1639,1 +1637,3 @@
+ return BinaryOperator::CreateAnd(
+ Op0, Builder->CreateNot(Y, Y->getName() + ".not"));
----------------
Not sure if this is Ok.
================
Comment at: lib/Transforms/InstCombine/InstCombineAddSub.cpp:1702
@@ -1703,1 +1701,3 @@
+ if (Value *V =
+ SimplifyFSubInst(Op0, Op1, I.getFastMathFlags(), DL, TLI, DT, AT))
return ReplaceInstUsesWith(I, V);
----------------
Not sure if this is ok.
http://reviews.llvm.org/D5649
More information about the llvm-commits
mailing list