[PATCH] D61544: Add FNeg IR constant folding

[Sanjay Patel via Phabricator via llvm-commits]

spatel added inline comments.


================
Comment at: llvm/include/llvm/Analysis/ConstantFolding.h:75-76
+/// Attempt to constant fold a unary operation with the specified
+/// operand. If it fails, it returns a constant expression of the specified
+/// operands.
+Constant *ConstantFoldUnaryOpOperand(unsigned Opcode, Constant *Op,
----------------
IIUC, this comment is wrong. If it fails, it returns a nullptr. No attempt is made to reduce a ConstExpr (ie, there is no SymbolicallyEvaluateUnOp()). 

The existing comment below for binops looks misleading if I interpreted that correctly.


================
Comment at: llvm/lib/Analysis/ConstantFolding.cpp:1002
 
+  // Handle easy unops first.
+  if (Instruction::isUnaryOp(Opcode))
----------------
I know this is modified from the existing comment line, but I don't see how either adds value. I'd remove both lines.


================
Comment at: llvm/lib/IR/ConstantFold.cpp:931
+      return C; // -undef -> undef
+    case Instruction::UnaryOpsEnd:
+      llvm_unreachable("Invalid UnaryOp");
----------------
We already asserted that we have a unary op, so can this be "default: llvm_unreachable..."?


================
Comment at: llvm/lib/IR/Constants.cpp:1834
+  if (Constant *FC = ConstantFoldUnaryInstruction(Opcode, C))
+    return FC;          // Fold a few common cases.
 
----------------
Code comment doesn't add anything IMO.


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D61544/new/

https://reviews.llvm.org/D61544