Hi. This patch is causing a couple of test failures on one of our internal bots which is still running Ubuntu 18.04. Do you have any idea what could be causing it? I'm posting the test output:

ping ...

ping ...

In D111185#3046921, @MaskRay wrote:

I'd prefer we don't work around such usage which is unsupported. This is an opportunity for users to migrate to -DLLVM_ENABLE_RUNTIMES=compiler-rt

In D111185#3046902, @hjl.tools wrote:

In D111185#3046891, @wolfgangp wrote:

This is causing problems with gcc versions that don't have cet.h yet. I think cet.h wasn't added until gcc 8.

Please try this:

0001-sanitizer-Include-cet.h-only-if-it-exists.patch3 KBDownload

This is causing problems with gcc versions that don't have cet.h yet. I think cet.h wasn't added until gcc 8.

In D110783#3035158, @dyung wrote:

Hi, the test you modified Driver/crash-report.cpp is failing on the PS4 bot after your change. Can you take a look?

https://lab.llvm.org/buildbot/#/builders/139/builds/10939

ping ...

@fmayer Hi Florian!
Looks like the double-free test case is still failing here.

Addressed the review comments:

• added an assert that verifies that either the addend or LocData is 0
• renamed the relocation section into .rela.xxx
• deleted some unnecessary section properties

Ping. Any comments on this more RISCV-specific approach?

Ignore LocData for RELA relocations except for RISCV, which uses both LocData and Addend.

In D105214#2852339, @peter.smith wrote:

I've not seen SHT_RELA used for debug data before and I think all of the other LLVM tools assume SHT_REL. Are you planning to add SHT_RELA support to the other LLVM tools?

If nobody has any more objections, I'll commit this change, then. Please let me know if you think otherwise.

In D103495#2808247, @rnk wrote:

So, a question for @wolfgangp and @probinson , do we need to make adjustments to the LTO library, or is this OK as is?

In D103495#2804685, @MaskRay wrote:

Given the assessment on GlobalOpt, removed the LangRef changes and updated the subject line.

Following the suggestion to define an order of initialization for the entries in llvm.global_ctors and llvm.global_dtors this is mainly a documentation change, as well as a simple reversed emission of global_ctors/dtors entries when InitArray is not used.
3 test cases are affected, 2 of them just by a reversed order of emission of priority-suffixed sections. I added a couple of entries to the third test case to verify in-order emission of entries with equal priority.

In D103495#2794329, @rnk wrote:

+@rsmith @hans @aeubanks

specifically when init_array is not used

Can you elaborate on why that is? Here's what I remember, and what I guess is happening. ELF has two initializer schemes: .init_array and .ctors. They are essentially equivalent, they are both arrays of function pointers, but one is called in reverse order and the other is called in forward order. The compiler knows which scheme is in use and it is controlled by -fuse-init-array.

ping...

ping ...

Ping. Just wondering if there are any new insights on the issue reported in PR48030.

In D86644#2514125, @GMNGeoffrey wrote:

In D86644#2513914, @wolfgangp wrote:

In D86644#2513898, @GMNGeoffrey wrote:

FYI I'm seeing clang diagnostic errors, I think from this commit:

Should be fixed now.

I'm still seeing some. Looks like your fix didn't get all of them:

In rGd38be2ba0e4ebfed4c13ab79f3a8631011d185eb#971631, @leonardchan wrote:

