-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://reviews.gem5.org/r/2119/
-----------------------------------------------------------

Review request for Default.


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
-------

I've tested this by writing a MIPS assembly test that converts a test set of 
words to
floats. For example::

        TEST( cvt.s.w, 0x4372a3d5, 0x4e86e548 )
        TEST( cvt.s.w, 0xffffffff, 0xbf800000 )

.. where TEST is a test macro. 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: 241d000d  li  sp,13
  808010: 3c014e86  lui at,0x4e86
  808014: 3421e548  ori at,at,0xe548
  808018: 14810095  bne a0,at,808270 <_fail>

Without this bug fix, all negative number tests fail.


Thanks,

Christopher Torng

_______________________________________________
gem5-dev mailing list
[email protected]
http://m5sim.org/mailman/listinfo/gem5-dev

Reply via email to