Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clarify __swizzled_vec__ class #514

Merged
merged 19 commits into from
Jun 27, 2024

Commits on Dec 5, 2023

  1. Clarify __swizzled_vec__ class

    Prior to this commit, the `__swizzled_vec__` class was not precisely
    specified.  We said that it had the same interface as `vec` except
    for a small set of differences that we enumerated.  However, it became
    clear after discussing with the original authors that this wasn't a
    complete list of differences.
    
    This commit adds a complete list of the members and friend functions
    for the `__swizzled_vec__` type.  In many cases, the descriptions
    simply defer to the `vec` specification, but there are also many cases
    where the semantics are special for `__swizzled_vec__`.
    
    The updated specification also describes `__swizzled_vec__` as a view
    over the original `vec` object, which I think is what we originally
    intended.  I intentionally removed wording like "may not be bound to a
    l-value or escape the expression it was constructed in."  I think the
    true restriction is that the lifetime of the `__swizzled_vec__` must
    not outlive the lifetime of the underlying `vec`, which is the case
    for any view.
    
    I also intentionally removed the statement:
    
    > Both kinds of swizzle member functions must not perform the swizzle
    > operation themselves, instead the swizzle operation must be performed
    > by the returned instance of `__swizzled_vec__` when used within an
    > expression, meaning if the returned `__swizzled_vec__` is never used
    > in an expression no swizzle operation is performed.
    
    This wording made is sound like the point of `__swizzled_vec__` was to
    delay the swizzle operation as a sort of optimization.  However, our
    original intent was only to specify `__swizzled_vec__` as a view.
    
    This commit also uses our updated style to specify the
    `__swizzled_vec__` type, using a format that gives more horizontal
    space for the synopses and descriptions.
    
    Closes KhronosGroup#504
    gmlueck committed Dec 5, 2023
    Configuration menu
    Copy the full SHA
    64fbeba View commit details
    Browse the repository at this point in the history

Commits on Dec 8, 2023

  1. Tweak introduction of swizzled_vec

    Improve the wording in the introduction of the section on
    `__swizzled_vec__` based on review comment feedback.
    gmlueck committed Dec 8, 2023
    Configuration menu
    Copy the full SHA
    0271b4e View commit details
    Browse the repository at this point in the history

Commits on Dec 18, 2023

  1. Add constraint for assignment operators

    Addresses a code review comment.  Some compiler implementations may not
    allow the lhs of a swizzled vector assignment to have repeated
    elements, so add a constraint to disallow this.  It should be easy to
    implement this using normal SFINAE techniques.
    gmlueck committed Dec 18, 2023
    Configuration menu
    Copy the full SHA
    9585110 View commit details
    Browse the repository at this point in the history

Commits on Dec 19, 2023

  1. Clarify "__swizzled_vec__</*unspecified*/>"

    Clarify the distinction between `__swizzled_vec__` and
    `__swizzled_vec__</*unspecified*/>` when used in the synopses.
    gmlueck committed Dec 19, 2023
    Configuration menu
    Copy the full SHA
    f71b77e View commit details
    Browse the repository at this point in the history

Commits on Dec 21, 2023

  1. Avoid use of "this" (in quotes)

    I got feedback that the convention of putting quotes around "this" to
    indicate the C++ "this" object was confusing.  I did this in cases
    where there was some ambiguity about which `__swizzled_vec__` we are
    referring to.  Reword these sentences to find another way to clarify
    the ambiguity.
    gmlueck committed Dec 21, 2023
    Configuration menu
    Copy the full SHA
    cd14ca9 View commit details
    Browse the repository at this point in the history

Commits on Mar 15, 2024

  1. Configuration menu
    Copy the full SHA
    4564727 View commit details
    Browse the repository at this point in the history

