Todd Lipcon has posted comments on this change. Change subject: KUDU-1448. Enable AVX2 bitshuffle at runtime ......................................................................
Patch Set 1: (4 comments) http://gerrit.cloudera.org:8080/#/c/5166/1/src/kudu/cfile/bshuf_block.h File src/kudu/cfile/bshuf_block.h: Line 29: #include "kudu/cfile/bitshuffle_arch_wrapper.h" > error: 'kudu/cfile/bitshuffle_arch_wrapper.h' file not found [clang-diagnos woops http://gerrit.cloudera.org:8080/#/c/5166/1/src/kudu/gutil/cpu.cc File src/kudu/gutil/cpu.cc: Line 61: : "a"(info_type) > My x86 knowledge is weak, but I'll ask the dumb question anyway: should the Yea, I guess it's dead code because we don't support i386, but don't think now's the time to shave that yak (we have 25 such places we check for i386) As for the actual question, it isn't relevant because ecx is only used for the "extended CPUID flags" (info_type = 7), and that didn't exist on 32-bit afaik. Line 71: : "a"(info_type), "c"(0) > Does Kudu use cpuid to selectively enable/disable other features? Or is thi This is the first case. We check other CPU flags in init.cc but only use them to do an early FATAL on unsupported cpus. http://gerrit.cloudera.org:8080/#/c/5166/1/thirdparty/build-definitions.sh File thirdparty/build-definitions.sh: Line 447: # iochain.c doesn't benefit from the architecture optimization. > Does it make sense to include iochain.c in the above if only to simplify th Done -- To view, visit http://gerrit.cloudera.org:8080/5166 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-MessageType: comment Gerrit-Change-Id: Ied8887c0240a649158085676fdd08ab8628bf7d6 Gerrit-PatchSet: 1 Gerrit-Project: kudu Gerrit-Branch: master Gerrit-Owner: Todd Lipcon <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Dan Burkert <[email protected]> Gerrit-Reviewer: Kudu Jenkins Gerrit-Reviewer: Tidy Bot Gerrit-Reviewer: Todd Lipcon <[email protected]> Gerrit-HasComments: Yes
