[PATCH] D100026: [X86] Support AMX fast register allocation

[Pengfei Wang via Phabricator via llvm-commits]

pengfei added a comment.

You can find more information about coding style here: https://llvm.org/docs/CodingStandards.html#don-t-use-braces-on-simple-single-statement-bodies-of-if-else-loop-statements.



================
Comment at: llvm/lib/Target/X86/X86FastTileConfig.cpp:297
+  fastTileConfig();
+  return true;
+}
----------------
Should this return true based on tilecfg rewrite?


================
Comment at: llvm/lib/Target/X86/X86LowerAMXType.cpp:413
+    II = cast<IntrinsicInst>(PhiOp);
+  } else
+    II = cast<IntrinsicInst>(V);
----------------
Coding style: Use `{}` when `if` uses it.


================
Comment at: llvm/lib/Target/X86/X86LowerAMXType.cpp:635-637
+    // if (F.hasFnAttribute(Attribute::OptimizeNone) ||
+    //     TM->getOptLevel() == CodeGenOpt::None)
+    //   return false;
----------------
Remove unused code.


================
Comment at: llvm/lib/Target/X86/X86PreAMXConfig.cpp:267
+BasicBlock::iterator
+X86PreAMXConfig::getShapesAndConfigPosEnd(BasicBlock::iterator Iter,
+                                          SmallVector<Value *, 8> &Shapes) {
----------------
Maybe better to use `BasicBlock::iterator &`, then you don't need to return it.


================
Comment at: llvm/lib/Target/X86/X86PreAMXConfig.cpp:281
+
+    if (isTileLoad(II))
+      Loads.insert(II);
----------------
Coding style: Use `{}` when the `else` uses it.


================
Comment at: llvm/lib/Target/X86/X86PreAMXConfig.cpp:371-373
+  for (auto &IPAndShapes : PosAndShapes) {
+    addTileConfig(IPAndShapes.first, IPAndShapes.second);
+  }
----------------
Coding style: Don't use `{}` for single line.


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

https://reviews.llvm.org/D100026