Trying to fix a windows bot failing test, dividing the test into multiple files and using full line matching.

In rL348121#300373, @RKSimon wrote:

In rL348121#300179, @RKSimon wrote:

@dnsampaio This broke the buildbots: http://lab.llvm.org:8011/builders/llvm-clang-x86_64-expensive-checks-win/builds/14363

Are you looking at this or should I revert?

@dnsampaio Reverted at rL348249

Fixed one comment.

NIT in the v8.4a trace tests.

Fix the tlb-rmi feature argument and test file.

Removed the register (system operand) constraint, and associated test case.
Fixed PAN, RCPC dependencies and instructions that depend on PA, not PAN.
Changed some names to be more meaningful.
Removed the prefix "has" from the features flags.
Fixed tests

Fixed:
the features implications/dependencies.
the name of the JavaScript conversion feature.
some test files.

Hi @olista01 and @bryanpkc. Thanks for the reviews.

In D54633#1306653, @olista01 wrote:

Have you discussed these feature names with the GCC devs? I know we can change the user-facing names used by clang in in TargetParser, but it's easier for us if they match.

Indeed I did not. As you said, they are not visible to the user. I'll check with the gcc devs how they have these features split and named though.

For the feature registers (ISAR6 and MMFR2), maybe we should just make these registers always available? These registers are all defined to read as all-zeroes in older architectures, which means no features described by that register are implemented. If we continue like this then we will have to add an extra feature to pretty much every new architectural feature for the ID register.

Indeed it seems the case for the ID_ISAR6_EL1 exist, so I'll just remove the constraints over it.
However, the ID_AA64MMFR2_EL1 register was added in v8.2. So I believe we should check as to warn the user if he might get undefined behavior by accessing it when not present.

Added FeatureRASv8_4 implies RAS.
NIT fix.

Ups, it was ment to be a clang patch, not a llvm one. Sry

Added blank lines at end of test files.

Re-inserted the missing tests. Small refactoring as to look alike neighbor code.

Refactoring.

Ping.

No custom decoding is required

Fixed the tests as to check boundaries, and one value in the middle.
Changed tests ordering as they generated empty lines not treated by FileCheck.

To send patch....

Removed processor and specific triple target from test.

Updated test to use llvm-mc instead of clang.

Simplified tests. Using the standard function for emitting data symbols, if required.

In D51683#1226329, @joerg wrote:

Correct. The protected name is double underscore as both suffix and prefix.

Fix test march triple.

Fixed comment explaining the elaborated growth cost-function.

Using FileCheck in test file.

Added missing test-file

Moving this pattern matching to AggressiveInstCombine following a suggestion of @lebedev.ri . Now it searches for minimal required patterns as desired by @spatel.

In D49229#1185608, @spatel wrote:

So all expensive operations have been eliminated, I do not see why it wouldn't fit in InstCombne. We detect a pattern and we reduce it.

Because the pattern that we are matching is larger than it needs to be (as the comment in the test file clearly shows - there is no 'or' in the minimal pattern). This problem of trying to make everything fit in instcombine has been discussed several times on llvm-dev in the last ~year. Eg:
http://lists.llvm.org/pipermail/llvm-dev/2017-September/117151.html

I agree that it won't handle all cases. But one will need to come with a more generic thinking as to create a new pass that handles this. Something like an abstract known bits, that tells that two values hold the same bits coming from a given instruction, or some simplification by demanded bits from the same values. It is feasible, but it is not my intention to do it so now.

Did you look at (new)-GVN to see if it fits in there?

I must confess that I did not quite understand all the work-flow of newGVN, but from what I did see, it mostly wouldn't fit. It seems to behave like InstCombine, expecting to replace the current instruction being visited. And it would require to create one value as to detect if there is a leader of that value and then reuse it. It is not that complicated, but quite awkward IMO.

If this is in instcombine (in addition to missing the pattern when there is no 'or'), I think you have to limit the transform based on uses as Roman mentioned in an earlier comment.

Added test that the inner and must be replaced by the new shift operation.
Converted the function to bool, as it does not require to create the Or operation after the replaceAll.

As we are replacing all the uses of the DeadAnd, it is not required to create and replace the visiting Or operation. Replace all uses does the job already.

Replaces all uses of the innermost and with the new shift.

Relaxed conditions for which the transformation is applied.
Added more tests for ashr.

Removed unused arguments.
Early exit.

All required values are obtained during the pattern matching.

Moved to a separated function. Placed function call after knowing more about the operands.
Added ashr case, that was being wrongly treated as lshr.
Added comments, including one that argues that this function would be useless if and instructions are move before any type of shift operations.
Using m_c_Or, and passing operands as arguments.

Did most fixes. Just don't how to capture non-leaf nodes of the pattern being matched. Using other match operations would actually be more complicated than just passing the operands as arguments to the new function, now that I already know they are AND operations due the function call placement.

Detect desired pattern from the binary operation using the results.

In D49229#1163319, @lebedev.ri wrote:

1. Please always upload all patches with the full context (-U99999)

Sorry, I thought that it being an entire function it wouldn't matter. Lesson learned.

Added context.

Using hasNUses() instead of numUses() ==

Replaced num_uses by !hasNUsesOfValue as requested.

Moved test to correct folder

Only accepts instructions with 2 uses (AND / SHIFT operations). So that looping through the uses is not expensive, and we avoid it in most cases.
Removed recursive bit value computations(computeKnownBits).

Fixed execution costs. See below.

Constrain to allow the transformation to happen only when the masked value has only 2 users (an AND and a SHIFT).
Removed value tracking operations.