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

[Roman Tereshin via Phabricator via llvm-commits]

rtereshin 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:
> qcolombet wrote:
> > 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.
> It is related in a sense that the TableGen change would break GlobalISel for some of the targets w/o this change.
Looks like if we put it in a separate patch it will go w/o a test case, as to catch the problem we need to call `constrainSelectedInstRegOperands` on a copy, and that never currently happens as:
1. `selectImpl` currently doesn't support `COPY` at all, and every target handles them before calling `selectImpl`
2. If `COPY` is introduced by a destination pattern nothing constrains it as we weren't constraining nested instructions before the tablegen part of this patch.
3. (2) never caused a problem as existing targets (apparently) don't have patterns like this.

So I don't quite see an a reasonably simple way to come up with a test for this that actually fails before the fix w/o introducing a fake testing target.

In other words, this part of this patch can't have a test case w/o the tablegen part, but the tablegen part can't go in w/o this as it will cause existing tests to fail.


https://reviews.llvm.org/D42565