Page MenuHomePhabricator

[AMDGPU][MC][GFX9] Added s_scratch* instructions
ClosedPublic

Authored by dp on Mar 23 2018, 9:10 AM.

Details

Summary

See bug 36836: https://bugs.llvm.org/show_bug.cgi?id=36836

Syntax of "scratch" instructions is identical to that of s_load/s_store.
The only difference is in semantics. “Scratch” variants multiply offset from SGPR by 64 (wavefront size).

Diff Detail

Event Timeline

dp created this revision.Mar 23 2018, 9:10 AM
dp updated this revision to Diff 139605.Mar 23 2018, 9:18 AM

Added missing test for s_scratch_store_dwordx4

arsenm added inline comments.Mar 26 2018, 7:37 AM
lib/Target/AMDGPU/SMInstructions.td
216

This should use HasFlatScratchInsts instead of isGFX9

dp added inline comments.Mar 26 2018, 8:45 AM
lib/Target/AMDGPU/SMInstructions.td
216

Thanks, I'll update the patch.

Just out of curiosity, is this because s_scratch* were added to be used together with FLAT "scratch" opcodes?
In other words, is this because they are a part of the same design?

Other than that I do not see how "s_scratch" relate to FLAT "scratch" opcodes.
Note that FLAT "scratch" opcodes use flat_scratch address while "s_scratch" do not.
I believe "s_scratch" opcodes require no additional HW compared with s_load/s_store.

dp updated this revision to Diff 139801.Mar 26 2018, 9:10 AM

Corrected as suggested by Matt.

arsenm added inline comments.Mar 26 2018, 9:28 AM
lib/Target/AMDGPU/SMInstructions.td
216

I thought this was for accessing the same scratch location without add tid enabled. Is that not the case? What are these for then?

dp added inline comments.Mar 26 2018, 10:20 AM
lib/Target/AMDGPU/SMInstructions.td
216

Public documentation states that s_scratch address calculation uses nether flat_scratch nor address swizzle:

ADDR = SGPR[base] + inst_offset + { M0 or SGPR[offset] or zero } * 64

However it looks like I was wrong about flat_scratch. Looking through some other AMD documentation I found a confirmation that s_scratch opcodes do use flat_scratch instead of SGPR[base] which is ignored.

That means that flat_scratch shall be added to uses, correct?

arsenm added inline comments.Mar 26 2018, 10:22 AM
lib/Target/AMDGPU/SMInstructions.td
216

Yes, it needs a flat_scratch use then

dp updated this revision to Diff 139816.Mar 26 2018, 10:48 AM

Updated after a discussion with Matt:

  • added flat_scratch to uses.
arsenm accepted this revision.Mar 26 2018, 4:17 PM

LGTM

This revision is now accepted and ready to land.Mar 26 2018, 4:17 PM
Closed by commit rL328704: [AMDGPU][MC][GFX9] Added s_scratch* instructions (authored by dpreobra, committed by ). · Explain WhyMar 28 2018, 7:12 AM
This revision was automatically updated to reflect the committed changes.