Michael Ho has posted comments on this change. Change subject: IMPALA-4164: Avoid overly aggressive inlining in LLVM IR ......................................................................
Patch Set 7: (3 comments) http://gerrit.cloudera.org:8080/#/c/6941/2/be/src/codegen/llvm-codegen.cc File be/src/codegen/llvm-codegen.cc: Line 645: // Check that there are no calls to FunctionContextImpl::GetConstFnAttr(). These should all > See my other comment about whether InlineHint is required. Done http://gerrit.cloudera.org:8080/#/c/6941/7/be/src/codegen/llvm-codegen.cc File be/src/codegen/llvm-codegen.cc: Line 700: // Set the cross-compiled functions' "target-cpu" and "target-features" to > It isn't limited to cross-compiled functions is it? Done Line 704: // If there is no "noinline" attribute, add an inline hint to the function. > Instead of adding the hint everywhere it seems better to adjust the inlinin In fact, it may be simpler to just use the default value (225) with -O2 optimization level. The new patch removes all inline hint and relies on the inlining pass to do the right thing. I ran some standard perf benchmarks with PS8 in the 16-node cluster and there was no regression. -- 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: 7 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
