[llvm-dev] SelectionDAGISel::Select's API considered harmful
Justin Bogner via llvm-dev
llvm-dev at lists.llvm.org
Sat May 21 10:57:46 PDT 2016
Update: All in tree backends now implement `void Select`. I'll be
removing the SelectImpl path on Monday.
Justin Bogner <mail at justinbogner.com> writes:
> TLDR: Heads up for out of tree backends - you're going to need to update
> your *DAGToDAGISel::Select method to unconditionally replace nodes
> directly instead of returning the desired replacement.
> So I'm working on fixing the undefined behaviour I described in
> llvm.org/PR26808. As part of this, we need to stop looking into deleted
> SDNodes to check if they were, in fact, deleted. A big place that we do
> this is in SelectionDAGISel::DoInstructionSelection, where we can find
> this helpful comment that came with the commit that added the check for
> SelectionDAGISel.cpp says:
>> // FIXME: This is pretty gross. 'Select' should be changed to not return
>> // anything at all and this code should be nuked with a tactical strike.
> I'm just gonna go ahead and take this advice.
> I'll phase this in a couple of steps:
> 1. Rename Select to SelectImpl in all targets, and implement "virtual
> void Select(SDNode *)" in SelectionDAGISel. I'll move the current
> sketchy behaviour into this version of Select.
> 2. Update backends one at a time to implement "void Select(SDNode *)"
> instead of SelectImpl.
> 3. Make SelectionDAGISel::Select pure virtual and remove SelectImpl
> If you have an out of tree backend and you merge from trunk, I recommend
> updating in between steps 1 and 3 to avoid breakage after 3 happens.
More information about the llvm-dev