[PATCH] D142823: Intrinsics: Allow tablegen to mark parameters with dereferenceable

[Matt Arsenault via Phabricator via cfe-commits]

arsenm closed this revision.
arsenm added a comment.

1717c18664d5880f78db85eb0075a2c1379df2d9 



CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D142823/new/

https://reviews.llvm.org/D142823

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D142823: Intrinsics: Allow tablegen to mark parameters with dereferenceable

[Nikita Popov via Phabricator via cfe-commits]

nikic accepted this revision.
nikic added a comment.
This revision is now accepted and ready to land.

LGTM


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D142823/new/

https://reviews.llvm.org/D142823

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D142823: Intrinsics: Allow tablegen to mark parameters with dereferenceable

[Matt Arsenault via Phabricator via cfe-commits]

arsenm updated this revision to Diff 529074.
arsenm added a comment.

Split out amdgpu parts


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D142823/new/

https://reviews.llvm.org/D142823

Files:
  llvm/include/llvm/IR/Intrinsics.td
  llvm/test/TableGen/intrin-side-effects.td
  llvm/test/TableGen/intrinsic-attrs.td
  llvm/utils/TableGen/CodeGenIntrinsics.cpp
  llvm/utils/TableGen/CodeGenIntrinsics.h
  llvm/utils/TableGen/IntrinsicEmitter.cpp

Index: llvm/utils/TableGen/IntrinsicEmitter.cpp
===
--- llvm/utils/TableGen/IntrinsicEmitter.cpp
+++ llvm/utils/TableGen/IntrinsicEmitter.cpp
@@ -474,6 +474,10 @@
   OS << "  Attribute::get(C, Attribute::Alignment, "
  << Attr.Value << "),\n";
   break;
+case CodeGenIntrinsic::Dereferenceable:
+  OS << "  Attribute::get(C, Attribute::Dereferenceable, "
+ << Attr.Value << "),\n";
+  break;
 }
   }
   OS << "});\n";
Index: llvm/utils/TableGen/CodeGenIntrinsics.h
===
--- llvm/utils/TableGen/CodeGenIntrinsics.h
+++ llvm/utils/TableGen/CodeGenIntrinsics.h
@@ -113,7 +113,8 @@
 WriteOnly,
 ReadNone,
 ImmArg,
-Alignment
+Alignment,
+Dereferenceable
   };
 
   struct ArgAttribute {
Index: llvm/utils/TableGen/CodeGenIntrinsics.cpp
===
--- llvm/utils/TableGen/CodeGenIntrinsics.cpp
+++ llvm/utils/TableGen/CodeGenIntrinsics.cpp
@@ -234,6 +234,10 @@
 unsigned ArgNo = R->getValueAsInt("ArgNo");
 uint64_t Align = R->getValueAsInt("Align");
 addArgAttribute(ArgNo, Alignment, Align);
+  } else if (R->isSubClassOf("Dereferenceable")) {
+unsigned ArgNo = R->getValueAsInt("ArgNo");
+uint64_t Bytes = R->getValueAsInt("Bytes");
+addArgAttribute(ArgNo, Dereferenceable, Bytes);
   } else
 llvm_unreachable("Unknown property!");
 }
Index: llvm/test/TableGen/intrinsic-attrs.td
===
--- llvm/test/TableGen/intrinsic-attrs.td
+++ llvm/test/TableGen/intrinsic-attrs.td
@@ -9,7 +9,16 @@
   int isAny = 0;
 }
 
-def llvm_i32_ty: LLVMType;
+def llvm_i32_ty : LLVMType;
+def llvm_ptr_ty : LLVMType;
+
+class AttrIndex {
+  int Value = idx;
+}
+
+def FuncIndex : AttrIndex<-1>;
+def RetIndex : AttrIndex<0>;
+class ArgIndex : AttrIndex;
 
 class IntrinsicProperty {
   bit IsDefault = is_default;
@@ -17,6 +26,10 @@
 
 def IntrNoMem : IntrinsicProperty;
 def IntrHasSideEffects : IntrinsicProperty;
+class Dereferenceable : IntrinsicProperty {
+  int ArgNo = idx.Value;
+  int Bytes = bytes;
+}
 
 class Intrinsic ret_types,
 list param_types = [],
@@ -40,12 +53,33 @@
 // ... this intrinsic.
 def int_random_gen   : Intrinsic<[llvm_i32_ty], [], [IntrNoMem, IntrHasSideEffects]>;
 
+def int_deref_ptr_ret : Intrinsic<[llvm_ptr_ty], [], [Dereferenceable]>;
+
+// CHECK: static AttributeSet getIntrinsicArgAttributeSet(LLVMContext &C, unsigned ID) {
+// CHECK-NEXT:   switch (ID) {
+// CHECK-NEXT: default: llvm_unreachable("Invalid attribute set number");
+// CHECK-NEXT: case 0:
+// CHECK-NEXT: return AttributeSet::get(C, {
+// CHECK-NEXT: Attribute::get(C, Attribute::Dereferenceable, 16),
+// CHECK-NEXT: });
+// CHECK-NEXT: }
+// CHECK-NEXT: }
+
 // CHECK: static AttributeSet getIntrinsicFnAttributeSet(
 // CHECK: case 0:
 // CHECK-NEXT: return AttributeSet::get(C, {
 // CHECK-NEXT: Attribute::get(C, Attribute::NoUnwind),
 // CHECK-NEXT: });
 
-// CHECK: 1, // llvm.random.gen
+
+// CHECK: 1, // llvm.deref.ptr.ret
+// CHECK: 2, // llvm.random.gen
+
 // CHECK: case 1:
-// CHECK-NEXT: AS[0] = {AttributeList::FunctionIndex, getIntrinsicFnAttributeSet(C, 0)};
+// CHECK-NEXT: AS[0] = {0, getIntrinsicArgAttributeSet(C, 0)};
+// CHECK-NEXT: AS[1] = {AttributeList::FunctionIndex, getIntrinsicFnAttributeSet(C, 0)};
+// CHECK-NEXT: NumAttrs = 2;
+
+// CHECK: case 2:
+// CHECK-NEXT: AS[0] = {AttributeList::FunctionIndex, getIntrinsicFnAttributeSet(C, 1)};
+// CHECK-NEXT: NumAttrs = 1;
Index: llvm/include/llvm/IR/Intrinsics.td
===
--- llvm/include/llvm/IR/Intrinsics.td
+++ llvm/include/llvm/IR/Intrinsics.td
@@ -94,6 +94,11 @@
   int Align = align;
 }
 
+class Dereferenceable : IntrinsicProperty {
+  int ArgNo = idx.Value;
+  int Bytes = bytes;
+}
+
 // Returned - The specified argument is always the return value of the
 // intrinsic.
 class Returned : IntrinsicProperty {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D142823: Intrinsics: Allow tablegen to mark parameters with dereferenceable

[Jay Foad via Phabricator via cfe-commits]

foad added a comment.

In D142823#4093363 , @arsenm wrote:

> In D142823#4093357 , @foad wrote:
>
>> I think the tablegen functionality should be a separate patch from the 
>> amdgpu changes.
>
> Maybe, but then it’s untested in the patch which adds it

Not if you add a test. There are some already like test/TableGen/immarg.td and 
test/TableGen/intrin-side-effects.td.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D142823/new/

https://reviews.llvm.org/D142823

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D142823: Intrinsics: Allow tablegen to mark parameters with dereferenceable

[Matt Arsenault via Phabricator via cfe-commits]

arsenm added a comment.

In D142823#4093357 , @foad wrote:

> I think the tablegen functionality should be a separate patch from the amdgpu 
> changes.

Maybe, but then it’s untested in the patch which adds it


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D142823/new/

https://reviews.llvm.org/D142823

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D142823: Intrinsics: Allow tablegen to mark parameters with dereferenceable

[Jay Foad via Phabricator via cfe-commits]

foad added a comment.

I think the tablegen functionality should be a separate patch from the amdgpu 
changes.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D142823/new/

https://reviews.llvm.org/D142823

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D142823: Intrinsics: Allow tablegen to mark parameters with dereferenceable

[Matt Arsenault via Phabricator via cfe-commits]

arsenm updated this revision to Diff 493566.
arsenm added a comment.

Revert implicitarg.ptr changes since not-HSA has different alignment for no 
reason. Also with the size differences between amdhsa and different CO versions 
we're already wrong for emitting 256 unconditionally


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D142823/new/

https://reviews.llvm.org/D142823

Files:
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/test/CodeGenCUDA/amdgpu-workgroup-size.cu
  clang/test/CodeGenCUDA/builtins-amdgcn.cu
  clang/test/CodeGenOpenCL/builtins-amdgcn.cl
  llvm/include/llvm/IR/Intrinsics.td
  llvm/include/llvm/IR/IntrinsicsAMDGPU.td
  llvm/test/CodeGen/AMDGPU/implicit-arg-v5-opt.ll
  llvm/unittests/CodeGen/GlobalISel/KnownBitsTest.cpp
  llvm/utils/TableGen/CodeGenIntrinsics.h
  llvm/utils/TableGen/CodeGenTarget.cpp
  llvm/utils/TableGen/IntrinsicEmitter.cpp

Index: llvm/utils/TableGen/IntrinsicEmitter.cpp
===
--- llvm/utils/TableGen/IntrinsicEmitter.cpp
+++ llvm/utils/TableGen/IntrinsicEmitter.cpp
@@ -726,6 +726,10 @@
   OS << "  Attribute::get(C, Attribute::Alignment, "
  << Attr.Value << "),\n";
   break;
+case CodeGenIntrinsic::Dereferenceable:
+  OS << "  Attribute::get(C, Attribute::Dereferenceable, "
+ << Attr.Value << "),\n";
+  break;
 }
   }
   OS << "});\n";
