rkavanap commented on a change in pull request #11053:
URL: https://github.com/apache/arrow/pull/11053#discussion_r740294476



##########
File path: cpp/src/gandiva/precompiled/arithmetic_ops.cc
##########
@@ -104,6 +104,69 @@ NUMERIC_DATE_TYPES(BINARY_RELATIONAL, 
greater_than_or_equal_to, >=)
 
 #undef BINARY_RELATIONAL
 
+// Returns the greatest or least value from a list of values
+#define COMPARE_TWO_VALUES(NAME, TYPE, OP)                            \
+  FORCE_INLINE                                                        \
+  gdv_##TYPE NAME##_##TYPE##_##TYPE(gdv_##TYPE in1, gdv_##TYPE in2) { \
+    return (in1 OP in2 ? in1 : in2);                                  \
+  }
+
+#define COMPARE_THREE_VALUES(NAME, TYPE, OP)                                 \
+  FORCE_INLINE                                                               \
+  gdv_##TYPE NAME##_##TYPE##_##TYPE##_##TYPE(gdv_##TYPE in1, gdv_##TYPE in2, \
+                                             gdv_##TYPE in3) {               \
+    gdv_##TYPE compared = (in1 OP in2 ? in1 : in2);                          \
+    return (compared OP in3 ? compared : in3);                               \
+  }
+
+#define COMPARE_FOUR_VALUES(NAME, TYPE, OP)                                    
         \
+  FORCE_INLINE                                                                 
         \
+  gdv_##TYPE NAME##_##TYPE##_##TYPE##_##TYPE##_##TYPE(gdv_##TYPE in1, 
gdv_##TYPE in2,   \
+                                                      gdv_##TYPE in3, 
gdv_##TYPE in4) { \
+    gdv_##TYPE compared = (in1 OP in2 ? in1 : in2);                            
         \
+    compared = (compared OP in3 ? compared : in3);                             
         \
+    return (compared OP in4 ? compared : in4);                                 
         \
+  }
+
+#define COMPARE_FIVE_VALUES(NAME, TYPE, OP)                                    
         \
+  FORCE_INLINE                                                                 
         \
+  gdv_##TYPE NAME##_##TYPE##_##TYPE##_##TYPE##_##TYPE##_##TYPE(                
         \
+      gdv_##TYPE in1, gdv_##TYPE in2, gdv_##TYPE in3, gdv_##TYPE in4, 
gdv_##TYPE in5) { \
+    gdv_##TYPE compared = (in1 OP in2 ? in1 : in2);                            
         \
+    compared = (compared OP in3 ? compared : in3);                             
         \
+    compared = (compared OP in4 ? compared : in4);                             
         \
+    return (compared OP in5 ? compared : in5);                                 
         \
+  }
+
+#define COMPARE_SIX_VALUES(NAME, TYPE, OP)                                     
       \

Review comment:
       Is six maximum for a list?

##########
File path: cpp/src/gandiva/precompiled/arithmetic_ops.cc
##########
@@ -104,6 +104,69 @@ NUMERIC_DATE_TYPES(BINARY_RELATIONAL, 
greater_than_or_equal_to, >=)
 
 #undef BINARY_RELATIONAL
 
+// Returns the greatest or least value from a list of values
+#define COMPARE_TWO_VALUES(NAME, TYPE, OP)                            \
+  FORCE_INLINE                                                        \
+  gdv_##TYPE NAME##_##TYPE##_##TYPE(gdv_##TYPE in1, gdv_##TYPE in2) { \
+    return (in1 OP in2 ? in1 : in2);                                  \
+  }
+
+#define COMPARE_THREE_VALUES(NAME, TYPE, OP)                                 \
+  FORCE_INLINE                                                               \
+  gdv_##TYPE NAME##_##TYPE##_##TYPE##_##TYPE(gdv_##TYPE in1, gdv_##TYPE in2, \
+                                             gdv_##TYPE in3) {               \
+    gdv_##TYPE compared = (in1 OP in2 ? in1 : in2);                          \
+    return (compared OP in3 ? compared : in3);                               \
+  }
+
+#define COMPARE_FOUR_VALUES(NAME, TYPE, OP)                                    
         \
+  FORCE_INLINE                                                                 
         \
