-
Notifications
You must be signed in to change notification settings - Fork 15.3k
[SPIRV] Add support for CodeSectionINTEL storage class in legalizer #167961
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Signed-off-by: Nick Sarnie <[email protected]>
| @@ -177,15 +179,22 @@ SPIRVLegalizerInfo::SPIRVLegalizerInfo(const SPIRVSubtarget &ST) { | |||
| getActionDefinitionsBuilder(G_UNMERGE_VALUES).alwaysLegal(); | |||
|
|
|||
| getActionDefinitionsBuilder({G_MEMCPY, G_MEMMOVE}) | |||
| .unsupportedIf(LegalityPredicates::any(typeIs(0, p9), typeIs(1, p9))) | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We need the LegalityPredicates:: qualification because of a clash between a function in another header.
|
@llvm/pr-subscribers-llvm-globalisel Author: Nick Sarnie (sarnex) ChangesThe SPV_INTEL_function_pointers extension defines a new storage class Per the spec, it is basically not allowed to be casted to or interact with pointers with other storage classes. Add Right now, if you try to run the backend on input with SPIR-V, basically everything errors saying that it is unable to legalize because Ideally the FE should not generate the illegal IR or error out earlier, but we should catch it before generating invalid SPIR-V. Right now, if you try to run the backend on input with SPIR-V, basically everything errors saying that it is unable to legalize because Full diff: https://github.com/llvm/llvm-project/pull/167961.diff 6 Files Affected:
diff --git a/llvm/lib/Target/SPIRV/SPIRVLegalizerInfo.cpp b/llvm/lib/Target/SPIRV/SPIRVLegalizerInfo.cpp
index 53074ea3b2597..822fee8a9da35 100644
--- a/llvm/lib/Target/SPIRV/SPIRVLegalizerInfo.cpp
+++ b/llvm/lib/Target/SPIRV/SPIRVLegalizerInfo.cpp
@@ -84,17 +84,19 @@ SPIRVLegalizerInfo::SPIRVLegalizerInfo(const SPIRVSubtarget &ST) {
const LLT p6 = LLT::pointer(6, PSize); // SPV_INTEL_usm_storage_classes (Host)
const LLT p7 = LLT::pointer(7, PSize); // Input
const LLT p8 = LLT::pointer(8, PSize); // Output
+ const LLT p9 =
+ LLT::pointer(9, PSize); // CodeSectionINTEL, SPV_INTEL_function_pointers
const LLT p10 = LLT::pointer(10, PSize); // Private
const LLT p11 = LLT::pointer(11, PSize); // StorageBuffer
const LLT p12 = LLT::pointer(12, PSize); // Uniform
// TODO: remove copy-pasting here by using concatenation in some way.
auto allPtrsScalarsAndVectors = {
- p0, p1, p2, p3, p4, p5, p6, p7, p8,
- p10, p11, p12, s1, s8, s16, s32, s64, v2s1,
- v2s8, v2s16, v2s32, v2s64, v3s1, v3s8, v3s16, v3s32, v3s64,
- v4s1, v4s8, v4s16, v4s32, v4s64, v8s1, v8s8, v8s16, v8s32,
- v8s64, v16s1, v16s8, v16s16, v16s32, v16s64};
+ p0, p1, p2, p3, p4, p5, p6, p7, p8,
+ p9, p10, p11, p12, s1, s8, s16, s32, s64,
+ v2s1, v2s8, v2s16, v2s32, v2s64, v3s1, v3s8, v3s16, v3s32,
+ v3s64, v4s1, v4s8, v4s16, v4s32, v4s64, v8s1, v8s8, v8s16,
+ v8s32, v8s64, v16s1, v16s8, v16s16, v16s32, v16s64};
auto allVectors = {v2s1, v2s8, v2s16, v2s32, v2s64, v3s1, v3s8,
v3s16, v3s32, v3s64, v4s1, v4s8, v4s16, v4s32,
@@ -121,10 +123,10 @@ SPIRVLegalizerInfo::SPIRVLegalizerInfo(const SPIRVSubtarget &ST) {
s16, s32, s64, v2s16, v2s32, v2s64, v3s16, v3s32, v3s64,
v4s16, v4s32, v4s64, v8s16, v8s32, v8s64, v16s16, v16s32, v16s64};
- auto allFloatAndIntScalarsAndPtrs = {s8, s16, s32, s64, p0, p1, p2, p3,
- p4, p5, p6, p7, p8, p10, p11, p12};
+ auto allFloatAndIntScalarsAndPtrs = {s8, s16, s32, s64, p0, p1, p2, p3, p4,
+ p5, p6, p7, p8, p9, p10, p11, p12};
- auto allPtrs = {p0, p1, p2, p3, p4, p5, p6, p7, p8, p10, p11, p12};
+ auto allPtrs = {p0, p1, p2, p3, p4, p5, p6, p7, p8, p9, p10, p11, p12};
bool IsExtendedInts =
ST.canUseExtension(
@@ -177,15 +179,22 @@ SPIRVLegalizerInfo::SPIRVLegalizerInfo(const SPIRVSubtarget &ST) {
getActionDefinitionsBuilder(G_UNMERGE_VALUES).alwaysLegal();
getActionDefinitionsBuilder({G_MEMCPY, G_MEMMOVE})
+ .unsupportedIf(LegalityPredicates::any(typeIs(0, p9), typeIs(1, p9)))
.legalIf(all(typeInSet(0, allPtrs), typeInSet(1, allPtrs)));
- getActionDefinitionsBuilder(G_MEMSET).legalIf(
- all(typeInSet(0, allPtrs), typeInSet(1, allIntScalars)));
+ getActionDefinitionsBuilder(G_MEMSET)
+ .unsupportedIf(typeIs(0, p9))
+ .legalIf(all(typeInSet(0, allPtrs), typeInSet(1, allIntScalars)));
getActionDefinitionsBuilder(G_ADDRSPACE_CAST)
+ .unsupportedIf(
+ LegalityPredicates::any(all(typeIs(0, p9), typeIsNot(1, p9)),
+ all(typeIsNot(0, p9), typeIs(1, p9))))
.legalForCartesianProduct(allPtrs, allPtrs);
- getActionDefinitionsBuilder({G_LOAD, G_STORE}).legalIf(typeInSet(1, allPtrs));
+ getActionDefinitionsBuilder({G_LOAD, G_STORE})
+ .unsupportedIf(typeIs(1, p9))
+ .legalIf(typeInSet(1, allPtrs));
getActionDefinitionsBuilder({G_SMIN, G_SMAX, G_UMIN, G_UMAX, G_ABS,
G_BITREVERSE, G_SADDSAT, G_UADDSAT, G_SSUBSAT,
@@ -247,9 +256,12 @@ SPIRVLegalizerInfo::SPIRVLegalizerInfo(const SPIRVSubtarget &ST) {
// ST.canDirectlyComparePointers() for pointer args is supported in
// legalizeCustom().
- getActionDefinitionsBuilder(G_ICMP).customIf(
- all(typeInSet(0, allBoolScalarsAndVectors),
- typeInSet(1, allPtrsScalarsAndVectors)));
+ getActionDefinitionsBuilder(G_ICMP)
+ .unsupportedIf(LegalityPredicates::any(
+ all(typeIs(0, p9), typeInSet(1, allPtrs), typeIsNot(1, p9)),
+ all(typeInSet(0, allPtrs), typeIsNot(0, p9), typeIs(1, p9))))
+ .customIf(all(typeInSet(0, allBoolScalarsAndVectors),
+ typeInSet(1, allPtrsScalarsAndVectors)));
getActionDefinitionsBuilder(G_FCMP).legalIf(
all(typeInSet(0, allBoolScalarsAndVectors),
diff --git a/llvm/test/CodeGen/SPIRV/GlobalISel/fn-ptr-addrspacecast.ll b/llvm/test/CodeGen/SPIRV/GlobalISel/fn-ptr-addrspacecast.ll
new file mode 100644
index 0000000000000..62f9c7f06b937
--- /dev/null
+++ b/llvm/test/CodeGen/SPIRV/GlobalISel/fn-ptr-addrspacecast.ll
@@ -0,0 +1,9 @@
+; RUN: not llc --global-isel %s -o /dev/null 2>&1 | FileCheck %s
+target datalayout = "e-i64:64-v16:16-v24:32-v32:32-v48:64-v96:128-v192:256-v256:256-v512:512-v1024:1024-n8:16:32:64-G1-P9-A0"
+target triple = "spirv64-intel"
+
+define void @addrspacecast(ptr addrspace(9) %a) {
+; CHECK: unable to legalize instruction: %{{.*}}:pid(p4) = G_ADDRSPACE_CAST %{{.*}}:pid(p9)
+ %res1 = addrspacecast ptr addrspace(9) %a to ptr addrspace(4)
+ ret void
+}
diff --git a/llvm/test/CodeGen/SPIRV/GlobalISel/fn-ptr-load.ll b/llvm/test/CodeGen/SPIRV/GlobalISel/fn-ptr-load.ll
new file mode 100644
index 0000000000000..888560d53a7ae
--- /dev/null
+++ b/llvm/test/CodeGen/SPIRV/GlobalISel/fn-ptr-load.ll
@@ -0,0 +1,9 @@
+; RUN: not llc --global-isel %s -o /dev/null 2>&1 | FileCheck %s
+target datalayout = "e-i64:64-v16:16-v24:32-v32:32-v48:64-v96:128-v192:256-v256:256-v512:512-v1024:1024-n8:16:32:64-G1-P9-A0"
+target triple = "spirv64-intel"
+
+define void @memset(ptr addrspace(9) %a) {
+; CHECK: unable to legalize instruction: %{{.*}}:iid(s32) = G_LOAD %{{.*}}:pid(p9)
+ %val = load i32, ptr addrspace(9) %a
+ ret void
+}
diff --git a/llvm/test/CodeGen/SPIRV/GlobalISel/fn-ptr-memcpy.ll b/llvm/test/CodeGen/SPIRV/GlobalISel/fn-ptr-memcpy.ll
new file mode 100644
index 0000000000000..26566e50f1db2
--- /dev/null
+++ b/llvm/test/CodeGen/SPIRV/GlobalISel/fn-ptr-memcpy.ll
@@ -0,0 +1,9 @@
+; RUN: not llc --global-isel %s -o /dev/null 2>&1 | FileCheck %s
+target datalayout = "e-i64:64-v16:16-v24:32-v32:32-v48:64-v96:128-v192:256-v256:256-v512:512-v1024:1024-n8:16:32:64-G1-P9-A0"
+target triple = "spirv64-intel"
+
+define void @memcpy(ptr addrspace(9) %a) {
+; CHECK: unable to legalize instruction: G_MEMCPY %{{.*}}:pid(p9), %{{.*}}:pid(p0), %{{.*}}:iid(s64), 0
+ call void @llvm.memcpy.p9.p0.i64(ptr addrspace(9) %a, ptr null, i32 1, i1 0)
+ ret void
+}
diff --git a/llvm/test/CodeGen/SPIRV/GlobalISel/fn-ptr-memset.ll b/llvm/test/CodeGen/SPIRV/GlobalISel/fn-ptr-memset.ll
new file mode 100644
index 0000000000000..3dd4ef0107495
--- /dev/null
+++ b/llvm/test/CodeGen/SPIRV/GlobalISel/fn-ptr-memset.ll
@@ -0,0 +1,9 @@
+; RUN: not llc --global-isel %s -o /dev/null 2>&1 | FileCheck %s
+target datalayout = "e-i64:64-v16:16-v24:32-v32:32-v48:64-v96:128-v192:256-v256:256-v512:512-v1024:1024-n8:16:32:64-G1-P9-A0"
+target triple = "spirv64-intel"
+
+define void @memset(ptr addrspace(9) %a) {
+; CHECK: unable to legalize instruction: G_MEMSET %{{.*}}:pid(p9), %{{.*}}:iid(s8), %{{.*}}:iid(s64)
+ call void @llvm.memset.p9.i32(ptr addrspace(9) %a, i8 0, i32 1, i1 0)
+ ret void
+}
diff --git a/llvm/test/CodeGen/SPIRV/extensions/SPV_INTEL_function_pointers/fp_cmp.ll b/llvm/test/CodeGen/SPIRV/extensions/SPV_INTEL_function_pointers/fp_cmp.ll
new file mode 100644
index 0000000000000..6b345f708442c
--- /dev/null
+++ b/llvm/test/CodeGen/SPIRV/extensions/SPV_INTEL_function_pointers/fp_cmp.ll
@@ -0,0 +1,34 @@
+; RUN: llc -verify-machineinstrs -O0 -mtriple=spirv64-intel --spirv-ext=+SPV_INTEL_function_pointers %s -o - | FileCheck %s
+; TODO: %if spirv-tools %{ llc -O0 -mtriple=spirv64-intel %s -o - -filetype=obj | spirv-val %}
+
+; CHECK-DAG: OpCapability FunctionPointersINTEL
+; CHECK: OpExtension "SPV_INTEL_function_pointers"
+
+; CHECK: OpName %[[F1:.*]] "f1"
+; CHECK: OpName %[[F2:.*]] "f2"
+
+; CHECK: %[[TyBool:.*]] = OpTypeBool
+
+; CHECK %[[F1Ptr:.*]] = OpConstantFunctionPointerINTEL %{{.*}} %[[F2]]
+; CHECK %[[F2Ptr:.*]] = OpConstantFunctionPointerINTEL %{{.*}} %[[F2]]
+
+; CHECK OpPtrEqual %[[TyBool]] %[[F1Ptr]] %[[F2Ptr]]
+
+target datalayout = "e-i64:64-v16:16-v24:32-v32:32-v48:64-v96:128-v192:256-v256:256-v512:512-v1024:1024-n8:16:32:64-G1-P9-A0"
+target triple = "spirv64-intel"
+
+define spir_func void @f1() addrspace(9) {
+entry:
+ ret void
+}
+
+define spir_func void @f2() addrspace(9) {
+entry:
+ ret void
+}
+
+define spir_kernel void @foo() addrspace(9) {
+entry:
+ %a = icmp eq ptr addrspace(9) @f1, @f2
+ ret void
+}
|
|
@llvm/pr-subscribers-backend-spir-v Author: Nick Sarnie (sarnex) ChangesThe SPV_INTEL_function_pointers extension defines a new storage class Per the spec, it is basically not allowed to be casted to or interact with pointers with other storage classes. Add Right now, if you try to run the backend on input with SPIR-V, basically everything errors saying that it is unable to legalize because Ideally the FE should not generate the illegal IR or error out earlier, but we should catch it before generating invalid SPIR-V. Right now, if you try to run the backend on input with SPIR-V, basically everything errors saying that it is unable to legalize because Full diff: https://github.com/llvm/llvm-project/pull/167961.diff 6 Files Affected:
diff --git a/llvm/lib/Target/SPIRV/SPIRVLegalizerInfo.cpp b/llvm/lib/Target/SPIRV/SPIRVLegalizerInfo.cpp
index 53074ea3b2597..822fee8a9da35 100644
--- a/llvm/lib/Target/SPIRV/SPIRVLegalizerInfo.cpp
+++ b/llvm/lib/Target/SPIRV/SPIRVLegalizerInfo.cpp
@@ -84,17 +84,19 @@ SPIRVLegalizerInfo::SPIRVLegalizerInfo(const SPIRVSubtarget &ST) {
const LLT p6 = LLT::pointer(6, PSize); // SPV_INTEL_usm_storage_classes (Host)
const LLT p7 = LLT::pointer(7, PSize); // Input
const LLT p8 = LLT::pointer(8, PSize); // Output
+ const LLT p9 =
+ LLT::pointer(9, PSize); // CodeSectionINTEL, SPV_INTEL_function_pointers
const LLT p10 = LLT::pointer(10, PSize); // Private
const LLT p11 = LLT::pointer(11, PSize); // StorageBuffer
const LLT p12 = LLT::pointer(12, PSize); // Uniform
// TODO: remove copy-pasting here by using concatenation in some way.
auto allPtrsScalarsAndVectors = {
- p0, p1, p2, p3, p4, p5, p6, p7, p8,
- p10, p11, p12, s1, s8, s16, s32, s64, v2s1,
- v2s8, v2s16, v2s32, v2s64, v3s1, v3s8, v3s16, v3s32, v3s64,
- v4s1, v4s8, v4s16, v4s32, v4s64, v8s1, v8s8, v8s16, v8s32,
- v8s64, v16s1, v16s8, v16s16, v16s32, v16s64};
+ p0, p1, p2, p3, p4, p5, p6, p7, p8,
+ p9, p10, p11, p12, s1, s8, s16, s32, s64,
+ v2s1, v2s8, v2s16, v2s32, v2s64, v3s1, v3s8, v3s16, v3s32,
+ v3s64, v4s1, v4s8, v4s16, v4s32, v4s64, v8s1, v8s8, v8s16,
+ v8s32, v8s64, v16s1, v16s8, v16s16, v16s32, v16s64};
auto allVectors = {v2s1, v2s8, v2s16, v2s32, v2s64, v3s1, v3s8,
v3s16, v3s32, v3s64, v4s1, v4s8, v4s16, v4s32,
@@ -121,10 +123,10 @@ SPIRVLegalizerInfo::SPIRVLegalizerInfo(const SPIRVSubtarget &ST) {
s16, s32, s64, v2s16, v2s32, v2s64, v3s16, v3s32, v3s64,
v4s16, v4s32, v4s64, v8s16, v8s32, v8s64, v16s16, v16s32, v16s64};
- auto allFloatAndIntScalarsAndPtrs = {s8, s16, s32, s64, p0, p1, p2, p3,
- p4, p5, p6, p7, p8, p10, p11, p12};
+ auto allFloatAndIntScalarsAndPtrs = {s8, s16, s32, s64, p0, p1, p2, p3, p4,
+ p5, p6, p7, p8, p9, p10, p11, p12};
- auto allPtrs = {p0, p1, p2, p3, p4, p5, p6, p7, p8, p10, p11, p12};
+ auto allPtrs = {p0, p1, p2, p3, p4, p5, p6, p7, p8, p9, p10, p11, p12};
bool IsExtendedInts =
ST.canUseExtension(
@@ -177,15 +179,22 @@ SPIRVLegalizerInfo::SPIRVLegalizerInfo(const SPIRVSubtarget &ST) {
getActionDefinitionsBuilder(G_UNMERGE_VALUES).alwaysLegal();
getActionDefinitionsBuilder({G_MEMCPY, G_MEMMOVE})
+ .unsupportedIf(LegalityPredicates::any(typeIs(0, p9), typeIs(1, p9)))
.legalIf(all(typeInSet(0, allPtrs), typeInSet(1, allPtrs)));
- getActionDefinitionsBuilder(G_MEMSET).legalIf(
- all(typeInSet(0, allPtrs), typeInSet(1, allIntScalars)));
+ getActionDefinitionsBuilder(G_MEMSET)
+ .unsupportedIf(typeIs(0, p9))
+ .legalIf(all(typeInSet(0, allPtrs), typeInSet(1, allIntScalars)));
getActionDefinitionsBuilder(G_ADDRSPACE_CAST)
+ .unsupportedIf(
+ LegalityPredicates::any(all(typeIs(0, p9), typeIsNot(1, p9)),
+ all(typeIsNot(0, p9), typeIs(1, p9))))
.legalForCartesianProduct(allPtrs, allPtrs);
- getActionDefinitionsBuilder({G_LOAD, G_STORE}).legalIf(typeInSet(1, allPtrs));
+ getActionDefinitionsBuilder({G_LOAD, G_STORE})
+ .unsupportedIf(typeIs(1, p9))
+ .legalIf(typeInSet(1, allPtrs));
getActionDefinitionsBuilder({G_SMIN, G_SMAX, G_UMIN, G_UMAX, G_ABS,
G_BITREVERSE, G_SADDSAT, G_UADDSAT, G_SSUBSAT,
@@ -247,9 +256,12 @@ SPIRVLegalizerInfo::SPIRVLegalizerInfo(const SPIRVSubtarget &ST) {
// ST.canDirectlyComparePointers() for pointer args is supported in
// legalizeCustom().
- getActionDefinitionsBuilder(G_ICMP).customIf(
- all(typeInSet(0, allBoolScalarsAndVectors),
- typeInSet(1, allPtrsScalarsAndVectors)));
+ getActionDefinitionsBuilder(G_ICMP)
+ .unsupportedIf(LegalityPredicates::any(
+ all(typeIs(0, p9), typeInSet(1, allPtrs), typeIsNot(1, p9)),
+ all(typeInSet(0, allPtrs), typeIsNot(0, p9), typeIs(1, p9))))
+ .customIf(all(typeInSet(0, allBoolScalarsAndVectors),
+ typeInSet(1, allPtrsScalarsAndVectors)));
getActionDefinitionsBuilder(G_FCMP).legalIf(
all(typeInSet(0, allBoolScalarsAndVectors),
diff --git a/llvm/test/CodeGen/SPIRV/GlobalISel/fn-ptr-addrspacecast.ll b/llvm/test/CodeGen/SPIRV/GlobalISel/fn-ptr-addrspacecast.ll
new file mode 100644
index 0000000000000..62f9c7f06b937
--- /dev/null
+++ b/llvm/test/CodeGen/SPIRV/GlobalISel/fn-ptr-addrspacecast.ll
@@ -0,0 +1,9 @@
+; RUN: not llc --global-isel %s -o /dev/null 2>&1 | FileCheck %s
+target datalayout = "e-i64:64-v16:16-v24:32-v32:32-v48:64-v96:128-v192:256-v256:256-v512:512-v1024:1024-n8:16:32:64-G1-P9-A0"
+target triple = "spirv64-intel"
+
+define void @addrspacecast(ptr addrspace(9) %a) {
+; CHECK: unable to legalize instruction: %{{.*}}:pid(p4) = G_ADDRSPACE_CAST %{{.*}}:pid(p9)
+ %res1 = addrspacecast ptr addrspace(9) %a to ptr addrspace(4)
+ ret void
+}
diff --git a/llvm/test/CodeGen/SPIRV/GlobalISel/fn-ptr-load.ll b/llvm/test/CodeGen/SPIRV/GlobalISel/fn-ptr-load.ll
new file mode 100644
index 0000000000000..888560d53a7ae
--- /dev/null
+++ b/llvm/test/CodeGen/SPIRV/GlobalISel/fn-ptr-load.ll
@@ -0,0 +1,9 @@
+; RUN: not llc --global-isel %s -o /dev/null 2>&1 | FileCheck %s
+target datalayout = "e-i64:64-v16:16-v24:32-v32:32-v48:64-v96:128-v192:256-v256:256-v512:512-v1024:1024-n8:16:32:64-G1-P9-A0"
+target triple = "spirv64-intel"
+
+define void @memset(ptr addrspace(9) %a) {
+; CHECK: unable to legalize instruction: %{{.*}}:iid(s32) = G_LOAD %{{.*}}:pid(p9)
+ %val = load i32, ptr addrspace(9) %a
+ ret void
+}
diff --git a/llvm/test/CodeGen/SPIRV/GlobalISel/fn-ptr-memcpy.ll b/llvm/test/CodeGen/SPIRV/GlobalISel/fn-ptr-memcpy.ll
new file mode 100644
index 0000000000000..26566e50f1db2
--- /dev/null
+++ b/llvm/test/CodeGen/SPIRV/GlobalISel/fn-ptr-memcpy.ll
@@ -0,0 +1,9 @@
+; RUN: not llc --global-isel %s -o /dev/null 2>&1 | FileCheck %s
+target datalayout = "e-i64:64-v16:16-v24:32-v32:32-v48:64-v96:128-v192:256-v256:256-v512:512-v1024:1024-n8:16:32:64-G1-P9-A0"
+target triple = "spirv64-intel"
+
+define void @memcpy(ptr addrspace(9) %a) {
+; CHECK: unable to legalize instruction: G_MEMCPY %{{.*}}:pid(p9), %{{.*}}:pid(p0), %{{.*}}:iid(s64), 0
+ call void @llvm.memcpy.p9.p0.i64(ptr addrspace(9) %a, ptr null, i32 1, i1 0)
+ ret void
+}
diff --git a/llvm/test/CodeGen/SPIRV/GlobalISel/fn-ptr-memset.ll b/llvm/test/CodeGen/SPIRV/GlobalISel/fn-ptr-memset.ll
new file mode 100644
index 0000000000000..3dd4ef0107495
--- /dev/null
+++ b/llvm/test/CodeGen/SPIRV/GlobalISel/fn-ptr-memset.ll
@@ -0,0 +1,9 @@
+; RUN: not llc --global-isel %s -o /dev/null 2>&1 | FileCheck %s
+target datalayout = "e-i64:64-v16:16-v24:32-v32:32-v48:64-v96:128-v192:256-v256:256-v512:512-v1024:1024-n8:16:32:64-G1-P9-A0"
+target triple = "spirv64-intel"
+
+define void @memset(ptr addrspace(9) %a) {
+; CHECK: unable to legalize instruction: G_MEMSET %{{.*}}:pid(p9), %{{.*}}:iid(s8), %{{.*}}:iid(s64)
+ call void @llvm.memset.p9.i32(ptr addrspace(9) %a, i8 0, i32 1, i1 0)
+ ret void
+}
diff --git a/llvm/test/CodeGen/SPIRV/extensions/SPV_INTEL_function_pointers/fp_cmp.ll b/llvm/test/CodeGen/SPIRV/extensions/SPV_INTEL_function_pointers/fp_cmp.ll
new file mode 100644
index 0000000000000..6b345f708442c
--- /dev/null
+++ b/llvm/test/CodeGen/SPIRV/extensions/SPV_INTEL_function_pointers/fp_cmp.ll
@@ -0,0 +1,34 @@
+; RUN: llc -verify-machineinstrs -O0 -mtriple=spirv64-intel --spirv-ext=+SPV_INTEL_function_pointers %s -o - | FileCheck %s
+; TODO: %if spirv-tools %{ llc -O0 -mtriple=spirv64-intel %s -o - -filetype=obj | spirv-val %}
+
+; CHECK-DAG: OpCapability FunctionPointersINTEL
+; CHECK: OpExtension "SPV_INTEL_function_pointers"
+
+; CHECK: OpName %[[F1:.*]] "f1"
+; CHECK: OpName %[[F2:.*]] "f2"
+
+; CHECK: %[[TyBool:.*]] = OpTypeBool
+
+; CHECK %[[F1Ptr:.*]] = OpConstantFunctionPointerINTEL %{{.*}} %[[F2]]
+; CHECK %[[F2Ptr:.*]] = OpConstantFunctionPointerINTEL %{{.*}} %[[F2]]
+
+; CHECK OpPtrEqual %[[TyBool]] %[[F1Ptr]] %[[F2Ptr]]
+
+target datalayout = "e-i64:64-v16:16-v24:32-v32:32-v48:64-v96:128-v192:256-v256:256-v512:512-v1024:1024-n8:16:32:64-G1-P9-A0"
+target triple = "spirv64-intel"
+
+define spir_func void @f1() addrspace(9) {
+entry:
+ ret void
+}
+
+define spir_func void @f2() addrspace(9) {
+entry:
+ ret void
+}
+
+define spir_kernel void @foo() addrspace(9) {
+entry:
+ %a = icmp eq ptr addrspace(9) @f1, @f2
+ ret void
+}
|
| %a = icmp eq ptr addrspace(9) @f1, @f2 | ||
| ret void |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Test with a value that isn't dead. Also better to test with a compare that can't be trivially folded out
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, reworked the test in my latest commit, hopefully it's better now.
| @@ -0,0 +1,9 @@ | |||
| ; RUN: not llc --global-isel %s -o /dev/null 2>&1 | FileCheck %s | |||
| target datalayout = "e-i64:64-v16:16-v24:32-v32:32-v48:64-v96:128-v192:256-v256:256-v512:512-v1024:1024-n8:16:32:64-G1-P9-A0" | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| target datalayout = "e-i64:64-v16:16-v24:32-v32:32-v48:64-v96:128-v192:256-v256:256-v512:512-v1024:1024-n8:16:32:64-G1-P9-A0" |
| @@ -0,0 +1,9 @@ | |||
| ; RUN: not llc --global-isel %s -o /dev/null 2>&1 | FileCheck %s | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| ; RUN: not llc --global-isel %s -o /dev/null 2>&1 | FileCheck %s | |
| ; RUN: not llc --global-isel -filetype=null %s 2>&1 | FileCheck %s |
| @@ -0,0 +1,9 @@ | |||
| ; RUN: not llc --global-isel %s -o /dev/null 2>&1 | FileCheck %s | |||
| target datalayout = "e-i64:64-v16:16-v24:32-v32:32-v48:64-v96:128-v192:256-v256:256-v512:512-v1024:1024-n8:16:32:64-G1-P9-A0" | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| target datalayout = "e-i64:64-v16:16-v24:32-v32:32-v48:64-v96:128-v192:256-v256:256-v512:512-v1024:1024-n8:16:32:64-G1-P9-A0" |
| @@ -0,0 +1,9 @@ | |||
| ; RUN: not llc --global-isel %s -o /dev/null 2>&1 | FileCheck %s | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| ; RUN: not llc --global-isel %s -o /dev/null 2>&1 | FileCheck %s | |
| ; RUN: not llc --global-isel -filetype=null %s 2>&1 | FileCheck %s |
| @@ -0,0 +1,9 @@ | |||
| ; RUN: not llc --global-isel %s -o /dev/null 2>&1 | FileCheck %s | |||
| target datalayout = "e-i64:64-v16:16-v24:32-v32:32-v48:64-v96:128-v192:256-v256:256-v512:512-v1024:1024-n8:16:32:64-G1-P9-A0" | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| target datalayout = "e-i64:64-v16:16-v24:32-v32:32-v48:64-v96:128-v192:256-v256:256-v512:512-v1024:1024-n8:16:32:64-G1-P9-A0" |
| @@ -0,0 +1,9 @@ | |||
| ; RUN: not llc --global-isel %s -o /dev/null 2>&1 | FileCheck %s | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| ; RUN: not llc --global-isel %s -o /dev/null 2>&1 | FileCheck %s | |
| ; RUN: not llc --global-isel -filetype=null %s 2>&1 | FileCheck %s |
| @@ -0,0 +1,9 @@ | |||
| ; RUN: not llc --global-isel %s -o /dev/null 2>&1 | FileCheck %s | |||
| target datalayout = "e-i64:64-v16:16-v24:32-v32:32-v48:64-v96:128-v192:256-v256:256-v512:512-v1024:1024-n8:16:32:64-G1-P9-A0" | |||
| target triple = "spirv64-intel" | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| target triple = "spirv64-intel" | |
| target triple = "spirv64" |
| target datalayout = "e-i64:64-v16:16-v24:32-v32:32-v48:64-v96:128-v192:256-v256:256-v512:512-v1024:1024-n8:16:32:64-G1-P9-A0" | ||
| target triple = "spirv64-intel" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| target datalayout = "e-i64:64-v16:16-v24:32-v32:32-v48:64-v96:128-v192:256-v256:256-v512:512-v1024:1024-n8:16:32:64-G1-P9-A0" | |
| target triple = "spirv64-intel" | |
| target triple = "spirv64" |
Triple also redundant with command line flag
| @@ -84,17 +84,19 @@ SPIRVLegalizerInfo::SPIRVLegalizerInfo(const SPIRVSubtarget &ST) { | |||
| const LLT p6 = LLT::pointer(6, PSize); // SPV_INTEL_usm_storage_classes (Host) | |||
| const LLT p7 = LLT::pointer(7, PSize); // Input | |||
| const LLT p8 = LLT::pointer(8, PSize); // Output | |||
| const LLT p9 = | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There should be an address space enum somewhere
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We don't have an enum but we do have a mapping between the storage class operand and LLVM addrspace, if I were to use that here the code would look like
const LLT p8 = LLT::pointer(storageClassToAddressSpace(SPIRV::StorageClass::Output), PSize);
I'm not sure that's much better but if you prefer this way I'm happy to implement it.
maarquitos14
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Other than the issues pointed out by @arsenm, I'm happy with this. Just one more nit: the last sentence of the description is repeated.
jmmartinez
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Overall it looks good to me. Added just to minor comments.
Signed-off-by: Nick Sarnie <[email protected]>
jmmartinez
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks !
|
@arsenm Do you mind re-reviewing this to see if your feedback is addressed? Thanks! |
The SPV_INTEL_function_pointers extension defines a new storage class
CodeSectionINTELthat is represented in LLVM IR asaddrspace(9).Per the spec, it is basically not allowed to be casted to or interact with pointers with other storage classes.
Add
addrspace(9)as a known pointer type to the legalizer, and then add some error cases for IR that is impossible to legalize.Right now, if you try to run the backend on input with SPIR-V, basically everything errors saying that it is unable to legalize because
ptr addrspace(9)is not considered a pointer type.Ideally the FE should not generate the illegal IR or error out earlier, but we should catch it before generating invalid SPIR-V.