commit cd1993a1c36c4fc44867354bf80688d2df6bff2b
author Cindy Liu <hcindyl@google.com> Mon Jul 24 21:21:16 2023 -0700
committer Cindy Liu <hcindyl@google.com> Mon Jul 24 21:26:13 2023 -0700
tree 286af233cabe9e5c858a435cc34d2a792b3da0c1
parent e48434aadf84ca02bd64f052ef6717ecf6b53a9a

Fix vabsd overflow problem

vabsd should always return unsigned result regardless the signess of the inputs. However, if we use the input datatype within the operation there could be overflow problem when vs1 and vs2 are in the opposite sign.

PiperOrigin-RevId: 550763471

sim/kelvin_vector_instructions.cc

diff --git a/sim/kelvin_vector_instructions.cc b/sim/kelvin_vector_instructions.cc
index 94c9724..5470b24 100644
--- a/sim/kelvin_vector_instructions.cc
+++ b/sim/kelvin_vector_instructions.cc
@@ -267,8 +267,12 @@
       inst, scalar, strip_mine,
       std::function<typename std::make_unsigned<T>::type(T, T)>(
           [](T vs1, T vs2) -> typename std::make_unsigned<T>::type {
-            T result = vs1 > vs2 ? vs1 - vs2 : vs2 - vs1;
-            return static_cast<typename std::make_unsigned<T>::type>(result);
+            using UT = typename std::make_unsigned<T>::type;
+            // Cast to unsigned type before the operation to avoid undefined
+            // overflow behavior in intx_t.
+            UT uvs1 = static_cast<UT>(vs1);
+            UT uvs2 = static_cast<UT>(vs2);
+            return vs1 > vs2 ? uvs1 - uvs2 : uvs2 - uvs1;
           }));
 }
 template void KelvinVAbsd<int8_t>(bool, bool, Instruction *);

sim/test/kelvin_vector_instructions_test.cc

diff --git a/sim/test/kelvin_vector_instructions_test.cc b/sim/test/kelvin_vector_instructions_test.cc
index ee6ccfb..6aede99 100644
--- a/sim/test/kelvin_vector_instructions_test.cc
+++ b/sim/test/kelvin_vector_instructions_test.cc
@@ -450,7 +450,9 @@
 template <typename Vd, typename Vs1, typename Vs2>
 struct VAbsdOp {
   static Vd Op(Vs1 vs1, Vs2 vs2) {
-    Vs1 result = vs1 > vs2 ? vs1 - vs2 : vs2 - vs1;
+    int64_t vs1_ext = static_cast<int64_t>(vs1);
+    int64_t vs2_ext = static_cast<int64_t>(vs2);
+    auto result = vs1_ext > vs2_ext ? vs1_ext - vs2_ext : vs2_ext - vs1_ext;
     return static_cast<Vd>(result);
   }
   static void KelvinOp(bool scalar, bool strip_mine, Instruction *inst) {
@@ -458,8 +460,7 @@
   }
 };
 
-// TODO(b/291683818): Check the implementation and re-enable the logic.
-TEST_F(KelvinVectorInstructionsTest, DISABLED_VAbsd) {
+TEST_F(KelvinVectorInstructionsTest, VAbsd) {
   KelvinVectorBinaryOpHelper<VAbsdOp, uint8_t, int8_t, int8_t, uint16_t,
                              int16_t, int16_t, uint32_t, int32_t, int32_t>(
       "VAbsd");