Index: llvm/utils/TableGen/CodeGenTarget.cpp
===
--- llvm/utils/TableGen/CodeGenTarget.cpp
+++ llvm/utils/TableGen/CodeGenTarget.cpp
@@ -923,6 +923,10 @@
 unsigned ArgNo = R->getValueAsInt("ArgNo");
 uint64_t Align = R->getValueAsInt("Align");
 addArgAttribute(ArgNo, Alignment, Align);
+  } else if (R->isSubClassOf("Dereferenceable")) {
+unsigned ArgNo = R->getValueAsInt("ArgNo");
+uint64_t Bytes = R->getValueAsInt("Bytes");
+addArgAttribute(ArgNo, Dereferenceable, Bytes);
   } else
 llvm_unreachable("Unknown property!");
 }
Index: llvm/utils/TableGen/CodeGenIntrinsics.h
===
--- llvm/utils/TableGen/CodeGenIntrinsics.h
+++ llvm/utils/TableGen/CodeGenIntrinsics.h
@@ -119,7 +119,8 @@
 WriteOnly,
 ReadNone,
 ImmArg,
-Alignment
+Alignment,
+Dereferenceable
   };
 
   struct ArgAttribute {
Index: llvm/unittests/CodeGen/GlobalISel/KnownBitsTest.cpp
===
--- llvm/unittests/CodeGen/GlobalISel/KnownBitsTest.cpp
+++ llvm/unittests/CodeGen/GlobalISel/KnownBitsTest.cpp
@@ -1012,8 +1012,8 @@
 
   GISelKnownBits Info(*MF);
 
-  EXPECT_EQ(Align(4), Info.computeKnownAlignment(CopyDispatchPtr));
-  EXPECT_EQ(Align(4), Info.computeKnownAlignment(CopyQueuePtr));
+  EXPECT_EQ(Align(8), Info.computeKnownAlignment(CopyDispatchPtr));
+  EXPECT_EQ(Align(8), Info.computeKnownAlignment(CopyQueuePtr));
   EXPECT_EQ(Align(4), Info.computeKnownAlignment(CopyKernargSegmentPtr));
   EXPECT_EQ(Align(4), Info.computeKnownAlignment(CopyImplicitArgPtr));
   EXPECT_EQ(Align(4), Info.computeKnownAlignment(CopyImplicitBufferPtr));
Index: llvm/test/CodeGen/AMDGPU/implicit-arg-v5-opt.ll
===
--- llvm/test/CodeGen/AMDGPU/implicit-arg-v5-opt.ll
+++ llvm/test/CodeGen/AMDGPU/implicit-arg-v5-opt.ll
@@ -47,7 +47,7 @@
 ; GCN-LABEL: @get_local_size_z(
 ; GCN-NEXT:[[IMPLICITARG_PTR:%.*]] = tail call ptr addrspace(4) @llvm.amdgcn.implicitarg.ptr()
 ; GCN-NEXT:[[GEP_LOCAL_SIZE:%.*]] = getelementptr inbounds i8, ptr addrspace(4) [[IMPLICITARG_PTR]], i64 16
-; GCN-NEXT:[[LOCAL_SIZE:%.*]] = load i16, ptr addrspace(4) [[GEP_LOCAL_SIZE]], align 4
+; GCN-NEXT:[[LOCAL_SIZE:%.*]] = load i16, ptr addrspace(4) [[GEP_LOCAL_SIZE]], align 8
 ; GCN-NEXT:store i16 [[LOCAL_SIZE]], ptr addrspace(1) [[OUT:%.*]], align 2
 ; GCN-NEXT:ret void
 ;
@@ -139,7 +139,7 @@
 ; GCN-LABEL: @get_work_group_size_z(
 ; GCN-NEXT:[[IMPLICITARG_PTR:%.*]] = tail call ptr addrspace(4) @llvm.amdgcn.implicitarg.ptr()
 ; GCN-NEXT:[[GEP_Z:%.*]] = getelementptr inbounds i8, ptr addrspace(4) [[IMPLICITARG_PTR]], i64 16
-; GCN-NEXT:[[GROUP_SIZE_Z:%.*]] = load i16, ptr addrspace(4) [[GEP_Z]], align 4
+; GCN-NEXT:[[GROUP_SIZE_Z:%.*]] = load i16, ptr addrspace(4) [[GEP_Z]], align 8
 ; GCN-NEXT:store i16 [[GROUP_SIZE_Z]], ptr addrspace(1) [[OUT:%.*]], align 2
 ; GCN-NEXT:ret void
 ;
Index: llvm/include/llvm/IR/IntrinsicsAMDGPU.td
===
--- llvm/include/llvm/IR/IntrinsicsAMDGPU.td
+++ llvm/include/llvm/IR/IntrinsicsAMDGPU.td
@@ -141,8 +141,10 @@
<"__builtin_amdgcn_workgroup_id">;
 
 def int_amdgcn_dispatch_ptr :
+  ClangBuiltin<"__builtin_amdgcn_dispatch_ptr">,
   DefaultAttrsIntrinsic<[LLVMQualPointerType], [],
-  [Align, IntrNoMem, IntrSpeculata

[PATCH] D142823: Intrinsics: Allow tablegen to mark parameters with dereferenceable

[Matt Arsenault via Phabricator via cfe-commits]

arsenm created this revision.
arsenm added reviewers: sstefan1, jdoerfert, yaxunl, AMDGPU, nikic, 
alexander-shaposhnikov.
Herald added subscribers: kosarev, StephenFan, kerbowa, jvesely.
Herald added a project: All.
arsenm requested review of this revision.
Herald added a subscriber: wdng.
Herald added a project: LLVM.

Also increases the alignment of llvm.amdgcn.implicitarg.ptr to 8 to
match clang.


https://reviews.llvm.org/D142823

Files:
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/test/CodeGenCUDA/amdgpu-workgroup-size.cu
  clang/test/CodeGenCUDA/builtins-amdgcn.cu
  clang/test/CodeGenOpenCL/builtins-amdgcn.cl
  llvm/include/llvm/IR/Intrinsics.td
  llvm/include/llvm/IR/IntrinsicsAMDGPU.td
  llvm/utils/TableGen/CodeGenIntrinsics.h
  llvm/utils/TableGen/CodeGenTarget.cpp
  llvm/utils/TableGen/IntrinsicEmitter.cpp

Index: llvm/utils/TableGen/IntrinsicEmitter.cpp
===
--- llvm/utils/TableGen/IntrinsicEmitter.cpp
+++ llvm/utils/TableGen/IntrinsicEmitter.cpp
@@ -726,6 +726,10 @@
   OS << "  Attribute::get(C, Attribute::Alignment, "
  << Attr.Value << "),\n";
   break;
+case CodeGenIntrinsic::Dereferenceable:
+  OS << "  Attribute::get(C, Attribute::Dereferenceable, "
+ << Attr.Value << "),\n";
+  break;
 }
   }
   OS << "});\n";
Index: llvm/utils/TableGen/CodeGenTarget.cpp
===
--- llvm/utils/TableGen/CodeGenTarget.cpp
+++ llvm/utils/TableGen/CodeGenTarget.cpp
@@ -923,6 +923,10 @@
 unsigned ArgNo = R->getValueAsInt("ArgNo");
 uint64_t Align = R->getValueAsInt("Align");
 addArgAttribute(ArgNo, Alignment, Align);
+  } else if (R->isSubClassOf("Dereferenceable")) {
+unsigned ArgNo = R->getValueAsInt("ArgNo");
+uint64_t Bytes = R->getValueAsInt("Bytes");
+addArgAttribute(ArgNo, Dereferenceable, Bytes);
   } else
 llvm_unreachable("Unknown property!");
 }
