macurtis-amd wrote:

@shiltian Thanks for taking a look at this.

A little history to help provide some context ...

A version of this change is already available in amd-staging 
([4218149b7252](https://github.com/ROCm/llvm-project/commit/4218149b72524e84a148d1f5681014b159483c82)).

It was reviewed 
[here](https://github.com/AMD-Lightning-Internal/llvm-project/pull/4455) and 
[here](https://github.com/AMD-Lightning-Internal/llvm-project/pull/4213) and is 
intended as a short-term workaround while @ssahasra works on better 
alternatives.

I'm finally getting around to upstreaming the change with support for 
additional targets.


>I think using a string for a user-facing interface is a bad design.

I think this was chosen for consistency with prior art. @ssahasra or 
@carlobertolli, can you clarify.


> The Sema check also doesn't check whether the scope string is valid or not.

This was intentional as 
[requested](https://github.com/AMD-Lightning-Internal/llvm-project/pull/4213#discussion_r2387697672)
 by @Pierre-vh:

> I think a better way to do this would be to just forward the syncscope as a
string to the backend without checking it any further, and let the backend
determine if that is lowerable or not. Then document the supported syncscopes.
This is what the other builtins do.

https://github.com/llvm/llvm-project/pull/172090
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to