[PATCH] D34543: Track globals promoted to coalesced const pool entries

Saleem Abdulrasool via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Jul 4 11:27:19 PDT 2017


compnerd added a comment.

Please trim down the test.  I dont think that any of the debug info metadata is needed to validate the CPE is coalesced nor are the attributes.



================
Comment at: lib/Target/ARM/ARMAsmPrinter.cpp:872
     auto *ACPC = cast<ARMConstantPoolConstant>(ACPV);
-    auto *GV = ACPC->getPromotedGlobal();
-    if (!EmittedPromotedGlobalLabels.count(GV)) {
-      MCSymbol *GVSym = getSymbol(GV);
-      OutStreamer->EmitLabel(GVSym);
-      EmittedPromotedGlobalLabels.insert(GV);
+    for (auto *GV : ACPC->promotedGlobals()) {
+      if (!EmittedPromotedGlobalLabels.count(GV)) {
----------------
Shouldn't this be `const auto *GV`?


================
Comment at: lib/Target/ARM/ARMConstantPoolValue.cpp:196
+    ARMConstantPoolValue *CPV =
+      (ARMConstantPoolValue*)CP->getConstants()[index].Val.MachineCPVal;
+    auto *Constant = cast<ARMConstantPoolConstant>(CPV);
----------------
Please use C++ style casts (`static_cast`).  Renaming the type doesn't offer much here, why not use `auto`?


================
Comment at: lib/Target/ARM/ARMConstantPoolValue.cpp:210
   ID.AddPointer(CVal);
+  for (auto GV : GVars)
+    ID.AddPointer(GV);
----------------
This should probably be `const auto *` or `const auto &`.


================
Comment at: lib/Target/ARM/ARMConstantPoolValue.h:179-180
+  typedef iterator_range<promoted_iterator> promoted_iterator_range;
+  promoted_iterator promoted_begin() const { return GVars.begin(); }
+  promoted_iterator promoted_end() const { return GVars.end(); }
+  promoted_iterator_range promotedGlobals() {
----------------
If these aren't used right now, we could just inline them into the `promotedGlobals` call.


Repository:
  rL LLVM

https://reviews.llvm.org/D34543





More information about the llvm-commits mailing list