[PATCH] D137653: [TableGen] More named sub-operands work.

[Gaëtan Bossu via Phabricator via llvm-commits]

gbossu added inline comments.


================
Comment at: llvm/lib/Target/ARM/Disassembler/ARMDisassembler.cpp:6557
+  // the decoder doesn't actually call it yet. That will be addressed in a
+  // future change.)
+  return MCDisassembler::Success;
----------------
jyknight wrote:
> gbossu wrote:
> > @jyknight I actually had a local patch which I planned to upstream in the next days, but that was made before your changes. That patch introduces an additional `bit DecodeZeroBitOperand = false;` flag in `DAGOperand` to allow targets to still get decoding callbacks for operands, even if they are encoded with zero bits. It came with a few changes for targets which have these "zero-bit" operands, like AVR, AArch64 or RiscV if I remember correctly.
> > 
> > It kept the default behavior of ignoring "zero-bit" operands when `DecodeZeroBitOperand` is false though. IIRC AMDGPU relies on this mechanism to purposely skip operands. Should I push this patch forward, or will this be superseded by some of your upcoming work by essentially always generating disassembler callbacks?
> Indeed I do have a work in progress change to fix this which I started back in November, but I haven't gotten back to yet. (I decided to complete the useDeprecatedPositionallyEncodedOperands cleanup first instead.)
> 
> I found that many targets do indeed currently "depend" on this behavior of ignoring no-bits operands -- but by-and-large only because they HAD to hardcode workarounds for the autogenerated decoder forgetting about them, in order to get correct behavior.
> 
> As with the other changes I've been making in this area, I intend to introduce the new behavior and control it with a Target option (e.g. useDeprecatedDecoderOperandSkipping), migrate all in-tree targets, and then delete the option.
> 
Thanks for the answer, I believe introducing `useDeprecatedDecoderOperandSkipping` is a better solution, as it forces targets in the right direction. Unless you don't intend to continue with your work in progress, I guess it doesn't make sense for me to push my patch forward? Please let me know if I can somehow assist though, as I attempted a very similar change.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D137653