Skip to content

[Fix][Kernel] convolution: fix Metax grouped conv and conv1d bias - #37

Open
ventijing wants to merge 1 commit into
MetaX-MACA:devfrom
ventijing:convolution_dev
Open

[Fix][Kernel] convolution: fix Metax grouped conv and conv1d bias#37
ventijing wants to merge 1 commit into
MetaX-MACA:devfrom
ventijing:convolution_dev

Conversation

@ventijing

Copy link
Copy Markdown

Grouped conv2d/conv3d kernels hit MACA TIR vectorization bugs
(make_longlong4); gate flat output store and disable_vectorize to
MACA only so CUDA behavior stays unchanged. Apply conv1d bias in
ops on MACA where in-kernel epilogue is unreliable. Use fp32 torch
ref for conv3d tests on MACA to match fp32 accumulation.

@github-actions github-actions Bot added the fix Auto-created by labeler label Jul 6, 2026

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Review

This pull request introduces MACA backend support for convolution operations, adding conditional logic to handle MACA-specific compilation flags, output flattening, and test reference programs. Notably, for 1D convolutions on MACA, in-kernel bias is disabled and fused during the forward pass instead. The review feedback highlights an opportunity to avoid redundant GPU tensor allocations on MACA by passing the validated bias directly to the kernel instead of None. Additionally, refactoring is suggested in the tests to eliminate duplicated F.conv3d calls by conditionally casting the inputs.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

)
_validate_tensor_shape("Conv1d", "bias", bias, (self.c_out,))
return self.kernel(input, weight, bias)
out = self.kernel(input, weight, None if is_maca() else bias)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

high

On MACA, self.kernel is initialized with has_bias=False, meaning the compiled kernel will not apply the bias in-kernel. However, passing None to self.kernel causes self.kernel.forward to allocate a new zero tensor via torch.zeros(self.c_out, ...) on every single forward pass.

Since bias is already validated and available, we can pass it directly to self.kernel instead of None. This avoids the redundant GPU tensor allocation in the hot path while still ensuring the bias is not applied in-kernel.

Suggested change
out = self.kernel(input, weight, None if is_maca() else bias)
out = self.kernel(input, weight, bias)

Comment on lines +780 to +794
if is_maca():
ref = F.conv3d(
x.float(), weight.float(), bias=None, stride=2, padding=2, dilation=2,
)
ref = ref.to(dtype=x.dtype).contiguous()
else:
ref = F.conv3d(
x,
weight,
bias=None,
stride=2,
padding=2,
dilation=2,
)
ref = ref.contiguous()

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

Refactor to avoid duplicating the F.conv3d call and its parameters.

    x_ref = x.float() if is_maca() else x
    weight_ref = weight.float() if is_maca() else weight
    ref = F.conv3d(
        x_ref,
        weight_ref,
        bias=None,
        stride=2,
        padding=2,
        dilation=2,
    )
    if is_maca():
        ref = ref.to(dtype=x.dtype)
    ref = ref.contiguous()

Comment on lines +836 to +849
if is_maca():
ref = F.conv3d(
x.float(), weight.float(), bias=bias.float(), stride=2, padding=1,
)
ref = ref.to(dtype=x.dtype).contiguous()
else:
ref = F.conv3d(
x,
weight,
bias=bias,
stride=2,
padding=1,
)
ref = ref.contiguous()

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

medium

Refactor to avoid duplicating the F.conv3d call and its parameters.

    x_ref = x.float() if is_maca() else x
    weight_ref = weight.float() if is_maca() else weight
    bias_ref = bias.float() if is_maca() else bias
    ref = F.conv3d(
        x_ref,
        weight_ref,
        bias=bias_ref,
        stride=2,
        padding=1,
    )
    if is_maca():
        ref = ref.to(dtype=x.dtype)
    ref = ref.contiguous()

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fix Auto-created by labeler

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant