Fixed! thanks

Remove unneeded Ret value check.

Remove unused parameter.

@RKSimon suggested the transformation might be caused by unneeded trunc/zext operations.
It is causing the DAG combiner to needlessly sink them under the OR operations so I have removed them where possible and this fixes the infinite loop.
The instructions generated end up being reordered is some cases but I think it is equivalent.

In D122900#3422413, @lebedev.ri wrote:

In D122900#3422365, @spatel wrote:

In D122900#3422344, @pgousseau wrote:

In D122900#3422266, @lebedev.ri wrote:

Do you know which commit introduced the problem, or which transforms conflict?
This looks like a pretty heavy hammer.

Thank you Roman for reviewing!
The regression appeared after https://github.com/llvm/llvm-project/commit/8a156d1c2795189389fadbf33702384f522f2506 (thank you @wristow for finding it)
As @RKSimon found out, the IR does not get generated anymore from the original C++ test case but the issue is still present.
I am hopeful we can reintroduce the optimization (at a later codegen stage maybe?) but I would like to do some performance analysis first to see if it is still a win.

That change might have exposed the bug from source/IR, but the backend loop has existed since at least the 10.0 release based on a godbolt check:
https://godbolt.org/z/Tfnaevboz

Then i think just fixing it may be better than removing some chunks of code.

In D122900#3422266, @lebedev.ri wrote:

Do you know which commit introduced the problem, or which transforms conflict?
This looks like a pretty heavy hammer.

Added a regression test.

Abandoned in favour of https://reviews.llvm.org/D118624

Add comment as suggested.

Hoist up __has_include ifdef guard as per comment.

I have made an alternative patch at https://reviews.llvm.org/D118624 to avoid adding all internal crt sources to the header search paths. Not ideal either, let me know what you think, thanks!

Updating patch following comments, using __GNUC__ instead of !_WIN32 which was wrong.

In D118162#3272215, @aaron.ballman wrote:

I'm a bit confused as to why this change is needed. My understanding is that __SANITIZE_ADDRESS__ is only defined when the user passes -fsanitize=address (so they explicitly opted in). So isn't the issue here that the user asked for something unsupported and the solution is for them to not pass that compiler argument on Windows? Alternatively, should the fix be to require __has_feature(leak_sanitizer) to return true instead of using ||?

(I'm a bit worried that we're carving out an exception for something that's feature testable and when the feature eventually gets supported on Windows, nobody will remember to go remove the special exceptions.)

LGTM!

Lgtm! Thank you for the quick fix!

LGTM!

Thank you vitalybuka eugenis for reviewing! I have pushed the change at https://reviews.llvm.org/rG27830bc2b1b8
Sorry I forgot to add the reviewers and phabricator url to the commit message -_-

In D95859#2575618, @vitalybuka wrote:

In D95859#2568230, @pgousseau wrote:

In D95859#2568106, @vitalybuka wrote:

I guess odr-indicator=1 is for reliable ODR violation detection and odr-indicator=0 of smaller binary size.

Thank you for the comment, yes, this change can potentially break the assumption that odr-indicator=0 reduces binary size, as it will prevent certain linker (eg bfd 2.30), to dead strip unused asan globals’ metadata. Do I understand your point correctly?

But I think correct semantic should take priority over binary size optimizations, please let me know your thoughts on this.

Sanitizers don't mind false-negatives if it can noticeably help with some performance metrics. But I can't reproduce expected size regression.

In D95859#2568106, @vitalybuka wrote:

I guess odr-indicator=1 is for reliable ODR violation detection and odr-indicator=0 of smaller binary size.

Ping

Ping

fix lint issue

• Always use MWAITX_DAG
• Remove uneeded Glue

Updated the patch to handle the case described in PR42064 where a separate basic block demands the use of a base pointer.
In that case we do the codegen of mwaitx during custom insertion.

In D77393#1969419, @probinson wrote:

It looks like you're not actually interested in the compiled output, but just whether warnings occurred; in that case you'd be better off with -verify -fsyntax-only and // expected-no-diagnostics.

Fix false negative test introduce while attempting to add label check.

Reopening to fix false negative test.

Added label

Renaming test and including x86intrin.h instead.

In D73475#1843805, @craig.topper wrote:

Is this subject to the same problem as PR42064?

In D62245#1526733, @jdoerfert wrote:

I think adding all these comments and updating the format is a great thing!

Minor comments to consider:

• I thought we used backslash instead of @ now but I might be wrong. The coding standard uses backslashs, if compiler-rt is not using @ all over the place you should change it.
• Mention in the commit message that you also add descriptions to undocumented functions.
• Mention in the commit message that you improved spelling, grammar, and formatting as well.

I'm not a compiler-rt person but if there is no objection in the next few days, you should go ahead (LGTM).

Replaced doxygen's '@' character by '\'

Ping!

Ping!

In D59815#1442868, @probinson wrote:

This is fine for PS4, I'm just curious whether anyone knows if there's a predicate that means something like "target does not use GNU tools" so we don't have to itemize Fuschia and PS4 this way.

Update patch to reflect changes to the llvm side of the patch.

In D59220#1443749, @vitalybuka wrote:

What is exactly going to be checked by -fsanitize=pointer-compare and -fsanitize=pointer-subtract?
Why this needs to be done in AddressSanitizer?
Is this going just checheck that pointers are from the same allocation?

Use backend option directly rather than introducing new parameters.
Add missing ALL check to test.

Ping!

In D57914#1416215, @jyknight wrote:

The intricate initialization-order workarounds apparently don't work in all build modes, so I've updated this code to have constexpr functions and initializations in rL355278.

Change hash_value() declaration's location as suggested.

move hash_value declaration to clang's namespace to solve lldb cmake bot build error.

Reopening because of buildbot failure http://green.lab.llvm.org/green/job/lldb-cmake/20537/