[PATCH] D66862: Make lround builtin constexpr (and others)

[Craig Topper via Phabricator via cfe-commits]

craig.topper added inline comments.


================
Comment at: clang/lib/AST/ExprConstant.cpp:9616
+    return EvaluateFloat(E->getArg(0), Val, Info) &&
+           Success(lround(Val.convertToDouble()), E);
+  }
----------------
zoecarver wrote:
> lebedev.ri wrote:
> > zoecarver wrote:
> > > rsmith wrote:
> > > > This assumes that the host `double` has the same behavior as the target `double`, which is not true in general. Generally the right thing to do is to implement the relevant functionality on `APFloat`, taking care to produce the right answer for the floating-point model being used by that `APFloat`. This also assumes that the host `long` is the same size as the target `long`, which again is not true in general. Finally, you need to catch the case where the `APFloat` does not fit into the target type; in that case, the best response is probably to treat the evaluation as non-constant and leave it to the runtime library to implement whatever error response it chooses.
> > > I've updated it to do just that: if there is an error, it will treat this evaluation as non-constant and leave it for the runtime library to handle. 
> > > 
> > > How should I determine the width of the APInt? Because it is run at compile-time, can I use the max size (long long)? Either way, how should I get the correct size, maybe from a builtin long type? 
> > You probably want to look at the original callexpr, take it's return type, and look it up in typesystem (i'm not sure exactly how to do that here)
> Ah, good idea. Will do.
Info.Ctx.getIntWidth(E->getType())    I think.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66862