Re: [Tinycc-devel] Broken commit e460f7dbb216

[Vincent Lefevre]

On 2022-07-30 19:48:29 +0200, grischka wrote:
> On 30.07.2022 12:45, Vincent Lefevre wrote:
> > I changed the & to && because it is more correct and makes more sense,
> > but note that this is not really redundant because n is signed and
> > n & (n - 1) is not portable when n = 0 or 1 if two's complement isn't
> > necessarily used, because 0 may have several representations (well,
> > tcc is not designed to work on such exotic platforms, but who knows
> > in the future... and moreover, because of this general portability
> > issue, compilers may issue warnings).
> 
> Sorry, what?  Are you saying that (n & (n - 1)) with n == 1 -> (1 & 0)
> and with n == 0 -> (0 & -1) does not evaluate to 0 in both cases always
> necessarily, in C?

Yes, if the 0 happens to be a negative 0, 1 & 0 will give 1
in ones' complement, and 0 & -1 will also give non-zero in
both ones' complement and "sign and magnitude".

ISO C17 says:

  It is unspecified whether these cases actually generate a
  negative zero or a normal zero, and whether a negative zero
  becomes a normal zero when stored in an object.

> > > As to the "n < 0 ||" clause by the way. the C-standards seem to say:
> > > 
> > >"Alignments are represented as values of the type size_t. Valid 
> > > alignments
> > > include only fundamental alignments, plus an additional 
> > > implementation-
> > > defined set of values, which may be empty.  Every valid alignment 
> > > value
> > > shall be a nonnegative integral power of two."
> > > 
> > > Well, when "size_t" means unsigned, how could it be not "nonnegative"
> > > then ?!?
> > 
> > I don't understand what you mean here. The fact is that in the code,
> > n is of type int, not size_t. So the case n < 0 needs to be taken
> > into account. Note that n comes from
> > 
> >n = expr_const();
> > 
> > thus may be negative.
> 
> Seems I have misunderstood what they are trying to say.  It seems that by
>"nonnegative integral power of two"
> they mean any powers 0..n of two, as in
> 2 ^ 0..n
> as opposed to
> 2 ^ -1.2
> for example.
> 
> That hasn't to do much with "int n = expr_const();" in tcc though because
> the 'n' there does not mean "2 ^ n", just as _Alignas(1) does not mean
> to align at 2 ^ 1.  It means to align at 2 ^ 0 = 1.

The point is that _Alignas(-2) is invalid.

