donat.nagy marked 7 inline comments as done.
donat.nagy added a comment.
I found a solution for determining the precision value of a floating point
type. Is this acceptable?
================
Comment at: lib/StaticAnalyzer/Checkers/ConversionChecker.cpp:163
+
+ switch (FloatingSize) {
+ case 64:
----------------
NoQ wrote:
> donat.nagy wrote:
> > NoQ wrote:
> > > Continuing the float semantics discussion on the new revision - Did you
> > > consider `llvm::APFloat`?
> > > (http://llvm.org/doxygen/classllvm_1_1APFloat.html) This looks like
> > > something that you're trying to re-implement.
> > This switch statement essentially fulfills two functions: it maps QualTypes
> > to standardized IEEE floating point types and it immediately maps the
> > standardized IEEE types to their precision values.
> >
> > The difficulty is that I don't think that the first map is available as a
> > public function in the clang libraries. This means that although a switch
> > over the bit width of the floating point type is certainly inelegant, I
> > cannot avoid it.
> >
> > The second map is available in the APFloat library (via the
> > llvm::fltSemantics constants, which describe the standard IEEE types).
> > However, this map is extremely stable (e.g. I don't think that the binary
> > structure of the IEEE double type will ever change), so I think that
> > re-implementing it is justified by the fact that it yields shorter and
> > cleaner code. However, as I had noted previously, I am open to using the
> > llvm::fltSemantics constants to implement this mapping.
> >
> > As an additional remark, my current code supports the _Float16 type, which
> > is currently not supported by the APFloat/fltSemantics library. If we
> > decide to use the llvm::fltSemantics constants, then we must either extend
> > the APFloat library or apply some workarounds for this case.
> >
> > The difficulty is that I don't think that the first map is available as a
> > public function in the clang libraries. This means that although a switch
> > over the bit width of the floating point type is certainly inelegant, I
> > cannot avoid it.
>
> `fltSemantics` are not just enum constants, [[
> http://llvm.org/doxygen/structllvm_1_1fltSemantics.html | they contain a lot
> of fancy data ]]:
> ```
> static const fltSemantics semIEEEhalf = {15, -14, 11, 16};
> static const fltSemantics semIEEEsingle = {127, -126, 24, 32};
> static const fltSemantics semIEEEdouble = {1023, -1022, 53, 64};
> static const fltSemantics semIEEEquad = {16383, -16382, 113, 128};
> static const fltSemantics semX87DoubleExtended = {16383, -16382, 64, 80};
> ```
I knew about the data members of fltSemantics, but I did not see a way to use
them elegantly (although fltSemantics has sizeInBits as a member, the possible
fltSemantics values are not collected into one data structure, so I could not
look up the precision field).
However, as I examined the APFloat.h source again, I found the function
llvm::APFloat::getAllOnesValue which allows me to map bit width to semantics.
While this is a somewhat indirect solution, it avoids using a switch.
The `missing _Float16' problem mentioned in my previous comments was my
mistake, somehow I managed to overlook semIEEEhalf.
Repository:
rC Clang
https://reviews.llvm.org/D52730
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
