After offline discussion, the extra waitcnt is needed because this is a fence, and an acquire needs a waitcnt to ensure a proceeding load atomic that pairs with the fence has completed before invalidating the cache. The memory model on AMDGPUUsage does show the extra waitcnt for the fence. Previously, the waitcnt was being generated as part of the release which is not required if the fence is just an acquire.

Fixed review comments concerning use of class instead of space.

Fix review comments.

Add common block DW_AT_location description.

Address review comment by adding a missing period.

• Correct DW_OP_LLVM_implicit_aspace_pointer to DW_OP_LLVM_aspace_implicit_pointer.

Correct section number of "Extensions" section.

Correct offset operand for DW_OP_bit_extract in DW_OP_LLVM_overlay definition.

Correct type in defintion of rbss.

LGTM

LGTM

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.