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

[llvm] applying Floating-Point Numerical Sanitizer patch #9201

Open
wants to merge 2 commits into
base: IB/CMSSW_15_0_X/master
Choose a base branch
from

Conversation

smuzaffar
Copy link
Contributor

Added llvm Floating-Point Numerical Sanitizer llvm/llvm-project#85916

FYI @VinInn

@smuzaffar
Copy link
Contributor Author

please test

@cmsbuild
Copy link
Contributor

A new Pull Request was created by @smuzaffar for branch IB/CMSSW_14_1_X/master.

@aandvalenzuela, @smuzaffar, @iarspider can you please review it and eventually sign? Thanks.
@sextonkennedy, @antoniovilela, @rappoccio you are the release manager for this.
cms-bot commands are listed here

@cmsbuild
Copy link
Contributor

cmsbuild commented May 21, 2024

cms-bot internal usage

@cmsbuild
Copy link
Contributor

-1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-0d3956/39452/summary.html
COMMIT: 161fe72
CMSSW: CMSSW_14_1_X_2024-05-20-2300/el8_amd64_gcc12
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmsdist/9201/39452/install.sh to create a dev area with all the needed externals and cmssw changes.

External Build

I found compilation warning when building: See details on the summary page.

@smuzaffar
Copy link
Contributor Author

please test

@cmsbuild
Copy link
Contributor

Pull request #9201 was updated.

@cmsbuild
Copy link
Contributor

-1

Summary: https://cmssdt.cern.ch/SDT/jenkins-artifacts/pull-request-integration/PR-0d3956/39453/summary.html
COMMIT: 40209eb
CMSSW: CMSSW_14_1_X_2024-05-20-2300/el8_amd64_gcc12
User test area: For local testing, you can use /cvmfs/cms-ci.cern.ch/week0/cms-sw/cmsdist/9201/39453/install.sh to create a dev area with all the needed externals and cmssw changes.

External Build

I found compilation error when building:

patching file llvm/test/Instrumentation/NumericalStabilitySanitizer/basic.ll
patching file llvm/test/Instrumentation/NumericalStabilitySanitizer/cfg.ll
patching file llvm/test/Instrumentation/NumericalStabilitySanitizer/invoke.ll
patching file llvm/test/Instrumentation/NumericalStabilitySanitizer/memory.ll
patching file llvm/test/Instrumentation/NumericalStabilitySanitizer/non_float_store.ll
error: Bad exit status from /data/cmsbld/jenkins/workspace/ib-run-pr-tests/testBuildDir/tmp/rpm-tmp.hXZZMU (%prep)


RPM build errors:
line 37: It's not recommended to have unversioned Obsoletes: Obsoletes: external+llvm+17.0.3-18bbc4ca84d2b4b92fa29e7ab26ed099
Macro expanded in comment on line 368: %{pkginstroot}/lib64


@VinInn
Copy link

VinInn commented May 22, 2024

Is the patch broken?

@fwyzard
Copy link
Contributor

fwyzard commented May 22, 2024

I see that the upstream PR (llvm/llvm-project#85916) is still under review, with various changes being applied.

@smuzaffar
Copy link
Contributor Author

yes, it is broken. Though I have updated it ( e.g. cms-externals/llvm-project#15 ) to be patched on top of llvm 18.1.6 but the command ninja check-nsan is not yet implemented . It fails at build time

Total Discovered Tests: 2812
Unsupported      :    8 (0.28%)
Passed           : 2802 (99.64%)
Expectedly Failed:    2 (0.07%)
+ ninja -v -j 16 check-nsan
ninja: error: unknown target 'check-nsan', did you mean 'check-lsan'?

@alexander-shaposhnikov
Copy link

just in case - my original PR got split (per reviewers' request), currently llvm/llvm-project#85916 contains the instrumentation pass only, the newly added tests can be run via ninja check-llvm-instrumentation.

@VinInn
Copy link

VinInn commented May 24, 2024

@alexander-shaposhnikov thanks for the info.
Does it mean that the UI (and notification?) is not yet present?
Can it be used to instrument a user program and get a report of "errors"?

@alexander-shaposhnikov
Copy link

alexander-shaposhnikov commented May 30, 2024

@VinInn - one can give it a try with a few caveats: the tool is very experimental, the coverage is incomplete (most notable blind spots are vector intrinsics and half precision). If you'd like - I can put together a combined (updated) version similar to my initial PR with all the components (instrumentation pass, runtime (compiler-rt), frontend/driver (clang)) and push it into my github repo. In this case you'll be able to run all the in-tree numerical tests via check-nsan and experiment with it by passing -fsanitize=numerical to Clang

@VinInn
Copy link

VinInn commented May 30, 2024

Thanks @alexander-shaposhnikov for the very complete answer.
for what concern the "private" updated version that would be interesting if the official version will take a while to concretize.
So please, if you have the time, go ahead with a updated version in your repository.

Thanks

@iarspider iarspider changed the base branch from IB/CMSSW_14_1_X/master to IB/CMSSW_14_2_X/master August 27, 2024 08:23
@smuzaffar smuzaffar changed the base branch from IB/CMSSW_14_2_X/master to IB/CMSSW_15_0_X/master November 22, 2024 13:34
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.

5 participants