Over time, SLPVectorizer.cpp has grown substantially in both size and complexity, which has made ongoing maintenance increasingly challenging. I am currently discussing with Alexey the possibility of improving modularity by decomposing this file into multiple, well-defined modules. Below is a proposed phased plan to initiate this effort:
Phase 0: Utilities: Introduce SLPUtils.h/.cpp.
This phase proposes extracting commonly used utility functions from SLPVectorizer.cpp into SLPUtils.h/.cpp. Examples include isValidElementType(), getValueType(), getNumElements(), getWidenedType() etc. These utilities will reside within slpvectorizer namespace. I have identified approximately 36 functions to be migrated; moving them in a single pull request would be error prone. Therefore, this phase will be split across at least three pull requests. This phase will also introduce new subdir called SLPVectorizer under llvm/lib/Transforms/Vectorize
Phase 1: Legality: Introduce SLPCompatibility.h/.cpp.
This phase proposes a new module defining classes such as InstructionsCompatibilityAnalysis (to encapsulate functionality like isSupportedOpcode(), findAndSetMainInstruction(), etc.) and BoUpSLPLegality (to host logic such as isLegalBroadcastLoad(), arePointersCompatible(), analyzeRtStridedRecurrence(), and related functionality). This phase would begin only after the completion of Phase 0.
Phase 2: Cost Model: Introduce SLPCostAnalysis.h/.cpp.
Given the degree of interdependence in the existing code, this phase would initially focus on establishing the overall structure and extracting a minimal, foundational set of functions. This phase would begin only after the preceding phase is completed.
This RFC is intended to solicit early feedback, perspectives, and concerns before pull requests are opened for formal review.
I’m not familiar enough with the implementation to have opinions on the details of the refactoring, but this would be nice to see from a LLVM build time perspective.
A lot of us build LLVM on reasonably wide machines where critical path length in the build can become reasonably important, and SLPVectorizer is now sometimes one of the most expensive files to compile (now usually more than X86ISelLowering.cpp, which has been proposed to be split in the past for this reason). Splitting it up would likely help massively in this area. It’s not exactly the biggest deal in the world and we should prioritize maintainability over build times, but it would be a nice added benefit of such a change.
I would very much be interested in the future maintainability of the current SLPVectorizer. The refactoring you’re proposing seems to me a necessary first step in that direction.
I’m concerned that SLP re-invents a lot of code, analysis and data structures that we already have in other parts of the codebase, often just with a subtle,niche difference - just splitting SLPVectorizer.cpp won’t address that and perhaps makes it even trickier to conform. But I’m all for any refactor that will allow us to replace SLP-specific code with generic llvm equivalents.
Yes, this is the first step towards better maintainability and readability of the code. I do note that there may be some re-inventing in the SLP code and at this time, I am not in a position to comment if LLVM utils will be a drop-in replacement for them. We need to wait for the splitting to conclude and revisit this later.
I will start pushing patches starting next week as per the mentioned phases.