Skip to content

Conversation

@h-sadia
Copy link
Contributor

@h-sadia h-sadia commented Jan 8, 2026

This is the last part of enabling dropout as request from GRAPH API. This will enable SDPA on the fast path.

@h-sadia h-sadia requested a review from a team as a code owner January 8, 2026 20:58
@github-actions github-actions bot added the platform:gpu-intel Codeowner: @oneapi-src/onednn-gpu-intel label Jan 8, 2026
const bool with_wei_scales
= !attr_scales.has_default_values(DNNL_ARG_WEIGHTS);
const bool with_dst_scales = !attr_scales.has_default_values(DNNL_ARG_DST);
const bool with_dropout = !attr()->dropout_.has_defaul_values();
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
const bool with_dropout = !attr()->dropout_.has_defaul_values();
const bool with_dropout = !attr()->dropout_.has_default_values();

Comment on lines 265 to 261
kernel_ctx.define_int("USE_OFFSET", use_offset);
kernel_ctx.define_int("USE_HOST_SCALARS", use_host_scalars);
kernel_ctx.define_int("HAS_OUTPUT_MASK", has_output_mask);
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you name these DROPOUT_* so it's clearer in the kernel what's being controlled by these macros? Specifically, USE_HOST_SCALARS is confusing as there are other scalars that may be supplied on the host or device side.

Copy link
Contributor Author

@h-sadia h-sadia Jan 9, 2026

Choose a reason for hiding this comment

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

They all actually come withing the WITH_DROPOUT macro, so it is going to be pretty clear. But I added the DROPOUT_* to HOST_SCALARS for clarity

Comment on lines 340 to 337
arg_list.set(idx++, GEMM_CTX_ARG_STORAGE(dropout_seed));
arg_list.set(idx++, GEMM_CTX_ARG_STORAGE(dropout_offset));
arg_list.set(idx, GEMM_CTX_ARG_STORAGE(dropout_prob));
Copy link
Contributor

Choose a reason for hiding this comment

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

I think these need to be set in gpu/intel/matmul/gemm.cpp similarly to the other gemm:exec_args_t values.

@h-sadia h-sadia force-pushed the hsadia/drpout_jit_path branch 8 times, most recently from 131cfc2 to f4fabd4 Compare January 9, 2026 06:14
@h-sadia h-sadia requested a review from a team as a code owner January 9, 2026 06:14
@h-sadia h-sadia force-pushed the hsadia/drpout_jit_path branch 2 times, most recently from 3b947f2 to 83991a2 Compare January 9, 2026 20:49
@h-sadia h-sadia force-pushed the hsadia/drpout_jit_path branch from 83991a2 to bc6ff50 Compare January 9, 2026 20:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

component:common platform:gpu-intel Codeowner: @oneapi-src/onednn-gpu-intel

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants