- User Since
- Dec 30 2017, 7:39 PM (11 w, 6 d)
LGTM with nits.
LGTM with a nit.
This was fixed by r328299, right?
Thu, Mar 22
Wed, Mar 21
> The current behaviour is only to fill the last page of executable segments with trap instructions. So even in the non linker script case, there still could be zero rather than trap instruction padding in the PLT. I don't know how big an issue this really is, but it feels better to be padding with trap instructions.
Do you have commit access?
Tue, Mar 20
LGTM with a test nit.
The callback would only be used for recoverable errors, though, yes? Allowing the user to differentiate the two cases ("recoverable things come through the callback, non-recoverable things are errors seen as a result that didn't pass through/come from the callback"). Seems fair to me.
I think you may have missed the difference between errors which allow parsing later tables and errors which prevent parsing any later tables. If the unrecoverable errors do not go through a callback, we need the extra information of whether the length field is valid or not available to the caller somehow. I suppose we could add an extra method to the LineTable class instead to query whether the length is valid or not instead of the custom error type, but there's a risk here that users will miss this function and still try to use the length field.
LGTM with a nit since this is just dumping something that gnu tools are now producing.
LGTM, but do use the git-clang-formated version.
Mon, Mar 19
This became D41644
The crash is from
One thing that I am probably missing is why we need this much.
I agree with George's comments, but LGTM otherwise.
You could use llvm-cxxfilt.cpp to more directly test the demangler, no?
Changes to the demangler should go first to libcxxabi/src/cxa_demangle.cpp.
I am actually surprised these are not hidden.
Some of these seem to be really unused (mutex in InputSection.cpp).
Thu, Mar 15
Please rebase on top of trunk.
LGTM with just a nit.
Actually, how does ELF/COFF linker know that if LTO decides to drop local_unnamed_addr (since only macho has flags to indicate that)? Is lld/gold looking at the object file to figure out if there are uses for the addr for this symbol?
I think my perspective is informed by the text in LangRef and whatever LLVM used to do for COFF when I started working on it. Meaning, weak and linkonce linkages just create a comdat, nothing else. The weak vs. linkonce bit indicates what's discardable. The ODR bit indicates whether you can optimize.
Being able to have undefined weak symbols was always just some weird ELF feature that wasn't spelled out in LangRef and didn't work on COFF so it must not be inside the model. It's entirely possible that I've given that old model too much weight and we should update the LangRef to talk about how weak declarations can be undefined at link time.
Now that we have comdats, weak_odr and linkonce_odr are pretty weird linkages. We could, for example, migrate weak_odr+comdat things to just external+comdat, and that would work pretty well. External GVs aren't discardable, but most of the optimizer still reasons from the linkage without looking at the comdat, so this would be a pretty big change.
While having a symbol table resolved some of the issues, I still think we should aim for the IR having real symbol names one day. It would make it impossible for two GVs to have the same symbol and much easier to reason about which GVs are used from inline assembly.
gnu objdump will print
If LTO does change the IR in some way that would drop local_unnamed_addr, it will not be present in the .o that is passed to the linker.
I agree with George. We currently produce an error and it would be better if we could keep doing that.
Can this be closed?
would it be possible to test this?
I agree that using function local static would be an improvement regardless of any speedup.
This is unfortunate, but I don't think we can do better with the current split. The alternative would be to effectively revert r325139 and have clang/llc use a single parser for all the inline assembly in a file.