Ping?

Since you approved the change I take you are happy with the code as-is, and marked all the comments as done. Thanks.

In D124266#3472842, @MaskRay wrote:

[lld/elf] fix quote usage in section names

"Support " is more appropriate. We don't claim this was supported so we cannot consider this as a bug.

• Use split-file.
• Remove .bss test case.
• Test presence of {text,data}_size in the output.
• Add comment about purpose of tests.

Thanks for the review, let me know if you agree with the reply. Will look into using split-file if that's desirable.

In D44531#1039804, @labath wrote:

llvm/tools/clang/lib/Basic/SourceManager.cpp:173:17: error: no viable conversion from 'const char [5]' to 'llvm::StringLiteral'

Are you using gcc (~4.x) by any chance? Older versions of gcc seem to have issues with the StringLiteral constructor. One solution is to wrap the argument in extra {}, so something like .StartsWith({"\x00\x00\xFE\xFF"}, "UTF-32 (BE)")

• Remove unneeded includes from MCAsmMacro.h

• Fix nits in the test.
• Properly format MCAsmMacro.h.

I still think the interface should be fixed, so that later on when lfence (or other thunks) is added clang doesn't end up with:

In D41723#967987, @chandlerc wrote:

Add support for externally provided thunks. This is an independent feature;
when combined with the overall retpoline feature it suppresses the thunk
emission and rotates the names to be distinct names that an external build
system for the kernel (for example) can provide.

I've added some minimal documentation about the semantic requirements of these
thunks to the commit log, although it is fairly obvious. More comprehensive
documentation will be part of the large follow-up effort around docs.

AFAICT this change doesn't allow to use different indirect thunks, gcc allows such usage with the -mindirect-branch=thunk-extern/-mindirect-branch-register options. The following patches for example allow Xen to choose the optimal thunk depending on the processor:

It's not clear to me whether upstream is going to do something about this or not. I would like to know in case I need to start passing "-Wno-address-of-packed-member" around.

In D20561#663069, @joerg wrote:

@royger: Your example is missing explicit alignment. packed has two side effects: remove internal padding and set the alignment to 1. That means that the offset of base doesn't matter so much because reg itself is not necessarily aligned.

In D20561#663006, @kimgr wrote:

... and both revisions should be in the 4.0 branch (taken from r291814)

I was looking forward to this warning to help iron out alignment issues at compile-time instead of runtime (our ARM CPUs don't like unaligned access). @royger, can you expand a little on what you mean by

For once, it doesn't really check whether the access is actually unaligned or no

? Isn't that exactly what it does? Or do you mean in relation to whether the target allows unaligned access or not?

In D20561#662959, @rogfer01 wrote:

We fixed all identified false positives in later patches to this one. So maybe you want to check against trunk clang. If trunk still diagnoses false positives, please report them to me and I will be more than happy to fix them.

This addition is silly, and too noisy for real world usage.