[PATCH] D44548: [DAGCombiner] Expand combining of FP logical operations to sign-setting FP operations

[Sanjay Patel via Phabricator via llvm-commits]

spatel added a comment.

In https://reviews.llvm.org/D44548#1259015, @nemanjai wrote:

> In https://reviews.llvm.org/D44548#1258889, @spatel wrote:
>
> > How about enabling the hook for PPC before adding to the generic DAG combine...
> >  Right now, you'll get something horrible for normal fabs (and fneg?) without this patch, right?
> >
> >   	xscvdpspn 0, 1
> >   	xxsldwi 0, 0, 0, 3
> >   	mfvsrwz 3, 0
> >   	rlwinm 3, 3, 0, 1, 31
> >   	mtvsrd 0, 3
> >   	xxsldwi 0, 0, 0, 1
> >   	xscvspdpn 1, 0
> >  
> >
>
>
> Yes. Although that is the worst case (since PPC `f32` isn't 32 bits in a register).
>  No doubt, the PPC-specific portion of this needs to go in and can certainly go in first. I can commit this as the PPC portion first (with the `fnabs` test missing). And then the generic part (adding the `fnabs` test).
>  The question is do I need to split this into two patches and post for review or can I just commit it as two separate patches (or maybe just commit the PPC-portion and post the generic portion for review)?


Please extract and commit the PPC change. That's a pure win in all cases AFAICT.
Then we can come back to the fnabs part here. I don't think there will be any controversy based on the earlier comments, but let me add a couple of non-PPC tests, so we're sure about that.


Repository:
  rL LLVM

https://reviews.llvm.org/D44548