Thanks @jhenderson, turned out I did need a one-line change for asmparser to work.

• Add new test for MCParser.
• Clang-format.

• clang-format.

Thanks for the comments @MaskRay

• Rebase.
• Unify access to exception symbols for basic-block-sections and no basic-block-sections.
• other nits.

LGTM.
Please just keep in mind that call-site-table is not rigorously tested in LLVM.

In D73739#2278247, @modimo wrote:

@rahmanl Have you had a chance to run your added test on ARM64 as another itanium C++ ABI target to make sure it works properly? Having testing there is fine as a follow-up but wanted to make sure we're in agreement here. Otherwise changes LGTM, please let @MaskRay have a chance to provide any additional feedback.

@MaskRay ping.

Sorry about the headache @MaskRay I should've built after rebasing.

• Change the getSectionName call to pass pointer.

• Rebase with upstream.

Use "Entries:" instead of "Content:" in test.

• Rebase

• Merge branch 'master' into arcpatch-D85408

• Remove the "labels" part of the clang test as the functionality is tested on LLVM tests.

In D85408#2266559, @dblaikie wrote:

In D85408#2262134, @rahmanl wrote:

@efriedma Would you please chime in specially with respect to @MaskRay 's comment about the clang test involving assembly?

I'll chime in, perhaps @efriedma will/can too: This patch has no changes to Clang's code, so it should have no changes to Clang's tests, generally speaking.

Whatever codepath/behavior is being tested by that Clang test change should be testable (& generally only tested) via LLVM's tools/tests, like llc, I would expect? (I guess that's what the basic-block-sections-labels.ll test is doing? Making the Clang test update redundant)

The only reason I'd expect to see the assembly tested in Clang is if the flag that enables this feature is an MCOptions (or other similar API level feature) member, rather than part of LLVM IR proper. In that case the only way to test that the flag has any effect (since we can't observe the MCOptions struct state directly in a test) is to test for the MCOptions effect on LLVM's output. Depending on the complexity, sometimes such tests are just skipped entirely (leaving the setting of the MCOption untested) - I think I did that for, for example, DWARF type units. Other times, an end-to-end test is used to ensure the flag is working. But that's all the test should do: test the flag is working, with some very basic/rudimentary property of the feature being enabled (so pretty much /any/ change to the way the feature works will still pass the test, but if the feature were turned off entirely (the MCOptions flag stopped being set correctly), the test would fail). Not test all the functionality of the feature that the flag enables - all that testing should be down in LLVM.

• Remove the complex assembly from the clang test.

In D73739#2264518, @MaskRay wrote:

Taking a closer look at the test (haven't delved into the code yet): the PIC vs non-PIC difference of .gcc_exception_table looks strange.
.gcc_except_table (please also test its section flags "a") has an absolute relocation referencing main.2. This is not good. In all targets (except RISC-V -mno-relax), .gcc_except_table is a relocation-free section.

I think you should just use PIC handling for -fno-PIC.

• clang-format.

• Do not use Optional for the CallSiteRanges parameter.
• Add the new function llvm::avoidZeroOffsetLandingPad(MachineFunction&) and expose it in BasicBlockSectionUtils.h.
• Use getNoop(MCInst&) instead of insertNoop() to insert NOP.

• Rebase.
• Override X86InstrInfo::InsertNoops and use it in BasicBlockSections.cpp to emit a NOP before the first EH pad.
• Create a lambda function for duplicated code in EHStreamer.cpp.
• Change some references to const.

In D73739#2264267, @modimo wrote:

In D73739#2261537, @snehasish wrote:

Once this patch is in we can look into splitting ehpads out though I'm more inclined to enhance the static profile count mechanism to account for ehpads appropriately rather than adding a new flag to MFS.

On this front, is there a specific reviewer that we're waiting for here to get this change through?

@efriedma Would you please chime in specially with respect to @MaskRay 's comment about the clang test involving assembly?

Thanks for the review @MaskRay. Is this ready to land now?

• Merge remote-tracking branch 'origin/master' into arcpatch-D85408

• Address +MaskRay's comments:
• Change the check prefix simply to "CHECK" for basic-block-sections-labels.ll
• Change the triple to x86_64 for this test.
• nits.
• Remove the "-LABEL" feature from basic-block-sections-labels.ll and use precise BB indices.
• Remove the assertion in emitBBAddrMapSection.

In D73739#2253361, @modimo wrote:

I think with this alongside the recently committed MachineFunctionSplitter (MFS) the MFS phase is a good place to split out EH landing pads in both profile and non-profile code. A small change in MFS will enable EH LP splits in profiling given they should always be cold. I think in non-profile mode a sub-flag can be added like -mfs-split-LP-only where EH LPs are always split out. Enabling a general static-analysis splitting could also be interesting but outside of EH LPs I'm not certain there's guaranteed gains in other scenarios.

Thanks a lot for the comments @MaskRay.

• Address @MaskRay's comments.
• Rebase with upstream.

• Move getBBAddrMetadata to AsmPrinter as a static function + nits.

Address Snehasish's comments.

• Move getBBAddrMetadata to AsmPrinter as a static function in AsmPrinter
• Remove unnecessary IR instructions in basic-block-sections-labels.ll

Fix failing clang test which used to check for the old ".bb_info" name.

Fix failing clang test which used to check for the old ".bb_info" name.

The previous diff was created incorrectly. This update fixes it.

Rebase.

Fix case style.

@eli.friedman and @MaskRay Does this small patch sound alright?

Fix exception handling on PIC targets (Credit to @amharc for implementing the fix).

This update does some refactoring on the CallSiteRange struct and how it is initialized.

Remove unrelated changes.

This syncs and updates the patch with upstream and propeller changes.

No longer pursued in LLD with BB-clusters ( D76954) as alternative route.

Re-uploading the diff since I missed test files in the latest update.

This final update applies @MaskRay and @efriedma's last suggestions.
Thanks to all the reviewers.

Thanks again @efriedma and @tmsriram.

The major change with this is a new struct called MBBSectionID to represent the basic block section IDs instead of a plain integer.
This makes the code cleaner (especially for the upcoming CFI and DebugInfo patches).