llvm dev - May 2020 - [llvm-commits@lists.llvm.org: Re: [llvm] 2dea3f1 - [SVE] Add new VectorType subclasses]

Regarding the numerical value of the LLVMTypeKind enum, my understanding is that
LLVM-C does not promise to maintain ABI compatability between versions. If I am
mistaken, I can fix this issue.

From: llvm-dev <llvm-dev-bounces at lists.llvm.org> On Behalf Of James Y
Knight via llvm-dev
Sent: Wednesday, May 13, 2020 7:33 AM
To: Joerg Sonnenberger <joerg at bec.de>; llvm-dev <llvm-dev at
lists.llvm.org>
Subject: [EXT] Re: [llvm-dev] [llvm-commits at lists.llvm.org: Re: [llvm]
2dea3f1 - [SVE] Add new VectorType subclasses]

Agreed. Those llvm-c changes are wrong, and compatibility needs to be maintained
for the numeric values at minimum. Probably also would be a good idea to make
LLVMVectorTypeKind a deprecated alias for LLVMFixedVectorTypeKind.

On Wed, May 13, 2020 at 9:11 AM Joerg Sonnenberger via llvm-dev <llvm-dev at
lists.llvm.org<mailto:llvm-dev at lists.llvm.org>> wrote:
Bringing this up on llvm-dev for more general attention.

The problem here is two fold:

(1) Reuse of enumeration values is just a major no-go.
(2) I'm not sure why the existing vector types had to be killed
completely.

But something clearly has to be done here. This majorly affects e.g.
Mesa.

Joerg

----- Forwarded message from Joerg Sonnenberger via llvm-commits
<llvm-commits at lists.llvm.org<mailto:llvm-commits at
lists.llvm.org>> -----

Date: Sun, 10 May 2020 02:34:12 +0200
From: Joerg Sonnenberger via llvm-commits <llvm-commits at
lists.llvm.org<mailto:llvm-commits at lists.llvm.org>>
To: Christopher Tetreault <ctetreau at quicinc.com<mailto:ctetreau at
quicinc.com>>, Christopher Tetreault <llvmlistbot at
llvm.org<mailto:llvmlistbot at llvm.org>>
Cc: llvm-commits at lists.llvm.org<mailto:llvm-commits at lists.llvm.org>
Subject: Re: [llvm] 2dea3f1 - [SVE] Add new VectorType subclasses
Reply-To: Joerg Sonnenberger <joerg at bec.de<mailto:joerg at
bec.de>>

On Wed, Apr 22, 2020 at 08:59:20AM -0700, Christopher Tetreault via llvm-commits
wrote:
>
> Author: Christopher Tetreault
> Date: 2020-04-22T08:59:01-07:00
> New Revision: 2dea3f129878e929e5d1f00b91a622eb1ec8be4e
>
> URL:
https://github.com/llvm/llvm-project/commit/2dea3f129878e929e5d1f00b91a622eb1ec8be4e
> DIFF:
https://github.com/llvm/llvm-project/commit/2dea3f129878e929e5d1f00b91a622eb1ec8be4e.diff
>
> LOG: [SVE] Add new VectorType subclasses
This seems to have basically broken both ABI and API of llvm-c.

Joerg
_______________________________________________
llvm-commits mailing list
llvm-commits at lists.llvm.org<mailto:llvm-commits at lists.llvm.org>
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits

----- End forwarded message -----
_______________________________________________
LLVM Developers mailing list
llvm-dev at lists.llvm.org<mailto:llvm-dev at lists.llvm.org>
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20200513/230622ce/attachment.html>

We can't guarantee everything. But, the goal is to not break things in the
C API, unless necessary. And, when it is necessary, to make sure it's as
obvious as possible of an ABI breakage. E.g., we might delete a
function, and create a new one with a new name, instead of incompatibly
changing the number of arguments for an existing function name. Or, might
leave a gap in enum values where an item is removed.

For your change, modifying the numeric values of the existing constants is
an unnecessary change, and a not-obvious ABI-incompatibility. Just
add LLVMScalableVectorTypeKind to the end, instead, and it's fine.

And, since the C API basically doesn't support scalable vectors yet, and
since you aren't modifying the rest of the C API's uses of
"VectorType" to
FixedVectorType (e.g. the LLVMVectorType function is still named that), I'd
suggest simply leaving the enum value for Fixed vectors named
"LLVMVectorTypeKind" as it was before, instead of renaming it to
LLVMFixedVectorTypeKind.


