arsenm added a comment.

In D139627#3981475 <https://reviews.llvm.org/D139627#3981475>, @pengfei wrote:

> The use of `min-legal-vector-width` doesn't look great to me either. I'm more 
> than glad if we can remove it totally without any user perceivable affects.
> I cannot agree with this change because it neither eliminates the clutter 
> (but makes it even worse [1]) nor is NFC to end user.
> I think we used `UINT32_MAX` just to be compatible with BCs that generated 
> before introduing the attribute. This change definitely breaks the 
> compatibility.

What I'm getting is this is only a performance hint, and definitively doesn't 
matter for ABI purposes. Bitcode backwards performance compatibility is not 
important. That would also be recovered by having a proper attribute 
propagation done as an optimization.

I think all of clang's handling of this should be purged, except for the part 
where it's passing through the user attribute.

> Placing a `"min-legal-vector-width" = "512"` doesn't make any sense either. 
> For one thing, we cannot place the attribute in pre-built BC files, for 
> another `512` is the current max vector suppoted on X86, we cannot guarantee 
> no `1024`, `2048` etc. in future and we cannot change it too once compiled 
> into BC files.

It's a test for specific behavior, with a specific configuration that exists 
today. It doesn't matter what this would be in the future for larger testcases

> [1] `"min-legal-vector-width" = "0"` was clear to indicate there are only 
> scalar operations.

It's not remotely clear what this means


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D139627/new/

https://reviews.llvm.org/D139627

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to