Skip to content

Reapply "[msan] Add avx512-intrinsics.ll and avx512-intrinsics-upgrade.ll test case (#123980)" #124500

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

Merged
merged 8 commits into from
Jan 28, 2025

Conversation

thurstond
Copy link
Contributor

@thurstond thurstond commented Jan 27, 2025

This reverts commit b2647ff i.e., relands 980e86f.

I had reverted the original patch because of buildbot failures (e.g., https://lab.llvm.org/buildbot/#/builders/13/builds/4938/steps/5/logs/FAIL__LLVM__avx512-intrinsics-upgrade_ll). This reland addresses the likely root cause; namely, that the target datalayout had not been specified.

This is forked from llvm/test/CodeGen/X86/avx512-intrinsics-upgrade.ll.
It tests intrinsics that LLVM "auto-upgrades"; for example,
@llvm.x86.avx512.mask.store is converted into @llvm.masked.store (which has the interesting side effect that MemorySanitizer can already handle it via its existing handleMaskedStore).
Copy link

github-actions bot commented Jan 27, 2025

✅ With the latest revision this PR passed the undef deprecator.

@vitalybuka
Copy link
Collaborator

I had reverted the original patch because of buildbot failures (e.g., https://lab.llvm.org/buildbot/#/builders/13/builds/4938/steps/5/logs/FAIL__LLVM__avx512-intrinsics-upgrade_ll). This reland addresses the likely root cause; namely, that the target datalayout had not been specified.

It can be more convenient to review reland, it consist of 2 commits, "revert of revert", and fixes
however, if fix is trivial, no need to request a new review

Is possible to fix "undef deprecator" ?

@thurstond thurstond force-pushed the reland_msan_avx512_tests branch from 7736f61 to 8555d06 Compare January 27, 2025 08:11
@thurstond
Copy link
Contributor Author

I had reverted the original patch because of buildbot failures (e.g., https://lab.llvm.org/buildbot/#/builders/13/builds/4938/steps/5/logs/FAIL__LLVM__avx512-intrinsics-upgrade_ll). This reland addresses the likely root cause; namely, that the target datalayout had not been specified.

It can be more convenient to review reland, it consist of 2 commits, "revert of revert", and fixes however, if fix is trivial, no need to request a new review

I've split the commit into "revert of revert" + the fix.

Is possible to fix "undef deprecator" ?

Since these are MSan tests, and we want to know how MSan deals with uninitialized memory, undef seems appropriate?

@vitalybuka
Copy link
Collaborator

Since these are MSan tests, and we want to know how MSan deals with uninitialized memory, undef seems appropriate?

Undef handling is simpler and have dedicated tests.
Here we'd like to see how msan propagate shadow, parameter is actually better.

@thurstond thurstond force-pushed the reland_msan_avx512_tests branch from 8555d06 to 3e0deee Compare January 28, 2025 22:20
@thurstond
Copy link
Contributor Author

Since these are MSan tests, and we want to know how MSan deals with uninitialized memory, undef seems appropriate?

Undef handling is simpler and have dedicated tests. Here we'd like to see how msan propagate shadow, parameter is actually better.

Fixed in 70668af, d0fab84 and 3938a45

@thurstond
Copy link
Contributor Author

Failing tests in CI (https://buildkite.com/llvm-project/github-pull-requests/builds/141497) are unrelated:

LLVM.CodeGen/AArch64/aarch64-build-attributes-all.ll
LLVM.CodeGen/AArch64/aarch64-build-attributes-bti.ll
LLVM.CodeGen/AArch64/aarch64-build-attributes-gcs.ll
LLVM.CodeGen/AArch64/aarch64-build-attributes-pac.ll
LLVM.CodeGen/AArch64/aarch64-build-attributes-pauthabi.ll

@thurstond thurstond merged commit b8cdc5e into llvm:main Jan 28, 2025
5 of 7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants