[PATCH] D42565: [ARM][GISel] PR35965 Constrain RegClasses of nested instructions built from Dst Pattern

[Quentin Colombet via Phabricator via llvm-commits]

qcolombet added inline comments.


================
Comment at: lib/CodeGen/GlobalISel/Utils.cpp:56-59
+  // Some of the instructions, like COPY, may not impose any register class
+  // constraints on some of their operands:
+  if (!RegClass)
+    return Reg;
----------------
rtereshin wrote:
> dsanders wrote:
> > This is a bit more general than I'd like. I agree that TargetOpcodes like COPY shouldn't fail, but I think GenericOpcodes and target specific instructions should. Could you `assert(!isPreISelGenericOpcode() && !isTargetSpecificOpcode())` inside this if-statement?
> Sure! That's just what I needed to know: how to describe in a generic way all the instructions like `COPY` here, huge thanks!
I believe this part should go in separately.
This is not related to the TableGen change.


================
Comment at: lib/CodeGen/GlobalISel/Utils.cpp:62
+           "with no register class constraints");
+    return Reg;
+  }
----------------
I believe bailing out won't be enough generally speaking, unless we expect the users of this method to do the right thing for PHIs and COPY.

E.g., by bailing out on COPYs we could end up with unconstrained VRegs in cases like this:
v1 = COPY // v1 won't get constrained
v2 = COPY v1 // <-- v2 will be constrained by its users

Usually that means we have useless COPYs though.


================
Comment at: test/CodeGen/ARM/GlobalISel/arm-select-fptosi-pr35965.mir:2
+# NOTE: Assertions have been autogenerated by utils/update_mir_test_checks.py
+# RUN: llc -mtriple armv7-gnueabihf -run-pass instruction-select -verify-machineinstrs -o - %s | FileCheck %s
+---
----------------
Could you use something more descriptive than prxxx in the file name or just drop it?

Reference the PR in the comment here though.


https://reviews.llvm.org/D42565