llvm dev - Mar 2019 - Scalable Vector Types in IR - Next Steps?

On Wed, 13 Mar 2019 at 13:57, Graham Hunter <Graham.Hunter at arm.com>
wrote:
> I did ask them to post their arguments on the list, but I guess they've
been busy for the last month (or forgot about it).
Who is "them" and who will write up a proposal / RFC on the use of
intrinsics for both lowering and vectorisation?

It goes without saying that those discussions should have been had in
the mailing list, not behind closed doors. Agreeing to implementations
in private is asking to get bad reviews in public, as the SVE process
has shown *over and over again*.

I don't understand why, after so many problems for so many years, this
is still the modus operandi...
> The basic argument was that they didn't believe the value gained from
enabling VLA autovectorization was worth the added complexity in maintaining the
codebase. They were open to changing their minds if we could demonstrate
sufficient demand for the feature.
In that case, the current patches to change the IR should be
abandoned, as well as reverting the previous change to the types, so
that we don't carry any unnecessary code forward.

The review you sent seems to be a mechanical change to include the
intrinsics, but the target lowering change seems to be too small to
actually be able to lower anything.

Without context, it's hard to know what's going on.

cheers,
--renato

On 3/13/19 9:29 AM, Renato Golin wrote:
> On Wed, 13 Mar 2019 at 13:57, Graham Hunter <Graham.Hunter at
arm.com> wrote:
>> I did ask them to post their arguments on the list, but I guess
they've been busy for the last month (or forgot about it).
> Who is "them" and who will write up a proposal / RFC on the use
of
> intrinsics for both lowering and vectorisation?
>
> It goes without saying that those discussions should have been had in
> the mailing list, not behind closed doors.

Renato, I understand your frustration, but I don't want an unproductive
conclusion to be drawn. We should encourage our community members to
talk to each other both on the mailing list and off the mailing list. We
have in-person discussions at the developers' meetings, and my
experience is that sitting in a room with someone, or sometimes talking
with someone over the phone, can really help reach a mutual
understanding more effectively than mailing-list communication. However,
the critical step is that the outcome of that conversation should be
summarized, in a timely manner, for the mailing list (or put in the
relevant code review, bug report, etc.) so that the rest of us can
provide input.
>  Agreeing to implementations
> in private is asking to get bad reviews in public

+1

 -Hal

> , as the SVE process
> has shown *over and over again*.
>
> I don't understand why, after so many problems for so many years, this
> is still the modus operandi...
>
>> The basic argument was that they didn't believe the value gained
from enabling VLA autovectorization was worth the added complexity in
maintaining the codebase. They were open to changing their minds if we could
demonstrate sufficient demand for the feature.
> In that case, the current patches to change the IR should be
> abandoned, as well as reverting the previous change to the types, so
> that we don't carry any unnecessary code forward.
>
> The review you sent seems to be a mechanical change to include the
> intrinsics, but the target lowering change seems to be too small to
> actually be able to lower anything.
>
> Without context, it's hard to know what's going on.
>
> cheers,
> --renato
-- 
Hal Finkel
Lead, Compiler Technology and Programming Languages
Leadership Computing Facility
Argonne National Laboratory

On Wed, 13 Mar 2019 at 15:27, Finkel, Hal J. <hfinkel at anl.gov>
wrote:
> However,
> the critical step is that the outcome of that conversation should be
> summarized, in a timely manner, for the mailing list (or put in the
> relevant code review, bug report, etc.) so that the rest of us can
> provide input.
That was my point. Discussing personally on the dev meetings is
essential, but without follow through, the people on the meeting will
be following a different path than the rest of the community.

Right now, whatever conclusions were drawn at that meeting are still
"behind closed doors", which pushed me and David to continue reviewing
patches that were essentially dead and have parallel discussions that
were not only moot, but counter-productive.

--renato

