[PATCH] D15616: [EarlyCSE] Simplify target intrinsic support

[Hal Finkel via llvm-commits]

hfinkel added a comment.

>   Finally, why did we decide to put these in EarlyCSE instead of GVN?


The eventual goal is still consistency between EarlyCSE and GVN here. Now that MemorySSA is arriving and our GVN will be rewritten, we'll have an opportunity to get convergence.

> Additionally, if we need them, why can't they be expressed as struct stores since that appears to what they represent?


In general, I don't think that stores of structures is a good model here. They're swizzled stores (some permutation, xoring, etc. of the data is applied).

> This change unwinds some complexity from the target intrinsic support. Specifically, it removes the idea of an 'ExpectedType'. While we do need to check the type of the returned value (or insert a bitcast), the amount of code involved in the original implementation was a bit excessive. Note that this version may end up doing slightly more work for target intrinsics (due to the lack of an early bail), but tuning for this rare case at the cost of extra complexity seems questionable.


The code being removed here does not seem excessive ;) -- I don't see the benefit of removing the 'ExpectedType' parameter. It will cause us to speculatively create new IR instructions. This is not really a matter of tuning, consensus seems to be that we avoid speculatively creating IR instructions whenever possible. Am I missing something?


http://reviews.llvm.org/D15616