[PATCH] [DAGCombine] Don't generate atrocious shuffles for constant vectors

[Andrea Di Biagio]

In http://reviews.llvm.org/D7093#111473, @mkuper wrote:

> Andrea, I'm not sure I understood. 
>  Do you prefer the logic to live only in getVectorShuffle() or be replicated both here and there? I'm not sure the first is sufficient.


I think you would need to fix both places.
When a new shuffle node is created, method 'getVectorShuffle' ensures that the resulting node is always in a canonical form. The reason the same canonicalization rules in 'getVectorShuffle' are performed by 'visitVECTOR_SHUFFLE' is because we want to preserve the canonical form of shuffles when optimizing the dag. For example, without those rules, we would break the canonical form of a shuffle if Undef gets propagated to its first operand. So, my opinion is that you should replicate that rule in both places. It's not ideal, I agree. Ideally if possible, those rules should be factored out into a common function at some point  but with the current implementation we need to update both.

> In any case, I'm not sure that would solve the issue Simon reported. That case also involves a bitcast that changes the number of vector elements, and  don't know if that's currently handled. I'll check.


Right. getVectorShuffle would look through any bitcast, however it would not try to simplify the shuffle if the bitcast changes the number of elements. So it would not fix the problem with that test.


http://reviews.llvm.org/D7093

EMAIL PREFERENCES
  http://reviews.llvm.org/settings/panel/emailpreferences/