gemini-code-assist[bot] commented on code in PR #81:
URL: https://github.com/apache/tvm-ffi/pull/81#discussion_r2395676230
##########
include/tvm/ffi/container/tensor.h:
##########
@@ -458,6 +458,160 @@ class Tensor : public ObjectRef {
TensorObj* get_mutable() const { return const_cast<TensorObj*>(get()); }
};
+/*!
+ * \brief A non-owning view of a Tensor.
+ *
+ * This class stores a light-weight non-owning view of a Tensor.
+ * This is useful for accessing tensor data without retaining a strong
reference to the Tensor.
+ * Since the caller may not always be able to pass in a strong referenced
tensor.
+ *
+ * It is the user's responsibility to ensure
+ * that the underlying tensor data outlives the `TensorView`.
+ *
+ * When exposing a function that expects only expects a TensorView, we
recommend using
+ * ffi::TensorView as the argument type instead of ffi::Tensor.
+ */
+class TensorView {
+ public:
+ /*!
+ * \brief Create a TensorView from a Tensor.
+ * \param tensor The input Tensor.
+ * \return The created TensorView.
+ */
+ TensorView(const Tensor& tensor) : tensor_(*tensor.operator->()) {} //
NOLINT(*)
Review Comment:
The constructor from `const Tensor&` does not check if the input tensor is
defined (not null). If a null `Tensor` is passed, `tensor.operator->()` will
dereference a null pointer, causing a crash. Please add a check for
`tensor.defined()`.
```c
TensorView(const Tensor& tensor) { // NOLINT(*)
TVM_FFI_ICHECK(tensor.defined());
tensor_ = *tensor.operator->();
}
```
##########
include/tvm/ffi/container/tensor.h:
##########
@@ -458,6 +458,160 @@ class Tensor : public ObjectRef {
TensorObj* get_mutable() const { return const_cast<TensorObj*>(get()); }
};
+/*!
+ * \brief A non-owning view of a Tensor.
+ *
+ * This class stores a light-weight non-owning view of a Tensor.
+ * This is useful for accessing tensor data without retaining a strong
reference to the Tensor.
+ * Since the caller may not always be able to pass in a strong referenced
tensor.
+ *
+ * It is the user's responsibility to ensure
+ * that the underlying tensor data outlives the `TensorView`.
+ *
+ * When exposing a function that expects only expects a TensorView, we
recommend using
+ * ffi::TensorView as the argument type instead of ffi::Tensor.
+ */
+class TensorView {
+ public:
+ /*!
+ * \brief Create a TensorView from a Tensor.
+ * \param tensor The input Tensor.
+ * \return The created TensorView.
+ */
+ TensorView(const Tensor& tensor) : tensor_(*tensor.operator->()) {} //
NOLINT(*)
+ /*!
+ * \brief Create a TensorView from a DLTensor.
+ * \param tensor The input DLTensor.
+ * \return The created TensorView.
+ */
+ TensorView(const DLTensor* tensor) : tensor_(*tensor) {} // NOLINT(*)
+ /*!
+ * \brief Copy constructor.
+ * \param tensor The input TensorView.
+ * \return The created TensorView.
+ */
+ TensorView(const TensorView& tensor) = default;
+ /*!
+ * \brief Move constructor.
+ * \param tensor The input TensorView.
+ * \return The created TensorView.
+ */
+ TensorView(TensorView&& tensor) = default;
+ /*!
+ * \brief Copy assignment operator.
+ * \param tensor The input TensorView.
+ * \return The created TensorView.
+ */
+ TensorView& operator=(const TensorView& tensor) = default;
+ /*!
+ * \brief Move assignment operator.
+ * \param tensor The input TensorView.
+ * \return The created TensorView.
+ */
+ TensorView& operator=(TensorView&& tensor) = default;
+ /*!
+ * \brief Assignment operator from a Tensor.
+ * \param tensor The input Tensor.
+ * \return The created TensorView.
+ */
+ TensorView& operator=(const Tensor& tensor) {
+ tensor_ = *tensor.operator->();
+ return *this;
+ }
Review Comment:
The assignment operator from `const Tensor&` does not check if the input
tensor is defined (not null). If a null `Tensor` is passed,
`tensor.operator->()` will dereference a null pointer, causing a crash. Please
add a check for `tensor.defined()`.
```c
TensorView& operator=(const Tensor& tensor) {
TVM_FFI_ICHECK(tensor.defined());
tensor_ = *tensor.operator->();
return *this;
}
```
##########
include/tvm/ffi/container/tensor.h:
##########
@@ -458,6 +458,160 @@ class Tensor : public ObjectRef {
TensorObj* get_mutable() const { return const_cast<TensorObj*>(get()); }
};
+/*!
+ * \brief A non-owning view of a Tensor.
+ *
+ * This class stores a light-weight non-owning view of a Tensor.
+ * This is useful for accessing tensor data without retaining a strong
reference to the Tensor.
+ * Since the caller may not always be able to pass in a strong referenced
tensor.
+ *
+ * It is the user's responsibility to ensure
+ * that the underlying tensor data outlives the `TensorView`.
+ *
+ * When exposing a function that expects only expects a TensorView, we
recommend using
+ * ffi::TensorView as the argument type instead of ffi::Tensor.
+ */
+class TensorView {
+ public:
+ /*!
+ * \brief Create a TensorView from a Tensor.
+ * \param tensor The input Tensor.
+ * \return The created TensorView.
+ */
+ TensorView(const Tensor& tensor) : tensor_(*tensor.operator->()) {} //
NOLINT(*)
+ /*!
+ * \brief Create a TensorView from a DLTensor.
+ * \param tensor The input DLTensor.
+ * \return The created TensorView.
+ */
+ TensorView(const DLTensor* tensor) : tensor_(*tensor) {} // NOLINT(*)
Review Comment:
The constructor from `const DLTensor*` does not check if the input pointer
is null. Dereferencing a null pointer will lead to a crash. Please add a null
check.
```c
TensorView(const DLTensor* tensor) { // NOLINT(*)
TVM_FFI_ICHECK_NOTNULL(tensor);
tensor_ = *tensor;
}
```
##########
include/tvm/ffi/container/tensor.h:
##########
@@ -458,6 +458,160 @@ class Tensor : public ObjectRef {
TensorObj* get_mutable() const { return const_cast<TensorObj*>(get()); }
};
+/*!
+ * \brief A non-owning view of a Tensor.
+ *
+ * This class stores a light-weight non-owning view of a Tensor.
+ * This is useful for accessing tensor data without retaining a strong
reference to the Tensor.
+ * Since the caller may not always be able to pass in a strong referenced
tensor.
+ *
+ * It is the user's responsibility to ensure
+ * that the underlying tensor data outlives the `TensorView`.
+ *
+ * When exposing a function that expects only expects a TensorView, we
recommend using
+ * ffi::TensorView as the argument type instead of ffi::Tensor.
+ */
+class TensorView {
+ public:
+ /*!
+ * \brief Create a TensorView from a Tensor.
+ * \param tensor The input Tensor.
+ * \return The created TensorView.
Review Comment:
Constructors do not have return values. Please remove the `\return` Doxygen
tag from this comment. This also applies to the other constructors and
assignment operators in `TensorView` (lines 485, 491, 497, 503, 509, 515).
##########
include/tvm/ffi/container/tensor.h:
##########
@@ -458,6 +458,160 @@ class Tensor : public ObjectRef {
TensorObj* get_mutable() const { return const_cast<TensorObj*>(get()); }
};
+/*!
+ * \brief A non-owning view of a Tensor.
+ *
+ * This class stores a light-weight non-owning view of a Tensor.
+ * This is useful for accessing tensor data without retaining a strong
reference to the Tensor.
+ * Since the caller may not always be able to pass in a strong referenced
tensor.
+ *
+ * It is the user's responsibility to ensure
+ * that the underlying tensor data outlives the `TensorView`.
+ *
+ * When exposing a function that expects only expects a TensorView, we
recommend using
+ * ffi::TensorView as the argument type instead of ffi::Tensor.
+ */
+class TensorView {
+ public:
+ /*!
+ * \brief Create a TensorView from a Tensor.
+ * \param tensor The input Tensor.
+ * \return The created TensorView.
+ */
+ TensorView(const Tensor& tensor) : tensor_(*tensor.operator->()) {} //
NOLINT(*)
+ /*!
+ * \brief Create a TensorView from a DLTensor.
+ * \param tensor The input DLTensor.
+ * \return The created TensorView.
+ */
+ TensorView(const DLTensor* tensor) : tensor_(*tensor) {} // NOLINT(*)
+ /*!
+ * \brief Copy constructor.
+ * \param tensor The input TensorView.
+ * \return The created TensorView.
+ */
+ TensorView(const TensorView& tensor) = default;
+ /*!
+ * \brief Move constructor.
+ * \param tensor The input TensorView.
+ * \return The created TensorView.
+ */
+ TensorView(TensorView&& tensor) = default;
+ /*!
+ * \brief Copy assignment operator.
+ * \param tensor The input TensorView.
+ * \return The created TensorView.
+ */
+ TensorView& operator=(const TensorView& tensor) = default;
+ /*!
+ * \brief Move assignment operator.
+ * \param tensor The input TensorView.
+ * \return The created TensorView.
+ */
+ TensorView& operator=(TensorView&& tensor) = default;
+ /*!
+ * \brief Assignment operator from a Tensor.
+ * \param tensor The input Tensor.
+ * \return The created TensorView.
+ */
+ TensorView& operator=(const Tensor& tensor) {
+ tensor_ = *tensor.operator->();
+ return *this;
+ }
+
+ // explicitly delete move constructor
+ TensorView(Tensor&& tensor) = delete; // NOLINT(*)
+ // delete move assignment operator from owned tensor
+ TensorView& operator=(Tensor&& tensor) = delete;
+ /*!
+ * \brief Get the underlying DLTensor pointer.
+ * \return The underlying DLTensor pointer.
+ */
+ const DLTensor* operator->() const { return &tensor_; }
+
+ /*!
+ * \brief Get the shape of the Tensor.
+ * \return The shape of the Tensor.
+ */
+ ShapeView shape() const { return ShapeView(tensor_.shape, tensor_.ndim); }
+
+ /*!
+ * \brief Get the strides of the Tensor.
+ * \return The strides of the Tensor.
+ */
+ ShapeView strides() const {
+ TVM_FFI_ICHECK(tensor_.strides != nullptr);
+ return ShapeView(tensor_.strides, tensor_.ndim);
+ }
+
+ /*!
+ * \brief Get the data pointer of the Tensor.
+ * \return The data pointer of the Tensor.
+ */
+ void* data_ptr() const { return tensor_.data; }
+
+ /*!
+ * \brief Get the number of dimensions in the Tensor.
+ * \return The number of dimensions in the Tensor.
+ */
+ int32_t ndim() const { return tensor_.ndim; }
+
+ /*!
+ * \brief Get the number of elements in the Tensor.
+ * \return The number of elements in the Tensor.
+ */
+ int64_t numel() const { return this->shape().Product(); }
+
+ /*!
+ * \brief Get the data type of the Tensor.
+ * \return The data type of the Tensor.
+ */
+ DLDataType dtype() const { return tensor_.dtype; }
+
+ /*!
+ * \brief Check if the Tensor is contiguous.
+ * \return True if the Tensor is contiguous, false otherwise.
+ */
+ bool IsContiguous() const { return tvm::ffi::IsContiguous(tensor_); }
+
+ private:
+ DLTensor tensor_;
+};
+
+// TensorView type, allow implicit casting from DLTensor*
+// NOTE: we deliberately do not support MoveToAny and MoveFromAny since it
does not retain ownership
+template <>
+struct TypeTraits<TensorView> : public TypeTraitsBase {
+ static constexpr bool storage_enabled = false;
Review Comment:
For consistency with the new flags added in `type_traits.h` and to correctly
reflect the capabilities of `TensorView`, please explicitly define
`convert_to_any_enabled` and `convert_to_any_view_enabled` in this `TypeTraits`
specialization. Since `TensorView` is non-owning and `MoveToAny` is not
supported, `convert_to_any_enabled` should be `false`. Since `CopyToAnyView` is
supported, `convert_to_any_view_enabled` should be `true`.
```c
static constexpr bool storage_enabled = false;
static constexpr bool convert_to_any_enabled = false;
static constexpr bool convert_to_any_view_enabled = true;
```
--
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]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
