[GitHub] [incubator-mxnet] Zha0q1 commented on a change in pull request #18816: [WIP] Add Large Dim Checks for linalg Operators

[GitBox <gi...@apache.org>]

Zha0q1 commented on a change in pull request #18816:
URL: https://github.com/apache/incubator-mxnet/pull/18816#discussion_r462028540



##########
File path: src/operator/tensor/la_op.h
##########
@@ -181,6 +189,21 @@ inline bool LaMatrixMultMacOpShape(const nnvm::NodeAttrs& attrs,
     const int ndim((*in_attrs)[0].ndim()), axis(axis_param < 0 ? ndim + axis_param : axis_param);
     CHECK(axis >= 0 && axis < ndim-1)
       << "Invalid row axis (" << axis_param << ")";
+    // check if any dim is too large
+    check_large_dim({(*in_attrs)[0][axis],
+		     (*in_attrs)[0][ndim-1],
+		     (*in_attrs)[1][axis],
+		     (*in_attrs)[1][ndim-1]});
+    /*
+    CHECK_LE((*in_attrs)[0][axis], INT_MAX)
+      << "Large matrix dimensions (>= 2^31) are not supported";
+    CHECK_LE((*in_attrs)[0][ndim-1], INT_MAX)
+      << "Large matrix dimensions (>= 2^31) are not supported";;
+    CHECK_LE((*in_attrs)[1][axis], INT_MAX)
+      << "Large matrix dimensions (>= 2^31) are not supported";;
+    CHECK_LE((*in_attrs)[1][ndim-1], INT_MAX)
+      << "Large matrix dimensions (>= 2^31) are not supported";;

Review comment:
       This is only for the sake of discussion. Hope you would agree that the above style (using a helper function) is better.




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

For queries about this service, please contact Infrastructure at:
users@infra.apache.org