Commits on Mar 19, 2024

  1. Make swizzle view members const

    We discovered after some implemenation experience that the definitions
    proposed in the early commits of this PR don't support all of the
    desired use cases.  A simple example is:
    
    ```
    sycl::vec<int, 4> v;
    v.swizzle<1>()++;
    ```
    
    This won't compile because the declaration of `operator++` takes a
    mutable reference to the swizzle object:
    
    ```
    friend vec<DataT, NumElements> operator++(__swizzled_vec__& sv, int);
    ```
    
    C++ language rules prohibit binding the temporary object returned by
    `v.swizzle<>()` to the mutable l-value reference parameter to
    `operator++`.
    
    To fix this, we changed all the swizzle operators to take a `const`
    reference to the swizzle object.  This makes sense because the swizzle
    class is a view over the underlying `vec`, and member functions on a
    view class are normally `const`, even for functions that mutate the
    object that underlies the view.  The logic here is that the view itself
    is not modified, only the object referenced by the view.
    
    This raises a new problem, though, because we don't want to allow code
    to change a `const vec` via a swizzle.  For example, code like this
    should result in a diagnostic:
    
    ```
    const sycl::vec<int, 4> v;
    v.swizzle<1>()++;
    ```
    
    To fix this, we replace `__swizzled_vec__` with two classes:
    `__writeable_swizzle__` and `__const_swizzle__`.  We use
    `__const_swizzle__` for swizzles of `const vec`, and this class
    provides only the non-mutating operators.
    gmlueck committed Mar 19, 2024
    Configuration menu
    Copy the full SHA
    0d9fbfb View commit details
    Browse the repository at this point in the history
  2. Change builtin functions to use either swizzle

    Now that we have two swizzle types (`__writeable_swizzle__` and
    `__const_swizzle__`), change the builtin functions to accept either
    of them.
    gmlueck committed Mar 19, 2024
    Configuration menu
    Copy the full SHA
    662d79c View commit details
    Browse the repository at this point in the history
  3. Change table style

    We used to enclose all the member function descriptions in a table, so
    that the description and synopsis of each function was visually
    enclosed in a single table cell.  This caused a problem with the PDF
    render, though, because some table rows were too big to fit on a single
    page, which causes an error when rendering the PDF.  Fix this by
    eliminating the table.  Instead, put a horizontal line between the
    descriptions of the functions to visually separate them.
    gmlueck committed Mar 19, 2024
    Configuration menu
    Copy the full SHA
    6bc7f96 View commit details
    Browse the repository at this point in the history

Commits on Apr 4, 2024

  1. Formatting

    Avoid line wrap in synopsis in PDF render.
    gmlueck committed Apr 4, 2024
    Configuration menu
    Copy the full SHA
    3f597d7 View commit details
    Browse the repository at this point in the history

Commits on May 17, 2024

  1. Configuration menu
    Copy the full SHA
    fa17042 View commit details
    Browse the repository at this point in the history
  2. Update adoc/chapters/programming_interface.adoc

    Co-authored-by: Gordon Brown <gordon@codeplay.com>
    gmlueck and AerialMantis authored May 17, 2024
    Configuration menu
    Copy the full SHA
    3581d9a View commit details
    Browse the repository at this point in the history
  3. Apply suggestions from code review

    Co-authored-by: Gordon Brown <gordon@codeplay.com>
    gmlueck and AerialMantis authored May 17, 2024
    Configuration menu
    Copy the full SHA
    6f188dd View commit details
    Browse the repository at this point in the history
  4. Fix reflow

    gmlueck committed May 17, 2024
    Configuration menu
    Copy the full SHA
    686e19c View commit details
    Browse the repository at this point in the history
  5. Improved wording from Gordon

    gmlueck committed May 17, 2024
    Configuration menu
    Copy the full SHA
    ebfda40 View commit details
    Browse the repository at this point in the history
  6. Improved wording from Gordon

    gmlueck committed May 17, 2024
    Configuration menu
    Copy the full SHA
    4e94a2c View commit details
    Browse the repository at this point in the history

Commits on Jun 25, 2024

  1. Document swizzle destructors

    Add a section documenting the destructors for the
    `__writeable_swizzle__` and `__const_swizzle__` classes.
    
    I debated whether we should require them to be trivial.  I think they
    probably will be trivial in a typical implementation, but I wasn't
    certain enough to put this in the spec.  For now, I just said that they
    have no visible effect.  This allows an implementation to contain some
    internal state that is destroyed by the destructor, but I suspect most
    implementations won't require this.
    gmlueck committed Jun 25, 2024
    Configuration menu
    Copy the full SHA
    3514fc2 View commit details
    Browse the repository at this point in the history
  2. Configuration menu
    Copy the full SHA
    4eb4fe3 View commit details
    Browse the repository at this point in the history
  3. Reflow

    gmlueck committed Jun 25, 2024
    Configuration menu
    Copy the full SHA
    ef571ed View commit details
    Browse the repository at this point in the history