Index: llvm/utils/TableGen/CodeGenIntrinsics.h
===
--- llvm/utils/TableGen/CodeGenIntrinsics.h
+++ llvm/utils/TableGen/CodeGenIntrinsics.h
@@ -119,7 +119,8 @@
 WriteOnly,
 ReadNone,
 ImmArg,
-Alignment
+Alignment,
+Dereferenceable
   };
 
   struct ArgAttribute {
Index: llvm/include/llvm/IR/IntrinsicsAMDGPU.td
===
--- llvm/include/llvm/IR/IntrinsicsAMDGPU.td
+++ llvm/include/llvm/IR/IntrinsicsAMDGPU.td
@@ -141,8 +141,10 @@
<"__builtin_amdgcn_workgroup_id">;
 
 def int_amdgcn_dispatch_ptr :
+  ClangBuiltin<"__builtin_amdgcn_dispatch_ptr">,
   DefaultAttrsIntrinsic<[LLVMQualPointerType], [],
-  [Align, IntrNoMem, IntrSpeculatable]>;
+  [Align, Dereferenceable, IntrNoMem,
+   IntrSpeculatable]>;
 
 def int_amdgcn_queue_ptr :
   ClangBuiltin<"__builtin_amdgcn_queue_ptr">,
@@ -157,7 +159,8 @@
 def int_amdgcn_implicitarg_ptr :
   ClangBuiltin<"__builtin_amdgcn_implicitarg_ptr">,
   DefaultAttrsIntrinsic<[LLVMQualPointerType], [],
-  [Align, IntrNoMem, IntrSpeculatable]>;
+  [Align, Dereferenceable,
+   IntrNoMem, IntrSpeculatable]>;
 
 def int_amdgcn_groupstaticsize :
   ClangBuiltin<"__builtin_amdgcn_groupstaticsize">,
Index: llvm/include/llvm/IR/Intrinsics.td
===
--- llvm/include/llvm/IR/Intrinsics.td
+++ llvm/include/llvm/IR/Intrinsics.td
@@ -94,6 +94,11 @@
   int Align = align;
 }
 
