[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