Skip to content

Conversation

@mgehre-amd
Copy link
Collaborator

No description provided.

nikalra and others added 2 commits October 1, 2024 09:48
This change makes two (related) changes: 

First, it updates the tablegen option for `ListOption` to emit a
`SmallVector` instead of an `ArrayRef`. This brings `ListOption` more
inline with the traditional `Option`, where values are typically
provided using types that have storage. After this change, all options
should be fully owned by a Pass' `Options` object after it has been
fully constructed, unless the underlying type of the `Option` explicitly
indicates otherwise.

Second, it updates the generated constructors for Passes to consume
options by value instead of reference, and prefers moving options into
the pass itself. This should be more efficient for non-trivial options
objects, where the previous interface forced a copy to be materialized.
Now, at worst case the API materializes a copy (no worse than before);
at best-case, all options objects are moved into place. Ideally, we
could update the Pass constructor to take an r-value reference to the
Options object instead, but this approach will require numerous changes
to existing passes and their factory functions.

---------

Authored-by: Nikhil Kalra <nkalra@apple.com>
@mgehre-amd mgehre-amd changed the title [AutoBump] Merge with fixes of fef3566a (Oct 01) (6) [AutoBump] Merge with fixes of fef3566a (Oct 01, needs downstream change) (6) Jan 10, 2025
@mgehre-amd mgehre-amd requested a review from cferry-AMD January 10, 2025 17:20
Base automatically changed from bump_to_afc0557a to bump_to_357c1970 January 13, 2025 08:21
Base automatically changed from bump_to_357c1970 to feature/fused-ops January 13, 2025 12:34
@mgehre-amd mgehre-amd merged commit f670e5d into feature/fused-ops Jan 17, 2025
4 checks passed
@mgehre-amd mgehre-amd deleted the bump_to_fef3566a branch January 17, 2025 06:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants