Landed in https://github.com/llvm/llvm-lnt/commit/e736349f7cca1b18e93b6a12e222db0bafcf54f5

Landed in https://github.com/llvm/llvm-lnt/commit/2e93cbd17fd5eb4b2e3773e5022186feac554d7c

Committed in https://github.com/llvm/llvm-lnt/commit/0982122b76efad15e244a18b42ce77de2a906140

LGTM

Sorry, I'm not aware of a buildbot for LNT. I did not even know there was one before.

I'm personally more enclined to prevent lit from running without lit installed because then some of the tests are skipped.

In D94418#2498606, @thopre wrote:

In D94418#2498419, @danilaml wrote:

In D94418#2498383, @thopre wrote:

In D94418#2498376, @danilaml wrote:

Sorry, pushed to the wrong branch (Phabricator/arcanist workflow is annoying >_<) and didn't notice your comment.

Hm, I've made sure that all tests pass on both py2 and py3 before submitting and I can't reproduce it on my machine.
Could you check if test_suite-profile-import.py.tmp.err and test_suite-profile-import.py.tmp.log have any suspicious errors (they should be in <..>/test_run_tmp/runtest/Output/)?

Do you use tox to run the tests?

I use lit directly (lit -svv ./tests) which what I understand tox does as well. I've rerun all tests using tox and the only test that failed was server/ui/V4Pages.py (I believe it's because I've already had an lnt instance running and that test doesn't account for that possibility).

Are you testing your patch on top of 042938b5c332fa63f1fefdd7116f39e161b8c51b? My guess is you are testing on a commit prior to f98bc1b3018248fbe551b0e7491ac5ba388216d5 which prevents tests involving profile from being run. Btw, using tox with a comparison of before/after logs allows you to spot codestyle issues added by your patch like the double glob import I pointed out.

Best regards,

Thomas

ping?

In D94418#2498383, @thopre wrote:

In D94418#2498376, @danilaml wrote:

Sorry, pushed to the wrong branch (Phabricator/arcanist workflow is annoying >_<) and didn't notice your comment.

Hm, I've made sure that all tests pass on both py2 and py3 before submitting and I can't reproduce it on my machine.
Could you check if test_suite-profile-import.py.tmp.err and test_suite-profile-import.py.tmp.log have any suspicious errors (they should be in <..>/test_run_tmp/runtest/Output/)?

Do you use tox to run the tests?

I use lit directly (lit -svv ./tests) which what I understand tox does as well. I've rerun all tests using tox and the only test that failed was server/ui/V4Pages.py (I believe it's because I've already had an lnt instance running and that test doesn't account for that possibility).

Sorry, pushed to the wrong branch (Phabricator/arcanist workflow is annoying >_<) and didn't notice your comment.

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.

Fixed tests.

Added a test.

I'll try to come up with a test case. I thought there were already tests for profile data, so I need to look into why they didn't catch it.

In D75746#2475681, @lebedev.ri wrote:

Is there no transform that can be taught to do this cleanup instead?

@TOCK
I've meant the surrounding context for the diff (https://llvm.org/docs/Phabricator.html#phabricator-request-review-web),
i.e. git show HEAD -U999999

The diffs lack context.

I've taken a liberty of adding more reviewers to hopefully help get this going.

Ping?

Rebased.
Had some time to come back to this.
I've tried implementing the following proposed way to get VTC locally:

Committed in 4c57b004f148dffc6db199890d91fc3e6e578972

Btw, it looks like lnt's .arcconfig is missing a callsign which broke moz-phab when I tried to submit using it.

In D62574#2135662, @ebevhan wrote:

It's generally not safe to alter address spaces below the top level. C is just very permissive about it.

What are the remaining roadblocks left before this patch can be merged? I'm interested in having a target-specific way to define the allowed explicit/implicit address space conversions.
Also, it appears that currently whether implicit casts between pointers of different AS are allowed is determined by the superset relations only, however https://reviews.llvm.org/D73360 introduced another (openCL-specific) variable into the mix: whether the conversion is a top level or not. IMHO, this should also be configured by the target, although I'm not sure whether it needs a separate hook (isBitcastNoop or similar?) or it needs to check the legality of the implicit casts differently.

ping

@RKSimon sadly not.

@dblaikie It was actually failing on some of the lit tests when compiler was compiled with EXPENSIVE_CHECKS enabled (kind of how I discovered it), but after this recent refactor which removed a sort step: https://github.com/llvm/llvm-project/commit/ed766f1bb1040a520fb5646ab75851e2b0fd66df#diff-7b43e801c5abbb03d571f9aad6af63a5L792
I don't know if the codepath with ::sort (which has those checks) can be hit from llvm IR. Instead, I've added an explicit check to AttributesTest.cpp that currently asserts.

Sorry for commenting on an old merge PR, but was looking through some asserts and stumbled on this code which raised some questions.

Updated revision with additional checks and rebased.

@lebedev.ri I've been successfully using a pass based on https://reviews.llvm.org/D12494 in a downstream port (although I'm not sure if it's enough to fix all ARM's regressions). I was planning to submit it for the review after some touch ups (i.e. it only uses legacy pm) but haven't found the time for that yet. I also need to evaluate whether it is still needed after D75908 (that might take some time).

ping

Test generated from the following C code:

@Carrot this seems to be reverted, but aggressive loop rotate still has mostly negative affect on performance in internal benchmarks that I've run.
Could you please elaborate on how branches taken was reduced from 2 to 1 (and how it was calculated) for the test case you mentioned in the https://reviews.llvm.org/D43256#1534855

@craig.topper this pass runs on IR. In the real code that prompted me to come up with this patch the BB block was split during ISel building phase, because (IUC) it was too big. And just so happens that the split was right between sext to i64 and mul. I've tried to simulate it with the code above.
@bjope Thanks. That might be a valid option for testing (although it's not really target specific at this point). However, it might be moot if it turns out there is no way to either 1) check whether two CopyFromRegs came from the same IR value or 2) find out in other ways that one node is just replicated sign bit of the other node.

One of the motivating examples (not sure how stable this IR to use as a test) is something like this (needs -mllvm -disable-cgp to prevent sext sinking):

define i64 @foo(i32 %a, i32 %b) local_unnamed_addr {
entry:
  %conv6 = sext i32 %a to i64
  %conv17 = sext i32 %b to i64
  %add = add i64 %conv6, 42
  %cmp0 = icmp slt i64 %conv17, %add
  br i1 %cmp0, label %exit, label %exit2 ; branch to prevent bb merging