[fpc-devel] Fwd: Re: [FPC 0034456]: r40027 breaks build

[J. Gareth Moreton]

Accidentally sent to just Pierre.

            	
            	----- Original Message ---
--
            	From: J. Gareth Moreton 
gareth at moreton-family.com
            	To: "Pierre Muller" 
pierre at freepascal.org
            	Sent: Thu 25/10/18 07:14
            	Subject: Fwd: Re: [FPC 
0034456]: r40027 breaks build
 Hi Pierre,
 I realised afterwads that my additions to 
compiler/x86/x86ins.dat were
incorrect, as the far-call version of RET 
is, by default, 32-bit even on
64-bit platforms, whereas the near-call 
version is 64-bit, so adding
NOX86_64 was wrong.  I corrected the 
patch.

 I did wonder about whether adding 64-bit 
support was correct or not. 
Thanks for checking that.  The bug report 
was set to "fixed" since that's
the default option when I select 
"resolved".  Is there a better option to
select when submitting a patch?

 Gareth

 On Thu 25/10/18 07:53 , Pierre Muller 
pierre at freepascal.org sent:
  Hi all, 

 sorry about this ... 

 This s completely my fault, 
 I forgot that the list of instructions 
 is not the same for i8086, i386 and 
x86_64 ... 

 I just committed revision 40028, that 
fixes the compilation failure. 

 I would need some advices on the notes 
added by J. Gareth Moreton. 

 Hi Gareth, 

 I have a few comments on your notes on 
that bug report: 

 First, let me thank you for the fast 
reaction to the bug report, 
 but I think that your patch proposal is 
not correct, 
 because as_i386_wasm is a i386 specific 
assembler, and thus 
 adding code to handle x86_64 versions of 
the same instruction is useless 
 as long as Watcom assembler does not 
support 64-bit instruction. 

 Even if the would later, we would add a 
new as_x86_64_wasm assembler id
for this. 

 Second, I have no idea about the 
correctnes of your patch related 
 to x86ins.dat 

 Index: compiler/x86/x86ins.dat 
 =========================================
========================== 
 --- compiler/x86/x86ins.dat (revision 
40027) 
 +++ compiler/x86/x86ins.dat (working 
copy) 
 @@ -1754,8 +1754,8 @@ 

 [RETFD,lret] 
 (Ch_All) 
 -void 3251xCB 386 
 -imm 3251xCA30 386,SW 
 +void 3251xCB 386,NOX86_64 
 +imm 3251xCA30 386,SW,NOX86_64 

 [RETND,ret] 
 (Ch_All) 

 I have no idea if this instruction might 
be valid for embedded 
 target when trying to switch between 32 
and 64 bit code... 

 Can someone else from core team please 
comment on that part? 

 Third, I do not understand why you 
changed the bug status to 'fixed'. 
 Shouldn't we wait until a fix has been 
committed to the relevant branch to
change the 
 bug report status to 'fixed'? 

 I did re-open and changed it back to 
fixed, adding reference to commit
40028. 

 Thanks! 

 Pierre