This is an archive of the discontinued LLVM Phabricator instance.

AMDGPU: Don't put constants in .text for Mesa
AbandonedPublic

Authored by cwabbott on Aug 30 2019, 6:25 AM.

Details

Reviewers
nhaehnle
arsenm
Summary

In Mesa we'd like to start creating constant globals. These should be in
the CONSTANT address space to let us use scalar loads where possible.
However, we also have to support pasting multiple text sections
together for RadeonSI, which means that we need to have constants in a
separate .rodata section so that they can get relocated after all the
pasted-together .text sections. We don't need this for radv, but it
shouldn't hurt either.

Event Timeline

cwabbott created this revision.Aug 30 2019, 6:25 AM
Herald added a project: Restricted Project. · View Herald TranscriptAug 30 2019, 6:25 AM

FYI, I think I don't have commit access anymore because of the whole licensing thing, so I'll need someone else to commit for me.

Is mesa actually using mesa3d now? I thought we were still in the unpleasant situation where "unknown" OS was treated like mesa3d, but not quite. I thought clover and radv were using mesa3d, but OpenGL was not. If it has, can we finally drop the second scratch implementation?

Is mesa actually using mesa3d now? I thought we were still in the unpleasant situation where "unknown" OS was treated like mesa3d, but not quite. I thought clover and radv were using mesa3d, but OpenGL was not. If it has, can we finally drop the second scratch implementation?

Ugh, you're right, radeonsi still has target triple = "amdgcn--". Why exactly is that? Would anything break if we just changed it to amdgcn-mesa-mesa3d like what radv does? I don't want that to block this, though, so is there any other user that needs to return true here? AMDVLK maybe?

Is mesa actually using mesa3d now? I thought we were still in the unpleasant situation where "unknown" OS was treated like mesa3d, but not quite. I thought clover and radv were using mesa3d, but OpenGL was not. If it has, can we finally drop the second scratch implementation?

Ugh, you're right, radeonsi still has target triple = "amdgcn--". Why exactly is that? Would anything break if we just changed it to amdgcn-mesa-mesa3d like what radv does? I don't want that to block this, though, so is there any other user that needs to return true here? AMDVLK maybe?

Mesa is the only oddball relying on the unknown OS behavior. IIRC this required work in mesa to switch the scratch ABI from using relocations to what radv does, but besides that I think everything would work

Is mesa actually using mesa3d now? I thought we were still in the unpleasant situation where "unknown" OS was treated like mesa3d, but not quite. I thought clover and radv were using mesa3d, but OpenGL was not. If it has, can we finally drop the second scratch implementation?

Ugh, you're right, radeonsi still has target triple = "amdgcn--". Why exactly is that? Would anything break if we just changed it to amdgcn-mesa-mesa3d like what radv does? I don't want that to block this, though, so is there any other user that needs to return true here? AMDVLK maybe?

Mesa is the only oddball relying on the unknown OS behavior. IIRC this required work in mesa to switch the scratch ABI from using relocations to what radv does, but besides that I think everything would work

Ok, but what I wanted to ask is whether just deleting this function and just always using .rodata is feasible, i.e. whether the only other user (AMDVLK) depends on sticking constant data in .text.

Is mesa actually using mesa3d now? I thought we were still in the unpleasant situation where "unknown" OS was treated like mesa3d, but not quite. I thought clover and radv were using mesa3d, but OpenGL was not. If it has, can we finally drop the second scratch implementation?

Ugh, you're right, radeonsi still has target triple = "amdgcn--". Why exactly is that? Would anything break if we just changed it to amdgcn-mesa-mesa3d like what radv does? I don't want that to block this, though, so is there any other user that needs to return true here? AMDVLK maybe?

Mesa is the only oddball relying on the unknown OS behavior. IIRC this required work in mesa to switch the scratch ABI from using relocations to what radv does, but besides that I think everything would work

Ok, but what I wanted to ask is whether just deleting this function and just always using .rodata is feasible, i.e. whether the only other user (AMDVLK) depends on sticking constant data in .text.

I don't think it depends on it, but I'm not sure @dstuttard @tpr @nhaehnle

cwabbott abandoned this revision.Sep 2 2019, 1:56 AM

I just noticed that this already came up in D65813 and it does the right thing, it's just waiting review.

tpr added a comment.Sep 2 2019, 4:14 AM

I just noticed that this already came up in D65813 and it does the right thing, it's just waiting review.

Hi Connor. Are you able to review D65813 for Jay please? Thanks.

FWIW, the reason that radeonsi uses amdgcn-- is the scratch buffer ABI.