Now I understand what you mean with the smart thing, thanks for clearing it up. I was inspired by this line <https://github.com/TinyCC/tinycc/blob/3b943bec5de423e234b5f92d9a8f110ad66a85a1/TODO#L9> from the "TODO" that says fastcall implementation is "mostly wrong". It didn't seem right to me to copy code that is known to be mostly wrong.
Anyway, I will change the "mov ecx" back into the "pop ecx" logic as per your request. And remove the test file. Does the "mov ecx" not work because the compiler would falsely believe ecx can be used again after a function call into windows 32 ABI while windows 32 can clobber it? Or what is the reason it won't work? Best of wishes, Gynt On Mon, Jun 10, 2024 at 8:14 PM grischka via Tinycc-devel < tinycc-devel@nongnu.org> wrote: > On 10.06.2024 11:06, Edward Gynt wrote: > > Hi Grischka, > > > > I am very new here, nice to meet you! I fixed the issues: > https://github.com/gynt/tinycc/pull/2 > > Let me know if I should push this to mob or whether you want to have a > look first. > > I hope this is "the route that's already proven to work instead of > trying to be smart" you meant. > > No, that is not what I meant. What I meant is what I wrote and > what I wrote is that the patch has > > >> - too much code ... anyway seen that > >> FUNC_THISCALL is identical to FUNC_FASTCALLW except > fastcall_nb_regs = 1; > >> instead of = 2; > > Merely logically, if some patch has a (way) "too much code" problem then > there is no hope that it can be fixed with even more code ... ;) > > You wrote: > > "implemented improved thiscall by using mov ecx instead of pop ecx" > > Yes sure... Don't do this. Don't try to be smart. > > And please remove the test that nobody except you will know how to use. > > -- gr > > > One note: > > > ECX may get overwritten when other params are loaded > > The "this" parameter is always the last one to be loaded (first > argument), so I don't think it can get overwritten by other params. > > EAX and ECX are scratch registers that can be clobbered after a function > call, is this true in tinycc too? > > To make sure ECX isn't also used in an indirect call, I wrote it now > such that it throws an error if ECX is used... Not very pretty, but good to > know if it happens. > > > > Best of wishes, > > > > Gynt > > > > On Sat, Jun 8, 2024 at 1:03 AM grischka via Tinycc-devel < > tinycc-devel@nongnu.org <mailto:tinycc-devel@nongnu.org>> wrote: > > > > On 07.06.2024 15:03, Edward Gynt wrote: > > > I am having trouble reproducing your issue. My .exe files output > the correct information. The "C file" code you mention doesn't mark main as > a __thiscall, but your decompiled function lists sub_401000 as __thiscall. > Why? > > > I don't own IDA so I can't reproduce with IDA. > > > > Hi Ed, > > > > honestly there are several issues with your patch. > > - thiscall_nb_regs uninitialized : > > causes compiled code crash mostly > > - load(get_reg(RC_ECX), vtop) : > > ECX may get overwritten when other params are loaded > > - too much code and funny variables (int func_call2) anyway seen that > > FUNC_THISCALL is identical to FUNC_FASTCALLW except > fastcall_nb_regs = 1; > > instead of = 2; > > > > So you'd really better go along the route that's already proven to > > work instead of trying to be smart ;) > > > > -- gr > > > > > What I do notice is that using i386-win32-tcc.exe (compiled with > gcc or tcc) I get identical output to before my commit. > > > With tcc.exe (compiled with gcc or tcc, which in turns was built > with build-tcc.bat -x), I get the situation that ecx is stored in the stack > and then moved into eax. Very strange. > > > So the x86_64 build is affected. Before I start digging into the > code (misplaced #ifdef somewhere?) I want to verify the byte code with you > to make sure we are talking about the same issue. > > > Can you post yours (objdump -D) ? > > > > > > Compiled with version on branch mob after my commit: > > > > > _______________________________________________ > 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
