[PATCH] D21534: GlobalISel: first outline of legalization interface.

[Quentin Colombet via llvm-commits]

Hi Matthias,

> On Jun 20, 2016, at 2:59 PM, Matthias Braun <matze at braunis.de> wrote:
> 
> MatzeB added a subscriber: MatzeB.
> MatzeB added a comment.
> 
>> ISelDAG seems to have overloaded "Expand" to mean quite a few different possible legalization strategies, which I'd like to avoid for GlobalISel. So at the moment, the possible generic options are:
> 
>> 
> 
>> Legal (nothing to do)
> 
>> NarrowScalar (e.g. i64 -> i32 twice, <4 x i64> -> <4 x i32>).
> 
>> WidenScalar (e.g. i8 -> i32, <8 x i1> -> <8 x i8>).
> 
>> FewerElements (e.g. <4 x i64> -> <2 x i64> twice).
> 
>> MoreElements (e.g. <2 x i8> -> <8 x i8>).
> 
>> Libcall (self-explanatory).
> 
>> Custom (as with ISelDAG, target will handle it).
> 
> 
> I always found the SelectionDAG interfaces that basically force you to create a table indexed by type and operation and only give you N pre-selected choices for the table entries. The amount of code we have in our  *ISelLowering.cpp files in the custom lowering parts may be an indication that it is...
> 
> So how about a radical change here and simply let the targets handle this all in code. This doesn't need to become too tedious in the targets when we provide the right set of convenience functions in generic codegen code. Some pseudocode of what I imagine:
> 
> In generic CodeGen/GlobalISel code:
> 
>  /* LegalizeHelper provides a set of "standard" legalizations in the form of different methods: */
>  void LegalizeHelper::narrowScalar(MI), LegalizeHelper::libcall(MI) ...
>  /* LegalizeHelper provides a set of standard query function to categorize operations into different "classes" that are often handled uniformly: */
>  boolean isIntArithmetic() { return isScalarIntType() && (is_Add || is_Sub || isMul) }, similar things like isVectorArithmetic(), isLoadStore(), ...
> 
> In target specific code:
> 
>  /* No need to inialize a big table with legalization actions in the constructor, just do the case-by-case decisions inside legalize(). The custom legalization code also becomes part of legalize(): */
>  MyTargetLegalizer::legalize(MI) {
>    if (isIntArithmetic(MI)) {
>      if (MVTSize < 32)
>        widenScalar(MI);
>      else if (MVTSize > 32)
>        splitScalar(MI, 32);
>    }
>    // ...
>  }

What prevents us to do the same things with the set actions?

On my side, I’d like we keep the set actions thing. All that stuff should be tablegened, or even better deduced from the isel patterns, at some point. If people need to actually write code on top of the setAction thing, we need to figure out why and improve that work flow.

Cheers,
-Quentin
> 
> While we possible waste some extra cycles in this scheme because of extra if() checks compared to a table lookup. However this scheme:
> 
> - Frees us from arguing semantics of the legalization actions enum; Just see what the function call is doing, if it doesn't suit you, add a new function or add a parameter to the legalization function for variations...
> - No artificial split between custom and "normal" lowering anymore.
> - Adding, renaming, adding parameters to the shared legalization functions keeps us more flexible in the future.
> 
> 
> Repository:
>  rL LLVM
> 
> http://reviews.llvm.org/D21534
> 
> 
>