[PATCH] D42793: [MergeICmps] Enable the MergeICmps Pass by default.
Sanjay Patel via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Fri Feb 2 06:46:56 PST 2018
spatel added reviewers: hfinkel, nemanjai, timshen.
spatel added a comment.
In https://reviews.llvm.org/D42793#996098, @courbet wrote:
> In https://reviews.llvm.org/D42793#995427, @spatel wrote:
>
> > Can you create at least one llc test each for x86 and PPC, so we see the result of merging + expansion together? This could just be copying existing tests from test/Transforms/MergeICmps?
>
>
> Done. Can you double check the PPC test ? It does make sense to me, but I'm not an expert.
Thanks. This test looks fine to me as PPC given the 'align 4' on the loads. I recommend checking these tests in now, so we just see the improvements from enabling MergeICmps when this patch is committed.
For reference, the current PPC codegen is something like this:
# %bb.0: # %entry
lwz 5, 0(3)
lwz 6, 0(4)
cmplw 5, 6
bne 0, .LBB0_2
# %bb.1: # %land.rhs.i
lwz 3, 4(3)
lwz 4, 4(4)
cmpw 3, 4
b .LBB0_3
.LBB0_2:
crxor 2, 2, 2
.LBB0_3: # %opeq1.exit
li 3, 0
li 4, 1
isel 3, 4, 3, 2
blr
My concern is that I don't see consideration for misalignment when combining to larger ops. We generally don't care on x86, but it could be a problem for some PPC? This may be a non-issue if some other pass is expected to split misaligned memops later.
Repository:
rL LLVM
https://reviews.llvm.org/D42793
More information about the llvm-commits
mailing list