[llvm-commits] [llvm] r171922 - in /llvm/trunk: include/llvm/IR/Constant.h include/llvm/IR/InstrTypes.h lib/IR/Constants.cpp lib/IR/Instructions.cpp lib/Transforms/InstCombine/InstCombine.h lib/Transforms/InstCombine/InstructionCombining.cpp test/Transforms/InstCombine/fast-math.ll

[Shuxin Yang]

Hi, Eric:

    Thank you for your feedback. See following inline comment.

On 1/8/13 4:28 PM, Eric Christopher wrote:
> Making no claims about this:
>
>     Consider expression "0.0 - X" as the negation of X if
>       - this expression is explicitly marked no-signed-zero, or
>       - no-signed-zero of this expression can be derived from some
>     context.
>
>
> since I haven't reviewed that part. However,
I sent the review request to the list last week.  There are few 
discussion about this change.
I guess I'm ok to commit now.

>
>
>     +  /// Return true if the value is negative zero or null value.
>     +  bool isZeroValue() const;
>     +
>
>
> The naming of this routine is really odd given the function just above 
> it in the file and

class Constant has a member function call isNegativeZeroValue().
I don't feel like to invent a completely different name, so I coin the 
name "isZeroValue()".
When the isZeroValue() and isNegativeZeroValue() stands side-by-side, it 
ease the reader
to guess that isZeroValue() tries to achieve.

What's your suggestion?
>
>     +bool Constant::isZeroValue() const {
>     +  // Floating point values have an explicit -0.0 value.
>     +  if (const ConstantFP *CFP = dyn_cast<ConstantFP>(this))
>     +    return CFP->isZero();
>     +
>     +  // Otherwise, just use +0.0.
>     +  return isNullValue();
>     +}
>     +
>
>
> and the implementation. Can you explain a bit more?
>
This function will return true iff :
    - is integer 0, or
    - is floating point positive or negative 0, i.e. we don't care sign.

Thanks
Shuxin




-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20130108/3770ae36/attachment.html>