[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