Issue 18188: VSIPL and VSIPL++ should define version macros Jira Issue VSIPL16-2
Issue 18196: scatter, gather should support multidimensional index views Jira Issue VSIPL16-3
Issue 18197: scatter, gather declarations should use const_View in place of View Jira Issue VSIPL16-4
Issue 18199: cmplx function is misplaced Jira Issue VSIPL16-5
Issue 18201: cscalar_i is required inconsistently Jira Issue VSIPL16-6
Issue 18202: maxmg etc. of complex should return real values Jira Issue VSIPL16-7
Issue 18203: VSIPL++ spec references nonexistent complex<int> types Jira Issue VSIPL16-8
Issue 18204: Conv. 'kernel_size()' incorrect for symmetric kernels Jira Issue VSIPL16-9
Issue 18205: tuple should provide value accessors Jira Issue VSIPL16-10
Issue 18206: Fir<> copy ctor must be allowed to throw std::bad_alloc Jira Issue VSIPL16-11
Issue 18207: Domain<> is used when unneeded Jira Issue VSIPL16-12
Issue 18208: Fir<> has duplicate 'symmetry' accessor, member Jira Issue VSIPL16-13
Issue 18209: 'Explicit alias' to clarify view copy semantics Jira Issue VSIPL16-14
Issue 18210: 'Explicit copy' function to clarify view copy semantics Jira Issue VSIPL16-15
Issue 18211: Convolution with decimation!=1 accesses outside input Jira Issue VSIPL16-16
Issue 18212: Meaning of vsip_symmetry enum values inconsistent Jira Issue VSIPL16-17
Issue 18216: Dangerous implicit conversions Jira Issue VSIPL16-18
Issue 18217: Unclear antecedent in ~Dense() destructor description Jira Issue VSIPL16-19
Issue 18218: Block refcounting interface error prone, wrong Jira Issue VSIPL16-20
Issue 18219: Block decrement_count() is problematic in some cases Jira Issue VSIPL16-21
Issue 18220: View's block reference-counting is inconsistent Jira Issue VSIPL16-22
Issue 18223: ~Dense() destructor should use "use count". Jira Issue VSIPL16-23
Issue 18224: Block reference-counting only described in Dense Jira Issue VSIPL16-24
Issue 18225: Vector destructor should call decrement_count Jira Issue VSIPL16-25
Issue 18259: Note on Deferred memory allocation Jira Issue VSIPL16-26
Issue 18261: Notes to implementors Jira Issue VSIPL16-27
Issue 18381: Distinction between "Restrictions" and "Errors" is unclear Jira Issue VSIPL16-28
Issue 18382: Implementation Dependent Input and Output should become optional Jira Issue VSIPL16-29
Issue 18383: Review entire document for typos Jira Issue VSIPL16-30
Issue 18575: Sort function Jira Issue VSIPL16-31
Issue 18188: VSIPL and VSIPL++ should define version macros (vsiplxx-rtf)
Click here for this issue's archive.
Nature: Enhancement
Severity: Minor
Summary: The VSIPL++ (and VSIPL) standards should define version macros, so that application code can be conditionalized on what version of the library they are being compiled with. This should be done with numeric macros (i.e., VSIP_VERSION=1 and VSIP_VERSION_MINOR=2) to enable numeric comparisons. In addition, there should be separate macros for VSIPL and VSIPL++, and either there should be separate version macros for VSIPL++ and Parallel VSIPL++, or there should be a macro ("VSIP_PARALLEL"?) to indicate whether the Parallel VSIPL++ pieces are supported.
Resolution: In the interest of closing the current RTF so that the solutions to resolved issues can be made publically available, this issue is deferred to the next RTF. Disposition: Deferred
Revised Text:
Actions taken:
October 19, 2012: received issue
Issue 18196: scatter, gather should support multidimensional index views (vsiplxx-rtf)
Click here for this issue's archive.
Nature: Enhancement
Severity: Minor
Summary: The gather() and scatter() functions, as defined, are capable of gathering from or scattering to multidimensional arrays, by virtue of supporting multidimensional Index variables. However, they cannot gather _to_ or scatter _from_ multidimensional arrays, because they explicitly specify only Vectors of Index variables. The extension to Matrices and Tensors of Index variables, with corresponding-type gather() output and scatter() input, is obvious and should be included.
Resolution: In the interest of closing the current RTF so that the solutions to resolved issues can be made publically available, this issue is deferred to the next RTF. Disposition: Deferred
Revised Text:
Actions taken:
October 23, 2012: received issue
Issue 18197: scatter, gather declarations should use const_View in place of View (vsiplxx-rtf)
Click here for this issue's archive.
Nature: Revision
Severity: Minor
Summary: The gather() and scatter() functions use an "Index<View<T, Block0>::dim>" template to describe the dimension of the Indices that they accept. However, "View" is not defined anywhere; the actual input/output type is given as "const_View". Thus, this should be changed to "Index<const_View<T, Block0>::dim>".
Resolution: In the interest of closing the current RTF so that the solutions to resolved issues can be made publically available, this issue is deferred to the next RTF. Disposition: Deferred
Revised Text:
Actions taken:
October 23, 2012: received issue
Issue 18199: cmplx function is misplaced (vsiplxx-rtf)
Click here for this issue's archive.
Nature: Revision
Severity: Minor
Summary: The cmplx() function is placed at the end of section 10.3.5, which is otherwise about "Scalar functions and their element-wise extensions" -- but cmplx() is defined only as an element-wise function, not as a scalar function. It does not belong in this section. In addition, the return value is given as "The element-wise extension of the complex(T1, T2) constructor to views." This is not the return value -- it returns a view, not a constructor! This should be clarified.
Resolution: In the interest of closing the current RTF so that the solutions to resolved issues can be made publically available, this issue is deferred to the next RTF. Disposition: Deferred
Revised Text:
Actions taken:
October 23, 2012: received issue
Issue 18201: cscalar_i is required inconsistently (vsiplxx-rtf)
Click here for this issue's archive.
Nature: Revision
Severity: Minor
Summary: There are only two functions in the entire VSIPL++ API that are required to support computations on cscalar_i values: sumval and cumsum. This seems quite inconsistent, and those requirements should probably be removed for consistency. (See also previous issue about removing cscalar_i entirely. The only other uses of it are in view constructors, gather, and scatter.)
Resolution: In the interest of closing the current RTF so that the solutions to resolved issues can be made publically available, this issue is deferred to the next RTF. Disposition: Deferred
Revised Text:
Actions taken:
October 23, 2012: received issue
Issue 18202: maxmg etc. of complex should return real values (vsiplxx-rtf)
Click here for this issue's archive.
Nature: Revision
Severity: Minor
Summary: The mag() and magsq() elementwise functions are defined to return real views from complex views, as one would expect. However, the maxmg, maxmgsq, minmg, and minmgsq reduction functions -- which ought to be simply the maximum/minimum of the mag and magsq results, are defined to return complex values. They should likewise return real values. (Note that the Mentor Graphics implementation already does return real values from these functions. The change is unlikely to break user code in any case; if the user assigns the result to a complex variable, that will still work when the functions return real values.)
Resolution: In the interest of closing the current RTF so that the solutions to resolved issues can be made publically available, this issue is deferred to the next RTF. Disposition: Deferred
Revised Text:
Actions taken:
October 23, 2012: received issue
Issue 18203: VSIPL++ spec references nonexistent complex<int> types (vsiplxx-rtf)
Click here for this issue's archive.
Nature: Revision
Severity: Minor
Summary: The specification defines complex_i as "The implementation shall define this type to be complex<short>, complex<int>, or complex<long int> such that the choice of underlying type is the same as scalar_i." This is problematic, because the C++ spec states that complex<T> is not necessarily defined when T is not float or double. In practice, GCC provides only incomplete definitions for the int/short/long cases. Since cscalar_i is only used in a very few places, I suggest that it should simply be removed from the spec. (The changelog in the back of the document mentions a change related to this, but the actual text does not appear to be changed.)
Resolution: In the interest of closing the current RTF so that the solutions to resolved issues can be made publically available, this issue is deferred to the next RTF. Disposition: Deferred
Revised Text:
Actions taken:
October 23, 2012: received issue
Issue 18204: Conv. 'kernel_size()' incorrect for symmetric kernels (vsiplxx-rtf)
Click here for this issue's archive.
Nature: Revision
Severity: Minor
Summary: Copied from an internal bug report: PROBLEM: Convolution accessor 'kernel_size()' is defined to return the domain having the same length for each dimension as 'filter_coeffs'. However, when constructing a convolution with a symmetric kernel (sym_even_len_odd or sym_even_len_even), 'filter_coeffs' only holds a subset of coefficients. In those cases, the true kernel size is larder (For example, in 1D convolutions the true size will either '2 * filter_coeffs.size()' or '2 * filter_coeff.size() + 1'). In contrast, the C-VSIPL spec defines the kernel size as M, and specifies the size of 'filter_coeffs' as either M if symmetry = non_sym, or 'floor(M/2)' if 'symmetry == sym_even_len_{odd,even}'. Queries to return the kernel size of a convolution return M. PROPOSED FIX: Change wording of 'kernel_size()' accessor (and Convolution constructor effects) to indicate correct kernel size. TODO: Review specification of Correlation, FIR, and IIR for similar error.
Resolution: In the interest of closing the current RTF so that the solutions to resolved issues can be made publically available, this issue is deferred to the next RTF. Disposition: Deferred
Revised Text:
Actions taken:
October 23, 2012: received issue
Issue 18205: tuple should provide value accessors (vsiplxx-rtf)
Click here for this issue's archive.
Nature: Enhancement
Severity: Minor
Summary: VSIPL++ implementations are not required to provide any member definitions for tuple<>. However, both the old reference implementation and the Mentor Graphics implementation define static members with the values of the dimension ordering. These are certainly useful within the implementations, and are likely to be useful to applications as well. Recommendation: Require definition of 'dim0', 'dim1', and 'dim2' tuple members, which match the tuple's template arguments.
Resolution: In the interest of closing the current RTF so that the solutions to resolved issues can be made publically available, this issue is deferred to the next RTF. Disposition: Deferred
Revised Text:
Actions taken:
October 23, 2012: received issue
Issue 18206: Fir<> copy ctor must be allowed to throw std::bad_alloc (vsiplxx-rtf)
Click here for this issue's archive.
Nature: Revision
Severity: Minor
Summary: The Fir<> copy constructor is declared "VSIP_NOTHROW". This is erroneous and should instead be "VSIP_THROW((std::bad_alloc))", because the copied Fir<> object must not share the storage allocated by the source instance (in case the source is destroyed). The same error applies to Convolution<>, Correlation<>, and potentially others.
Resolution: In the interest of closing the current RTF so that the solutions to resolved issues can be made publically available, this issue is deferred to the next RTF. Disposition: Deferred
Revised Text:
Actions taken:
October 23, 2012: received issue
Issue 18207: Domain<> is used when unneeded (vsiplxx-rtf)
Click here for this issue's archive.
Nature: Enhancement
Severity: Minor
Summary: There are several places where the VSIPL++ passes Domain<> arguments where only the length portions of the domain are used. The Dense() constructors are the most egregious cases, but there are others such as Fftm(). This is confusing to users -- we've noticed this in training classes -- and leads to unnecessarily initializing and passing around the unused fields in the Domain objects. It would be better to add a smaller object -- e.g., Length<>, equivalent to Index<> -- that contains only the length values, and to redefine these interfaces in terms of it. Implementations can continue to support the Domain<> versions of the interfaces for backwards compatibility. In most of these cases where N is fixed, it would also be an even clearer (and unambiguous) interface to simply accept N length values as integer arguments, and I suggest that those interfaces should be added as well. We already effectively have them for the N=1 case because of the implicit Domain<1>(int) constructor.
Resolution: In the interest of closing the current RTF so that the solutions to resolved issues can be made publically available, this issue is deferred to the next RTF. Disposition: Deferred
Revised Text:
Actions taken:
October 23, 2012: received issue
Issue 18208: Fir<> has duplicate 'symmetry' accessor, member (vsiplxx-rtf)
Click here for this issue's archive.
Nature: Revision
Severity: Minor
Summary: The Fir<> class is required to include both "static symmetry_type const symmetry" and "symmetry_type symmetry()". This duplication of names is illegal, and the duplication of functionality is pointless. (Thus, the Mentor Graphics implementation provides only the const member, not the method.) Less illegally, the continuous_filter member and continuous_filtering() method duplicate each other. Similarly, in [signal.convol] we have a misspelled symmtry member and a symmetry() method, and in both [signal.convol] and [signal.correl] we have a supprt member and support() method. These duplicates and misspellings are confusing and redundant, and should be removed.
Resolution: In the interest of closing the current RTF so that the solutions to resolved issues can be made publically available, this issue is deferred to the next RTF. Disposition: Deferred
Revised Text:
Actions taken:
October 23, 2012: received issue
Issue 18209: 'Explicit alias' to clarify view copy semantics (vsiplxx-rtf)
Click here for this issue's archive.
Nature: Enhancement
Severity: Minor
Summary: In my previous issue, I stated with regards to clarifying the semantics of a "View v(w)" statement: "Unfortunately this is not possible with the alias case, but we can at least resolve it for the copy case. This is not actually true. We could define an alias() function (or method) on a View which returns a unspecified type, and then a View constructor that accepts that type as input and either creates an alias to the precursor view or throws a compile error. Thus, you could write View v(alias(w)); and know that, if it compiles, you are guaranteed to get a true alias.
Resolution: In the interest of closing the current RTF so that the solutions to resolved issues can be made publically available, this issue is deferred to the next RTF. Disposition: Deferred
Revised Text:
Actions taken:
October 23, 2012: received issue
Issue 18210: 'Explicit copy' function to clarify view copy semantics (vsiplxx-rtf)
Click here for this issue's archive.
Nature: Enhancement
Severity: Minor
Summary: The rules for when a copy constructor "View v(w);" will create a view v that aliases the view w are somewhat esoteric and likely to lead to confusion -- the result depends on details of the type of w, which do not appear on this line and (with templates) may be in an entirely separate section of the code. It would be handy to have versions of this statement which are predictable with regards to copy or alias, and which make the intent explicit. Unfortunately this is not possible with the alias case, but we can at least resolve it for the copy case. Thus, I propose we add a deep_copy() function (or a method on views) which would allow us to write "View v(deep_copy(w));" -- with the result that v has the same size as w, and the same values, but is guaranteed to be a copy rather than an alias. (Note that the "deep_copy" name is just a stand-in; there may be better names for this functionality.)
Resolution: In the interest of closing the current RTF so that the solutions to resolved issues can be made publically available, this issue is deferred to the next RTF. Disposition: Deferred
Revised Text:
Actions taken:
October 23, 2012: received issue
Issue 18211: Convolution with decimation!=1 accesses outside input (vsiplxx-rtf)
Click here for this issue's archive.
Nature: Revision
Severity: Minor
Summary: (Copied from an internal Mentor Graphics issue.) Introduction ------------ C-VSIPL defines the following equation to compute the values y_n of a minimum support 1-D convolution: y_n = Sum k from 0 to M-1 of ( h_k * x_(n*D + (M-1) - k) ) Where: N is the input vector length, M is the kernel length, D is the deimation, x_j is the input vector (of size N) h_k is the kernel vector (of size M) C-VSIPL defines the length of the output vector (i.e. values n for which y_n is defined) as: floor( (N-1) / D ) - floor( (M-1) / D) + 1 In some cases, this equation requires the implementation to access elements of x outside the x proper. Example Illustrating the Problem -------------------------------- To see this, first consider the case (where the equation is correct): N = 5, M = 4, D = 1. The expected output length is: floor((5-1)/1) - floor((4-1)/1) + 1 = 4 - 3 + 1 = 2 Consider a diagram showing how each output is computed: x_0 x_1 x_2 x_3 x_4 y_0 = h_3 h_2 h_1 h_0 y_1 = h_3 h_2 h_1 h_0 I.e. y_0 = h_0*x_3 + h_1*x_2 + h_2*x_1 + h_3*x_0 y_1 = h_0*x_4 + h_1*x_3 + h_2*x_2 + h_3*x_1 In computing y_0 and y_1, only values { x_j | 0 <= j < 5 } are required. Next, consider the case: N = 5, M = 4, D = 2. The expected output length is: floor((5-1)/2) - floor((4-1)/2) + 1 = floor(4/2) - floor(3/2) + 1 = 2 - 1 + 1 = 2 To compute y_n, we need to multiply h_k by x_(n D + (M-1) - k). For n=1, k=0, the index to x is n D + (M-1) - k = 1 2 + (4-1) - 0 = 2 + 3 - 0 = 5 However, x_5 is not a valid element of x. Pictorally: x_0 x_1 x_2 x_3 x_4 y_0 = h_3 h_2 h_1 h_0 y_1 = h_3 h_2 h_1 h_0 Finally, consider two more cases: N = 5, M = 4, D = 3 -> output length = 1 N = 5, M = 4, D = 4 -> output length = 2 (Why should D=3 be any different from the other cases?) This does not make sense for several reasons: - First, when D=2, a value (x_5) outside the support of x is required to compute the result. This might run counter to user's expectations of how the minimal output region of support should be defined. - Second, changing the decimation A New Equation for the Output Length ------------------------------------ What should the output length be? Starting from the assumption that the formula for y_n is correct, we need to determine the range of indices {n} for which all indices to x are valid. In particular, we want to find the largest n such that the following holds for all k in 0 <= k <= M-1: 0 <= n*D + (M-1) - k < N The lower bound is true for all n >= 0 since n*D >= 0 and (M-1)-k >= 0. The upper bound: n*D + (M-1) - k < N The largest LHS occurs when k = 0: n*D + (M-1) < N n*D < N - M + 1 n < ceil((N-M+1) / D) The largest n that we can compute y_n for is n = ceil((N-M+1) / D)-1 Hence the output length is: ceil((N-M+1) / D) In the case where D=1, this reduces to: N-M+1 (which is what the current equation reduces too when D=1). Comparing the new equation with the old one for the previous examples: N=5, M=4, D=1 old=2 new=2 N=5, M=4, D=2 old=2 new=1 N=5, M=4, D=3 old=1 new=1 N=5, M=4, D=4 old=2 new=1 Equation for Full Support Output Length --------------------------------------- Similarly, the lengths for full support case are also incorrect. For full support, the assumption is that each output vector should have at least one value from x used to compute it: n*D - k < N (minimal index occurs when k = M-1) n*D - (M-1) < N n*D < N + (M-1) n < ceil((N+M-1)/D) However, here the error is less of an issue because the equation to compute y_n is based on < x_k > rather than x_k. <x_k> is defined to be 0 if k < 0 of k >= N
Resolution: In the interest of closing the current RTF so that the solutions to resolved issues can be made publically available, this issue is deferred to the next RTF. Disposition: Deferred
Revised Text:
Actions taken:
October 23, 2012: received issue
Issue 18212: Meaning of vsip_symmetry enum values inconsistent (vsiplxx-rtf)
Click here for this issue's archive.
Nature: Revision
Severity: Minor
Summary: The vsip_symmetry enumerator is used in a number of places, and in general the VSIP_SYM_EVEN_LEN_* values are defined as "(Even) Symmetric, {odd,even} length". However, in the text there are sporadic mentions that imply that symmetric here really means conjugate-symmetric; for instance, see the bottom of page 567 ("The filter kernel can be even (conjugate) symmetric or non-symmetric.") and the definition of the "kernel" argument to vsip_*fir_create on page 612. This needs to be much more clearly and consistently stated. Regardless of whether the kernel is defined as symmetric or conjugate-symmetric, the method for constructing the complete kernel from the input in the symmetric cases is not defined. Is the input the first half of the kernel, or the second half? Is the first half conjugated, or the second half? This needs to be clearly specified.
Resolution: In the interest of closing the current RTF so that the solutions to resolved issues can be made publically available, this issue is deferred to the next RTF. Disposition: Deferred
Revised Text:
Actions taken:
October 23, 2012: received issue
Issue 18216: Dangerous implicit conversions (vsiplxx-rtf)
Click here for this issue's archive.
Nature: Revision
Severity: Minor
Summary: This issue is copied from an internally-filed issue at Mentor that dates back to the early days of the specification. Since that time, some usage patterns have emerged that appear useful -- most notably, the implicit use of a length_type when a Domain<1> is required. This particular case should either be preserved or additional overloads of the relevant functions should be added. However, in general this complaint is still quite relevant -- for instance, we should not be implicitly converting a length_type value to a LU_solver object. Note that I have not edited this to account for any changes to the specification (either section numbers or normative changes) since the issue was initially submitted in 2005. In most cases, the resolution -- adding an "explicit" keyword to many of these constructors -- seems straightforward and obvious. Chances are very high that any existing code which is broken thereby was erroneous in the first place. ---- Many of the constructors specified can take a single argument but are not declared "explicit". A complete list: Domain<1>(length_type) Index<1>(index_type) Dense<D>(Domain<D> const&) const_Vector(Block&) const_Vector(Vector const&) * const_Vector<>(Vector const&) Vector<>(Block&) template <typename T0, typename Block0> Vector(Vector<T0, Block0> const&) const_Matrix<>(Block&) * const_Matrix<>(Matrix const&) Matrix(Block&) template <typename T0, typename Block0> Matrix(Matrix<T0, Block0> const&) const_Tensor<>(Block&) * const_Tensor<>(Tensor const&) Tensor(Block&) template <typename T0, typename Block0> Tensor(Tensor<T0, Block0> const&) lud<>(length_type) [or LU_solver<>(length_type), see #29] Rand<>(index_type) Of these, only the starred (*) conversions are defensible, and even they will require some design adjustments. All the rest are a problem; most are fixable simply by adding "explicit". For the case of Domain<1>, this will require redesign of other components to restore convenient usage; perhaps similarly for some of the conversions from Block&. Also, in the requirements list 6.3, the line View(Block &) should be annotated to indicate that the conversion should not be implicit. To expand on why these implicit conversions are dangerous... First, implicit conversions from native numeric types are especially problematic because of the promiscuous conversions among those types, inherited from C, that occur even before calling the constructor. As a result, any numeric type, whether int, float, char, or bool, matches the constructor argument, and thus is accepted freely in place of the class type by any function that takes an argument of that type, regardless of whether it makes sense. These mistakes can be very hard to spot; sometimes it results from calling f(a,b) when f(b,a) was meant. (Functions declared to take numeric types can sometimes be made safer by overloading with another that takes a different, but acceptable, numeric type.) Second, implicit conversions of any kind provoke annoying compiler error messages complaining of ambiguities when more than one conversion path is possible. The more implicit conversions that are possible, the more frequently such ambiguities occur. The workaround for such an ambiguity is to add a cast to the exact type, but casts are themselves a source of errors, and besides obscuring the logic of the code, they eliminate the convenience the implicit conversion was supposed to provide. Third, in cases where one conversion path is favored by the compiler over another, the compiler may silently choose what the programmer would consider the wrong one. Fourth, template argument matching ignores conversions, making those that do work less conveniently useful than they may seem.
Resolution: In the interest of closing the current RTF so that the solutions to resolved issues can be made publically available, this issue is deferred to the next RTF. Disposition: Deferred
Revised Text:
Actions taken:
October 23, 2012: received issue
Issue 18217: Unclear antecedent in ~Dense() destructor description (vsiplxx-rtf)
Click here for this issue's archive.
Nature: Revision
Severity: Minor
Summary: The description of the ~Dense() destructor states "It should no longer be used." The antecedent of "it" is grammatically unclear -- although users can guess from context, this should really say "The Dense object should no longer be used." It may perhaps be worth noting explicitly that any views bound to the block should also not be used (although, in that case, what exactly does "used" mean? Is it valid to access their size?).
Resolution: In the interest of closing the current RTF so that the solutions to resolved issues can be made publically available, this issue is deferred to the next RTF. Disposition: Deferred
Revised Text:
Actions taken:
October 23, 2012: received issue
Issue 18218: Block refcounting interface error prone, wrong (vsiplxx-rtf)
Click here for this issue's archive.
Nature: Revision
Severity: Minor
Summary: This issue is copied from an internally-filed issue at Mentor that dates back to the early days of the specification, circa 2005. I expect it will be marked as "out of scope" for the VSIPL++ 1.2 FTF, but it should be considered for a future 2.0 revision of the spec. ---- The specification requiring Blocks to implement reference counting has several problems. First, it overconstrains implementations; they may well have a better way to implement storage management, if permitted. Second, as an interface it is clumsy and fails to take advantage of C++ language strengths (most particularly, destructors) for better safety. Finally, it is far from clear that the library should ever dispose of a user block. If the user created it, isn't the user's responsibility to dispose of it when the user chooses, by whatever means the user chooses? If in fact the library should take responsibility for discarding a user's block when it is no longer needed, it suffices to provide a way for the user to ask that ownership be transferred to the library, without going into detail about how the library will manage it afterward. ... Note that this is also an exception-safety issue. In particular, with the existing interface block storage could easily leak if an exception were thrown before whoever was responsible to decrement the count got to it. If the spec were to say that all blocks derive from template <bool isManaged, bool isConst, typename valueT=..., typename mapT=...> class vsip::Block_base; then that base class and the views could handle all the refcounting business invisibly. Besides being exception-safe and less messy, it would be incrementally more convenient for users to define their own block types. (Cf. std::iterator<>.) Of course users could keep the library from deleting a managed block by keeping their own view on it, too. It is possible to implement the library so that blocks and programs written to the old interface would still work as before.
Resolution: In the interest of closing the current RTF so that the solutions to resolved issues can be made publically available, this issue is deferred to the next RTF. Disposition: Deferred
Revised Text:
Actions taken:
October 23, 2012: received issue
Issue 18219: Block decrement_count() is problematic in some cases (vsiplxx-rtf)
Click here for this issue's archive.
Nature: Revision
Severity: Minor
Summary: The effects of the decrement_count() accessor for blocks are described as "Decrease the object�s use count. If the count becomes zero, the block deallocates itself." This is not always possible. Consider, for instance, this simple code: Dense<...> block(...); block.decrement_count(); Since block is created on the stack, it cannot deallocate itself. Similar issues will occur with blocks created using placement-new into existing allocated memory. At minimum, this should be addressed by a clause stating that the object's use-count shall not be decremented to zero (or allowed to become decremented to zero) except for blocks that have been created using "new".
Resolution: In the interest of closing the current RTF so that the solutions to resolved issues can be made publically available, this issue is deferred to the next RTF. Disposition: Deferred
Revised Text:
Actions taken:
October 23, 2012: received issue
Issue 18220: View's block reference-counting is inconsistent (vsiplxx-rtf)
Click here for this issue's archive.
Nature: Revision
Severity: Minor
Summary: The description of how block reference-counting works with view-allocated blocks is inconsistent. In 6.1.1. [block.alloc], it is stated that "For a block allocated by a view constructor, the effect of creation should include the effect of invoking increment_count." In 8.1 [view.view], however, the contrary requirement is given: "A constructor that does allocate its block must not invoke its block's increment_count member function." These do not appear to be consistent -- or, at the very least, are very confusing. I suppose that the intended meaning is that, when a view allocates a block, the act of creating that block should increment the count, but that the view constructor should not otherwise increment the count. However, that's not at all clear from the text. Further, in 6.1.1, there is this note: "Note: For a block allocated by a view constructor, the number of increment_count invocations should meet or exceed the number of decrement_count invocations if the block is to be used. When the number of decrement_count invocations exceeds, not just equals, increment_count invocations, the block should be deallocated." This seems at first glance to be inconsistent with the definition of decrement_count, which states that the block will be deallocated when the value reaches zero, not goes below zero. It is only consistent if we assume that the phrase "the effect of invoking increment_count" does not include the effect of incrementing the number of times we consider increment_count to have been invoked! Much of this would be clarified if we simply explicitly stated, in the block requirements, that the "use count" should be set to one in the block constructor, rather than the "effect of calling increment_count" wording. Then we could omit the 6.1.1 paragraph 7 note entirely. (That points out a related omission: We do not actually specify the initial use-count value for the general case; for blocks that are not Dense and not created by a view constructor, a conforming implementation could initialize the use-count to infinity. Or three, just for perversity. This is probably not desirable.)
Resolution: In the interest of closing the current RTF so that the solutions to resolved issues can be made publically available, this issue is deferred to the next RTF. Disposition: Deferred
Revised Text:
Actions taken:
October 23, 2012: received issue
Issue 18223: ~Dense() destructor should use "use count". (vsiplxx-rtf)
Click here for this issue's archive.
Nature: Revision
Severity: Minor
Summary: The description of the ~Dense() destructor refers to the "reference count". However, elsewhere the phrase "use count" is used. This should be consistent
Resolution: In the interest of closing the current RTF so that the solutions to resolved issues can be made publically available, this issue is deferred to the next RTF. Disposition: Deferred
Revised Text:
Actions taken:
October 23, 2012: received issue
Issue 18224: Block reference-counting only described in Dense (vsiplxx-rtf)
Click here for this issue's archive.
Nature: Revision
Severity: Minor
Summary: It is clear from the existence of the increment_count and decrement_count functions in the block requirements that the expectation is that the reference-counting should apply to all VSIPL++ blocks. However, there is no text at all that states this, aside from those two function signatures. There is no definition of "use count" anywhere. (See also my previous issue noting that we need to explicitly state that the block constructor initializes it to one.) The only definition of the increment_count and decrement_count is in the section describing the Dense block, although those definitions ought to apply to all blocks. This needs to be rearranged.
Resolution: In the interest of closing the current RTF so that the solutions to resolved issues can be made publically available, this issue is deferred to the next RTF. Disposition: Deferred
Revised Text:
Actions taken:
October 23, 2012: received issue
Issue 18225: Vector destructor should call decrement_count (vsiplxx-rtf)
Click here for this issue's archive.
Nature: Revision
Severity: Minor
Summary: The ~Vector() destructor function is described as "If this object is the only one using its block, the block is deleted. Otherwise, its block's use count is decremented by one." This is a redundant description of the decrement_block functionality, and further spreads the description of the block reference-counting throughout the standard. It would be much better to either (a) explicitly state that "This calls the block's decrement_count accessor, which may deallocate the block." Or, if we do not want to constrain the implementation to literally call the block method, we can alternately add a paragraph to the block requirements section describing increment-count and decrement-count functionality in general (with the increment_count and decrement_count functions being interfaces to it), and the ~Vector description should refer to that.
Resolution: In the interest of closing the current RTF so that the solutions to resolved issues can be made publically available, this issue is deferred to the next RTF. Disposition: Deferred
Revised Text:
Actions taken:
October 23, 2012: received issue
Issue 18259: Note on Deferred memory allocation (vsiplxx-rtf)
Click here for this issue's archive.
Nature: Revision
Severity: Minor
Summary: p48 says "Note to Implementors: memory allocation shall not be deferred." Why not? This is clearly an oblique reference to some unspoken semantics of the interface that isn't documented, and perhaps should be.
Resolution: In the interest of closing the current RTF so that the solutions to resolved issues can be made publically available, this issue is deferred to the next RTF. Disposition: Deferred
Revised Text:
Actions taken:
November 13, 2012: received issue
Issue 18261: Notes to implementors (vsiplxx-rtf)
Click here for this issue's archive.
Nature: Revision
Severity: Minor
Summary: There are several oblique references to hidden interface semantics in "Notes to implementors", particularly the expectation that a development mode should keep reference counts on various data structures. However, since providing a development mode is not mandatory, this cannot be a normative part of the specification. This could be made clearer.
Resolution: In the interest of closing the current RTF so that the solutions to resolved issues can be made publically available, this issue is deferred to the next RTF. Disposition: Deferred
Revised Text:
Actions taken:
November 13, 2012: received issue
Issue 18381: Distinction between "Restrictions" and "Errors" is unclear (vsiplxx-rtf)
Click here for this issue's archive.
Source: Mentor Graphics Corporation (Mr. Stefan Seefeld, stefan(at)seefeld.name)
Nature: Clarification
Severity: Minor
Summary:
Throughout the document functions are defined with formal sections for "Restrictions" and "Errors". It isn't clear what the purpose of these two items is. In particular, "Errors" should actually spell out what errors (if any) the given function may generate. What it actually does, however, is specify conditions for the function arguments. These really belong into the "Restrictions" section
VSIPL and VSIPL++ both provide APIs to serialize certain objects. The VSIPL++ API supports serialization of blocks, while the VSIPL API supports serialization of function objects. The API for both should be unified and made optional (a "profile"), as it is orthogonal to the rest of the spec.
While working on the translation to DocBook I noticed lots of small typos ("matrix" instead of "vector", "2d" instead of "1d", references to non-existing functions in "see also" sections, etc.) While these may all be obvious, i.e. not require any discussion or even semantic changes to the spec, the document needs a careful review to correct these.
I don't think that Sort is defined the best way in VSIPL. Why spec doesn't define create function? Such function will let user to allocate additional memory which are required by some sorting methods. Why not to define out-of-place sort? If I work on 64-bit system, sort array of 32-bit FP values and need index vector, I'll get 64-bit indices which are not supposed to be that long. This is because last argument of sort function is const vsip_vview_vi *index and _vi means (unsigned long int) i.e. 64-bit elements. In the sort I implemented for AVX I permute data in vector registers and apply same permute to the indices which occupy other vector register. It's OK if function has to produce 32-bit indices, but it will be wrong when I have to shuffle 64-bit values. This last comment is applicable to other functions as well - index vector on 64-bit systems doesn't have to be 64-bit value because data arrays today are not huge yet.