[FunctionAttrs] Volatile operations can access inaccessible memory

[nikic]

Summary

Per LangRef, volatile operations are allowed to access the location of their pointer argument, plus inaccessible memory:

Any volatile operation can have side effects, and any volatile operation can read and/or modify state which is not accessible via a regular load or store in this module.
[...]
The allowed side-effects for volatile accesses are limited. If a non-volatile store to a given address would be legal, a volatile operation may modify the memory at that address. A volatile operation may not modify any other memory accessible by the module being compiled. A volatile operation may not call any code in the current module.

FunctionAttrs currently does not model this and ends up marking functions with volatile accesses on arguments as argmemonly, even though they should be inaccessiblemem_or_argmemonly.

[jdoerfert]

Not an expert on our requirements for volatile. I agree that this is the more conservative approach though. Fine with me.

[nikic]

Rebase over rename.

[efriedma]

LGTM

llvm/lib/Transforms/IPO/FunctionAttrs.cpp

for (Instruction &I : instructions(F)) {
     Optional<MemoryLocation> Loc = MemoryLocation::getOrNone(&I);
     if (!Loc) {
       // If no location is known, conservatively assume anything can be
       // accessed.
       ME |= MemoryEffects(MR);
       continue;
     }
 
-    // Ignore non-volatile accesses from local memory. (Atomic is okay here.)
-    if (!I.isVolatile() && AAR.pointsToConstantMemory(*Loc, /*OrLocal=*/true))
+    // Volatile operations may access inaccessible memory.
+    if (I.isVolatile())
+      ME |= MemoryEffects::inaccessibleMemOnly(MR);
+
+    // Ignore accesses to local memory.
+    if (AAR.pointsToConstantMemory(*Loc, /*OrLocal=*/true))
       continue;
 
     // The accessed location can be either only argument memory, or
     // argument & other memory, but never inaccessible memory.
     ME |= MemoryEffects::argMemOnly(MR);
     if (!IsArgumentOrAlloca(Loc->Ptr))
       ME |= MemoryEffects(MemoryEffects::Other, MR);
   }

llvm/test/Transforms/FunctionAttrs/nosync.ll

