[PATCH] D69486: PowerPC: Fix SPE f64 VAARG handling.

[Justin Hibbits via Phabricator via llvm-commits]

jhibbits marked an inline comment as done.
jhibbits added a comment.

Hi Stefan, thanks for the review.  You're right this and the other two reviews (D69483 <https://reviews.llvm.org/D69483>, D69484 <https://reviews.llvm.org/D69484>) need tests.  @kthomsen created these, I'm trying to marshall them in.  He has some tests, but not in a state that's committable yet, and is working on that/looking for help to create reduced cases.



================
Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:3099
   // select overflow_area if index > 8
   SDValue CC = DAG.getSetCC(dl, MVT::i32, VT.isInteger() ? GprIndex : FprIndex,
                             DAG.getConstant(8, dl, MVT::i32), ISD::SETLT);
----------------
stefanp wrote:
> You are computing a new `GprIndex` above for `MVT::f64` on SPE.
> However, down here you don't use the newly computed `GprIndex` because `VT.isInteger()` is going to return false so you are going to get `FprIndex` instead. 
> The same thing happens later on in this function where we use `VT.isInteger() ? GprIndex : FprIndex` again. 
> It looks like the newly computed `GprIndex` is not going to be used since you are always going to be getting `FprIndex` when you have `MVT::f64`. 
Oops, you're right, I need to update RegConstant as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69486