[PATCH] D33572: [PPC CodeGen] Expand the bitreverse.i32 intrinsic.

Hal Finkel via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Jun 29 07:48:49 PDT 2017


hfinkel accepted this revision.
hfinkel added a comment.
This revision is now accepted and ready to land.

In https://reviews.llvm.org/D33572#794920, @nemanjai wrote:

> In https://reviews.llvm.org/D33572#794182, @hfinkel wrote:
>
> > In https://reviews.llvm.org/D33572#794146, @jtony wrote:
> >
> > > Address comments from Hal Finkel and add one more IR test case to test the original situation in Bugzilla (the IR is equivalent form of fast bit-reverse but NOT the intrinsic).
> >
> >
> > So, we don't currently recognize the 32-bit version as a bit permutation in 64-bit mode? Otherwise, we'd end up in the same situation as the PR and the test would fail right now, right?
>
>
> I think that the point that needs to be mentioned in this patch is that idiom recognition will fire now that we've legalized `ISD::BITREVERSE` and we will not consider this for the bit permutation handling. Namely, what we'll have in the SDAG is just the `ISD::BITREVERSE` node.


Ah, okay. We're doing idiom recognition in the backend (in CGP). That might be suboptimal (now or in the future), but that's a separate matter. This LGTM.


https://reviews.llvm.org/D33572





More information about the llvm-commits mailing list