Invariant start/end intrinsics overloaded for address space

[anna]

Summary

The llvm.invariant.start and llvm.invariant.end intrinsics currently
support specifying invariant memory objects only in the default address space.

With this change, these intrinsics are overloaded for any adddress space for memory objects
and we can use these llvm invariant intrinsics in non-default address spaces.

Example: llvm.invariant.start.p1i8(i64 4, i8 addrspace(1)* %ptr)

This overloaded intrinsic is needed for representing final or invariant memory in managed languages.

[anna]

Ran regression tests on all supported platforms.

[apilipenko]

Please add a test case for autoupgrade functionality. See auto_upgrade_intrinsics.ll for examples.

docs/LangRef.rst

11845

I'd omit this for the sake of consistency with other intrinsics.

lib/IR/AutoUpgrade.cpp

155

I'd extract locals for Arg[1] and Arg[2]. The name of these variables will make it clear what the overloaded types are for.

[anna]

Addressed comments above: added test for auto uppgrade, localized args in Auto upgrade code.

docs/LangRef.rst

11845

I actually added the statement for the sake of consistency wth other overloaded intrinsics :)

All intrinsics that overload explicitly state that fact (examples: llvm.memcpy: This is an overloaded intrinsic. You can use llvm.memcpy on any
integer bit width and for different address spaces.)

[apilipenko]

LGTM

lib/IR/AutoUpgrade.cpp

154

It's a Type not a Value, right? That's not obvious. I'd name it accordingly or/and use the type instead of auto.

[anna]

changed from auto to type *.

[anna]

