1. Simplify return stmt in LoopAccessInfoManager::invalidate according to feedback.
2. Use a separate RUN line in the test case to verify invalidata<access-info>.
3. Reduce amount of CHECK:s in the test case. Don't think we need to verify what is going on with all other analyses. Better to make the CHECK:s focus on what is important for each sub-test.

I have no objections about doing this revert.

In the description you have written "The copy operation with a predicated dependency is currently marked with an implicit operand". I don't really understand what that is referring to. Is that something AMDGPU specific, or what is this referring to?

In D143754#4132515, @cdevadas wrote:

The IsPredicable field will always be turned to 1 for Pred_Copy to indicate that it is the Predicated version of the copy.

I actually wanted to have the Pred_Copy without these implicit operands. We should be able to model the dependency using an additional operand.
But I don't know how to model this operand and hence decided to continue using the implicit operand itself.

May be an immediate operand that takes const values (0, 1, 2, etc.)?
The immediate values can be mapped to enumerations defined via target files to represent these predicated dependencies.
When there is no dependency, use the value 0. It means, there is no dependency and can change to the regular COPY.
If required, the Pred_Copy with Imm_opnd value 0 can also be modeled for the "predicate is false" scenario.

For example. the following can be defined for AMDGPU target in llvm/lib/Target/AMDGPU/SIDefines.h

enum Copy_Predication {

None = 0,    // No dependency. It is for SGPR copies. At some point, convert the instruction into regular COPY.
Lane_Copy,  // copy only the active lanes of VGPRs.
Wave_Copy // copy all lanes of VGPRs

}

Any thoughts?

Had to revert this due to lots of buildbots failing when linking bugpoint.
Typically like this:

In D143388#4107271, @aeubanks wrote:

can we delete the legacy pass?

Simplified code based on review feedback.

Gentle ping!

In D118943#4023264, @RKSimon wrote:

@bjope This has been identified as the cause of a major compile time regression - https://github.com/llvm/llvm-project/issues/59800 - please can you take a look?

I do not plan to give any review feedback at the moment. I've only touched this for legacy PM deprecation and I don't know much about the implementation details.

LGTM

In D138452#3976219, @fhahn wrote:

In D138452#3974418, @bjope wrote:

Ok, here is a godbolt repro: https://llvm.godbolt.org/z/98PvGaTe9

The difference is that the branch in the entry block now looks like:

br i1 %0, label %trap, label %1

Thanks, interestingly this doesn't seem to crash on macOS, but I was able to reproduce on Linux. Should be fixed in 9eda78107c4d

Ok, here is a godbolt repro: https://llvm.godbolt.org/z/98PvGaTe9

! In D138452#3974119, @fhahn wrote:

Thanks for the report. Is it possible this is over-reduced? I cannot reproduce it locally or on godbolt: https://llvm.godbolt.org/z/T8f1q33hq

Got a benchmark downstream were we no longer get SLP vectorization after the canonicalizatoin. But the test is rather large and complicated so I do not have anything that show the full pipeline including SLP (at least not yet).

In D137796#3921661, @aeubanks wrote:

could I have a couple days until submitting this? we have one more internal usage of this

Splitted out removal of adjustPassManager.

Added lit test.

In D137626#3915570, @aeubanks wrote:

we should add a check in opt.cpp that we don't mix -O# with other passes (there's already a similar check for the new PM)

Just a side not here.

(Only concern is that -p used to mean something else. So we are re-introducing a short-option for something new. But I doubt that it will cause much confusion.)

This currently depends on D136616 since it refer to the -p option. I think it looks good otherwise.

I guess we either want to do this (changing codegen to match the code comments). Or the other way around (changing code comments to match with codegen) as in D135574.