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

[Nemanja Ivanovic via Phabricator via llvm-commits]

nemanjai added a comment.

I assume that we are fixing this specific instance because we have a test case that breaks if we turn on verification by default. However, I would assume that we actually have a lot of other failures hiding in here which would be uncovered by doing a bootstrap build with `-O0`. I am not suggesting that we put all the fixes in a single patch, just that we want to do more thorough testing before we turn on `MachineFunction` verification by default.



================
Comment at: llvm/lib/Target/PowerPC/PPCFastISel.cpp:900
       } else
-        CmpOpc = PPC::FCMPUD;
+        CmpOpc = PPCSubTarget->hasVSX() ? PPC::XSCMPUDP : PPC::FCMPUD;
       break;
----------------
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.).


Repository:
  rL LLVM

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

https://reviews.llvm.org/D55686