kpet created this revision. kpet added reviewers: tra, ABataev, echristo, jlebar, hfinkel, rsmith. Herald added subscribers: cfe-commits, kristof.beyls, javed.absar. Herald added a project: clang.
The action collapsing logic introduced in https://reviews.llvm.org/D21840 generally works by replacing action A followed by B by an action that uses the tool for A with action B. The logic in combineAssembleBackend however does something different and selects a tool for an action that is not part of the list of actions under consideration for collapsing, specifically it selects the tool for the CompileJobAction on which the BackendJobAction depends. There doesn't seem to be any good reason that I could find for doing this. My understanding is that all cases of interest rely on the Clang tool being selected for the CompileJobAction. However the same Clang tool would be selected for a BackendJobAction and doing so would make the collapsing logic more symmetric and make it easier to further refactor into something more generic. The reason I'm looking at this is that I am currently working on a toolchain that uses a single tool with combined BackendJobAction and AssembleJobAction that is not Clang but uses Clang as the tool for CompileJobAction. Am I missing something? Are we missing a test? Signed-off-by: Kevin Petit <kevin.pe...@arm.com> Repository: rC Clang https://reviews.llvm.org/D62722 Files: clang/lib/Driver/Driver.cpp Index: clang/lib/Driver/Driver.cpp =================================================================== --- clang/lib/Driver/Driver.cpp +++ clang/lib/Driver/Driver.cpp @@ -3733,18 +3733,8 @@ if (!AJ || !BJ) return nullptr; - // Retrieve the compile job, backend action must always be preceded by one. - ActionList CompileJobOffloadActions; - auto *CJ = getPrevDependentAction(BJ->getInputs(), CompileJobOffloadActions, - /*CanBeCollapsed=*/false); - if (!AJ || !BJ || !CJ) - return nullptr; - - assert(isa<CompileJobAction>(CJ) && - "Expecting compile job preceding backend job."); - - // Get compiler tool. - const Tool *T = TC.SelectTool(*CJ); + // Get backend tool. + const Tool *T = TC.SelectTool(*BJ); if (!T) return nullptr;
Index: clang/lib/Driver/Driver.cpp =================================================================== --- clang/lib/Driver/Driver.cpp +++ clang/lib/Driver/Driver.cpp @@ -3733,18 +3733,8 @@ if (!AJ || !BJ) return nullptr; - // Retrieve the compile job, backend action must always be preceded by one. - ActionList CompileJobOffloadActions; - auto *CJ = getPrevDependentAction(BJ->getInputs(), CompileJobOffloadActions, - /*CanBeCollapsed=*/false); - if (!AJ || !BJ || !CJ) - return nullptr; - - assert(isa<CompileJobAction>(CJ) && - "Expecting compile job preceding backend job."); - - // Get compiler tool. - const Tool *T = TC.SelectTool(*CJ); + // Get backend tool. + const Tool *T = TC.SelectTool(*BJ); if (!T) return nullptr;
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits