Skip to content

Implement the tile intrinsics - #975

Open
fbernier wants to merge 2 commits into
rust-lang:masterfrom
fbernier:amx-tile-intrinsics
Open

fbernier wants to merge 2 commits into
rust-lang:masterfrom
fbernier:amx-tile-intrinsics

Conversation

@fbernier

@fbernier fbernier commented Sep 10, 2026

Copy link
Copy Markdown
Contributor

Replace the traps for the 15 fixed-register AMX intrinsics in #563. Use GCC builtins for tile configuration and volatile inline asm for the rest. Loads and stores carry memory clobbers so GCC cannot discard input writes or reuse stale output values.

Add assembly tests for both dialects, operand order and memory effects. Compiler-allocated tiles and newer AMX extensions are unchanged.

The assembly suite passes. All 15 upstream runtime cases passed under Intel SDE at -O0 and -O3; libcore, mini/std, Clippy and the no-default-features check also passed.

Fixes #563.

@antoyo antoyo left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Some things to fix:

View changes since this review

# FIXME: remove --skip test_tile_ and --skip --skip test__tile when it's implemented.
./y.sh test --release --stdarch-tests -- --skip test_tile_ --skip test__tile
# Compiler-allocated __tile_* APIs are not supported yet.
./y.sh test --release --stdarch-tests -- --skip test__tile

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Keep a FIXME for --skip test__tile.

# FIXME: these tests fail when the sysroot is compiled with LTO because of a missing symbol in proc-macro.
# FIXME: remove --skip test_tile_ and --skip --skip test__tile when it's implemented.
STDARCH_TEST_SKIP_FUNCTION="xsave,xsaveopt,xsave64,xsaveopt64" STDARCH_TEST_EVERYTHING=1 CHANNEL=release CARGO_TARGET_X86_64_UNKNOWN_LINUX_GNU_RUNNER="${{ matrix.cargo_runner }}" TARGET=x86_64-unknown-linux-gnu CG_RUSTFLAGS="-Ainternal_features" ./y.sh cargo test --manifest-path build/build_sysroot/sysroot_src/library/stdarch/Cargo.toml -- --skip rtm --skip tbm --skip sse4a --skip test_tile_ --skip test__tile
# Compiler-allocated __tile_* APIs are not supported yet.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Same here.

Comment thread src/intrinsic/llvm.rs
Comment on lines +9 to +12
#[cfg(feature = "master")]
use rustc_middle::ty;
#[cfg(feature = "master")]
use rustc_middle::ty::layout::LayoutOf;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
#[cfg(feature = "master")]
use rustc_middle::ty;
#[cfg(feature = "master")]
use rustc_middle::ty::layout::LayoutOf;
#[cfg(feature = "master")]
use rustc_middle::ty::{self, layout::LayoutOf};

Comment thread src/intrinsic/llvm.rs
use crate::context::{CodegenCx, new_array_type};
use crate::type_::{StructAttribute, apply_struct_attributes};

/// Lower AMX operations whose tile operands name architectural registers, not SSA values.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
/// Lower AMX operations whose tile operands name architectural registers, not SSA values.
/// Lower AMX operations whose tile operands name registers, not SSA values.

Comment thread src/intrinsic/llvm.rs
mut args: Cow<'b, [RValue<'gcc>]>,
func_name: &str,
) -> Cow<'b, [RValue<'gcc>]> {
// FIXME: this might not be a good way to workaround the missing tile builtins.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Please also keep this comment.

Comment thread src/intrinsic/llvm.rs
Comment on lines +102 to +107
let sig = builder
.tcx
.fn_sig(instance.def_id())
.instantiate(builder.tcx, instance.args)
.skip_norm_wip();
let sig = builder.tcx.instantiate_bound_regions_with_erased(sig);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This code is not obvious to me, so please write a comment to explain this (perhaps not needed if the above comment is clearer).

Comment thread src/intrinsic/llvm.rs
Comment on lines +115 to +116
// GCC does not allocate these tile registers. Keep their implicit state changes, including
// operations with no memory effects, ordered with the other AMX operations.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This comment is not clear to me, so please clarify it.

Comment thread src/intrinsic/llvm.rs
.skip_norm_wip();
let sig = builder.tcx.instantiate_bound_regions_with_erased(sig);
let result_type = builder.backend_type(builder.layout_of(sig.output()));
Some(builder.current_func().new_local(builder.location, result_type, "amx_row"))

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Please use Builder::new_temp instead of new_local.

Comment thread src/intrinsic/mod.rs
} else {
let sym = self.tcx.symbol_name(instance).name;
#[cfg(feature = "master")]
if let Some(result) = llvm::codegen_x86_amx(self, instance, sym, args) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Please add a comment here to explain that it is OK for this call to be in the else since codegen_x86_amx does not insert in the cache.

Comment thread src/intrinsic/llvm.rs
Comment on lines +85 to +86
"llvm.x86.tilemovrow" => tile_row!("tilemovrow", "%k2", "r"),
"llvm.x86.tilemovrowi" => tile_row!("tilemovrow", "%2", "i"),

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Instead of repeating "%k2", "r" and "%2", "i" on many lines, make the macro generate those by having something like this:

Suggested change
"llvm.x86.tilemovrow" => tile_row!("tilemovrow", "%k2", "r"),
"llvm.x86.tilemovrowi" => tile_row!("tilemovrow", "%2", "i"),
"llvm.x86.tilemovrow" => tile_row!("tilemovrow", register),
"llvm.x86.tilemovrowi" => tile_row!("tilemovrow", immediate),

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.

Implement the tile intrinsics

2 participants