This is an archive of the discontinued LLVM Phabricator instance.

AMDGPU/GlobalISel: Legalize non-a16 non-NSA images
ClosedPublic

Authored by arsenm on Jan 26 2020, 8:08 PM.

Details

Diff Detail

Event Timeline

arsenm created this revision.Jan 26 2020, 8:08 PM
arsenm updated this revision to Diff 240473.Jan 26 2020, 8:08 PM

Fix leftover junk from merge error

arsenm updated this revision to Diff 240596.Jan 27 2020, 8:09 AM

Don't try to pack a scalar address

I guess the same question applies here as to the other change: Why is this a legalization change and not an instruction selection one?

arsenm updated this revision to Diff 240867.Jan 28 2020, 7:00 AM

Need to move atomic early exit, and remove feature check since gfx10 features are broken

I guess the same question applies here as to the other change: Why is this a legalization change and not an instruction selection one?

The legalizer's job is to get instructions to have the right operand types. If we expanded this in instruction selection, we wouldn't get the combining benefit of the pack/unpack instructions (and they could be inside a waterfall loop inserted during RegBankSelect)

arsenm updated this revision to Diff 242502.Feb 4 2020, 7:03 PM

Rebase, call observer, and use wrapper instruction to avoid figuring out when it's been already legalized

arsenm updated this revision to Diff 242690.Feb 5 2020, 10:31 AM

Rebase test changes

arsenm updated this revision to Diff 242697.Feb 5 2020, 10:55 AM

Rebase test changes

Thank you, this looks good to me modulo one question.

llvm/lib/Target/AMDGPU/SIInstructions.td
2269–2285

What about atomics? They should be mayLoad = mayStore = 1, right?

If you plan to handle image atomics in a later patch, then just go ahead with this one.

arsenm accepted this revision.Mar 17 2020, 6:48 AM

If you plan to handle image atomics in a later patch, then just go ahead with this one.

Atomics are handled in D74318

This revision is now accepted and ready to land.Mar 17 2020, 6:48 AM