> Therfor the combination
> 
> if (n < 0 || ...
> tcc_error("alignment must be a positive power of two");
> 
> still seems misleading.  "_Alignas(0x8000) int x;" is not an
> invalid declaration, per se.

Well, this depends on the implementation. If supported by tcc,
this is fine. But this should be documented, as this is an
implementation-defined feature.

-- 
Vincent Lefèvre  - Web: 
100% accessible validated (X)HTML - Blog: 
Work: CR INRIA - computer arithmetic / AriC project (LIP, ENS-Lyon)

___
Tinycc-devel mailing list
Tinycc-devel@nongnu.org
https://lists.nongnu.org/mailman/listinfo/tinycc-devel

Re: [Tinycc-devel] Broken commit e460f7dbb216

[grischka]

On 30.07.2022 12:45, Vincent Lefevre wrote:

I changed the & to && because it is more correct and makes more sense,
but note that this is not really redundant because n is signed and
n & (n - 1) is not portable when n = 0 or 1 if two's complement isn't
necessarily used, because 0 may have several representations (well,
tcc is not designed to work on such exotic platforms, but who knows
in the future... and moreover, because of this general portability
issue, compilers may issue warnings).


Sorry, what?  Are you saying that (n & (n - 1)) with n == 1 -> (1 & 0)
and with n == 0 -> (0 & -1) does not evaluate to 0 in both cases always
necessarily, in C?


As to the "n < 0 ||" clause by the way. the C-standards seem to say:

   "Alignments are represented as values of the type size_t. Valid alignments
include only fundamental alignments, plus an additional implementation-
defined set of values, which may be empty.  Every valid alignment value
shall be a nonnegative integral power of two."

Well, when "size_t" means unsigned, how could it be not "nonnegative"
then ?!?


I don't understand what you mean here. The fact is that in the code,
n is of type int, not size_t. So the case n < 0 needs to be taken
into account. Note that n comes from

   n = expr_const();

thus may be negative.


Seems I have misunderstood what they are trying to say.  It seems that by
   "nonnegative integral power of two"
they mean any powers 0..n of two, as in
2 ^ 0..n
as opposed to
2 ^ -1.2
for example.

That hasn't to do much with "int n = expr_const();" in tcc though because
the 'n' there does not mean "2 ^ n", just as _Alignas(1) does not mean
to align at 2 ^ 1.  It means to align at 2 ^ 0 = 1.

Therfor the combination

if (n < 0 || ...
tcc_error("alignment must be a positive power of two");

still seems misleading.  "_Alignas(0x8000) int x;" is not an
invalid declaration, per se.


BTW, the condition seems incomplete because large values are forbidden.
ISO C17 says in 6.2.8p2:

   A fundamental alignment is a valid alignment less than or equal to
   _Alignof (max_align_t).

and in 6.7.5p3 (a constraint):

   The constant expression shall be an integer constant expression.
   It shall evaluate to a valid fundamental alignment, or to a valid
   extended alignment supported by the implementation for an object
   of the storage duration (if any) being declared, or to zero.



Then I think we can still accept larger values as "extended alignments
supported by implementation".  (Where really large values would crash
"the implementation" rather efficiently though).

--- grischka

___
Tinycc-devel mailing list
Tinycc-devel@nongnu.org
https://lists.nongnu.org/mailman/listinfo/tinycc-devel

Re: [Tinycc-devel] Broken commit e460f7dbb216

[Vincent Lefevre]

Hi,

On 2022-07-30 11:02:39 +0200, grischka wrote:
> On 28.07.2022 16:34, Detlef Riekenberg wrote:
> > Hi grischka.
> > 
> 
> Hi Detlef,
> 
> Please try to be more precise.  For example, in your last commit,
> you wrote:
> 
>"Do not fail with _Alignas(0) and _Alignas(1), used by autotools"
> 
> But there wasn't anything wrong with _Alignas(1), to begin with.
> Also, the "(n > 1) &" part that you added in your one-line patch
> 
>-  if (n <= 0 || (n & (n - 1)) != 0)
>+  if (n < 0 || ((n > 1) & ((n & (n - 1)) != 0)))
> 
> is just redundant.  Unnecessary redundancy confuses the readers, and
> confused readers may add even more confusing stuff in sequence.
> (Now I see that "(n > 1) &" has changed to "(n > 1) &&" which, well,
> is still equally redundant.)

I changed the & to && because it is more correct and makes more sense,
but note that this is not really redundant because n is signed and
n & (n - 1) is not portable when n = 0 or 1 if two's complement isn't
necessarily used, because 0 may have several representations (well,
tcc is not designed to work on such exotic platforms, but who knows
in the future... and moreover, because of this general portability
issue, compilers may issue warnings).

> As to the "n < 0 ||" clause by the way. the C-standards seem to say:
> 
>   "Alignments are represented as values of the type size_t. Valid alignments
>include only fundamental alignments, plus an additional implementation-
>defined set of values, which may be empty.  Every valid alignment value
>shall be a nonnegative integral power of two."
> 
> Well, when "size_t" means unsigned, how could it be not "nonnegative"
> then ?!?

I don't understand what you mean here. The fact is that in the code,
n is of type int, not size_t. So the case n < 0 needs to be taken
into account. Note that n comes from

  n = expr_const();

thus may be negative.

BTW, the condition seems incomplete because large values are forbidden.
ISO C17 says in 6.2.8p2:

  A fundamental alignment is a valid alignment less than or equal to
  _Alignof (max_align_t).

and in 6.7.5p3 (a constraint):

  The constant expression shall be an integer constant expression.
  It shall evaluate to a valid fundamental alignment, or to a valid
  extended alignment supported by the implementation for an object
  of the storage duration (if any) being declared, or to zero.

-- 
Vincent Lefèvre  - Web: 
100% accessible validated (X)HTML - Blog: 
Work: CR INRIA - computer arithmetic / AriC project (LIP, ENS-Lyon)

___
Tinycc-devel mailing list
Tinycc-devel@nongnu.org
https://lists.nongnu.org/mailman/listinfo/tinycc-devel

Re: [Tinycc-devel] Broken commit e460f7dbb216

[grischka]

On 28.07.2022 16:34, Detlef Riekenberg wrote:

Hi grischka.



Hi Detlef,

Please try to be more precise.  For example, in your last commit,
you wrote:

   "Do not fail with _Alignas(0) and _Alignas(1), used by autotools"

But there wasn't anything wrong with _Alignas(1), to begin with.
Also, the "(n > 1) &" part that you added in your one-line patch

   -  if (n <= 0 || (n & (n - 1)) != 0)
   +  if (n < 0 || ((n > 1) & ((n & (n - 1)) != 0)))

is just redundant.  Unnecessary redundancy confuses the readers, and
confused readers may add even more confusing stuff in sequence.
(Now I see that "(n > 1) &" has changed to "(n > 1) &&" which, well,
is still equally redundant.)

As to the "n < 0 ||" clause by the way. the C-standards seem to say:

  "Alignments are represented as values of the type size_t. Valid alignments
   include only fundamental alignments, plus an additional implementation-
   defined set of values, which may be empty.  Every valid alignment value
   shall be a nonnegative integral power of two."

Well, when "size_t" means unsigned, how could it be not "nonnegative"
then ?!?

As to your "My local workaround" below (-DTCC_LIBTCC1="\"libtcc1.a\"")
all I can say is that it does not have any effect in the context of a
tinycc as it is available in the public repo.

Maybe you do have some local configuration hacks in place that may or
may not have been disturbed.

--- grischka


After your commit related to CONFIG_TCC_CROSSPREFIX,
every compiler failed to work on Linux.
I rechecked with a clean checkout.

Broken commit:
https://repo.or.cz/tinycc.git/commitdiff/e460f7dbb2165112bc618816ec15be312b257de2

Message examples:
repo.or.cz_tinycc$ ./tcc helloworld.c -o helloworld_tcc
In file included from helloworld.c:6:
/usr/include/stdio.h:33: error: include file 'stddef.h' not found
repo.or.cz_tinycc$ ./i386-tcc helloworld.c -o helloworld_tcc
tcc: error: file 'crt1.o' not found
tcc: error: file 'crti.o' not found
In file included from helloworld.c:6:
/usr/include/stdio.h:33: error: include file 'stddef.h' not found



My local workaround:
diff --git a/Makefile b/Makefile
index 947f757..6d27eaf 100644
--- a/Makefile
+++ b/Makefile
@@ -175,10 +175,10 @@ DEFINES += $(DEF-$(or $(findstring win,$T),unx))

  ifneq ($(X),)
  ifeq ($(CONFIG_WIN32),yes)
-DEF-win += -DCONFIG_TCC_CROSSPREFIX="\"$X\""
-DEF-unx += -DCONFIG_TCC_CROSSPREFIX="\"lib/$X\""
+DEF-win += -DTCC_LIBTCC1="\"libtcc1.a\"" -DCONFIG_TCC_CROSSPREFIX="\"$X\""
+DEF-unx += -DTCC_LIBTCC1="\"libtcc1.a\"" -DCONFIG_TCC_CROSSPREFIX="\"lib/$X\""
  else
-DEF-all += -DCONFIG_TCC_CROSSPREFIX="\"$X\""
+DEF-all += -DTCC_LIBTCC1="\"libtcc1.a\"" -DCONFIG_TCC_CROSSPREFIX="\"$X\""
  DEF-win += -DCONFIG_TCCDIR="\"$(tccdir)/win32\""
  endif
  endif
And your patch is incomplete.
The cross prefix for libtcc1 is missing with -print-search-dirs


Can you please take a look?

Thanks


--
Bye bye ... Detlef


___
Tinycc-devel mailing list
Tinycc-devel@nongnu.org
https://lists.nongnu.org/mailman/listinfo/tinycc-devel



___
Tinycc-devel mailing list
Tinycc-devel@nongnu.org
https://lists.nongnu.org/mailman/listinfo/tinycc-devel