Rebase

Rebase

In D100486#2821347, @lebedev.ri wrote:

No i didn't.
I think while this may be somewhat correct,
is not really correct. For example, before AVX,
there is no non-32-bit shuffles, only unpacks.

Rebase

LG

In D102107#2823706, @jdoerfert wrote:

In D102107#2821976, @ABataev wrote:

We used this kind of codegen initially but later found out that it causes a large overhead when gathering pointers into a record. What about hybrid scheme where the first args are passed as arguments and others (if any) are gathered into a record?

I'm confused, maybe I misunderstand the problem. The parallel function arguments need to go from the main thread to the workers somehow, I don't see how this is done w/o a record. This patch makes it explicit though.

We used this kind of codegen initially but later found out that it causes a large overhead when gathering pointers into a record. What about hybrid scheme where the first args are passed as arguments and others (if any) are gathered into a record?

Rebase + address a comment

LG

I would add an option to control whether to allow it or not.

Rebase + fixed gathering cost calculation.

There are still regressions, even after we allowed reordering of insertelements. It is because the reordering is not quite effective. I have an idea of how to improve it (and avoid rebuilding the tree for the second time and improve compile time), will try to implement it next week.

Rebase

LG

Rebase

LG, but add an extra description in summary

LG

LG

LG

In D101109#2807872, @RKSimon wrote:

rebase?

Ping

Rebase

LG

Tests?

I assume the proper fix is to handle declare variants in checkUndefinedButUsed in Sema.cpp. This should fix both PRs, mentioned here

In D103529#2802564, @zahiraam wrote:

In D103529#2802350, @ABataev wrote:

In D103529#2802346, @zahiraam wrote:

Review anyone please? Thanks.

Add more description to the summary. Would be good to see an analysis, why do we get this error message.

The warning warning: function 'foo' has internal linkage but is not defined

[-Wundefined-internal]

is generated at https://github.com/llvm/llvm-project/blob/main/clang/lib/Sema/Sema.cpp#L814

In ActoOnstartFunctionDefinitioninOpenMPDeclareVariantScope there is new Decl that is created here https://github.com/llvm/llvm-project/blob/main/clang/lib/Sema/SemaOpenMP.cpp#L6685. This Decl is added to the UndefinedButUsed map here https://github.com/llvm/llvm-project/blob/main/clang/lib/Sema/SemaExpr.cpp#L17118
This newly created Decl was marked 'imiplicit' but not marked 'used' as expected here https://github.com/llvm/llvm-project/blob/main/clang/include/clang/AST/DeclBase.h#L578.

In D102834#2802402, @fhahn wrote:

In D102834#2771053, @ABataev wrote:

You can easily get the first instruction - VectorizableTree.front(). Stores can be only in the front of the vectorizable tree. As to the last instruction, I think we need to scan all tree entries and find all gather nodes with memaccess which may alias with stores. The main problem that the versioning better to perform before we try to build the tree, otherwise we may decide to gather some instructions instead of trying vectorizing them because of possible aliasing. And it may not be profitable.

I updated the patch to only collect possible bounds for versioning first and queue basic blocks for which we found bounds for re-processing. When re-processing those blocks, we create a versioned block with the appropriate !noalias metadata and re-run vectorization (for now just seeded by stores). As as, this should be correct, but may not be optimal, because either:
a) there were issues preventing vectorization other than aliasing, which may cause us to not vectorize anything in the versioned block
b) the runtime checks are too expensive and offset the gain from additional vectorization.

We should be able to solve both by comparing the cost of the versioned & non-versioned BB after vectorization. If versioning is not profitable, we can remove the conditional branch again. What do you think of this direction?

In D103529#2802346, @zahiraam wrote:

Review anyone please? Thanks.

Address comments.

Address comments, better analysis of gathering order.

1. Emit __kmpc_cancel_barrier only for parallel cancellation.
2. Synced CreateCancel.

LG

LG

Rebase

Rebase

Rebase

LG

LG

In D102920#2791168, @nikic wrote:

FYI there seems to be some (but not very large) compile-time impact on ClamAV: https://llvm-compile-time-tracker.com/compare.php?from=e60f147324b64f7740de58e6b936cdc0e26daadd&to=36911971a58d1ba8b15e97790ac816eaadb0603e&stat=instructions

The file with the largest impact is libclamav_htmlnorm.c with 3.4% regression in NewPM-O3 (and 5.6% in NewPM-ReleaseLTO-g). Might be worth taking a look.

In D102920#2791168, @nikic wrote:

FYI there seems to be some (but not very large) compile-time impact on ClamAV: https://llvm-compile-time-tracker.com/compare.php?from=e60f147324b64f7740de58e6b936cdc0e26daadd&to=36911971a58d1ba8b15e97790ac816eaadb0603e&stat=instructions

The file with the largest impact is libclamav_htmlnorm.c with 3.4% regression in NewPM-O3 (and 5.6% in NewPM-ReleaseLTO-g). Might be worth taking a look.

Rebase

In D101109#2771153, @RKSimon wrote:

Have you been able to investigate any of these instruction increase regressions?