The pre-commit CI crashes do not seem to relate to this patch. I will land it soon.

Addressing review comments. Thanks for the review @nemanjai !!

In D139720#3989651, @jamieschmeiser wrote:

I'm fine with handling the return for common as a separate change, if necessary.

Updating to revert back to const char * in the big StringSwitch.

Update to include a new test case against the unknown target error message. Thanks for the suggestion @amyk !

It is decided that we no longer need this patch.

Fix a clang-format issue in CommonArgs.cpp.

Address review comment. Thanks for the review!

In D138585#3953869, @reames wrote:

Can you explain why memory dependence wasn't enough to prevent the problematic reordering? I'm a bit fuzzy on the details - it's been a while - but I though that stacksave and stackrestore were modeled as modifying all memory to prevent this reordering?

@ABataev Thanks so much for the fast review! I will land the patch in a moment.

Ping for review. @reames could you also let me know if I should tag anyone else for a review? Thanks so much!

Updating code comment.

Rebase and cleanup some test cases.

Adding two test cases to match G_NOTUNING.

Address the review comment.

Adding one more test case.

@w2yehia @daltenty may I ask for a review? Thanks so much!

Address a review comment.

Clean up test cases.

Address code review.

Ah I just saw 5d3139aef113881fff9430c96069faf96630a5d8. Thanks @MaskRay !

This patch seems to be causing build failures when a build is configured with -Werror due to the warnings (e.g. https://lab.llvm.org/buildbot/#/builders/57/builds/22957). May I get your help @mingmingl for some advices/fixes to get the build going? Thank you!

Rebase with latest main.

Thanks for the review! Comment addressed.

In D134668#3841464, @MaskRay wrote:

Err since -plugin-opt= was used a lot of before this change, I think this is fine for consistency.

In D135400#3841321, @MaskRay wrote:

I missed https://reviews.llvm.org/D134668 . See my comment there I don't think the change is necessary.

In D134668#3841312, @MaskRay wrote:

I think I want to revert this change. --plugin-opt= is entirely fine with lld and GNU gold/ld... And generally the double-dash long options are preferred.

Fix a patch upload error.

Address review comments. Remove unnecessary Twines.

Address a code review comment.

Try fixing the test case on Windows, hopefully one last time.

Fix test case on Windows.

Fix the test failure on Windows.

Addressing code review comments and fixing a test failure on Linux. I will land patch once the pre-commit CI finishes.

Addressing code review comments.

Gentle ping for review. Thanks!

Updating function names so they are more explicit.

Minor formatting update.

Addressing code review:

Update the diff to include the context.

Does that help?

May I ask for some guidance/example on how to use the attributes to identify that the removed functions (e.g. malloc, calloc, realloc) are still allocation functions?

In D125482#3510333, @bzcheeseman wrote:

In D125482#3510332, @qiongsiwu1 wrote:

In D125482#3510328, @bzcheeseman wrote:

In D125482#3510327, @qiongsiwu1 wrote:

In D125482#3510322, @bzcheeseman wrote:

@qiongsiwu1 I assume you ran the whole test suite with no problems? My machine is rather slow so running check-all takes ~many hours for me

Good call - no I did not run the whole test suite. I did run ninja check-all. Let me give the test suite a try as well.

That's what I meant. I think build kite is running ninja check-clang-tools check-bolt check-mlir check-flang check-all check-lld check-openmp check-clang check-polly check-libc check-llvm

build kite is being flaky right now so I'd love a clean bill of health before landing to unblock folks.

Ah I see. I tested check-all, but I did not build flang/openmp/polly/libc for my local testing. Let me give those a go instead of running the "test-suite".

Many thanks - I assume your build machine is better than mine :)

I got check-clang and check-llvm on my machine and both are all good (which is usually a good sign)

In D125482#3510328, @bzcheeseman wrote:

In D125482#3510327, @qiongsiwu1 wrote:

In D125482#3510322, @bzcheeseman wrote:

@qiongsiwu1 I assume you ran the whole test suite with no problems? My machine is rather slow so running check-all takes ~many hours for me

Good call - no I did not run the whole test suite. I did run ninja check-all. Let me give the test suite a try as well.

That's what I meant. I think build kite is running ninja check-clang-tools check-bolt check-mlir check-flang check-all check-lld check-openmp check-clang check-polly check-libc check-llvm

build kite is being flaky right now so I'd love a clean bill of health before landing to unblock folks.

In D125482#3510322, @bzcheeseman wrote:

@qiongsiwu1 I assume you ran the whole test suite with no problems? My machine is rather slow so running check-all takes ~many hours for me

LGTM! Thanks!

In D125482#3509800, @bzcheeseman wrote:

@qiongsiwu1 or @uabelho please let me know if this fixes the issue you saw. I don't think I see the warning emitted anywhere I was able to repro.

In D123901#3508156, @uabelho wrote:

from
https://lab.llvm.org/buildbot/#/builders/57/builds/17829/steps/6/logs/stdio

I see the same thing compiling locally with clang 8.

This patch is updated to address

In D123943#3465282, @arsenm wrote:

In D123943#3465190, @qiongsiwu1 wrote:

Hi Matt @arsenm !

May I check with you why commit https://reviews.llvm.org/rG9209a519180b478f7a77d7c4781ea857536d77ed does not have a link to this patch?

Thanks!!

FYI @zibi

I don't use arc which adds those comments

In D123498#3465139, @rastogishubham wrote:

Hi, this patch causes an issue with the CMake Xcode build configuration, if I try to use xcode as the generator with CMake, using the build command:

I used git bisect to figure this out, is there some solution to this?

Hi Matt @arsenm !