[PATCH] SelectionDAG: Share ISD::MUL lowering code between the SelectionDAGLegalize and DAGTypeLegalizer

[Tom Stellard]

On Fri, Feb 07, 2014 at 02:54:39PM -0800, Owen Anderson wrote:
> 
> On Feb 7, 2014, at 2:20 PM, Hal Finkel <hfinkel at anl.gov> wrote:
> 
> > ----- Original Message -----
> >> From: "Tom Stellard" <tom at stellard.net>
> >> To: llvm-commits at cs.uiuc.edu
> >> Sent: Friday, February 7, 2014 4:12:13 PM
> >> Subject: [PATCH] SelectionDAG: Share ISD::MUL lowering code between the	SelectionDAGLegalize and  DAGTypeLegalizer
> >> 
> >> Hi,
> >> 
> >> The attached patches allow the SelectionDAGLegalizer to reuse ISD:MUL
> >> lowering code from the DAGTypeLegalizer.
> >> 
> >> The first patch moves the DAGTypeLegalizer ISD:MUL lowering code into
> >> a public
> >> SelectionDAG function called expandMUL() and the second patch
> >> modifies
> >> the SelectionDAGLegalizer to use this code.
> >> 
> >> I think there may be other opportunities to share code between these
> >> two classes,
> >> so I'm wondering if the approach I've taken would work for other
> >> operations too.
> >> I'm interested in hearing what other people think.
> > 
> > I've not yet looked at the patch, but I'll add: There is also expansion logic hidden away in the legalization code that could be used by *ISelLowering in the various targets, and I think that moving things into utility functions in SelectionDAG will help with reuse there as well (GatherAllAliases comes to mind, for example). So, generally speaking, I think this kind of refactoring is a good idea.
> 
> I agree with Hal, and the general sentiment that it would be great to share more of these expansions between type and operation legalization.  That said, I’d like to find a cleaner way to factor them out than just moving them all into methods on SelectionDAG.
>

Do you have any suggestions for where these functions should go?  Would it make
sense to put them in a common parent class of SelectionDAGLegalize and
DAGTypeLegalizer?

-Tom