[PATCH] D35014: [X86] PR32755 : Improvement in CodeGen instruction selection for LEAs.

[Simon Pilgrim via Phabricator via llvm-commits]

RKSimon added a comment.

Some style comments, but @lsaba 's comments need to be dealt with first.



================
Comment at: include/llvm/CodeGen/MachineInstr.h:1296
   MachineRegisterInfo *getRegInfo();
+private:
 
----------------
(style) newline before private


================
Comment at: include/llvm/CodeGen/SelectionDAG.h:305
+  bool IsDAGPartOfLoop = false;
 private:
   /// DAGUpdateListener is a friend so it can manipulate the listener stack.
----------------
(style) newline before private


================
Comment at: lib/Target/X86/X86ISelDAGToDAG.cpp:171
+    bool OptForAggressingFolding;
   public:
     explicit X86DAGToDAGISel(X86TargetMachine &tm, CodeGenOpt::Level OptLevel)
----------------
(style) newline


================
Comment at: lib/Target/X86/X86ISelDAGToDAG.cpp:1169
+                                              X86ISelAddressMode &AM,
                                               unsigned Depth) {
   SDLoc dl(N);
----------------
clang-format? If so, commit it as an NFC change


================
Comment at: lib/Target/X86/X86ISelDAGToDAG.cpp:1178
+  if ((!EnableAggressiveFolding && Depth > 5) ||
+        (EnableAggressiveFolding && Depth > 8) )
     return matchAddressBase(N, AM);
----------------
Two hard coded depths like this is weird - better to have a getMaxOperandFoldingDepth() helper?


================
Comment at: lib/Target/X86/X86OptimizeLEAs.cpp:224
 };
-}
+} // namespace llvm
 
----------------
NFC change - just commit it if you want, but don't pollute a patch with it


================
Comment at: lib/Target/X86/X86OptimizeLEAs.cpp:528
 char OptimizeLEAPass::ID = 0;
-}
+} // namespace
 
----------------
NFC change


================
Comment at: lib/Target/X86/X86OptimizeLEAs.cpp:561
+  int MemOpNo =
+      X86II::getMemoryOperandNo(Desc.TSFlags) + X86II::getOperandBias(Desc);
 
----------------
clang-format? If so, commit it as an NFC change


================
Comment at: lib/Target/X86/X86OptimizeLEAs.cpp:854
           const MCInstrDesc &Desc = MI.getDesc();
-          int MemOpNo =
-              X86II::getMemoryOperandNo(Desc.TSFlags) +
-              X86II::getOperandBias(Desc);
+          int MemOpNo = X86II::getMemoryOperandNo(Desc.TSFlags) +
+                        X86II::getOperandBias(Desc);
----------------
clang-format? If so, commit it as an NFC change


================
Comment at: lib/Target/X86/X86OptimizeLEAs.cpp:969
+      List.sort(CompareFn);
+    }
+    // Loop over all LEA pairs.
----------------
(style) Remove braces


https://reviews.llvm.org/D35014