Page MenuHomePhabricator

AMDGPU: Don't assert on unknown address spaces
ClosedPublic

Authored by arsenm on May 7 2020, 8:24 AM.

Details

Reviewers
rampitec
b-sumner
Summary

Assume unknown address spaces behave like some flavor of global
memory.

Diff Detail

Event Timeline

arsenm created this revision.May 7 2020, 8:24 AM
Herald added a project: Restricted Project. · View Herald TranscriptMay 7 2020, 8:24 AM

How do we get these address spaces? I'd rather error out.

How do we get these address spaces? I'd rather error out.

You can use attribute((address_space)) or hand write IR. We should not error on valid IR. Someday a frontend may wish to track special information with a custom address space number, which we can handle as a global alias similar to how x86 accepts any arbitrary address space

How do we get these address spaces? I'd rather error out.

You can use attribute((address_space)) or hand write IR. We should not error on valid IR. Someday a frontend may wish to track special information with a custom address space number, which we can handle as a global alias similar to how x86 accepts any arbitrary address space

It sounds dangerous. An unknown address space has unknown semantics. How could we assume anything about it?

arsenm added a comment.May 7 2020, 2:30 PM

How do we get these address spaces? I'd rather error out.

You can use attribute((address_space)) or hand write IR. We should not error on valid IR. Someday a frontend may wish to track special information with a custom address space number, which we can handle as a global alias similar to how x86 accepts any arbitrary address space

It sounds dangerous. An unknown address space has unknown semantics. How could we assume anything about it?

It doesn't have unknown semantics, it has target defined semantics. We can interpret them however we want. At a minimum any pointer computation on the values should work, even if load and store don't select

How do we get these address spaces? I'd rather error out.

You can use attribute((address_space)) or hand write IR. We should not error on valid IR. Someday a frontend may wish to track special information with a custom address space number, which we can handle as a global alias similar to how x86 accepts any arbitrary address space

It sounds dangerous. An unknown address space has unknown semantics. How could we assume anything about it?

It doesn't have unknown semantics, it has target defined semantics. We can interpret them however we want. At a minimum any pointer computation on the values should work, even if load and store don't select

Is there any particular motivation to add this support?

arsenm added a comment.May 7 2020, 4:25 PM

How do we get these address spaces? I'd rather error out.

You can use attribute((address_space)) or hand write IR. We should not error on valid IR. Someday a frontend may wish to track special information with a custom address space number, which we can handle as a global alias similar to how x86 accepts any arbitrary address space

It sounds dangerous. An unknown address space has unknown semantics. How could we assume anything about it?

It doesn't have unknown semantics, it has target defined semantics. We can interpret them however we want. At a minimum any pointer computation on the values should work, even if load and store don't select

Is there any particular motivation to add this support?

Just generally making a best effort attempt to handle any IR that passes the verifier. We already try to handle these in a variety of other places the same way, and theoretically someone could want to use it.

How do we get these address spaces? I'd rather error out.

You can use attribute((address_space)) or hand write IR. We should not error on valid IR. Someday a frontend may wish to track special information with a custom address space number, which we can handle as a global alias similar to how x86 accepts any arbitrary address space

It sounds dangerous. An unknown address space has unknown semantics. How could we assume anything about it?

It doesn't have unknown semantics, it has target defined semantics. We can interpret them however we want. At a minimum any pointer computation on the values should work, even if load and store don't select

Is there any particular motivation to add this support?

Just generally making a best effort attempt to handle any IR that passes the verifier. We already try to handle these in a variety of other places the same way, and theoretically someone could want to use it.

That sounds reasonable. I can think of a situation where it would enable certain investigations.

rampitec accepted this revision.May 7 2020, 5:43 PM

This still sounds very suspicious to me to handle IR which we do not really understand. But since there is a consensus, then OK. There are no questions to the code itself.

This revision is now accepted and ready to land.May 7 2020, 5:43 PM

Thinking more about it, why do you want to treat it as global? There is one universal address space, generic.

arsenm added a comment.May 7 2020, 8:39 PM

Thinking more about it, why do you want to treat it as global? There is one universal address space, generic

Flat is for addressing, not allocation. The non-global flat addressable address spaces need to be specially managed by the compiler, so a user address space aliasing either wouldn’t really work