[PATCH] D72961: In early-ifconversion check that the operands of a PHI share a common regclass with the destination regclass.

[Amara Emerson via Phabricator via llvm-commits]

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

In D72961#1827624 <https://reviews.llvm.org/D72961#1827624>, @efriedma wrote:

> What is the restriction on the register classes of PHI operands, if they don't have to have to be same register class as the result?  I don't see any relevant documentation, or checks in MachineVerifier::checkPHIOps().


My initial reaction was that the PHI was invalid, but according to @qcolombet it's ok to have different register banks on the operands. The problem here is that ifcvt tries to generate a target instruction, AArch64::CSELXr in place of the PHI with incompatible regclasses. If we leave it to PHI elimination it works fine.



================
Comment at: llvm/lib/CodeGen/EarlyIfConversion.cpp:522
+    // We need to make sure the register classes have a common subclass
+    // to avoid cross-class copies.
+    const TargetRegisterClass *DstRC =
----------------
efriedma wrote:
> Not sure why we want to avoid a cross-class copy here; won't we generate a cross-class copy anyway in PHIElimination?
The comment isn't correct, it's from an earlier version of the fix. What we actually want to avoid is a set of disjoint regclasses for the operands and destination register.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72961