+  gdv_##TYPE NAME##_##TYPE##_##TYPE##_##TYPE##_##TYPE(gdv_##TYPE in1, 
gdv_##TYPE in2,   \
+                                                      gdv_##TYPE in3, 
gdv_##TYPE in4) { \
+    gdv_##TYPE compared = (in1 OP in2 ? in1 : in2);                            
         \
+    compared = (compared OP in3 ? compared : in3);                             
         \
+    return (compared OP in4 ? compared : in4);                                 
         \
+  }
+
+#define COMPARE_FIVE_VALUES(NAME, TYPE, OP)                                    
         \
+  FORCE_INLINE                                                                 
         \
+  gdv_##TYPE NAME##_##TYPE##_##TYPE##_##TYPE##_##TYPE##_##TYPE(                
         \
+      gdv_##TYPE in1, gdv_##TYPE in2, gdv_##TYPE in3, gdv_##TYPE in4, 
gdv_##TYPE in5) { \
+    gdv_##TYPE compared = (in1 OP in2 ? in1 : in2);                            
         \
+    compared = (compared OP in3 ? compared : in3);                             
         \
+    compared = (compared OP in4 ? compared : in4);                             
         \
+    return (compared OP in5 ? compared : in5);                                 
         \
+  }
+
+#define COMPARE_SIX_VALUES(NAME, TYPE, OP)                                     
       \
+  FORCE_INLINE                                                                 
       \
+  gdv_##TYPE NAME##_##TYPE##_##TYPE##_##TYPE##_##TYPE##_##TYPE##_##TYPE(       
       \
+      gdv_##TYPE in1, gdv_##TYPE in2, gdv_##TYPE in3, gdv_##TYPE in4, 
gdv_##TYPE in5, \
+      gdv_##TYPE in6) {                                                        
       \
+    gdv_##TYPE compared = (in1 OP in2 ? in1 : in2);                            
       \

Review comment:
       won't the number of assignments reduce if a for loop is used where a 
reassignment happens only if it is lesser (than greater). Isn't this slightly 
less performing to use so many tertiary operators?

##########
File path: cpp/src/gandiva/precompiled/arithmetic_ops_test.cc
##########
@@ -101,6 +101,121 @@ TEST(TestArithmeticOps, TestDiv) {
   context.Reset();
 }
 
