[PATCH] D55686: [PowerPC] Fix assert from machine verify pass that unmatched register class about fcmp selection in fast-isel

[Zixuan Wu via Phabricator via llvm-commits]

wuzish marked 2 inline comments as done.
wuzish added inline comments.


================
Comment at: llvm/lib/Target/PowerPC/PPCFastISel.cpp:900
       } else
-        CmpOpc = PPC::FCMPUD;
+        CmpOpc = PPCSubTarget->hasVSX() ? PPC::XSCMPUDP : PPC::FCMPUD;
       break;
----------------
nemanjai wrote:
> hfinkel wrote:
> > This would be the only place I see here where we're checking hasVSX(). Should we call getRegForValue earlier and then use isVSFRCRegClass? Do we need to do the same thing about with isVSSRCRegClass and PPC::FCMPUS?
> > 
> I would actually suggest we go one step further and leave the switch the way it is but change the opcode after we create the virtual registers based on whether the `Src...` VRegs are `VSSRC/VSFRC`. This would also allow us to add an assert if we ever hit a situation where the register classes don't match (i.e. one is `VSFRC` and the other `F8RC`, etc.).
I find it's relative easy and OK to move the getRegForValue earlier. It's hard to make an assert of "one is VSFRC and the other F8RC" since the src2 is an imm result materialized by a `load` which is a normal f8rc load instead of vsx load instruction, which I think is a limitation now since we don't handle load selection well.


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

https://reviews.llvm.org/D55686