[PATCH] D117451: [MCA] Proposing the InstrPostProcess:modifyInstrDesc() method.

Patrick Holland via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Sun Jan 16 23:36:56 PST 2022


holland11 added a comment.

@myhsu

> In the cases where Modified is true (and it's not variadic / variant), should we still cache InstrDesc by the opcode?
> Or more specifically, can modifyInstrDesc create different changes for two separate MCInst-s that has the same opcode? (e.g. I'm only changing the MayStore flag in an ADD32 MCInst if it's preceded by a LD16, given the fact that IPP can have state)
> Because if that's true, I think the Descriptors cache here will return a stale / incorrect InstrDesc.
>
> I understand that in your motivating example ("...where we wanted to set the RetireOOO flag for all of our targets, but we didn't want to have to modify the tablegen for every single one of those targets...") you're probably making homogeneous changes across all MCInst-s that have the same opcode but I feel like there needs to be some clarifications on the model of modifyInstrDesc.

Yeah I should have made that more clear, sorry about that. Each `MCInst` that shares the same opcode will also share the same `InstrDesc` object. So if you have two instructions that share the same opcode and you only want to modify one of them (and the modifications require modifications to the `InstrDesc` object) then this patch won't help with that. `InstrPostProcess::postProcessInstruction()` **can** modify specific instructions, however if the attributes you want to modify are part of the `InstrDesc` class then you're out of luck since that will be `const` (and shared) by then.

This is maybe something for me to look into in the future though as I do see the potential benefit.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D117451



More information about the llvm-commits mailing list