[PATCH] D56586: [PPC64] Update LocalEntry from assigned symbols

[Leandro Lupori via Phabricator via llvm-commits]

luporl marked 3 inline comments as done.
luporl added inline comments.


================
Comment at: lib/Target/PowerPC/MCTargetDesc/PPCMCTargetDesc.cpp:194
     // the st_other bits encoding the local entry point offset.
-    if (Value->getKind() != MCExpr::SymbolRef)
-      return;
-    const auto &RhsSym = cast<MCSymbolELF>(
-        static_cast<const MCSymbolRefExpr *>(Value)->getSymbol());
-    unsigned Other = Symbol->getOther();
+    if (copyLocalEntry(Symbol, Value))
+      UpdateOther.insert(Symbol);
----------------
MaskRay wrote:
> ```
> if (copyLocalEntry(Symbol, Value))
>   (void)UpdateOther.insert(Symbol);
> else
>   (void)UpdateOther.erase(Symbol);
> ```
> 
> ?
Just to confirm, the suggestion is to replace lines 188-195 by the code above, right?


================
Comment at: lib/Target/PowerPC/MCTargetDesc/PPCMCTargetDesc.cpp:204
+private:
+  std::set<MCSymbolELF *> UpdateOther;
+
----------------
MaskRay wrote:
> Consider `SmallPtrSet`
Ok


================
Comment at: lib/Target/PowerPC/MCTargetDesc/PPCMCTargetDesc.cpp:207
+  bool copyLocalEntry(MCSymbolELF *D, const MCExpr *S) {
+    if (S->getKind() != MCExpr::SymbolRef)
+      return false;
----------------
MaskRay wrote:
> `S->getKind() != MCExpr::SymbolRef` + `static_cast<const MCSymbolRefExpr *>(S)->getSymbol()` may be changed to
> 
> ```
> auto *Ref = dyn_cast<const MCSymbolRefExpr>(S);
> if (!Ref)
>   return false;
> ...
> ```
Ok


Repository:
  rL LLVM

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

https://reviews.llvm.org/D56586