This is an archive of the discontinued LLVM Phabricator instance.

[AMDGPU] Disassembler code refactored + error messages.
ClosedPublic

Authored by vpykhtin on Feb 29 2016, 8:04 AM.

Details

Summary

Idea behind this change is to make code shorter and as much common for all targets as possible. Let's even accept more code than is valid for a particular target, leaving it for the assembler to sort out.

64bit instructions decoding added.

Error\warning messages on unrecognized instructions operands added, InstPrinter allowed to print invalid operands helping to find invalid/unsupported code.

The change is massive and hard to compare with previous version, so it makes sense just to take a look on the new version. As a bonus, with a few TD changes following, it disassembles the majority of instructions. Currently it fully disassembles >300K binary source of some blas kernel.

Previous TODOs were saved whenever possible.

Diff Detail

Repository
rL LLVM

Event Timeline

vpykhtin updated this revision to Diff 49379.Feb 29 2016, 8:04 AM
vpykhtin retitled this revision from to [AMDGPU] Disassembler code refactored + error messages..
vpykhtin updated this object.
vpykhtin added a subscriber: nhaustov.
vpykhtin updated this object.Feb 29 2016, 8:06 AM
vpykhtin added a project: Restricted Project.
tstellarAMD accepted this revision.Feb 29 2016, 9:49 AM
tstellarAMD edited edge metadata.

LGTM.

This revision is now accepted and ready to land.Feb 29 2016, 9:49 AM
SamWot edited edge metadata.Mar 1 2016, 4:49 AM

LGTM

lib/Target/AMDGPU/InstPrinter/AMDGPUInstPrinter.cpp
375 ↗(On Diff #49379)

I'm not sure it is good idea. At least we would need to remove this later.

vpykhtin marked an inline comment as done.Mar 1 2016, 5:38 AM
vpykhtin added inline comments.
lib/Target/AMDGPU/InstPrinter/AMDGPUInstPrinter.cpp
375 ↗(On Diff #49379)

Disassembler is a hacker's tool and should allow to get as much code out of binary as it can. Nobody wants a disassebmler asserting on the first instruction with invalid operand. It might be a good idea to add diagnostic message to the error stream here.

This revision was automatically updated to reflect the committed changes.