IMPALA-6291: disable AVX512 codegen in LLVM Adds a whitelist of LLVM CPU attributes that I know that we routinely test Impala with. This excludes the problematic AVX512 attributes as well as some other flags we don't test with - e.g. AMD-only instructions, NVM-related instructions, etc. We're unlikely to get significant benefit from these instruction set extensions without explicitly using them via instrinsics.
Testing: Ran core tests on a system with AVX512 support with a prototype patch that disabled only the AVX512 flags. Added a backend test to make sure that the whitelisting is working as expected. Change-Id: Ic7c3ee3e370bafc50d855113485b7e6925f7bf6a Reviewed-on: http://gerrit.cloudera.org:8080/8802 Reviewed-by: Tim Armstrong <tarmstr...@cloudera.com> Tested-by: Impala Public Jenkins Project: http://git-wip-us.apache.org/repos/asf/impala/repo Commit: http://git-wip-us.apache.org/repos/asf/impala/commit/eeb34b65 Tree: http://git-wip-us.apache.org/repos/asf/impala/tree/eeb34b65 Diff: http://git-wip-us.apache.org/repos/asf/impala/diff/eeb34b65 Branch: refs/heads/master Commit: eeb34b657105e87bb7ee82a4724bfc6688d54cce Parents: e81b7c6 Author: Tim Armstrong <tarmstr...@cloudera.com> Authored: Fri Dec 8 09:16:25 2017 -0800 Committer: Impala Public Jenkins <impala-public-jenk...@gerrit.cloudera.org> Committed: Sat Dec 9 03:45:01 2017 +0000 ---------------------------------------------------------------------- be/src/codegen/llvm-codegen-test.cc | 20 +++++++++++++++ be/src/codegen/llvm-codegen.cc | 44 ++++++++++++++++++++++++++++++-- be/src/codegen/llvm-codegen.h | 9 +++++++ 3 files changed, 71 insertions(+), 2 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/impala/blob/eeb34b65/be/src/codegen/llvm-codegen-test.cc ---------------------------------------------------------------------- diff --git a/be/src/codegen/llvm-codegen-test.cc b/be/src/codegen/llvm-codegen-test.cc index af6245a..cdd2b89 100644 --- a/be/src/codegen/llvm-codegen-test.cc +++ b/be/src/codegen/llvm-codegen-test.cc @@ -485,6 +485,26 @@ TEST_F(LlvmCodeGenTest, HandleLinkageError) { codegen->Close(); } +// Test that the default whitelisting disables the expected attributes. +TEST_F(LlvmCodeGenTest, CpuAttrWhitelist) { + // Non-existent attributes should be disabled regardless of initial states. + // Whitelisted attributes like sse2 and lzcnt should retain their initial + // state. + EXPECT_EQ(vector<string>( + {"-dummy1", "-dummy2", "-dummy3", "-dummy4", "+sse2", "-lzcnt"}), + LlvmCodeGen::ApplyCpuAttrWhitelist( + {"+dummy1", "+dummy2", "-dummy3", "+dummy4", "+sse2", "-lzcnt"})); + + // IMPALA-6291: Test that all AVX512 attributes are disabled. + vector<string> avx512_attrs; + EXPECT_EQ(vector<string>( + {"-avx512ifma", "-avx512dqavx512er", "-avx512f", "-avx512bw", "-avx512vl", + "-avx512cd", "-avx512vbmi", "-avx512pf"}), + LlvmCodeGen::ApplyCpuAttrWhitelist( + {"+avx512ifma", "+avx512dqavx512er", "+avx512f", "+avx512bw", "+avx512vl", + "+avx512cd", "+avx512vbmi", "+avx512pf"})); +} + } int main(int argc, char **argv) { http://git-wip-us.apache.org/repos/asf/impala/blob/eeb34b65/be/src/codegen/llvm-codegen.cc ---------------------------------------------------------------------- diff --git a/be/src/codegen/llvm-codegen.cc b/be/src/codegen/llvm-codegen.cc index 0dcbd1c..ca78bbb 100644 --- a/be/src/codegen/llvm-codegen.cc +++ b/be/src/codegen/llvm-codegen.cc @@ -101,6 +101,19 @@ DEFINE_string(opt_module_dir, "", DEFINE_string(asm_module_dir, "", "if set, saves disassembly for generated IR modules to the specified directory."); DECLARE_string(local_library_dir); +// IMPALA-6291: AVX-512 and other CPU attrs the community doesn't routinely test are +// disabled. AVX-512 is affected by known bugs in LLVM 3.9.1. The following attrs that +// exist in LLVM 3.9.1 are disabled: avx512bw,avx512cd,avx512dq,avx512er,avx512f, +// avx512ifma,avx512pf,avx512vbmi,avx512vl,clflushopt,clwb,fma4,mwaitx.1.2,pcommit,pku, +// prefetchwt1,sgx,sha,sse4a,tbm,xop,xsavec,xsaves. If new attrs are added to LLVM, +// they will be disabled until added to this whitelist. +DEFINE_string_hidden(llvm_cpu_attr_whitelist, "adx,aes,avx,avx2,bmi,bmi2,cmov,cx16,f16c," + "fma,fsgsbase,hle,invpcid,lzcnt,mmx,movbe,pclmul,popcnt,prfchw,rdrnd,rdseed,rtm,smap," + "sse,sse2,sse3,sse4.1,sse4.2,ssse3,xsave,xsaveopt", + "(Experimental) a comma-separated list of LLVM CPU attribute flags that are enabled " + "for runtime code generation. The default flags are a known-good set that are " + "routinely tested. This flag is provided to enable additional LLVM CPU attribute " + "flags for testing."); namespace impala { @@ -138,8 +151,10 @@ Status LlvmCodeGen::InitializeLlvm(bool load_backend) { cpu_name_ = llvm::sys::getHostCPUName().str(); LOG(INFO) << "CPU class for runtime code generation: " << cpu_name_; GetHostCPUAttrs(&cpu_attrs_); - target_features_attr_ = boost::algorithm::join(cpu_attrs_, ","); - LOG(INFO) << "CPU flags for runtime code generation: " << target_features_attr_; + LOG(INFO) << "Detected CPU flags: " << boost::join(cpu_attrs_, ","); + cpu_attrs_ = ApplyCpuAttrWhitelist(cpu_attrs_); + target_features_attr_ = boost::join(cpu_attrs_, ","); + LOG(INFO) << "CPU flags enabled for runtime code generation: " << target_features_attr_; // Write an empty map file for perf to find. if (FLAGS_perf_map) CodegenSymbolEmitter::WritePerfMap(); @@ -1637,6 +1652,31 @@ llvm::Constant* LlvmCodeGen::ConstantsToGVArrayPtr(llvm::Type* element_type, return ConstantToGVPtr(array_type, array_const, name); } +vector<string> LlvmCodeGen::ApplyCpuAttrWhitelist(const vector<string>& cpu_attrs) { + vector<string> result; + vector<string> attr_whitelist; + boost::split(attr_whitelist, FLAGS_llvm_cpu_attr_whitelist, boost::is_any_of(",")); + for (const string& attr : cpu_attrs) { + DCHECK_GE(attr.size(), 1); + DCHECK(attr[0] == '-' || attr[0] == '+') << attr; + if (attr[0] == '-') { + // Already disabled - copy it over unmodified. + result.push_back(attr); + continue; + } + const string attr_name = attr.substr(1); + auto it = std::find(attr_whitelist.begin(), attr_whitelist.end(), attr_name); + if (it != attr_whitelist.end()) { + // In whitelist - copy it over unmodified. + result.push_back(attr); + } else { + // Not in whitelist - disable it. + result.push_back("-" + attr_name); + } + } + return result; +} + void LlvmCodeGen::DiagnosticHandler::DiagnosticHandlerFn( const llvm::DiagnosticInfo& info, void* context) { if (info.getSeverity() == llvm::DiagnosticSeverity::DS_Error) { http://git-wip-us.apache.org/repos/asf/impala/blob/eeb34b65/be/src/codegen/llvm-codegen.h ---------------------------------------------------------------------- diff --git a/be/src/codegen/llvm-codegen.h b/be/src/codegen/llvm-codegen.h index bbfda3e..672ed27 100644 --- a/be/src/codegen/llvm-codegen.h +++ b/be/src/codegen/llvm-codegen.h @@ -540,6 +540,7 @@ class LlvmCodeGen { private: friend class ExprCodegenTest; friend class LlvmCodeGenTest; + friend class LlvmCodeGenTest_CpuAttrWhitelist_Test; friend class SubExprElimination; /// Top level codegen object. 'module_id' is used for debugging when outputting the IR. @@ -659,6 +660,14 @@ class LlvmCodeGen { /// generated is retained by the execution engine. void DestroyModule(); + /// Disable CPU attributes in 'cpu_attrs' that are not present in + /// the '--llvm_cpu_attr_whitelist' flag. The same attributes in the input are + /// always present in the output, except "+" is flipped to "-" for the disabled + /// attributes. E.g. if 'cpu_attrs' is {"+x", "+y", "-z"} and the whitelist is + /// {"x", "z"}, returns {"+x", "-y", "-z"}. + static std::vector<std::string> ApplyCpuAttrWhitelist( + const std::vector<std::string>& cpu_attrs); + /// Whether InitializeLlvm() has been called. static bool llvm_initialized_;
