Avoid postfix iterator increment in a loop in Slice op and some minor formatting fixes (#820)

* Initial commit

* Fix comment

* More nit fixes
This commit is contained in:
Hariharan Seshadri 2019-04-11 17:44:32 -07:00 committed by GitHub
parent ccf3566c35
commit b6936e71cb
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
5 changed files with 71 additions and 57 deletions

View file

@ -272,7 +272,7 @@ void RegisterOnnxOperatorKernels(KernelRegistry& kernel_registry) {
kernel_registry.Register(BuildKernelCreateInfo<ONNX_OPERATOR_KERNEL_CLASS_NAME(kCpuExecutionProvider, kOnnxDomain, 6, Sigmoid)>());
kernel_registry.Register(BuildKernelCreateInfo<ONNX_OPERATOR_KERNEL_CLASS_NAME(kCpuExecutionProvider, kOnnxDomain, 1, Softplus)>());
kernel_registry.Register(BuildKernelCreateInfo<ONNX_OPERATOR_KERNEL_CLASS_NAME(kCpuExecutionProvider, kOnnxDomain, 1, Softsign)>());
kernel_registry.Register(BuildKernelCreateInfo<ONNX_OPERATOR_KERNEL_CLASS_NAME(kCpuExecutionProvider, kOnnxDomain, 6, Tanh)>());
kernel_registry.Register(BuildKernelCreateInfo<ONNX_OPERATOR_KERNEL_CLASS_NAME(kCpuExecutionProvider, kOnnxDomain, 6, Tanh)>());
kernel_registry.Register(BuildKernelCreateInfo<ONNX_OPERATOR_VERSIONED_KERNEL_CLASS_NAME(kCpuExecutionProvider, kOnnxDomain, 7, 9, PRelu)>());
kernel_registry.Register(BuildKernelCreateInfo<ONNX_OPERATOR_KERNEL_CLASS_NAME(kCpuExecutionProvider, kOnnxDomain, 1, RandomNormal)>());
kernel_registry.Register(BuildKernelCreateInfo<ONNX_OPERATOR_KERNEL_CLASS_NAME(kCpuExecutionProvider, kOnnxDomain, 1, RandomUniform)>());

View file

@ -48,26 +48,23 @@ static void PadAxisConstant(T* output, T constant, size_t size) {
}
// Flatten no padding inner most Axis, so one memcpy cover multiple Axis.
// For example, for a shape of [1,224,224,3] with padding [0,3,3,0,0,3,3,0], can be flatten as
// For example, for a shape of [1,224,224,3] with padding [0,3,3,0,0,3,3,0], can be flatten as
// [1,224,224*3] with padding [0,3,3*3,0,3,3*3].
static void FlattenInnerShape(const std::vector<int64_t>& input_dims, const std::vector<int64_t>& pads,
const std::vector<int64_t>& slices, std::vector<int64_t>& reshaped_dims)
{
static void FlattenInnerShape(const std::vector<int64_t>& input_dims, const std::vector<int64_t>& pads,
const std::vector<int64_t>& slices, std::vector<int64_t>& reshaped_dims) {
size_t dims_count = input_dims.size();
size_t inner_axis = dims_count - 1;
size_t inner_size = 1;
// Find all inner most dimensions that can be flattened.
do
{
do {
inner_size *= input_dims[inner_axis];
if (inner_axis == 0)
break;
// Break on first Axis that has padding
if (!(pads[inner_axis] == 0 && pads[inner_axis + dims_count] == 0
&& slices[inner_axis] == 0 && slices[inner_axis + dims_count] == 0))
if (!(pads[inner_axis] == 0 && pads[inner_axis + dims_count] == 0 && slices[inner_axis] == 0 && slices[inner_axis + dims_count] == 0))
break;
} while (inner_axis-- > 0);
@ -79,13 +76,12 @@ static void FlattenInnerShape(const std::vector<int64_t>& input_dims, const std:
reshaped_dims[inner_axis] = inner_size;
}
static void ReshapePads(const std::vector<int64_t>& src_pad, size_t src_dim_count, size_t new_dim_count,
static void ReshapePads(const std::vector<int64_t>& src_pad, size_t src_dim_count, size_t new_dim_count,
size_t inner_no_pad_size, std::vector<int64_t>& reshaped_pad) {
size_t inner_axis = new_dim_count - 1;
std::copy(src_pad.begin(), src_pad.begin() + inner_axis, reshaped_pad.begin());
std::copy(src_pad.begin() + src_dim_count, src_pad.begin() + src_dim_count + inner_axis, reshaped_pad.begin() + new_dim_count);
// Flatten inner axis.
reshaped_pad[inner_axis] = src_pad[inner_axis] * inner_no_pad_size;
reshaped_pad[inner_axis + new_dim_count] = src_pad[inner_axis + src_dim_count] * inner_no_pad_size;

View file

@ -34,14 +34,14 @@ ADD_TYPED_SLICE_V9_OP(string);
#ifndef DISABLE_CONTRIB_OPS
namespace contrib {
#define ADD_TYPED_DYNAMIC_SLICE_OP(data_type) \
ONNX_CPU_OPERATOR_TYPED_KERNEL( \
DynamicSlice, \
1, \
data_type, \
KernelDefBuilder().TypeConstraint("T", DataTypeImpl::GetTensorType<data_type>()) \
.TypeConstraint("Tind", {DataTypeImpl::GetTensorType<int32_t>(), \
DataTypeImpl::GetTensorType<int64_t>()}), \
#define ADD_TYPED_DYNAMIC_SLICE_OP(data_type) \
ONNX_CPU_OPERATOR_TYPED_KERNEL( \
DynamicSlice, \
1, \
data_type, \
KernelDefBuilder().TypeConstraint("T", DataTypeImpl::GetTensorType<data_type>()) \
.TypeConstraint("Tind", {DataTypeImpl::GetTensorType<int32_t>(), \
DataTypeImpl::GetTensorType<int64_t>()}), \
Slice<data_type, true>);
ADD_TYPED_DYNAMIC_SLICE_OP(uint8_t);
@ -61,14 +61,14 @@ ADD_TYPED_DYNAMIC_SLICE_OP(string);
} // namespace contrib
#endif
#define ADD_TYPED_SLICE_V10_OP(data_type) \
ONNX_CPU_OPERATOR_TYPED_KERNEL( \
Slice, \
10, \
data_type, \
KernelDefBuilder().TypeConstraint("T", DataTypeImpl::GetTensorType<data_type>()) \
.TypeConstraint("Tind", {DataTypeImpl::GetTensorType<int32_t>(), \
DataTypeImpl::GetTensorType<int64_t>()}), \
#define ADD_TYPED_SLICE_V10_OP(data_type) \
ONNX_CPU_OPERATOR_TYPED_KERNEL( \
Slice, \
10, \
data_type, \
KernelDefBuilder().TypeConstraint("T", DataTypeImpl::GetTensorType<data_type>()) \
.TypeConstraint("Tind", {DataTypeImpl::GetTensorType<int32_t>(), \
DataTypeImpl::GetTensorType<int64_t>()}), \
Slice<data_type, true>);
ADD_TYPED_SLICE_V10_OP(uint8_t);
@ -171,7 +171,7 @@ Status SliceBase::PrepareForCompute(const std::vector<int64_t>& raw_starts,
return Status(ONNXRUNTIME, INVALID_ARGUMENT, "'axes' has duplicates");
unique_axes.insert(axis);
// process step
// process step
auto step = axis_index < raw_steps.size() ? raw_steps[axis_index] : 1;
if (step == 0)
return Status(ONNXRUNTIME, INVALID_ARGUMENT, "'step' value cannot be 0");
@ -191,7 +191,7 @@ Status SliceBase::PrepareForCompute(const std::vector<int64_t>& raw_starts,
// INT_MAX has a special meaning for end according to spec
// equivalent to 'None' in numpy
// it represent slicing to the end of the dimension
if (end == std::numeric_limits<int32_t>::max() ||
if (end == std::numeric_limits<int32_t>::max() ||
end == std::numeric_limits<int64_t>::max()) {
end = step < 0 ? -1 : input_dimensions[axis];
}
@ -202,8 +202,8 @@ Status SliceBase::PrepareForCompute(const std::vector<int64_t>& raw_starts,
if (step < 0)
end = clamp(end, int64_t{-1}, input_dimensions[axis]);
else
end = clamp(end, int64_t{0}, input_dimensions[axis]);
}
end = clamp(end, int64_t{0}, input_dimensions[axis]);
}
// find output dim value for this axis
auto temp = static_cast<int64_t>(ceil(1.0 * (end - starts[axis]) / step));
@ -290,8 +290,11 @@ Status SliceImpl(OpKernelContext* ctx,
const auto* output_end = output + output_tensor.Shape().Size();
SliceIterator<T> input_iterator(input_tensor, starts, output_dims, steps);
while (output != output_end)
*output++ = *input_iterator++;
while (output != output_end) {
*output = *input_iterator;
++output;
++input_iterator;
}
return Status::OK();
}

View file

@ -39,7 +39,7 @@ class SliceBase {
std::vector<int64_t>& steps,
std::vector<int64_t>& output_dims) const;
// Slice V10 & DynamicSlice
// Slice V10 & DynamicSlice
void FillVectorsFromInput(const OpKernelContext* context,
std::vector<int64_t>& input_starts,
std::vector<int64_t>& input_ends,

View file

@ -120,15 +120,15 @@ struct ExtentAxisCounters {
};
// A std::vector that holds the number of entries to skip to go to the next axis start given an extent
// and optionally steps along each axis:
// and optionally steps along each axis:
// This is used by the SliceIterator to iterate over a slice of a tensor
struct SliceSkips : std::vector<int64_t> {
SliceSkips(const TensorShape& input_shape, gsl::span<const int64_t> extents, gsl::span<const int64_t> steps)
: std::vector<int64_t>(input_shape.NumDimensions(), 0) {
: std::vector<int64_t>(input_shape.NumDimensions(), 0) {
auto& dims = input_shape.GetDims();
ORT_ENFORCE(static_cast<ptrdiff_t>(dims.size()) == extents.size() &&
static_cast<ptrdiff_t>(dims.size()) >= steps.size());
int64_t inner_most_dim = dims.size() - 1;
// assume step == 1 if not present
ptrdiff_t steps_size = steps.size();
@ -140,48 +140,47 @@ struct SliceSkips : std::vector<int64_t> {
for (size_t i = size(); i-- > 0;) {
auto prevPitch = pitch;
pitch *= dims[i];
// assume step == 1 if not present
int64_t steps_i_minus_1 = 1;
if (i > 0 && static_cast<ptrdiff_t>(i) - 1 < steps_size)
steps_i_minus_1 = steps[i - 1];
steps_i_minus_1 = steps[i - 1];
// first "revert" back to the old starting position (term with -ve sign)
// and then "step" over the pitch accordingly (term with +ve sign)
operator[](i) = steps_i_minus_1 * pitch - steps_i * extents[i] * prevPitch;
steps_i = steps_i_minus_1;
}
}
};
// This provides easy sequential iteration over a subset of a tensor given a span of starts, extents & optionally steps
// This provides easy sequential iteration over a subset of a tensor given a span of starts, extents & optionally steps
template <typename T>
struct SliceIterator {
SliceIterator(const Tensor& tensor, gsl::span<const int64_t> starts,
SliceIterator(const Tensor& tensor, gsl::span<const int64_t> starts,
gsl::span<const int64_t> extents, gsl::span<const int64_t> steps)
: tensor_(tensor), extents_(extents), skips_(tensor_.Shape(), extents, steps), indices_(extents.size(), 0) {
: tensor_(tensor), extents_(extents), skips_(tensor_.Shape(), extents, steps), indices_(extents.size(), 0) {
auto& dims = tensor_.Shape().GetDims();
Init(dims, starts, steps);
}
// This construct takes a explicit tensor_shape which might be different from the shape defined in input tensor.
// The explicit tensor_shape usually has inner most axis flattened. For example, given shape[1,4,4,2], if last axis
// does not have padding or slice, then it will be flattened as [1,4,8] for better performance (One inner most copy instead of 4).
// Also supports arbitrary positive and negative stepping along individual axes
SliceIterator(const Tensor& tensor, const TensorShape& tensor_shape, gsl::span<const int64_t> starts,
SliceIterator(const Tensor& tensor, const TensorShape& tensor_shape, gsl::span<const int64_t> starts,
gsl::span<const int64_t> extents, gsl::span<const int64_t> steps)
: tensor_(tensor), extents_(extents), skips_(tensor_shape, extents, steps), indices_(extents.size(), 0) {
: tensor_(tensor), extents_(extents), skips_(tensor_shape, extents, steps), indices_(extents.size(), 0) {
auto& dims = tensor_shape.GetDims();
Init(dims, starts, steps);
}
// Initialize initial skip and inner_extent.
void Init(const std::vector<int64_t>& dims, gsl::span<const int64_t> starts,
void Init(const std::vector<int64_t>& dims, gsl::span<const int64_t> starts,
gsl::span<const int64_t> steps) {
ORT_ENFORCE(static_cast<ptrdiff_t>(dims.size()) == starts.size() &&
static_cast<ptrdiff_t>(dims.size()) == extents_.size() &&
ORT_ENFORCE(static_cast<ptrdiff_t>(dims.size()) == starts.size() &&
static_cast<ptrdiff_t>(dims.size()) == extents_.size() &&
static_cast<ptrdiff_t>(dims.size()) >= steps.size());
size_t pitch = 1;
@ -192,8 +191,9 @@ struct SliceIterator {
}
inner_extent_ = extents_[dims.size() - 1];
inner_step_ = static_cast<ptrdiff_t>(dims.size()) == steps.size()
? steps[dims.size() - 1] : 1;
inner_step_ = static_cast<ptrdiff_t>(dims.size()) == steps.size()
? steps[dims.size() - 1]
: 1;
}
void AdvanceOverInnerExtent() {
@ -205,18 +205,33 @@ struct SliceIterator {
}
}
const T* operator++(int) {
const T* input = input_;
void IncrementInnerDimension() {
input_ += inner_step_;
if (++inner_counter_ == inner_extent_) {
inner_counter_ = 0;
AdvanceOverInnerExtent();
}
}
// postfix iterator increment
const T* operator++(int) {
const T* input = input_;
IncrementInnerDimension();
return input;
}
// spliting the function that copies the innermost dimension into 2 separate methods,
// as this is most likely being called within a loop
// prefix iterator increment
const T* operator++() {
IncrementInnerDimension();
return input_;
}
const T& operator*() const {
return *input_;
}
// spliting the function that copies the innermost dimension into 2 separate methods,
// as this is most likely being called within a loop
// and we want to avoid the check inside to avoid overhead
// upto the caller to call the relevant one