diff --git a/cuda-oxide/crates/cuda-macros/src/launch_attrs.rs b/cuda-oxide/crates/cuda-macros/src/launch_attrs.rs index 54e7bfbdbe..b10f4cd455 100644 --- a/cuda-oxide/crates/cuda-macros/src/launch_attrs.rs +++ b/cuda-oxide/crates/cuda-macros/src/launch_attrs.rs @@ -21,6 +21,7 @@ use syn::{ parse::{Parse, ParseStream}, parse_macro_input, parse_quote, punctuated::Punctuated, + spanned::Spanned, visit::{self, Visit}, visit_mut::{self, VisitMut}, }; @@ -443,10 +444,13 @@ pub(crate) struct LoopUnrollAttrVisitor { impl LoopUnrollAttrVisitor { /// If `attrs` contains an outer `#[unroll]` / `#[unroll(N)]`, remove it and - /// return the parsed factor. Returns `None` when no `unroll` attribute is - /// present (leaving `attrs` untouched). Records a parse error and returns - /// `None` if the attribute is malformed. - fn take_unroll_factor(&mut self, attrs: &mut Vec) -> Option { + /// return the parsed factor with the attribute's span. Returns `None` when + /// no `unroll` attribute is present (leaving `attrs` untouched). Records a + /// parse error and returns `None` if the attribute is malformed. + fn take_unroll_factor( + &mut self, + attrs: &mut Vec, + ) -> Option<(ConstU32Expr, proc_macro2::Span)> { let idx = attrs .iter() .position(|attr| attr.path().is_ident("unroll"))?; @@ -477,14 +481,20 @@ impl LoopUnrollAttrVisitor { if factor.literal_value.is_none() { self.const_expressions.push(factor.clone()); } - Some(factor) + Some((factor, attr.span())) } - /// Build the `__unroll_config::()` marker statement. - fn marker_stmt(factor: &ConstU32Expr) -> Stmt { + /// Build the `__unroll_config::()` marker statement. The call + /// carries `span`, the `#[unroll]` attribute's, so diagnostics about the + /// loop point at the attribute. The braces around the factor keep the + /// macro's own span. On the attribute's span rustc treats them as written + /// by the user, and its `unused_braces` lint then reports every literal + /// factor with a fix-it that deletes the attribute. + fn marker_stmt(factor: &ConstU32Expr, span: proc_macro2::Span) -> Stmt { let expr = &factor.expr; - parse_quote! { - cuda_device::thread::__unroll_config::<{ #expr }>(); + let factor_arg = quote! { { #expr } }; + syn::parse_quote_spanned! {span=> + cuda_device::thread::__unroll_config::<#factor_arg>(); } } } @@ -496,18 +506,27 @@ impl VisitMut for LoopUnrollAttrVisitor { // else falls through to the default recursion below. match expr { Expr::ForLoop(for_loop) => { - if let Some(factor) = self.take_unroll_factor(&mut for_loop.attrs) { - for_loop.body.stmts.insert(0, Self::marker_stmt(&factor)); + if let Some((factor, span)) = self.take_unroll_factor(&mut for_loop.attrs) { + for_loop + .body + .stmts + .insert(0, Self::marker_stmt(&factor, span)); } } Expr::While(while_loop) => { - if let Some(factor) = self.take_unroll_factor(&mut while_loop.attrs) { - while_loop.body.stmts.insert(0, Self::marker_stmt(&factor)); + if let Some((factor, span)) = self.take_unroll_factor(&mut while_loop.attrs) { + while_loop + .body + .stmts + .insert(0, Self::marker_stmt(&factor, span)); } } Expr::Loop(loop_expr) => { - if let Some(factor) = self.take_unroll_factor(&mut loop_expr.attrs) { - loop_expr.body.stmts.insert(0, Self::marker_stmt(&factor)); + if let Some((factor, span)) = self.take_unroll_factor(&mut loop_expr.attrs) { + loop_expr + .body + .stmts + .insert(0, Self::marker_stmt(&factor, span)); } } _ => {} diff --git a/cuda-oxide/crates/cuda-macros/src/tests/launch.rs b/cuda-oxide/crates/cuda-macros/src/tests/launch.rs index d6eb177f50..4cee9d382d 100644 --- a/cuda-oxide/crates/cuda-macros/src/tests/launch.rs +++ b/cuda-oxide/crates/cuda-macros/src/tests/launch.rs @@ -14,7 +14,7 @@ use crate::launch_attrs::{ }; use quote::{format_ident, quote}; use reserved_oxide_symbols::INSTANTIATE_PREFIX; -use syn::{ItemFn, Stmt, parse_quote, visit_mut::VisitMut}; +use syn::{Expr, ItemFn, Stmt, parse_quote, spanned::Spanned, visit_mut::VisitMut}; #[test] fn generated_kernel_siblings_preserve_qualified_paths_and_generics() { @@ -266,6 +266,32 @@ fn unroll_n_on_for_loop_injects_factor_n_marker() { ); } +#[test] +fn unroll_marker_takes_the_position_of_its_attribute() { + // `parse_str` keeps real lines; `parse_quote!` gives every span line 0. + let cases = [ + ("fn k() {\n #[unroll]\n while true {}\n}", 2, 4), + ( + "fn k() {\n\n #[unroll(4)]\n for _ in 0..8 {}\n}", + 3, + 4, + ), + ("fn k() {\n #[unroll(2)]\n loop { break; }\n}", 2, 2), + ]; + for (source, line, column) in cases { + let mut func: ItemFn = syn::parse_str(source).expect("test source parses"); + rewrite_loop_unroll_attrs(&mut func).expect("valid unroll attribute"); + let marker = match &func.block.stmts[0] { + Stmt::Expr(Expr::While(l), _) => &l.body.stmts[0], + Stmt::Expr(Expr::ForLoop(l), _) => &l.body.stmts[0], + Stmt::Expr(Expr::Loop(l), _) => &l.body.stmts[0], + other => panic!("expected a loop, got {}", quote!(#other)), + }; + let start = marker.span().start(); + assert_eq!((start.line, start.column), (line, column), "{source}"); + } +} + #[test] fn unroll_on_bare_loop_injects_marker() { let func: ItemFn = parse_quote! { diff --git a/cuda-oxide/crates/cuda-macros/tests/compile_fail/policy_config_invalid_unroll.stderr b/cuda-oxide/crates/cuda-macros/tests/compile_fail/policy_config_invalid_unroll.stderr index 3d37cf49ca..fe4d9eb24e 100644 --- a/cuda-oxide/crates/cuda-macros/tests/compile_fail/policy_config_invalid_unroll.stderr +++ b/cuda-oxide/crates/cuda-macros/tests/compile_fail/policy_config_invalid_unroll.stderr @@ -26,9 +26,7 @@ note: erroneous constant encountered | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ note: the above error was encountered while instantiating `fn __unroll_config::<1>` - --> tests/compile_fail/policy_config_invalid_unroll.rs:12:1 + --> tests/compile_fail/policy_config_invalid_unroll.rs:15:5 | -12 | #[kernel] - | ^^^^^^^^^ - | - = note: this note originates in the attribute macro `kernel` (in Nightly builds, run with -Z macro-backtrace for more info) +15 | #[unroll(Invalid::UNROLL)] + | ^^^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/cuda-oxide/crates/cuda-macros/tests/macro_guard.rs b/cuda-oxide/crates/cuda-macros/tests/macro_guard.rs index 46683828d3..6491b8f631 100644 --- a/cuda-oxide/crates/cuda-macros/tests/macro_guard.rs +++ b/cuda-oxide/crates/cuda-macros/tests/macro_guard.rs @@ -9,6 +9,7 @@ fn macro_guards() { let t = trybuild::TestCases::new(); t.pass("tests/pass/const_generic_hygiene.rs"); + t.pass("tests/pass/unroll_literal_factor_lint_clean.rs"); t.pass("tests/pass/cuda_module_inline_namespaces.rs"); t.compile_fail("tests/compile_fail/kernel_reserved_name.rs"); t.compile_fail("tests/compile_fail/device_reserved_name.rs"); diff --git a/cuda-oxide/crates/cuda-macros/tests/pass/unroll_literal_factor_lint_clean.rs b/cuda-oxide/crates/cuda-macros/tests/pass/unroll_literal_factor_lint_clean.rs new file mode 100644 index 0000000000..e44e921cc5 --- /dev/null +++ b/cuda-oxide/crates/cuda-macros/tests/pass/unroll_literal_factor_lint_clean.rs @@ -0,0 +1,45 @@ +// SPDX-FileCopyrightText: Copyright (c) 2026 NVIDIA CORPORATION & AFFILIATES. All rights reserved. +// SPDX-License-Identifier: Apache-2.0 + +// A literal `#[unroll(N)]` expands to a marker call whose const argument sits +// in braces. Those braces must keep the macro's own span. On the attribute's +// span rustc treats them as written by the user, and its `unused_braces` lint +// then reports every literal factor in the user's crate, with a fix-it that +// deletes the attribute. This crate denies that lint, so the build fails if +// the braces ever take the attribute's span again. +#![deny(unused_braces)] + +use cuda_device::{device, kernel}; + +#[device] +fn count_down(mut remaining: u32) -> u32 { + let mut steps = 0; + #[unroll] + loop { + if remaining == 0 { + break; + } + remaining -= 1; + steps += 1; + } + steps +} + +#[kernel] +fn literal_factors(limit: u32) { + let mut total = 0; + #[unroll(4)] + for i in 0..8 { + total += i; + } + let mut index = 0; + #[unroll(2)] + while index < limit { + index += 1; + } + let _ = (total, count_down(limit)); +} + +fn main() { + assert_eq!(count_down(3), 3); +} diff --git a/cuda-oxide/crates/mir-transforms/src/unroll.rs b/cuda-oxide/crates/mir-transforms/src/unroll.rs index c15bcf938e..1616439232 100644 --- a/cuda-oxide/crates/mir-transforms/src/unroll.rs +++ b/cuda-oxide/crates/mir-transforms/src/unroll.rs @@ -63,6 +63,7 @@ use pliron::irbuild::{ rewriter::IRRewriter, }; use pliron::linked_list::ContainsLinkedList; +use pliron::location::{Location, Source}; use pliron::op::{Op, op_cast}; use pliron::operation::Operation; use pliron::opts::constants::sccp::sccp; @@ -112,6 +113,54 @@ fn unroll_kind(factor: u32) -> String { } } +/// A hint op's source position as `file:line:column`, the form editors and +/// terminals turn into a link. Returns `None` when the op does not carry a +/// single position in a file. +fn hint_location(ctx: &Context, op: Ptr) -> Option { + match op.deref(ctx).loc() { + Location::SrcPos { + src: Source::File(path), + pos, + } => Some(format!( + "{}:{}:{}", + pliron::uniqued_any::get(ctx, *path).display(), + pos.line, + pos.column + )), + _ => None, + } +} + +/// Unlink every hint op annotating `loop_id` and return the first located +/// one's position, for the warning that speaks for the loop. Returns `None` +/// when none of them carries a source position. +fn take_loop_hints( + ctx: &Context, + hints: &[(Ptr, Ptr, u32)], + info: &LoopInfo, + loop_id: usize, +) -> Option { + let mut location = None; + for (op, block, _f) in hints { + if info.innermost_loop(*block) == Some(loop_id) { + location = location.or_else(|| hint_location(ctx, *op)); + op.unlink(ctx); + } + } + location +} + +/// The full text of one "not unrolled" warning: the reason line, then the +/// annotation's source position on a `-->` line when `location` is `Some`. +fn not_unrolled_warning(kind: &str, reason: &str, location: Option<&str>) -> String { + let mut warning = format!("warning: {kind} requested but the loop was not unrolled: {reason}"); + if let Some(location) = location { + warning.push_str("\n --> "); + warning.push_str(location); + } + warning +} + /// Check code-growth budgets whose inputs have already been counted safely. fn check_growth_budget( copies: u64, @@ -257,8 +306,12 @@ pub fn unroll_annotated_loops( // for unrolling, so say so loudly, then drop the markers and stop. for (op, _block, factor) in &hints { eprintln!( - "warning: {} requested but the loop was not unrolled: the annotation is not inside a recognizable loop", - unroll_kind(*factor) + "{}", + not_unrolled_warning( + &unroll_kind(*factor), + "the annotation is not inside a recognizable loop", + hint_location(ctx, *op).as_deref(), + ) ); op.unlink(ctx); } @@ -268,13 +321,14 @@ pub fn unroll_annotated_loops( let Some(ph) = info.preheader(ctx, region, loop_id) else { // The author asked for unrolling; never silently do nothing. - for (op, block, _f) in &hints { - if info.innermost_loop(*block) == Some(loop_id) { - op.unlink(ctx); - } - } + let location = take_loop_hints(ctx, &hints, &info, loop_id); eprintln!( - "warning: {kind} requested but the loop was not unrolled: it has no single preheader (it is entered from more than one place)" + "{}", + not_unrolled_warning( + &kind, + "it has no single preheader (it is entered from more than one place)", + location.as_deref(), + ) ); continue; }; @@ -289,12 +343,11 @@ pub fn unroll_annotated_loops( continue; } CanonicalizeOutcome::Unsupported(reason) => { - for (op, block, _f) in &hints { - if info.innermost_loop(*block) == Some(loop_id) { - op.unlink(ctx); - } - } - eprintln!("warning: {kind} requested but the loop was not unrolled: {reason}"); + let location = take_loop_hints(ctx, &hints, &info, loop_id); + eprintln!( + "{}", + not_unrolled_warning(&kind, &reason, location.as_deref()) + ); continue; } } @@ -307,13 +360,14 @@ pub fn unroll_annotated_loops( let exiting = info.exiting_blocks(ctx, region, loop_id); let exits = info.exit_blocks(ctx, region, loop_id); if exiting.len() != 1 || exiting[0] != lp.header || exits.len() != 1 { - for (op, block, _f) in &hints { - if info.innermost_loop(*block) == Some(loop_id) { - op.unlink(ctx); - } - } + let location = take_loop_hints(ctx, &hints, &info, loop_id); eprintln!( - "warning: {kind} requested but the loop was not unrolled: partial unrolling does not yet support an early `break` or multiple exits" + "{}", + not_unrolled_warning( + &kind, + "partial unrolling does not yet support an early `break` or multiple exits", + location.as_deref(), + ) ); continue; } @@ -332,13 +386,14 @@ pub fn unroll_annotated_loops( continue; } CanonicalizeOutcome::Unsupported(reason) => { - for (op, block, _f) in &hints { - if info.innermost_loop(*block) == Some(loop_id) { - op.unlink(ctx); - } - } + let location = take_loop_hints(ctx, &hints, &info, loop_id); eprintln!( - "warning: {kind} requested but the loop was not unrolled: could not route its live-out values: {reason}" + "{}", + not_unrolled_warning( + &kind, + &format!("could not route its live-out values: {reason}"), + location.as_deref(), + ) ); continue; } @@ -357,13 +412,14 @@ pub fn unroll_annotated_loops( continue; } CanonicalizeOutcome::Unsupported(reason) => { - for (op, block, _f) in &hints { - if info.innermost_loop(*block) == Some(loop_id) { - op.unlink(ctx); - } - } + let location = take_loop_hints(ctx, &hints, &info, loop_id); eprintln!( - "warning: {kind} requested but the loop was not unrolled: could not normalize its back-edges: {reason}" + "{}", + not_unrolled_warning( + &kind, + &format!("could not normalize its back-edges: {reason}"), + location.as_deref(), + ) ); continue; } @@ -371,11 +427,7 @@ pub fn unroll_annotated_loops( // Consume this loop's hint(s) before any cloning so the markers are // never copied into the unrolled bodies. - for (op, block, _f) in &hints { - if info.innermost_loop(*block) == Some(loop_id) { - op.unlink(ctx); - } - } + let location = take_loop_hints(ctx, &hints, &info, loop_id); let rec = induction::analyze(ctx, &info, loop_id, ph); if verbose() { @@ -399,7 +451,10 @@ pub fn unroll_annotated_loops( // Requested but unsupported shape: report exactly why, loudly, so // it is never a silent no-op. UnrollOutcome::Skipped(reason) => { - eprintln!("warning: {kind} requested but the loop was not unrolled: {reason}"); + eprintln!( + "{}", + not_unrolled_warning(&kind, &reason, location.as_deref()) + ); } } } @@ -1632,7 +1687,15 @@ fn collect_hints( #[cfg(test)] mod tests { - use super::check_growth_budget; + use std::path::PathBuf; + + use super::{ + MirUnrollHintOp, check_growth_budget, hint_location, not_unrolled_warning, unroll_kind, + }; + use pliron::combine::stream::position::SourcePosition; + use pliron::context::Context; + use pliron::location::{Location, Source}; + use pliron::op::Op; #[test] fn growth_budget_counts_operations_not_only_blocks() { @@ -1643,4 +1706,72 @@ mod tests { "the exact operation limit remains allowed" ); } + + /// A `SrcPos` location like the importer attaches to a hint op. + fn src_pos(ctx: &mut Context, file: &str, line: i32, column: i32) -> Location { + Location::SrcPos { + src: Source::new_from_file(ctx, PathBuf::from(file)), + pos: SourcePosition { line, column }, + } + } + + #[test] + fn a_located_hint_op_adds_its_position_to_the_not_unrolled_warning() { + let cases = [ + ( + 0u32, + "kernel.rs", + 12, + 5, + "the annotation is not inside a recognizable loop", + ), + ( + 2u32, + "device/gemm.rs", + 3, + 27, + "partial unrolling does not yet support an early `break` or multiple exits", + ), + ]; + for (factor, file, line, column, reason) in cases { + let mut ctx = Context::new(); + let loc = src_pos(&mut ctx, file, line, column); + let hint = MirUnrollHintOp::new(&mut ctx, factor).get_operation(); + hint.deref_mut(&ctx).set_loc(loc); + + let location = hint_location(&ctx, hint).expect("a source position renders"); + let warning = not_unrolled_warning(&unroll_kind(factor), reason, Some(&location)); + let lines: Vec<&str> = warning.lines().collect(); + let case = format!("factor={factor}: {reason}"); + assert_eq!(lines.len(), 2, "{case}"); + assert!(lines[0].starts_with("warning: "), "{case}"); + assert!(lines[0].contains(reason), "{case}"); + let location_line = lines[1]; + assert!(location_line.starts_with(" --> "), "{case}"); + assert!( + location_line.ends_with(&format!("{file}:{line}:{column}")), + "{case}: {location_line}" + ); + } + } + + #[test] + fn a_hint_op_without_a_position_prints_only_the_reason_line() { + let mut ctx = Context::new(); + let hint = MirUnrollHintOp::new(&mut ctx, 0).get_operation(); + assert_eq!( + hint_location(&ctx, hint), + None, + "a fresh op has no position" + ); + + let reason = "it has no single preheader (it is entered from more than one place)"; + let warning = not_unrolled_warning( + &unroll_kind(0), + reason, + hint_location(&ctx, hint).as_deref(), + ); + assert!(!warning.contains('\n'), "{warning}"); + assert!(warning.ends_with(reason), "{warning}"); + } } diff --git a/cuda-oxide/cuda-oxide-book/compiler/compiler-optimizations.md b/cuda-oxide/cuda-oxide-book/compiler/compiler-optimizations.md index 9041a94ba6..3a2d6515af 100644 --- a/cuda-oxide/cuda-oxide-book/compiler/compiler-optimizations.md +++ b/cuda-oxide/cuda-oxide-book/compiler/compiler-optimizations.md @@ -303,7 +303,10 @@ first. To bound compile time and memory, one annotation may create at most 1,024 body copies, 8,192 cloned basic blocks, and 65,536 cloned operations. A request over -one of those limits warns and is not unrolled. +one of those limits warns and is not unrolled. Every warning about a loop that +was not unrolled names the `#[unroll]` attribute it refers to as +`file:line:column` on a `-->` line, so a kernel with several annotated loops +shows which one was left alone. For the complete kernel-author reference, see {ref}`Loop unrolling `. diff --git a/cuda-oxide/cuda-oxide-book/gpu-programming/kernels-and-device-functions.md b/cuda-oxide/cuda-oxide-book/gpu-programming/kernels-and-device-functions.md index 7fee6612fa..5b40e87048 100644 --- a/cuda-oxide/cuda-oxide-book/gpu-programming/kernels-and-device-functions.md +++ b/cuda-oxide/cuda-oxide-book/gpu-programming/kernels-and-device-functions.md @@ -352,6 +352,9 @@ unroll that loop. Partial unrolling also requires a counted-up loop: the counter must have a positive step, use `<` or `<=`, and compare against a limit that does not change inside the loop. The compiler warns and does not unroll unsupported requests. +Every warning about a loop that was not unrolled names the `#[unroll]` +attribute it refers to as `file:line:column` on a `-->` line, so a kernel with +several annotated loops shows which one was left alone. To keep generated code bounded, one annotation may create at most 1,024 body copies, 8,192 cloned basic blocks, and 65,536 cloned operations. A larger