[PATCH] D124056: [ELF] Fix wrapping symbols produced during LTO codegen

Fri Apr 22 15:32:38 PDT 2022

smeenai added inline comments.

================
Comment at: lld/ELF/SymbolTable.cpp:47
+    sym->isUsedInRegularObj = true;
+  else if (sym->isUndefined())
     sym->isUsedInRegularObj = false;
----------------
MaskRay wrote:
> smeenai wrote:
> > MaskRay wrote:
> > > It will be good to have a comment for the `if (sym->isUndefined())` condition.
> > Note that I'm tightening the condition further in D124065, which is a follow-up to this diff. Would you prefer adding a comment here and then changing it in D124065, or just adding the comment in D124065? (I think it'll be easier to explain the logic of the conditional with the change in D124065.)
> Ah adding the comment to D124065 will be good.
> 
> Are you splitting the two patches for easy bisection? I.e. in case the second patch  breaks something?
Yup, I wanted to keep the changes isolated so that each patch was an individual behavior change (i.e. this one only fixes the wrapping behavior and doesn't change any existing tests, and the next one changes symbol retention and needs an existing test to be modified).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124056