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

[Jinsong Ji via Phabricator via llvm-commits]

jsji added inline comments.


================
Comment at: llvm/lib/Target/PowerPC/PPCISelDAGToDAG.cpp:3886
+  default:
+    llvm_unreachable("Undefined SPE Predicate for CC\n");
+    break;
----------------
jhibbits wrote:
> jsji wrote:
> > jhibbits wrote:
> > > jsji wrote:
> > > > If we don't merge this into `getPredicateForSetCC`, maybe it would be better to list those should have been `legalized` ones like `getPredicateForSetCC`.
> > > Since this is called in more places than getPredicateForSetCC, this comment makes more sense to implement.
> > ? Why more places than `getPredicateForSetCC`? I think we call it after each  `getPredicateForSetCC`
> You're right.  I didn't check the full path below before the getPredicateForSetCCForSPE().  However, getPredicateForSetCCForSPE() is *only* to be used for floating point checks.  getPredicateForSetCC() doesn't take a type.  I guess I could add a type argument for getPredicateForCC().
Yes, we need to add arguments for `getPredicateForCC` in order to consider SPE.


================
Comment at: llvm/test/CodeGen/PowerPC/spe.ll:1
 ; RUN: llc -verify-machineinstrs < %s -mtriple=powerpc-unknown-linux-gnu \
 ; RUN:          -mattr=+spe |  FileCheck %s
----------------
jhibbits wrote:
> jsji wrote:
> > jhibbits wrote:
> > > jsji wrote:
> > > > jsji wrote:
> > > > > Maybe we should include one RUN line for `-O0` to test FastIsel as well?
> > > > Why not generate the checks using script?
> > > Is there an example of generating checks via a script?  These checks were generated from a C file provided by @kthomsen a while back, with -emit-llvm, and pared down to the smallest working set I could.
> > Just run `llvm/utils/update_llc_test_checks.py --llc-binary ../build/bin/llc llvm/test/CodeGen/PowerPC/spe.ll` .
> > The script should generate CHECKs automatically.
> > We can then examine each of them to see whether they are desired.
> Thanks.  I was unaware of that script, and should probably poke around more to see what else is there that I can make use of in the future.
Great! Let me know if you meet problems in the script, we should fix it if there is bug.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69483