You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tvm.apache.org by GitBox <gi...@apache.org> on 2021/09/27 19:56:16 UTC
[GitHub] [tvm] kparzysz-quic opened a new pull request #9138: [LLVM] Make changes needed for opaque pointers
kparzysz-quic opened a new pull request #9138:
URL: https://github.com/apache/tvm/pull/9138
- Pass value type to all `Create.*Load` and `Create.*GEP` functions.
- Create type `TypedPointer` to keep both the address and the pointee's type when buffer pointers etc. are created.
- Eliminate calls to `getPointerElementType`, except one in creating debug info (that seems necessary for the time being).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] csullivan commented on a change in pull request #9138: [LLVM] Make changes needed for opaque pointers
Posted by GitBox <gi...@apache.org>.
csullivan commented on a change in pull request #9138:
URL: https://github.com/apache/tvm/pull/9138#discussion_r721761938
##########
File path: src/target/llvm/codegen_llvm.cc
##########
@@ -861,10 +871,10 @@ llvm::Value* CodeGenLLVM::CreateIntrinsic(const CallNode* op) {
: llvm::Type::getVoidTy(*ctx_);
llvm::Function* f = GetIntrinsicDecl(id, return_type, arg_type);
ICHECK(f) << "Cannot find intrinsic declaration, possible type mismatch: "
-#if TVM_LLVM_VERSION <= 130
- << llvm::Intrinsic::getName(id, {});
+#if TVM_LLVM_VERSION >= 130
+ << llvm::Intrinsic::getBaseName(id).str();
#else
- << llvm::Intrinsic::getName(id, return_type, {});
+ << llvm::Intrinsic::getName(id, {});
Review comment:
>release/181/llvm-10.0.0/bin/llvm-config --version
13.0.0
Using a copy of the hexagon llvm libraries from early this summer (rev 181) this now triggers a compile error. Did you mean `> 130` instead of `>=` here?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] kparzysz-quic commented on pull request #9138: [LLVM] Make changes needed for opaque pointers
Posted by GitBox <gi...@apache.org>.
kparzysz-quic commented on pull request #9138:
URL: https://github.com/apache/tvm/pull/9138#issuecomment-930291317
More changes related to that will be needed, and I'm planning to keep making them as the LLVM IR evolves.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] kparzysz-quic edited a comment on pull request #9138: [LLVM] Make changes needed for opaque pointers
Posted by GitBox <gi...@apache.org>.
kparzysz-quic edited a comment on pull request #9138:
URL: https://github.com/apache/tvm/pull/9138#issuecomment-930279683
LLVM IR is eliminating types in pointers, so there will only be `ptr` for a pointer type (i.e. pointers will be "opaque"), instead of `i32*` or `float*`. This will require passing pointer type in addition to the pointer itself to a bunch of LLVM IR creation functions, plus functions like `getPointerElementType` will be removed.
This is an ongoing change in LLVM, but it's a pervasive one with a lot of impact on downstream consumers.
Edit: Link to a document about this effort:
https://github.com/llvm/llvm-project/blob/main/llvm/docs/OpaquePointers.rst
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] jroesch commented on pull request #9138: [LLVM] Make changes needed for opaque pointers
Posted by GitBox <gi...@apache.org>.
jroesch commented on pull request #9138:
URL: https://github.com/apache/tvm/pull/9138#issuecomment-930359394
> https://github.com/llvm/llvm-project/blob/main/llvm/docs/OpaquePointers.rst
Ah I didn't know about this, is this fallout related to the semantic changes in the IR? I'll check out the docs as well. LGTM thanks for educating me on this one.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] jroesch commented on pull request #9138: [LLVM] Make changes needed for opaque pointers
Posted by GitBox <gi...@apache.org>.
jroesch commented on pull request #9138:
URL: https://github.com/apache/tvm/pull/9138#issuecomment-929898316
Changes over all make sense, is this to prep for another change? was confused about "opaque pointer"
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] kparzysz-quic commented on pull request #9138: [LLVM] Make changes needed for opaque pointers
Posted by GitBox <gi...@apache.org>.
kparzysz-quic commented on pull request #9138:
URL: https://github.com/apache/tvm/pull/9138#issuecomment-930369662
Since memory in LLVM is generally untyped, specific data types are provided at the time of accessing the memory, or computing addresses. For example `getelementptr` needs to know the "starting type" (i.e. the type associated with the input pointer) in order to properly calculate the address corresponding to the index arguments. In the past, this information was included in the pointer type, i.e. from `i32*`, you could easily infer that the pointee was a `i32`. With this information removed from pointers, these instructions need that detailed type information passed in separately.
The fallout comes from the fact that many functions in LLVM now need the extra type information passed to them. This mostly applies to the IR builder functions, but any code using LLVM may be subject to changes, if it relied on getting pointee type information from the pointer type itself.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] kparzysz-quic commented on pull request #9138: [LLVM] Make changes needed for opaque pointers
Posted by GitBox <gi...@apache.org>.
kparzysz-quic commented on pull request #9138:
URL: https://github.com/apache/tvm/pull/9138#issuecomment-930279683
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] kparzysz-quic edited a comment on pull request #9138: [LLVM] Make changes needed for opaque pointers
Posted by GitBox <gi...@apache.org>.
kparzysz-quic edited a comment on pull request #9138:
URL: https://github.com/apache/tvm/pull/9138#issuecomment-930279683
LLVM IR is eliminating types in pointers, so there will only be `ptr` for a pointer type (i.e. pointers will be "opaque"), instead of `i32*` or `float*`. This will require passing pointer type in addition to the pointer itself to a bunch of LLVM IR creation functions, plus functions like `getPointerElementType` will be removed.
This is an ongoing change in LLVM, but it's a pervasive one with a lot of impact on downstream consumers.
Edit: Link to a document about this effort:
https://github.com/llvm/llvm-project/blob/main/llvm/docs/OpaquePointers.rst
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] jroesch commented on pull request #9138: [LLVM] Make changes needed for opaque pointers
Posted by GitBox <gi...@apache.org>.
jroesch commented on pull request #9138:
URL: https://github.com/apache/tvm/pull/9138#issuecomment-929898316
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] jroesch merged pull request #9138: [LLVM] Make changes needed for opaque pointers
Posted by GitBox <gi...@apache.org>.
jroesch merged pull request #9138:
URL: https://github.com/apache/tvm/pull/9138
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] jroesch merged pull request #9138: [LLVM] Make changes needed for opaque pointers
Posted by GitBox <gi...@apache.org>.
jroesch merged pull request #9138:
URL: https://github.com/apache/tvm/pull/9138
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] kparzysz-quic commented on pull request #9138: [LLVM] Make changes needed for opaque pointers
Posted by GitBox <gi...@apache.org>.
kparzysz-quic commented on pull request #9138:
URL: https://github.com/apache/tvm/pull/9138#issuecomment-930279683
LLVM IR is eliminating types in pointers, so there will only be `ptr` for a pointer type (i.e. pointers will be "opaque"), instead of `i32*` or `float*`. This will require passing pointer type in addition to the pointer itself to a bunch of LLVM IR creation functions, plus functions like `getPointerElementType` will be removed.
This is an ongoing change in LLVM, but it's a pervasive one with a lot of impact on downstream consumers,
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tvm] csullivan commented on a change in pull request #9138: [LLVM] Make changes needed for opaque pointers
Posted by GitBox <gi...@apache.org>.
csullivan commented on a change in pull request #9138:
URL: https://github.com/apache/tvm/pull/9138#discussion_r721761938
##########
File path: src/target/llvm/codegen_llvm.cc
##########
@@ -861,10 +871,10 @@ llvm::Value* CodeGenLLVM::CreateIntrinsic(const CallNode* op) {
: llvm::Type::getVoidTy(*ctx_);
llvm::Function* f = GetIntrinsicDecl(id, return_type, arg_type);
ICHECK(f) << "Cannot find intrinsic declaration, possible type mismatch: "
-#if TVM_LLVM_VERSION <= 130
- << llvm::Intrinsic::getName(id, {});
+#if TVM_LLVM_VERSION >= 130
+ << llvm::Intrinsic::getBaseName(id).str();
#else
- << llvm::Intrinsic::getName(id, return_type, {});
+ << llvm::Intrinsic::getName(id, {});
Review comment:
>release/181/llvm-10.0.0/bin/llvm-config --version
13.0.0
Using a copy of the hexagon llvm libraries from early this summer (rev 181) this now triggers a compile error. Did you mean `>` instead of `>=` here?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tvm.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org