-
Notifications
You must be signed in to change notification settings - Fork 751
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
[SYCL] [NATIVECPU] Implement missing math builtins for scalar data types #11321
Changes from 6 commits
806f49a
ec01ed2
908425c
c9d1bda
126f265
a08c279
b8f667a
a9d6d09
7556444
edfe058
5009f36
45b75c5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -16,3 +16,4 @@ math/native_sqrt.cl | |
math/rint.cl | ||
math/round.cl | ||
math/trunc.cl | ||
shared/helpers.ll |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,9 @@ | ||
#include "func.h" | ||
|
||
#define GEN_UNARY_BUILTIN_T(NAME, TYPE) \ | ||
_CLC_OVERLOAD TYPE __##NAME##_helper(TYPE); \ | ||
_CLC_OVERLOAD TYPE __spirv_ocl_##NAME(TYPE n) { return __##NAME##_helper(n); } | ||
|
||
#define GEN_UNARY_BUILTIN(NAME) \ | ||
GEN_UNARY_BUILTIN_T(NAME, int) \ | ||
GEN_UNARY_BUILTIN_T(NAME, signed char) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
#include "helpers.h" | ||
|
||
GEN_UNARY_BUILTIN(popcount) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
#include "helpers.h" | ||
|
||
GEN_UNARY_BUILTIN(ceil) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,4 @@ | ||
#define IS_FABS | ||
#include "helpers.h" | ||
|
||
GEN_UNARY_BUILTIN(fabs) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
#include "helpers.h" | ||
|
||
GEN_UNARY_BUILTIN(floor) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,4 @@ | ||
#include "helpers.h" | ||
|
||
GEN_TERNARY_BUILTIN(fma); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,82 @@ | ||
#include "func.h" | ||
#include "types.h" | ||
|
||
#ifdef NO_CLANG_BUILTINS | ||
|
||
#define GEN_UNARY_BUILTIN_T(NAME, TYPE) \ | ||
_CLC_OVERLOAD TYPE __##NAME##_helper(TYPE); \ | ||
_CLC_OVERLOAD TYPE __spirv_ocl_##NAME(TYPE n) { return __##NAME##_helper(n); } | ||
|
||
#define GEN_TERNARY_BUILTIN_T(NAME, TYPE) \ | ||
_CLC_OVERLOAD TYPE __##NAME##_helper(TYPE, TYPE, TYPE); \ | ||
_CLC_OVERLOAD TYPE __spirv_ocl_##NAME(TYPE a, TYPE b, TYPE c) { \ | ||
return __##NAME##_helper(a, b, c); \ | ||
} | ||
#define GEN_UNARY_BUILTIN(NAME) \ | ||
GEN_UNARY_BUILTIN_T(NAME, float) \ | ||
GEN_UNARY_BUILTIN_T(NAME, double) | ||
|
||
#define GEN_TERNARY_BUILTIN(NAME) \ | ||
GEN_TERNARY_BUILTIN_T(NAME, float) \ | ||
GEN_TERNARY_BUILTIN_T(NAME, double) | ||
|
||
#else | ||
|
||
#ifndef IS_NATIVE | ||
#define GETNAME(ID) __spirv_ocl_##ID | ||
#else | ||
#define GETNAME(ID) __spirv_ocl_native_##ID | ||
#endif | ||
|
||
// Todo: fabs is the only builtin whose vector version is not named | ||
// __builtin_elementwise_##NAME | ||
#ifndef IS_FABS | ||
#define GEN_UNARY_VECTOR_BUILTIN(NAME, TYPE, NUM) \ | ||
_CLC_OVERLOAD TYPE##NUM GETNAME(NAME)(TYPE##NUM n) { \ | ||
return __builtin_elementwise_##NAME(n); \ | ||
} | ||
#else | ||
#define GEN_UNARY_VECTOR_BUILTIN(NAME, TYPE, NUM) \ | ||
_CLC_OVERLOAD TYPE##NUM GETNAME(NAME)(TYPE##NUM n) { \ | ||
return __builtin_elementwise_abs(n); \ | ||
} | ||
#endif | ||
|
||
#define GEN_UNARY_VECTOR_BUILTIN_T(NAME, TYPE) \ | ||
GEN_UNARY_VECTOR_BUILTIN(NAME, TYPE, 2) \ | ||
GEN_UNARY_VECTOR_BUILTIN(NAME, TYPE, 3) \ | ||
GEN_UNARY_VECTOR_BUILTIN(NAME, TYPE, 4) \ | ||
GEN_UNARY_VECTOR_BUILTIN(NAME, TYPE, 8) \ | ||
GEN_UNARY_VECTOR_BUILTIN(NAME, TYPE, 16) | ||
|
||
#define GEN_UNARY_BUILTIN(NAME) \ | ||
_CLC_OVERLOAD float GETNAME(NAME)(float n) { \ | ||
return __builtin_##NAME##f(n); \ | ||
} \ | ||
_CLC_OVERLOAD double GETNAME(NAME)(double n) { return __builtin_##NAME(n); } \ | ||
GEN_UNARY_VECTOR_BUILTIN_T(NAME, float) \ | ||
GEN_UNARY_VECTOR_BUILTIN_T(NAME, double) | ||
|
||
#define GEN_TERNARY_VECTOR_BUILTIN(NAME, TYPE, NUM) \ | ||
_CLC_OVERLOAD TYPE##NUM GETNAME(NAME)(TYPE##NUM n1, TYPE##NUM n2, \ | ||
TYPE##NUM n3) { \ | ||
return __builtin_elementwise_##NAME(n1, n2, n3); \ | ||
} | ||
|
||
#define GEN_TERNARY_VECTOR_BUILTIN_T(NAME, TYPE) \ | ||
GEN_TERNARY_VECTOR_BUILTIN(NAME, TYPE, 2) \ | ||
GEN_TERNARY_VECTOR_BUILTIN(NAME, TYPE, 3) \ | ||
GEN_TERNARY_VECTOR_BUILTIN(NAME, TYPE, 4) \ | ||
GEN_TERNARY_VECTOR_BUILTIN(NAME, TYPE, 8) \ | ||
GEN_TERNARY_VECTOR_BUILTIN(NAME, TYPE, 16) | ||
|
||
#define GEN_TERNARY_BUILTIN(NAME) \ | ||
_CLC_OVERLOAD float GETNAME(NAME)(float n1, float n2, float n3) { \ | ||
return __builtin_##NAME##f(n1, n2, n3); \ | ||
} \ | ||
_CLC_OVERLOAD double GETNAME(NAME)(double n1, double n2, double n3) { \ | ||
return __builtin_##NAME(n1, n2, n3); \ | ||
} \ | ||
GEN_TERNARY_VECTOR_BUILTIN_T(NAME, float) \ | ||
GEN_TERNARY_VECTOR_BUILTIN_T(NAME, double) | ||
#endif |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,4 @@ | ||
#define IS_NATIVE | ||
#include "helpers.h" | ||
|
||
GEN_UNARY_BUILTIN(cos) |
Original file line number | Diff line number | Diff line change | ||||||
---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,4 @@ | ||||||||
#define IS_NATIVE | ||||||||
#include "helpers.h" | ||||||||
|
||||||||
GEN_UNARY_BUILTIN(exp) | ||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think
Suggested change
etc There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good point, that's done, thank you |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,4 @@ | ||
#define IS_NATIVE | ||
#include "helpers.h" | ||
|
||
GEN_UNARY_BUILTIN(exp2) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,4 @@ | ||
#define IS_NATIVE | ||
#include "helpers.h" | ||
|
||
GEN_UNARY_BUILTIN(log) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
#define IS_NATIVE | ||
#include "helpers.h" | ||
|
||
GEN_UNARY_BUILTIN(log10) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
#define IS_NATIVE | ||
#include "helpers.h" | ||
|
||
GEN_UNARY_BUILTIN(log2) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
#define IS_NATIVE | ||
#include "helpers.h" | ||
|
||
GEN_UNARY_BUILTIN(sin) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,4 @@ | ||
#define IS_NATIVE | ||
#include "helpers.h" | ||
|
||
GEN_UNARY_BUILTIN(sqrt) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
#include "helpers.h" | ||
|
||
GEN_UNARY_BUILTIN(rint) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
#include "helpers.h" | ||
|
||
GEN_UNARY_BUILTIN(round) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
#include "helpers.h" | ||
|
||
GEN_UNARY_BUILTIN(sqrt) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
#include "helpers.h" | ||
|
||
GEN_UNARY_BUILTIN(trunc) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,17 @@ | ||
declare i32 @llvm.ctpop.i32(i32 %n) | ||
declare i8 @llvm.ctpop.i8(i8 %n) | ||
npmiller marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
|
||
define dso_local i32 @_Z17__popcount_helperi(i32 %x) { | ||
entry: | ||
%call = call i32 @llvm.ctpop.i32(i32 %x) | ||
ret i32 %call | ||
} | ||
|
||
|
||
define dso_local i8 @_Z17__popcount_helpera(i8 %x) { | ||
entry: | ||
%call = call i8 @llvm.ctpop.i8(i8 %x) | ||
ret i8 %call | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -45,5 +45,18 @@ RenameKernelSYCLNativeCPUPass::run(Module &M, ModuleAnalysisManager &MAM) { | |
} | ||
} | ||
|
||
//<<<<<<< HEAD | ||
// for (auto &F : CalledSet) { | ||
// auto NewName = sycl::utils::addSYCLNativeCPUSuffix(F->getName()); | ||
// F->setName(NewName); | ||
// auto Comdat = F->getComdat(); | ||
// if (Comdat) { | ||
// auto NewComdat = M.getOrInsertComdat(NewName.str()); | ||
// F->setComdat(NewComdat); | ||
// } | ||
// ModuleChanged |= true; | ||
// } | ||
//======= | ||
//>>>>>>> sycl | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. A merge gone wrong? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes :) That's fixed now, thanks for spotting it |
||
return ModuleChanged ? PreservedAnalyses::none() : PreservedAnalyses::all(); | ||
} |
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.
Are these __builtin* specific to nativecpu?
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.
They are part of
clang
see hereThere 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.
That's what I was thinking. It seems a bit strange to me to be calling generic clang builtins in a target specific libclc backend. Is there a good reason to do it like this?
Maybe these clang builtins better than the ones in the libclc/generic backend? Such generic impls that would be called if you don't add a target specific impl. Or could the clang builtins used here just be added to the libclc/generic impls?
Thanks
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.
The clang builtins should lead to the same LLVM intrinsics used in
libclc/generic
, but the way the SPIRV builtins are implemented ingeneric
doesn't play well with the x86 ABI on Linux (see #10970). I think you are right and it's worth considering changing the implementation ingeneric
, but I'm not 100% sure about the implications of doing so.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.
OK I see, makes sense, thanks for the clarification.