I may not quite understand what the verifier is looking for here, but rather than trying to undo the optimization in 2 different places, would it be possible to implement this by having the InstructionSelector emit some kind of Pseudo Instruction for inputs to the comparison? For example, what would normally be selected to:

• Add quotes around ${CMAKE_CURRENT_SOURCE_DIR}

• Fix indentation.
• Remove placeholder AttributeReference.rst. The install target was replacing the generated AttributeReference.rst with this which caused the html page to be empty.
• Tested with stand-alone clang builds: cmake ../clang -G Ninja -DCLANG_INCLUDE_DOCS=ON -DLLVM_BUILD_DOCS=ON -DLLVM_ENABLE_SPHINX=ON -DSPHINX_WARNINGS_AS_ERRORS=OFF

+ Remove InstsToMove from CombineInfo struct
+ Address other review comments.

Rebase.

There is no plan for a 9.0.2 release.

LGTM.

Rebase and switch to using default visibility on Mac OS.

In D68520#1782387, @tstellar wrote:

After thinking about this more, do we want to even try to support the BUILD_SHARED=ON + CLANG_LINK_CLANG_DYLIB=ON configuration? We don't support this in llvm.

After thinking about this more, do we want to even try to support the BUILD_SHARED=ON + CLANG_LINK_CLANG_DYLIB=ON configuration? We don't support this in llvm.

In D59780#1778863, @ruiu wrote:

Hmm, I'm sorry but I'm confused. IIRC I had a discussion in the LLVM dev meeting that we were going to submit a change with a single PLT scheme rather than IPLT, so when I said that I'm going to submit a patch, I meant that I'm going to submit a patch for the 1PLT scheme rather than the 2PLT scheme. But this is for the 2PLT scheme. This is not something I want.

In D65966#1701129, @nhaehnle wrote:

I think the code would benefit from the refactoring I've mentioned on the other patch, where the lists only hold a structure with information on a single instruction. Maybe call it CandidateInfo (information of one instruction, persistent in lists) vs. CombineInfo (information on a pair, only temporary).

Rebase on master and update for a recent refactoring.

Use correct checks for Windows.

In D65961#1766760, @foad wrote:

In D65961#1765026, @tstellar wrote:

In D65961#1763143, @foad wrote:

Hi @tstellar, I'm looking into a case where this patch slowed down a shader by 10%. Before I go too far, was this patch supposed to change the behaviour at all, or was it supposed to be purely a compile time improvement?

The intention was to not change the behavior at all.

In the case I'm looking at it seems to do the same amount of load merging as before, but the merged loads are inserted at different places in the basic block.

Do you have a MIR or .ll dump of the shader I could look at ? Also, does https://reviews.llvm.org/D65966 help?

P8173 is a MIR test case. See the RUN line for how to run it. I see significant differences in the placing of the merged BUFFER_LOAD instructions with/without D65961 (or before/after it was committed).

I tried applying D65966 on top of rGe6f51713054f but it made no difference to the output.

Also generate a tarball with all sub-project sources.

Ping.

In D67463#1769557, @rnk wrote:

In D67463#1769238, @tstellar wrote:

In D67463#1767919, @rnk wrote:

@hans, are we still accepting 9.0.1 patches? I thought we'd already made a release candidate.

I'm still accepting patches until rc2 (which should have been Monday, but was delayed), so I can pull it if you think it is important.

You know, it's a significant change in behavior. This will cause hard errors for users of -fms-extensions who do not use -fms-compatibility. Maybe it's better to release that changes in the 10.0 release.

In D67463#1767919, @rnk wrote:

@hans, are we still accepting 9.0.1 patches? I thought we'd already made a release candidate.

In rG7b86188b50bf6e537fe98b326f258fbd23108b83#856097, @xbolva00 wrote:

In rG7b86188b50bf6e537fe98b326f258fbd23108b83#855776, @tstellar wrote:

I had to revert this in 3c5142597a451a03db21c2ffe8f6520c7eacce59 due to some bot failures. Some parts of the llvm code base hit this warning and fail to build with -Werror.

This is kinda strange, the LLVM code is already passing with gcc's -Wmisleading-indention.

Can you show us a log?

In D65961#1763143, @foad wrote:

Hi @tstellar, I'm looking into a case where this patch slowed down a shader by 10%. Before I go too far, was this patch supposed to change the behaviour at all, or was it supposed to be purely a compile time improvement?

Reverted in 0e12815566b2f8dd2d3bfe2319e55b3ffb9767ae.

I had to revert this in 3c5142597a451a03db21c2ffe8f6520c7eacce59 due to some bot failures. Some parts of the llvm code base hit this warning and fail to build with -Werror.

I'm planning to revert this patch along with the zorg patch, and then we can discussion the next steps.

In rG9353c5dd0664ea444236e527bf93566e11dc34df#855624, @nemanjai wrote:

In rG9353c5dd0664ea444236e527bf93566e11dc34df#855593, @xbolva00 wrote:

Disabled -Werror on sanitizer bots.

https://reviews.llvm.org/rZORGb0ee3a60875b0bed660cd52ddaf19fb19867b6ea

Ping. All the prerequisite patches have been committed now.

Ping.

Should we backport this to the release/9.x branch?

In D54439#1744353, @rnk wrote:

In D54439#1744319, @tstellar wrote:

I've confirmed that this change does fix the warning. How do we we want to proceed with this patch? The warnings will still be present on existing Mac OS installs. Should we disable this change on Mac OS for now or should we keep it and live with the warnings?

I'd be in favor of letting Mac use default visibility for now. One way to make this more palatable would be to have a CMake option that allows users to override the default and get hidden visibility if they really want it.

Address some review comments.