[PATCH] D55182: InstCombine: Scalarize single use icmp/fcmp

Tue Dec 4 11:07:45 PST 2018

arsenm added a comment.

In D55182#1316746 <https://reviews.llvm.org/D55182#1316746>, @spatel wrote:

> I think this is the right change for IR (I'm trying to do something similar with D50992 <https://reviews.llvm.org/D50992> but that's been held up while we try to get the backend in shape). .
>
> 1. I see that the new code is copying the existing handling of binops around line 315, but can we update this to use 'match()' and reduce the duplication?
> 2. There should be a test that shows the benefit of using 'isCheapToScalarize()'. IIUC, that would involve a sequence with multiple binops and/or compares between an insertelement and the trailing extract.
> 3. Please commit the tests with baseline checks as a preliminary commit, so we only show the diffs in this patch.

I don't think there's much of a useful way to combine this with the BinOp handling. At most it's going to share the pair of extracts, and would require more ugly code when creating the new operation so I think it would be overall worse

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D55182/new/

https://reviews.llvm.org/D55182