[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-bugs-dist
Subject:    [valgrind] [Bug 214950] New tool exp-floattrap
From:       Florian Krohm <florian () eich-krohm ! de>
Date:       2015-08-01 10:32:56
Message-ID: bug-214950-17878-i5HKuC6VwR () http ! bugs ! kde ! org/
[Download RAW message or body]

https://bugs.kde.org/show_bug.cgi?id=214950

--- Comment #7 from Florian Krohm <florian@eich-krohm.de> ---
(In reply to Mathias Fröhlich from comment #5)

I'm going to answer a few quick questions here:

> When I see c++ comments, I would think that valgrind wants c comments, right?
> 

For multi-line comments we'd like C-style. For inline comments C++ comments are
fine, but there is no strong preference one way or the other.

> > +insert_check_double128(IRSB* bb, IRTemp value)
> > +{
> > +  IRTemp tempHi, tempLo;
> > +
> > +  tempHi = addUnop(bb, Ity_F64, Iop_F128HItoF64, IRExpr_RdTmp(value));
> > +  insert_check_double(bb, tempHi);
> > +
> > +  tempLo = addUnop(bb, Ity_F64, Iop_F128LOtoF64, IRExpr_RdTmp(value));
> > +  insert_check_double(bb, tempLo);
> > 
> > So you're using insert_check_double twice. I doubt that is correct.
> Twice, yes, once on the high and once on the low part.
> Actually, that is something I was wondering about. I was looking for a
> conversion
> operator from F128 to I128 so that I can do the same bitmask trick like the
> others precisions do. But I could not find such a conversion.
> So how is that F128 supposed to work with VEX? Is this the usual doubledouble
> approach or is this a real 128 bit IEEE float?

F128 is a true 128 bit IEEE float. You cannot look at it as two 64 bit floats.
There is no need for a F128 to I128 conversion because there are no 128 bit
wide integers on the platforms we support. You can convert to I64 or I32 or
take the low and high 8 bytes of an F128.
I have access to a machine with 128 bit floating point values to I can help
with testing that.

> Is there some more documentation on VEX? I just did match what appears
> plausible to me.

libvex_ir.h is the main documentation and the lackey source tree (and 'none'
source tree) are good for inspiration.

> So, having said that, if there is a chance for this to go upstream, I can
> polish whatever
> you want. But if not, I have to realize that there is life beyond software
> which I enjoy also.

This is the main point we need to discuss before I take another look at the
patch.
The tool itself is simple enough so, once finalised, I do not see much of a
maintenance issue. We can just carry it along and adjust to VEX changes of
which there are not many.

However, having a proper set of testcases is critical to getting the patch
upstream. What is currently there is not good enough. Also, you need to
convince us that the code is working properly. It currently does not for 128
bit floats and I'm not at all sure that the handling of the
various SIMD IROps is correct either. Testcases help here as well as do more
comments in the code.
You need to figure out for yourself whether that is something you're willing to
do.

-- 
You are receiving this mail because:
You are watching all bug changes.=
[prev in list] [next in list] [prev in thread] [next in thread]