In D94687#2498336, @jdoerfert wrote:

Some notable exceptions I think are exclusive loads and stores as well as the memory barrier intrinsics, for which nosync does not apply I think.

FWIW, that sounds about right.

Really happy people start to use this for target intrinsics as well.

In D92404#2436313, @mtrofin wrote:

Gentle reminder - @jdoerfert , is blank-allowing all the Transforms/Attributor to have unused prefixes "as intended"? (I remember you indicating that was the case)

Definitely not intentional, sorry.

Cool :)

In D90103#2410284, @abidmalikwaterloo wrote:

The tests should be in llvm-project/llvm/test/Transforms/OpenMP.

On the first look, you should at least clang-format this as it is not very readable right now.

In D89826#2348164, @nhaehnle wrote:

Oh, and on the change itself: I'm not familiar enough with the attributor framework to judge the implementation, but the described reasoning (being able to make deductions from indirect calls) is sound.

Commited in rGfbfb1c790982.

Switch to opt-in approach. Intrinsic class now has the same meaning as bofore.
To get default attributes we now use DefaultAttrsIntrinsic.

In D75384#2317444, @abidmalikwaterloo wrote:

I have been trying to update my patch by following the steps on:

https://llvm.org/docs/Contributing.html#how-to-submit-a-patch

However, I always end up loading nothing.

Only move the declaration

@nikic do you have a preference on any of the suggested names?

opt-out of few more intrinsics
add comment for DefaultIntrinsic class

update clang tests

In D86021#2303405, @nikic wrote:

I find the naming here confusing: Looking at this patch my expectation was that DefaultIntrinsic is used for intrinsics with default attributes, based on the name. Took me a while to understand it's actually the other way around. Shouldn't the Intrinsic <-> DefaultIntrinsic naming be inverted?

Opt-out of default attributes for llvm.coro.* intrinsics.

Actually coro intrinsics had default attributes, completelly missed that. Sorry for the confusion.
I'll update the patch shortly.

In D86021#2301868, @efriedma wrote:

We probably shouldn't be messing with the attributes on llvm.coro.* intrinsics. The rules for transforms around coroutine intrinsics aren't completely settled (see also https://reviews.llvm.org/D87817), but at least some of them can run arbitrary code.

precommit (local) coro test changes with update_test_checks

Adding @rjmccall as a reviewer as I'm unsure about some coroutine changes that appeared after rebase.

Rebase

Its been more than a month now. Are there any additional concern about this?

group default properties together

Switched to D88096.

Now I see where the problem was. Thanks for taking a look.

LGTM

I think you should split this in two patches. One that introduces the IR attribite and one for attributor deduction.

Sorry for the delay.

Looks like parallel_deletion.ll needs to be updated as well.

Looks same as the failures before. I may be able to do a windows build next week and try to see what is going on.

Actually windows tests passed, I was looking at the wrong place.

I remember this being windows only issue. I was able to always reproduce it on windows, but never had the chance to look into it.

LGTM, thanks!

Rebasing after recent changes to AANoUndef

Don't skip ;\n comments

Sorry for the delay on this.

Are you going to move the AAIsDead query to the update?

Thanks for the explanation. Since the patch is not small and it is a WIP, can you add some more information about the failing tests? Maybe some specific examples?

In D86565#2237287, @jdoerfert wrote:

LGTM, one minor note. The test will be part of the module slice patch, I'm fairly certain this cannot be tested right now but it makes sense.

What should be the next step here? There was no response on the RFC update.

Does the fix for the performance issue seem ok?

LGTM, thanks!

LGTM.

In D86319#2229661, @kuter wrote:

This is interesting, I thought you couldn't change the IR outside of the current SCC.

We can't delete functions outside of the SCC, the rest should be fine.

In D70365#2227627, @mehdi_amini wrote:

After this patch, llvm-tblgen in debug mode is 10x slower:
Before:

$ time bin/llvm-tblgen -gen-global-isel -I ../llvm/test/TableGen/../../include -I ..//llvm/test/TableGen/Common -optimize-match-table=true ../llvm/test/TableGen/GlobalISelEmitter.td -o /tmp/test.cpp

real	0m1.238s
user	0m0.970s
sys	0m0.274s

After:

$ time bin/llvm-tblgen -gen-global-isel -I ../llvm/test/TableGen/../../include -I ..//llvm/test/TableGen/Common -optimize-match-table=true ../llvm/test/TableGen/GlobalISelEmitter.td -o /tmp/test.cpp

real	0m11.671s
user	0m11.607s
sys	0m0.067s

Thanks for reporting this.
Numbers are similar in the release mode as well. The problem was that we go through all IntrProperties for every Intrinsic in search for ones that are default. However that can be done only once.

fix slowdown issue

Maybe the commit message could be a bit more descriptive?

addressing comments

addressing comments

update affected tests.

add comment for DisableDefaultAttributes flag.

Ping.
Can we move forward with this approach?

update gc intrinsics to DefaultIntrinsic