[PATCH] Reassociate x + -0.1234 * y into x - 0.1234 * y (part 2)

Chandler Carruth chandlerc at gmail.com
Sat Nov 1 18:50:16 PDT 2014


Generally looks good. Some minor code improvements suggested, but nothing significant really.

================
Comment at: lib/Transforms/Scalar/Reassociate.cpp:1948-1955
@@ +1947,10 @@
+  unsigned ConstIdx = C0 ? 0 : 1;
+  if (!isa<ConstantInt>(C) && !isa<ConstantFP>(C))
+    return nullptr;
+  if (ConstantInt *CI = dyn_cast<ConstantInt>(C))
+    if (!CI->isNegative())
+      return nullptr;
+  if (ConstantFP *CF = dyn_cast<ConstantFP>(C))
+    if (!CF->isNegative())
+      return nullptr;
+
----------------
Rather than this, it would be nice to write in a way that there is only one test of the constant type:

  if (auto *CI = dyn_cast<ConstantInt>(C)) {
    // ..
  } else if (auto *CF = dyn_cast<ConstantFP>(C)) {
    // ..
  } else {
    return nullptr;
  }

================
Comment at: lib/Transforms/Scalar/Reassociate.cpp:1977
@@ +1976,3 @@
+
+  unsigned UsrOpcode = User->getOpcode();
+  if (UsrOpcode != Instruction::Add && UsrOpcode != Instruction::FAdd &&
----------------
The shortening "Usr" is a bit ... awkward to me. ;] Is there a problem with just calling it "UserOpcode"?

http://reviews.llvm.org/D5363






More information about the llvm-commits mailing list