[PATCH] D21802: Ensure all uses of permute instruction feed vector stores

[Hal Finkel via llvm-commits]

hfinkel added a comment.

In http://reviews.llvm.org/D21802#470162, @kbarton wrote:

> In http://reviews.llvm.org/D21802#469731, @nemanjai wrote:
>
> > I'm just curious how this happens since the swap instruction that defines the register for the store is emitted together with the actual store. How is there another user for a node that the back end added as part of DAG combine? I assume this is some sort of CSE that is running between the combine step and the swap removal step? If it is, might it also be advantageous to teach it not to eliminate expressions when one of them is a swap feeding a store?
> >
> > I'm not suggesting this is necessarily the right thing to do, but one thing that will prevent `MachineCSE` from eliminating swaps is by adding `let hasSideEffects = 1` in the definition of `XXPERMDI`. That fixes this issue, but has the disadvantage of disabling CSE for other forms of the instruction.
> >
> > In any case, disabling `MachineCSE` for swaps might be something we want to consider.
>
>
> I thought about disabling MachineCSE in this case, but after talking to Bill I decided it wasn't the right approach. If we do disable it, we would need to do it through the cost model. There isn't a flag that we can use to make MachineCSE illegal on swap instructions that wouldn't have broader impacts throughout the optimizer. Marking XXPERMDI as having side effects, for example, can limit many other things that move code around, which we don't want.
>
> There is also the problem that XXPERMDI doesn't really have side effects, so marking it to disable MachineCSE would likely draw some "attention" during reviews ;)


Indeed it would. ;) -- There are certainly cases where we don't want to CSE certain kinds of instructions (generally because of macro fusion and other microarchitectural pattern matching, to manage register pressure, etc.), but in those cases, you'd want to remateralize later instead of preventing CSE earlier (because CSEing the swap might enable CSEing other things).


http://reviews.llvm.org/D21802