[PATCH] D69483: [PowerPC]: Fix predicate handling with SPE

[Jinsong Ji via Phabricator via llvm-commits]

jsji added a comment.

Looks like to me that Vector Compare in SPE also has different CR bit semantics . 
So this patch will only handle floating point part for SPE? If so, maybe we should make it explicit in title.



================
Comment at: llvm/lib/Target/PowerPC/PPCISelDAGToDAG.cpp:3866
+// Prior to this, the Compare must have been modified to EF?CMP?? in SelectCC
+static PPC::Predicate getPredicateForSetCCForSPE(ISD::CondCode CC) {
+  PPC::Predicate Opc = PPC::PRED_SPE;
----------------
Can we merge this into `getPredicateForSetCC`? So that we don't need to add code in every callsite of `getPredicateForSetCC`?



================
Comment at: llvm/lib/Target/PowerPC/PPCISelDAGToDAG.cpp:3886
+  default:
+    llvm_unreachable("Undefined SPE Predicate for CC\n");
+    break;
----------------
If we don't merge this into `getPredicateForSetCC`, maybe it would be better to list those should have been `legalized` ones like `getPredicateForSetCC`.


================
Comment at: llvm/lib/Target/PowerPC/PPCISelDAGToDAG.cpp:5089
+    if (PPCSubTarget->hasSPE() &&
+        N->getOperand(2).getValueType().isFloatingPoint()) {
+      // For SPE instructions the result is in GT bit of the CR
----------------
Why we are using `isFloatingPoint()` here, while checking `MVT::f32/MVT::f64` above?


================
Comment at: llvm/test/CodeGen/PowerPC/spe.ll:1
 ; RUN: llc -verify-machineinstrs < %s -mtriple=powerpc-unknown-linux-gnu \
 ; RUN:          -mattr=+spe |  FileCheck %s
----------------
Maybe we should include one RUN line for `-O0` to test FastIsel as well?


================
Comment at: llvm/test/CodeGen/PowerPC/spe.ll:1
 ; RUN: llc -verify-machineinstrs < %s -mtriple=powerpc-unknown-linux-gnu \
 ; RUN:          -mattr=+spe |  FileCheck %s
----------------
jsji wrote:
> Maybe we should include one RUN line for `-O0` to test FastIsel as well?
Why not generate the checks using script?


================
Comment at: llvm/test/CodeGen/PowerPC/spe.ll:110
 ; CHECK-LABEL: test_fcmpugt
 ; CHECK: efscmpgt
 ; CHECK: blr
----------------
Why don't we check bits with `ugt`?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D69483/new/

https://reviews.llvm.org/D69483