Joe McDonnell has posted comments on this change. ( http://gerrit.cloudera.org:8080/19083 )
Change subject: IMPALA-11623: Put *-ir.cc files into their own libraries to avoid extra recompilation ...................................................................... Patch Set 3: (1 comment) http://gerrit.cloudera.org:8080/#/c/19083/3/be/src/exprs/CMakeLists.txt File be/src/exprs/CMakeLists.txt: http://gerrit.cloudera.org:8080/#/c/19083/3/be/src/exprs/CMakeLists.txt@85 PS3, Line 85: # TODO: Should we depend on gen_ir_descriptions? > The headers that include the generated ir description files are indeed scal I experimented with removing the references to gen_ir_descriptions across the codebase, and for Make it fails. For Ninja, it doesn't, but the Ninja build graph doesn't have an edge as far as I can tell. Builds can often succeed even with a missing edge due to the ordering in which targets are visited. In theory we need more edges, but I'm also ok with having the XIr inherit whatever was on X. If X had gen_ir_descriptions before, then XIr gets it. If not, then they both don't. The build is working prior to this change, and we also have options if we run into failure cases. In terms of actions: 1. Remove this comment. Keep this gen_ir_descriptions (and the one above on ExprIr). 2. Add a gen_ir_descriptions to UtilIr. -- To view, visit http://gerrit.cloudera.org:8080/19083 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: Impala-ASF Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: I63b9285bac6494a19f614d0ebc694a91bdf7a8a0 Gerrit-Change-Number: 19083 Gerrit-PatchSet: 3 Gerrit-Owner: Daniel Becker <[email protected]> Gerrit-Reviewer: Daniel Becker <[email protected]> Gerrit-Reviewer: Impala Public Jenkins <[email protected]> Gerrit-Reviewer: Joe McDonnell <[email protected]> Gerrit-Comment-Date: Tue, 11 Oct 2022 20:54:54 +0000 Gerrit-HasComments: Yes
