Hello,
2014-05-04 2:44 GMT+08:00 Michael Matz matz@frakked.de:
Hello,
okay, are the last commits to mob from jiang meant as joke or vandalism?
* 32bit code generation is hosed already in the testsuite, * gawk doesn't
work anymore even for x86_64,
* arm codegen is broken already in the testsuite (adding an internal
compiler error)
* they contain ugly white-space changes making review exceedingly hard
* despite the unnecessarily hard review I think there are numerous
problems in the actual implementation:
+ the new parse_number uses inexact floating point directly (e.g. 1.0L/b
when b==10 isn't exactly representable, cumulating errors while
parsing)
+ There's a new subtype VT_VLS meaning VLA plus STRUCT, which makes no
sense at all (VLA is VL _array_)
+ TREG_MEM (also new) doesn't follow convention for type flags, and
seems like a layering violation
+ It reverts a cleanup by Thomas (eda2c756edc4dca004ba217) without
discussion
+ It renames libtcc1.a to libcrt.a, thereby trading a sensibly
tcc-specific name for something tcc-specific with something generic
(what if gcc had libcrt as well?)
+ It increases VT_STRUCT_SHIFT to 20, breaking bitfields larger than 31
bits (we needs 12 bits to encode bitfield position and size, so the
maximum bit shift can be 19
+ It changes gv2() so that VT_CMP/VT_JMP results aren't special-cased
anymore, without obvious compensation in all its users to avoid the
errors that the comment specifically mentioned
+ It implements some strange non-standard preprocessor extension
push_macro/pop_macro (as pragmas) without discussion; it enlargens
some heavily used internal data structures for this.
+ It adds some fix x86-64 vla commit, without testcase showing what's
actually broken, and for that shuffles the internal code generations
in large and unobvious ways (and removes the correct calls to alloca()
on x86-64 PE)
And that's just what I saw on a cursory read of the commits. Due to the
white-space changes the more intricate parts are terrible to review and I've
skipped them.
When I wrote above without discussion, then this was just for the most
controversial parts. It's true for all the patches. I've seen no messages
at all from jiang to this mailing list. No discussion about implementation
approaches, no discussions about bugs, no nothing. The commit messages are
mostly non-informative as well.
All in all I think this approach is pretty unacceptable, but others here
might differ. If the patch series were a smaller then the problems in it
could reasonably be fixed after the fact by others. But as it stands we now
have something in which every single one of the 22 topmost patches (ignoring
the white-space fixup patch from grischka) has issues.
If it were just my project I'd be tempted to revert the whole mob state to
be before your (jiangs) patches, and expect you to work with the community
to fix what you actually wanted to fix or improve. (From the patch series I
gather that one thing you wanted to fix was parameter passing on stack when
memcpy is needed). It the very minimum you have to subscribe to this
mailing list (that's even listed in the mob rules), and of course also take
part in discussions. You also have to _review_ your patches before
commiting (you would have seen the useless white-space changes) and write
meaningful commit messages.
Any opinions from others?
IMO I'd urge jiang to create fork in github instead.
Ciao,
Michael.
___
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
