Re: [Tinycc-devel] __asm__ "unknown constraint 't'" issue

[Thomas Preud'homme]

Le mercredi 06 août 2014, 18:40:19 Roy Tam a écrit :
> Hello,
> 
> 2014-08-06 18:27 GMT+08:00 YX Hao :
> > Hi there,
> > 
> > Here is what I found:
> > 
> > 
> > math.h:341: error: unknown constraint 't'
> > 
> > 
> > And more around this line.
> > 
> > I know little about asm. Can somebody take a look at it?
> 
> I have a little patch for that, someone please review it.
> Thank you.

Note that i386-asm.c is probably one of the piece of tcc I know the least (I 
think it comes second after the Windows stuff). If you are still interested in 
my review, see below.

> 
> diff --git a/i386-asm.c b/i386-asm.c
> index 664aade..1a24e30 100644
> --- a/i386-asm.c
> +++ b/i386-asm.c
> @@ -1029,6 +1029,9 @@ static inline int constraint_priority(const char *str)
> case 'i':
>  case 'm':
>  case 'g':
> +case 'f':
> +case 't':
> +case 'u':
>  pr = 4;
>  break;
>  default:

It would be nice if you take advantage of your patch to sort this block as 
with LC_COLLATE=C (f, i, g, m, t, u, I, N, M). I can't comment on the priority 
though, I didn't know about the priority between constraints.


> @@ -1211,6 +1214,11 @@ ST_FUNC void asm_compute_constraints(ASMOperand
> *operands,
>  if (!((op->vt->r & (VT_VALMASK | VT_LVAL | VT_SYM)) ==
> VT_CONST)) goto try_next;
>  break;
> +case 'f':
> +case 't':
> +case 'u':
> +/* float */
> +break;
>  case 'm':
>  case 'g':
>  /* nothing special to do because the operand is already in

As I said I'm not a specialist of this code but it seems to me that 't' should 
do reg = TREG_ST0, then add something that says it's a float register and 
finally call goto alloc_reg. For 'u' it would be similar with TREG_ST1 but it 
doesn't exist so it probably needs to be declared. For 'f' it should be any 
float register so you should probably follow what is done for 'r' I guess.

Right now no register is allocated so a random general register will be used 
(according to what's in the stack at the address &op->reg).

Best regards,

Thomas

signature.asc
Description: This is a digitally signed message part.
___
Tinycc-devel mailing list
Tinycc-devel@nongnu.org
https://lists.nongnu.org/mailman/listinfo/tinycc-devel

Re: [Tinycc-devel] tcc grammar problems

[Thomas Preud'homme]

Le mercredi 06 août 2014, 21:10:29 jiang a écrit :
> Hi, Thomas
> My p1 patch, there is a fatal flaw, but I know it was not satisfied!
> Thank you for writing to me.

Can you be more precise? What's the fatal flaw you are talking about?

> 
> But I have a patch, want to join the mob, excuse me, please review it!
> 
> My patch:See Attachment

Maybe you could explain what you are trying to fix with these two patches. I'd 
suggest to give them a more meaningful name (instead of p1, p2) and I'd 
appreciate if you could include them inline in the message. When doing this 
make sure that your MUA (Mail User Agent) doesn't mangle the message, that is 
keeps the original formatting.

Best regards,

Thomas

signature.asc
Description: This is a digitally signed message part.
___
Tinycc-devel mailing list
Tinycc-devel@nongnu.org
https://lists.nongnu.org/mailman/listinfo/tinycc-devel

Re: [Tinycc-devel] tcc grammar problems

[Thomas Preud'homme]

Le mardi 05 août 2014, 22:08:13 Thomas Preud'homme a écrit :
> Le vendredi 01 août 2014, 16:37:15 jiang a écrit :
> > my patch:See Attachment
> > You look at, if no problem, I'll push mob
> 
> Ok, here is what I noticed so far:
> 

[SNIP review part 1]

> 
> 
> Ok. I'll stop this for now and will continue to review tomorrow or the day
> after tomorrow. Wait before doing any modifications as I might have some
> more important remarks that would require a more important rewrite. However
> from what I've seen so far (the ctrl enum excepted) it seems to be going in
> the right direction.

Let's continue, shall we?

@@ -2024,67 +2058,78 @@ static void gen_cast(CType *type)


What about the "else if (sf) case before that? A cast from a float to a 
bitfield 
is possible and does not seem to be handled here. Am I wrong?


 gen_cast(type);
 }
 }
+} else {
+
 #ifndef TCC_TARGET_X86_64
-} else if ((dbt & VT_BTYPE) == VT_LLONG) {
-if ((sbt & VT_BTYPE) != VT_LLONG) {
-/* scalar to long long */
-/* machine independent conversion */
-gv(RC_INT);
-/* generate high word */
-if (sbt == (VT_INT | VT_UNSIGNED)) {
-vpushi(0);
+if ((dbt & VT_BTYPE) == VT_LLONG) {
+if ((sbt & VT_BTYPE) != VT_LLONG) {
+/* scalar to long long */
+/* machine independent conversion */
 gv(RC_INT);
-} else {
-if (sbt == VT_PTR) {
-/* cast from pointer to int before we apply
-   shift operation, which pointers don't 
support*/
-gen_cast(&int_type);
+/* generate high word */
+if (sbt == (VT_INT | VT_UNSIGNED)) {
+vpushi(0);
+gv(RC_INT);
+} else {
+if (sbt == VT_PTR) {
+/* cast from pointer to int before we apply
+   shift operation, which pointers don't 
support*/


You need to reformat this comment to don't go beyond 80 columns.


+gen_cast(&int_type);
+}
+gv_dup();
+vpushi(31);
+gen_op(TOK_SAR);
 }
-gv_dup();
-vpushi(31);
-gen_op(TOK_SAR);
+/* patch second register */
+vtop[-1].r2 = vtop->r;
+vpop();
 }
-/* patch second register */
-vtop[-1].r2 = vtop->r;
-vpop();
 }
 #else
-} else if ((dbt & VT_BTYPE) == VT_LLONG ||
-   (dbt & VT_BTYPE) == VT_PTR ||
-   (dbt & VT_BTYPE) == VT_FUNC) {
-if ((sbt & VT_BTYPE) != VT_LLONG &&
-(sbt & VT_BTYPE) != VT_PTR &&
-(sbt & VT_BTYPE) != VT_FUNC) {
-/* need to convert from 32bit to 64bit */
-int r = gv(RC_INT);
-if (sbt != (VT_INT | VT_UNSIGNED)) {
-/* x86_64 specific: movslq */
-o(0x6348);
-o(0xc0 + (REG_VALUE(r) << 3) + REG_VALUE(r));
+if((dbt & VT_BTYPE) == VT_LLONG || (dbt & VT_BTYPE) == VT_PTR 
||


For the same reason, you need to use a new line after the first ||.


+(dbt & VT_BTYPE) == VT_FUNC) {
+if ((sbt & VT_BTYPE) != VT_LLONG && (sbt & VT_BTYPE) != 
VT_PTR &&


And again here after the first &&.


+(sbt & VT_BTYPE) != VT_FUNC) {
+/* need to convert from 32bit to 64bit */
+int r = gv(RC_INT);
+if (sbt != (VT_INT | VT_UNSIGNED)) {
+/* x86_64 specific: movslq */
+o(0x6348);
+o(0xc0 + (REG_VALUE(r) << 3) + REG_VALUE(r));
+}
 }
 }
 #endif
-} else if (dbt == VT_BOOL) {
-/* scalar to bool */
-vpushi(0);
-gen_op(TOK_NE);
-} else if ((dbt & VT_BTYPE) == VT_BYTE || 
-   (dbt & VT_BTYPE) == VT_SHORT) {
-if (sbt == VT_PTR) {
-vtop->type.t = VT_INT;
-tcc_warning("nonportable conversion from pointer to 
char/short");
+else if (dbt == VT_BOOL) {
+   

Re: [Tinycc-devel] tcc grammar problems

[Thomas Preud'homme]

Le mardi 05 août 2014, 22:08:13 Thomas Preud'homme a écrit :
> 
> +warr = 0;
> +if(bb){
> +s = 64 - ((type->t >> (VT_STRUCT_SHIFT + 6)) &
> 0x3f);
> 
> Spacing issue again and I don't understand the + 6. The bitfield size is
> encoded from bit (1 << VT_STRUCT_SHIFT) on 6 bits. So you should do:
> (type->t >> VT_STRUCT_SHIFT) & 0x3f (that is remove all the bits before the
> bitfield size and then only keep the 6 least significant bits.

Sorry my apologize. I forgot there is 6 bits for the position and then again 6 
bits for the size. Please ignore above remark.

Best regards,

Thomas

signature.asc
Description: This is a digitally signed message part.
___
Tinycc-devel mailing list
Tinycc-devel@nongnu.org
https://lists.nongnu.org/mailman/listinfo/tinycc-devel

Re: [Tinycc-devel] tcc grammar problems

[jiang]

Hi, Thomas
My p1 patch, there is a fatal flaw, but I know it was not satisfied!
Thank you for writing to me.

But I have a patch, want to join the mob, excuse me, please review it!

My patch:See Attachment


Best regards,

Jiang

在 2014年08月05日 22:08, Thomas Preud'homme 写道:

Le vendredi 01 août 2014, 16:37:15 jiang a écrit :

my patch:See Attachment
You look at, if no problem, I'll push mob

Ok, here is what I noticed so far:

commit 1988c974137f3042d9c38000fda3e00779fecab3
Author: Jiang <30155...@qq.com>
Date:   Fri Aug 1 16:27:58 2014 +0800

 fix bitfields
 
 see:

 http://lists.nongnu.org/archive/html/tinycc-devel/2014-07/msg00023.html


"Fix casts to bitfield" followed by the quick testcase provided by grishka
would be better.


diff --git a/tcc.h b/tcc.h
index c93cedf..a8cabb6 100644
--- a/tcc.h
+++ b/tcc.h
@@ -1192,6 +1192,17 @@ ST_DATA int func_var; /* true if current function is
variadic */
  ST_DATA int func_vc;
  ST_DATA int last_line_num, last_ind, func_ind; /* debug last line number and
pc */
  ST_DATA char *funcname;
+/* gen_ctrl */
+enum {
+CTRL_NONE,
+CTRL_CALL,
+CTRL_FOCE,
+CTRL_ARGS,
+CTRL_RETS,
+CTRL_INIT,
+CTRL_USED,
+};
+ST_DATA int gen_ctrl;

A description of the use for each enumerator would be nice. Also, unless you
mean something else, I think you should rename CTRL_FOCE into CTRL_FORCE. Also
you seem to only use CTRL_FOCE and CTRL_INIT. So you should probably remove
all the others. And why separating the kind of check? Why not just a switch
which enables or not warnings? Please explain me why you feel the need for
this enum.
  
  ST_INLN int is_float(int t);

  ST_FUNC int ieee_finite(double d);
diff --git a/tccgen.c b/tccgen.c
index 1a89d4a..73b759f 100644
--- a/tccgen.c
+++ b/tccgen.c
@@ -70,6 +70,7 @@ ST_DATA int func_var; /* true if current function is
variadic (used by return in
  ST_DATA int func_vc;
  ST_DATA int last_line_num, last_ind, func_ind; /* debug last line number and
pc */
  ST_DATA char *funcname;
+ST_DATA int gen_ctrl;
  
  ST_DATA CType char_pointer_type, func_old_type, int_type, size_type;
  
@@ -1909,8 +1910,9 @@ static void force_charshort_cast(int t)

  /* cast 'vtop' to 'type'. Casting to bitfields is forbidden. */
  static void gen_cast(CType *type)
  {
-int sbt, dbt, sf, df, c, p;
+int sbt, dbt, dt, sf, df, c, p, bb;
  
+bb = type->t & VT_BITFIELD;


Why bb for the bitfield? Maybe you could use db (destination bitfield) to
follow the same naming convention as df (destination float). Also you should
add it just before the c, but that's really nitpick.

I'm not sure about dt. On one hand its the real destination basic type but on
the other hand dbt is already used.

  /* special delayed cast for char/short */
  /* XXX: in some cases (multiple cascaded casts), it may still
 be incorrect */
@@ -1925,9 +1927,10 @@ static void gen_cast(CType *type)
  }
  
  dbt = type->t & (VT_BTYPE | VT_UNSIGNED);

+dt = dbt & VT_BTYPE;
  sbt = vtop->type.t & (VT_BTYPE | VT_UNSIGNED);
  
-if (sbt != dbt) {

+if (sbt != dbt || bb) {
  sf = is_float(sbt);
  df = is_float(dbt);
  c = (vtop->r & (VT_VALMASK | VT_LVAL | VT_SYM)) == VT_CONST;
@@ -1959,6 +1962,8 @@ static void gen_cast(CType *type)
  vtop->c.d = (double)vtop->c.ld;
  } else if (sf && dbt == (VT_LLONG|VT_UNSIGNED)) {
  vtop->c.ull = (unsigned long long)vtop->c.ld;
+if(bb)
+goto to_min;
  } else if (sf && dbt == VT_BOOL) {
  vtop->c.i = (vtop->c.ld != 0);
  } else {
@@ -1975,24 +1980,53 @@ static void gen_cast(CType *type)
  else if (sbt != VT_LLONG)
  vtop->c.ll = vtop->c.i;
  
-if (dbt == (VT_LLONG|VT_UNSIGNED))

+if (dbt == (VT_LLONG|VT_UNSIGNED)){
  vtop->c.ull = vtop->c.ll;
-else if (dbt == VT_BOOL)
+if(bb)
+goto to_min;
+}else if (dbt == VT_BOOL)

Spacing issue. You should have a space before '{' and after '}'

  vtop->c.i = (vtop->c.ll != 0);
  #ifdef TCC_TARGET_X86_64
  else if (dbt == VT_PTR)
  ;
  #endif
  else if (dbt != VT_LLONG) {
-int s = 0;
-if ((dbt & VT_BTYPE) == VT_BYTE)
-s = 24;
-else if ((dbt & VT_BTYPE) == VT_SHORT)
-s = 16;
-if(dbt & VT_UNSIGNED)
-vtop->c.ui = ((unsigned int)vtop->c.ll << s) >> s;
-else
-vtop->c.i = ((int)vtop->c.ll << s) >> s;
+unsigned long long ull;
+long long ll;
+int s, warr;

It would be better to name this variable "warn".

+to_min:

Why not use the la

Re: [Tinycc-devel] __asm__ "unknown constraint 't'" issue

[Roy Tam]

Hello,

2014-08-06 18:27 GMT+08:00 YX Hao :
> Hi there,
>
> Here is what I found:
>
> 
> math.h:341: error: unknown constraint 't'
> 
>
> And more around this line.
>
> I know little about asm. Can somebody take a look at it?
>

I have a little patch for that, someone please review it.
Thank you.

diff --git a/i386-asm.c b/i386-asm.c
index 664aade..1a24e30 100644
--- a/i386-asm.c
+++ b/i386-asm.c
@@ -1029,6 +1029,9 @@ static inline int constraint_priority(const char *str)
 case 'i':
 case 'm':
 case 'g':
+case 'f':
+case 't':
+case 'u':
 pr = 4;
 break;
 default:
@@ -1211,6 +1214,11 @@ ST_FUNC void asm_compute_constraints(ASMOperand
*operands,
 if (!((op->vt->r & (VT_VALMASK | VT_LVAL | VT_SYM)) == VT_CONST))
 goto try_next;
 break;
+case 'f':
+case 't':
+case 'u':
+/* float */
+break;
 case 'm':
 case 'g':
 /* nothing special to do because the operand is already in

___
Tinycc-devel mailing list
Tinycc-devel@nongnu.org
https://lists.nongnu.org/mailman/listinfo/tinycc-devel

[Tinycc-devel] __asm__ "unknown constraint 't'" issue

[YX Hao]

Hi there,

Here is what I found:


math.h:341: error: unknown constraint 't'


And more around this line.

I know little about asm. Can somebody take a look at it?

Regards,
Yuxi


___
Tinycc-devel mailing list
Tinycc-devel@nongnu.org
https://lists.nongnu.org/mailman/listinfo/tinycc-devel