[PATCH] D72047: Add an interface emitPrefix for MCCodeEmitter

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: llvm/lib/Target/X86/MCTargetDesc/X86MCCodeEmitter.cpp:1367
+  // Pseudo instructions don't get encoded.
+  if ((TSFlags & X86II::FormMask) == X86II::Pseudo)
+    return;
----------------
skan wrote:
> craig.topper wrote:
> > When would emitPrefix be called on a X86II::Pseudo?
> I don't know, `encodeInstruction` checks `(TSFlags & X86II::FormMask) == X86II::Pseudo`, so I do the same for `emitPrefix`.  My thought is `emitPrefix` can be used anywhere where `encodeInstruction` can be used.
Ok thanks. I missed that.


================
Comment at: llvm/lib/Target/X86/MCTargetDesc/X86MCCodeEmitter.cpp:1421
     errs() << "FORM: " << Form << "\n";
     llvm_unreachable("Unknown FormMask value in X86MCCodeEmitter!");
+  case X86II::RawFrmDstSrc:
----------------
skan wrote:
> craig.topper wrote:
> > Why is Pseudo no longer in this switch?
> `encodeInstruction` always calls `emitPrefixImpl`, which already checks
> Pseudo in the corresponding switch.
Can you put it back just so that all possible values are mentioned by name in the switch. I think you can remove it from the emitPrefixImpl switch and just let it go to the default case.


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

https://reviews.llvm.org/D72047