[PATCH] D35248: [ARM] Tidy up and organise better ARM.td. NFC.

Florian Hahn via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Jul 11 11:14:57 PDT 2017


fhahn added a comment.

Looks like a NFC to me. I left some inline comments, mostly potential indentation issues and some comments I think are worth keeping, unless I missed something and they are invalid now.



================
Comment at: lib/Target/ARM/ARM.td:88
+                                             "HasHardwareDivideInARM", "true",
+                                        "Enable divide instructions in ARM mode">;
+
----------------
Indent is off


================
Comment at: lib/Target/ARM/ARM.td:119
+def FeatureCrypto         : SubtargetFeature<"crypto", "HasCrypto", "true",
+                                        "Enable support for Cryptography extensions",
                           [FeatureNEON]>;
----------------
Indent is now off, as in the line below


================
Comment at: lib/Target/ARM/ARM.td:141
+def FeatureZCZeroing      : SubtargetFeature<"zcz", "HasZeroCycleZeroing", "true",
                                         "Has zero-cycle zeroing instructions">;
 
----------------
Indent is off


================
Comment at: lib/Target/ARM/ARM.td:191
+                                             "ExpandMLx", "true",
                                         "Expand VFP/NEON MLA/MLS instructions">;
 
----------------
Indent is off now


================
Comment at: lib/Target/ARM/ARM.td:255
+                                             "AvoidMOVsShifterOperand", "true",
                                 "Avoid movs instructions with shifter operand">;
 
----------------
Indent is off


================
Comment at: lib/Target/ARM/ARM.td:299
+                                          "GenExecuteOnly", "true",
+                              "Enable the generation of execute only code.">;
 
----------------
Indent is off now I think


================
Comment at: lib/Target/ARM/ARM.td:615
 
-// FIXME: A5 has currently the same Schedule model as A8
 def : ProcessorModel<"cortex-a5",   CortexA8Model,      [ARMv7a, ProcA5,
----------------
This is still true (it still uses CortexA8Model if I am not mistaken) and the fixme is still valid I think? Shouldn't we keep this fimex?


================
Comment at: lib/Target/ARM/ARM.td:659
 
-// FIXME: A12 has currently the same Schedule model as A9
 def : ProcessorModel<"cortex-a12",  CortexA9Model,      [ARMv7a, ProcA12,
----------------
This is still true (it still uses CortexA9Model if I am not mistaken) and the fixme is still valid I think? Shouldn't we keep this fimex?


================
Comment at: lib/Target/ARM/ARM.td:669
 
-// FIXME: A15 has currently the same Schedule model as A9.
 def : ProcessorModel<"cortex-a15",  CortexA9Model,      [ARMv7a, ProcA15,
----------------
This is still true (it still uses CortexA9Model if I am not mistaken) and the fixme is still valid I think? Shouldn't we keep this fimex?


================
Comment at: lib/Target/ARM/ARM.td:681
 
-// FIXME: A17 has currently the same Schedule model as A9
 def : ProcessorModel<"cortex-a17",  CortexA9Model,      [ARMv7a, ProcA17,
----------------
This is still true (it still uses CortexA9Model if I am not mistaken) and the fixme is still valid I think? Shouldn't we keep this fimex?


================
Comment at: lib/Target/ARM/ARM.td:723
 
-// FIXME: R4 has currently the same ProcessorModel as A8.
 def : ProcessorModel<"cortex-r4",   CortexA8Model,      [ARMv7r, ProcR4,
----------------
This is still true (it still uses CortexA8Model if I am not mistaken) and the fixme is still valid I think?


================
Comment at: lib/Target/ARM/ARM.td:728
 
-// FIXME: R4F has currently the same ProcessorModel as A8.
 def : ProcessorModel<"cortex-r4f",  CortexA8Model,      [ARMv7r, ProcR4,
----------------
This is still true (it still uses CortexA8Model if I am not mistaken) and the fixme is still valid I think?


================
Comment at: lib/Target/ARM/ARM.td:737
 
-// FIXME: R5 has currently the same ProcessorModel as A8.
 def : ProcessorModel<"cortex-r5",   CortexA8Model,      [ARMv7r, ProcR5,
----------------
This is still true (it still uses CortexA8Model if I am not mistaken) and the fixme is still valid I think?


================
Comment at: lib/Target/ARM/ARM.td:747
 
-// FIXME: R7 has currently the same ProcessorModel as A8 and is modelled as R5.
 def : ProcessorModel<"cortex-r7",   CortexA8Model,      [ARMv7r, ProcR7,
----------------
This is still true (it still uses CortexA8Model if I am not mistaken) and the fixme is still valid I think?


================
Comment at: lib/Target/ARM/ARM.td:838
 
-// Cyclone is very similar to swift
 def : ProcessorModel<"cyclone",     SwiftModel,         [ARMv8a, ProcSwift,
----------------
This comment is still true, no?


================
Comment at: lib/Target/ARM/ARM.td:915
 def ARM : Target {
-  // Pull in Instruction Info:
   let InstructionSet = ARMInstrInfo;
----------------
This comment is still true, no?


================
Comment at: lib/Target/ARM/ARM.td:934
 //===----------------------------------------------------------------------===//
-
+//
 include "ARMRegisterInfo.td"
----------------
Do we need this empty comment line?


================
Comment at: lib/Target/ARM/ARM.td:953
   int PassSubtarget = 1;
   int Variant = 0;
   bit isMCAsmWriter = 1;
----------------
Do we need this comment line?


https://reviews.llvm.org/D35248





More information about the llvm-commits mailing list