[PATCH] D136204: [BOLT][DWARF] Add support for DW_FORM_addr for DW_AT_call_return_pc

[Maksim Panchenko via Phabricator via llvm-commits]

maksfb added inline comments.


================
Comment at: bolt/lib/Rewrite/DWARFRewriter.cpp:510
           DebugInfoPatcher.addUDataPatch(AttrVal.Offset, Index, AttrVal.Size);
+        } else if (AttrVal.V.getForm() == dwarf::DW_FORM_addr)
+          DebugInfoPatcher.addLE32Patch(AttrVal.Offset, UpdatedAddress);
----------------
ayermolo wrote:
> maksfb wrote:
> > nit: use curly braces for the `else` block(s) if you've used them for the `if` block.
> I was wondering about that.
> Is it because of 
> "Similarly, braces should be used when a single-statement body is complex enough that it becomes difficult to see where the block containing the following statement began. An if/else chain or a loop is considered a single statement for this rule, and this rule applies recursively."
It mostly b/c of: "... readability is also harmed if an if/else chain does not use braced bodies for either all or none of its members..."


================
Comment at: bolt/lib/Rewrite/DWARFRewriter.cpp:511
+        } else if (AttrVal.V.getForm() == dwarf::DW_FORM_addr)
+          DebugInfoPatcher.addLE32Patch(AttrVal.Offset, UpdatedAddress);
         else
----------------
ayermolo wrote:
> maksfb wrote:
> > Probably no need to update the address if it hasn't changed to reduce the number of patches?
> I was going for simpler logic. I doubt it will add that many more patches.
Okay.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D136204