[llvm] [TableGen] fix tlbgen for EncodingByHwMode (PR #84906)

[via llvm-commits]

superZWT123 wrote:

> Regarding the `DecoderEmitter` part of the patch: One crucial point not touched on above (but this should be discussed in the follow-up decoder-related PR)-- is that decoder suppression _already_ exists in TOT, and this part of the patch seems entirely unaware of that. Indeed it essentially just cancels/undoes some of the existing support (as if the merge conflict was just resolved in favor of downstream without consideration of the existing functionality). This isn't just the command line option, but the actual suppression code. See the `bar` example in `HwModeEncodeDecode2.td` pointed to earlier, where the current patch regresses. That unit test was deliberately constructed to make sure suppression occurs-- but this patch removed that as well. As mentioned above, it appears that the decoder patch is trying to achieve what TOT already does (and TOT does more thoroughly and cleanly)-- just with a different (blank) name for the `AllModes` tables. This is what I meant above when I said "_The current TOT code seems to entirely obviate the decoder part of the patch_". The follow-up decoder patch (if needed at all) should address those points: explain what is meant to be achieved relative to TOT; how/why TOT does not already achieve that aim; ensure no suppression regressions.

Thank you very much for your response[nvjle](https://github.com/nvjle), which I saw in the middle of the night. Regarding your third point, I roughly understand what you mean. You meant that the suppression was already implemented in your previous patch, and there is no need to repeat that. I have carefully considered and compared the implementation of our two patches, and I will explain my specific ideas.

Firstly, if we define two hwmodes for instruction A, the old logic will split all  DecoderTable into two form, each corresponding to one of the two hwmodes. Except for the DecoderTable containing instruction A, the rest of the tables do not need to be duplicated and must be preserved in their original form. My patch accomplishes this by removing unnecessary table duplication and retaining necessary table duplication for hwmode enablement.

As for your patch, you first adds an ‘AllModes’ suffix to the DecoderTables that do not need any modification(no hwmodes in them), which is unnecessary in my opinion. Leaving that aside, instruction A is then individually selected, and two DecoderTables, one for hwmode A and another for hwmode B, are created storing the different encodings infos for instruction A. All the other instructions under the same DecoderNamespace but without the hwmode attribute as instruction A are saved in an AllModes table. Overall, I see that your suppression method separates the instructions assigned with hwmode attributes from their original DecoderNamespace, which is not suitable since hwmode should not change the DecoderNamespace of a given instruction.

Once you extract hwmode-specific instructions using your approach, you will no longer be able to check whether the encoding of instruction A under hwmode A and hwmode B conflicts when disassembling with other instructions that belong to the same DecoderNamespace. 

Actually I have considered extracting instructions with hwmode attributes into sperate tables before I seeing your patch, but I felt that this way might break the logic of DecoderNamespace, so I just give it up. My view is that if users do not specify a new DecoderNamespace, the encodings of instructions under different hwmodes should still remain within the previous DecoderNamespace.

Of course, your suppression method is more radical and can significantly reduce the size of the table if users are made aware of it and know that they can use it intentionally. But still want to say , if you can remove the ‘AllModes’ suffix , it would be better.



https://github.com/llvm/llvm-project/pull/84906