On Wed, May 13, 2020 at 1:57 PM Chris Tetreault <ctetreau at quicinc.com>
wrote:
> Regarding the numerical value of the LLVMTypeKind enum, my understanding
> is that LLVM-C does not promise to maintain ABI compatability between
> versions. If I am mistaken, I can fix this issue.
>
>
>
> *From:* llvm-dev <llvm-dev-bounces at lists.llvm.org> *On Behalf Of
*James Y
> Knight via llvm-dev
> *Sent:* Wednesday, May 13, 2020 7:33 AM
> *To:* Joerg Sonnenberger <joerg at bec.de>; llvm-dev <llvm-dev at
lists.llvm.org
> >
> *Subject:* [EXT] Re: [llvm-dev] [llvm-commits at lists.llvm.org: Re: [llvm]
> 2dea3f1 - [SVE] Add new VectorType subclasses]
>
>
>
> Agreed. Those llvm-c changes are wrong, and compatibility needs to be
> maintained for the numeric values at minimum. Probably also would be a good
> idea to make LLVMVectorTypeKind a deprecated alias
> for LLVMFixedVectorTypeKind.
>
>
>
> On Wed, May 13, 2020 at 9:11 AM Joerg Sonnenberger via llvm-dev <
> llvm-dev at lists.llvm.org> wrote:
>
> Bringing this up on llvm-dev for more general attention.
>
> The problem here is two fold:
>
> (1) Reuse of enumeration values is just a major no-go.
> (2) I'm not sure why the existing vector types had to be killed
> completely.
>
> But something clearly has to be done here. This majorly affects e.g.
> Mesa.
>
> Joerg
>
> ----- Forwarded message from Joerg Sonnenberger via llvm-commits <
> llvm-commits at lists.llvm.org> -----
>
> Date: Sun, 10 May 2020 02:34:12 +0200
> From: Joerg Sonnenberger via llvm-commits <llvm-commits at
lists.llvm.org>
> To: Christopher Tetreault <ctetreau at quicinc.com>, Christopher
Tetreault <
> llvmlistbot at llvm.org>
> Cc: llvm-commits at lists.llvm.org
> Subject: Re: [llvm] 2dea3f1 - [SVE] Add new VectorType subclasses
> Reply-To: Joerg Sonnenberger <joerg at bec.de>
>
> On Wed, Apr 22, 2020 at 08:59:20AM -0700, Christopher Tetreault via
> llvm-commits wrote:
> >
> > Author: Christopher Tetreault
> > Date: 2020-04-22T08:59:01-07:00
> > New Revision: 2dea3f129878e929e5d1f00b91a622eb1ec8be4e
> >
> > URL:
>
https://github.com/llvm/llvm-project/commit/2dea3f129878e929e5d1f00b91a622eb1ec8be4e
> > DIFF:
>
https://github.com/llvm/llvm-project/commit/2dea3f129878e929e5d1f00b91a622eb1ec8be4e.diff
> >
> > LOG: [SVE] Add new VectorType subclasses
>
> This seems to have basically broken both ABI and API of llvm-c.
>
> Joerg
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
> ----- End forwarded message -----
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20200513/031e028a/attachment-0001.html>

On Wed, 13 May 2020 at 18:57, Chris Tetreault via llvm-dev
<llvm-dev at lists.llvm.org> wrote:
> Regarding the numerical value of the LLVMTypeKind enum, my understanding is
that LLVM-C does not promise to maintain ABI compatability between versions. If
I am mistaken, I can fix this issue.
The way I see it, the enum change brought no positive outcome, so
shouldn't be done. It's not because we don't guarantee backward
compatibility that we should trivially break it for no benefit.

The benefit of not having the gap is not having to spend a bunch of time and
effort on ABI maintenance. I actually left LLVMVectorTypeKind in at first, and
was asked to remove it in code review. Then, I left a gap in the enum where
LLVMVectorTypeKind used to be and was asked in code review to remove the gap.

As for not exposing the names, I think that would be a mistake. The whole point
of refactoring vector types is to expose the fact that scalable vectors exist
and have to be dealt with.  If a ScalableVectorType object is handed to you from
outside of your code, you need to be able to deal with it. Having the ability to
correctly handle a scalable vector and having the ability to create one are two
separate things. The C api currently has the first ability but not the second.
Adding the ability to construct scalable vectors from C still remains to be
implemented. There’s a lot of outstanding work. It all needs to get done, but
with limited resources, some tasks will get prioritized over others. I would
argue that since support for scalable vectors is still a work in progress, you
should probably not be constructing them and passing them into it.

It seems to me that you don’t actually care about constructing scalable vectors
(otherwise you’d be asking me to complete the work rather than back it out), so
the C api should fully support your use case. You should just update your
codebase to correctly handle scalable vs fixed width vectors. That way, when the
day comes that LLVM fully supports scalable vectors, you will be ready.

From: James Y Knight <jyknight at google.com>
Sent: Wednesday, May 13, 2020 11:30 AM
To: Chris Tetreault <ctetreau at quicinc.com>
Cc: Joerg Sonnenberger <joerg at bec.de>; LLVM Dev <llvm-dev at
lists.llvm.org>
Subject: [EXT] Re: [llvm-dev] [llvm-commits at lists.llvm.org: Re: [llvm]
2dea3f1 - [SVE] Add new VectorType subclasses]

