[Attributor] Deduce "nosync" function attribute.

[sstefan1]

Summary

Introduce and deduce "nosync" function attribute to indicate that a function does not synchronize with another thread in a way that other thread might free memory.

[jdoerfert]

llvm/docs/LangRef.rst

1479 ↗

(On Diff #202526)

The part after "causing" is too specific. We want nosync to be generic.

llvm/lib/Transforms/IPO/Attributor.cpp

3 ↗

(On Diff #202526)

Leftover comments?

12 ↗

(On Diff #202526)

(only a leftover of an old attributor patch version)

170 ↗

(On Diff #202526)

You have to check against "nosync".

378 ↗

(On Diff #202526)

Copy & paste

385 ↗

(On Diff #202526)

Make it static or member function. Also, describe what it does in the comment.

398 ↗

(On Diff #202526)

Description missing.

418 ↗

(On Diff #202526)

What about calls? Maybe you need to look at all side-effect instructions?

442 ↗

(On Diff #202526)

This is not a fixpoint. UpdateImpl is called multiple times (potentially). Remove the fixpoint call.

[sstefan1]

• small fix
• comments and LangRef

[sstefan1]

• removing fixpoint call

[jdoerfert]

The diff seems to include changes in the attributor. May download the latest version of the Attributor patch, rebase this one, and remove everything that is not part of the nosync. Also, please include the changes to the test cases you have in this patch.

llvm/lib/Transforms/IPO/Attributor.cpp

374 ↗

(On Diff #202600)

Typo: "if"

423 ↗

(On Diff #202600)

You have to add fences here as well and look at calls explicitly (as they are not in the above opcode list). Alternatively you could do:

Use the InformationCache::getReadOrWriteInstsForFunction method to get all potential read & write instructions. That will include calls you need to look at and everything above. You will need to look at calls first, if they are fine, you can check for volatile and atomic. The code to determine if a call is OK is already present down there.

431 ↗

(On Diff #202600)

Please use the type here and start variables with an upper case letter.

[sstefan1]

• Checking calls first. Adding checks for fences. Now using InfoCache.

[sstefan1]

llvm/lib/Transforms/IPO/Attributor.cpp

600 ↗

(On Diff #202629)

Just to make sure, when using InformationCache::getReadOrWriteInstsForFunction I don't need this, right?

[jdoerfert]

Some more comments but it looks much better already. The test changes are missing though.

llvm/lib/Transforms/IPO/Attributor.cpp

377 ↗

(On Diff #202629)

"if"

388 ↗

(On Diff #202629)

Put the documentation on the declaration.

406 ↗

(On Diff #202629)

Can you describe the logic here?

435 ↗

(On Diff #202629)

Only do the stuff below if I is actually a call, so if ICS is not null. If you run this, it should crash on you right now because you access ICS unconditionally.

438 ↗

(On Diff #202629)

I think getReadOrWriteInstsForFunction will never pick up a readnone call as it will neither read nor write memory.

450 ↗

(On Diff #202629)

Can we have a single call to isVolatile, maybe always call that one and getOrdering and decide on the result what to do. That would mean move I->isAtomic() into getOrdering() and ensure we catch all opcodes in the switch (so the default prints an error)..

600 ↗

(On Diff #202629)

Correct.

[sstefan1]

Tests almost done. I'll update in couple hours.

llvm/lib/Transforms/IPO/Attributor.cpp

406 ↗

(On Diff #202629)

I had to return one. So if Success isn't intresting it returns Failure ordering. Ohterwise it doesn't matter since Success already syncs. I didn't give this much tought, if you have any suggestions, I'll apply them.

438 ↗

(On Diff #202629)

So is it safe to remove it then, as it does not have side-effects?

450 ↗

(On Diff #202629)

If I do it this way, I think it would be better to change AtomicOrdering getOrdering() to bool isSyncOrdering() or whatever is appropriate for the name. It can than return true if ordering is not Unordered or Monotonic. That way everything can be checked in one if.

and ensure we catch all opcodes in the switch (so the default prints an error)..

I only miss GetElementPtr and alloca which are not of great interest here, if I'm not wrong. But I can add them as well.

[sstefan1]

addressed most of the comments.

[jdoerfert]

llvm/lib/Transforms/IPO/Attributor.cpp

418 ↗

(On Diff #202831)

Shouldn't we here directly assume sync as it is atomic but we don't know what kind?

452 ↗

(On Diff #202831)

You need to check volatile and atomic for all instructions I guess and for calls nosync as well

[sstefan1]

Does everything else look ok?

llvm/lib/Transforms/IPO/Attributor.cpp

418 ↗

(On Diff #202831)

Yes, my bad. I'll return true.

[sstefan1]

small fixes

[jdoerfert]

More comments including various small style suggestions.

You also need to rewrite the commit message and the test case impact is missing.
For the commit message it is probably enough to drop the last part, thus:

Introduce and deduce the "nosync" function attribute which indicates that a function does not synchronize with another thread in any way.

Remind me, is there a language ref patch for nosync somewhere? If not, we need to add a description in the LangRef.doc as well.

llvm/lib/Transforms/IPO/Attributor.cpp

337 ↗

(On Diff #203021)

I forgot that before but I think it is better if we split it into a struct AANoSync in the header and a struct AANoSyncFunction in the cpp file. Some functions would go in the generic header struct, but not getState, getManifestPosition, updateImpl, and ID. This makes it easier to use the result in other attributes.

352 ↗

(On Diff #203021)

2 new lines.

383 ↗

(On Diff #203021)

Comment needs to be updated.

384 ↗

(On Diff #203021)

make it a static member function if possible. that way we can reuse it easier. Same for isVolatile.

408 ↗

(On Diff #203021)

Given that Success and Failure are only needed in this case you can declare them here.
To do so you need to add brackets around the case:

case Instruction::AtomicCmpXchg: {
...
}

413 ↗

(On Diff #203021)

Why is it sufficient that one ordering is "weak enough"? Don't we have to test both? Either way, we need a comment to explain what is happening here.

418 ↗

(On Diff #203021)

No worries, all good. Please also add a comment to explain what this means and why we return true.

420 ↗

(On Diff #203021)

Indention. And maybe add a few more words here ;)

451 ↗

(On Diff #203021)

I think the`!` in front of ICS is a problem. Did you run this?

[sstefan1]

Addresing comments.

LangRef was here from the beginning, I just messed up the diffs. Now its here.

[sstefan1]

Inline comments are now not in original order, so I'll reply here.

I think the`!` in front of ICS is a problem. Did you run this?

Yes.

Why is it sufficient that one ordering is "weak enough"? Don't we have to test both? Either way, we need a comment to explain what is happening here.

My thinking is that if either one of them 'weak enough', than "no-sync" is no longer possible since at any point it can be one of the orderings. If you disagree, I can change and require both.

Indention. And maybe add a few more words here ;)

I updated the function comment, hope thats enough.

[jdoerfert]

In D62766#1531219, @sstefan1 wrote:

Inline comments are now not in original order, so I'll reply here.

I think the`!` in front of ICS is a problem. Did you run this?

Yes.

So remove it ;)

Why is it sufficient that one ordering is "weak enough"? Don't we have to test both? Either way, we need a comment to explain what is happening here.

My thinking is that if either one of them 'weak enough', than "no-sync" is no longer possible since at any point it can be one of the orderings. If you disagree, I can change and require both.

I mixed up the meaning of the return value. It looks fine once I read the comment.

Indention. And maybe add a few more words here ;)

I updated the function comment, hope thats enough.

Looks good.

I added more comments but I think this is almost done. Go through the code and tests yourself and make sure there is no spurious newlines or other changes you did not intend.

llvm/docs/LangRef.rst

1477 ↗

(On Diff #203183)

Maybe add something like:

If the function does ever synchronize with another thread, the behavior is undefined.

llvm/include/llvm/Transforms/IPO/Attributor.h

647 ↗

(On Diff #203183)

You don't need to inherit from BooleanState here. That is an implementation detail we probably want to hide. Let AANoSyncFunction inherit from BooleanState but keep the functions isAssumedNoSync and isKnownNoSync here. They will not have an implementation and are overwritten in AANoSyncFunction.

667 ↗

(On Diff #203183)

Copy and paste, this is not a namespace ;)

llvm/test/Transforms/FunctionAttrs/nosync.ll

115 ↗

(On Diff #203183)

Remove the commented instruction here and in the next test. Also, fix the indention.

139 ↗

(On Diff #203183)

Isn't the "nosync" attribute missing for this function?

197 ↗

(On Diff #203183)

The function names don't match the IR names.

[sstefan1]

• nosync small fixes.
• fixing tests.

[jdoerfert]

llvm/lib/Transforms/IPO/Attributor.cpp

285 ↗

(On Diff #203466)

The above functions should have a comment referring to the base class.

288 ↗

(On Diff #203466)

This has to go in the base class I think.

324 ↗

(On Diff #203466)

I'm still confused. The pessimistic return value is true, correct? If so, Why can we return false after we've seen only the success ordering? Don't we have to look at both success and failure ordering and only if both are "fine" we can return false?

[sstefan1]

llvm/lib/Transforms/IPO/Attributor.cpp

324 ↗

(On Diff #203466)

I agree. I messed this up. Before I update, does this look ok?

if (Success != AtomicOrdering::Unordered ||
        Success != AtomicOrdering::Monotonic)
      return true;

if (Failure != AtomicOrdering::Unordered ||
        Failure != AtomicOrdering::Monotonic)
      return true;

return false;

[sstefan1]

• fixed isNonRelaxedAtomic

[jdoerfert]

LGTM

[arsenm]

llvm/docs/LangRef.rst

1476–1478 ↗

(On Diff #203655)

I think this is a bit vague. In particular I don't think the LangRef defines what a "thread" means anywhere. I also think this needs to be more clear on what kinds of synchronization is allowed. Is this only communication through some addressable memory? What about GPU cross lane communication operations?

I'm wondering if this is sufficient to solve this problem: http://lists.llvm.org/pipermail/llvm-dev/2013-November/067359.html
TLDR, memory instructions can currently be hoisted over an arbitrary call if they are accessing a noalias argument

[arsenm]

llvm/docs/LangRef.rst

1476–1478 ↗

(On Diff #203655)

This is also mentioned as a proper attribute here (which I would greatly prefer to adding another string attribute), but only handled as a string attribute

[jdoerfert]

llvm/docs/LangRef.rst

1476–1478 ↗

(On Diff #203655)

That is a good point. I was initially thinking string attributes are fine but D62784 seems to be stuck which makes the testing of them hard.

Long story short, lets make them enum attributes.

@sstefan1 could you please make this a proper enum attribute? This will require some additional "mechanics" in:
llvm/lib/AsmParser/LLParser.cpp
llvm/lib/Bitcode/Reader/BitcodeReader.cpp
llvm/lib/Bitcode/Writer/BitcodeWriter.cpp
llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp
llvm/lib/IR/Attributes.cpp
llvm/lib/IR/Verifier.cpp

Could be more though. Look for an existing attribute, e.g. Cold, and how that is handled.

@uenoku Could you please also make nofree an enum attribute?

[jdoerfert]

llvm/docs/LangRef.rst

1476–1478 ↗

(On Diff #203655)

I think this is a bit vague. In particular I don't think the LangRef defines what a "thread" means anywhere.

I did think/hope we do not have to. There is the implicit execution thread and nosync says there is "nothing else" while the function is executed. Basically, there are no side-effects that did not originate from the code we see. Please object if you think this is not sufficient.

I also think this needs to be more clear on what kinds of synchronization is allowed.

None, if nosync is present.

Is this only communication through some addressable memory? What about GPU cross lane communication operations?

I'd say, not allowed if nosync is present.

TLDR, memory instructions can currently be hoisted over an arbitrary call if they are accessing a noalias argument

I tried to expose that lately [1] but failed, do you have an example?

[1] https://bugs.llvm.org/show_bug.cgi?id=41781

[sstefan1]

• Making nosync an enum attribute.

[sstefan1]

fixing diff

[jdoerfert]

Sorry for my delayed review, I want to move this ahead now and commit it.

Can you make sure this works on top-of-trunk (origin/master), hence, rebase it please. Also make sure make check-all works without problems.

llvm/lib/Transforms/IPO/Attributor.cpp

93 ↗

(On Diff #204513)

This should now be checked in the switch below, it is an enum attribute now.

281 ↗

(On Diff #204513)

Delete this please.

llvm/test/Transforms/FunctionAttrs/nosync.ll

2 ↗

(On Diff #204513)

You need to enable the attributor explicitly, for now.

[jdoerfert]

Just realized that the basic attribute test in test/Bitcode/attributes.ll is missing, see https://reviews.llvm.org/D49165#change-K8gwLFRSXwEe for an example.

[jfb]

Please add tests for the things I mention in comments, as well as:

• relaxed volatile atomic load / store
• inline assembly

llvm/include/llvm/Transforms/IPO/Attributor.h

381 ↗

(On Diff #204513)

"Underlying"

llvm/lib/Transforms/IPO/Attributor.cpp

314 ↗

(On Diff #204513)

A fence with singlethread sync scope doesn't sync with other threads, even if seq_cst.

331 ↗

(On Diff #204513)

We probably want llvm_unreachable here, so the code gets updated if we add new atomic operations.

341 ↗

(On Diff #204513)

You're missing memcpy and similar intrinsics, I think you want to handle them here and not in the generic intrinsic handling.

llvm/test/Transforms/FunctionAttrs/nosync.ll

5 ↗

(On Diff #204513)

"designet"?

[sstefan1]

• addressing comments.

[arsenm]

llvm/docs/LangRef.rst

1476–1478 ↗

(On Diff #203655)

Is it then disallowed to merge any calls that aren't nosync? e.g.

if (foo)
  bar(x) readnone
else
  bar(y) readnone

is no longer legal to combine these as

bar(foo ? x : y) readnone

[sstefan1]

• small fix
• added include
• Fix cast

[sstefan1]

In D62766#1555926, @jfb wrote:

Please add tests for the things I mention in comments, as well as:

• relaxed volatile atomic load / store
• inline assembly

Only added one test for now. I will add more tomorrow.

In D62766#1555578, @jdoerfert wrote:

Sorry for my delayed review, I want to move this ahead now and commit it.

Can you make sure this works on top-of-trunk (origin/master), hence, rebase it please. Also make sure make check-all works without problems.

@jdoerfert As for the intrinsics. I added checks for memcpy, memmove & memset as @jfb suggested. What do you think? I also kept the FIXME comment which can indicate that we might take a different approach. I did make check-all there were few problems with some FunctionAttr tests not checking for nosync attributes. I will fix that tomorrow. Also I seem to have a problem with a test/Bitcode/attributes.ll with nobuiltin attribute.

[sstefan1]

• updated tests

[nhaehnle]

This does seem useful, although the description is overly narrow (what does nosync on its own have to do with freeing memory?).

I also think that the definition of nosync needs some work, as just "synchronization" is a rather vague term. Can you define it in terms of fences and atomic instructions instead, e.g. by saying that a nosync function does not perform such operations (or some subset of such operations)?

llvm/docs/LangRef.rst

1476–1478 ↗

(On Diff #203655)

No, I think that would still be allowed. The sync (aka not-nosync) functions have a potential side effect in terms of the memory model, but it's the same side effect in either case since the memory model at this point doesn't care about subgroups.

I guess you're thinking of subgroup operations, but the issue with those is that the set of threads with which communication occurs is a function of where the operation occurs in control flow. It makes sense to keep that issue separate from this attribute.

llvm/test/Transforms/FunctionAttrs/nosync.ll

293–295 ↗

(On Diff #206764)

The negative check line here is missing.

[sstefan1]

In D62766#1560305, @nhaehnle wrote:

This does seem useful, although the description is overly narrow (what does nosync on its own have to do with freeing memory?).

The idea was to use this with nofree for dereferencable, like @hfinkel proposed in this email.

I also think that the definition of nosync needs some work, as just "synchronization" is a rather vague term. Can you define it in terms of fences and atomic instructions instead, e.g. by saying that a nosync function does not perform such operations (or some subset of such operations)?

I will update the definition with more details. Maybe I should put that in patch description instead of the current (narrow) one?

[jdoerfert]

llvm/docs/LangRef.rst

1476–1478 ↗

(On Diff #203655)

1. readnone implies nosync in my opinion.
2. if we forget about readnone in the example, I think the above merge is still legal.

[arsenm]

llvm/docs/LangRef.rst

1482–1484 ↗

(On Diff #206764)

This should mention that synchronization means through some kind of memory side-effect. This needs to be distinguished from a cross-lane operations, which could be interpreted as a kind of of synchronization where treating it as a memory dependence is not sufficient

[sstefan1]

• changed nosync LangRef definition

@arsenm I used most of your comment/suggestion.

[jfb]

You should also add a test function with inline assembly.

llvm/lib/Transforms/IPO/Attributor.cpp

315 ↗

(On Diff #206925)

"then"

378 ↗

(On Diff #206925)

"improve"

llvm/test/Transforms/FunctionAttrs/nosync.ll

317 ↗

(On Diff #206925)

I don't think you can generally treat intrinsics as nosync. Unless you know they're actually nosync you should assume that intrinsics might synchronize.

For example:

int a;
void i_totally_sync() {
 __builtin_ia32_clflush(&a);
}

Corresponds to:

tail call void @llvm.x86.sse2.clflush(i8* bitcast (i32* @a to i8*))

You should have a test for this, and it should definitely *not* be nosync.

The other option here is to go and add a field to all intrinsics, so when creating a new one we have to figure out whether it'll definitely sync, maybe sync, or never sync. I don't think that's in scope for this patch.

[nhaehnle]

llvm/docs/LangRef.rst

1481–1488 ↗

(On Diff #206925)

Thanks, I think this is better, but there are still some problems:

• There are no relaxed atomics in LLVM, only unordered, monotonic, and stronger orderings.
• What about fences?

I would put the part about cross-lane operations at the end and rephrase it slightly for clarity. Suggestion:

This attribute is only concerned with synchronization through memory operations and is therefore orthogonal to cross-lane and convergent operations. In particular, an operation such as a barrier can be convergent but also nosync.

Assuming we can agree about the actual statement of that last sentence...

[jdoerfert]

llvm/docs/LangRef.rst

1481–1488 ↗

(On Diff #206925)

> This attribute is only concerned with synchronization through memory operations and is therefore orthogonal to cross-lane and convergent operations. In particular, an operation such as a barrier can be convergent but also nosync.

Assuming we can agree about the actual statement of that last sentence...

This proposed change, and the one requested earlier and integrated (sync goes through memory), are problematic.
I first though they are fine but they will probably make the attribute unusable.

An alternative proposal would be:

This function attribute indicates that the function does not communicate
(synchronize) with another thread through memory or other well-defined means.
Synchronization is considered possible in the presence of `atomic` accesses that enforce an order, thus not "unordered" and "monotonic", `volatile` accesses, as well as `convergent` function calls. Note that through the latter non-memory communication, e.g., cross-lane operations, is also considered synchronization. If an annotated function does ever synchronize with another thread, the behavior is undefined.

If this is where we are heading, we need to make sure we test:

1. non-convergent does not allow nosync, e.g., readnone does not imply nosync
2. readnone and non-convergent does imply nosync

@arsenm, @nhaehnle. @jfb, what do you think?

llvm/test/Transforms/FunctionAttrs/nosync.ll

317 ↗

(On Diff #206925)

I don't think you can generally treat intrinsics as nosync. Unless you know they're actually nosync you should assume that intrinsics might synchronize.

Good point. Maybe the best way (for now and in general) is to "not look for" intrinsics. Use the same logic for all instructions. That is, if it is a call and not annotated as no-sync it may-sync. The test with llvm.cos can be adjusted by adding readnone to the decleration of`llvm.cos`.

The other option here is to go and add a field to all intrinsics, so when creating a new one we have to figure out whether it'll definitely sync, maybe sync, or never sync. I don't think that's in scope for this patch.

Agreed, we will have to do that for various attributes at some point (soon) but not in this patch.

[jfb]

llvm/test/Transforms/FunctionAttrs/nosync.ll

317 ↗

(On Diff #206925)

I'd still accept the volatile mem* intrinsics as is already done, but otherwise yeah intrinsics should be assumed to synchronize.

[arsenm]

llvm/docs/LangRef.rst

1481–1488 ↗

(On Diff #206925)

What makes it unusable exactly?

This wording confuses me:

Note that through the latter non-memory communication, e.g., cross-lane operations, is also considered synchronization.

I'm not 100% comfortable specifically referring to convergent, since I'm still worried about the yet-to-be-defined anticonvergent attribute. Though it is hard to define something around an unsolved problem.

This phrasing also implies to me that call site merging is not legal, which is what I thought you were trying to avoid.

Conclusion 2 sounds OK to me. Conclusion 1 sounds like the opposite of what the goal is?

[jdoerfert]

llvm/docs/LangRef.rst

1481–1488 ↗

(On Diff #206925)

What makes it unusable exactly?

nosync would then still allow non-memory synchronization which it shouldn't.

I think the IRC conversation helped. What I want us to have is:

nosync means no synchronization/communication between "threads". Any potential synchronization, e.g., through memory or registers, precludes nosync.

llvm/test/Transforms/FunctionAttrs/nosync.ll

317 ↗

(On Diff #206925)

Agreed.

[nhaehnle]

llvm/docs/LangRef.rst

1481–1488 ↗

(On Diff #206925)

I think the IRC conversation helped.

Is that recorded somewhere?

nosync would then still allow non-memory synchronization which it shouldn't.

This is questionable. There are convergent operations that do not imply synchronization. For example, some of the llvm.amdgcn.image.sample.* intrinsics are convergent, but they do not imply any kind of synchronization in the memory model.

In Vulkan/SPIR-V parlance, the intrinsic may have an implied control barrier, but it definitely has no memory barrier (the control barrier part isn't fully spec'd out in SPIR-V either at the moment).

For the initial intended usage of this attribute: if there is a pointer that you know to be dereferencable before the image sample, then you still know it to be dereferencable afterwards. So it seems reasonable to want the intrinsic to be marked both convergent and nosync.

That said, I'm okay with this part of it:

If this is where we are heading, we need to make sure we test:

1. non-convergent does not allow nosync, e.g., readnone does not imply nosync
2. readnone and non-convergent does imply nosync

... so long as it's understood that those are "merely" the rules for the attributor.

[jdoerfert]

llvm/docs/LangRef.rst

1481–1488 ↗

(On Diff #206925)

This is questionable. There are convergent operations that do not imply synchronization. For example, some of the llvm.amdgcn.image.sample.* intrinsics are convergent, but they do not imply any kind of synchronization in the memory model.

For me, nosync has to mean absence of any kind of synchronization, including control barriers.

For the initial intended usage of this attribute: if there is a pointer that you know to be dereferencable before the image sample, then you still know it to be dereferencable afterwards. So it seems reasonable to want the intrinsic to be marked both convergent and nosync.

I see why you want this but I don't think that is what it should mean. nosync should not allow control synchronization as it will inevitably cause problems down the road. So, let me rephrase my earlier comment:

By default, we have to assume a convergent & readnone function might cause control synchronization between threads and is therefore not nosync. However, a function can be convergent and nosync. Finally, a function that is not-convergent and readonly is nosync.

[hfinkel]

llvm/docs/LangRef.rst

1481–1488 ↗

(On Diff #206925)

However, a function can be convergent and nosync.

I think that this is important. We can mark convergent intrinsics that don't provide synchronizing semantics as nosync.

In general, we need a nosync attribute to mean that, in the function marked as nosync, the current thread cannot complete communication with any other threads (e.g., it can't send a value to another thread).

The interesting thing, to me, that has been highlighted in this discussion is: convergent functions, by default, can have things like inter-thread register shuffles, but are otherwise readnone, and so must be excluded from automated nosync deduction (because, without accessing memory at all, communicate values to other threads).

[sstefan1]

• Add inline assembly test.

[sstefan1]

llvm/test/Transforms/FunctionAttrs/nosync.ll

293–295 ↗

(On Diff #206764)

I skipped the negative check line, because for now only nosync is deduced and if function is not nosync there will be no Function Attrs at all. Once we have some more attributes, I'll add the negative check. Is that ok with you?

317 ↗

(On Diff #206925)

Replaced llvm.cos test with inline assembly. llvm.cos test was my mistake, since with the current implementation it would be considered sync.

[jdoerfert]

llvm/test/Transforms/FunctionAttrs/nosync.ll

293–295 ↗

(On Diff #206764)

Just give the function the nounwind attribute and then add the negative check line.

317 ↗

(On Diff #206925)

The test comment is off, and again add nounwind to allow for the check lines

[sstefan1]

• fixed tests & improved definition of nosync in langRef

[sstefan1]

• non-convergent and readnone check.
• Changed handling of intrinsics.
• Added more tests.

@jdoerfert, @jfb, @arsenm, @nhaehnle does this look alright now?

[jdoerfert]

I added last minor comments from my side. Other than that I think this looks fine. We will have to wait for the others though.

(You will need to rebase and make sure ninja check-all passes because there are other new attributes.)

llvm/lib/Transforms/IPO/Attributor.cpp

357 ↗

(On Diff #208469)

I think the "unknown" case is handled the wrong way here. Shouldn't it be:

if (Arg->getType()->isIntegerTy(1) &&
    cast<ConstantInt>(Arg)->getValue() == 0)
  return true;
return false;

such that "unknown" values, e.g., %cmp = icmp ... used as the 4th argument will conservatively make it sync?

(+ Test case for this)

412 ↗

(On Diff #208469)

I was puzzled by this check for a second, add a comment indicating that the above loop handles calls with read/write effects already. Mention that the fact there is a read/write effect caused us already to make sure it is nosync and there is consequently no need to check for convergent.

llvm/test/Transforms/FunctionAttrs/nosync.ll

311 ↗

(On Diff #208469)

Copy&paste

350 ↗

(On Diff #208469)

Shouldn't this be nosync? Is it?

[sstefan1]

llvm/lib/Transforms/IPO/Attributor.cpp

357 ↗

(On Diff #208469)

4th argument, isvolatile, is immarg, so I guess this not necessary?

llvm/test/Transforms/FunctionAttrs/nosync.ll

350 ↗

(On Diff #208469)

Yes, this falls under copy & paste as well.

[jdoerfert]

llvm/lib/Bitcode/Reader/BitcodeReader.cpp

1199 ↗

(On Diff #208469)

I think, you can remove this change. All should be fine without.

llvm/lib/Transforms/IPO/Attributor.cpp

357 ↗

(On Diff #208469)

Agreed, not necessary. However, if you keep it this way, add the above reasoning to the comment, it confused me now and it can easily confuse the next person. My advice, just swap the order to make it easier for people now and in the future ;)

[arsenm]

llvm/lib/Transforms/IPO/Attributor.cpp

352–355 ↗

(On Diff #208469)

It would be clearer to cast to MemIntrinsic and check isVolatile

366–379 ↗

(On Diff #208469)

I'm pretty sure this is repeated in several passes, and incomplete. Target intrinsics can also be considered volatile, as there is a hook to get the memory properties for them

[jdoerfert]

llvm/lib/Transforms/IPO/Attributor.cpp

366–379 ↗

(On Diff #208469)

I guess we should not reach this function with calls. If that seams reasonable, we need an assert here and change the source below to skip these checks if a call is assumed/known nosync.

[sstefan1]

• rebase
• addressing comments
• ninja check-all passed

[jdoerfert]

LGTM, assunming check-all passes.

[arsenm]

LGTM with nits

llvm/include/llvm/Transforms/IPO/Attributor.h

689 ↗

(On Diff #209013)

Don't need virtual, only override

llvm/lib/Transforms/IPO/Attributor.cpp

745 ↗

(On Diff #209013)

No virtual necessary (and for the rest of the overrides)

llvm/test/Bitcode/attributes.ll

367 ↗

(On Diff #209013)

Brace placement

llvm/trunk/docs/LangRef.rst

``norecurse``
     undefined behavior at runtime if the function ever does recurse.
 ``willreturn``
     This function attribute indicates that a call of this function will
     either exhibit undefined behavior or comes back and continues execution
     at a point in the existing call stack that includes the current invocation.
     Annotated functions may still raise an exception, i.a., ``nounwind`` is not implied.
     If an invocation of an annotated function does not return control back
     to a point in the call stack, the behavior is undefined.
+``nosync``
+    This function attribute indicates that the function does not communicate
+    (synchronize) with another thread through memory or other well-defined means.
+    Synchronization is considered possible in the presence of `atomic` accesses
+    that enforce an order, thus not "unordered" and "monotonic", `volatile` accesses,
+    as well as `convergent` function calls. Note that through `convergent` function calls
+    non-memory communication, e.g., cross-lane operations, are possible and are also
+    considered synchronization. However `convergent` does not contradict `nosync`.
+    If an annotated function does ever synchronize with another thread,
+    the behavior is undefined.
 ``nounwind``
     This function attribute indicates that the function never raises an
     exception. If the function does raise an exception, its runtime
     behavior is undefined. However, functions marked nounwind may still
     trap or generate asynchronous exceptions. Exception handling schemes
     that are recognized by LLVM to handle asynchronous exceptions, such
     as SEH, will still provide their implementation defined semantics.
 ``"null-pointer-is-valid"``

llvm/trunk/include/llvm/Bitcode/LLVMBitCodes.h

enum AttributeKindCodes {
   ATTR_KIND_STRICT_FP = 54,
   ATTR_KIND_SANITIZE_HWADDRESS = 55,
   ATTR_KIND_NOCF_CHECK = 56,
   ATTR_KIND_OPT_FOR_FUZZING = 57,
   ATTR_KIND_SHADOWCALLSTACK = 58,
   ATTR_KIND_SPECULATIVE_LOAD_HARDENING = 59,
   ATTR_KIND_IMMARG = 60,
   ATTR_KIND_WILLRETURN = 61,
-  ATTR_KIND_NOFREE = 62
+  ATTR_KIND_NOFREE = 62,
+  ATTR_KIND_NOSYNC = 63
 };
 
 enum ComdatSelectionKindCodes {
   COMDAT_SELECTION_KIND_ANY = 1,
   COMDAT_SELECTION_KIND_EXACT_MATCH = 2,
   COMDAT_SELECTION_KIND_LARGEST = 3,
   COMDAT_SELECTION_KIND_NO_DUPLICATES = 4,
   COMDAT_SELECTION_KIND_SAME_SIZE = 5,

llvm/trunk/include/llvm/IR/Attributes.td

 def NoRecurse : EnumAttr<"norecurse">;
 
 /// Disable redzone.
 def NoRedZone : EnumAttr<"noredzone">;
 
 /// Mark the function as not returning.
 def NoReturn : EnumAttr<"noreturn">;
 
+/// Function does not synchronize.
+def NoSync : EnumAttr<"nosync">;
+
 /// Disable Indirect Branch Tracking.
 def NoCfCheck : EnumAttr<"nocf_check">;
 
 /// Function doesn't unwind stack.
 def NoUnwind : EnumAttr<"nounwind">;
 
 /// Select optimizations for best fuzzing signal.
 def OptForFuzzing : EnumAttr<"optforfuzzing">;

llvm/trunk/include/llvm/Transforms/IPO/Attributor.h

 /// bits. Users can only add known bits and, except through adding known bits,
 /// they can only remove assumed bits. This should guarantee monotoniticy and
 /// thereby the existence of a fixpoint (if used corretly). The fixpoint is
 /// reached when the assumed and known state/bits are equal. Users can
 /// force/inidicate a fixpoint. If an optimistic one is indicated, the known
 /// state will catch up with the assumed one, for a pessimistic fixpoint it is
 /// the other way around.
 struct IntegerState : public AbstractState {
-  /// Undrlying integer type, we assume 32 bits to be enough.
+  /// Underlying integer type, we assume 32 bits to be enough.
   using base_t = uint32_t;
 
   /// Initialize the (best) state.
   IntegerState(base_t BestState = ~0) : Assumed(BestState) {}
 
   /// Return the worst possible representable state.
   static constexpr base_t getWorstState() { return 0; }
 
struct AAReturnedValues : public AbstractAttribute {
   /// See AbstractAttribute::getAttrKind()
   virtual Attribute::AttrKind getAttrKind() const override { return ID; }
 
   /// The identifier used by the Attributor for this class of attributes.
   static constexpr Attribute::AttrKind ID = Attribute::Returned;
 };
 
 struct AANoUnwind : public AbstractAttribute {
-    /// An abstract interface for all nosync attributes.
-    AANoUnwind(Value &V, InformationCache &InfoCache)
-        : AbstractAttribute(V, InfoCache) {}
 
-    /// See AbstractAttribute::getAttrKind()/
-    virtual Attribute::AttrKind getAttrKind() const override { return ID; }
 
-    static constexpr Attribute::AttrKind ID = Attribute::NoUnwind;
 
-    /// Returns true if nounwind is assumed.
-    virtual bool isAssumedNoUnwind() const = 0;
 
-    /// Returns true if nounwind is known.
-    virtual bool isKnownNoUnwind() const = 0;
 };
 
+struct AANoSync : public AbstractAttribute {
+  /// An abstract interface for all nosync attributes.
+  AANoSync(Value &V, InformationCache &InfoCache)
+      : AbstractAttribute(V, InfoCache) {}
+
+  /// See AbstractAttribute::getAttrKind().
+  virtual Attribute::AttrKind getAttrKind() const override {
+    return ID;
+  }
+
+  static constexpr Attribute::AttrKind ID =
+      Attribute::AttrKind(Attribute::NoSync);
+
+  /// Returns true if "nosync" is assumed.
+  virtual bool isAssumedNoSync() const = 0;
+
+  /// Returns true if "nosync" is known.
+  virtual bool isKnownNoSync() const = 0;
+};
+
 } // end namespace llvm
 
 #endif // LLVM_TRANSFORMS_IPO_FUNCTIONATTRS_H

llvm/trunk/lib/AsmParser/LLLexer.cpp

#define KEYWORD(STR)                                                           \
   KEYWORD(nofree);
   KEYWORD(noimplicitfloat);
   KEYWORD(noinline);
   KEYWORD(norecurse);
   KEYWORD(nonlazybind);
   KEYWORD(nonnull);
   KEYWORD(noredzone);
   KEYWORD(noreturn);
+  KEYWORD(nosync);
   KEYWORD(nocf_check);
   KEYWORD(nounwind);
   KEYWORD(optforfuzzing);
   KEYWORD(optnone);
   KEYWORD(optsize);
   KEYWORD(readnone);
   KEYWORD(readonly);
   KEYWORD(returned);

llvm/trunk/lib/AsmParser/LLParser.cpp

while (true) {
     case lltok::kw_noduplicate: B.addAttribute(Attribute::NoDuplicate); break;
     case lltok::kw_nofree: B.addAttribute(Attribute::NoFree); break;
     case lltok::kw_noimplicitfloat:
       B.addAttribute(Attribute::NoImplicitFloat); break;
     case lltok::kw_noinline: B.addAttribute(Attribute::NoInline); break;
     case lltok::kw_nonlazybind: B.addAttribute(Attribute::NonLazyBind); break;
     case lltok::kw_noredzone: B.addAttribute(Attribute::NoRedZone); break;
     case lltok::kw_noreturn: B.addAttribute(Attribute::NoReturn); break;
+    case lltok::kw_nosync: B.addAttribute(Attribute::NoSync); break;
     case lltok::kw_nocf_check: B.addAttribute(Attribute::NoCfCheck); break;
     case lltok::kw_norecurse: B.addAttribute(Attribute::NoRecurse); break;
     case lltok::kw_nounwind: B.addAttribute(Attribute::NoUnwind); break;
     case lltok::kw_optforfuzzing:
       B.addAttribute(Attribute::OptForFuzzing); break;
     case lltok::kw_optnone: B.addAttribute(Attribute::OptimizeNone); break;
     case lltok::kw_optsize: B.addAttribute(Attribute::OptimizeForSize); break;
     case lltok::kw_readnone: B.addAttribute(Attribute::ReadNone); break;

llvm/trunk/lib/AsmParser/LLToken.h

enum Kind {
   kw_nofree,
   kw_noimplicitfloat,
   kw_noinline,
   kw_norecurse,
   kw_nonlazybind,
   kw_nonnull,
   kw_noredzone,
   kw_noreturn,
+  kw_nosync,
   kw_nocf_check,
   kw_nounwind,
   kw_optforfuzzing,
   kw_optnone,
   kw_optsize,
   kw_readnone,
   kw_readonly,
   kw_returned,

llvm/trunk/lib/Bitcode/Reader/BitcodeReader.cpp

static uint64_t getRawAttributeMask(Attribute::AttrKind Val) {
   case Attribute::SpeculativeLoadHardening:
     return 1ULL << 60;
   case Attribute::ImmArg:
     return 1ULL << 61;
   case Attribute::WillReturn:
     return 1ULL << 62;
   case Attribute::NoFree:
     return 1ULL << 63;
+  case Attribute::NoSync:
+    llvm_unreachable("nosync attribute not supported in raw format");
+    break;
   case Attribute::Dereferenceable:
     llvm_unreachable("dereferenceable attribute not supported in raw format");
     break;
   case Attribute::DereferenceableOrNull:
     llvm_unreachable("dereferenceable_or_null attribute not supported in raw "
                      "format");
     break;
   case Attribute::ArgMemOnly:

 static void addRawAttributeValue(AttrBuilder &B, uint64_t Val) {
   if (!Val) return;
 
   for (Attribute::AttrKind I = Attribute::None; I != Attribute::EndAttrKinds;
        I = Attribute::AttrKind(I + 1)) {
     if (I == Attribute::Dereferenceable ||
         I == Attribute::DereferenceableOrNull ||
         I == Attribute::ArgMemOnly ||
-        I == Attribute::AllocSize)
+        I == Attribute::AllocSize ||
+        I == Attribute::NoSync)
       continue;
     if (uint64_t A = (Val & getRawAttributeMask(I))) {
       if (I == Attribute::Alignment)
         B.addAlignmentAttr(1ULL << ((A >> 16) - 1));
       else if (I == Attribute::StackAlignment)
         B.addStackAlignmentAttr(1ULL << ((A >> 26)-1));
       else
         B.addAttribute(I);
static Attribute::AttrKind getAttrFromCode(uint64_t Code) {
   case bitc::ATTR_KIND_DEREFERENCEABLE_OR_NULL:
     return Attribute::DereferenceableOrNull;
   case bitc::ATTR_KIND_ALLOC_SIZE:
     return Attribute::AllocSize;
   case bitc::ATTR_KIND_NO_RED_ZONE:
     return Attribute::NoRedZone;
   case bitc::ATTR_KIND_NO_RETURN:
     return Attribute::NoReturn;
+  case bitc::ATTR_KIND_NOSYNC:
+    return Attribute::NoSync;
   case bitc::ATTR_KIND_NOCF_CHECK:
     return Attribute::NoCfCheck;
   case bitc::ATTR_KIND_NO_UNWIND:
     return Attribute::NoUnwind;
   case bitc::ATTR_KIND_OPT_FOR_FUZZING:
     return Attribute::OptForFuzzing;
   case bitc::ATTR_KIND_OPTIMIZE_FOR_SIZE:
     return Attribute::OptimizeForSize;

llvm/trunk/lib/Bitcode/Writer/BitcodeWriter.cpp

static uint64_t getAttrKindEncoding(Attribute::AttrKind Kind) {
   case Attribute::Dereferenceable:
     return bitc::ATTR_KIND_DEREFERENCEABLE;
   case Attribute::DereferenceableOrNull:
     return bitc::ATTR_KIND_DEREFERENCEABLE_OR_NULL;
   case Attribute::NoRedZone:
     return bitc::ATTR_KIND_NO_RED_ZONE;
   case Attribute::NoReturn:
     return bitc::ATTR_KIND_NO_RETURN;
+  case Attribute::NoSync:
+    return bitc::ATTR_KIND_NOSYNC;
   case Attribute::NoCfCheck:
     return bitc::ATTR_KIND_NOCF_CHECK;
   case Attribute::NoUnwind:
     return bitc::ATTR_KIND_NO_UNWIND;
   case Attribute::OptForFuzzing:
     return bitc::ATTR_KIND_OPT_FOR_FUZZING;
   case Attribute::OptimizeForSize:
     return bitc::ATTR_KIND_OPTIMIZE_FOR_SIZE;

llvm/trunk/lib/IR/Attributes.cpp

std::string Attribute::getAsString(bool InAttrGrp) const {
   if (hasAttribute(Attribute::NonLazyBind))
     return "nonlazybind";
   if (hasAttribute(Attribute::NonNull))
     return "nonnull";
   if (hasAttribute(Attribute::NoRedZone))
     return "noredzone";
   if (hasAttribute(Attribute::NoReturn))
     return "noreturn";
+  if (hasAttribute(Attribute::NoSync))
+    return "nosync";
   if (hasAttribute(Attribute::WillReturn))
     return "willreturn";
   if (hasAttribute(Attribute::NoCfCheck))
     return "nocf_check";
   if (hasAttribute(Attribute::NoRecurse))
     return "norecurse";
   if (hasAttribute(Attribute::NoUnwind))
     return "nounwind";

llvm/trunk/lib/IR/Verifier.cpp

void Verifier::visitModuleFlagCGProfileEntry(const MDOperand &MDO) {
   Assert(Count && Count->getType()->isIntegerTy(),
          "expected an integer constant", Node->getOperand(2));
 }
 
 /// Return true if this attribute kind only applies to functions.
 static bool isFuncOnlyAttr(Attribute::AttrKind Kind) {
   switch (Kind) {
   case Attribute::NoReturn:
+  case Attribute::NoSync:
   case Attribute::WillReturn:
   case Attribute::NoCfCheck:
   case Attribute::NoUnwind:
   case Attribute::NoInline:
   case Attribute::NoFree:
   case Attribute::AlwaysInline:
   case Attribute::OptimizeForSize:
   case Attribute::StackProtect:

llvm/trunk/lib/Transforms/IPO/Attributor.cpp

 #include "llvm/ADT/SetVector.h"
 #include "llvm/ADT/SmallPtrSet.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/Statistic.h"
 #include "llvm/Analysis/GlobalsModRef.h"
 #include "llvm/IR/Argument.h"
 #include "llvm/IR/Attributes.h"
 #include "llvm/IR/InstIterator.h"
+#include "llvm/IR/IntrinsicInst.h"
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/Debug.h"
 #include "llvm/Support/raw_ostream.h"
 #include <cassert>
 
 using namespace llvm;
 
 #define DEBUG_TYPE "attributor"

 STATISTIC(NumAttributesManifested,
           "Number of abstract attributes manifested in IR");
 STATISTIC(NumFnNoUnwind, "Number of functions marked nounwind");
 
 STATISTIC(NumFnUniqueReturned, "Number of function with unique return");
 STATISTIC(NumFnKnownReturns, "Number of function with known return values");
 STATISTIC(NumFnArgumentReturned,
           "Number of function arguments marked returned");
+STATISTIC(NumFnNoSync, "Number of functions marked nosync");
 
 // TODO: Determine a good default value.
 //
 // In the LLVM-TS and SPEC2006, 32 seems to not induce compile time overheads
 // (when run with the first 5 abstract attributes). The results also indicate
 // that we never reach 32 iterations but always find a fixpoint sooner.
 //
 // This will become more evolved once we perform two interleaved fixpoint
if (!Attr.isEnumAttribute())
     return;
   switch (Attr.getKindAsEnum()) {
   case Attribute::NoUnwind:
     NumFnNoUnwind++;
     return;
   case Attribute::Returned:
     NumFnArgumentReturned++;
     return;
+  case Attribute::NoSync:
+    NumFnNoSync++;
+    break;
   default:
     return;
   }
 }
 
 template <typename StateTy>
 using followValueCB_t = std::function<bool(Value *, StateTy &State)>;
 template <typename StateTy>
ChangeStatus AAReturnedValuesImpl::updateImpl(Attributor &A) {
   if (!HasCallSite) {
     indicateOptimisticFixpoint();
     return ChangeStatus::CHANGED;
   }
 
   return Changed;
 }
 
+/// ------------------------ NoSync Function Attribute -------------------------
+
+struct AANoSyncFunction : AANoSync, BooleanState {
+
+  AANoSyncFunction(Function &F, InformationCache &InfoCache)
+      : AANoSync(F, InfoCache) {}
+
+  /// See AbstractAttribute::getState()
+  /// {
+  AbstractState &getState() override { return *this; }
+  const AbstractState &getState() const override { return *this; }
+  /// }
+
+  /// See AbstractAttribute::getManifestPosition().
+  virtual ManifestPosition getManifestPosition() const override {
+    return MP_FUNCTION;
+  }
+
+  virtual const std::string getAsStr() const override {
+    return getAssumed() ? "nosync" : "may-sync";
+  }
+
+  /// See AbstractAttribute::updateImpl(...).
+  virtual ChangeStatus updateImpl(Attributor &A) override;
+
+  /// See AANoSync::isAssumedNoSync()
+  virtual bool isAssumedNoSync() const override { return getAssumed(); }
+
+  /// See AANoSync::isKnownNoSync()
+  virtual bool isKnownNoSync() const override { return getKnown(); }
+
+  /// Helper function used to determine whether an instruction is non-relaxed
+  /// atomic. In other words, if an atomic instruction does not have unordered
+  /// or monotonic ordering
+  static bool isNonRelaxedAtomic(Instruction *I);
+
+  /// Helper function used to determine whether an instruction is volatile.
+  static bool isVolatile(Instruction *I);
+
+  /// Helper function uset to check if intrinsic is volatile (memcpy, memmove, memset).
+  static bool isNoSyncIntrinsic(Instruction *I);
+};
+
+bool AANoSyncFunction::isNonRelaxedAtomic(Instruction *I) {
+  if (!I->isAtomic())
+    return false;
+
+  AtomicOrdering Ordering;
+  switch (I->getOpcode()) {
+  case Instruction::AtomicRMW:
+    Ordering = cast<AtomicRMWInst>(I)->getOrdering();
+    break;
+  case Instruction::Store:
+    Ordering = cast<StoreInst>(I)->getOrdering();
+    break;
+  case Instruction::Load:
+    Ordering = cast<LoadInst>(I)->getOrdering();
+    break;
+  case Instruction::Fence: {
+    auto *FI = cast<FenceInst>(I);
+    if (FI->getSyncScopeID() == SyncScope::SingleThread)
+      return false;
+    Ordering = FI->getOrdering();
+    break;
+  }
+  case Instruction::AtomicCmpXchg: {
+    AtomicOrdering Success = cast<AtomicCmpXchgInst>(I)->getSuccessOrdering();
+    AtomicOrdering Failure = cast<AtomicCmpXchgInst>(I)->getFailureOrdering();
+    // Only if both are relaxed, than it can be treated as relaxed.
+    // Otherwise it is non-relaxed.
+    if (Success != AtomicOrdering::Unordered &&
+        Success != AtomicOrdering::Monotonic)
+      return true;
+    if (Failure != AtomicOrdering::Unordered &&
+        Failure != AtomicOrdering::Monotonic)
+      return true;
+    return false;
+  }
+  default:
+    llvm_unreachable(
+        "New atomic operations need to be known in the attributor.");
+  }
+
+  // Relaxed.
+  if (Ordering == AtomicOrdering::Unordered ||
+      Ordering == AtomicOrdering::Monotonic)
+    return false;
+  return true;
+}
+
+/// Checks if an intrinsic is nosync. Currently only checks mem* intrinsics.
+/// FIXME: We should ipmrove the handling of intrinsics.
+bool AANoSyncFunction::isNoSyncIntrinsic(Instruction *I) {
+  if (auto *II = dyn_cast<IntrinsicInst>(I)) {
+    switch (II->getIntrinsicID()) {
+    /// Element wise atomic memory intrinsics are can only be unordered,
+    /// therefore nosync.
+    case Intrinsic::memset_element_unordered_atomic:
+    case Intrinsic::memmove_element_unordered_atomic:
+    case Intrinsic::memcpy_element_unordered_atomic:
+      return true;
+    case Intrinsic::memset:
+    case Intrinsic::memmove:
+    case Intrinsic::memcpy:
+      if (!cast<MemIntrinsic>(II)->isVolatile())
+        return true;
+      return false;
+    default:
+      return false;
+    }
+  }
+  return false;
+}
+
+bool AANoSyncFunction::isVolatile(Instruction *I) {
+  assert(!ImmutableCallSite(I) && !isa<CallBase>(I) &&
+         "Calls should not be checked here");
+
+  switch (I->getOpcode()) {
+  case Instruction::AtomicRMW:
+    return cast<AtomicRMWInst>(I)->isVolatile();
+  case Instruction::Store:
+    return cast<StoreInst>(I)->isVolatile();
+  case Instruction::Load:
+    return cast<LoadInst>(I)->isVolatile();
+  case Instruction::AtomicCmpXchg:
+    return cast<AtomicCmpXchgInst>(I)->isVolatile();
+  default:
+    return false;
+  }
+}
+
+ChangeStatus AANoSyncFunction::updateImpl(Attributor &A) {
+  Function &F = getAnchorScope();
+
+  /// We are looking for volatile instructions or Non-Relaxed atomics.
+  /// FIXME: We should ipmrove the handling of intrinsics.
+  for (Instruction *I : InfoCache.getReadOrWriteInstsForFunction(F)) {
+    ImmutableCallSite ICS(I);
+    auto *NoSyncAA = A.getAAFor<AANoSyncFunction>(*this, *I);
+
+    if (isa<IntrinsicInst>(I) && isNoSyncIntrinsic(I))
+        continue;
+
+    if (ICS && (!NoSyncAA || !NoSyncAA->isAssumedNoSync()) &&
+        !ICS.hasFnAttr(Attribute::NoSync)) {
+      indicatePessimisticFixpoint();
+      return ChangeStatus::CHANGED;
+    }
+
+    if(ICS)
+      continue;
+
+    if (!isVolatile(I) && !isNonRelaxedAtomic(I))
+      continue;
+
+    indicatePessimisticFixpoint();
+    return ChangeStatus::CHANGED;
+  }
+
+  auto &OpcodeInstMap = InfoCache.getOpcodeInstMapForFunction(F);
+  auto Opcodes = {(unsigned)Instruction::Invoke, (unsigned)Instruction::CallBr,
+                  (unsigned)Instruction::Call};
+
+  for (unsigned Opcode : Opcodes) {
+    for (Instruction *I : OpcodeInstMap[Opcode]) {
+      // At this point we handled all read/write effects and they are all
+      // nosync, so they can be skipped.
+      if (I->mayReadOrWriteMemory())
+        continue;
+
+      ImmutableCallSite ICS(I);
+
+      // non-convergent and readnone imply nosync.
+      if (!ICS.isConvergent())
+        continue;
+
+      indicatePessimisticFixpoint();
+      return ChangeStatus::CHANGED;
+    }
+  }
+
+  return ChangeStatus::UNCHANGED;
+}
+
 /// ----------------------------------------------------------------------------
 ///                               Attributor
 /// ----------------------------------------------------------------------------
 
 ChangeStatus Attributor::run() {
   // Initialize all abstract attributes.
   for (AbstractAttribute *AA : AllAbstractAttributes)
     AA->initialize(*this);

 
 void Attributor::identifyDefaultAbstractAttributes(
     Function &F, InformationCache &InfoCache,
     DenseSet</* Attribute::AttrKind */ unsigned> *Whitelist) {
 
   // Every function can be nounwind.
   registerAA(*new AANoUnwindFunction(F, InfoCache));
 
+  // Every function might be marked "nosync"
+  registerAA(*new AANoSyncFunction(F, InfoCache));
+
   // Return attributes are only appropriate if the return type is non void.
   Type *ReturnType = F.getReturnType();
   if (!ReturnType->isVoidTy()) {
     // Argument attribute "returned" --- Create only one per function even
     // though it is an argument attribute.
     if (!Whitelist || Whitelist->count(AAReturnedValues::ID))
       registerAA(*new AAReturnedValuesImpl(F, InfoCache));
   }

llvm/trunk/lib/Transforms/Utils/CodeExtractor.cpp

if (Attr.isStringAttribute()) {
       case Attribute::InaccessibleMemOrArgMemOnly:
       case Attribute::JumpTable:
       case Attribute::Naked:
       case Attribute::Nest:
       case Attribute::NoAlias:
       case Attribute::NoBuiltin:
       case Attribute::NoCapture:
       case Attribute::NoReturn:
+      case Attribute::NoSync:
       case Attribute::None:
       case Attribute::NonNull:
       case Attribute::ReadNone:
       case Attribute::ReadOnly:
       case Attribute::Returned:
       case Attribute::ReturnsTwice:
       case Attribute::SExt:
       case Attribute::Speculatable:

llvm/trunk/test/Bitcode/attributes.ll

; CHECK: define void @f33() #22
         ret void;
 }
 
 declare void @nobuiltin()
 
 define void @f34()
 ; CHECK: define void @f34()
 {
-        call void @nobuiltin() nobuiltin
-; CHECK: call void @nobuiltin() #38
+; CHECK: call void @nobuiltin() #39
         ret void;
 }
 
 define void @f35() optnone noinline
 ; CHECK: define void @f35() #23
 {
         ret void;
 }
define void @f60() willreturn
   ret void
 }
 
 ; CHECK: define void @f61() #37
 define void @f61() nofree {
   ret void
 }
 
+; CHECK: define void @f62() #38
+define void @f62() nosync
+{
+  ret void
+}
+
 ; CHECK: attributes #0 = { noreturn }
 ; CHECK: attributes #1 = { nounwind }
 ; CHECK: attributes #2 = { readnone }
 ; CHECK: attributes #3 = { readonly }
 ; CHECK: attributes #4 = { noinline }
 ; CHECK: attributes #5 = { alwaysinline }
 ; CHECK: attributes #6 = { optsize }
 ; CHECK: attributes #7 = { ssp }

 ; CHECK: attributes #30 = { allocsize(0) }
 ; CHECK: attributes #31 = { allocsize(0,1) }
 ; CHECK: attributes #32 = { writeonly }
 ; CHECK: attributes #33 = { speculatable }
 ; CHECK: attributes #34 = { sanitize_hwaddress }
 ; CHECK: attributes #35 = { shadowcallstack }
 ; CHECK: attributes #36 = { willreturn }
 ; CHECK: attributes #37 = { nofree }
-; CHECK: attributes #38 = { nobuiltin }
+; CHECK: attributes #38 = { nosync }
+; CHECK: attributes #39 = { nobuiltin }

llvm/trunk/test/Transforms/FunctionAttrs/arg_returned.ll

 ; RUN: opt -functionattrs -S < %s | FileCheck %s --check-prefix=FNATTR
 ; RUN: opt -attributor -attributor-disable=false -S < %s | FileCheck %s --check-prefix=ATTRIBUTOR
 ; RUN: opt -attributor -attributor-disable=false -functionattrs -S < %s | FileCheck %s --check-prefix=BOTH
 ;
 ; Test cases specifically designed for the "returned" argument attribute.
 ; We use FIXME's to indicate problems and missing attributes.
 ;
 
 ; TEST SCC test returning an integer value argument
 ;
-; BOTH: Function Attrs: noinline norecurse nounwind readnone uwtable
+; BOTH: Function Attrs: noinline norecurse nosync nounwind readnone uwtable
 ; BOTH-NEXT: define i32 @sink_r0(i32 returned %r)
-; BOTH: Function Attrs: noinline nounwind readnone uwtable
+; BOTH: Function Attrs: noinline nosync nounwind readnone uwtable
 ; BOTH-NEXT: define i32 @scc_r1(i32 %a, i32 returned %r, i32 %b)
-; BOTH: Function Attrs: noinline nounwind readnone uwtable
+; BOTH: Function Attrs: noinline nosync nounwind readnone uwtable
 ; BOTH-NEXT: define i32 @scc_r2(i32 %a, i32 %b, i32 returned %r)
-; BOTH: Function Attrs: noinline nounwind readnone uwtable
+; BOTH: Function Attrs: noinline nosync nounwind readnone uwtable
 ; BOTH-NEXT: define i32 @scc_rX(i32 %a, i32 %b, i32 %r)
 ;
 ; FNATTR: define i32 @sink_r0(i32 returned %r)
 ; FNATTR: define i32 @scc_r1(i32 %a, i32 %r, i32 %b)
 ; FNATTR: define i32 @scc_r2(i32 %a, i32 %b, i32 %r)
 ; FNATTR: define i32 @scc_rX(i32 %a, i32 %b, i32 %r)
 ;
 ; ATTRIBUTOR: define i32 @sink_r0(i32 returned %r)

 return:                                           ; preds = %cond.end, %if.then3, %if.then
   %retval.0 = phi i32 [ %call1, %if.then ], [ %call11, %if.then3 ], [ %cond, %cond.end ]
   ret i32 %retval.0
 }
 
 
 ; TEST SCC test returning a pointer value argument
 ;
-; BOTH: Function Attrs: noinline norecurse nounwind readnone uwtable
+; BOTH: Function Attrs: noinline norecurse nosync nounwind readnone uwtable
 ; BOTH-NEXT: define double* @ptr_sink_r0(double* readnone returned %r)
-; BOTH: Function Attrs: noinline nounwind readnone uwtable
+; BOTH: Function Attrs: noinline nosync nounwind readnone uwtable
 ; BOTH-NEXT: define double* @ptr_scc_r1(double* %a, double* readnone returned %r, double* nocapture readnone %b)
-; BOTH: Function Attrs: noinline nounwind readnone uwtable
+; BOTH: Function Attrs: noinline nosync nounwind readnone uwtable
 ; BOTH-NEXT: define double* @ptr_scc_r2(double* readnone %a, double* readnone %b, double* readnone returned %r)
 ;
 ; FNATTR: define double* @ptr_sink_r0(double* readnone returned %r)
 ; FNATTR: define double* @ptr_scc_r1(double* %a, double* readnone %r, double* nocapture readnone %b)
 ; FNATTR: define double* @ptr_scc_r2(double* readnone %a, double* readnone %b, double* readnone %r)
 ;
-; ATTRIBUTOR: define double* @ptr_sink_r0(double* returned %r)
-; ATTRIBUTOR: define double* @ptr_scc_r1(double* %a, double* returned %r, double* %b)
-; ATTRIBUTOR: define double* @ptr_scc_r2(double* %a, double* %b, double* returned %r)
+; ATTRIBUTOR: Function Attrs: noinline nosync nounwind uwtable
+; ATTRIBUTOR-NEXT: define double* @ptr_sink_r0(double* returned %r)
+; ATTRIBUTOR: Function Attrs: noinline nosync nounwind uwtable
+; ATTRIBUTOR-NEXT: define double* @ptr_scc_r1(double* %a, double* returned %r, double* %b)
+; ATTRIBUTOR: Function Attrs: noinline nosync nounwind uwtable
+; ATTRIBUTOR-NEXT: define double* @ptr_scc_r2(double* %a, double* %b, double* returned %r)
 ;
 ; double* ptr_scc_r1(double* a, double* b, double* r);
 ; double* ptr_scc_r2(double* a, double* b, double* r);
 ;
 ; __attribute__((noinline)) double* ptr_sink_r0(double* r) {
 ;   return r;
 ; }
 ;

 ; TEST a no-return singleton SCC
 ;
 ; int* rt0(int *a) {
 ;   return *a ? a : rt0(a);
 ; }
 ;
 ; FIXME: no-return missing
 ; FNATTR:  define i32* @rt0(i32* readonly %a)
-; BOTH: Function Attrs: noinline nounwind readonly uwtable
+; BOTH: Function Attrs: noinline nosync nounwind readonly uwtable
 ; BOTH-NEXT:    define i32* @rt0(i32* readonly returned %a)
 define i32* @rt0(i32* %a) #0 {
 entry:
   %v = load i32, i32* %a, align 4
   %tobool = icmp ne i32 %v, 0
   %call = call i32* @rt0(i32* %a)
   %sel = select i1 %tobool, i32* %a, i32* %call
   ret i32* %sel
 }
 
 ; TEST a no-return singleton SCC
 ;
 ; int* rt1(int *a) {
 ;   return *a ? undef : rt1(a);
 ; }
 ;
 ; FIXME: no-return missing
 ; FNATTR:  define noalias i32* @rt1(i32* nocapture readonly %a)
-; BOTH: Function Attrs: noinline nounwind readonly uwtable
+; BOTH: Function Attrs: noinline nosync nounwind readonly uwtable
 ; BOTH-NEXT:    define noalias i32* @rt1(i32* nocapture readonly %a)
 define i32* @rt1(i32* %a) #0 {
 entry:
   %v = load i32, i32* %a, align 4
   %tobool = icmp ne i32 %v, 0
   %call = call i32* @rt1(i32* %a)
   %sel = select i1 %tobool, i32* undef, i32* %call
   ret i32* %sel

 ;
 ; double select_and_phi(double b) {
 ;   double x = b;
 ;   if (b > 0)
 ;     x = b;
 ;   return b == 0? b : x;
 ; }
 ;
-; BOTH: Function Attrs: noinline norecurse nounwind readnone uwtable
+; BOTH: Function Attrs: noinline norecurse nosync nounwind readnone uwtable
 ; BOTH-NEXT: define double @select_and_phi(double returned %b)
 ;
 ; FNATTR:     define double @select_and_phi(double %b)
-; ATTRIBUTOR: define double @select_and_phi(double returned %b)
+; ATTRIBUTOR: Function Attrs: noinline nosync nounwind uwtable
+; ATTRIBUTOR-NEXT: define double @select_and_phi(double returned %b)
 define double @select_and_phi(double %b) #0 {
 entry:
   %cmp = fcmp ogt double %b, 0.000000e+00
   br i1 %cmp, label %if.then, label %if.end
 
 if.then:                                          ; preds = %entry
   br label %if.end
 

 ;
 ; double recursion_select_and_phi(int a, double b) {
 ;   double x = b;
 ;   if (a-- > 0)
 ;     x = recursion_select_and_phi(a, b);
 ;   return b == 0? b : x;
 ; }
 ;
-; BOTH: Function Attrs: noinline nounwind readnone uwtable
+; BOTH: Function Attrs: noinline nosync nounwind readnone uwtable
 ; BOTH-NEXT: define double @recursion_select_and_phi(i32 %a, double returned %b)
 ;
 ; FNATTR:     define double @recursion_select_and_phi(i32 %a, double %b)
-; ATTRIBUTOR: define double @recursion_select_and_phi(i32 %a, double returned %b)
+;
+; ATTRIBUTOR: Function Attrs: noinline nosync nounwind uwtable
+; ATTRIBUTOR-NEXT: define double @recursion_select_and_phi(i32 %a, double returned %b)
 define double @recursion_select_and_phi(i32 %a, double %b) #0 {
 entry:
   %dec = add nsw i32 %a, -1
   %cmp = icmp sgt i32 %a, 0
   br i1 %cmp, label %if.then, label %if.end
 
 if.then:                                          ; preds = %entry
   %call = call double @recursion_select_and_phi(i32 %dec, double %b)
   br label %if.end
 
 if.end:                                           ; preds = %if.then, %entry
   %phi = phi double [ %call, %if.then ], [ %b, %entry ]
   %cmp1 = fcmp oeq double %b, 0.000000e+00
   %sel = select i1 %cmp1, double %b, double %phi
   ret double %sel
 }
 
 
 ; TEST returned argument goes through bitcasts
 ;
 ; double* bitcast(int* b) {
 ;   return (double*)b;
 ; }
 ;
-; BOTH: Function Attrs: noinline norecurse nounwind readnone uwtable
+; BOTH: Function Attrs: noinline norecurse nosync nounwind readnone uwtable
 ; BOTH-NEXT:  define double* @bitcast(i32* readnone returned %b)
 ;
 ; FNATTR:     define double* @bitcast(i32* readnone %b)
-; ATTRIBUTOR: define double* @bitcast(i32* returned %b)
+;
+; ATTRIBUTOR: Function Attrs: noinline nosync nounwind uwtable
+; ATTRIBUTOR-NEXT: define double* @bitcast(i32* returned %b)
 define double* @bitcast(i32* %b) #0 {
 entry:
   %bc0 = bitcast i32* %b to double*
   ret double* %bc0
 }
 
 
 ; TEST returned argument goes through select and phi interleaved with bitcasts
 ;
 ; double* bitcasts_select_and_phi(int* b) {
 ;   double* x = b;
 ;   if (b == 0)
 ;     x = b;
 ;   return b != 0 ? b : x;
 ; }
 ;
-; BOTH: Function Attrs: noinline norecurse nounwind readnone uwtable
+; BOTH: Function Attrs: noinline norecurse nosync nounwind readnone uwtable
 ; BOTH-NEXT: define double* @bitcasts_select_and_phi(i32* readnone returned %b)
 ;
 ; FNATTR:     define double* @bitcasts_select_and_phi(i32* readnone %b)
-; ATTRIBUTOR: define double* @bitcasts_select_and_phi(i32* returned %b)
+;
+; ATTRIBUTOR: Function Attrs: noinline nosync nounwind uwtable
+; ATTRIBUTOR-NEXT: define double* @bitcasts_select_and_phi(i32* returned %b)
 define double* @bitcasts_select_and_phi(i32* %b) #0 {
 entry:
   %bc0 = bitcast i32* %b to double*
   %cmp = icmp eq double* %bc0, null
   br i1 %cmp, label %if.then, label %if.end
 
 if.then:                                          ; preds = %entry
   %bc1 = bitcast i32* %b to double*

 ; double* ret_arg_arg_undef(int* b) {
 ;   if (b == 0)
 ;     return (double*)b;
 ;   if (b == 0)
 ;     return (double*)b;
 ;   /* return undef */
 ; }
 ;
-; BOTH: Function Attrs: noinline norecurse nounwind readnone uwtable
+; BOTH: Function Attrs: noinline norecurse nosync nounwind readnone uwtable
 ; BOTH-NEXT:  define double* @ret_arg_arg_undef(i32* readnone returned %b)
 ;
 ; FNATTR:     define double* @ret_arg_arg_undef(i32* readnone %b)
-; ATTRIBUTOR: define double* @ret_arg_arg_undef(i32* returned %b)
+;
+; ATTRIBUTOR: Function Attrs: noinline nosync nounwind uwtable
+; ATTRIBUTOR-NEXT: define double* @ret_arg_arg_undef(i32* returned %b)
 define double* @ret_arg_arg_undef(i32* %b) #0 {
 entry:
   %bc0 = bitcast i32* %b to double*
   %cmp = icmp eq double* %bc0, null
   br i1 %cmp, label %ret_arg0, label %if.end
 
 ret_arg0:
   %bc1 = bitcast i32* %b to double*

 ; double* ret_undef_arg_arg(int* b) {
 ;   if (b == 0)
 ;     return (double*)b;
 ;   if (b == 0)
 ;     return (double*)b;
 ;   /* return undef */
 ; }
 ;
-; BOTH: Function Attrs: noinline norecurse nounwind readnone uwtable
+; BOTH: Function Attrs: noinline norecurse nosync nounwind readnone uwtable
 ; BOTH-NEXT:  define double* @ret_undef_arg_arg(i32* readnone returned %b)
 ;
 ; FNATTR:     define double* @ret_undef_arg_arg(i32* readnone %b)
-; ATTRIBUTOR: define double* @ret_undef_arg_arg(i32* returned %b)
+;
+; ATTRIBUTOR: Function Attrs: noinline nosync nounwind uwtable
+; ATTRIBUTOR-NEXT: define double* @ret_undef_arg_arg(i32* returned %b)
 define double* @ret_undef_arg_arg(i32* %b) #0 {
 entry:
   %bc0 = bitcast i32* %b to double*
   %cmp = icmp eq double* %bc0, null
   br i1 %cmp, label %ret_undef, label %if.end
 
 ret_undef:
   ret double *undef

 ; double* ret_undef_arg_undef(int* b) {
 ;   if (b == 0)
 ;     /* return undef */
 ;   if (b == 0)
 ;     return (double*)b;
 ;   /* return undef */
 ; }
 ;
-; BOTH: Function Attrs: noinline norecurse nounwind readnone uwtable
+; BOTH: Function Attrs: noinline norecurse nosync nounwind readnone uwtable
 ; BOTH-NEXT:  define double* @ret_undef_arg_undef(i32* readnone returned %b)
 ;
 ; FNATTR:     define double* @ret_undef_arg_undef(i32* readnone %b)
 ; ATTRIBUTOR: define double* @ret_undef_arg_undef(i32* returned %b)
 define double* @ret_undef_arg_undef(i32* %b) #0 {
 entry:
   %bc0 = bitcast i32* %b to double*
   %cmp = icmp eq double* %bc0, null

 unreachableblock2:
   %C = call i32 @deadblockcall1(i32 %C)
   ret i32 %C
 }
 
 attributes #0 = { noinline nounwind uwtable }
 
 ; BOTH-NOT: attributes #
-; BOTH-DAG: attributes #{{[0-9]*}} = { noinline norecurse nounwind readnone uwtable }
-; BOTH-DAG: attributes #{{[0-9]*}} = { noinline nounwind readnone uwtable }
-; BOTH-DAG: attributes #{{[0-9]*}} = { noinline nounwind readonly uwtable }
+; BOTH-DAG: attributes #{{[0-9]*}} = { noinline norecurse nosync nounwind readnone uwtable }
+; BOTH-DAG: attributes #{{[0-9]*}} = { noinline nosync nounwind readnone uwtable }
+; BOTH-DAG: attributes #{{[0-9]*}} = { noinline nosync nounwind readonly uwtable }
 ; BOTH-DAG: attributes #{{[0-9]*}} = { noinline nounwind uwtable }
 ; BOTH-NOT: attributes #

llvm/trunk/test/Transforms/FunctionAttrs/fn_noreturn.ll

 
 ; TEST 1
 ;
 ; void srec0() {
 ;   return srec0();
 ; }
 ;
 ; FIXME: no-return missing
-; CHECK: Function Attrs: noinline nounwind readnone uwtable
+; CHECK: Function Attrs: noinline nosync nounwind readnone uwtable
 ; CHECK: define void @srec0()
 ;
 define void @srec0() #0 {
 entry:
   call void @srec0()
   ret void
 }
 
 
 ; TEST 2
 ;
 ; int srec16(int a) {
 ;   return srec16(srec16(srec16(srec16(srec16(srec16(srec16(srec16(srec16(srec16(srec16(srec16(srec16(srec16(srec16(srec16(a))))))))))))))));
 ; }
 ;
 ; FIXME: no-return missing
-; CHECK: Function Attrs: noinline nounwind readnone uwtable
+; CHECK: Function Attrs: noinline nosync nounwind readnone uwtable
 ; CHECK: define i32 @srec16(i32 %a)
 ;
 define i32 @srec16(i32 %a) #0 {
 entry:
   %call = call i32 @srec16(i32 %a)
   %call1 = call i32 @srec16(i32 %call)
   %call2 = call i32 @srec16(i32 %call1)
   %call3 = call i32 @srec16(i32 %call2)

 
 ; TEST 3
 ;
 ; int endless_loop(int a) {
 ;   while (1);
 ; }
 ;
 ; FIXME: no-return missing
-; CHECK: Function Attrs: noinline norecurse nounwind readnone uwtable
+; CHECK: Function Attrs: noinline norecurse nosync nounwind readnone uwtable
 ; CHECK: define i32 @endless_loop(i32 %a)
 ;
 define i32 @endless_loop(i32 %a) #0 {
 entry:
   br label %while.body
 
 while.body:                                       ; preds = %entry, %while.body
   br label %while.body
 }
 
 
 ; TEST 4
 ;
 ; int endless_loop(int a) {
 ;   while (1);
 ;   return a;
 ; }
 ;
 ; FIXME: no-return missing
-; CHECK: Function Attrs: noinline norecurse nounwind readnone uwtable
+; CHECK: Function Attrs: noinline norecurse nosync nounwind readnone uwtable
 ; CHECK: define i32 @dead_return(i32 returned %a)
 ;
 define i32 @dead_return(i32 %a) #0 {
 entry:
   br label %while.body
 
 while.body:                                       ; preds = %entry, %while.body
   br label %while.body
 
 return:                                           ; No predecessors!
   ret i32 %a
 }
 
 
 ; TEST 5
 ;
 ; int multiple_noreturn_calls(int a) {
 ;   return a == 0 ? endless_loop(a) : srec16(a);
 ; }
 ;
 ; FIXME: no-return missing
-; CHECK: Function Attrs: noinline nounwind readnone uwtable
+; CHECK: Function Attrs: noinline nosync nounwind readnone uwtable
 ; CHECK: define i32 @multiple_noreturn_calls(i32 %a)
 ;
 define i32 @multiple_noreturn_calls(i32 %a) #0 {
 entry:
   %cmp = icmp eq i32 %a, 0
   br i1 %cmp, label %cond.true, label %cond.false
 
 cond.true:                                        ; preds = %entry

llvm/trunk/test/Transforms/FunctionAttrs/nosync.ll

+; RUN: opt -functionattrs -S < %s | FileCheck %s --check-prefix=FNATTR
+; RUN: opt -attributor -attributor-disable=false -S < %s | FileCheck %s --check-prefix=ATTRIBUTOR
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+
+; Test cases designed for the nosync function attribute.
+; FIXME's are used to indicate problems and missing attributes.
+
+; struct RT {
+;   char A;
+;   int B[10][20];
+;   char C;
+; };
+; struct ST {
+;   int X;
+;   double Y;
+;   struct RT Z;
+; };
+;
+; int *foo(struct ST *s) {
+;   return &s[1].Z.B[5][13];
+; }
+
+; TEST 1
+; non-convergent and readnone implies nosync
+%struct.RT = type { i8, [10 x [20 x i32]], i8 }
+%struct.ST = type { i32, double, %struct.RT }
+
+; FNATTR: Function Attrs: norecurse nounwind optsize readnone ssp uwtable
+; FNATTR-NEXT: define nonnull i32* @foo(%struct.ST* readnone %s)
+; ATTRIBUTOR: Function Attrs: nosync nounwind optsize readnone ssp uwtable
+; ATTRIBUTOR-NEXT: define i32* @foo(%struct.ST* %s)
+define i32* @foo(%struct.ST* %s) nounwind uwtable readnone optsize ssp {
+entry:
+  %arrayidx = getelementptr inbounds %struct.ST, %struct.ST* %s, i64 1, i32 2, i32 1, i64 5, i64 13
+  ret i32* %arrayidx
+}
+
+; TEST 2
+; atomic load with monotonic ordering
+; int load_monotonic(_Atomic int *num) {
+;   int n = atomic_load_explicit(num, memory_order_relaxed);
+;   return n;
+; }
+
+; FNATTR: Function Attrs: nofree norecurse nounwind uwtable
+; FNATTR-NEXT: define i32 @load_monotonic(i32* nocapture readonly)
+; ATTRIBUTOR: Function Attrs: norecurse nosync nounwind uwtable
+; ATTRIBUTOR-NEXT: define i32 @load_monotonic(i32* nocapture readonly)
+define i32 @load_monotonic(i32* nocapture readonly) norecurse nounwind uwtable {
+  %2 = load atomic i32, i32* %0 monotonic, align 4
+  ret i32 %2
+}
+
+
+; TEST 3
+; atomic store with monotonic ordering.
+; void store_monotonic(_Atomic int *num) {
+;   atomic_load_explicit(num, memory_order_relaxed);
+; }
+
+; FNATTR: Function Attrs: nofree norecurse nounwind uwtable
+; FNATTR-NEXT: define void @store_monotonic(i32* nocapture)
+; ATTRIBUTOR: Function Attrs: norecurse nosync nounwind uwtable
+; ATTRIBUTOR-NEXT: define void @store_monotonic(i32* nocapture)
+define void @store_monotonic(i32* nocapture) norecurse nounwind uwtable {
+  store atomic i32 10, i32* %0 monotonic, align 4
+  ret void
+}
+
+; TEST 4 - negative, should not deduce nosync
+; atomic load with acquire ordering.
+; int load_acquire(_Atomic int *num) {
+;   int n = atomic_load_explicit(num, memory_order_acquire);
+;   return n;
+; }
+
+; FNATTR: Function Attrs: nofree norecurse nounwind uwtable
+; FNATTR-NEXT: define i32 @load_acquire(i32* nocapture readonly)
+; ATTRIBUTOR: Function Attrs: norecurse nounwind uwtable
+; ATTRIBUTOR-NOT: nosync
+; ATTRIBUTOR-NEXT: define i32 @load_acquire(i32* nocapture readonly)
+define i32 @load_acquire(i32* nocapture readonly) norecurse nounwind uwtable {
+  %2 = load atomic i32, i32* %0 acquire, align 4
+  ret i32 %2
+}
+
+; TEST 5 - negative, should not deduce nosync
+; atomic load with release ordering
+; void load_release(_Atomic int *num) {
+;   atomic_store_explicit(num, 10, memory_order_release);
+; }
+
+; FNATTR: Function Attrs: nofree norecurse nounwind uwtable
+; FNATTR-NEXT: define void @load_release(i32* nocapture)
+; ATTRIBUTOR: Function Attrs: norecurse nounwind uwtable
+; ATTRIBUTOR-NOT: nosync
+; ATTRIBUTOR-NEXT: define void @load_release(i32* nocapture)
+define void @load_release(i32* nocapture) norecurse nounwind uwtable {
+  store atomic volatile i32 10, i32* %0 release, align 4
+  ret void
+}
+
+; TEST 6 - negative volatile, relaxed atomic
+
+; FNATTR: Function Attrs: nofree norecurse nounwind uwtable
+; FNATTR-NEXT: define void @load_volatile_release(i32* nocapture)
+; ATTRIBUTOR: Function Attrs: norecurse nounwind uwtable
+; ATTRIBUTOR-NOT: nosync
+; ATTRIBUTOR-NEXT: define void @load_volatile_release(i32* nocapture)
+define void @load_volatile_release(i32* nocapture) norecurse nounwind uwtable {
+  store atomic volatile i32 10, i32* %0 release, align 4
+  ret void
+}
+
+; TEST 7 - negative, should not deduce nosync
+; volatile store.
+; void volatile_store(volatile int *num) {
+;   *num = 14;
+; }
+
+; FNATTR: Function Attrs: nofree norecurse nounwind uwtable
+; FNATTR-NEXT: define void @volatile_store(i32*)
+; ATTRIBUTOR: Function Attrs: norecurse nounwind uwtable
+; ATTRIBUTOR-NOT: nosync
+; ATTRIBUTOR-NEXT: define void @volatile_store(i32*)
+define void @volatile_store(i32*) norecurse nounwind uwtable {
+  store volatile i32 14, i32* %0, align 4
+  ret void
+}
+
+; TEST 8 - negative, should not deduce nosync
+; volatile load.
+; int volatile_load(volatile int *num) {
+;   int n = *num;
+;   return n;
+; }
+
+; FNATTR: Function Attrs: nofree norecurse nounwind uwtable
+; FNATTR-NEXT: define i32 @volatile_load(i32*)
+; ATTRIBUTOR: Function Attrs: norecurse nounwind uwtable
+; ATTRIBUTOR-NOT: nosync
+; ATTRIBUTOR-NEXT: define i32 @volatile_load(i32*)
+define i32 @volatile_load(i32*) norecurse nounwind uwtable {
+  %2 = load volatile i32, i32* %0, align 4
+  ret i32 %2
+}
+
+; TEST 9
+
+; FNATTR: Function Attrs: noinline nosync nounwind uwtable
+; FNATTR-NEXT: declare void @nosync_function()
+; ATTRIBUTOR: Function Attrs: noinline nosync nounwind uwtable
+; ATTRIBUTOR-NEXT: declare void @nosync_function()
+declare void @nosync_function() noinline nounwind uwtable nosync
+
+; FNATTR: Function Attrs: noinline nounwind uwtable
+; FNATTR-NEXT: define void @call_nosync_function()
+; ATTRIBUTOR: Function Attrs: noinline nosync nounwind uwtable
+; ATTRIBUTOR-next: define void @call_nosync_function()
+define void @call_nosync_function() nounwind uwtable noinline {
+  tail call void @nosync_function() noinline nounwind uwtable
+  ret void
+}
+
+; TEST 10 - negative, should not deduce nosync
+
+; FNATTR: Function Attrs: noinline nounwind uwtable
+; FNATTR-NEXT: declare void @might_sync()
+; ATTRIBUTOR: Function Attrs: noinline nounwind uwtable
+; ATTRIBUTOR-NEXT: declare void @might_sync()
+declare void @might_sync() noinline nounwind uwtable
+
+; FNATTR: Function Attrs: noinline nounwind uwtable
+; FNATTR-NEXT: define void @call_might_sync()
+; ATTRIBUTOR: Function Attrs: noinline nounwind uwtable
+; ATTRIBUTOR-NOT: nosync
+; ATTRIBUTOR-NEXT: define void @call_might_sync()
+define void @call_might_sync() nounwind uwtable noinline {
+  tail call void @might_sync() noinline nounwind uwtable
+  ret void
+}
+
+; TEST 11 - negative, should not deduce nosync
+; volatile operation in same scc. Call volatile_load defined in TEST 8.
+
+; FNATTR: Function Attrs: nofree noinline nounwind uwtable
+; FNATTR-NEXT: define i32 @scc1(i32*)
+; ATTRIBUTOR: Function Attrs: noinline nounwind uwtable
+; ATTRIBUTOR-NOT: nosync
+; ATTRIBUTOR-NEXT: define i32 @scc1(i32*)
+define i32 @scc1(i32*) noinline nounwind uwtable {
+  tail call void @scc2(i32* %0);
+  %val = tail call i32 @volatile_load(i32* %0);
+  ret i32 %val;
+}
+
+; FNATTR: Function Attrs: nofree noinline nounwind uwtable
+; FNATTR-NEXT: define void @scc2(i32*)
+; ATTRIBUTOR: Function Attrs: noinline nounwind uwtable
+; ATTRIBUTOR-NOT: nosync
+; ATTRIBUTOR-NEXT: define void @scc2(i32*)
+define void @scc2(i32*) noinline nounwind uwtable {
+  tail call i32 @scc1(i32* %0);
+  ret void;
+}
+
+; TEST 12 - fences, negative
+;
+; void foo1(int *a, std::atomic<bool> flag){
+;   *a = 100;
+;   atomic_thread_fence(std::memory_order_release);
+;   flag.store(true, std::memory_order_relaxed);
+; }
+;
+; void bar(int *a, std::atomic<bool> flag){
+;   while(!flag.load(std::memory_order_relaxed))
+;     ;
+;
+;   atomic_thread_fence(std::memory_order_acquire);
+;   int b = *a;
+; }
+
+%"struct.std::atomic" = type { %"struct.std::__atomic_base" }
+%"struct.std::__atomic_base" = type { i8 }
+
+; FNATTR: Function Attrs: nofree norecurse nounwind
+; FNATTR-NEXT: define void @foo1(i32* nocapture, %"struct.std::atomic"* nocapture)
+; ATTRIBUTOR-NOT: nosync
+; ATTRIBUTOR: define void @foo1(i32*, %"struct.std::atomic"*)
+define void @foo1(i32*, %"struct.std::atomic"*) {
+  store i32 100, i32* %0, align 4
+  fence release
+  %3 = getelementptr inbounds %"struct.std::atomic", %"struct.std::atomic"* %1, i64 0, i32 0, i32 0
+  store atomic i8 1, i8* %3 monotonic, align 1
+  ret void
+}
+
+; FNATTR: Function Attrs: nofree norecurse nounwind
+; FNATTR-NEXT: define void @bar(i32* nocapture readnone, %"struct.std::atomic"* nocapture readonly)
+; ATTRIBUTOR-NOT: nosync
+; ATTRIBUTOR: define void @bar(i32*, %"struct.std::atomic"*)
+define void @bar(i32 *, %"struct.std::atomic"*) {
+  %3 = getelementptr inbounds %"struct.std::atomic", %"struct.std::atomic"* %1, i64 0, i32 0, i32 0
+  br label %4
+
+4:                                                ; preds = %4, %2
+  %5 = load atomic i8, i8* %3  monotonic, align 1
+  %6 = and i8 %5, 1
+  %7 = icmp eq i8 %6, 0
+  br i1 %7, label %4, label %8
+
+8:                                                ; preds = %4
+  fence acquire
+  ret void
+}
+
+; TEST 13 - Fence syncscope("singlethread") seq_cst
+; FNATTR: Function Attrs: nofree norecurse nounwind
+; FNATTR-NEXT: define void @foo1_singlethread(i32* nocapture, %"struct.std::atomic"* nocapture)
+; ATTRIBUTOR: Function Attrs: nosync
+; ATTRIBUTOR: define void @foo1_singlethread(i32*, %"struct.std::atomic"*)
+define void @foo1_singlethread(i32*, %"struct.std::atomic"*) {
+  store i32 100, i32* %0, align 4
+  fence syncscope("singlethread") release
+  %3 = getelementptr inbounds %"struct.std::atomic", %"struct.std::atomic"* %1, i64 0, i32 0, i32 0
+  store atomic i8 1, i8* %3 monotonic, align 1
+  ret void
+}
+
+; FNATTR: Function Attrs: nofree norecurse nounwind
+; FNATTR-NEXT: define void @bar_singlethread(i32* nocapture readnone, %"struct.std::atomic"* nocapture readonly)
+; ATTRIBUTOR: Function Attrs: nosync
+; ATTRIBUTOR: define void @bar_singlethread(i32*, %"struct.std::atomic"*)
+define void @bar_singlethread(i32 *, %"struct.std::atomic"*) {
+  %3 = getelementptr inbounds %"struct.std::atomic", %"struct.std::atomic"* %1, i64 0, i32 0, i32 0
+  br label %4
+
+4:                                                ; preds = %4, %2
+  %5 = load atomic i8, i8* %3  monotonic, align 1
+  %6 = and i8 %5, 1
+  %7 = icmp eq i8 %6, 0
+  br i1 %7, label %4, label %8
+
+8:                                                ; preds = %4
+  fence syncscope("singlethread") acquire
+  ret void
+}
+
+declare void @llvm.memcpy(i8* %dest, i8* %src, i32 %len, i1 %isvolatile)
+declare void @llvm.memset(i8* %dest, i8 %val, i32 %len, i1 %isvolatile)
+
+; TEST 14 - negative, checking volatile intrinsics.
+
+; ATTRIBUTOR: Function Attrs: nounwind
+; ATTRIBUTOR-NOT: nosync
+; ATTRIBUTOR-NEXT: define i32 @memcpy_volatile(i8* %ptr1, i8* %ptr2)
+define i32 @memcpy_volatile(i8* %ptr1, i8* %ptr2) {
+  call void @llvm.memcpy(i8* %ptr1, i8* %ptr2, i32 8, i1 1)
+  ret i32 4
+}
+
+; TEST 15 - positive, non-volatile intrinsic.
+
+; ATTRIBUTOR: Function Attrs: nosync
+; ATTRIBUTOR-NEXT: define i32 @memset_non_volatile(i8* %ptr1, i8 %val)
+define i32 @memset_non_volatile(i8* %ptr1, i8 %val) {
+  call void @llvm.memset(i8* %ptr1, i8 %val, i32 8, i1 0)
+  ret i32 4
+}
+
+; TEST 16 - negative, inline assembly.
+
+; ATTRIBUTOR: define i32 @inline_asm_test(i32 %x)
+define i32 @inline_asm_test(i32 %x) {
+  call i32 asm "bswap $0", "=r,r"(i32 %x)
+  ret i32 4
+}
+
+declare void @readnone_test() convergent readnone
+
+; ATTRIBUTOR: define void @convergent_readnone()
+; TEST 17 - negative. Convergent
+define void @convergent_readnone(){
+    call void @readnone_test()
+    ret void
+}
+
+; ATTRIBUTOR: Function Attrs: nounwind
+; ATTRIBUTOR-NEXT: declare void @llvm.x86.sse2.clflush(i8*)
+declare void @llvm.x86.sse2.clflush(i8*)
+@a = common global i32 0, align 4
+
+; TEST 18 - negative. Synchronizing intrinsic
+
+; ATTRIBUTOR: Function Attrs: nounwind
+; ATTRIBUTOR-NOT: nosync
+; ATTRIBUTOR-NEXT: define void @i_totally_sync()
+define void @i_totally_sync() {
+  tail call void @llvm.x86.sse2.clflush(i8* bitcast (i32* @a to i8*))
+  ret void
+}
+
+declare float @llvm.cos(float %val) readnone
+
+; TEST 19 - positive, readnone & non-convergent intrinsic.
+
+; ATTRIBUTOR: Function Attrs: nosync nounwind
+; ATTRIBUTOR-NEXT: define i32 @cos_test(float %x)
+define i32 @cos_test(float %x) {
+  call float @llvm.cos(float %x)
+  ret i32 4
+}

llvm/trunk/test/Transforms/FunctionAttrs/nounwind.ll

 ; RUN: opt < %s -functionattrs -S | FileCheck %s
 ; RUN: opt < %s -attributor -attributor-disable=false -S | FileCheck %s --check-prefix=ATTRIBUTOR
 
 ; TEST 1
 ; CHECK: Function Attrs: norecurse nounwind readnone
 ; CHECK-NEXT: define i32 @foo1()
-; ATTRIBUTOR: Function Attrs: nounwind
+; ATTRIBUTOR: Function Attrs: nosync nounwind
 ; ATTRIBUTOR-NEXT: define i32 @foo1()
 define i32 @foo1() {
   ret i32 1
 }
 
 ; TEST 2
 ; CHECK: Function Attrs: nounwind readnone
 ; CHECK-NEXT: define i32 @scc1_foo()
-; ATTRIBUTOR: Function Attrs: nounwind
+; ATTRIBUTOR: Function Attrs: nosync nounwind
 ; ATTRIBUTOR-NEXT: define i32 @scc1_foo()
 define i32 @scc1_foo() {
   %1 = call i32 @scc1_bar()
   ret i32 1
 }
 
 
 ; TEST 3
 ; CHECK: Function Attrs: nounwind readnone
 ; CHECK-NEXT: define i32 @scc1_bar()
-; ATTRIBUTOR: Function Attrs: nounwind
+; ATTRIBUTOR: Function Attrs: nosync nounwind
 ; ATTRIBUTOR-NEXT: define i32 @scc1_bar()
 define i32 @scc1_bar() {
   %1 = call i32 @scc1_foo()
   ret i32 1
 }
 
 ; CHECK: declare i32 @non_nounwind()
 declare i32 @non_nounwind()

llvm/trunk/test/Transforms/FunctionAttrs/read_write_returned_arguments_scc.ll

 ;  16 - Number of arguments marked nocapture
 ;   4 - Number of arguments marked readnone
 ;   6 - Number of arguments marked writeonly
 ;   6 - Number of arguments marked readonly
 ;   6 - Number of arguments marked returned
 ;
 target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
 
-; CHECK: Function Attrs: nofree nounwind
+; CHECK: Function Attrs: nofree nosync nounwind
 ; CHECK-NEXT: define i32* @external_ret2_nrw(i32* %n0, i32* %r0, i32* returned %w0)
 define i32* @external_ret2_nrw(i32* %n0, i32* %r0, i32* %w0) {
 entry:
   %call = call i32* @internal_ret0_nw(i32* %n0, i32* %w0)
   %call1 = call i32* @internal_ret1_rrw(i32* %r0, i32* %r0, i32* %w0)
   %call2 = call i32* @external_sink_ret2_nrw(i32* %n0, i32* %r0, i32* %w0)
   %call3 = call i32* @internal_ret1_rw(i32* %r0, i32* %w0)
   ret i32* %call3
 }
 
-; CHECK: Function Attrs: nofree nounwind
+; CHECK: Function Attrs: nofree nosync nounwind
 ; CHECK-NEXT: define internal i32* @internal_ret0_nw(i32* returned %n0, i32* %w0)
 define internal i32* @internal_ret0_nw(i32* %n0, i32* %w0) {
 entry:
   %r0 = alloca i32, align 4
   %r1 = alloca i32, align 4
   %tobool = icmp ne i32* %n0, null
   br i1 %tobool, label %if.end, label %if.then
 
if.end:                                           ; preds = %entry
   %call5 = call i32* @internal_ret0_nw(i32* %n0, i32* %w0)
   br label %return
 
 return:                                           ; preds = %if.end, %if.then
   %retval.0 = phi i32* [ %call5, %if.end ], [ %n0, %if.then ]
   ret i32* %retval.0
 }
 
-; CHECK: Function Attrs: nofree nounwind
+; CHECK: Function Attrs: nofree nosync nounwind
 ; CHECK-NEXT: define internal i32* @internal_ret1_rrw(i32* %r0, i32* returned %r1, i32* %w0)
 define internal i32* @internal_ret1_rrw(i32* %r0, i32* %r1, i32* %w0) {
 entry:
   %0 = load i32, i32* %r0, align 4
   %tobool = icmp ne i32 %0, 0
   br i1 %tobool, label %if.end, label %if.then
 
 if.then:                                          ; preds = %entry
if.end:                                           ; preds = %entry
   %call8 = call i32* @internal_ret0_nw(i32* %r1, i32* %w0)
   br label %return
 
 return:                                           ; preds = %if.end, %if.then
   %retval.0 = phi i32* [ %call8, %if.end ], [ %r1, %if.then ]
   ret i32* %retval.0
 }
 
-; CHECK: Function Attrs: nofree norecurse nounwind
+; CHECK: Function Attrs: nofree norecurse nosync nounwind
 ; CHECK-NEXT: define i32* @external_sink_ret2_nrw(i32* readnone %n0, i32* nocapture readonly %r0, i32* returned %w0)
 define i32* @external_sink_ret2_nrw(i32* %n0, i32* %r0, i32* %w0) {
 entry:
   %tobool = icmp ne i32* %n0, null
   br i1 %tobool, label %if.end, label %if.then
 
 if.then:                                          ; preds = %entry
   br label %return
 
 if.end:                                           ; preds = %entry
   %0 = load i32, i32* %r0, align 4
   store i32 %0, i32* %w0, align 4
   br label %return
 
 return:                                           ; preds = %if.end, %if.then
   ret i32* %w0
 }
 
-; CHECK: Function Attrs: nofree nounwind
+; CHECK: Function Attrs: nofree nosync nounwind
 ; CHECK-NEXT: define internal i32* @internal_ret1_rw(i32* %r0, i32* returned %w0)
 define internal i32* @internal_ret1_rw(i32* %r0, i32* %w0) {
 entry:
   %0 = load i32, i32* %r0, align 4
   %tobool = icmp ne i32 %0, 0
   br i1 %tobool, label %if.end, label %if.then
 
 if.then:                                          ; preds = %entry
if.end:                                           ; preds = %entry
   %call4 = call i32* @external_ret2_nrw(i32* %r0, i32* %r0, i32* %w0)
   br label %return
 
 return:                                           ; preds = %if.end, %if.then
   %retval.0 = phi i32* [ %call4, %if.end ], [ %w0, %if.then ]
   ret i32* %retval.0
 }
 
-; CHECK: Function Attrs: nofree nounwind
+; CHECK: Function Attrs: nofree nosync nounwind
 ; CHECK-NEXT: define i32* @external_source_ret2_nrw(i32* %n0, i32* %r0, i32* returned %w0)
 define i32* @external_source_ret2_nrw(i32* %n0, i32* %r0, i32* %w0) {
 entry:
   %call = call i32* @external_sink_ret2_nrw(i32* %n0, i32* %r0, i32* %w0)
   %call1 = call i32* @external_ret2_nrw(i32* %n0, i32* %r0, i32* %w0)
   ret i32* %call1
 }
 
 ; Verify that we see only expected attribute sets, the above lines only check
 ; for a subset relation.
 ;
 ; CHECK-NOT: attributes #
-; CHECK: attributes #{{.*}} = { nofree nounwind }
-; CHECK: attributes #{{.*}} = { nofree norecurse nounwind }
+; CHECK: attributes #{{.*}} = { nofree nosync nounwind }
+; CHECK: attributes #{{.*}} = { nofree norecurse nosync nounwind }
 ; CHECK-NOT: attributes #