Build bot was green only when I scheduled older revisons to find a root
cause.

• msg-2958-164.txt163 BDownload

(as you already noticed this was approved).

Could we hold off pushing this forwards please for a couple of days? Easter
weekend is time off work for many people.

Sure—please let me know the email address and author name to I use for attribution!

Same. I am happy for you to commit your patch.

Thanks, Johannes, for providing a fix to get around this problem.

Thanks for the heads up. I've landed
https://github.com/llvm/llvm-project/commit/97184ab99c46e35ae94f828ee90f5d6af2c47e11
which should have addressed that issue, but now the bot is failing with a
different issue that looks like a stale output. I'm still looking into it,
but if I cannot find a culprit quickly, I'll revert the change.

As a drive-by comment, I'll add that MDTuple was originally meant to be a final version of MDNode, as in if isa<MDTuple> returns true then you know you have a standard metadata tuple, and not some other special type. Inheriting from MDNode sounds more in line with the original intent to me. (I'm not sure it's important to maintain that as an invariant, but thought I'd mention it.)

Point your worker to the port 9994, instead of 9990. Everything is the same
as for the production bot.

• msg-20731-208.txt163 BDownload

ahh, naming is hard. I was thinking of required to run. Yes, it could also be required pass which is misleading. I'll go with BeforePassIsNotSkippedFunc which is not ambiguous.

Thanks for sending this!
I think fs::real_path isn't a great precedent to be following here, but
that we can put the same functionality behind a slightly different API.

• msg-17782-346.txt162 BDownload

For dotest style tests, the debug format to test is chosen by the test driver. All the tests should run with any format, but sometimes there are bugs in one reader or another (or in one version of DWARF or another) so you can skip or xfail a test based on format. Sounds like this test should be skipped when the debug format is PDB, since it seems like either the PDB format doesn't express a way to find the actual size of the vla (or the current PDB reader doesn't process those records).

2. You are using Vector Function ABI mangled names, which means you ware assuming to link agains libsleefgnuabi.so instead of libsleef.so. For this reason, I think it is worth to replace the -fveclib=SLEEF option with -fveclib=SLEEFGNUABI. This request applies to all places where you have used SLEEF.

Sorry but I wholeheartedly disagree with this idea.

For starters, saying -fveclib=SLEEFGNUABI is just ugly IMO.

Furthermore, I do not understand why this change is necessary. It provides nothing that the current naming convention doesn't already provide. So, really, it's a matter of personal preference.

Ugh, I put absolute paths in the test. I can fix it in a few hours, i had
to leave tge office early. otherwise you’re free to revert, sorry :-/

Alex - any update on this?

Sam Parker

Should be fixed by https://reviews.llvm.org/D52183 https://reviews.llvm.org/D52183

The symptoms of a collision are just going to be you can’t debug the
program, so not very severe imo, especially since it would almost certainly
be resolved on the next incremental build

That’s fine

I think this particular problem was fixed a long time ago, likely
before I started working on SCEV. I think the relevant bit is here:
https://github.com/llvm-mirror/llvm/blob/master/lib/Analysis/ScalarEvolution.cpp#L6451

Lgtm

Yea this approach is fine I guess, but I’m not really a fan of gigantic
headers, so if it’s possible to use runtime polymorphism that seems better
to me. Otoh, I’m not the maintainer of the itanium demangler, so feel free
to ignore :)

Yea, I agree it’s mostly bugfixes. My hypothesis is that bugfixes are, on
average, pretty small and isolated, so it shouldn’t be too hard to port one
between the two even if they evolve separately. I’m curious what Erik
thinks though.

(Yep)

Or just doing nothing. It’s a private header and is only included in 1 TU.
In that fine, cxxabi-config is already included first

Il 18/07/2018 20:52, David Li via Phabricator ha scritto:

Hi Brian

The default is actually true on Windows I think. I know I always have to
manually set false.

• msg-839-131.txt162 BDownload

I attached my test cases to this email.

Don't wait on me until June 10th. I'm just skimming emails.