We can't guarantee everything. But, the goal is to not break things in the C
API, unless necessary. And, when it is necessary, to make sure it's as
obvious as possible of an ABI breakage. E.g., we might delete a function, and
create a new one with a new name, instead of incompatibly changing the number of
arguments for an existing function name. Or, might leave a gap in enum values
where an item is removed.

For your change, modifying the numeric values of the existing constants is an
unnecessary change, and a not-obvious ABI-incompatibility. Just add
LLVMScalableVectorTypeKind to the end, instead, and it's fine.

And, since the C API basically doesn't support scalable vectors yet, and
since you aren't modifying the rest of the C API's uses of
"VectorType" to FixedVectorType (e.g. the LLVMVectorType function is
still named that), I'd suggest simply leaving the enum value for Fixed
vectors named "LLVMVectorTypeKind" as it was before, instead of
renaming it to LLVMFixedVectorTypeKind.


On Wed, May 13, 2020 at 1:57 PM Chris Tetreault <ctetreau at
quicinc.com<mailto:ctetreau at quicinc.com>> wrote:
Regarding the numerical value of the LLVMTypeKind enum, my understanding is that
LLVM-C does not promise to maintain ABI compatability between versions. If I am
mistaken, I can fix this issue.

From: llvm-dev <llvm-dev-bounces at lists.llvm.org<mailto:llvm-dev-bounces
at lists.llvm.org>> On Behalf Of James Y Knight via llvm-dev
Sent: Wednesday, May 13, 2020 7:33 AM
To: Joerg Sonnenberger <joerg at bec.de<mailto:joerg at bec.de>>;
llvm-dev <llvm-dev at lists.llvm.org<mailto:llvm-dev at
lists.llvm.org>>
Subject: [EXT] Re: [llvm-dev] [llvm-commits at
lists.llvm.org<mailto:llvm-commits at lists.llvm.org>: Re: [llvm] 2dea3f1
- [SVE] Add new VectorType subclasses]

Agreed. Those llvm-c changes are wrong, and compatibility needs to be maintained
for the numeric values at minimum. Probably also would be a good idea to make
LLVMVectorTypeKind a deprecated alias for LLVMFixedVectorTypeKind.

On Wed, May 13, 2020 at 9:11 AM Joerg Sonnenberger via llvm-dev <llvm-dev at
lists.llvm.org<mailto:llvm-dev at lists.llvm.org>> wrote:
Bringing this up on llvm-dev for more general attention.

The problem here is two fold:

(1) Reuse of enumeration values is just a major no-go.
(2) I'm not sure why the existing vector types had to be killed
completely.

But something clearly has to be done here. This majorly affects e.g.
Mesa.

Joerg

----- Forwarded message from Joerg Sonnenberger via llvm-commits
<llvm-commits at lists.llvm.org<mailto:llvm-commits at
lists.llvm.org>> -----

Date: Sun, 10 May 2020 02:34:12 +0200
From: Joerg Sonnenberger via llvm-commits <llvm-commits at
lists.llvm.org<mailto:llvm-commits at lists.llvm.org>>
To: Christopher Tetreault <ctetreau at quicinc.com<mailto:ctetreau at
quicinc.com>>, Christopher Tetreault <llvmlistbot at
llvm.org<mailto:llvmlistbot at llvm.org>>
Cc: llvm-commits at lists.llvm.org<mailto:llvm-commits at lists.llvm.org>
Subject: Re: [llvm] 2dea3f1 - [SVE] Add new VectorType subclasses
Reply-To: Joerg Sonnenberger <joerg at bec.de<mailto:joerg at
bec.de>>

On Wed, Apr 22, 2020 at 08:59:20AM -0700, Christopher Tetreault via llvm-commits
wrote:
>
> Author: Christopher Tetreault
> Date: 2020-04-22T08:59:01-07:00
> New Revision: 2dea3f129878e929e5d1f00b91a622eb1ec8be4e
>
> URL:
https://github.com/llvm/llvm-project/commit/2dea3f129878e929e5d1f00b91a622eb1ec8be4e
> DIFF:
https://github.com/llvm/llvm-project/commit/2dea3f129878e929e5d1f00b91a622eb1ec8be4e.diff
>
> LOG: [SVE] Add new VectorType subclasses
This seems to have basically broken both ABI and API of llvm-c.

Joerg
_______________________________________________
llvm-commits mailing list
llvm-commits at lists.llvm.org<mailto:llvm-commits at lists.llvm.org>
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits

----- End forwarded message -----
_______________________________________________
LLVM Developers mailing list
llvm-dev at lists.llvm.org<mailto:llvm-dev at lists.llvm.org>
https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20200513/c7a8f08c/attachment.html>