[cfe-commits] Patch for evaluating FP constant expressions

[Christopher Lamb]

On Nov 29, 2007, at 11:51 AM, Chris Lattner wrote:

> On Nov 28, 2007, at 10:18 PM, Christopher Lamb wrote:
>
>> I needed this to support negation of FP literals, which is pretty  
>> unambiguous. It looks like GCC evaluates more complex constant  
>> expressions too, so I put in support for some of that (though  
>> rounding mode is an issue). All of this is modeled on the work for  
>> integer expressions and uses APFloat.
>
> Very interesting, I didn't realize that this was non-conforming:
>
> double x = -1.0;
>
> A couple of comments:
>
> +  /// isRealFloatingConstantExpr - Return true if this expression  
> is a valid
> +  /// floating point constant expression, and, if so, return its  
> value in
> +  /// Result.  If not a valid rfce, return false and fill in Loc  
> (if specified)
> +  /// with the location of the invalid expression.
> +  bool isRealFloatingConstantExpr(llvm::APFloat &Result,  
> ASTContext &Ctx,
> +                                  SourceLocation *Loc = 0,
> +                                  bool isEvaluated = true) const;
>
> Please add a comment that states this can only be used for  
> statically initialized values.  For non-static initializers, the  
> code should be evaluated in its current context, which could be  
> affected by rounding mode etc.
>

Ok.

> +    // FIXME: How to choose the rounding mode? Emulate GCC behavior?
>
> This codegen routine should only be used for static initializers.   
> Static inits are evaluated before main and static ctors run (e.g.  
> at compile time ;-), so they can only be evaluated with the default  
> rounding mode.  Please add a comment to this effect.

Ok.

>
> +  case BinaryOperatorClass: {
> +    const BinaryOperator *Exp = cast<BinaryOperator>(this);
> +
> +    // The LHS of a constant expr is always evaluated and needed.
> +    if (!Exp->getLHS()->isRealFloatingConstantExpr(Result, Ctx, Loc,
> +                                                   isEvaluated))
> +      return false;
> +
> +    llvm::APFloat RHS(Result);
> +
> +    // The short-circuiting &&/|| operators don't necessarily  
> evaluate their
> +    // RHS.  Make sure to pass isEvaluated down correctly.
> +    if (Exp->isLogicalOp()) {
> +      bool RHSEval;
> +      if (Exp->getOpcode() == BinaryOperator::LAnd)
> +        RHSEval = Result.isNonZero();
> +      else {
> +        assert(Exp->getOpcode() == BinaryOperator::LOr  
> &&"Unexpected logical");
> +        RHSEval = Result.isZero();
> +      }
>
> Logical ops like && always return bool, not an fp type, so I think  
> this code is dead.

I agree.

> It would probably also be worthwhile to handle int <-> fp casts as  
> well, but that can be a follow-on patch if you prefer.

Indeed. Later, though.

> +    case BinaryOperator::Div:
> +      if (llvm::APFloat::opOK !=
> +          Result.divide(RHS, llvm::APFloat::rmNearestTiesToEven)) {
> +        if (!isEvaluated) break;
> +        if (Loc) *Loc = getLocStart();
> +        return false;
> +      }
>
> It would be "nice" if this reported diagnostics for overflow etc as  
> warnings.


This is in Expr.cpp and it's called from CodeGen, how can I get the  
diagnostics object here?

--
Christopher Lamb



-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20071130/2b260965/attachment.html>