| Message ID | 20190812033120.43013-4-natechancellor@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Clang build fixes for MIPS | expand |
On Sun, Aug 11, 2019 at 08:31:18PM -0700, Nathan Chancellor wrote: > From: Vladimir Serbinenko <phcoder@gmail.com> > > clang doesn't recognise =l / =h assembly operand specifiers but apparently > handles C version well. > > lib/mpi/generic_mpih-mul1.c:37:24: error: invalid use of a cast in a > inline asm context requiring an l-value: remove the cast or build with > -fheinous-gnu-extensions > umul_ppmm(prod_high, prod_low, s1_ptr[j], s2_limb); > ~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > lib/mpi/longlong.h:652:20: note: expanded from macro 'umul_ppmm' > : "=l" ((USItype)(w0)), \ > ~~~~~~~~~~^~~ > lib/mpi/generic_mpih-mul1.c:37:3: error: invalid output constraint '=h' > in asm > umul_ppmm(prod_high, prod_low, s1_ptr[j], s2_limb); > ^ > lib/mpi/longlong.h:653:7: note: expanded from macro 'umul_ppmm' > "=h" ((USItype)(w1)) \ > ^ > 2 errors generated. > > Fixes: 5ce3e312ec5c ("crypto: GnuPG based MPI lib - header files (part 2)") > Link: https://github.com/ClangBuiltLinux/linux/issues/605 > Link: https://github.com/gpg/libgcrypt/commit/1ecbd0bca31d462719a2a6590c1d03244e76ef89 > Signed-off-by: Vladimir Serbinenko <phcoder@gmail.com> > [jk: add changelog, rebase on libgcrypt repository, reformat changed > line so it does not go over 80 characters] > Signed-off-by: Jussi Kivilinna <jussi.kivilinna@iki.fi> > [nc: Added build error and tags to commit message > Added Vladimir's signoff with his permission > Adjusted Jussi's comment to wrap at 73 characters > Modified commit subject to mirror MIPS64 commit > Removed space between defined and (__clang__)] > Signed-off-by: Nathan Chancellor <natechancellor@gmail.com> > --- > lib/mpi/longlong.h | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/lib/mpi/longlong.h b/lib/mpi/longlong.h > index 3bb6260d8f42..8a1507fc94dd 100644 > --- a/lib/mpi/longlong.h > +++ b/lib/mpi/longlong.h > @@ -639,7 +639,8 @@ do { \ > ************** MIPS ***************** > ***************************************/ > #if defined(__mips__) && W_TYPE_SIZE == 32 > -#if (__GNUC__ >= 5) || (__GNUC__ >= 4 && __GNUC_MINOR__ >= 4) > +#if defined(__clang__) || (__GNUC__ >= 5) || (__GNUC__ == 4 && \ > + __GNUC_MINOR__ >= 4) > #define umul_ppmm(w1, w0, u, v) \ > do { \ > UDItype __ll = (UDItype)(u) * (v); \ > -- > 2.23.0.rc2 > Hi Paul, I noticed you didn't pick up this patch with the other ones you applied. I just wanted to make sure it wasn't because it was sent to the wrong person. This set of files doesn't appear to have an owner in MAINTAINERS, I guess based on git history either Andrew or Hubert (on CC) pick up patches for this set of files? If I need to, I can resend it to the proper people. Cheers, Nathan
On Sun, Aug 11, 2019 at 10:23:55PM -0700, Nathan Chancellor wrote: > On Sun, Aug 11, 2019 at 08:31:18PM -0700, Nathan Chancellor wrote: > > From: Vladimir Serbinenko <phcoder@gmail.com> > > > > clang doesn't recognise =l / =h assembly operand specifiers but apparently > > handles C version well. > > > > lib/mpi/generic_mpih-mul1.c:37:24: error: invalid use of a cast in a > > inline asm context requiring an l-value: remove the cast or build with > > -fheinous-gnu-extensions > > umul_ppmm(prod_high, prod_low, s1_ptr[j], s2_limb); > > ~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > > lib/mpi/longlong.h:652:20: note: expanded from macro 'umul_ppmm' > > : "=l" ((USItype)(w0)), \ > > ~~~~~~~~~~^~~ > > lib/mpi/generic_mpih-mul1.c:37:3: error: invalid output constraint '=h' > > in asm > > umul_ppmm(prod_high, prod_low, s1_ptr[j], s2_limb); > > ^ > > lib/mpi/longlong.h:653:7: note: expanded from macro 'umul_ppmm' > > "=h" ((USItype)(w1)) \ > > ^ > > 2 errors generated. > > > > Fixes: 5ce3e312ec5c ("crypto: GnuPG based MPI lib - header files (part 2)") > > Link: https://github.com/ClangBuiltLinux/linux/issues/605 > > Link: https://github.com/gpg/libgcrypt/commit/1ecbd0bca31d462719a2a6590c1d03244e76ef89 > > Signed-off-by: Vladimir Serbinenko <phcoder@gmail.com> > > [jk: add changelog, rebase on libgcrypt repository, reformat changed > > line so it does not go over 80 characters] > > Signed-off-by: Jussi Kivilinna <jussi.kivilinna@iki.fi> > > [nc: Added build error and tags to commit message > > Added Vladimir's signoff with his permission > > Adjusted Jussi's comment to wrap at 73 characters > > Modified commit subject to mirror MIPS64 commit > > Removed space between defined and (__clang__)] > > Signed-off-by: Nathan Chancellor <natechancellor@gmail.com> > > --- > > lib/mpi/longlong.h | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/lib/mpi/longlong.h b/lib/mpi/longlong.h > > index 3bb6260d8f42..8a1507fc94dd 100644 > > --- a/lib/mpi/longlong.h > > +++ b/lib/mpi/longlong.h > > @@ -639,7 +639,8 @@ do { \ > > ************** MIPS ***************** > > ***************************************/ > > #if defined(__mips__) && W_TYPE_SIZE == 32 > > -#if (__GNUC__ >= 5) || (__GNUC__ >= 4 && __GNUC_MINOR__ >= 4) > > +#if defined(__clang__) || (__GNUC__ >= 5) || (__GNUC__ == 4 && \ > > + __GNUC_MINOR__ >= 4) > > #define umul_ppmm(w1, w0, u, v) \ > > do { \ > > UDItype __ll = (UDItype)(u) * (v); \ > > -- > > 2.23.0.rc2 > > > > Hi Paul, > > I noticed you didn't pick up this patch with the other ones you > applied. I just wanted to make sure it wasn't because it was sent to > the wrong person. This set of files doesn't appear to have an owner in > MAINTAINERS, I guess based on git history either Andrew or Hubert (on > CC) pick up patches for this set of files? If I need to, I can resend > it to the proper people. > > Cheers, > Nathan Sigh, actually add Andrew and Herbert and get his name right, sorry :(
Hello, On 12.8.2019 6.31, Nathan Chancellor wrote: > From: Vladimir Serbinenko <phcoder@gmail.com> > > clang doesn't recognise =l / =h assembly operand specifiers but apparently > handles C version well. > > lib/mpi/generic_mpih-mul1.c:37:24: error: invalid use of a cast in a > inline asm context requiring an l-value: remove the cast or build with > -fheinous-gnu-extensions > umul_ppmm(prod_high, prod_low, s1_ptr[j], s2_limb); > ~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > lib/mpi/longlong.h:652:20: note: expanded from macro 'umul_ppmm' > : "=l" ((USItype)(w0)), \ > ~~~~~~~~~~^~~ > lib/mpi/generic_mpih-mul1.c:37:3: error: invalid output constraint '=h' > in asm > umul_ppmm(prod_high, prod_low, s1_ptr[j], s2_limb); > ^ > lib/mpi/longlong.h:653:7: note: expanded from macro 'umul_ppmm' > "=h" ((USItype)(w1)) \ > ^ > 2 errors generated. > > Fixes: 5ce3e312ec5c ("crypto: GnuPG based MPI lib - header files (part 2)") > Link: https://github.com/ClangBuiltLinux/linux/issues/605 > Link: https://github.com/gpg/libgcrypt/commit/1ecbd0bca31d462719a2a6590c1d03244e76ef89 > Signed-off-by: Vladimir Serbinenko <phcoder@gmail.com> > [jk: add changelog, rebase on libgcrypt repository, reformat changed > line so it does not go over 80 characters] > Signed-off-by: Jussi Kivilinna <jussi.kivilinna@iki.fi> This is my signed-off-by for libgcrypt project, not kernel. I do not think signed-offs can be passed from other projects in this way. -Jussi > [nc: Added build error and tags to commit message > Added Vladimir's signoff with his permission > Adjusted Jussi's comment to wrap at 73 characters > Modified commit subject to mirror MIPS64 commit > Removed space between defined and (__clang__)] > Signed-off-by: Nathan Chancellor <natechancellor@gmail.com> > --- > lib/mpi/longlong.h | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/lib/mpi/longlong.h b/lib/mpi/longlong.h > index 3bb6260d8f42..8a1507fc94dd 100644 > --- a/lib/mpi/longlong.h > +++ b/lib/mpi/longlong.h > @@ -639,7 +639,8 @@ do { \ > ************** MIPS ***************** > ***************************************/ > #if defined(__mips__) && W_TYPE_SIZE == 32 > -#if (__GNUC__ >= 5) || (__GNUC__ >= 4 && __GNUC_MINOR__ >= 4) > +#if defined(__clang__) || (__GNUC__ >= 5) || (__GNUC__ == 4 && \ > + __GNUC_MINOR__ >= 4) > #define umul_ppmm(w1, w0, u, v) \ > do { \ > UDItype __ll = (UDItype)(u) * (v); \ >
On Sun, Aug 11, 2019 at 10:26:53PM -0700, Nathan Chancellor wrote: > > > I noticed you didn't pick up this patch with the other ones you > > applied. I just wanted to make sure it wasn't because it was sent to > > the wrong person. This set of files doesn't appear to have an owner in > > MAINTAINERS, I guess based on git history either Andrew or Hubert (on > > CC) pick up patches for this set of files? If I need to, I can resend > > it to the proper people. > > > > Cheers, > > Nathan > > Sigh, actually add Andrew and Herbert and get his name right, sorry :( You can route it through the crypto tree by posting this to the linux-crypto@vger.kernel.org mailing list. Cheers,
On Mon, Aug 12, 2019 at 10:35:53AM +0300, Jussi Kivilinna wrote: > Hello, > > On 12.8.2019 6.31, Nathan Chancellor wrote: > > From: Vladimir Serbinenko <phcoder@gmail.com> > > > > clang doesn't recognise =l / =h assembly operand specifiers but apparently > > handles C version well. > > > > lib/mpi/generic_mpih-mul1.c:37:24: error: invalid use of a cast in a > > inline asm context requiring an l-value: remove the cast or build with > > -fheinous-gnu-extensions > > umul_ppmm(prod_high, prod_low, s1_ptr[j], s2_limb); > > ~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~ > > lib/mpi/longlong.h:652:20: note: expanded from macro 'umul_ppmm' > > : "=l" ((USItype)(w0)), \ > > ~~~~~~~~~~^~~ > > lib/mpi/generic_mpih-mul1.c:37:3: error: invalid output constraint '=h' > > in asm > > umul_ppmm(prod_high, prod_low, s1_ptr[j], s2_limb); > > ^ > > lib/mpi/longlong.h:653:7: note: expanded from macro 'umul_ppmm' > > "=h" ((USItype)(w1)) \ > > ^ > > 2 errors generated. > > > > Fixes: 5ce3e312ec5c ("crypto: GnuPG based MPI lib - header files (part 2)") > > Link: https://github.com/ClangBuiltLinux/linux/issues/605 > > Link: https://github.com/gpg/libgcrypt/commit/1ecbd0bca31d462719a2a6590c1d03244e76ef89 > > Signed-off-by: Vladimir Serbinenko <phcoder@gmail.com> > > [jk: add changelog, rebase on libgcrypt repository, reformat changed > > line so it does not go over 80 characters] > > Signed-off-by: Jussi Kivilinna <jussi.kivilinna@iki.fi> > > This is my signed-off-by for libgcrypt project, not kernel. I do not think > signed-offs can be passed from other projects in this way. > > -Jussi Hi Jussi, I am no signoff expert but if I am reading the developer certificate of origin in the libgcrypt repo correctly [1], your signoff on this commit falls under: (d) I understand and agree that this project and the contribution are public and that a record of the contribution (including all personal information I submit with it, including my sign-off) is maintained indefinitely and may be redistributed consistent with this project or the open source license(s) involved. This file is maintained under the LGPL because it was taken straight from the libgcrypr repo and per (b), I can submit this commit here with everything intact. However, I don't want to upset you in any way though so if you are not comfortable with that, I suppose I can remove it as if Vladimir submitted this fix to me directly (as I got permission for his signoff). I need to resubmit this fix to an appropriate maintainer so let me know what you think. [1]: https://github.com/gpg/libgcrypt/blob/3bb858551cd5d84e43b800edfa2b07d1529718a9/doc/DCO Cheers, Nathan
Hi Nathan, On Sun, Aug 11, 2019 at 10:23:55PM -0700, Nathan Chancellor wrote: > I noticed you didn't pick up this patch with the other ones you > applied. I just wanted to make sure it wasn't because it was sent to > the wrong person. This set of files doesn't appear to have an owner in > MAINTAINERS, I guess based on git history either Andrew or Hubert (on > CC) pick up patches for this set of files? If I need to, I can resend > it to the proper people. The 3 arch/mips patches were trivial for me to apply because I'm very familiar with the code & know they should go via the MIPS tree. I'm far less familiar with lib/mpi & needed to look up maintainers, which is why I didn't apply them in the few minutes I had spare. Thanks, Paul
Hello, On 12.8.2019 20.14, Nathan Chancellor wrote: > On Mon, Aug 12, 2019 at 10:35:53AM +0300, Jussi Kivilinna wrote: >> Hello, >> >> On 12.8.2019 6.31, Nathan Chancellor wrote: >>> From: Vladimir Serbinenko <phcoder@gmail.com> >>> >>> clang doesn't recognise =l / =h assembly operand specifiers but apparently >>> handles C version well. >>> >>> lib/mpi/generic_mpih-mul1.c:37:24: error: invalid use of a cast in a >>> inline asm context requiring an l-value: remove the cast or build with >>> -fheinous-gnu-extensions >>> umul_ppmm(prod_high, prod_low, s1_ptr[j], s2_limb); >>> ~~~~~~~~~~~~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~ >>> lib/mpi/longlong.h:652:20: note: expanded from macro 'umul_ppmm' >>> : "=l" ((USItype)(w0)), \ >>> ~~~~~~~~~~^~~ >>> lib/mpi/generic_mpih-mul1.c:37:3: error: invalid output constraint '=h' >>> in asm >>> umul_ppmm(prod_high, prod_low, s1_ptr[j], s2_limb); >>> ^ >>> lib/mpi/longlong.h:653:7: note: expanded from macro 'umul_ppmm' >>> "=h" ((USItype)(w1)) \ >>> ^ >>> 2 errors generated. >>> >>> Fixes: 5ce3e312ec5c ("crypto: GnuPG based MPI lib - header files (part 2)") >>> Link: https://github.com/ClangBuiltLinux/linux/issues/605 >>> Link: https://github.com/gpg/libgcrypt/commit/1ecbd0bca31d462719a2a6590c1d03244e76ef89 >>> Signed-off-by: Vladimir Serbinenko <phcoder@gmail.com> >>> [jk: add changelog, rebase on libgcrypt repository, reformat changed >>> line so it does not go over 80 characters] >>> Signed-off-by: Jussi Kivilinna <jussi.kivilinna@iki.fi> >> >> This is my signed-off-by for libgcrypt project, not kernel. I do not think >> signed-offs can be passed from other projects in this way. >> >> -Jussi > > Hi Jussi, > > I am no signoff expert but if I am reading the developer certificate of > origin in the libgcrypt repo correctly [1], your signoff on this commit > falls under: > > (d) I understand and agree that this project and the contribution > are public and that a record of the contribution (including all > personal information I submit with it, including my sign-off) is > maintained indefinitely and may be redistributed consistent with > this project or the open source license(s) involved. There is nothing wrong with the commit in libgcrypt repo and/or my libgcrypt-DCO-sign-off. > > This file is maintained under the LGPL because it was taken straight > from the libgcrypr repo and per (b), I can submit this commit here > with everything intact. But you do not have my kernel-DCO-sign-off for this patch. I have not been involved with this kernel patch in anyway, have not integrated it to kernel, not testing it on kernel.. I do not own it. However, with this signed-off-by line you have involved me to kernel patch process in which for this patch I'm not interested. So to be clear, I retract my kernel-DCO-signed-off for this kernel patch: NOT-Signed-off-by: Jussi Kivilinna <jussi.kivilinna@iki.fi> Of course you can copy the original libgcrypt commit message to this patch, but I think it needs to be clearly quoted so that my libgcrypt-DCO-signed-off line wont be mixed with kernel-DOC-signed-off lines. > > However, I don't want to upset you in any way though so if you are not > comfortable with that, I suppose I can remove it as if Vladimir > submitted this fix to me directly (as I got permission for his signoff). > I need to resubmit this fix to an appropriate maintainer so let me know > what you think. That's quite complicated approach. Fast and easier process would be if you just own the patch yourself. Libgcrypt (and target file in libgcrypt) is LGPL v2.1+, so the license is compatible with kernel and you are good to go with just your own (kernel DCO) signed-off-by. -Jussi > > [1]: https://github.com/gpg/libgcrypt/blob/3bb858551cd5d84e43b800edfa2b07d1529718a9/doc/DCO > > Cheers, > Nathan >
On Mon, Aug 12, 2019 at 10:40:49PM +0300, Jussi Kivilinna wrote: > That's quite complicated approach. Fast and easier process would be if you > just own the patch yourself. Libgcrypt (and target file in libgcrypt) > is LGPL v2.1+, so the license is compatible with kernel and you are good > to go with just your own (kernel DCO) signed-off-by. > > -Jussi I have gone this route as another developer pointed out that we can eliminate all of the inline asm umul_ppmm definitions because the kernel requires GCC 4.6 and newer and that is completely different from the libgcrypt patches. https://lore.kernel.org/lkml/20190812193256.55103-1-natechancellor@gmail.com/ Thanks for weighing in and cheers! Nathan
diff --git a/lib/mpi/longlong.h b/lib/mpi/longlong.h index 3bb6260d8f42..8a1507fc94dd 100644 --- a/lib/mpi/longlong.h +++ b/lib/mpi/longlong.h @@ -639,7 +639,8 @@ do { \ ************** MIPS ***************** ***************************************/ #if defined(__mips__) && W_TYPE_SIZE == 32 -#if (__GNUC__ >= 5) || (__GNUC__ >= 4 && __GNUC_MINOR__ >= 4) +#if defined(__clang__) || (__GNUC__ >= 5) || (__GNUC__ == 4 && \ + __GNUC_MINOR__ >= 4) #define umul_ppmm(w1, w0, u, v) \ do { \ UDItype __ll = (UDItype)(u) * (v); \