impala-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Tim Armstrong (Code Review)" <ger...@cloudera.org>
Subject [Impala-CR](cdh5-trunk) IMPALA-775,IMPALA-3374: Upgrade LLVM to 3.8.0
Date Tue, 19 Apr 2016 04:13:51 GMT
Tim Armstrong has posted comments on this change.

Change subject: IMPALA-775,IMPALA-3374: Upgrade LLVM to 3.8.0
......................................................................


Patch Set 11:

(4 comments)

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

Line 545:     return false;
> now that the DCHECK is gone, will we still notice during testing when this 
We should get an error propagated up to the user even if it switches to codegen. For IR udfs,
the query fails. We could consider modifying the test framework to assert that there are no
errors or warnings by default.


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

Line 292: VerifyFunction
> can this be private?
Done


http://gerrit.cloudera.org:8080/#/c/2486/11/be/src/exprs/scalar-fn-call.cc
File be/src/exprs/scalar-fn-call.cc:

Line 391:     return Status("Function codegen failed verification: see error log for details.");
> maybe put the udf name in the error?
There was actually an error template so used that.


http://gerrit.cloudera.org:8080/#/c/2486/11/bin/impala-config.sh
File bin/impala-config.sh:

Line 255:   # Debug builds should use the release+asserts build to get additional coverage.
> briefly explain why not using llvm debug build.
Done


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I17d7afd05ad3b472a0bfe035bfc3daada5597b2d
Gerrit-PatchSet: 11
Gerrit-Project: Impala
Gerrit-Branch: cdh5-trunk
Gerrit-Owner: Tim Armstrong <tarmstrong@cloudera.com>
Gerrit-Reviewer: Dan Hecht <dhecht@cloudera.com>
Gerrit-Reviewer: Michael Ho <kwho@cloudera.com>
Gerrit-Reviewer: Skye Wanderman-Milne <skye@cloudera.com>
Gerrit-Reviewer: Tim Armstrong <tarmstrong@cloudera.com>
Gerrit-HasComments: Yes

Mime
View raw message