[llvm-dev] [RFC] Supporting ARM's SVE in LLVM

Renato Golin via llvm-dev llvm-dev at lists.llvm.org
Wed Nov 16 04:46:04 PST 2016


On 4 November 2016 at 14:13, Graham Hunter via llvm-dev
<llvm-dev at lists.llvm.org> wrote:
> We've been working for the last two years on support for ARM's Scalable Vector Extension in LLVM, and we'd like to upstream our work. We've had to make several design decisions without community input, and would like to discuss the major changes we've made. To help with the discussions, I've attached a technical document (also in plain text below) to describe the semantics of our changes. We've made the entire patch available as a github repository at https://github.com/ARM-software/LLVM-SVE and our presentation slides and video from the DevMeeting should be available on the llvm website soon.

Hi Graham,

I began looking at this now and before I even delve deep into
understanding the IR changes, I have a few high-level comments on the
whole exercise...


> * This is not intended for code review, just to serve as context for discussion when we post patches for individual pieces (the first of which should be posted within the next two weeks)

This email is long and hard to read. I'm not surprised no one replied
yet. I think your PDF attached is a good start away from the
complexity, but we're not going to get far if we try to do things in
one step.

Also, it would be good if at least some documentation was made
publicly available (specs, ISA, examples, scheduling, pipelines, etc),
so we could cross-reference and understand the tests. Better still, if
some kind of emulator existed, in order to test and validate the
tests.

Based on your repository, the number of changes is so great, and the
changes so invasive, that we really should look back at what we need
to do, one step at a time, and only perform the refactoring changes
that are needed for each step.


> * This is a warts-and-all release of our development tree, with plenty of TODOs and unfinished experiments
> * We haven't posted our clang changes yet

I don't mind FIXMEs or TODOs, but I did see a lot of spurious name
changes, enum value moves (breaking old binaries) and a lot of new
high-level passes (LoopVectorisationAnalysis) which will need a long
review on their own before we even start thinking about SVE.

I recommend you guys separate the refactoring from the implementation
and try to upstream the initial and uncontroversial refactorings (name
changes, etc), as well as move out the current functionality into new
passes, so then you can extend for SVE as a refactoring, not
move-and-extend in the same pass.

We want to minimise the number of changes, so that we can revert
breakages more easily, and have a steady progress, rather than a
break-the-world situation.

Finally, *every* test change needs to be scrutinised and guaranteed to
make sense. We really dislike spurious test changes, unless we can
prove that the test was unstable to being with, in which case we
change it to a better test.

For all those issues, the smaller the patches and the less
controversies they have, the better. Rule of thumb, have no more than
one controversy per patch.

Hope this makes sense.

cheers,
--renato

PS: If someone in the team is not subscribed to the list, I highly
recommend you to.

PS2: If you do subscribe, I highly recommend you develop some serious
filters. :)


More information about the llvm-dev mailing list