This is an archive of the discontinued LLVM Phabricator instance.

AMDGPU/R600: Add PatFrags for selecting the correct vtx id for loads
ClosedPublic

Authored by tstellarAMD on Jun 24 2016, 6:17 PM.

Diff Detail

Event Timeline

tstellarAMD retitled this revision from to AMDGPU/R600: Add PatFrags for selecting the correct vtx id for loads.
tstellarAMD updated this object.
tstellarAMD added reviewers: jvesely, arsenm.
tstellarAMD added a subscriber: llvm-commits.
arsenm accepted this revision.Jun 24 2016, 6:38 PM
arsenm edited edge metadata.

LGTM, but I still thinking checking GetUnderlyingObject for selection is broken. It should at least check for it returning null which is common

This revision is now accepted and ready to land.Jun 24 2016, 6:38 PM
jvesely accepted this revision.Jun 25 2016, 1:42 PM
jvesely edited edge metadata.

LGTM, but I still thinking checking GetUnderlyingObject for selection is broken. It should at least check for it returning null which is common

What are the problems with GetUnderLyingObject? Don't CLC restrictions shield us from potentially failing cases (the only objects are initialized in constant AS)?

with or without renaming the instructions LGTM.

lib/Target/AMDGPU/EvergreenInstructions.td
239

I think it'd also make sense to rename the instructions:
VTX_READ_GLOBAL -> VTX_READ_ID1. so the vtx to AS assignment is reduced to one place (the pattern).

LGTM, but I still thinking checking GetUnderlyingObject for selection is broken. It should at least check for it returning null which is common

What are the problems with GetUnderLyingObject? Don't CLC restrictions shield us from potentially failing cases (the only objects are initialized in constant AS)?

with or without renaming the instructions LGTM.

You could loop over them and have an unanalyzed select or phi on the pointer which will fail to find the object. It also has a search depth lint, and it's a pretty heavyweight check for a select pattern

Checking IR values in the DAG also doesn't seem like the best idea although AA does

Checking IR values in the DAG also doesn't seem like the best idea although AA does

OK, thanks. I guess the solution for r600 is:
move rodata out of code segment.
introduce constant AS pool in mesa, make sure that rodata is at the beginning of that pool (also takes care of NULL address for user buffers)
allocate __constant user buffers from the new pool.

This revision was automatically updated to reflect the committed changes.