Page MenuHomePhabricator
Differential D84223

[AMDGPU] Don't combine memory intrs to v3i16
ClosedPublic
Actions

Authored by Flakebi on Jul 21 2020, 2:14 AM.

Details

Reviewers
arsenm
piotr
Commits
rG2c659082bda6: [AMDGPU] Don't combine memory intrs to v3i16
Summary

v3i16 and v3f16 currently cannot be legalized and lowered so they should
not be emitted by inst combining.

Moved the check down to still allow extracting 1 or 2 elements via the dmask.

Fixes image intrinsics being combined to return v3x16.

Diff Detail

Event Timeline

Flakebi created this revision.Jul 21 2020, 2:14 AM
Herald added a project: Restricted Project. · View Herald TranscriptJul 21 2020, 2:14 AM
Herald added subscribers: llvm-commits, kerbowa, hiraditya and 8 others. · View Herald Transcript
piotr added a comment.Jul 21 2020, 3:51 AM

Looks good to me as a stop-gap solution.

Harbormaster failed remote builds in B65044: Diff 279451!Jul 21 2020, 4:03 AM
arsenm added a comment.Jul 21 2020, 8:09 AM

Is there any real obstacle to handling these? Even the DAG has v3i16/v3f16 types now

Flakebi added a comment.Jul 21 2020, 9:27 AM

I’m also trying to get it working properly (currently for SDag). I think I got the legalization/widening part working but I’m still trying to figure out how to select the right instruction patterns.

The next two weeks I’m on vacation, so it will still take a while. I think Marek wants a slightly quicker fix, probably something in mesa hit this.

arsenm accepted this revision.Jul 21 2020, 10:01 AM

LGTM but I think we should stop hacking around these

This revision is now accepted and ready to land.Jul 21 2020, 10:01 AM
Closed by commit rG2c659082bda6: [AMDGPU] Don't combine memory intrs to v3i16 (authored by sebastian-ne). · Explain WhyJul 22 2020, 3:44 AM
This revision was automatically updated to reflect the committed changes.
mareko added a subscriber: mareko.Jul 29 2020, 2:57 AM

Can you please cherry-pick this to the LLVM 11 branch?

arsenm added a comment.Jul 29 2020, 8:23 AM
In D84223#2181158, @mareko wrote:

Can you please cherry-pick this to the LLVM 11 branch?

https://bugs.llvm.org/show_bug.cgi?id=46893

Revision Contents
Changeset List

PathSize
llvm/
lib/
Transforms/
InstCombine/
11 lines
test/
Transforms/
InstCombine/
AMDGPU/
58 lines

Diff 279757

View Options

llvm/lib/Transforms/InstCombine/InstCombineSimplifyDemanded.cpp

Loading...
View Options

llvm/test/Transforms/InstCombine/AMDGPU/amdgcn-demanded-vector-elts.ll

Loading...