tinycc-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [Tinycc-devel] Recent patches for the TinyCC 0.9.28 Release Candidat


From: Eric Raible
Subject: Re: [Tinycc-devel] Recent patches for the TinyCC 0.9.28 Release Candidate
Date: Wed, 7 Feb 2024 22:07:02 -0800

Detlef,

Thanks for taking the time to respond!  I'm new here, and still finding my way around.

Let me respond to everything:
> A custom allocator is not a new idea.  TinyCC has already a custom allocator and uses it by default.
I
'm using libtcc in my application, and I didn't see a way to tell libtcc to use my _application_'s allocator.
If by "custom allocator" you mean tcc_realloc()/tcc_realloc_debug() etc, that's fine, but unless
I can replace them the fact that it's "custom" does me no good.

> How often will it be useful for a libtcc user to force TinyCC to use an application provided allocator?

I have no idea, but if the application is a language that uses libtcc as a back-end then perhaps often.
libtcc is reliable enough to be part of long-lived sophisticated applications.  Such applications
often care a great deal about memory management.

> You add an overhead for every user of TinyCC for every memory allocation/free.

Yes.  The preexisting tcc_realloc() called realloc() directly, mine calls a function which calls realloc(). 
I measured it on some large input (40K lines of C) and saw no difference.

> * it is a far to big change for a release candidate

That is not up to me to evaluate.  But as the commit message says, the resulting code is quite
a bit more clear.  The diffs don't show that!  But if you look I"ll bet your eyeballs would agree.

> * it needs a lot of testing

I wasn't aware of the release candidate status, but I have tested it extensively.
With tcc's tests, with my application, with valgrind.  Personally, I'm hard to satisfy...
but I'm satisfied.

> * it adds more complexity

I would seriously claim that it is _simpler_.  Again, the diffs don't show that but
the resulting code is more modular and easier to read.  The #defines are much
better organized, and my addition of "void libc_free(void *ptr)" makes what was
slightly obscure code more intelligible.

Thanks again.  If it turns out that it needs to be reverted for the release, that's fine.
In that case, I'd certainly like it to become official after the release is tagged.

- Eric


On Wed, Feb 7, 2024 at 5:49 PM Detlef Riekenberg <wine.dev@web.de> wrote:
Hi TinyCC follower.


The current branch is a release candidate for TinyCC 0.9.28 since some month.

>From the active people here, only grischka has the rights to make a release,
and I suggest, that we should help him to be comfortable to make a release by:
* more testing
* add only minimal fixes to the current mob branch to increase stability
* avoid to push any new features for now

I further suggest, that for any patch, which is longer than one or two lines,
the change should be discussed here before pushing.


@grischka
I tested your recent fixes:
The i386-tcc search path works now. Thanks
(I tested with a simple Hello_World for X11 and a simple ppm viewer for X11)


@Eric Raible:
A custom allocator is not a new idea.
TinyCC has already a custom allocator and uses it by default.
How often will it be useful for a libtcc user to force TinyCC
to use an application provided allocator?
You add an overhead for every user of TinyCC for every memory allocation/free.

My additional comments to your patch:
* it is a far to big change for a release candidate
* it needs a lot of testing
* it adds more complexity

#####

@kbkpbot
Yes, there are many things missing for atomic support,
but when i read your patch, i'm not sure,
if the code works with any windows compiler, which is incompatible to gcc.
(I changed my SSD and msvc is not installed yet, so I can't test that now)

Reason: "__attribute__" and "__asm__" are gcc extensions.
Are they supported by MSVC?
(Yes, i saw, that you mostly just moved the code around...)

+#ifndef __TINYC__
+void __atomic_signal_fence(int memorder) __attribute__((alias("__tcc_atomic_signal_fence")));


In the meantime, a macro for a fence can help


#####

I suggest to revert both changes (with the cleanup from @Herman)
and wait until @grischka pushes a release.

@grischka, what do you think about reverting everything after your cleanup patch
and then push a release?


--
Bye bye ... Detlef


reply via email to

[Prev in Thread] Current Thread [Next in Thread]