I intend to merge the patch soon, based on the fact that the patch was reviewed in D70315. Compared to that version, 3 problematic cases were removed and tests updated.

Ping.

This is another attempt to trim unused components of buffer loads. Comparing to D70315, this commit does not handle formatted buffer loads, as it is not safe.

Ping.

Addressed review comments, updated var name and fixed language in a comment. Regarding the new test: the code is in the function that can only be called by SimplifyDemandedVectorElts, so only for intrinsics with vector types, not pointer type.

Ping

Widening instead of splitting.

Using the alignment from the ABI type alignment.

I investigated briefly the possibility of extending LoadStoreVectorizer, but using that pass looked tricky because, among other things, the intrinsics which I want to handle here are not instances of LoadInst for which that pass is well-suited. In fact, extending the si-load-store-opt pass seemed simpler. This patch adds support for MTBUF instructions (analogous to the existing MUBUF) and handles querying of buffer formats.

As suggested by Nicolai, I extended the patch to also handle shufflevector users in addition to extractelement. The new generalized version also simplifies the case where three extract elements are used (the previous patch only handled two). The new code also makes it easy to extend the case list even more in the future.

Added TODO.

Changed "== 16" to "!= 32".

Rebased and addressed review comments.

Fixed wrong spacing.

Rebased after D68384 has been merged.

A recent patch in SILoadOptimizer (D65496) conflicted with this patch. I have rebased it to the latest master, but for clarity I will split the review into two parts: a separate review for the NFC refactoring (D68384) and the current review only for merging MIMG instruction.

Addressed review comments and rebased.

In D67345#1688379, @nhaehnle wrote:

This is a useful change, but there is an unfortunate asymmetry here in how the code is structured: in addition to extractelement, we could also have shufflevector users (or masked stores etc.). Presumably we'd be able to handle all of those together without duplicating the code. Is there a way to take this into account?

Also, two comments inline.

In D68200#1688092, @arsenm wrote:

I thought this was a property of the resource descriptor? Why do you need to add it to the intrinsic?

Ping

Ping

Rebased and added missing 'const' in SILoadStoreOptimizer::dmasksCanBeCombined.

Reverting the patch and leaving the test only as it passes without the change in code.

Thank you for bearing with me, I have been away.

Ping

Ping

I think a generic pass would not be suited for our image instructions, due to the dmask special treatment. I had decided to extend the si-load-store-opt pass, because similar transformations were already handled there.

V2: Addressed review comments.

V5: Refactored code.

V4: Simplified the condition and test.

In D63860#1576545, @anton-afanasyev wrote:

In D63860#1574717, @piotr wrote:

In D63860#1572882, @anton-afanasyev wrote:

Hi @piotr, have you tried regression benchmarking with this patch? For instance, test suite.

Hi Anton, Yes, I have just run regression testing on the test-suite and the tests pass ("Expected Passes : 917").

Hmm, I'm talking about performance benchmarking, from llvm Test Suite: https://llvm.org/docs/TestingGuide.html#test-suite.

In D63860#1572882, @anton-afanasyev wrote:

Hi @piotr, have you tried regression benchmarking with this patch? For instance, test suite.

V3: Updating commit message.

V2: Removing condition that BB has to contain phis only. Added a MIR test.

Related to D63709.