You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@doris.apache.org by "zhiqiang-hhhh (via GitHub)" <gi...@apache.org> on 2024/02/22 03:41:31 UTC
[PR] [opt](performance) strict aliasing for Int8 [doris]
zhiqiang-hhhh opened a new pull request, #31251:
URL: https://github.com/apache/doris/pull/31251
before


after


--
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@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [opt](performance) strict aliasing for Int8 [doris]
Posted by "zhiqiang-hhhh (via GitHub)" <gi...@apache.org>.
zhiqiang-hhhh commented on PR #31251:
URL: https://github.com/apache/doris/pull/31251#issuecomment-1958776921
run buildall
--
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@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [opt](performance) strict aliasing for Int8 [doris]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on code in PR #31251:
URL: https://github.com/apache/doris/pull/31251#discussion_r1498690957
##########
be/src/vec/functions/math.cpp:
##########
@@ -183,7 +183,8 @@ struct AbsImpl {
static inline ResultType apply(A a) {
if constexpr (IsDecimalNumber<A>)
return a < A(0) ? A(-a) : a;
- else if constexpr (std::is_integral_v<A> && std::is_signed_v<A>)
+ else if constexpr ((std::is_integral_v<A> && std::is_signed_v<A>) ||
+ std::is_same_v<A, doris::vectorized::Int8>)
Review Comment:
warning: statement should be inside braces [readability-braces-around-statements]
```suggestion
std::is_same_v<A, doris::vectorized::Int8>) {
```
be/src/vec/functions/math.cpp:188:
```diff
- else if constexpr (std::is_integral_v<A> && std::is_unsigned_v<A>)
+ } else if constexpr (std::is_integral_v<A> && std::is_unsigned_v<A>)
```
--
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@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [opt](performance) strict aliasing for Int8 [doris]
Posted by "zhiqiang-hhhh (via GitHub)" <gi...@apache.org>.
zhiqiang-hhhh commented on PR #31251:
URL: https://github.com/apache/doris/pull/31251#issuecomment-1958749955
run buildall
--
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@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [opt](performance) strict aliasing for Int8 [doris]
Posted by "doris-robot (via GitHub)" <gi...@apache.org>.
doris-robot commented on PR #31251:
URL: https://github.com/apache/doris/pull/31251#issuecomment-1958599682
Thank you for your contribution to Apache Doris.
Don't know what should be done next? See [How to process your PR](https://cwiki.apache.org/confluence/display/DORIS/How+to+process+your+PR)
--
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@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [opt](performance) strict aliasing for Int8 [doris]
Posted by "zhiqiang-hhhh (via GitHub)" <gi...@apache.org>.
zhiqiang-hhhh closed pull request #31251: [opt](performance) strict aliasing for Int8
URL: https://github.com/apache/doris/pull/31251
--
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@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [opt](performance) strict aliasing for Int8 [doris]
Posted by "github-actions[bot] (via GitHub)" <gi...@apache.org>.
github-actions[bot] commented on code in PR #31251:
URL: https://github.com/apache/doris/pull/31251#discussion_r1498578888
##########
be/src/exprs/runtime_filter.cpp:
##########
@@ -574,7 +575,7 @@ class RuntimePredicateWrapper {
case TYPE_TINYINT: {
batch_assign(in_filter, [](std::shared_ptr<HybridSetBase>& set, PColumnValue& column,
ObjectPool* pool) {
- int8_t int_val = static_cast<int8_t>(column.intval());
+ vectorized::Int8 int_val = static_cast<vectorized::Int8>(column.intval());
Review Comment:
warning: use auto when initializing with a cast to avoid duplicating the type name [modernize-use-auto]
```suggestion
auto int_val = static_cast<vectorized::Int8>(column.intval());
```
##########
be/src/vec/common/hash_table/hash.h:
##########
@@ -20,6 +20,7 @@
#pragma once
+#include <cstdint>
#include <type_traits>
#include "parallel_hashmap/phmap_utils.h"
Review Comment:
warning: 'parallel_hashmap/phmap_utils.h' file not found [clang-diagnostic-error]
```cpp
#include "parallel_hashmap/phmap_utils.h"
^
```
##########
be/src/exprs/runtime_filter.cpp:
##########
@@ -752,8 +753,10 @@
return _context.minmax_func->assign(&min_val, &max_val);
}
case TYPE_TINYINT: {
- int8_t min_val = static_cast<int8_t>(minmax_filter->min_val().intval());
- int8_t max_val = static_cast<int8_t>(minmax_filter->max_val().intval());
+ vectorized::Int8 min_val =
+ static_cast<vectorized::Int8>(minmax_filter->min_val().intval());
+ vectorized::Int8 max_val =
Review Comment:
warning: use auto when initializing with a cast to avoid duplicating the type name [modernize-use-auto]
```suggestion
auto max_val =
```
##########
be/src/vec/functions/math.cpp:
##########
@@ -183,7 +183,7 @@ struct AbsImpl {
static inline ResultType apply(A a) {
if constexpr (IsDecimalNumber<A>)
return a < A(0) ? A(-a) : a;
- else if constexpr (std::is_integral_v<A> && std::is_signed_v<A>)
+ else if constexpr ((std::is_integral_v<A> && std::is_signed_v<A>) || std::is_same_v<A, doris::vectorized::Int8>)
Review Comment:
warning: statement should be inside braces [readability-braces-around-statements]
```suggestion
else if constexpr ((std::is_integral_v<A> && std::is_signed_v<A>) || std::is_same_v<A, doris::vectorized::Int8>) {
```
be/src/vec/functions/math.cpp:187:
```diff
- else if constexpr (std::is_integral_v<A> && std::is_unsigned_v<A>)
+ } else if constexpr (std::is_integral_v<A> && std::is_unsigned_v<A>)
```
##########
be/src/util/int8_to_string.h:
##########
@@ -0,0 +1,12 @@
+#pragma once
+
+#include <fmt/format.h>
Review Comment:
warning: 'fmt/format.h' file not found [clang-diagnostic-error]
```cpp
#include <fmt/format.h>
^
```
##########
be/src/exprs/runtime_filter.cpp:
##########
@@ -752,8 +753,10 @@
return _context.minmax_func->assign(&min_val, &max_val);
}
case TYPE_TINYINT: {
- int8_t min_val = static_cast<int8_t>(minmax_filter->min_val().intval());
- int8_t max_val = static_cast<int8_t>(minmax_filter->max_val().intval());
+ vectorized::Int8 min_val =
Review Comment:
warning: use auto when initializing with a cast to avoid duplicating the type name [modernize-use-auto]
```suggestion
auto min_val =
```
--
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@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org
Re: [PR] [opt](performance) strict aliasing for Int8 [doris]
Posted by "zhiqiang-hhhh (via GitHub)" <gi...@apache.org>.
zhiqiang-hhhh commented on PR #31251:
URL: https://github.com/apache/doris/pull/31251#issuecomment-1960693651
waiting BitInt support on GCC
--
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@doris.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@doris.apache.org
For additional commands, e-mail: commits-help@doris.apache.org