https://github.com/carlos4242 closed
https://github.com/llvm/llvm-project/pull/72298
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
carlos4242 wrote:
That makes sense. I think I’ll close this PR for now and work on the apple fork
to get these issues resolved with them. Then I’ll come back once we have
squared off the edges. Cheers Ben.
https://github.com/llvm/llvm-project/pull/72298
benshi001 wrote:
> Sure thing @benshi001 ... I'll create tests. The Swift ABI is documented
> here: https://github.com/apple/swift/blob/main/docs/ABI/CallingConvention.rst
>
> I don't think this change will actually change any ABI. All my code uses the
> normal avr-gcc ABI you referenced.
>
carlos4242 wrote:
Sure thing @benshi001 ... I'll create tests. The Swift ABI is documented here:
https://github.com/apple/swift/blob/main/docs/ABI/CallingConvention.rst
I don't think this change will actually change any ABI. All my code uses the
normal avr-gcc ABI you referenced.
I'll work
benshi001 wrote:
I am not familiar with swift. So is there an official document for the SWIFT
specific ABI on AVR? I think this is necessary, just like C++:
https://gcc.gnu.org/wiki/avr-gcc.
And what's more, tests are required to be committed with functional changes,
this is a requirement
rjmccall wrote:
> That makes a lot of sense. Thank you John. I guess here are my thoughts. As I
> understand it, the `SwiftABIInfo` by default does something like "if
> something can be passed in 4 registers or fewer then pass by register,
> otherwise pass indirectly"? I think that sweet spot
benshi001 wrote:
Please refer to my previous commit of AVR ABI, how tests are provided for a
functional change.
https://github.com/llvm/llvm-project/pull/72298
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
benshi001 wrote:
As I have suggested, any functional change need tests. So I think you need to
add some tests to show what your changes affect.
https://github.com/llvm/llvm-project/pull/72298
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
carlos4242 wrote:
@benshi001 Have you got any thoughts on this as the AVR maintainer? I've been
using various versions of this patch in my own branches for years. Should we
merge?
I think ultimately it's your call as you're the AVR backend owner?
carlos4242 wrote:
> This is less about the implementation weeds of LLVM and more about the actual
> details of your calling convention — the decisions about how arguments and
> results are passed that are ultimately downstream of the choices made here.
> Mostly, I'm encouraging you as a
rjmccall wrote:
This is less about the implementation weeds of LLVM and more about the actual
details of your calling convention — the decisions about how arguments and
results are passed that are ultimately downstream of the choices made here.
Mostly, I'm encouraging you as a platform
carlos4242 wrote:
Thanks, that sounds like it's worth looking into and might avoid issues with
AVR.
I'm still nowhere near enough of an LLVM expert to follow all the aspects of
the discussion. Although from our perspective, I've never seen an issue that I
know, using the above patch for the
rjmccall wrote:
> @efriedma-quic Cool. So it sounds like it's worth parking this for now, until
> Kuba's work #71986 is merged?
>
> @rjmccall I'm not 100% sure I understand? The existing code in AVR.cpp
> handles the standard AVR ABI, which has a few simple rules based on GCC
> behaviour.
carlos4242 wrote:
@efriedma-quic Cool. So it sounds like it's worth parking this for now, until
Kuba's work #71986 is merged?
@rjmccall I'm not 100% sure I understand? The existing code in AVR.cpp handles
the standard AVR ABI, which has a few simple rules based on GCC behaviour. Here
we are
rjmccall wrote:
Right, that would be the way to test it.
I don't know much about AVR, but you should also look at some of the parameters
to the lowering (e.g. how many max values it's okay to break an aggregate into)
and make sure you're happy with them.
efriedma-quic wrote:
See also #71986
https://github.com/llvm/llvm-project/pull/72298
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
efriedma-quic wrote:
Can you not test this by checking an `__attribute__((swiftcall))` function
works in C?
https://github.com/llvm/llvm-project/pull/72298
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://github.com/carlos4242 updated
https://github.com/llvm/llvm-project/pull/72298
>From 333916a07e90955564d03f14e004695802d9f618 Mon Sep 17 00:00:00 2001
From: Carl Peto
Date: Tue, 14 Nov 2023 17:27:37 +
Subject: [PATCH] [AVR] make the AVR ABI Swift compatible
This patch is needed to
https://github.com/carlos4242 updated
https://github.com/llvm/llvm-project/pull/72298
>From fe214850572ba740c0027e8f2908cde0bae75517 Mon Sep 17 00:00:00 2001
From: Carl Peto
Date: Tue, 14 Nov 2023 17:27:37 +
Subject: [PATCH] [AVR] make the AVR ABI Swift compatible
This patch is needed to
github-actions[bot] wrote:
:warning: C/C++ code formatter, clang-format found issues in your code.
:warning:
You can test this locally with the following command:
``bash
git-clang-format --diff 410f130bb99b88f1a8f21659d98053e6f3e5e8f6
ed8b63c31f8b9a496bc5c51ab83a132c224594f0 --
llvmbot wrote:
@llvm/pr-subscribers-clang
@llvm/pr-subscribers-clang-codegen
Author: Carl Peto (carlos4242)
Changes
This patch is needed to add support to clang's AVR ABI for the Swift language.
It is a pre-requisite for adding AVR support to the public Swift compiler
itself.
I'm open
https://github.com/carlos4242 created
https://github.com/llvm/llvm-project/pull/72298
This patch is needed to add support to clang's AVR ABI for the Swift language.
It is a pre-requisite for adding AVR support to the public Swift compiler
itself.
I'm open to any suggestions how I might
22 matches
Mail list logo
