-
Notifications
You must be signed in to change notification settings - Fork 229
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
Support llvm.is.constant #672
Support llvm.is.constant #672
Conversation
@AlexeySachkov, could you address this request, please? |
CLA appears to be signed already |
e860496
to
d8ca85f
Compare
lib/SPIRV/SPIRVWriter.cpp
Outdated
@@ -259,6 +259,19 @@ static bool recursiveType(const StructType *ST, const Type *Ty) { | |||
return Run(Ty); | |||
} | |||
|
|||
static bool isManifestConstant(const Constant *c) { |
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.
Hi, @AlexeySachkov
Based the pre-ci log, we have some tidying errors here, could you help replace the input parameter "const Constant *c" with "const Constant *C"?
Thanks very much.
lib/SPIRV/SPIRVWriter.cpp
Outdated
if (isa<ConstantData>(c)) { | ||
return true; | ||
} else if (isa<ConstantAggregate>(c) || isa<ConstantExpr>(c)) { | ||
for (const Value *subc : c->operand_values()) { |
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 same for "subc" here, should change to "Subc". The error log from pre-ci system:
Found tidying errors!
/home/travis/build/KhronosGroup/SPIRV-LLVM-Translator/lib/SPIRV/SPIRVWriter.cpp:262:48: warning: invalid case style for parameter 'c' [readability-identifier-naming]
static bool isManifestConstant(const Constant *c) {
^~
C
/home/travis/build/KhronosGroup/SPIRV-LLVM-Translator/lib/SPIRV/SPIRVWriter.cpp:266:23: warning: invalid case style for variable 'subc' [readability-identifier-naming]
for (const Value *subc : c->operand_values()) {
^~~~
Thanks very much.
040ca3f
to
f35ccf5
Compare
Signed-off-by: Ge Jin <ge.jin@intel.com> Co-authored-by: Alexey Sachkov <alexey.sachkov@intel.com> Co-authored-by: Artem Gindinson <artem.gindinson@intel.com>
f35ccf5
to
b9e2bdc
Compare
This might be not the best level of support for the intrinsic, as further optimizations could help to get more precise results (i.e. constant propagation, inlining, etc.), but at the same time it should be good enough to preserve functionality of modules containing
@llvm.is.constant
intrinsic.The issue were reported in intel/llvm#2122 and can be reproduced by compiling SYCL app with C++ headers from GCC9 (they contain calls to
__builtin_constant_p
which produces@llvm.is.constant
intrinsic)