[PATCH] D22038: [X86] Transform zext+seteq+cmp into shr+lzcnt on btver2 architecture.

[pierre gousseau via llvm-commits]

pgousseau added a comment.

In http://reviews.llvm.org/D22038#476724, @spatel wrote:

> In http://reviews.llvm.org/D22038#476520, @pgousseau wrote:
>
> > In http://reviews.llvm.org/D22038#475372, @spatel wrote:
> >
> > > Did you look at doing this in DAGCombine with a TLI hook? PPC already has this or something very close in PPCTargetLowering::LowerSETCC(), so it seems like the code could simply be lifted up from there?
> > >
> > > Would x86 targets other than btver2 want to do this transform when optimizing for size?
> >
> >
> > I did not know there was a similar change in PPCTargetLowering, thanks for pointing it out, I will investigate this.
> >  For the TLI hook I could add a method 'bool isCTLZFast()', which on X86, I suppose would be something like:
> >  bool isCTLZFast() { return (hasLZCNT() && getCPU() == "btver2");}
> >  It looks a bit less maintainable than the Target feature way though, what do you think?
>
>
> You should keep the HasFastLZCNT attribute that you already have proposed, and the hook will trigger off of that. We don't want to base transforms on CPU models; that doesn't evolve well.


Makes sense yes thanks. This should be done in 3 patches I think.
The first patch will be NFC, adding the "isCTLZFast" TLI hook, enabling it for PPC and moving the PPC specific code to DAGCombiner.
The second patch will enable "isCTLZFast" for X86.
The third patch will add the transformation for the OR case.
I think this is what you and Simon are suggesting?
Will experiment a bit with it and abandon this review once the first patch is ready.


http://reviews.llvm.org/D22038