[PATCH] D86203: [GlobalISel][TableGen] Add handling of unannotated dst pattern ops

[Bjorn Pettersson via Phabricator via llvm-commits]

bjope added a comment.

In D86203#2453302 <https://reviews.llvm.org/D86203#2453302>, @arsenm wrote:

> In D86203#2445098 <https://reviews.llvm.org/D86203#2445098>, @bjope wrote:
>
>> But if I understand this patch correctly it has tried to derive the type from "some_isnt" when it is omitted, rather than reusing the type from the source pattern. Would it make more sense to infer the type from the source pattern instead of from the target instruction when it is omitted? Is that possible?
>
> The problem isn't the type, it's the register class. It can be ambiguous to go from the target instruction back to the source bank if the target register class supports multiple banks

I assume tblgen will complain if it find something ambigous?

However, this pattern seem to work with both SelectionDAGISel and GISel

  def : Pat<(add i32:$src1, i32:$src2),
            (some_inst i32:$src1, i32:$src2)>;

and there aren't even any register class annotations (that is why I don't quite get how the register class is the problem).
Also, I could not find any differences in the GenGlobalISel.inc file is using a registerclass instead of "i32" (e.g. in the output part of the pattern), so it did not seem to matter really (unless some_inst was INSERT_SUBREG or EXTRACT_SUBREG or similar that actually might depend on the register class).

I still haven't figured out the use-case when I want/need to use different annotations for an operand in the input pattern compared to the output pattern (is that perhaps an "implicit" COPY_TO_REGCLASS?). It really feels a bit unexpected/unintuitive that for example $src1 could have different types (or register classes if you prefer) in the input and output pattern fragments (isn't it a weird syntax?).

My idea was simply to support an omitted annotation in the output part of the pattern, by defining it as having the same annotation as in the input pattern if it is omitted. After all, it is allowed to omit the annotation today (but then the GlobalISelEmitter ignores the pattern so it isn't really that useful). I think that either we should require an annotation also in the output pattern, or we should support omitting it in some useful way.

Defining what an omitted annotation means does not prevent anyone from adding annotations when they are needed (to resolve ambiguity etc).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D86203