Skip to content

Rip out core::arch wrappers#216

Merged
Shnatsel merged 4 commits into
linebender:mainfrom
Shnatsel:rip-out-core-arch
May 19, 2026
Merged

Rip out core::arch wrappers#216
Shnatsel merged 4 commits into
linebender:mainfrom
Shnatsel:rip-out-core-arch

Conversation

@Shnatsel
Copy link
Copy Markdown
Contributor

@Shnatsel Shnatsel commented May 18, 2026

Remove the wrappers for intrinsics inherited from pulp now that #214 provides a way to access intrinsics without us having to wrap every single one.

Part of #166

Supersedes #209

@Shnatsel Shnatsel requested a review from LaurenzV May 18, 2026 13:59
@Shnatsel Shnatsel marked this pull request as ready for review May 18, 2026 14:03
@LaurenzV
Copy link
Copy Markdown
Collaborator

Should this be merged before #214 or after?

@Shnatsel
Copy link
Copy Markdown
Contributor Author

I would prefer merging this after #214. To keep CI passing I had to create some editing conflicts in samples, I'd rather resolve them here by simply overriding them with the samples from #214.

@LaurenzV
Copy link
Copy Markdown
Collaborator

I approved the other PR, not sure if you wanted to wait for input from Daniel though.

@Shnatsel
Copy link
Copy Markdown
Contributor Author

Thanks! I'll wait a few days to let Daniel and the others chime in. This isn't blocking any follow-up work as far as I know.

If it holds anything up, please let me know and I'll go ahead and merge it.

@Shnatsel
Copy link
Copy Markdown
Contributor Author

Shnatsel commented May 19, 2026

Now that #214 is in, this should also be good to go. Daniel has explicitly stated he supports removing this: #212 (review)

I'd appreciate an approval on this PR

@Shnatsel Shnatsel enabled auto-merge May 19, 2026 13:54
@DJMcNab
Copy link
Copy Markdown
Member

DJMcNab commented May 19, 2026

I've not checked yet, as I'm on mobile. There aren't two levels of tokens now, right, or are there?
Would you think we'll tear that out at some point?

@Shnatsel
Copy link
Copy Markdown
Contributor Author

There aren't two levels, no. The tokens were simply in the same file as all the macros to generate wrappers.

Copy link
Copy Markdown
Collaborator

@LaurenzV LaurenzV left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, just two things but can/should be follow-ups:

  1. Is there even any point in keeping the SSE3/FMA/etc. tokens since those aren't supported natively anyway?
  2. We probably should remove the subfolders and put everything into a single file per architecture or even just everything into one file.

@Shnatsel Shnatsel added this pull request to the merge queue May 19, 2026
@LaurenzV LaurenzV removed this pull request from the merge queue due to a manual request May 19, 2026
@LaurenzV
Copy link
Copy Markdown
Collaborator

(Removing it from the queue just to make sure you saw my comment, feel free to re-add)

@DJMcNab
Copy link
Copy Markdown
Member

DJMcNab commented May 19, 2026

I mean these structures:

pub struct Avx2 {
    pub avx2: crate::core_arch::x86::Avx2,
}

I'm on mobile, so it's not easy to browse your branch, but I don't think those are collapsed. It's fine for that to be another follow-up, fwiw

@Shnatsel
Copy link
Copy Markdown
Contributor Author

Ah, good catch! I totally missed that the token structs are wrapped and are indeed duplicated.

I'll remove them before re-enabling auto-merge. Thanks!

@Shnatsel Shnatsel force-pushed the rip-out-core-arch branch from e972e69 to 0471987 Compare May 19, 2026 22:07
…emit the wrapper structs directly without core_arch references
@Shnatsel Shnatsel force-pushed the rip-out-core-arch branch from 0471987 to d1711d6 Compare May 19, 2026 22:08
@Shnatsel
Copy link
Copy Markdown
Contributor Author

Committing file deletions to git is hard 😅

@Shnatsel Shnatsel added this pull request to the merge queue May 19, 2026
Merged via the queue into linebender:main with commit a6ed0e4 May 19, 2026
22 checks passed
@Shnatsel Shnatsel deleted the rip-out-core-arch branch May 19, 2026 22:15
Comment thread fearless_simd/src/lib.rs
Comment on lines -420 to -421
/// This can be used in combination with the `safe_wrappers` feature to gain checked access to
/// the level-specific SIMD capabilities.
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Possibly we should point to the kernel macro here. This isn't urgent

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.

3 participants