Here are the changes I mentioned I would do: https://github.com/gynt/tinycc/pull/3 Let me know if it looks good then I will push it to mob.
On Mon, Jun 10, 2024 at 8:45 PM Edward Gynt <edwardg...@gmail.com> wrote: > 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
