[PATCH] D20019: [PPC] exploitation of new xscmp*, as well as xsmaxcdp and xsmincdp

[Ehsan Amiri via llvm-commits]

amehsan added a comment.

Thanks CY for the comments on the test cases. I will review to make sure all right combinations are there and function names are consistent. The fast flag on fcmp IR instruction does not matter. We don't pay any attention to it when we construct Selection DAG. It is in my IR because they come from an example that was compiled with -ffast-math.

One of the outstanding issues in SelectionDAGs is that many ISD opcodes are not equipped with fast math flags. For a limited number of opcodes this issue has been fixed, but the work should be extended to all opcodes. While that is the better way of checking for fast math flags, it is a different issue than the what the current patch tries to achieve.


================
Comment at: lib/Target/PowerPC/PPCISelDAGToDAG.cpp:2834
@@ +2833,3 @@
+      SelectCCSummary Summary;
+      if (useP9VSXScalarComparisonInstr(N, Summary)) {
+        if (Summary.Leave) break; // Will be handled in tablegen.
----------------
cycheng wrote:
> I feel it is a little bit strange, if useP9VSXScalarComparisonInstr returns true, it should mean we can use p9vsx instructions for N, but actually even if the function returns true, we still have some cases that can't use p9vsx instructions.
> 
> Would it be better if:
> 
> ```
> if (useP9VSXScalarComparisonInstr(N, Summary)) {
>   if (Summary.CC == ISD::CondCode::SETNE) { return ..; }
>   if (Summary.Comp0 == Summary.Ret0 && ..) { return ..; }
>   if (Summary.Comp0 == Summary.Ret1 && ..) { return ..; }
>   if (CurDAG->getTarget().Options.NoNaNsFPMath) { return ..; }
>   llvm_unreachable(..);
> }
> ```
> 
> So useP9VSXScalarComparisonInstr might need additional arguments to help it judge if N is able to use p9vsx instructions.
> 
Good point. I will rename the function. It makes more sense to check for non-nan outside the function. So even when the function return true, there is a possibility that we do not want to use the new instructions.


http://reviews.llvm.org/D20019