Re: [PATCH] include/longlong.h: Remove incorrect lvalue to rvalue conversion from asm output constraints

2021-10-14 Thread Fāng-ruì Sòng via Gcc-patches

On 2021-10-12, Jakub Jelinek wrote:

On Tue, Oct 12, 2021 at 09:21:21AM -0700, Fāng-ruì Sòng wrote:

> > An output constraint takes a lvalue. While GCC happily strips the
> > incorrect lvalue to rvalue conversion, Clang rejects the code by default:
> >
> > error: invalid use of a cast in a inline asm context requiring an 
lvalue: remove the cast or build with -fheinous-gnu-extensions
> >
> > The file appears to share the same origin with gmplib longlong.h but
> > they differ much now (gmplib version is much longer).
> >
> > I don't have write access to the git repo.
> > ---
> >  include/longlong.h | 186 ++---
> >  1 file changed, 93 insertions(+), 93 deletions(-)
> >
> > diff --git a/include/longlong.h b/include/longlong.h
> > index c3e92e54ecc..0a21a441d2d 100644
> > --- a/include/longlong.h
> > +++ b/include/longlong.h
> > @@ -194,8 +194,8 @@ extern UDItype __udiv_qrnnd (UDItype *, UDItype, 
UDItype, UDItype);
> >  #if defined (__arc__) && W_TYPE_SIZE == 32
> >  #define add_ss(sh, sl, ah, al, bh, bl) \
> >__asm__ ("add.f%1, %4, %5\n\tadc   %0, %2, %3" \
> > -: "=r" ((USItype) (sh)), \
> > -  "=" ((USItype) (sl)) \
> > +: "=r" (sh), \
> > +  "=" (sl) \
> >  : "%r" ((USItype) (ah)), \
> >"rICal" ((USItype) (bh)),  \
> >"%r" ((USItype) (al)), \
>
> This seems to alter the meanining of existing programs if sh and sl do
> not have the expected type.
>
> I think you need to add a compound expression and temporaries of type
> USItype if you want to avoid the cast.

Add folks who may comment on the output constraint behavior when a
lvalue to rvalue conversion like (`(USItype)`) is added.


Allowing the casts in there is intentional, the comment about this
e.g. in GCC's C FE says:
Really, this should not be here.  Users should be using a
proper lvalue, dammit.  But there's a long history of using casts
in the output operands.  In cases like longlong.h, this becomes a
primitive form of typechecking -- if the cast can be removed, then
the output operand had a type of the proper width; otherwise we'll
get an error.

If you try e.g.:

void
foo (void)
{
 int i;
 long l;
 __asm ("" : "=r" ((unsigned) i));
 __asm ("" : "=r" ((long) l));
 __asm ("" : "=r" ((long long) l));
 __asm ("" : "=r" ((int) l));
 __asm ("" : "=r" ((long) i));
}

then on e.g. x86-64 the first 3 asms are accepted by GCC, the last two
rejected, because the modes are different there.

So the above change throws away important typechecking.  As it is
used in a macro, something different should verify that if the casts are
removed.

Jakub



Thanks for the detailed explanation. I see the type checking effect now.
In many cases the assembler can provide some checks, e.g.

  long foo(long a) {
long r;
__asm__ ("movq %1, %0" : "=r"(r) : "r"(a));
return r;
  }

I came here from trying to build glibc with Clang where I came across errors 
like

In file included from strtof_l.c:44:
./strtod_l.c:1500:26: error: invalid use of a cast in a inline asm context 
requiring an lvalue: remove the cast or build with -fheinous-gnu-extensions
  udiv_qrnnd (quot, n, n, 0, d);
  ~~^~~
./longlong.h:518:24: note: expanded from macro 'udiv_qrnnd'
 "=d" ((UDItype) (r))   \
   ~~~^~
In file included from strtof_l.c:44:
./strtod_l.c:1606:21: error: invalid use of a cast in a inline asm context 
requiring an lvalue: remove the cast or build with -fheinous-gnu-extensions
  add_ss (n1, n0, r - d0, 0, 0, d0);
  ^

(Seems that building GCC with Clang doesn't run into the issues?)

If the desire to keep the checking is still strong, perhaps I will just stick 
with using the -f option for glibc.


Re: [PATCH] include/longlong.h: Remove incorrect lvalue to rvalue conversion from asm output constraints

2021-10-12 Thread Jakub Jelinek via Gcc-patches
On Tue, Oct 12, 2021 at 09:21:21AM -0700, Fāng-ruì Sòng wrote:
> > > An output constraint takes a lvalue. While GCC happily strips the
> > > incorrect lvalue to rvalue conversion, Clang rejects the code by default:
> > >
> > > error: invalid use of a cast in a inline asm context requiring an 
> > > lvalue: remove the cast or build with -fheinous-gnu-extensions
> > >
> > > The file appears to share the same origin with gmplib longlong.h but
> > > they differ much now (gmplib version is much longer).
> > >
> > > I don't have write access to the git repo.
> > > ---
> > >  include/longlong.h | 186 ++---
> > >  1 file changed, 93 insertions(+), 93 deletions(-)
> > >
> > > diff --git a/include/longlong.h b/include/longlong.h
> > > index c3e92e54ecc..0a21a441d2d 100644
> > > --- a/include/longlong.h
> > > +++ b/include/longlong.h
> > > @@ -194,8 +194,8 @@ extern UDItype __udiv_qrnnd (UDItype *, UDItype, 
> > > UDItype, UDItype);
> > >  #if defined (__arc__) && W_TYPE_SIZE == 32
> > >  #define add_ss(sh, sl, ah, al, bh, bl) \
> > >__asm__ ("add.f%1, %4, %5\n\tadc   %0, %2, %3" \
> > > -: "=r" ((USItype) (sh)), \
> > > -  "=" ((USItype) (sl)) \
> > > +: "=r" (sh), \
> > > +  "=" (sl) \
> > >  : "%r" ((USItype) (ah)), \
> > >"rICal" ((USItype) (bh)),  \
> > >"%r" ((USItype) (al)), \
> >
> > This seems to alter the meanining of existing programs if sh and sl do
> > not have the expected type.
> >
> > I think you need to add a compound expression and temporaries of type
> > USItype if you want to avoid the cast.
> 
> Add folks who may comment on the output constraint behavior when a
> lvalue to rvalue conversion like (`(USItype)`) is added.

Allowing the casts in there is intentional, the comment about this
e.g. in GCC's C FE says:
Really, this should not be here.  Users should be using a
proper lvalue, dammit.  But there's a long history of using casts
in the output operands.  In cases like longlong.h, this becomes a
primitive form of typechecking -- if the cast can be removed, then
the output operand had a type of the proper width; otherwise we'll
get an error.

If you try e.g.:

void
foo (void)
{
  int i;
  long l;
  __asm ("" : "=r" ((unsigned) i));
  __asm ("" : "=r" ((long) l));
  __asm ("" : "=r" ((long long) l));
  __asm ("" : "=r" ((int) l));
  __asm ("" : "=r" ((long) i));
}

then on e.g. x86-64 the first 3 asms are accepted by GCC, the last two
rejected, because the modes are different there.

So the above change throws away important typechecking.  As it is
used in a macro, something different should verify that if the casts are
removed.

Jakub



Re: [PATCH] include/longlong.h: Remove incorrect lvalue to rvalue conversion from asm output constraints

2021-10-12 Thread Fāng-ruì Sòng via Gcc-patches
On Sun, Oct 10, 2021 at 10:03 PM Florian Weimer  wrote:
>
> * Fangrui Song:
>
> > An output constraint takes a lvalue. While GCC happily strips the
> > incorrect lvalue to rvalue conversion, Clang rejects the code by default:
> >
> > error: invalid use of a cast in a inline asm context requiring an 
> > lvalue: remove the cast or build with -fheinous-gnu-extensions
> >
> > The file appears to share the same origin with gmplib longlong.h but
> > they differ much now (gmplib version is much longer).
> >
> > I don't have write access to the git repo.
> > ---
> >  include/longlong.h | 186 ++---
> >  1 file changed, 93 insertions(+), 93 deletions(-)
> >
> > diff --git a/include/longlong.h b/include/longlong.h
> > index c3e92e54ecc..0a21a441d2d 100644
> > --- a/include/longlong.h
> > +++ b/include/longlong.h
> > @@ -194,8 +194,8 @@ extern UDItype __udiv_qrnnd (UDItype *, UDItype, 
> > UDItype, UDItype);
> >  #if defined (__arc__) && W_TYPE_SIZE == 32
> >  #define add_ss(sh, sl, ah, al, bh, bl) \
> >__asm__ ("add.f%1, %4, %5\n\tadc   %0, %2, %3" \
> > -: "=r" ((USItype) (sh)), \
> > -  "=" ((USItype) (sl)) \
> > +: "=r" (sh), \
> > +  "=" (sl) \
> >  : "%r" ((USItype) (ah)), \
> >"rICal" ((USItype) (bh)),  \
> >"%r" ((USItype) (al)), \
>
> This seems to alter the meanining of existing programs if sh and sl do
> not have the expected type.
>
> I think you need to add a compound expression and temporaries of type
> USItype if you want to avoid the cast.

Add folks who may comment on the output constraint behavior when a
lvalue to rvalue conversion like (`(USItype)`) is added.


Re: [PATCH] include/longlong.h: Remove incorrect lvalue to rvalue conversion from asm output constraints

2021-10-10 Thread Florian Weimer
* Fangrui Song:

> An output constraint takes a lvalue. While GCC happily strips the
> incorrect lvalue to rvalue conversion, Clang rejects the code by default:
>
> error: invalid use of a cast in a inline asm context requiring an lvalue: 
> remove the cast or build with -fheinous-gnu-extensions
>
> The file appears to share the same origin with gmplib longlong.h but
> they differ much now (gmplib version is much longer).
>
> I don't have write access to the git repo.
> ---
>  include/longlong.h | 186 ++---
>  1 file changed, 93 insertions(+), 93 deletions(-)
>
> diff --git a/include/longlong.h b/include/longlong.h
> index c3e92e54ecc..0a21a441d2d 100644
> --- a/include/longlong.h
> +++ b/include/longlong.h
> @@ -194,8 +194,8 @@ extern UDItype __udiv_qrnnd (UDItype *, UDItype, UDItype, 
> UDItype);
>  #if defined (__arc__) && W_TYPE_SIZE == 32
>  #define add_ss(sh, sl, ah, al, bh, bl) \
>__asm__ ("add.f%1, %4, %5\n\tadc   %0, %2, %3" \
> -: "=r" ((USItype) (sh)), \
> -  "=" ((USItype) (sl)) \
> +: "=r" (sh), \
> +  "=" (sl) \
>  : "%r" ((USItype) (ah)), \
>"rICal" ((USItype) (bh)),  \
>"%r" ((USItype) (al)), \

This seems to alter the meanining of existing programs if sh and sl do
not have the expected type.

I think you need to add a compound expression and temporaries of type
USItype if you want to avoid the cast.


[PATCH] include/longlong.h: Remove incorrect lvalue to rvalue conversion from asm output constraints

2021-10-10 Thread Fangrui Song
An output constraint takes a lvalue. While GCC happily strips the
incorrect lvalue to rvalue conversion, Clang rejects the code by default:

error: invalid use of a cast in a inline asm context requiring an lvalue: 
remove the cast or build with -fheinous-gnu-extensions

The file appears to share the same origin with gmplib longlong.h but
they differ much now (gmplib version is much longer).

I don't have write access to the git repo.
---
 include/longlong.h | 186 ++---
 1 file changed, 93 insertions(+), 93 deletions(-)

diff --git a/include/longlong.h b/include/longlong.h
index c3e92e54ecc..0a21a441d2d 100644
--- a/include/longlong.h
+++ b/include/longlong.h
@@ -194,8 +194,8 @@ extern UDItype __udiv_qrnnd (UDItype *, UDItype, UDItype, 
UDItype);
 #if defined (__arc__) && W_TYPE_SIZE == 32
 #define add_ss(sh, sl, ah, al, bh, bl) \
   __asm__ ("add.f  %1, %4, %5\n\tadc   %0, %2, %3" \
-  : "=r" ((USItype) (sh)), \
-"=" ((USItype) (sl)) \
+  : "=r" (sh), \
+"=" (sl) \
   : "%r" ((USItype) (ah)), \
 "rICal" ((USItype) (bh)),  \
 "%r" ((USItype) (al)), \
@@ -203,8 +203,8 @@ extern UDItype __udiv_qrnnd (UDItype *, UDItype, UDItype, 
UDItype);
   : "cc")
 #define sub_ddmmss(sh, sl, ah, al, bh, bl) \
   __asm__ ("sub.f  %1, %4, %5\n\tsbc   %0, %2, %3" \
-  : "=r" ((USItype) (sh)), \
-"=" ((USItype) (sl)) \
+  : "=r" (sh), \
+"=" (sl) \
   : "r" ((USItype) (ah)),  \
 "rICal" ((USItype) (bh)),  \
 "r" ((USItype) (al)),  \
@@ -230,16 +230,16 @@ extern UDItype __udiv_qrnnd (UDItype *, UDItype, UDItype, 
UDItype);
  && W_TYPE_SIZE == 32
 #define add_ss(sh, sl, ah, al, bh, bl) \
   __asm__ ("adds   %1, %4, %5\n\tadc   %0, %2, %3" \
-  : "=r" ((USItype) (sh)), \
-"=" ((USItype) (sl)) \
+  : "=r" (sh), \
+"=" (sl) \
   : "%r" ((USItype) (ah)), \
 "rI" ((USItype) (bh)), \
 "%r" ((USItype) (al)), \
 "rI" ((USItype) (bl)) __CLOBBER_CC)
 #define sub_ddmmss(sh, sl, ah, al, bh, bl) \
   __asm__ ("subs   %1, %4, %5\n\tsbc   %0, %2, %3" \
-  : "=r" ((USItype) (sh)), \
-"=" ((USItype) (sl)) \
+  : "=r" (sh), \
+"=" (sl) \
   : "r" ((USItype) (ah)),  \
 "rI" ((USItype) (bh)), \
 "r" ((USItype) (al)),  \
@@ -262,8 +262,8 @@ extern UDItype __udiv_qrnnd (UDItype *, UDItype, UDItype, 
UDItype);
   "addcs   %0, %0, #65536\n"   \
   "adds%1, %1, %3, lsl #16\n"  \
   "adc %0, %0, %3, lsr #16"\
-  : "=" ((USItype) (xh)),\
-"=r" ((USItype) (xl)), \
+  : "=" (xh),\
+"=r" (xl), \
 "=" (__t0), "=" (__t1), "=r" (__t2)\
   : "r" ((USItype) (a)),   \
 "r" ((USItype) (b)) __CLOBBER_CC );\
@@ -348,16 +348,16 @@ extern UDItype __umulsidi3 (USItype, USItype);
 #if defined (__hppa) && W_TYPE_SIZE == 32
 #define add_ss(sh, sl, ah, al, bh, bl) \
   __asm__ ("add %4,%5,%1\n\taddc %2,%3,%0" \
-  : "=r" ((USItype) (sh)), \
-"=" ((USItype) (sl)) \
+  : "=r" (sh), \
+"=" (sl) \