impala-reviews mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Michael Ho (Code Review)" <ger...@cloudera.org>
Subject [Impala-ASF-CR] IMPALA-4164: Use inline hints instead of always-inline
Date Tue, 23 May 2017 21:21:43 GMT
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 <kwho@cloudera.com>
Gerrit-Reviewer: Michael Ho <kwho@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <tarmstrong@cloudera.com>
Gerrit-HasComments: Yes

Mime
View raw message