Tamas Mate has posted comments on this change. ( http://gerrit.cloudera.org:8080/15718 )
Change subject: IMPALA-9645 Port LLVM codegen to adapt aarch64 ...................................................................... Patch Set 9: (3 comments) Hi Zhao, thank you for the update. I had some questions on the whitelisted CPU attributes. http://gerrit.cloudera.org:8080/#/c/15718/9/be/src/codegen/codegen-anyval.cc File be/src/codegen/codegen-anyval.cc: http://gerrit.cloudera.org:8080/#/c/15718/9/be/src/codegen/codegen-anyval.cc@416 PS9, Line 416: // Lowered type is of form { {i8}, [15 x i8], {i128} }. Get the i128 value and : // truncate it to the correct size. (The {i128} corresponds to the union of the : // different width int types.) : // On aarch64, the Lowered type is of form { {i8}, {i128} }. No padding add. : #ifdef __aarch64__ : uint32_t idxs[] = {1, 0}; : #else : uint32_t idxs[] = {2, 0}; : #endif Tim mentioned earlier that the comment could be next to the relevant code like this: #ifdef __aarch64__ // On aarch64, the Lowered type is of form { {i8}, {i128} }. No padding add. uint32_t idxs[] = {1, 0}; #else // Lowered type is of form { {i8}, [15 x i8], {i128} }. Get the i128 value and // truncate it to the correct size. (The {i128} corresponds to the union of the // different width int types.) uint32_t idxs[] = {2, 0}; #endif Same later from line #470. http://gerrit.cloudera.org:8080/#/c/15718/9/be/src/codegen/llvm-codegen.cc File be/src/codegen/llvm-codegen.cc: http://gerrit.cloudera.org:8080/#/c/15718/9/be/src/codegen/llvm-codegen.cc@113 PS9, Line 113: 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, Are these cpu attributes relevant on aarch64? http://gerrit.cloudera.org:8080/#/c/15718/9/be/src/codegen/llvm-codegen.cc@117 PS9, Line 117: The default flags are a known-good set that are " : "routinely tested. Does this sentence remain true in aarch64 context? -- To view, visit http://gerrit.cloudera.org:8080/15718 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I3f30ee84ea9bf5245da88154632bb69079103d11 Gerrit-Change-Number: 15718 Gerrit-PatchSet: 9 Gerrit-Owner: Anonymous Coward <zhaoren...@hotmail.com> Gerrit-Reviewer: Anonymous Coward <zhaoren...@hotmail.com> Gerrit-Reviewer: Impala Public Jenkins <impala-public-jenk...@cloudera.com> Gerrit-Reviewer: Tamas Mate <tm...@cloudera.com> Gerrit-Reviewer: Tim Armstrong <tarmstr...@cloudera.com> Gerrit-Comment-Date: Mon, 11 May 2020 12:58:33 +0000 Gerrit-HasComments: Yes
