[PATCH] D157273: [GlobalISel] Add dead flags to implicit defs in ISel

[Pierre van Houtryve via Phabricator via llvm-commits]

Pierre-vh added a comment.

In D157273#4567283 <https://reviews.llvm.org/D157273#4567283>, @arsenm wrote:

> In D157273#4565233 <https://reviews.llvm.org/D157273#4565233>, @Pierre-vh wrote:
>
>> A couple of things to note:
>>
>> - This is a bit naive, it only works on the `Record*` and doesn't test for subregs or RC, should it?
>
> Probably can ignore this
>
>> - One of the tradeoffs of doing this at the TableGen level is that we don't know about reserved registers. Do they matter in this case? How could we handle them?
>
> It doesn't matter, dead flags shouldn't be set on reserved registers but also shouldn't matter

I guess if we want to be 100% correct we could check for the presence of `RegState::Dead` in the flags, and check if we're dealing with a reserved register.
If it doesn't matter though let's leave it at that, it can always be added later.



================
Comment at: llvm/include/llvm/CodeGen/GlobalISel/GIMatchTableExecutor.h:335
   /// - RegNum - The register to add
+  /// - Dead - Whether this is a dead def.
   GIR_AddImplicitDef,
----------------
arsenm wrote:
> arsenm wrote:
> > Can just make the flag value?
> actually you can compact the two into one field, register is 32-bits and the flags are 16 I think
I think compacting it is more trouble than it's worth, and it'd make this opcode an outlier - e.g. `AddTempRegister` doesn't compact, `AddTempSubRegister` doesn't, etc.

We should just look into making this a variable-length encoded array at some point, but it's quite a bit of work. Probably not worth it until we get actual performance numbers.

I wonder if we ever make use of the upper 32 bits though, maybe the table should always be 32 bits, with an option to variable-length-encode bigger values.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157273