[PATCH] D83710: TableGen/GlobalISel: Allow output instructions with multiple defs

[Matt Arsenault via Phabricator via llvm-commits]

arsenm marked 4 inline comments as done.
arsenm added inline comments.


================
Comment at: llvm/utils/TableGen/GlobalISelEmitter.cpp:2655-2661
+    if (IsDef) {
+      if (IsDead)
+        Table << MatchTable::NamedValue("RegState::Define|RegState::Dead");
+      else
+        Table << MatchTable::NamedValue("RegState::Define");
+    } else
       Table << MatchTable::IntValue(0);
----------------
paquette wrote:
> Are we going to need more variations on RegState in the future?
> 
> If so, maybe it'd be better to do something like the pseudocode below?
> 
> ```
> RegState = nothing
> if (IsDef) {
>    RegState = "RegState::Define";
>    if (IsDead)
>       RegState += "|RegState::Dead";
> }
> 
> if (RegState != nothing)
>   Table << MatchTable::NamedValue(RegState);
> else
>    Table << MatchTable::IntValue(0);
> ```
I don't think so. I think implicit is the only other one that might make sense, but I don't think there's any contexts where the selector would need implicit virtual registers


================
Comment at: llvm/utils/TableGen/GlobalISelEmitter.cpp:4850
 
-  if (DstI.Operands.NumDefs != Src->getExtTypes().size())
-    return failedImport("Src pattern results and dst MI defs are different (" +
+  if (DstI.Operands.NumDefs < Src->getExtTypes().size())
+    return failedImport("Src pattern result has more defs than dst MI (" +
----------------
madhur13490 wrote:
> I was thinking about this for the other change. If DstI's NumDefs are less than what source pattern is defining then we can ignore the defs from source pattern.  So, this error message shouldn't be there and may be we can emit a warning or something. OTOH, if it's opposite, i.e. source  pattern is defining a fewer values then dstI's some value will be undefined. In any case, there need not be an error because both of these are valid some or the other backend. I am not sure what we should be doing ideally from matching perspective but both cases can occur.
I think a warning would be counterproductive. TableGen doesn't really emit warnings. We could maybe invent some new construct to indicate explicitly discarded results, but I'm not sure what it would look like


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

https://reviews.llvm.org/D83710