Hi. It seems this commit is causing this failure on our aarch64 bulder (https://luci-milo.appspot.com/p/fuchsia/builders/ci/clang-linux-arm64/b8857452110716658368):

In D86644#2513898, @GMNGeoffrey wrote:

FYI I'm seeing clang diagnostic errors, I think from this commit:

Sorry about the missing files. Some hiccup while generating the diff.

Based on Andrea's request to use an OutputKind rather than overriding the printView method in a separate class for JSON output

This change is trying to use a more streamlined interface based on Andrea's feedback. I removed the printJSON method from the PipelinePrinter and the singleton object that was used to list the instructions.
Instead, each implemented view now has a <ViewName>JSON derived class that overrides the printView method. When JSON output is requested, objects of these class types are added as views. This leaves the PipelinePrinter largely unchanged.

Apologies for the long delay, I'm finally ready to resume work on this feature. The update does the following:

Isn't COMPILER_RT_HAS_FLOAT16 generated using the host compiler? If so, using the same flag for a compiler_rt test could fail if the host compiler supports f16 and the target compiler does not.

This seems to fail on some buildbots: e.g. here

The test seems to fail on some Linux buildbots, see here. Please review.

In D86644#2333464, @andreadb wrote:

Hey Wolfgang,
are you still looking at this patch? Do you have any updates?

This commit seems to cause a test failure on some bots, e.g. here.

In D86644#2239673, @lebedev.ri wrote:

Can it simply be autogenerated, without omitting details?

After some consultation with @andreadb this is some more extensive refactoring:
We create a new base class InstructionView that is meant for views that deal with individual instructions (e.g. print them). The 3 members MCIP (the instruction printer), STI (subtarget info) and Source (the array of machine instructions) live there and can be accessed via getters (they could be made protected as well). Printing the instructions is effected by using a string member in the new base class (which must be mutable along with its associated stream).

Here is another proposal: We're using statics for the Instruction string and the stream to write it and return a reference to it.
The users all immediately write the string to a different stream. This should have even fewer allocations than the original code.

Addressed review comment:

• using enumerate() instead of an explicit index variable

In D86177#2225748, @andreadb wrote:

Nice!

Thank you Andrea!

I agree with Roman in that we should not guarantee stability of data and/or structure.
Also, changes to the output structure should always be advertised (for example, by adding a line in the release notes), so that people are always aware of it.

Addressed review comments:

• Using MutableArrayRef
• Using range-based for loops with zip
• Using default initializers with InstructionInfoViewData

In D68620#1995191, @dblaikie wrote:

In D68620#1729184, @rupprecht wrote:

It looks like this caused a very large increase in binary size (627M->686M). Is that expected/has anyone else observed this?

sorry I didn't see this (somehow ended up muting this thread) - which binary built with what flags? could you run bloaty or otherwise compare the objects before/after. I'd expect some growth in linked executable size of a non-split, optimized debug build, but that seems a bit more than I'd expect/seems to be observed elsewhere so far as I know.

Shortly after this patch went in, we started to see intermittent failures with the unittest TUSchedulerTests.ManyUpdates. The build server (Windows) we're running it on is quite powerful and our own hand-run tests only take about 400 ms, so we don't really understand what could be happening. Any help would be appreciated.

LGTM, with a slight caveat regarding reporting an error for degenerate stubs of string offset sections.

I'm unfortunately still getting a loop in DAGcombiner with this one (llc -mattr=+avx on linux). If you make the 1.000 in the select instr into 0.000 it finishes.

define float @test(<8 x float> *%a0) {
entry:
  %0 = load <8 x float>, <8 x float>* %a0, align 32
  %vecext = extractelement <8 x float> %0, i32 1
  %cmp = fcmp oeq float %vecext, 0.000000e+00
  %cond = select i1 %cmp, float 1.000000e+00, float %vecext
  ret float %cond
}

I applied your patch to ToT, but now I'm seeing a loop with the following IR (on linux). Seems still stuck in DAGCombine.

In D67998#1683795, @jhenderson wrote:

In D67998#1682945, @wolfgangp wrote:

Yes, good observation. There is also the question what to do if the sizes of 2 different entries with the same address don't match. This should probably elicit a warning.

<snip>

1. Display the first entry size, for each symbol. This is probably the simplest.

For some reason, I didn't see your response until I had posted my updated. I basically implemented number 3.

1. Rebased with the latest upstream changes.
2. Addressed review comments as follows:
   • Keeping track of stack sizes already dumped so that duplicate entries only are printed once.
   • Issue a warning when a duplicate entry with a different stack size is encountered.
   • Simplified code by collapsing some warning code into a lambda.
   • added the appropriate test cases for the changed functionality and added one for STT_NOTYPE.
   • added an early return in a case where no valid symbol is found.

In D67998#1682076, @jhenderson wrote:

One issue that just occurred to me, is what happens if you still have the stack size entry in this case? Something like:

<snip>

I think this is undesirable. Perhaps you need to track whether a stack size entry for a given address has been printed yet, and skip it if it has?

Addressed review comments:

This sentence in the description:

The reasons for this are unclear, so this patch adds the option to llvm-strip.

We do not add options merely for "unclear" reasons. It is because the option is a good fit for llvm-strip.

I'm a bit unclear on what this means: