LGTM

LGTM

LGTM

• Improve doument overview in first section.
• Improve opening descrition of appendix A.

Improve the summary of the extensions.

• Add section numbers.

• Add section numbers.
• Other minor corrections.

Additional corrections.

Additional corrections.

Further corrections.

• Remove mention of DW_OP_offset.
• Other minor corrections.

Update review feedback from @scott.linder .

My understanding was that it was not guaranteed that an MI instruction has a MMO. That passes could drop the MMO. Are you saying that is not the case? As Matt says, if that is the case the verifier needs to enforce that.

LGTM

@tpr does this change break anything in LLPC?

LGTM

If LocalMemorySize is unsigned, how can it ever be less than 0? Shouldn't the code be corrected instead of adding pragmas?

In D106729#2935273, @efriedma wrote:

Haven't looked at the test changes in detail, but code changes look fine.

@rampitec I believe the other changes do not change the behavior from the perspective of the C++ 20 standard.

LGTM

In D106729#2904861, @efriedma wrote:

No testcases?

Remove unnecessary ()s.

In D106338#2905365, @scott.linder wrote:

I think the change itself LGTM, but I did a grep for GNUInstallDirs and it it only used in two other places currently. I suppose that means it is OK to use, but it does seem to imply there can be cases where we install to inconsistent locations. I don't know who to add to get more info on this, though?

Replaced comment with a trailing "Slash indicates contents of" comment as suggested by @scott.linder.

Remove mention of easy_install as deprecated. MacOS still appears to only come with easy_install.

In D106339#2890258, @ldionne wrote:

What's the benefit of having docx documentation? We generate HTML documentation, which ends up in the website, and that seems strictly superior to generating docx. What do you need it for?

The libc++ changes are almost trivial so I would not object to the change on that basis, however in general I think it's better to avoid adding support for things we won't be using on a regular basis.

Factor out documentation and CMake file changes unrelated to adding DOCX support to D106736.

Split change for clang makefile to elimnate Sphinx warnings of missing .rst fies when building man pages into D106734.

It is also not necessary to request the scratch SRD to be set up in the User SGPRs. There is a significant performance cost for small kernels to requesting this unneeded set of 4 SGPRs to be initialized by the hardware.

In D105222#2851208, @krzysz00 wrote:

My understanding of the documentation is that global_buffer causes the kernel launcher to put the argument into a buffer. Is that not the case?

LGTM Thanks for documenting the differences.

In D103225#2803733, @rampitec wrote:

In D103225#2792240, @rampitec wrote:

In D103225#2785687, @rampitec wrote:

You probably need to wrap all prologue LDS stores into a block to execute it only from lane 0 and add a barrier after. @t-tye correct me if I am wrong.

But, I remember that we had decided to avoid barrier here, and instead just make sure that each thread within each wave execute the store instructions? In anycase, let me clarify it with @t-tye and @b-sumner.

I do not remember, but probably we can omit it since it is a singe store readonly memory. Anyway a confirmation from @t-tye would be nice.

For the record, the agreed way is to do a store from lane 0 of each wave and follow with a wave barrier.

Add feedback from @dp .