Tests for nosync function attribute.

[sstefan1]

Summary

This commit is test for upcoming patch on nosync attribute deduction

[jdoerfert]

Some comments inlined. Please also add:

• SCC tests with multiple functions calling each other. You don't need them for all kinds of sync operations but a positive and a negative test with volatile would be good.
• Test with calls to declared functions. Positive (with nosync attribute present) and negative again.

llvm/test/Transforms/FunctionAttrs/nosync.ll

45

Remove the dso_local and local_unamed_addr everywhere

71

Maybe mention that we should *not* deduce nosync in the negative test cases.

117

We don't need these

[sstefan1]

• updated tests for nosync.

[sstefan1]

Addressed most of the comments.

In D62424#1517384, @jdoerfert wrote:

a positive and a negative test with volatile would be good.

What can be the positive case for SCC tests with volatile? Isn't volatile always supposed to sync?

[sstefan1]

• typo in test 10.

[jdoerfert]

In D62424#1517471, @sstefan1 wrote:

Addressed most of the comments.

In D62424#1517384, @jdoerfert wrote:

a positive and a negative test with volatile would be good.

What can be the positive case for SCC tests with volatile? Isn't volatile always supposed to sync?

Good point. Ignore that part.

[jdoerfert]

llvm/test/Transforms/FunctionAttrs/nosync.ll

71

Add a
CHECK-NOT: nosync
after the check line because you could otherwise have nosync at the end and pass anyway.

117

FIXME: nosync missing
instead of nosync in the attribute list

We should be able to pass the test right now so we can integrate it already.
To this end, please also use the string attribute syntax "nosync" which should be accepted right now. (You can run lit on the test)

[jdoerfert]

I'm now also unsure if a Release build will print the definitions are you check for them, that is the attributes listed at the end.
I think the form below should always work.

; CHECK:      Function Attrs: noinline nounwind uwtable
; CHECK-NEXT: define void @scc2(i32*)

[sstefan1]

• updated tests. Will run lit first thing in the morning.

[jdoerfert]

llvm/test/Transforms/FunctionAttrs/nosync.ll

76

This is my bad but I think the combination of my prior to requests will not work.

Could you see if

; CHECK: Function Attrs: norecurse nounwind uwtable
; CHECK-NOT: nosync
; CHECK-NEXT: define i32 @load_acquire(i32* nocapture readonly)

works and if not remove the NEXT part.

[sstefan1]

• Fixed tests. Removed some 'CHECK-NEXT' directives.
• Test now passing.

CHECK-NEXT was the problem in TEST 8 & 9 so I removed it only from those two. Is that ok?

[jdoerfert]

In D62424#1520080, @sstefan1 wrote:

• Fixed tests. Removed some 'CHECK-NEXT' directives.
• Test now passing.

CHECK-NEXT was the problem in TEST 8 & 9 so I removed it only from those two. Is that ok?

What was the problem there? How does the output actually look?

[sstefan1]

In D62424#1520101, @jdoerfert wrote:

What was the problem there? How does the output actually look?

nosync.ll:130:15: error: CHECK-NEXT: is not on the line after the previous match
; CHECK-NEXT: define void @call_nosync_function()
          ^
<stdin>:55:1: note: 'next' match was here
define void @call_nosync_function() #2 {
^
<stdin>:51:44: note: previous match ended here
; Function Attrs: noinline nounwind uwtable
                                       ^
<stdin>:52:1: note: non-matching line after previous match is here
declare void @nosync_function() #2
^

[jdoerfert]

In D62424#1520564, @sstefan1 wrote:

In D62424#1520101, @jdoerfert wrote:

What was the problem there? How does the output actually look?

nosync.ll:130:15: error: CHECK-NEXT: is not on the line after the previous match
; CHECK-NEXT: define void @call_nosync_function()
          ^
<stdin>:55:1: note: 'next' match was here
define void @call_nosync_function() #2 {
^
<stdin>:51:44: note: previous match ended here
; Function Attrs: noinline nounwind uwtable
                                       ^
<stdin>:52:1: note: non-matching line after previous match is here
declare void @nosync_function() #2
^

