[PATCH] D72194: [MC][ELF] Ensure that mergeable globals with an explicit section are assigned to SHF_MERGE sections with compatible entsizes

[Nick Desaulniers via Phabricator via llvm-commits]

nickdesaulniers added inline comments.


================
Comment at: llvm/include/llvm/MC/MCContext.h:272
+  public:
     std::map<ELFSectionKey, MCSectionELF *> ELFUniquingMap;
+  private:
----------------
are there getters/setters for this member? Might be better than exposing the raw member.


================
Comment at: llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp:102
+  auto &ELFSectionsMap = getContext().ELFUniquingMap;
+  for (auto i = ELFSectionsMap.begin(); i != ELFSectionsMap.end(); i++) {
+    MCSectionELF *ELFSection = i->second;
----------------
can a range based for loop be used here?


================
Comment at: llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp:106
+
+      auto i = ELFEntrySizeMap.insert(std::make_pair(
+          ELFEntrySizeKey{ELFSection->getSectionName(), ELFSection->getFlags(),
----------------
`i` shadows another `i` from above, and also isn't used.


================
Comment at: llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp:606
 
+  unsigned int EntrySize = getEntrySizeForKind(Kind);
+
----------------
LLVM coding style is just to use `unsigned` (drop the optional `int`) as per the `Flags` variable above.


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

https://reviews.llvm.org/D72194