[Fix][Kernel] convolution: fix Metax grouped conv and conv1d bias - #37
[Fix][Kernel] convolution: fix Metax grouped conv and conv1d bias#37ventijing wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
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) |
There was a problem hiding this comment.
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.
| out = self.kernel(input, weight, None if is_maca() else bias) | |
| out = self.kernel(input, weight, bias) |
| 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() |
There was a problem hiding this comment.
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()| 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() |
There was a problem hiding this comment.
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()
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.