[PATCH] D94134: [X86] Add TLBSYNC, INVLPGB and SNP instructions

[Craig Topper via Phabricator via llvm-commits]

craig.topper added inline comments.


================
Comment at: llvm/lib/Target/X86/X86InstrInfo.td:965
 def HasCLZERO    : Predicate<"Subtarget->hasCLZERO()">;
+def HasINVLPGB   : Predicate<"Subtarget->hasINVLPGB()">;
+def HasTLBSYNC   : Predicate<"Subtarget->hasTLBSYNC()">;
----------------
These feature flags should probably only be here if intrinsics are added. Without that nothing will ever check the features.


================
Comment at: llvm/lib/Target/X86/X86InstrInfo.td:2937
+                  "invlpgb}", []>,
+                  TB, Requires<[HasINVLPGB, Not64BitMode]>;
+  let Uses = [RAX, EDX] in
----------------
TB should be PS since we have XD and XS instructions that use the same opcode. TB is supposed to indicate that 0xf2/0xf3 prefixes don't effect the instruction.


================
Comment at: llvm/lib/Target/X86/X86InstrInfo.td:2954
+                  "tlbsync", []>,
+                  TB, Requires<[HasTLBSYNC]>;
+} // SchedRW
----------------
TB should be PS since we have XD and XS opcodes.


================
Comment at: llvm/lib/Target/X86/X86InstrSNP.td:21
+def PSMASH: I<0x01, MRM_FF, (outs), (ins), "psmash", []>, XS,
+            Requires<[]>;
+
----------------
Should this have In64BitMode in the requires field?


================
Comment at: llvm/lib/Target/X86/X86InstrSNP.td:35
+def RMPUPDATE: I<0x01, MRM_FE, (outs), (ins), "rmpupdate", []>, XD,
+               Requires<[]>;
+
----------------
Same here


================
Comment at: llvm/lib/Target/X86/X86InstrSNP.td:40
+def RMPADJUST: I<0x01, MRM_FE, (outs), (ins), "rmpadjust", []>, XS,
+               Requires<[]>;
+} // SchedRW
----------------
And here


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94134