riscv-software-src / riscv-isa-sim

Spike, a RISC-V ISA Simulator
Other
2.4k stars 849 forks source link

Zfa extension instruction fcvtmod_w_d behavior conflict with sail model on too large/ too small boundary #1669

Open GuoShibo-cn opened 5 months ago

GuoShibo-cn commented 5 months ago
  int true_exp = exp - 1023;
  int shift = true_exp - 52;

  /* Restore implicit bit.  */
  frac |= 1ull << 52;

  /* Shift the fraction into place.  */
  if (shift >= 64) {
    /* The fraction is shifted out entirely.  */
    frac = 0;
  } else  if ((shift >= 0) && (shift < 64)) {
    /* The number is so large we must shift the fraction left.  */
    frac <<= shift;
  } else if ((shift > -64) && (shift < 0)) {
    /* Normal case -- shift right and notice if bits shift out.  */
    inexact = (frac << (64 + shift)) != 0;
    frac >>= -shift;
  } else {
    /* The fraction is shifted out entirely.  */
    frac = 0;
    inexact = true;
  }

According to spike fcvtmod_w_d.h too large/ too small boundary true_exp is shift+52 which is (64 + 52 = 116)/ (-64 + 52 = -12) however according to sail model sail model

  let is_too_large  = true_exp >= 84;  /* bits will be 'multiplied' out */
  let is_too_small  = true_exp <  0;   /* bits will be 'divided' out    */

       if is_zero       then (zeros(),  zeros())
  else if is_subnorm    then (nxFlag(), zeros())
  else if is_nan_or_inf then (nvFlag(), zeros())
  else if is_too_large  then (nvFlag(), zeros())
  else if is_too_small  then (nxFlag(), zeros())
  else {

boundary true_exp is 84/0 which model could be considered as meet specification?

aswaterman commented 5 months ago

Can you provide a particular input where Spike and the SAIL model disagree, and let us know what the result is in both cases?

nibrunieAtSi5 commented 5 months ago

I am not a SAIL expert but it seems the implementations differ in how they handle the building of a temporary value (84 bits fixed point for SAIL while spike uses a 64-bit supporting integer type for the integer part).