• msg-12284-175.txt162 BDownload

• msg-29308-243.txt162 BDownload

Currently llvm-mca doesn't know how to resolve variant scheduling classes.
This problem mostly affects the ARM target.
This has been reported here: https://bugs.llvm.org/show_bug.cgi?id=36672

Sorry, i'll stare at this again.

• msg-30867-326.txt162 BDownload

The MIR-Canon changes I've got coming soon will test createVirtualRegister but not createGenericVirtualRegister. It should be good enough for now I think.

Could this bit be in a subsequent patch?
Right now im not exactly sure what the right way to go about this is because you have a vreginfos but they are named in much the way physregs are named. The named physregs have the map in MIParser. Still thinking of the right way to do this.

• msg-24322-420.txt162 BDownload

Thanks for fixing that!

• msg-28320-371.txt162 BDownload

• msg-8842-58.txt162 BDownload

Scott Linder via Phabricator <reviews@reviews.llvm.org> writes:

Is this from a bot? A bit more info would be helpful.

• msg-22155-3.txt162 BDownload

Committed in r274912.

anna marked an inline comment as done.
anna added a comment.

In http://reviews.llvm.org/D22001#474590, @dexonsmith wrote:

I don't see a patch on the list. Is this a problem with how you created the Phab review?

Sent patch separately to @dexonsmith. Response on patch:
isa works on references as well as pointers, so this can just be

if (isa<FenceInst>(*BBI))

Agree with Duncan, but the llvm ref states that this implicit conversion comes at a cost, and at some point in time, may be reverted to mean what standard iterators are. So, I'll leave it as-is to avoid any fix-ups required in future.

From: Rui Ueyama [ruiu@google.com]
Sent: 06 July 2016 18:24
To: reviews+D21779+public+dea87c23f5e22381@reviews.llvm.org
Cc: Davide Italiano; Rafael Ávila de Espíndola; Daniel Sanders; George Rimar; Sean Silva; Peter Collingbourne; llvm-commits; Tim Amini Golling
Subject: Re: [PATCH] D21779: [LTO] Infer EKind/EMachine from Bitcode files

On Tue, Jul 5, 2016 at 6:33 AM, Daniel Sanders <daniel.sanders@imgtec.com<mailto:daniel.sanders@imgtec.com>> wrote:
dsanders added a subscriber: dsanders.

Comment at: lld/trunk/ELF/InputFiles.cpp:559-562
@@ +558,6 @@
+ Triple TheTriple(TripleStr);
+ bool Is64Bits = TheTriple.isArch64Bit();
+ if (TheTriple.isLittleEndian())
+ return Is64Bits ? ELF64LEKind : ELF32LEKind;
+ return Is64Bits ? ELF64BEKind : ELF32BEKind;

+}

Sorry for the late comment but Triple::isArch64Bit() doesn't correspond to ELF64 or ELF32 on MIPS. In particular, the triple used for the
N32 ABI will return true for isArch64Bit() but this ABI uses ELF32 objects. If lld doesn't support the N32 ABI yet then the current code is
ok for the remaining two ABI's (O32 and N64).

Ah, interesting. So N32 is a 32-bit ABI on 64-bit MIPS just like x32 ABI is on x86-64. Do you know how to determine whether it is ELF32 or ELF64 for such triple?

You can mark the dependency in phabricator explicitly.

Hi Jonas,

LGTM.

If you don't mind, I'll add the test as a separate patch - we don't only want

>this specific test, we need tests for a bunch of sexts/zexts, like ARM has.
>

[Demikhovsky, Elena] yes, you can add tests later. You can look at this revision as a test reference for X86

http://reviews.llvm.org/D15604

Intel Israel (74) Limited

Should we add overloads of the UTF8 conversion functions that accept
wstrings?

lemon is mostly stable for me as well, but from time to time it has huge outliers (just speculating but maybe iOS throttles too much forking in some circumstances and that benchmark is at the edge of such throttling...)

Sounds reasonable, but why not return a SubtargetFeatures?
dsanders created this revision.
dsanders added a subscriber: llvm-commits.
Herald added subscribers: sdardis, dsanders.

