This is an archive of the discontinued LLVM Phabricator instance.

[AMDGPU][MC][GFX8+] Added syntactic sugar for 'vgpr index' operand of instructions s_set_gpr_idx_on and s_set_gpr_idx_mode
ClosedPublic

Authored by dp on Feb 15 2019, 9:26 AM.

Diff Detail

Event Timeline

dp created this revision.Feb 15 2019, 9:26 AM
arsenm added inline comments.Feb 15 2019, 9:30 AM
lib/Target/AMDGPU/InstPrinter/AMDGPUInstPrinter.cpp
948

Why can't you just write to O directly without the intermediate string?

dp marked an inline comment as done.Feb 15 2019, 10:48 AM
dp added inline comments.
lib/Target/AMDGPU/InstPrinter/AMDGPUInstPrinter.cpp
948

The last comma in the list of modifiers should not be emitted, otherwise we get something like this:

gpr_idx(SRC1,)

I found no simpler logic to handle this problem. Any ideas?

arsenm added inline comments.Feb 21 2019, 6:32 AM
lib/Target/AMDGPU/InstPrinter/AMDGPUInstPrinter.cpp
948

Something like

bool Comma = false;
for (unsigne Test: {src0, src1...}) {
if (!(Val & Test)
  continue

  if (Comma)
    OS << ',';
  OS << srcName(Test);
  Comma = true;
}
dp added a comment.Feb 21 2019, 8:10 AM

Thanks, Matt! Sorry for my dumbness :-)

This revision is now accepted and ready to land.Feb 21 2019, 11:15 AM
dp updated this revision to Diff 188146.Feb 25 2019, 5:39 AM

Updated as suggested by Matt

arsenm accepted this revision.Feb 25 2019, 7:24 AM

LGTM

lib/Target/AMDGPU/InstPrinter/AMDGPUInstPrinter.cpp
967

Single quotes

972

Single quotes

dp marked an inline comment as done.EditedFeb 27 2019, 3:33 AM

Typos have been corrected. Thanks!

This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptFeb 27 2019, 5:11 AM