@fhahn: thanks for your comments -- you have persuaded me. 🙂

Address @fhahn's comments.

Actually, I think I need to be smarter than changing the default. We want to let clang auto-detect the tty and behave that way by default if the option isn't specified. Otherwise you'd get ASNI color codes when you pipe to a file.

In D94418#2498190, @thopre wrote:

I've tried applying your patch and its test fail for me.

I've tried applying your patch and its test fail for me.

I've seen it landed on the upstream branch rather than the main branch. Was that a mistake?

Probably the CrossTranslationUnitContext::ImportedFileIDs can be removed. The FileIDImportHandler in ASTImporter was used only for this (if I know correctly) too but it can be useful if a similar mechanism is needed later, but it remains there as "dead code" if not removed.

In D93447#2498085, @awarzynski wrote:

Hi @sameeranjoshi , thank you for working on this!

Sadly with this patch one of our upstream builders has started failing: http://lab.llvm.org:8011/#/builders/33/builds/1866. More specifically,Semantics/omp-parallell01.f90 is failing.

All other builders are fine, which makes this weird. The only unique thing about the failing builder (flang-aarch64-ubuntu-clang) is that it uses clang-10. Could you take a look?

Edit: @kiranchandramohan just pointed out to me offline that this is probably fixed here: https://reviews.llvm.org/D94618.

Thank you for the patch.
I did get an email from the bot with a weird text Control flow escapes from \xcb\x1c\xda\xff\xff\nexpect at 16: Control flow escapes from PARALLEL\n'
I had a build of gcc 9.2 and the tried to check with the recent pull from main and all tests seem to work.
It was pointed[1] by @awarzynski that the issue was specifically due to clang-10.

• replace patterns with custom-lowering

Hi @lebedev.ri, I have uploaded an IR file that demonstrates the compile-time implications on our system. I used the following invocation.

Added two nits, but LGTM otherwise.

Nitpick: this would be better titled "[RISCV] Add intrinsics for vector AMO instructions" - I was a little confused seeing the title come past :)

This was a direct move from SelectionDAG's implementation as I wanted to avoid introducing any subtle behaviour changes - I've added the exhaustive tests (which has already exposed one issue I'm fixing now) and I'll start cleaning up the KnownBits::sextInReg implementation after that.

Stop setting the default for LaxVectorConversions in CompilerInvocation

In D78105#2487169, @nickdesaulniers wrote:

Compiler crash reported in: https://bugs.llvm.org/show_bug.cgi?id=48695

Hi @sameeranjoshi , thank you for working on this!

Looks ok.

LGTM

LGTM

LGTM - cheers

In D93946#2493599, @sanwou01 wrote:

In D93946#2493440, @aeubanks wrote:

Could you do some initial investigation or come up with a repro? Maybe an optimization remark that doesn't fire after this change?

Yes, I will do an initial investigation, hopefully resulting in a reproducer. It is always a bit of a challenge with LTO benchmarks, but the regression is large enough that it should be easy to spot.

LGTM, thanks!

Get tile register number from backend.

I also don't think we need to add unit tests for this, just because we can. The tests would be more complex than the code and wouldn't add much value. https://softwareengineering.stackexchange.com/a/147342 There are lots of resources about this.

In D94077#2489064, @werat wrote:

Thanks for the explanation, this makes sense. I've checked the mailing list archives and it seems there was already a discussion about the enumerators in the .debug_names index back in 2018 -- http://lists.dwarfstd.org/pipermail/dwarf-discuss-dwarfstd.org/2018-April/004443.html. You were the one to bring it up and the consensus was that the enumerators should go into the index too.

But it seems this was never actually implemented, the latest version of the toolchain doesn't add DW_TAG_enumerator entries to the .debug_names index. Is there a reason for that or it just slipped through the cracks? Should I bring it up again on the mailing list or we can assume the consensus is still the same and this should be just implemented?

In D94667#2498005, @tschuett wrote:

Does it have to stay in llvm-nm.cpp? A different file for MachO stuff?

We also need check tile config register interference. Since we
don't model the config register we should check interference from the
ldtilecfg to each tile data register def.

      ldtilecfg
      /       \
    BB1      BB2
     /         \
    call       BB3
    /           \
%1=tileload   %2=tilezero

We can start from the instruction of each tile def, and backward to
ldtilecfg. If there is any call instruction, and tile data register is
not preserved, we should insert ldtilecfg after the call instruction.

In D94418#2497746, @thopre wrote:

Could you explain me what does the test you added do? I'm not familiar with lnt runtest and its option nor with all the lit thingy.

In D94568#2495052, @kito-cheng wrote:

RVB 0.93 is an awkward version to me, there is mnemonic conflict which is not resolved during release process since it's kind of too rush, the conflict one is bext in zbe and zbs...

However it's also a milestone for B-ext, since this version claim zba, zbb and zbc is frozen, maybe those 3 sub-ext. could be removed from the umbrella of -menable-experimental-extension ?

@asb What do you think about this?

In D94624#2497992, @steveire wrote:

Can we make using color the default too? We already use colors for Decls I think, so this just adds colors for other Node types.

Does it have to stay in llvm-nm.cpp? A different file for MachO stuff?