Hi Renato,
> It goes without saying that those discussions should have been had in
> the mailing list, not behind closed doors.
I have encouraged people to respond on the list or the RFC many times,
but I've not had much luck in getting people to post even if they
approve of the idea.
> Agreeing to implementations
> in private is asking to get bad reviews in public, as the SVE process
> has shown *over and over again*.
There isn't an agreement on the implementation yet; I have posted two
possibilities and am trying to get consensus on an approach from the
community.
>> The basic argument was that they didn't believe the value gained
from enabling VLA autovectorization was worth the added complexity in
maintaining the codebase. They were open to changing their minds if we could
demonstrate sufficient demand for the feature.
> 
> In that case, the current patches to change the IR should be
> abandoned, as well as reverting the previous change to the types, so
> that we don't carry any unnecessary code forward.
There's no consensus on supporting the opaque types either yet. Even
if we do end up going down that route, it could be modified -- as I
mentioned in my notes, I could introduce a single toplevel type to
the IR if I stored additional data in it (making it effectively the
same as the current VectorType, just opaque to existing optimization
passes), and then would be able to lower directly to the existing
scalable MVTs we have.

> The review you sent seems to be a mechanical change to include the
> intrinsics, but the target lowering change seems to be too small to
> actually be able to lower anything.
The new patches are just meant to demonstrate the basics of the opaque
type to see if there's greater consensus in exploring this approach
instead of the VLA approach.
> Without context, it's hard to know what's going on.
The current state is just what you stated in your initial email in this
chain; we have a solution that seems to work (in principal) for SVE, RVV,
and SX-Aurora, but not enough people that care about VLA vectorization
beyond those groups.

Given the time constraints, Arm is being pushed to consider a plan B to
get something working in time for early 2020.

-Graham

Disclaimer: I’m only speaking for myself, not Apple.

This is really disappointing. Resorting to multi-versioned fixed length
vectorization isn’t a solution that’s competitive with the native VLA support,
so it doesn’t look like a credible alternative suggestion (at least not without
elaborating it on the mailing list). Without a practical alternative, it’s
essentially saying “no” to a whole class of vector architectures of which SVE is
only one.

Amara
> On Mar 13, 2019, at 9:04 AM, Graham Hunter via llvm-dev <llvm-dev at
lists.llvm.org> wrote:
> 
> Hi Renato,
> 
>> It goes without saying that those discussions should have been had in
>> the mailing list, not behind closed doors.
> 
> I have encouraged people to respond on the list or the RFC many times,
> but I've not had much luck in getting people to post even if they
> approve of the idea.
> 
>> Agreeing to implementations
>> in private is asking to get bad reviews in public, as the SVE process
>> has shown *over and over again*.
> 
> There isn't an agreement on the implementation yet; I have posted two
> possibilities and am trying to get consensus on an approach from the
> community.
> 
>>> The basic argument was that they didn't believe the value
gained from enabling VLA autovectorization was worth the added complexity in
maintaining the codebase. They were open to changing their minds if we could
demonstrate sufficient demand for the feature.
>> 
>> In that case, the current patches to change the IR should be
>> abandoned, as well as reverting the previous change to the types, so
>> that we don't carry any unnecessary code forward.
> 
> There's no consensus on supporting the opaque types either yet. Even
> if we do end up going down that route, it could be modified -- as I
> mentioned in my notes, I could introduce a single toplevel type to
> the IR if I stored additional data in it (making it effectively the
> same as the current VectorType, just opaque to existing optimization
> passes), and then would be able to lower directly to the existing
> scalable MVTs we have.
> 
> 
>> The review you sent seems to be a mechanical change to include the
>> intrinsics, but the target lowering change seems to be too small to
>> actually be able to lower anything.
> 
> The new patches are just meant to demonstrate the basics of the opaque
> type to see if there's greater consensus in exploring this approach
> instead of the VLA approach.
> 
>> Without context, it's hard to know what's going on.
> 
> The current state is just what you stated in your initial email in this
> chain; we have a solution that seems to work (in principal) for SVE, RVV,
> and SX-Aurora, but not enough people that care about VLA vectorization
> beyond those groups.
> 
> Given the time constraints, Arm is being pushed to consider a plan B to
> get something working in time for early 2020.
> 
> -Graham
> 
> 
> 
> 
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev