Skip to content

Commit

Permalink
Delete copy and move operations of some Matrix classes (#2103)
Browse files Browse the repository at this point in the history
Summary:
Pull Request resolved: #2103

For those matrices managing their buffers, the copy and move operations should be deleted to avoid default versions generated by the compiler. Another solution is to manage the buffer ownership in user defined versions, but deleting them is simpler given that no memory corruption has been found since it can be inferred that they are not used.

Another solution is use std::share_ptr to manager buffers. However, it will incur some atomic operations on the reference counter, which is not desirable .

Pull Request resolved: #2100

Reviewed By: spcyppt

Differential Revision: D50822752

Pulled By: q10

fbshipit-source-id: 87c0a495c146c23e8f5a284ac87357990578d8d8
  • Loading branch information
cyyever authored and facebook-github-bot committed Oct 31, 2023
1 parent 4651326 commit 488fa2e
Showing 1 changed file with 17 additions and 6 deletions.
23 changes: 17 additions & 6 deletions include/fbgemm/Fbgemm.h
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,10 @@ template <typename PT, typename inpType, typename accType = std::int32_t>
class PackMatrix {
public:
PackMatrix() = delete; // no default constructor
PackMatrix(const PackMatrix&) = delete; // no copy
PackMatrix& operator==(const PackMatrix&) = delete; // no copy
PackMatrix(PackMatrix&&) = delete; // no move
PackMatrix& operator==(PackMatrix&& rhs) noexcept = delete; // no move

/**
* @param rows total number of rows in the matrix
Expand Down Expand Up @@ -296,7 +300,7 @@ class PackMatrix {
std::int32_t bcol_; ///< the number of columns in each block
std::int32_t nbrow_; ///< the number of blocks along rows
std::int32_t nbcol_; ///< the number of blocks along columns
bool bufAllocatedHere_;
bool bufAllocatedHere_{false};
const BlockingFactors*
blocking_params; ///< MCB, KCB, NCB, MR, NR, NR_MIN, ROW_INTERLEAVE;

Expand Down Expand Up @@ -505,6 +509,13 @@ class FBGEMM_API PackWeightMatrixForGConv {
using accType = accT;

PackWeightMatrixForGConv() = delete; // no default constructor
PackWeightMatrixForGConv(const PackWeightMatrixForGConv&) = delete; // no copy
PackWeightMatrixForGConv& operator==(const PackWeightMatrixForGConv&) =
delete; // no copy

PackWeightMatrixForGConv(PackWeightMatrixForGConv&&) = delete; // no move
PackWeightMatrixForGConv& operator==(PackWeightMatrixForGConv&&) =
delete; // no move

/**
* @param pmat if nullptr, a buffer is allocated and owned by this class.
Expand Down Expand Up @@ -550,7 +561,7 @@ class FBGEMM_API PackWeightMatrixForGConv {
const conv_param_t<SPATIAL_DIM> conv_param_;
const T* sdata_;
T* pdata_;
bool bufAllocatedHere_;
bool bufAllocatedHere_{false};
// Number of groups we work at a time to fill the full simd width
int GTogether_;

Expand Down Expand Up @@ -836,8 +847,8 @@ class FBGEMM_API PackAWithRowOffset final
matrix_op_t trans_;
const T* smat_;
std::uint32_t ld_;
std::int32_t* row_offset_;
bool rowOffsetAllocatedHere;
std::int32_t* row_offset_{nullptr};
bool rowOffsetAllocatedHere{false};
std::int32_t row_interleave_B_;
};

Expand Down Expand Up @@ -930,8 +941,8 @@ class FBGEMM_API PackAWithQuantRowOffset final
std::int32_t ld_;
float scale_;
std::int32_t zero_pt_;
std::int32_t* row_offset_;
bool rowOffsetAllocatedHere;
std::int32_t* row_offset_{nullptr};
bool rowOffsetAllocatedHere{false};
std::int32_t row_interleave_B_;
};

Expand Down

0 comments on commit 488fa2e

Please sign in to comment.