;
   store atomic i32 10, ptr %0 unordered, align 4
   ret void
 }
 
 
 ; negative, should not deduce nosync
 ; atomic load with release ordering
 define void @load_release(ptr nocapture %0) norecurse nounwind uwtable {
-; CHECK: Function Attrs: argmemonly nofree norecurse nounwind uwtable
+; CHECK: Function Attrs: inaccessiblemem_or_argmemonly nofree norecurse nounwind uwtable
 ; CHECK-LABEL: @load_release(
 ; CHECK-NEXT:    store atomic volatile i32 10, ptr [[TMP0:%.*]] release, align 4
 ; CHECK-NEXT:    ret void
 ;
   store atomic volatile i32 10, ptr %0 release, align 4
   ret void
 }
 
 ; negative volatile, relaxed atomic
 define void @load_volatile_release(ptr nocapture %0) norecurse nounwind uwtable {
-; CHECK: Function Attrs: argmemonly nofree norecurse nounwind uwtable
+; CHECK: Function Attrs: inaccessiblemem_or_argmemonly nofree norecurse nounwind uwtable
 ; CHECK-LABEL: @load_volatile_release(
 ; CHECK-NEXT:    store atomic volatile i32 10, ptr [[TMP0:%.*]] release, align 4
 ; CHECK-NEXT:    ret void
 ;
   store atomic volatile i32 10, ptr %0 release, align 4
   ret void
 }
 
 ; volatile store.
 define void @volatile_store(ptr %0) norecurse nounwind uwtable {
-; CHECK: Function Attrs: argmemonly nofree norecurse nounwind uwtable
+; CHECK: Function Attrs: inaccessiblemem_or_argmemonly nofree norecurse nounwind uwtable
 ; CHECK-LABEL: @volatile_store(
 ; CHECK-NEXT:    store volatile i32 14, ptr [[TMP0:%.*]], align 4
 ; CHECK-NEXT:    ret void
 ;
   store volatile i32 14, ptr %0, align 4
   ret void
 }
 
 ; negative, should not deduce nosync
 ; volatile load.
 define i32 @volatile_load(ptr %0) norecurse nounwind uwtable {
-; CHECK: Function Attrs: argmemonly mustprogress nofree norecurse nounwind willreturn uwtable
+; CHECK: Function Attrs: inaccessiblemem_or_argmemonly mustprogress nofree norecurse nounwind willreturn uwtable
 ; CHECK-LABEL: @volatile_load(
 ; CHECK-NEXT:    [[TMP2:%.*]] = load volatile i32, ptr [[TMP0:%.*]], align 4
 ; CHECK-NEXT:    ret i32 [[TMP2]]
 ;
   %2 = load volatile i32, ptr %0, align 4
   ret i32 %2
 }
 
 ; CHECK: Function Attrs: noinline nosync nounwind uwtable
 ; CHECK-NEXT: declare void @nosync_function()
 declare void @nosync_function() noinline nounwind uwtable nosync
 
 define void @call_nosync_function() nounwind uwtable noinline {
 ; CHECK: Function Attrs: noinline nosync nounwind uwtable
 ; CHECK-LABEL: @call_nosync_function(
-; CHECK-NEXT:    tail call void @nosync_function() #[[ATTR10:[0-9]+]]
+; CHECK-NEXT:    tail call void @nosync_function() #[[ATTR11:[0-9]+]]
 ; CHECK-NEXT:    ret void
 ;
   tail call void @nosync_function() noinline nounwind uwtable
   ret void
 }
 
 ; CHECK: Function Attrs: noinline nounwind uwtable
 ; CHECK-NEXT: declare void @might_sync()
 declare void @might_sync() noinline nounwind uwtable
 
 define void @call_might_sync() nounwind uwtable noinline {
 ; CHECK: Function Attrs: noinline nounwind uwtable
 ; CHECK-LABEL: @call_might_sync(
-; CHECK-NEXT:    tail call void @might_sync() #[[ATTR10]]
+; CHECK-NEXT:    tail call void @might_sync() #[[ATTR11]]
 ; CHECK-NEXT:    ret void
 ;
   tail call void @might_sync() noinline nounwind uwtable
   ret void
 }
 
 declare void @llvm.memcpy(ptr %dest, ptr %src, i32 %len, i1 %isvolatile)
 declare void @llvm.memset(ptr %dest, i8 %val, i32 %len, i1 %isvolatile)

llvm/test/Transforms/FunctionAttrs/readattrs.ll

 ; CHECK-NEXT:    [[RES:%.*]] = call <4 x i32> @test12_1(<4 x ptr> [[PTRS]])
 ; CHECK-NEXT:    ret <4 x i32> [[RES]]
 ;
   %res = call <4 x i32> @test12_1(<4 x ptr> %ptrs)
   ret <4 x i32> %res
 }
 
 define i32 @volatile_load(ptr %p) {
-; CHECK: Function Attrs: argmemonly mustprogress nofree norecurse nounwind willreturn
+; CHECK: Function Attrs: inaccessiblemem_or_argmemonly mustprogress nofree norecurse nounwind willreturn
 ; CHECK-LABEL: define {{[^@]+}}@volatile_load
 ; CHECK-SAME: (ptr [[P:%.*]]) #[[ATTR13:[0-9]+]] {
 ; CHECK-NEXT:    [[LOAD:%.*]] = load volatile i32, ptr [[P]], align 4
 ; CHECK-NEXT:    ret i32 [[LOAD]]
 ;
   %load = load volatile i32, ptr %p
   ret i32 %load
 }

llvm/test/Transforms/FunctionAttrs/writeonly.ll

 ; CHECK-NEXT:    ret void
 ;
   %v = load i8, ptr %p
   store i8 %v, ptr %p
   ret void
 }
 
 define void @test_volatile(ptr %p) {
-; CHECK: Function Attrs: argmemonly nofree norecurse nounwind
+; CHECK: Function Attrs: inaccessiblemem_or_argmemonly nofree norecurse nounwind
 ; CHECK-LABEL: define {{[^@]+}}@test_volatile
 ; CHECK-SAME: (ptr [[P:%.*]]) #[[ATTR6:[0-9]+]] {
 ; CHECK-NEXT:    store volatile i8 0, ptr [[P]], align 1
 ; CHECK-NEXT:    ret void
 ;
   store volatile i8 0, ptr %p
   ret void
 }