+class Dereferenceable : IntrinsicProperty {
+  int ArgNo = idx.Value;
+  int Bytes = bytes;
+}
+
 // Returned - The specified argument is always the return value of the
 // intrinsic.
 class Returned : IntrinsicProperty {
Index: clang/test/CodeGenOpenCL/builtins-amdgcn.cl
===
--- clang/test/CodeGenOpenCL/builtins-amdgcn.cl
+++ clang/test/CodeGenOpenCL/builtins-amdgcn.cl
@@ -516,12 +516,15 @@
 }
 
 // CHECK-LABEL: @test_dispatch_ptr
-// CHECK: call align 4 dereferenceable(64) ptr addrspace(4) @llvm.amdgcn.dispatch.ptr()
+// CHECK: call ptr addrspace(4) @llvm.amdgcn.dispatch.ptr()
 void test_dispatch_ptr(__constant unsigned char ** out)
 {
   *out = __builtin_amdgcn_dispatch_ptr();
 }
 
+// CHECK: declare align 4 dereferenceable(64) ptr addrspace(4) @llvm.amdgcn.dispatch.ptr()
+
+
 // CHECK-LABEL: @test_queue_ptr
 // CHECK: call ptr addrspace(4) @llvm.amdgcn.queue.ptr()
 void test_queue_ptr(__constant unsigned char ** out)
@@ -543,6 +546,9 @@
   *out = __builtin_amdgcn_implicitarg_ptr();
 }
 
+// CHECK: declare align 8 dereferenceable(256) ptr addrspace(4) @llvm.amdgcn.implicitarg.ptr()
+
+
 // CHECK-LABEL: @test_get_group_id(
 // CHECK: tail call i32 @llvm.amdgcn.workgroup.id.x()
 // CHECK: tail call i32 @llvm.amdgcn.workgroup.id.y()
@@ -583,7 +589,7 @@
 }
 
 // CHECK-LABEL: @test_get_workgroup_size(
-// CHECK: call align 4 dereferenceable