This is an archive of the discontinued LLVM Phabricator instance.

AMDGPU: Add skeleton GlobalIsel implementation
ClosedPublic

Authored by tstellarAMD on Apr 13 2016, 2:46 PM.

Details

Summary

This adds the necessary target code to be able to run the ir translator.
Lowering function arguments and returns is a nop and there is no support
for RegBankSelect.

Diff Detail

Event Timeline

tstellarAMD retitled this revision from to AMDGPU: Add skeleton GlobalIsel implementation.
tstellarAMD updated this object.
tstellarAMD added reviewers: arsenm, qcolombet.
tstellarAMD added a subscriber: llvm-commits.
arsenm added inline comments.Apr 13 2016, 2:53 PM
lib/Target/AMDGPU/AMDGPUCallLowering.h
1

Missing -*- C++ -*-

15

Missing _H

28–29

Should probably update these to use the current start with lowercase naming convention while it's new

lib/Target/AMDGPU/AMDGPUSubtarget.h
112

Typo onwership, although it seems like this comment would only go on the base class

lib/Target/AMDGPU/CMakeLists.txt
19

Weird indent

Address some code review comments.

tstellarAMD marked 3 inline comments as done.Apr 13 2016, 3:51 PM
tstellarAMD added inline comments.
lib/Target/AMDGPU/AMDGPUCallLowering.h
28–29

Ok, I can try to fix up the parent in a new class.

lib/Target/AMDGPU/AMDGPUSubtarget.h
112

I dropped this comment.

arsenm accepted this revision.Apr 13 2016, 3:54 PM
arsenm edited edge metadata.

LGTM

This revision is now accepted and ready to land.Apr 13 2016, 3:54 PM
qcolombet edited edge metadata.Apr 13 2016, 4:39 PM

Hi Tom,

Mostly LGTM, but I do not see AMDGPUGISelAccessor.h in the diff.

Cheers,
-Quentin

lib/Target/AMDGPU/AMDGPUCallLowering.h
30–31

By all means, fix that in the base class :).

PS: Side question, since I see you used the GISelAccessor trick, would it make sense that we provide the basic GISelAccessor class in GlobalISel header?

tstellarAMD edited edge metadata.

Update patch after making some changes to core GlobalISel.

qcolombet accepted this revision.Apr 14 2016, 9:44 AM
qcolombet edited edge metadata.

LGTM.

This revision was automatically updated to reflect the committed changes.