Reverted change in r276320, due to failure in clang.
Assertion failure:
clang: /var/lib/buildbot/slaves/hexagon-build-03/clang-hexagon-elf/llvm/include/llvm/ADT/ArrayRef.h:196: const T &llvm::ArrayRef<llvm::Type *>::operator[](size_t) const [T = llvm::Type *]: Assertion `Index < Length && "Invalid index!"' failed.

[anna]

The fix for build break is to pass in array of Type* to Intrinsic::getDeclaration and Intrinsic::getName.

[anna]

fix for clang build break: create array of Type* containing the single element,
and passing this Type*[] into Intrinsic::getDeclaration and Intrinsic::getName

[anna]

Committed llvm changes in https://reviews.llvm.org/rL276447

Commited clang changes in: https://reviews.llvm.org/rL276448

No clang failures.

[anna]

In D22519#493183, @anna wrote:

Committed llvm changes in https://reviews.llvm.org/rL276447

Commited clang changes in: https://reviews.llvm.org/rL276448

No clang failures.

The clang build break was due to the absence of overloaded type in the function call for getting the invariant intrinsic in clang::CodeGen::CodeGenFunction::EmitDeclInvariant. Clang test failures are due to the insertion of llvm.invariant.start calls after the constructor. This is turned on only at O1.
Discussed with Artur offline and the fix being simple and similar to the llvm invariant intrinsic change in AutoUpgrade, is checked in to clang directly. All clang and llvm buildbots pass.

docs/LangRef.rst

 never be used and has an undefined value. Any stores into the memory
 object following this intrinsic may be removed as dead.
 
 '``llvm.invariant.start``' Intrinsic
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 Syntax:
 """""""
+This is an overloaded intrinsic. The memory object can belong to any address space.
 
 ::
 
-      declare {}* @llvm.invariant.start(i64 <size>, i8* nocapture <ptr>)
+      declare {}* @llvm.invariant.start.p0i8(i64 <size>, i8* nocapture <ptr>)
 
 Overview:
 """""""""
 
 The '``llvm.invariant.start``' intrinsic specifies that the contents of
 a memory object will not change.
 
 Arguments:

 the return value, the referenced memory location is constant and
 unchanging.
 
 '``llvm.invariant.end``' Intrinsic
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 Syntax:
 """""""
+This is an overloaded intrinsic. The memory object can belong to any address space.
 
 ::
 
-      declare void @llvm.invariant.end({}* <start>, i64 <size>, i8* nocapture <ptr>)
+      declare void @llvm.invariant.end.p0i8({}* <start>, i64 <size>, i8* nocapture <ptr>)
 
 Overview:
 """""""""
 
 The '``llvm.invariant.end``' intrinsic specifies that the contents of a
 memory object are mutable.
 
 Arguments:

include/llvm/IR/Intrinsics.td

 //
 def int_lifetime_start  : Intrinsic<[],
                                     [llvm_i64_ty, llvm_ptr_ty],
                                     [IntrArgMemOnly, NoCapture<1>]>;
 def int_lifetime_end    : Intrinsic<[],
                                     [llvm_i64_ty, llvm_ptr_ty],
                                     [IntrArgMemOnly, NoCapture<1>]>;
 def int_invariant_start : Intrinsic<[llvm_descriptor_ty],
-                                    [llvm_i64_ty, llvm_ptr_ty],
+                                    [llvm_i64_ty, llvm_anyptr_ty],
                                     [IntrArgMemOnly, NoCapture<1>]>;
 def int_invariant_end   : Intrinsic<[],
                                     [llvm_descriptor_ty, llvm_i64_ty,
-                                     llvm_ptr_ty],
+                                     llvm_anyptr_ty],
                                     [IntrArgMemOnly, NoCapture<2>]>;
 
 def int_invariant_group_barrier : Intrinsic<[llvm_ptr_ty], 
                                             [llvm_ptr_ty], 
                                             [IntrNoMem]>;
 
 //===------------------------ Stackmap Intrinsics -------------------------===//
 //

lib/IR/AutoUpgrade.cpp

case 'c': {
     if (Name.startswith("cttz.") && F->arg_size() == 1) {
       F->setName(Name + ".old");
       NewFn = Intrinsic::getDeclaration(F->getParent(), Intrinsic::cttz,
                                         F->arg_begin()->getType());
       return true;
     }
     break;
   }
-
+  case 'i': {
+    if (Name.startswith("invariant.start")) {
+      auto Args = F->getFunctionType()->params();
+      Type* ObjectPtr[1] = {Args[1]};
+      if (F->getName() !=
+          Intrinsic::getName(Intrinsic::invariant_start, ObjectPtr)) {
+        F->setName(Name + ".old");
+        NewFn = Intrinsic::getDeclaration(
+            F->getParent(), Intrinsic::invariant_start, ObjectPtr);
+        return true;
+      }
+    }
+    if (Name.startswith("invariant.end")) {
+      auto Args = F->getFunctionType()->params();
+      Type* ObjectPtr[1] = {Args[2]};
+      if (F->getName() !=
+          Intrinsic::getName(Intrinsic::invariant_end, ObjectPtr)) {
+        F->setName(Name + ".old");
+        NewFn = Intrinsic::getDeclaration(F->getParent(),
+                                          Intrinsic::invariant_end, ObjectPtr);
+        return true;
+      }
+    }
+    break;
+  }
   case 'm': {
     if (Name.startswith("masked.load.")) {
       Type *Tys[] = { F->getReturnType(), F->arg_begin()->getType() };
       if (F->getName() != Intrinsic::getName(Intrinsic::masked_load, Tys)) {
         F->setName(Name + ".old");
         NewFn = Intrinsic::getDeclaration(F->getParent(),
                                           Intrinsic::masked_load,
                                           Tys);
void llvm::UpgradeIntrinsicCall(CallInst *CI, Function *NewFn) {
   }
 
   case Intrinsic::thread_pointer: {
     CI->replaceAllUsesWith(Builder.CreateCall(NewFn, {}));
     CI->eraseFromParent();
     return;
   }
 
+  case Intrinsic::invariant_start:
+  case Intrinsic::invariant_end:
   case Intrinsic::masked_load:
   case Intrinsic::masked_store: {
     SmallVector<Value *, 4> Args(CI->arg_operands().begin(),
                                  CI->arg_operands().end());
     CI->replaceAllUsesWith(Builder.CreateCall(NewFn, Args));
     CI->eraseFromParent();
     return;
   }

test/Assembler/auto_upgrade_intrinsics.ll

 
 define void @tests.masked.store(<2 x double>* %ptr, <2 x i1> %mask, <2 x double> %val)  {
 ; CHECK-LABEL: @tests.masked.store(
 ; CHECK: @llvm.masked.store.v2f64.p0v2f64
   call void @llvm.masked.store.v2f64(<2 x double> %val, <2 x double>* %ptr, i32 3, <2 x i1> %mask)
   ret void
 }
 
+
+declare {}* @llvm.invariant.start(i64, i8* nocapture) nounwind readonly
+declare void @llvm.invariant.end({}*, i64, i8* nocapture) nounwind
+
+define void @tests.invariant.start.end() {
+  ; CHECK-LABEL: @tests.invariant.start.end(
+  %a = alloca i8
+  %i = call {}* @llvm.invariant.start(i64 1, i8* %a)
+  ; CHECK: call {}* @llvm.invariant.start.p0i8
+  store i8 0, i8* %a
+  call void @llvm.invariant.end({}* %i, i64 1, i8* %a)
+  ; CHECK: call void @llvm.invariant.end.p0i8
+  ret void
+}
+
 @__stack_chk_guard = external global i8*
 declare void @llvm.stackprotectorcheck(i8**)
 
 define void @test.stackprotectorcheck() {
 ; CHECK-LABEL: @test.stackprotectorcheck(
 ; CHECK-NEXT: ret void
   call void @llvm.stackprotectorcheck(i8** @__stack_chk_guard)
   ret void
 }
 
 ; This is part of @test.objectsize(), since llvm.objectsize declaration gets
 ; emitted at the end.
 ; CHECK: declare i32 @llvm.objectsize.i32.p0i8

test/CodeGen/AMDGPU/promote-alloca-invariant-markers.ll

 ; RUN: llc -march=amdgcn -mattr=+promote-alloca -verify-machineinstrs < %s | FileCheck -check-prefix=GCN %s
 
-declare {}* @llvm.invariant.start(i64, i8* nocapture) #0
-declare void @llvm.invariant.end({}*, i64, i8* nocapture) #0
+declare {}* @llvm.invariant.start.p0i8(i64, i8* nocapture) #0
+declare void @llvm.invariant.end.p0i8({}*, i64, i8* nocapture) #0
 declare i8* @llvm.invariant.group.barrier(i8*) #1
 
 ; GCN-LABEL: {{^}}use_invariant_promotable_lds:
 ; GCN: buffer_load_dword
 ; GCN: ds_write_b32
 define void @use_invariant_promotable_lds(i32 addrspace(1)* %arg) #2 {
 bb:
   %tmp = alloca i32, align 4
   %tmp1 = bitcast i32* %tmp to i8*
   %tmp2 = getelementptr inbounds i32, i32 addrspace(1)* %arg, i64 1
   %tmp3 = load i32, i32 addrspace(1)* %tmp2
   store i32 %tmp3, i32* %tmp
-  %tmp4 = call {}* @llvm.invariant.start(i64 4, i8* %tmp1) #0
-  call void @llvm.invariant.end({}* %tmp4, i64 4, i8* %tmp1) #0
+  %tmp4 = call {}* @llvm.invariant.start.p0i8(i64 4, i8* %tmp1) #0
+  call void @llvm.invariant.end.p0i8({}* %tmp4, i64 4, i8* %tmp1) #0
   %tmp5 = call i8* @llvm.invariant.group.barrier(i8* %tmp1) #1
   ret void
 }
 
 attributes #0 = { argmemonly nounwind }
 attributes #1 = { nounwind readnone }
 attributes #2 = { nounwind }

test/Feature/memorymarkers.ll

 ; RUN: llvm-as -disable-output < %s
 
 %"struct.std::pair<int,int>" = type { i32, i32 }
 
 declare void @_Z3barRKi(i32*)
 
 declare void @llvm.lifetime.start(i64, i8* nocapture) nounwind
 declare void @llvm.lifetime.end(i64, i8* nocapture) nounwind
-declare {}* @llvm.invariant.start(i64, i8* nocapture) readonly nounwind
-declare void @llvm.invariant.end({}*, i64, i8* nocapture) nounwind
+declare {}* @llvm.invariant.start.p0i8(i64, i8* nocapture) readonly nounwind
+declare void @llvm.invariant.end.p0i8({}*, i64, i8* nocapture) nounwind
 
 define i32 @_Z4foo2v() nounwind {
 entry:
   %x = alloca %"struct.std::pair<int,int>"
   %y = bitcast %"struct.std::pair<int,int>"* %x to i8*
 
   ;; Constructor starts here (this isn't needed since it is immediately
   ;; preceded by an alloca, but shown for completeness).
   call void @llvm.lifetime.start(i64 8, i8* %y)
 
   %0 = getelementptr %"struct.std::pair<int,int>", %"struct.std::pair<int,int>"* %x, i32 0, i32 0
   store i32 4, i32* %0, align 8
   %1 = getelementptr %"struct.std::pair<int,int>", %"struct.std::pair<int,int>"* %x, i32 0, i32 1
   store i32 5, i32* %1, align 4
 
   ;; Constructor has finished here.
-  %inv = call {}* @llvm.invariant.start(i64 8, i8* %y)
+  %inv = call {}* @llvm.invariant.start.p0i8(i64 8, i8* %y)
   call void @_Z3barRKi(i32* %0) nounwind
   %2 = load i32, i32* %0, align 8
 
   ;; Destructor is run here.
-  call void @llvm.invariant.end({}* %inv, i64 8, i8* %y)
+  call void @llvm.invariant.end.p0i8({}* %inv, i64 8, i8* %y)
   ;; Destructor is done here.
   call void @llvm.lifetime.end(i64 8, i8* %y)
   ret i32 %2
 }

test/Transforms/GlobalOpt/invariant-nodatalayout.ll

 ; RUN: opt -globalopt -S -o - < %s | FileCheck %s
 ; The check here is that it doesn't crash.
 
-declare {}* @llvm.invariant.start(i64 %size, i8* nocapture %ptr)
+declare {}* @llvm.invariant.start.p0i8(i64 %size, i8* nocapture %ptr)
 
 @object1 = global { i32, i32 } zeroinitializer
 ; CHECK: @object1 = global { i32, i32 } zeroinitializer
 
 define void @ctor1() {
   %ptr = bitcast {i32, i32}* @object1 to i8*
-  call {}* @llvm.invariant.start(i64 4, i8* %ptr)
+  call {}* @llvm.invariant.start.p0i8(i64 4, i8* %ptr)
   ret void
 }
 
 @llvm.global_ctors = appending constant
   [1 x { i32, void ()* }]
   [ { i32, void ()* } { i32 65535, void ()* @ctor1 } ]

test/Transforms/GlobalOpt/invariant.ll

 ; RUN: opt -globalopt -S -o - < %s | FileCheck %s
 
 target datalayout = "e-p:64:64:64-i1:8:8-i8:8:8-i16:16:16-i32:32:32-i64:64:64-f32:32:32-f64:64:64-v64:64:64-v128:128:128-a0:0:64-s0:64:64-f80:128:128-n8:16:32:64-S128"
 target triple = "x86_64-unknown-linux-gnu"
 
-declare {}* @llvm.invariant.start(i64 %size, i8* nocapture %ptr)
+declare {}* @llvm.invariant.start.p0i8(i64 %size, i8* nocapture %ptr)
 
 define void @test1(i8* %ptr) {
-  call {}* @llvm.invariant.start(i64 4, i8* %ptr)
+  call {}* @llvm.invariant.start.p0i8(i64 4, i8* %ptr)
   ret void
 }
 
 @object1 = global i32 0
 ; CHECK: @object1 = constant i32 -1
 define void @ctor1() {
   store i32 -1, i32* @object1
   %A = bitcast i32* @object1 to i8*
   call void @test1(i8* %A)
   ret void
 }
 
 
 @object2 = global i32 0
 ; CHECK: @object2 = global i32 0
 define void @ctor2() {
   store i32 -1, i32* @object2
   %A = bitcast i32* @object2 to i8*
-  %B = call {}* @llvm.invariant.start(i64 4, i8* %A)
+  %B = call {}* @llvm.invariant.start.p0i8(i64 4, i8* %A)
   %C = bitcast {}* %B to i8*
   ret void
 }
 
 
 @object3 = global i32 0
 ; CHECK: @object3 = global i32 -1
 define void @ctor3() {
   store i32 -1, i32* @object3
   %A = bitcast i32* @object3 to i8*
-  call {}* @llvm.invariant.start(i64 3, i8* %A)
+  call {}* @llvm.invariant.start.p0i8(i64 3, i8* %A)
   ret void
 }
 
 
 @object4 = global i32 0
 ; CHECK: @object4 = global i32 -1
 define void @ctor4() {
   store i32 -1, i32* @object4
   %A = bitcast i32* @object4 to i8*
-  call {}* @llvm.invariant.start(i64 -1, i8* %A)
+  call {}* @llvm.invariant.start.p0i8(i64 -1, i8* %A)
   ret void
 }
 
 
 @llvm.global_ctors = appending constant
   [4 x { i32, void ()* }]
   [ { i32, void ()* } { i32 65535, void ()* @ctor1 },
     { i32, void ()* } { i32 65535, void ()* @ctor2 },
     { i32, void ()* } { i32 65535, void ()* @ctor3 },
     { i32, void ()* } { i32 65535, void ()* @ctor4 } ]

test/Transforms/InstCombine/invariant.ll

 ; Test to make sure unused llvm.invariant.start calls are not trivially eliminated
 ; RUN: opt < %s -instcombine -S | FileCheck %s
 
 declare void @g(i8*)
+declare void @g_addr1(i8 addrspace(1)*)
 
-declare {}* @llvm.invariant.start(i64, i8* nocapture) nounwind readonly
+declare {}* @llvm.invariant.start.p0i8(i64, i8* nocapture) nounwind readonly
+declare {}* @llvm.invariant.start.p1i8(i64, i8 addrspace(1)* nocapture) nounwind readonly
 
 define i8 @f() {
   %a = alloca i8                                  ; <i8*> [#uses=4]
   store i8 0, i8* %a
-  %i = call {}* @llvm.invariant.start(i64 1, i8* %a) ; <{}*> [#uses=0]
-  ; CHECK: call {}* @llvm.invariant.start
+  %i = call {}* @llvm.invariant.start.p0i8(i64 1, i8* %a) ; <{}*> [#uses=0]
+  ; CHECK: call {}* @llvm.invariant.start.p0i8
   call void @g(i8* %a)
   %r = load i8, i8* %a                                ; <i8> [#uses=1]
   ret i8 %r
 }
+
+; make sure llvm.invariant.call in non-default addrspace are also not eliminated.
+define i8 @f_addrspace1(i8 addrspace(1)* %a) {
+  store i8 0, i8 addrspace(1)* %a
+  %i = call {}* @llvm.invariant.start.p1i8(i64 1, i8 addrspace(1)* %a) ; <{}*> [#uses=0]
+  ; CHECK: call {}* @llvm.invariant.start.p1i8
+  call void @g_addr1(i8 addrspace(1)* %a)
+  %r = load i8, i8 addrspace(1)* %a                                ; <i8> [#uses=1]
+  ret i8 %r
+}