Changed comment

In rL362901#663831, @asbirlea wrote:

@anton-afanasyev: FYI, we're still seeing this and it presents itself as a miscompile.
We're still working on getting a test case but it's proving to be very difficult.

Also, first "Frontend" section contains "ParseTemplate" and "PerformPendingInstantiations" sections.

In D63325#1553458, @lebedev.ri wrote:

Hm, i started writing previous comment before you posted your last comment, so i didn't see the last update.
This looks less intrusive, yes, but two observations:

1. You now have two "Frontend" sections - first one being for lexing time
2. That lexing section is not within the "Frontend" section, even though it is for sure part of frontend of the compiler. Like i said, i'm not sure what the right solution here is.

Hi @lebedev.ri, I've turned back to the solution with one TimeTraceScope for "Frontend" inside BackendConsumer::HandleTranslationUnit(), since it looks more robust. This admits several (two for now) "Frontend" sections, but I see no problem with this. Other solutions have to rely on BackendConsumer and ASTConsumer calling correct relations, which are not guaranteed (though it is looking correct for now).

Updated, changed test

In D63325#1549316, @lebedev.ri wrote:

Hmm, i'm only now noticing how fraglie this looks.
How do we know that clang::ParseAST will only ever be called with BackendConsumer ASTConsumer?
How do we know that BackendConsumer will only ever be ASTConsumer from clang::ParseAST?

Updated

Updated

Updated

Small fix

In rL362901#661288, @asbirlea wrote:

@anton-afanasyev:
The test failures occur in Halide generated code, with the HVX backend. They are not public.
r363164 does not fix the tests; it does change an erroneous output to another erroneous one.

@asbirlea Btw, this fix was commited for the issue noted here: https://reviews.llvm.org/D56772#1537772

In rL362901#661260, @asbirlea wrote:

FYI, we're seeing test failures due to this revision. Currently trying to isolate a testcase, but it may be challenging to minimize if the failures are due to a miscompile.

Last issue fixed by this revision: https://reviews.llvm.org/D63148

Update description

Add test

In D63148#1538399, @lebedev.ri wrote:

Missing a test

Hi @john.brawn , here is the fix, could you please look to it: https://reviews.llvm.org/D63148

The issue actually is not with this instruction, but it's related to exception handling. Hoisted instruction is inserted before getFirstTerminator(), but there could be EH_LABEL's which are not terminators, but could change control flow.

In D56772#1537772, @john.brawn wrote:

This is causing incorrect code generation for this piece of IR:

target datalayout = "e-m:e-i8:8:32-i16:16:32-i64:64-i128:128-n32:64-S128"
target triple = "aarch64-arm-none-eabi"

@var = hidden local_unnamed_addr global i32 0, align 4
@_ZTIi = external dso_local constant i8*
declare dso_local void @_Z2fnv() local_unnamed_addr #1
declare dso_local i32 @__gxx_personality_v0(...)
declare i32 @llvm.eh.typeid.for(i8*) #2
declare dso_local i8* @__cxa_begin_catch(i8*) local_unnamed_addr
declare dso_local void @__cxa_end_catch() local_unnamed_addr

define hidden i32 @_Z7examplev() personality i8* bitcast (i32 (...)* @__gxx_personality_v0 to i8*) {
entry:
  invoke void @_Z2fnv()
          to label %try.cont unwind label %lpad

lpad:                                             ; preds = %entry
  %0 = landingpad { i8*, i32 }
          catch i8* bitcast (i8** @_ZTIi to i8*)
          catch i8* null
  %1 = extractvalue { i8*, i32 } %0, 0
  %2 = extractvalue { i8*, i32 } %0, 1
  %3 = tail call i32 @llvm.eh.typeid.for(i8* bitcast (i8** @_ZTIi to i8*))
  %matches = icmp eq i32 %2, %3
  %4 = tail call i8* @__cxa_begin_catch(i8* %1)
  %5 = load i32, i32* @var, align 4
  br i1 %matches, label %catch1, label %catch

catch1:                                           ; preds = %lpad
  %or3 = or i32 %5, 4
  store i32 %or3, i32* @var, align 4
  tail call void @__cxa_end_catch()
  br label %try.cont

try.cont:                                         ; preds = %entry, %catch1, %catch
  %6 = load i32, i32* @var, align 4
  ret i32 %6

catch:                                            ; preds = %lpad
  %or = or i32 %5, 8
  store i32 %or, i32* @var, align 4
  tail call void @__cxa_end_catch()
  br label %try.cont
}

As part of the accesses to var an ADRP instruction is being generated and MachineCSE is hoisting it to the entry block, but it gets hoisted to after the call to fn so when we catch an exception the ADRP hasn't been executed so the loads and stores use an undefined base register.

Closed by commit https://reviews.llvm.org/rL362901

In D61914#1534508, @plotfi wrote:

cfe/trunk/test/Driver/check-time-trace.cpp appears to fail on Darwin. Did you mean to pass the target explicitly ?

In D61914#1512021, @aganea wrote:

Could you please move the test to a more approriate location? (ie. clang/trunk/test/Driver/)

Updated

LGTM, thanks.

In rL361356#652786, @anil9 wrote:

Hi @anil9 . Yes, you are right -- it's my change being causing the stage 2 failure. I'm working on fix. Should I revert it meanwhile?

Yeah I think reverting will help since the buildbots are in failed stage for more than 24 hours and a planned outage is coming.

Thanks!

Yes, thank you, it was planned, sorry for pending this.

In rL361356#652500, @anil9 wrote:

Hi,

Your changeset might be causing failures on this buildbot http://lab.llvm.org:8011/builders/clang-ppc64be-linux-multistage?numbuilds=100
http://lab.llvm.org:8011/builders/clang-ppc64be-linux-multistage/builds/18248
Most of the stage 2 changes are still failing consistently, could you please, take a look.
Given the machine is going for a planned outage on Friday I may not be able to sort the exact cause so I am letting you earlier.
Thanks.

In D62067#1508281, @philip.pfaffe wrote:

I might have fallen a bit out of the loop, but can you tell me a bit more details about the use case here? There is no in-tree support for new-PM backend passes currently, is this for an out-of-tree user?
What about all the other IRUnit nestings beyond function passes in module pipelines?

Those questions aside, I'm not happy with the implementation. It uses global state, which is bad on its own, but in a way that isn't remotely threadsafe. It also cannot be easily made threadsafe without bigger redesign. Therefore, this isn't going to work in the new PM.

In the new PassManager we have a quite nice instrumentation infrastructure, which is perfectly suited to hook tracers such as this one into the pipeline execution. I suggest implementing -ftime-trace on top of that infrastructure. That being said, it appears as if there's quite some overlap between this tracer and the pass timing things. Why is the existing mechanism insufficient? Is the difference really just the output format? In which case, why not make that customizable instead of rolling out a second timing instrumentation?

In D62067#1506690, @lebedev.ri wrote:

I know this is in line with what is done in old-pm, but i find the middle-end names to be very useless.
In particular, why is the actual optimization pass name is always in the description, and the actual timeslice name is always the same generic "Opt???"?
IMHO the actual optimization pass name should be the Name, not Detail.

LGTM, thanks!

LGTM
Thanks!

Update, clang-format

Small fix and tests update

Rebase. Please, review this.

Looks great, thank you!

Update

Updated assertion

Updated

Updated, clang-format

In D60788#1469005, @lebedev.ri wrote:

As i have previously discussed in IRC with @sammccall this change needs motivational numbers.
Just copying them (+methodology) from https://reviews.llvm.org/D60609#inline-538565 sounds good.
-5.5% sounds rather great to me.

Patch by @thakis for BE passes tracing: https://reviews.llvm.org/D60782