Page MenuHomePhabricator

[AMDGPU] Extend the SI Load/Store optimizer
Needs ReviewPublic

Authored by piotr on Jul 18 2019, 3:58 AM.

Details

Reviewers
nhaehnle
Summary

Extend the SI Load/Store optimizer to merge MIMG load instructions. Handle
different flavours of image_load and image_sample instructions.

When the instructions of the same subclass differ only in dmask, merge
them and update dmask accordingly.

Diff Detail

Event Timeline

piotr created this revision.Jul 18 2019, 3:58 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 18 2019, 3:58 AM

I still think we should be handling these on the IR level

lib/Target/AMDGPU/SILoadStoreOptimizer.cpp
459–460

Can this actually, legitimately fail? I would expect this to be an assert

498

C++ style comment

1114–1115

Weird to put the assert string first

1120–1125

static const

test/CodeGen/AMDGPU/merge-image-load.mir
5

Can you include the merged MMO in the check lines

piotr updated this revision to Diff 210829.Jul 19 2019, 7:05 AM

V2: Addressed review comments.

I still think we should be handling these on the IR level

Matt, where do you think would be the right place to do this at the IR level? Presumably you have an existing pass in mind?

I still think we should be handling these on the IR level

Matt, where do you think would be the right place to do this at the IR level? Presumably you have an existing pass in mind?

Either a new pass, or teach the LoadStoreVectorizer about intrinsics. Things are generally easier in the IR, and I don't think this needs any information only available after selection

piotr added a comment.Jul 19 2019, 8:14 AM

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.

piotr updated this revision to Diff 218359.Mon, Sep 2, 6:54 AM

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