Pierre-vh wrote:

> This line of thought is unnecessarily confrontational. Every time we change 
> something in the code, we are always moving the needle. And every such 
> occasion is an opportunity to discuss and agree on how much to move the 
> needle.

Sorry, it was not my intent to be confrontational. It's uncharacteristic of me 
and I really want to apologize for it.

> Until now, __shared__ implied a specific address space, modulo the missing 
> checks for the address_space attribute. Now we have a type specifying an 
> address space, and that is loading more on the already incomplete picture.

I see, that makes sense why we want to double check this.

> Until then, everything is just a hack. As long as we agree on it being a 
> hack, we can always move forward with lots of TODO and FIXME comments in 
> place.

IMHO the named barrier type is already a hack in itself. It has been exposed to 
the language w/o much documentation nor concerns for how it feels like to use 
it and how it interacts with other language features.

I think this is why I got a bit carried away with my earlier comment. The 
fundamental problem is that the named barrier type was upstreamed with minimal 
review, so it has lots of issues, and your and other reviewers comments are 
entirely fair, but to me it felt a bit like I accidentally signed up for fixing 
all issues about this type without noticing.

For instance I checked on Godbolt the state of the type right now and it's not 
pretty at all: https://godbolt.org/z/9TGTcKe6c

- We can declare it on the host, when this is a device per-workgroup construct.
- Declaring it on the device w/o __shared__ crashes the compiler in ROCm 7.2

Please let me know how we can move this review forward if it's still possible.

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

Reply via email to