Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WA] Disable MLIR when building debug to workaround sanitizer issue #3200

Merged
merged 1 commit into from
Aug 16, 2024

Conversation

BrianHarrisonAMD
Copy link
Collaborator

Update the build pipeline to disable compiling with MLIR when building debug builds.

See relevant PR #3181, PR #3198, and Issue #3192 for additional details.

After a discussion on PR #3198, we want to disable MLIR instead of suppressing the UBSan errors caused by MLIR.

The issue can be tracked down to simply creating the MLIR handle after upgrading to the latest ROCm 6.2.
Since we froze MLIR to an older release, suppressing this warning is our only real option.

Note:

Params used when constructing the MLIR handle to replicate, "--x2 1 --operation conv2d --kernel_id 0 --num_cu 104 --arch amdgcn-amd-amdhsa:gfx90a:sramecc+:xnack- --groupsize 1 --fil_layout GNCHW --fil_type fp32 --in_layout NGCHW --out_layout NGCHW --in_type fp32 --out_type fp32 --batchsize 100 --in_channels 25 --out_channels 300 --in_h 32 --in_w 32 --out_h 30 --out_w 30 --fil_h 3 --fil_w 3 --dilation_h 1 --dilation_w 1 --conv_stride_h 1 --conv_stride_w 1 --padding_h 0 --padding_w 0 --kernel_name mlir_gen_igemm_conv2d_v4r4_fwd_xdlops0". Simply creating a handle with those command line arguments will result in the vptr issue when the test application is exiting after upgrading to ROCm 6.2.
ROCm 6.2 also updated the version of clang.

Copy link
Contributor

@amberhassaan amberhassaan left a comment

Choose a reason for hiding this comment

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

Looks good.

@BrianHarrisonAMD
Copy link
Collaborator Author

@atamazov Here is the changed version to disable MLIR if you get a chance to review.

Thanks!

@junliume junliume merged commit 4be2a03 into develop Aug 16, 2024
143 checks passed
@junliume junliume deleted the bharriso/disable-mlir-for-debug-builds-workaround branch August 16, 2024 23:54
Copy link
Contributor

@atamazov atamazov left a comment

Choose a reason for hiding this comment

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

This is good.

Comment on lines +40 to +44
// WORKAROUND_ISSUE_3192 Disabling MLIR for debug builds since MLIR generates sanitizer errors.
if (build_type_debug || code_conv_enabled)
{
mlir_args = " -DMIOPEN_USE_MLIR=OFF"
}
Copy link
Contributor

Choose a reason for hiding this comment

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

[Notice] @BrianHarrisonAMD This overrides value of "mlir_build", even if it is provided explicitly.

Recommended way:

def mlir_build_default = "ON"
if (build_type_debug || code_conv_enabled) {
    mlir_build_default = "OFF"
}
def mlir_args = " -DMIOPEN_USE_MLIR=" + conf.get("mlir_build", mlir_build_default)

However, current code is acceptable as a workaround.

/cc @junliume

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

Successfully merging this pull request may close these issues.

4 participants