ebevhan added inline comments.
================
Comment at: include/clang/AST/Type.h:6577
+
+inline unsigned getFixedPointFBits(const QualType &Ty) {
+ return getFixedPointFBits(*Ty);
----------------
As I mentioned in another patch, this should be in ASTContext and ask
TargetInfo for the information.
I'm not a fan of the term 'fbits'. The correct terminology should be the
'scaling factor'.
================
Comment at: lib/CodeGen/CGExprScalar.cpp:1009
+ // a float and perform some floating point arithmetic, though this may cost
+ // more. Enable that if #pragma FX_FULL_PRECISION is provided.
+ if (dest_fbits > src_fbits) {
----------------
For future reference, please do not rely on the behavior of floating point for
fixed-point operations.
================
Comment at: lib/CodeGen/CGExprScalar.cpp:1184
+ // Casting down, so we will need to shift early as to not lose data
+ Src = EmitFixedPointRadixShift(Src, SrcType, DstType);
+ }
----------------
I think it's safer to simply do all relevant fixed-point conversion by hand
here instead of falling through and relying on the semantics of other
conversions.
We can provide patches for our implementation here if wanted.
================
Comment at: lib/CodeGen/CGExprScalar.cpp:1214
+ if (WorkingOnFixedPoints && order >= 0) {
+ // Casting up (or same type), so we can safely shift without losing data
----------------
This case should not be needed. Perform all fixed-point conversion before the
integer type check above and do an early return instead.
It's not safe to rely on the behavior of other conversions for the behavior of
these.
Repository:
rC Clang
https://reviews.llvm.org/D46927
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
