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();
> Yeah that approach seems reasonable unless there's some major cost to dupli
Will see if I can get a Jenkins machine without avx2. My suspicion is that it 
will not compile with either approaches (or in the existing code). When we 
start cross-compiling the avx2 code we probably need to handle it in a way 
similar to how we handle SSE4.2 (by cross-compiling different modules with and 
without SSE4.2). An alternative would be to compile functions relying on 
certain CPU features into separate modules and at initialization time, link in 
the appropriate modules based on the host's CPU features.

There is a function defined for removeFnAttribute() but I don't see how to use 
it with String attribute.


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

PS4, Line 644: ions to allow inlining.
             :   static std::string target_cpu_attr_;
garbled.


-- 
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