Good thing we looked into this. You should look at the IR output (without lit) to see what the problem is but I would assume the decleration of nosync_function is placed before call_nosync_function. Then, the CHECK line you added for the attributes of the latter actually matches the attributes of the former. That would mean, adding another check line for the former, including the check-not nosync, will fix the issue and check-next will do what we want.

[sstefan1]

fixed the problem from last comment.

[jdoerfert]

LGTM

[jdoerfert]

Afaik, this is in tree.

llvm/test/Transforms/FunctionAttrs/nosync.ll

+; RUN: opt -functionattrs -S < %s | FileCheck %s
+target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
+
+; Test cases designet 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
+; attribute readnone implies "nosync"
+%struct.RT = type { i8, [10 x [20 x i32]], i8 }
+%struct.ST = type { i32, double, %struct.RT }
+
+; FIXME: missing "nosync"
+; CHECK Function Attrs: nounwind uwtable readnone optsize ssp
+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;
+; }
+  
+; FIXME: missing "nosync"
+; CHECK: Function Attrs: norecurse nounwind uwtable
+; CHECK-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);
+; }
+
+; FIXME: missing "nosync"
+; CHECK: Function Attrs: norecurse nounwind uwtable
+; CHECK-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;
+; }
+
+; CHECK: Function Attrs: norecurse nounwind uwtable
+; CHECK-NOT: nosync
+; CHECK-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);
+; }
+
+; CHECK: Function Attrs: norecurse nounwind uwtable
+; CHECK-NOT: nosync
+; CHECK-NEXT: define void @load_release(i32* nocapture)
+define void @load_release(i32* nocapture) norecurse nounwind uwtable {
+  store atomic i32 10, i32* %0 release, align 4
+  ret void
+}
+
+; TEST 6 - negative, should not deduce "nosync"
+; volatile store.
+; void volatile_store(volatile int *num) {
+;   *num = 14;
+; }
+
+; CHECK: Function Attrs: norecurse nounwind uwtable
+; CHECK-NOT: nosync
+; CHECK-NEXT: define void @volatile_store(i32*)
+define void @volatile_store(i32*) norecurse nounwind uwtable {
+  ;store volatile i32 14, i32* %0, align 4, !tbaa !2
+  store volatile i32 14, i32* %0, align 4
+  ret void
+}
+
+; TEST 7 - negative, should not deduce "nosync"
+; volatile load.
+; int volatile_load(volatile int *num) {
+;   int n = *num;
+;   return n;
+; }
+
+; CHECK: Function Attrs: norecurse nounwind uwtable
+; CHECK-NOT: nosync
+; CHECK-NEXT: define i32 @volatile_load(i32*)
+define i32 @volatile_load(i32*) norecurse nounwind uwtable {
+  ;%2 = load volatile i32, i32* %0, align 4, !tbaa !2
+  %2 = load volatile i32, i32* %0, align 4
+  ret i32 %2
+}
+
+; TEST 8 
+declare void @nosync_function() noinline nounwind uwtable
+
+; FIXME: missing "nosync"
+; CHECK: Function Attrs: noinline nounwind uwtable
+; CHECK: define void @call_nosync_function()
+define void @call_nosync_function() nounwind uwtable noinline {
+  tail call void @nosync_function() noinline nounwind uwtable
+  ret void
+}
+
+; TEST 9 - negative, should not deduce "nosync"
+declare void @might_sync() noinline nounwind uwtable
+
+; CHECK: Function Attrs: noinline nounwind uwtable
+; CHECK-NOT: nosync
+; CHECK: define void @call_might_sync()
+define void @call_might_sync() nounwind uwtable noinline {
+  tail call void @might_sync() noinline nounwind uwtable
+  ret void
+}
+
+; TEST 10 - negative, should not deduce "nosync"
+; volatile operation in same scc. Call volatile_load defined in TEST 7.
+
+; CHECK: Function Attrs: noinline nounwind uwtable
+; CHECK-NOT: nosync
+; CHECK-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;
+}
+
+; CHECK: Function Attrs: noinline nounwind uwtable
+; CHECK-NOT: nosync
+; CHECK-NEXT: define void @scc2(i32*)
+define void @scc2(i32*) noinline nounwind uwtable {
+  tail call i32 @scc1(i32* %0);
+  ret void;
+}