[LLVMdev] Representing -ffast-math at the IR level

[Chandler Carruth]

Thanks for the updates!

Minor comments:
+  if (!Accuracy)
+    // If it's not a floating point number then it must be 'fast'.
+    return HUGE_VALF;

Can we add an assert instead of a comment? It's just as documenting and
will catch any goofs.

+  // If it's not a floating point number then it must be 'fast'.
+  return !isa<ConstantFP>(MD->getOperand(0));

Here as well.

+    if (ConstantFP *CFP0 = dyn_cast_or_null<ConstantFP>(Op0)) {
+      APFloat Accuracy = CFP0->getValueAPF();
+      Assert1(Accuracy.isNormal() && !Accuracy.isNegative(),
+              "fpmath accuracy not a positive number!", &I);

To be pedantic for a moment, zero is not a positive number. What about
asserting these individually to give us more clear asserts if they fire?
That also makes the string easier to write: "fpmath accuracy is a negative
number!".

+  /// SetDefaultFPMathTag - Set the floating point math metadata to be
used.
+  void SetDefaultFPMathTag(MDNode *FPMathTag) { DefaultFPMathTag =
FPMathTag; }

This should be 'setDefault...' much like 'getDefault...' above.

+  Instruction *AddFPMathTag(Instruction *I, MDNode *FPMathTag) const {

Another bad case, but I think this instruction is gone...

+    MDString *GetFastString() const {
+      return CreateString("fast");
+    }

'getFastString'.

+    /// CreateFastFPMath - Return metadata with appropriate settings for
'fast
+    /// math'.

I would prefer the more modern doxygen style:

/// \brief Return metadata ...

+    MDNode *CreateFastFPMath() {

Capitalization.

The capitalization and doxygen style comments apply to the next function as
well.


Both the Clang and DragonEgg patches look good, but both need test cases. =]
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-dev/attachments/20120416/466d8d0e/attachment.html>