[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