lgtm, apart from clang tests. Someone else should take a look as well, though.

cover "maybe written" by another unkown call case and add test for that.

addressing comments

In D85052#2188697, @jdoerfert wrote:

This is still "intra-procedural", correct? Or can it replace across function boundaries too?

We are now able to check if call changes an ICV through function attribute. Follow up should make it possible to get an ICV value for a call.

I see you reverted this twice. Just curious what happened?

Just in case you haven't seen already, clang/test/OpenMP/barrier_codegen.cpp needs to be updated as well.

In D83744#2154384, @kuter wrote:

I inspected the output that opt generates and it makes sense.
also the script's output (included in the patch) also makes sense the.

I will double check everything but this is weird.
File check fails to match the first check line.

I am not sure we can help you much without the actual failure output.

In D83744#2153411, @kuter wrote:

merge with D83299
add positive and negative tests.

@jdoerfert I have a problem with this patch even though I auto generate tests, llvm-lit fails.
Is this a bug with the update_test_checks.py script ?

add a test

In D82861#2146242, @okura wrote:

In D82861#2137685, @jdoerfert wrote:

Can you merge this?

Do I have a right to merge this by myself? I did arc patch and tried to git push https://github.com/llvm/llvm-project.git HEAD:master according to the document, but I failed to do that.

remove unnecessary flag, fix update_cc_test_checks

fixing the issue with dead uses.

Thanks for figuring this out!

LGTM

Can you add a test using this option?

small fix

Since this is not a small change, I think it would be good if @hoyFB could test if this resolves the issue.

In D83172#2131779, @bbn wrote:

Attributes are going to be decleared outside of the Attributor soon.

Do you mean that there might be other abstract attributes defined outside the AttributorAttributes.cpp ?

As of yesterday, there is an AA outside of AttributorAttributes :). You should rebase.

I think you should split this in 2 patches. One being the refactoring. (this happened to me with the ICV patch)

LGTM

small fix

In D70365#2128556, @jdoerfert wrote:

In D70365#2123539, @efriedma wrote:

@sstefan1 could u update the patch? We then send another email out with the list of 5 attributes we will make opt-out for intrinsics. A week or so later, assuming no one objected too much, we'll land it. In the meantime we need to update the target independent ones to opt-out appropriately. I guess my nosync list will catch almost all of them already but we'll have to give it another go as the patch is updated. WDYT?

I was under the impression that the attributes will be made default in another patch. Right now we don't opt out of the default attributes, but none are set to be default yet.
If it is better to do this in this patch, no problem. I can send an email, on Monday maybe?

DisableDefaultAttributes flag instead of opt-out list

In D70365#2123539, @efriedma wrote:

On a related note, we might want to consider doing something for pointer/memory handling as well, since that's currently pretty awkward. Most pointer arguments should be nocapture/nofree, but we don't really mark that consistently.

In D70365#2121590, @jdoerfert wrote:

One way to avoid some of the complexity would be to opt-out of everything with a single bit. So if this is set, you don't get any default attributes. Otherwise you get all.

Agreed, that could be another way to go about this. By now, I'm not really sure what is the best out of all the options. What do others think?

addressing comments

For now I've only set IntrNoSync to be default and changed memset to opt-out of IntrNoSync just as an example. Maybe the part where attributes are actually made default should be done in another patch?

Introduction of IsDefault field in IntrinsicProperty as suggested by @nhaehnle.

One more thing, looks like you included only the last change, not the complete diff.

Personally, callsite still sounds more natural in some places, but LGTM.

Sorry this took so long.

• revisit this patch with different implementation

• clean up the patch
• basic deduplication

leftover check lines

new test file for icv remarks

This was my bad. Thanks for noticing!

left out icv part for another patch

This is the first split of D81788

In D81788#2091453, @jdoerfert wrote:

I left a first set of comments. I think the overall direction is good, some interesting opportunities are opening up (see below). We need tests and also to split the patch (some suggestions below).