Michael Ho has posted comments on this change.

Change subject: IMPALA-4164: Use inline hints instead of always-inline
......................................................................


Patch Set 2:

(2 comments)

http://gerrit.cloudera.org:8080/#/c/6941/2/be/src/codegen/llvm-codegen.cc
File be/src/codegen/llvm-codegen.cc:

Line 163:       target_cpu_attr_ = 
fn->getFnAttribute("target-cpu").getValueAsString();
> This solves the inlining problem but the outcome is a bit counterintuitive 
When we add AVX2 instruction to cross-compiled code, wouldn't we need to add 
"-mavx2" to the compilation flag when cross-compiling the code ? In that case, 
wouldn't the lowest-common-denominator also include "avx2" ? May be I 
misunderstood the comment ?


Line 166:       LOG(INFO) << "target_features_attr: " << target_features_attr_;
> Should this logging be left in?
This is logged once during start-up. Would be helpful for debugging.


-- 
To view, visit http://gerrit.cloudera.org:8080/6941
To unsubscribe, visit http://gerrit.cloudera.org:8080/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I2d87ae8d222b415587e7320cb9072e4a8d6615ce
Gerrit-PatchSet: 2
Gerrit-Project: Impala-ASF
Gerrit-Branch: master
Gerrit-Owner: Michael Ho <[email protected]>
Gerrit-Reviewer: Michael Ho <[email protected]>
Gerrit-Reviewer: Tim Armstrong <[email protected]>
Gerrit-HasComments: Yes

Reply via email to