[PATCH] D121768: [tablgen] Auto-generate fields & interfaces for target features

[Phoebe Wang via Phabricator via llvm-commits]

pengfei added inline comments.


================
Comment at: llvm/lib/Target/X86/X86.td:22
 
-def Mode64Bit : SubtargetFeature<"64bit-mode", "In64BitMode", "true",
                                   "64-bit mode (x86_64)">;
----------------
I think `In64BitMode` is more readable, we usually generate 32 bit instructions under 64 bit mode.


================
Comment at: llvm/lib/Target/X86/X86.td:33
 
 def FeatureX87     : SubtargetFeature<"x87","HasX87", "true",
                                       "Enable X87 float instructions">;
----------------
How about define a new class like `TrivalSubtargetFeature` so that we don't need to add `, [], 0` to the old one?


================
Comment at: llvm/lib/Target/X86/X86InstrInfo.td:882
 def HasMMX       : Predicate<"Subtarget->hasMMX()">;
-def Has3DNow     : Predicate<"Subtarget->has3DNow()">;
-def Has3DNowA    : Predicate<"Subtarget->has3DNowA()">;
+def Has3DNow     : Predicate<"Subtarget->hasThreeDNow()">;
+def Has3DNowA    : Predicate<"Subtarget->hasThreeNowA()">;
----------------
Should change here too? The same below.


================
Comment at: llvm/lib/Target/X86/X86Subtarget.cpp:56
 
-
 /// Classify a blockaddress reference for the current subtarget according to how
----------------
Unrelated change.


================
Comment at: llvm/lib/Target/X86/X86Subtarget.h:89
-  /// True if this processor has NOPL instruction
-  /// (generally pentium pro+).
-  bool HasNOPL = false;
----------------
Some comments like this doesn't appear on X86.td, should we move all these comments there?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121768