@awarzynski, @sscalpone added the rule in C++Style.md in fdb58597e859. Perhaps Steve could explain the motivation for deviating from Clang's style guide (https://llvm.org/docs/CodingStandards.html#error-and-warning-messages)? IMO Flang should behave more consistently with Clang, especially if we start sharing more code between the two frontends going forward.

@erikjv Are you still working on this? Or is a better alternative being pursued? IIUC, this functionality is still missing in trunk.

Apologies for the bug and thanks @hliao for the quick fix.

Thanks Sanjay.

Pinging reviewers.

Removed the change to the default value of EarlyCSEDebugHash as per @lebedev.ri's suggestion.

Fixed linter warning.

In D86843#2246700, @lebedev.ri wrote:

In D86843#2246699, @bryanpkc wrote:

In D86843#2246545, @lebedev.ri wrote:

1. As a preparatory patch, we should adjust every test that uses EarlyCSE to also specify -earlycse-debug-hash, at least to those where it doesn't result in an assertion

Thanks for your suggestions, Roman. I have added the reproducer to the EarlyCSE tests, next to other similar test cases. Regarding your first suggestion, do you mean that we should enable -earlycse-debug-hash in all tests under llvm/test/Transforms/EarlyCSE/, or even all tests under llvm/test/ that may implicitly use EarlyCSE?

At least for every test that explicitly uses -early-cse.

That seems like overkill.

OTOH these bugs aren't fun, and they should be trivially caught by existing tests, since the test coverage should be good anyways.

In D86843#2246545, @lebedev.ri wrote:

1. As a preparatory patch, we should adjust every test that uses EarlyCSE to also specify -earlycse-debug-hash, at least to those where it doesn't result in an assertion

Added a regression test using the -earlycse-debug-hash option.

Cleaned up unused code.

Moved the fix into matchSelectWithOptionalNotCond instead of duplicating code.

In D80801#2102291, @efriedma wrote:

We are only pulling the trailing addition in with an existing multiply.

The problem here is that it's the "wrong" multiply: you have, essentially (A*B+D*E)+F, and you're turning it into A*B+(D*E+F). I don't think contraction is supposed to cover that.

Sorry for the late question, but I don't understand why this kind of folding is not considered reassociation. I thought reassociation was not allowed even when -ffp-contract=fast.

In D77603#1978909, @AndreyChurbanov wrote:

Yes, ideally barriers of all types should have these extra synchronizations, though with lesser priority I think. Because other barriers do not work by default, they need to be explicitly requested.

In D77603#1977658, @AndreyChurbanov wrote:

Sorry, found one more issue.

I think the first MB should be moved inside the block:

for (level...
  if (((tid...

Pair the memory barriers correctly at the same nesting level within the loop.

Add a paired memory barrier to the child thread's path after it finishes writing to its own data and before releasing the parent thread.

In D77603#1970402, @AndreyChurbanov wrote:

I don't see paired memory barrier in a child thread between assigning th.th_local.reduce_data in __kmp_barrier_template() and releasing b_arrived barrier flag that frees parent to go to reduce data.

@Hahnfeld Thanks for the explanation!

In D76780#1942144, @bryanpkc wrote:

Why doesn't the KMP_MB after the store to th_next_waiting guarantee that the unblocked thread sees that store?

In D76780#1941764, @Hahnfeld wrote:

Yes, this looks related to memory consistency. As far as I understand the threads synchronize on th_spin_here, so this is guaranteed to be updated. Any other write before this in __kmp_release_queuing_lock is not guaranteed to be synchronized by a weak memory model. This includes th_next_waiting (which triggers the assertion), but also writes by the user application. That's particularly bad because this should be taken care of by the runtime!

Removed unrelated changes from this patch.

Pinging reviewers.

Add tests for -fopenmp-targets=.

In D70804#1763289, @ABataev wrote:

Tests are required

Avoid reading CTR_EL0 on every call to __clear_cache.

I actually read that LKML post before submitting the patch, and I had the same understanding as @ktkachov. I will change the code to cache the result of the MRS instruction as suggested.

Pinging reviewers.

In D53927#1399283, @steleman wrote:

In D53927#1399263, @rengolin wrote:

Thanks Stephan, now looks good.

Did you see the comment on lgamma coming after isoc99? I wonder if that's reproducible in all systems...

Yup I saw it. But I don't get any errors here (Ubuntu 16.04 / Ubuntu 18.something) AArch64.

And it's not clear to me how that sorted alphabetical order is supposed to work. There's a bunch of other functions that come after lgamma/tgamma that aren't sorted.

For example:

/// char * __strtok_r(char *s, const char *delim, char **save_ptr);
TLI_DEFINE_ENUM_INTERNAL(dunder_strtok_r)
TLI_DEFINE_STRING_INTERNAL("__strtok_r")
/// int abs(int j);
TLI_DEFINE_ENUM_INTERNAL(abs)
TLI_DEFINE_STRING_INTERNAL("abs")
/// int access(const char *path, int amode);
TLI_DEFINE_ENUM_INTERNAL(access)
TLI_DEFINE_STRING_INTERNAL("access")

Is there any plan to fix this patch and land it? We use --max-time in our tests and intermittently run into the Python assertion. We can work around the problem by killing lit.py with another timer, but then we lose the report that lit.py produces at the end of the run.

Rebased on trunk.

Removed trailing spaces. NFC.

Pinging reviewers.

Thanks Kristof!

Pinging reviewers....

Added a unit test for lib/Support/Host.cpp and removed some redundant tests.

In D53633#1274621, @t.p.northover wrote:

I think this is reasonable.

Updated the patch to avoid using monorepo path names.

Pinging reviewers...

Removed impossible code path from NormalizeBuildVector.