+TEST(TestArithmeticOps, TestGreatestLeast) {
+  // Comparable functions - Greatest and Least
+  EXPECT_EQ(greatest_int32_int32(1, 2), 2);
+  EXPECT_EQ(greatest_int32_int32(2, 1), 2);
+  EXPECT_EQ(greatest_int64_int64(1, 2), 2);
+  EXPECT_EQ(greatest_int64_int64(2, 1), 2);
+  EXPECT_EQ(greatest_float32_float32(1.0f, 2.0f), 2.0f);
+  EXPECT_EQ(greatest_float32_float32(2.0f, 1.0f), 2.0f);
+  EXPECT_EQ(greatest_float64_float64(1.0, 2.0), 2.0);
+  EXPECT_EQ(greatest_float64_float64(2.0, 1.0), 2.0);
+  EXPECT_EQ(least_int32_int32(1, 2), 1);
+  EXPECT_EQ(least_int32_int32(2, 1), 1);
+  EXPECT_EQ(least_int64_int64(1, 2), 1);
+  EXPECT_EQ(least_int64_int64(2, 1), 1);
+  EXPECT_EQ(least_float32_float32(1.0f, 2.0f), 1.0f);
+  EXPECT_EQ(least_float32_float32(2.0f, 1.0f), 1.0f);
+  EXPECT_EQ(least_float64_float64(1.0, 2.0), 1.0);
+  EXPECT_EQ(least_float64_float64(2.0, 1.0), 1.0);
+
+  EXPECT_EQ(greatest_int32_int32_int32(1, 2, 3), 3);
+  EXPECT_EQ(greatest_int32_int32_int32(3, 2, 1), 3);
+  EXPECT_EQ(greatest_int64_int64_int64(1, 2, 3), 3);
+  EXPECT_EQ(greatest_int64_int64_int64(3, 2, 1), 3);
+  EXPECT_EQ(greatest_float32_float32_float32(1.0f, 2.0f, 3.0f), 3.0f);
+  EXPECT_EQ(greatest_float32_float32_float32(3.0f, 2.0f, 1.0f), 3.0f);
+  EXPECT_EQ(greatest_float64_float64_float64(1.0, 2.0, 3.0), 3.0);
+  EXPECT_EQ(greatest_float64_float64_float64(3.0, 2.0, 1.0), 3.0);
+  EXPECT_EQ(least_int32_int32_int32(1, 2, 3), 1);
+  EXPECT_EQ(least_int32_int32_int32(2, 1, 3), 1);
+  EXPECT_EQ(least_int64_int64_int64(1, 2, 3), 1);
+  EXPECT_EQ(least_int64_int64_int64(2, 1, 3), 1);
+  EXPECT_EQ(least_float32_float32_float32(1.0f, 2.0f, 3.0f), 1.0f);
+  EXPECT_EQ(least_float32_float32_float32(3.0f, 2.0f, 1.0f), 1.0f);
+  EXPECT_EQ(least_float64_float64_float64(1.0, 2.0, 2.0), 1.0);
+  EXPECT_EQ(least_float64_float64_float64(3.0, 2.0, 1.0), 1.0);
+
+  EXPECT_EQ(greatest_int32_int32_int32_int32(1, 2, 3, 4), 4);
+  EXPECT_EQ(greatest_int32_int32_int32_int32(2, 4, 3, 1), 4);
+  EXPECT_EQ(greatest_int64_int64_int64_int64(1, 2, 3, 4), 4);
+  EXPECT_EQ(greatest_int64_int64_int64_int64(2, 4, 3, 1), 4);
+  EXPECT_EQ(greatest_float32_float32_float32_float32(1.0f, 2.0f, 3.0f, 4.0f), 
4.0f);
+  EXPECT_EQ(greatest_float32_float32_float32_float32(2.0f, 4.0f, 3, 1.0f), 
4.0f);
+  EXPECT_EQ(greatest_float64_float64_float64_float64(1.0, 2.0, 3.0, 4.0), 4.0);
+  EXPECT_EQ(greatest_float64_float64_float64_float64(2.0, 4.0, 3.0, 1.0), 4.0);
+  EXPECT_EQ(least_int32_int32_int32_int32(1, 2, 3, 4), 1);
+  EXPECT_EQ(least_int32_int32_int32_int32(2, 4, 3, 1), 1);
+  EXPECT_EQ(least_int64_int64_int64_int64(1, 2, 3, 4), 1);
+  EXPECT_EQ(least_int64_int64_int64_int64(2, 4, 3, 1), 1);
+  EXPECT_EQ(least_float32_float32_float32_float32(1.0f, 2.0f, 3.0f, 4.0f), 
1.0f);
+  EXPECT_EQ(least_float32_float32_float32_float32(2.0f, 4.0f, 3, 1.0f), 1.0f);
+  EXPECT_EQ(least_float64_float64_float64_float64(1.0, 2.0, 3.0, 4.0), 1.0);
+  EXPECT_EQ(least_float64_float64_float64_float64(2.0, 4.0, 3.0, 1.0), 1.0);
+
+  EXPECT_EQ(greatest_int32_int32_int32_int32_int32(1, 2, 3, 5, 4), 5);
+  EXPECT_EQ(greatest_int32_int32_int32_int32_int32(2, 4, 5, 3, 1), 5);
+  EXPECT_EQ(greatest_int64_int64_int64_int64_int64(1, 2, 3, 5, 4), 5);
+  EXPECT_EQ(greatest_int64_int64_int64_int64_int64(2, 4, 5, 3, 1), 5);
+  EXPECT_EQ(
+      greatest_float32_float32_float32_float32_float32(1.0f, 2.0f, 3.0f, 5.0f, 
4.0f),
+      5.0f);
+  EXPECT_EQ(
+      greatest_float32_float32_float32_float32_float32(2.0f, 4.0f, 5.0f, 3.0f, 
1.0f),
+      5.0f);
+  EXPECT_EQ(greatest_float64_float64_float64_float64_float64(1.0, 2.0, 3.0, 
5.0, 4.0),
+            5.0);
+  EXPECT_EQ(greatest_float64_float64_float64_float64_float64(2.0, 4.0, 5.0, 
3.0, 1.0),
+            5.0);
+  EXPECT_EQ(least_int32_int32_int32_int32_int32(1, 2, 3, 4, -10), -10);

Review comment:
       maybe a test with INT_MAX and other min/max limits. what about limits 
for floats? Also, do we have to deal with infinity(), NaN etc for floats? Just 
a question, as I am not sure..




-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to