Right, my idea was to make loop-rotation more generic, thus enabling it on more complicated loops. It has some interesting corner cases though: for instance you can end up with several nested loops after rotating a single loop, but that's details, which we'll figure out in the process:)

Sorry, though i clicked accept, i'll do so.
please commit.

On second thought i think i can actually make that work. I was thinking
you'd want a full blown interval tree, but it could be done using the same
walk I'm doing for writing. Since this is still a rare case, I'm fine
adding this, or not adding it, depending on what people prefer

llvm-commits mailing list
llvm-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits

The renaming also makes the diff hard to read which I don't like either. I
will revert the renaming part.

To elaborate a bit here: what's happening is that the update the list sees
is that you added the list and some reviewers, rather than an initial email
that contains the patch and your description of it so that people can scan
and look at patches without needing to go to phabricator.

I wonder if that's the right long term solution?

llvm-commits mailing list
llvm-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits

Although I like the change, I proposed something similar in http://lists.llvm.org/pipermail/llvm-dev/2016-May/099752.html and it was rejected.

Yeah, i don't either.
At this point, why don't we just have it own the caching walker and see
what it breaks.

I know some of the issues are more a matter of personal preference than there being one true way. I just felt I should point out how I feel about them before people start writing them down as the one true way in the coding conventions :) I will of course not block the patches if we end up with a different style here.

• msg-475-150.txt162 BDownload

You should probably note somewhere that assume is, in fact, readnone, we
just don't model it that way right now because it hasn't been changed to
use a real control dependency instead of a fake memory one.

This seems strange to me. For example, it breaks the otherwise very convenient:

$ clang -flto t.c -mllvm -some-internal-option-for-cc1

ping

llvm-commits mailing list
llvm-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits

Commit message typo: "depricated" should be "deprecated". Probably also
missing a "we" before [or use "it is" instead of] "are" in the last line.

Can you investigate? So that we can have a correct implementation to propose instead.

Thanks for taking on this work!

I am fine with the change as is now after realizing that it does not change the behaviour of the subregister def case. So this change is fine!

+Fred

LGTM.

Hi Simon,

I have no problem with the suggestion for the body, but please leave the
PR# bit in the subject. The PR is the least useful part of any subject
to me and I don't want to devote more space to it.

The cpio command was dropped, but the format is still documented. The pax
command has to support it.

Comment at: ELF/DriverUtils.cpp:130
@@ +129,3 @@
+ OS << "000000"; c_dev
+ OS << "000000"; c_ino // FIXME: should be unique

+ OS << "100664"; // c_mode: C_ISREG | rw-rw-r--

If you are not going to fix it, it is not a fixme. Please change the comment to say that this should be unique, but since no one cares, we don't bother to set a unique number.

Looking again, sorry for the long delay.

• msg-7357-55.txt162 BDownload

I link Polly into my tools anyway, thus I'm in favor of changing the
default.

Hi Evandro,

• msg-5793-513.txt162 BDownload

Yes, I had already kicked off a build with your patches. Unfortunately it does not (at least no significantly) fix the issue and the test case is still running (for about 30 min now). My fix brings CT down to 2 min on an RA compiler.

the only code change is

I'm still vaguely thinking we should be doing this in the frontend instead.

This looks fine to me.

If at all possible my preference would be:

• Require PRODIVE to be in an unambiguous location: inside an

OutputSection definition.

• Produce DefinedSynthetic symbols for them so that the section is set

correctly.

Cheers,
Rafael

I like this. I think you're missing updates to ValueMapper.cpp, so
llvm-link will crash if you leave this as is. For the code there,
it would be best to treat DITypeIndex exactly like MDString.

Hi all,

What about the subtargets themselves?

It should already be fixed with r266151

• signature.asc219 BDownload
  • msg-30043-152.txt162 BDownload

I'll get to the rest of it, but for this Kit's question about O2: It's
preferred to test such things in the back end.

• msg-15314-33.txt162 BDownload