Ping

Ping @rsmith

Ping @rsmith

Should be backported to 9.0 branch?

It would be nice to merge this fix into 9.0 branch after some time in trunk. What do you think?

(should be probably reverted from 9.0 branch)

Can you provide benchmark results from "public" ones like SPEC or LLVM test-suite? I think this should be mandatory to land performance critical patches (always provide results from SPEC/test suite).

Fixes https://bugs.llvm.org/show_bug.cgi?id=42147 ?

Test?

Added multi-use tests.

Thanks, I think this is fine solution for now.

Looks fine

Revert unneeded change.

Probably @aaron.ballman would like to review this too..

Updated.

Thanks. I wanted to land it for you, but since I dont use monorepo now, it does not apply well for me.

Check if prefix is unique.

FileCheck says
"Prefixes must be unique, *start with a letter* and contain only alphanumeric characters, hyphens and underscores"

Handle empty prefixes.

@gbedwell Thanks for ideas!

Reworked to use regex to check prefixes.

b) yes, one more test never hurts
c) e.g.; broken precondition, missing 'fast' ?

Not sure how other reviewers, but I consider the use of 'C' as confusing here, since 'C' is usually a constant. I would recommend to avoid 'C', if it is not a constant.

I think we need a few more tests here

Any futher comments?

Please rebase

In D64666#1583667, @ziangwan wrote:

In D64666#1583633, @xbolva00 wrote:

You can check also https://reviews.llvm.org/D52835. I hit there issue which I didn't know how to solve.

Hi David,

Can you elaborate what issues you encountered? Thank you.

You can check also https://reviews.llvm.org/D52835. I hit there issue which I didn't know how to solve.

Add checker to all update scripts.

In D64285#1583596, @craig.topper wrote:

In D64285#1583589, @xbolva00 wrote:

if (Bitwidth == 0) {

  IC->dump();
}

%cmp.i = icmp ne %struct.LIST_HELP* %List.addr.0, null

Yep, its a pointer. Just disable the transform if !CmpRHs->getType()->isIntOrIntVectorTy()

In D64285#1583581, @craig.topper wrote:

getScalarSizeInBits probably returns 0 for pointers. Only DataLayout knows pointer size.

%cmp.i = icmp ne %struct.LIST_HELP* %List.addr.0, null

In D60761#1470317, @luke wrote:

Hopefully not!
Also just a heads up, I don't have commit access to land this

Also check IR.

In D64589#1582936, @RKSimon wrote:

is there anyway that we could move this to common.py so we can reuse it in the other update scripts?

Moved to common.

What about CTMark? SPEC?

Renamed variable.

Improved.

In D64589#1581499, @lebedev.ri wrote:

I'm not confident that will do the right thing. (i expect this to have some false-negatives, iff it is allowed to specify -check-prefix= more than once)
I see two paths:

1. split the runline by whitespace, and check every element that starts with -check-prefix= that it does not contain ,. Will not have false-positives and no false-negatives, but kinda ugly.
2. Revert to the previous diff, complain if there are any -check-prefix= IFF we had more than a single prefix; the warning will need to be changed - it will have false-positives - so it is more of a stylistic guideline.

^ This won't be detected, not sure if worth to match. What do you think?

In D64589#1581406, @lebedev.ri wrote:

Thank you, i have stumbled into this a few times :)

To be noted, the problem is: --check-prefix=A,B,C.
But i'm not sure about --check-prefix=A --check-prefix=B --check-prefix=C.

More precise checker.

lebedev.ri hit this recently so I added him as reviewer.

Thank you all!

It should be fine now.

Fixes

Maybe @spatel would like to leave a comment too?

Updated..

(yeah, I saw this missed optimization on the rust github issue tracker)

In D64572#1580897, @jdoerfert wrote:

LGTM.

Improve code, emit warning instead of error.