[PATCH] D26277: [SLP] Fixed cost model for horizontal reduction.

[Michael Kuperstein via llvm-commits]

mkuper added a comment.

The point I'm trying to make is that there's still no adequate test coverage for this patch.

Could you please split this into two patches:

1. A patch that contains line 4286, and a test where the behavior actually changes due to that line.

2. A patch that contains getReductionCost() and a test where the behavior changes due to those changes. I assume (a) the change in reduction.ll you have in this patch is due to this part, and (b) 11 is a better cost here than 17, but it seems like <8 x i32> should be legal on AVX2, and illegal on SSE/AVX, so I wonder why we get the same cost for both case.

Would a smaller fix achieve the same result? If so, can you add a test that demonstrates where the logic you've added for illegal types matters.


https://reviews.llvm.org/D26277