-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/2119/
-----------------------------------------------------------
(Updated Dec. 16, 2013, 10:55 a.m.)
Review request for Default.
Changes
-------
Additional testing.
Repository: gem5
Description
-------
mips: Floating point convert bug fix
Floating point convert instructions use the FloatConvertOp format defined
in src/arch/mips/isa/formats/fp.isa. The type of the operands in the ISA
description file (_sw for signed word, or _sf for signed float, etc.) is
used to create a type for the operand in C++. Then the operand is converted
using the fpConvert() function in src/arch/mips/utility.cc.
Here's what an fp cvt looks like in the generated execution function in
build/MIPS/arch/mips/generated/inorder_cpu_exec.cc::
uint32_t val = Fs;
Fd = fpConvert(WORD_TO_SINGLE, val);
The fpConvert function is in src/arch/mips/utility.cc::
uint64_t
fpConvert(ConvertType cvt_type, double fp_val)
{
switch (cvt_type)
{
case SINGLE_TO_DOUBLE:
{
double sdouble_val = fp_val;
void *sdouble_ptr = &sdouble_val;
uint64_t sdp_bits = *(uint64_t *) sdouble_ptr;
return sdp_bits;
}
case SINGLE_TO_WORD:
{
int32_t sword_val = (int32_t) fp_val;
void *sword_ptr = &sword_val;
uint64_t sword_bits= *(uint32_t *) sword_ptr;
return sword_bits;
}
(...)
}
}
Basically it does pointer casting to read values as a different type.
The bug is that _val_ is unsigned instead of signed. So if we are converting
from
a word to a float, and we want to convert 0xffffffff, we expect -1 to be passed
into
fpConvert(). Instead, we see MAX_INT passed in. Then fpConvert() converts _val_
to
MAX_INT in single-precision floating point, and we get the wrong value.
To fix it, I changed the signs of the convert operands from unsigned to signed
in the MIPS ISA description.
Then, I changed the FloatConvertOp format to insert a int32_t into the C++ code
instead
of a uint32_t.
Diffs
-----
src/arch/mips/isa/decoder.isa bdd606534bdc
src/arch/mips/isa/formats/fp.isa bdd606534bdc
Diff: http://reviews.gem5.org/r/2119/diff/
Testing (updated)
-------
I've tested this in two ways: 1) by writing a MIPS assembly test that
converts a test set of words to floats, and 2) in a C++ isolation test.
**MIPS assembly test**
I've compiled some handwritten assembly like this::
TEST( cvt.s.w, 0x4372a3d5, 0x4e86e548 )
TEST( cvt.s.w, 0xffffffff, 0xbf800000 )
.. where TEST is a test macro that uses the instruction in arg0 with the data
in arg1.
The reference output is in arg2.
The objdump looks like this for each test::
808000: 3c024372 lui v0,0x4372
808004: 3442a3d5 ori v0,v0,0xa3d5
808008: 46801120 cvt.s.w a0,v0
80800c: 3c014e86 lui at,0x4e86
808010: 3421e548 ori at,at,0xe548
808014: 14810095 bne a0,at,808270 <_fail>
$V0 gets the src data. $A0 gets the fp-converted data. $AT gets the reference
data.
We branch to fail code if they don't match.
All negative number tests fail without this patch.
**C++ testing**
I copied the fpConvert() function and the ConvertType enum to a separate C++
test file to test in isolation.
fptest.cc::
#include <iostream>
#include <stdint.h>
//used in FP convert & round function
enum ConvertType{
SINGLE_TO_DOUBLE,
SINGLE_TO_WORD,
SINGLE_TO_LONG,
DOUBLE_TO_SINGLE,
DOUBLE_TO_WORD,
DOUBLE_TO_LONG,
LONG_TO_SINGLE,
LONG_TO_DOUBLE,
LONG_TO_WORD,
LONG_TO_PS,
WORD_TO_SINGLE,
WORD_TO_DOUBLE,
WORD_TO_LONG,
WORD_TO_PS,
PL_TO_SINGLE,
PU_TO_SINGLE
};
uint64_t
fpConvert(ConvertType cvt_type, double fp_val)
{
switch (cvt_type)
{
case SINGLE_TO_DOUBLE:
{
double sdouble_val = fp_val;
void *sdouble_ptr = &sdouble_val;
uint64_t sdp_bits = *(uint64_t *) sdouble_ptr;
return sdp_bits;
}
case SINGLE_TO_WORD:
{
int32_t sword_val = (int32_t) fp_val;
void *sword_ptr = &sword_val;
uint64_t sword_bits= *(uint32_t *) sword_ptr;
return sword_bits;
}
case WORD_TO_SINGLE:
{
float wfloat_val = fp_val;
void *wfloat_ptr = &wfloat_val;
uint64_t wfloat_bits = *(uint32_t *) wfloat_ptr;
return wfloat_bits;
}
case WORD_TO_DOUBLE:
{
double wdouble_val = fp_val;
void *wdouble_ptr = &wdouble_val;
uint64_t wdp_bits = *(uint64_t *) wdouble_ptr;
return wdp_bits;
}
default:
std::cout << "ERROR" << std::endl;
return 0;
}
}
int main(int argc, const char *argv[])
{
uint32_t test_value_original = 0xffffffff;
int32_t test_value_patched = 0xffffffff;
uint64_t converted_value_original = fpConvert(WORD_TO_SINGLE,
test_value_original);
uint64_t converted_value_patched = fpConvert(WORD_TO_SINGLE,
test_value_patched);
std::cout << std::hex << converted_value_original << std::endl;
std::cout << std::hex << converted_value_patched << std::endl;
return 0;
}
In main, I print out the fpConvert'ed values for 0xffffffff inputs wrapped
inside uint32_t and int32_t. The output is::
4f800000
bf800000
0xBF800000 is the expected value (-1 in single precision floating point).
0x4F800000 is MAX_INT in single precision floating point -- not what we
want.
Thanks,
Christopher Torng
_______________________________________________
gem5-dev mailing list
[email protected]
http://m5sim.org/mailman/listinfo/gem5-dev
