[PATCH] Add a Scalarize pass

[Richard Sandiford]

Michael Ilseman <milseman at apple.com> writes:
> This comment pertains purely to implementation details of the pass.
>
> I notice that you seem to lazily expand values (which may exist very far
> away from the current visit point) and just iterate over the blocks in
> any given order. Would you perhaps be better off if you could instead
> visit (almost) all the defs before the uses (caveat: backedges)? You
> should be able to use the ReversePostOrderTraversal class to visit
> (non-backedge) defs before their uses. For the backedge case, you could
> just defer those Phis until the end. Then you don't ever need any lazy
> expansion, and can just map vector values to scalar values.

The reason for using lazy creation was just to cut down on the amount
of garbage IR, rather than because of traversal order.  I initially did
it eagerly but wasn't happy with the number of dead extractelement
instructions it created.  Obviously they'd be cleaned up by DCE, but it
seemed better not to create them unless we knew they would be used.

E.g. if an argument is only used by a function call (which we don't
scalarise), the eager form would generate a sequence of extractelements
for the argument anyway, as soon as it sees the argument.  Same for
function call results if the result is passed to another function,
or used as the operand to a store (which we don't scalarise by default).

It also felt more natural to handle insertelement like this rather than
create a scattered form for every insertelement instruction.

I agree that using a reverse post-order traversal would cut down
on the number of cases in which we create a sequence of extractelements
for an instruction only for us later to scalarise the instruction itself.
I'm just not sure whether the overhead of creating the traversal
order would outweight the benefits there or not.  One thing in favour
of the current approach is that it should be cheaper for functions that
don't have vector operations.

> I notice inside Scatterer's operator [], you might return null at the
> bottom of the function. Is this safe? It seems like users pass on the
> result directly to the builder, which sounds dangerous.

No, it never returns null.  If the operand[] optimisations fail,
the fallback is always to create an extractelement (for vectors)
or a getelementptr (for pointers).

> There's no test in this patch with dynamic InsertValues, so I'm not
> sure if it's a case you're planning on handling or not. Either way, a
> comment/assert would be nice :)

Yeah, good catch, thanks.  There should definitely be a test for that.
I've just uploaded an updated patch with this as f12 in basic.ll.

Thanks,
Richard