public inbox for binutils@sourceware.org
 help / color / mirror / Atom feed
From: Jan Beulich <jbeulich@suse.com>
To: Binutils <binutils@sourceware.org>
Subject: [PATCH 2/4] gas: maintain O_constant signedness in more cases
Date: Fri, 5 May 2023 15:01:57 +0200	[thread overview]
Message-ID: <905e0a5d-d49d-03a6-d441-302bc5c2da6a@suse.com> (raw)
In-Reply-To: <a55dc85d-47b4-8494-461a-f8b955cb033f@suse.com>

Unary '~' doesn't really produce an unsigned result. Neither does
subtraction (unless taking operand values into consideration). And an
abstract operator applied to two operands which aren't both unsigned
can't be assumed to yield an unsigned result; exceptions are
- shifts, where only signedness of the left hand operand matters,
- comparisons, which - unlike unary '!' - produce signed results (they
  deliver 0 or ~0, as opposed to '!', which yields 0 or 1),
- logical operators (yielding 0 or 1 and hence treated like unary '!').

While doing this (specifically while extending the all/quad testcase),
update .quad and .8byte documentation: With 64-bit architectures now
being common, it is highly inappropriate to state that these directives
unconditionally require bignums.
---
Similarly X_extrabit looks to be poorly maintained when folding
expressions ("Optimize common cases."); only binary '+' / '-' (and unary
operators) are updating it. A sub-aspect here is that
subtract_from_result(), when not passed an RHS X_extrabit, is (unlike
add_to_result()) passed 0 instead of whether the RHS value is negative
(there are two such cases). With X_unsigned properly maintained I wonder
though whether we really need X_extrabit.

Considering that octa.d has only two variants of expectations, I've
omitted the 3rd and 4th in quad2.d as well.

As to documentation: .{2,4,8}byte are documented to insert values
unaligned, but cons_worker() doesn't treat them any different than
.long etc. In particular, if defined, md_cons_align() is invoked. Is
this a doc error or a bug?

--- a/gas/doc/as.texi
+++ b/gas/doc/as.texi
@@ -4631,7 +4631,7 @@ Some machine configurations provide addi
 @end ifclear
 * 2byte::                       @code{.2byte @var{expressions}}
 * 4byte::                       @code{.4byte @var{expressions}}
-* 8byte::                       @code{.8byte @var{bignums}}
+* 8byte::                       @code{.8byte @var{expressions}}
 * Deprecated::                  Deprecated Directives
 @end menu
 
@@ -6579,14 +6579,16 @@ as in the @code{.section} (@pxref{Sectio
 @end ifset
 
 @node Quad
-@section @code{.quad @var{bignums}}
+@section @code{.quad @var{expressions}}
 
 @cindex @code{quad} directive
-@code{.quad} expects zero or more bignums, separated by commas.  For
-each bignum, it emits
 @ifclear bignum-16
-an 8-byte integer.  If the bignum won't fit in 8 bytes, it prints a
-warning message; and just takes the lowest order 8 bytes of the bignum.
+For 64-bit architectures, or more generally with any GAS configured to support
+64-bit target virtual addresses, this is like @samp{.int}, but emitting 64-bit
+quantities.  Otherwise @code{.quad} expects zero or more bignums, separated by
+commas.  For each item, it emits an 8-byte integer.  If a bignum won't fit in
+8 bytes, a warning message is printed and just the lowest order 8 bytes of the
+bignum are taken.
 @cindex eight-byte integer
 @cindex integer, 8-byte
 
@@ -6594,8 +6596,11 @@ The term ``quad'' comes from contexts in
 hence @emph{quad}-word for 8 bytes.
 @end ifclear
 @ifset bignum-16
-a 16-byte integer.  If the bignum won't fit in 16 bytes, it prints a
-warning message; and just takes the lowest order 16 bytes of the bignum.
+@code{.quad} expects zero or more bignums, separated by commas.  For
+each bignum, it emits a 16-byte integer.  If the bignum won't fit in 16
+bytes, it prints a warning message; and just takes the lowest order 16
+bytes of the bignum.
+@xref{Octa,,@code{.octa @var{bignums}}}.
 @cindex sixteen-byte integer
 @cindex integer, 16-byte
 @end ifset
@@ -7693,8 +7698,11 @@ long values into the output.
 @cindex eight-byte integer
 @cindex integer, 8-byte
 
-Like the @option{.2byte} directive, except that it inserts unaligned, eight
-byte long bignum values into the output.
+For 64-bit architectures, or more generally with any GAS configured to support
+64-bit target virtual addresses, this is like the @option{.2byte} directive,
+except that it inserts unaligned, eight byte long values into the output.
+Otherwise, like @ref{Quad,,@code{.quad @var{expressions}}}, it expects zero or
+more bignums, separated by commas.
 
 @node Deprecated
 @section Deprecated Directives
--- a/gas/expr.c
+++ b/gas/expr.c
@@ -1056,6 +1056,7 @@ operand (expressionS *expressionP, enum
 	      {
 		expressionP->X_add_number = ~ expressionP->X_add_number;
 		expressionP->X_extrabit ^= 1;
+		expressionP->X_unsigned = 0;
 	      }
 	    else if (c == '!')
 	      {
@@ -1816,6 +1817,7 @@ expr (int rankarg,		/* Larger # is highe
   while (op_left != O_illegal && op_rank[(int) op_left] > rank)
     {
       segT rightseg;
+      bool is_unsigned;
       offsetT frag_off;
 
       input_line_pointer += op_chars;	/* -> after operator.  */
@@ -1883,6 +1885,8 @@ expr (int rankarg,		/* Larger # is highe
 	  right.X_op_symbol = NULL;
 	}
 
+      is_unsigned = resultP->X_unsigned && right.X_unsigned;
+
       if (mode == expr_defer
 	  && ((resultP->X_add_symbol != NULL
 	       && S_IS_FORWARD_REF (resultP->X_add_symbol))
@@ -1895,7 +1899,7 @@ expr (int rankarg,		/* Larger # is highe
       if (md_optimize_expr (resultP, op_left, &right))
 	{
 	  /* Skip.  */
-	  ;
+	  is_unsigned = resultP->X_unsigned;
 	}
       else
 #endif
@@ -1928,12 +1932,14 @@ expr (int rankarg,		/* Larger # is highe
 	  add_to_result (resultP, symval_diff, symval_diff < 0);
 	  resultP->X_op = O_constant;
 	  resultP->X_add_symbol = 0;
+	  is_unsigned = false;
 	}
       else if (op_left == O_subtract && right.X_op == O_constant
 	       && (md_register_arithmetic || resultP->X_op != O_register))
 	{
 	  /* X - constant.  */
 	  subtract_from_result (resultP, right.X_add_number, right.X_extrabit);
+	  is_unsigned = false;
 	}
       else if (op_left == O_add && resultP->X_op == O_constant
 	       && (md_register_arithmetic || right.X_op != O_register))
@@ -1988,6 +1994,7 @@ expr (int rankarg,		/* Larger # is highe
 	      else
 		resultP->X_add_number
 		  = (valueT) resultP->X_add_number >> (valueT) v;
+	      is_unsigned = resultP->X_unsigned;
 	      break;
 	    case O_bit_inclusive_or:	resultP->X_add_number |= v; break;
 	    case O_bit_or_not:		resultP->X_add_number |= ~v; break;
@@ -1998,36 +2005,45 @@ expr (int rankarg,		/* Larger # is highe
 		 here.  */
 	    case O_subtract:
 	      subtract_from_result (resultP, v, 0);
+	      is_unsigned = false;
 	      break;
 	    case O_eq:
 	      resultP->X_add_number =
 		resultP->X_add_number == v ? ~ (offsetT) 0 : 0;
+	      is_unsigned = false;
 	      break;
 	    case O_ne:
 	      resultP->X_add_number =
 		resultP->X_add_number != v ? ~ (offsetT) 0 : 0;
+	      is_unsigned = false;
 	      break;
 	    case O_lt:
 	      resultP->X_add_number =
 		resultP->X_add_number <  v ? ~ (offsetT) 0 : 0;
+	      is_unsigned = false;
 	      break;
 	    case O_le:
 	      resultP->X_add_number =
 		resultP->X_add_number <= v ? ~ (offsetT) 0 : 0;
+	      is_unsigned = false;
 	      break;
 	    case O_ge:
 	      resultP->X_add_number =
 		resultP->X_add_number >= v ? ~ (offsetT) 0 : 0;
+	      is_unsigned = false;
 	      break;
 	    case O_gt:
 	      resultP->X_add_number =
 		resultP->X_add_number >  v ? ~ (offsetT) 0 : 0;
+	      is_unsigned = false;
 	      break;
 	    case O_logical_and:
 	      resultP->X_add_number = resultP->X_add_number && v;
+	      is_unsigned = true;
 	      break;
 	    case O_logical_or:
 	      resultP->X_add_number = resultP->X_add_number || v;
+	      is_unsigned = true;
 	      break;
 	    }
 	}
@@ -2065,10 +2081,11 @@ expr (int rankarg,		/* Larger # is highe
 	  resultP->X_op_symbol = make_expr_symbol (&right);
 	  resultP->X_op = op_left;
 	  resultP->X_add_number = 0;
-	  resultP->X_unsigned = 1;
 	  resultP->X_extrabit = 0;
 	}
 
+      resultP->X_unsigned = is_unsigned;
+
       if (retval != rightseg)
 	{
 	  if (retval == undefined_section)
--- a/gas/testsuite/gas/all/gas.exp
+++ b/gas/testsuite/gas/all/gas.exp
@@ -427,6 +427,11 @@ if { ![istarget "tic4x*-*-*"] && ![istar
 
 run_dump_test quad
 
+# ~ isn't an operator on PDP-11
+if { ![istarget "pdp11-*-*"] } {
+    run_dump_test quad2
+}
+
 # poor little PDP-11 can't handle 16-byte values
 if { ![istarget "pdp11-*-*"] } {
     run_dump_test octa
--- a/gas/testsuite/gas/all/octa.d
+++ b/gas/testsuite/gas/all/octa.d
@@ -6,3 +6,11 @@
 Contents of section .data:
  [^ ]* (ffff3344 55667788 99aabbcc ddeeffff|ffffeedd ccbbaa99 88776655 4433ffff) .*
  [^ ]* (00003444 55667788 99aabbcc ddeeffff|ffffeedd ccbbaa99 88776655 44340000) .*
+ [^ ]* (00000080 00000000 00000000 00000000|00000000 00000000 00000000 80000000) .*
+ [^ ]* (ffffffff 00000000 00000000 00000000|00000000 00000000 00000000 ffffffff) .*
+ [^ ]* (00000080 ffffffff ffffffff ffffffff|ffffffff ffffffff ffffffff 80000000) .*
+ [^ ]* (01000000 ffffffff ffffffff ffffffff|ffffffff ffffffff ffffffff 00000001) .*
+ [^ ]* (ffffff7f ffffffff ffffffff ffffffff|ffffffff ffffffff ffffffff 7fffffff) .*
+ [^ ]* (00000000 ffffffff ffffffff ffffffff|ffffffff ffffffff ffffffff 00000000) .*
+ [^ ]* (00000080 ffffffff ffffffff ffffffff|ffffffff ffffffff ffffffff 80000000) .*
+ [^ ]* (01000000 ffffffff ffffffff ffffffff|ffffffff ffffffff ffffffff 00000001) .*
--- a/gas/testsuite/gas/all/octa.s
+++ b/gas/testsuite/gas/all/octa.s
@@ -1,3 +1,11 @@
  .data
  .octa ~0x112233445566778899aabbcc0000
  .octa -347510587133311339321256747728896
+ .octa 0x80000000
+ .octa 0xffffffff
+ .octa -0x80000000
+ .octa -0xffffffff
+ .octa ~0x80000000
+ .octa ~0xffffffff
+ .octa 0 - 0x80000000
+ .octa 0 - 0xffffffff
--- a/gas/testsuite/gas/all/quad.d
+++ b/gas/testsuite/gas/all/quad.d
@@ -8,5 +8,6 @@ Contents of section (\.data|\$DATA\$):
  00.. (00000000 87654321 00000000 ffffffff|21436587 00000000 ffffffff 00000000|00000000 65872143 00000000 ffffffff|43218765 00000000 ffffffff 00000000) .*
  00.. (ffffffff 89abcdf0 ffffffff 80000000|f0cdab89 ffffffff 00000080 ffffffff|ffffffff ab89f0cd ffffffff 00800000|cdf089ab ffffffff 00008000 ffffffff) .*
  00.. (ffffffff 789abcdf ffffffff 00000001|dfbc9a78 ffffffff 01000000 ffffffff|ffffffff 9a78dfbc ffffffff 00000100|bcdf789a ffffffff 00010000 ffffffff) .*
+ 00.. (ffffffff 80000000 ffffffff 00000001|00000080 ffffffff 01000000 ffffffff|ffffffff 00800000 ffffffff 00000100|00008000 ffffffff 00010000 ffffffff) .*
  00.. (01234567 89abcdef fedcba98 76543211|efcdab89 67452301 11325476 98badcfe|23016745 ab89efcd dcfe98ba 54761132|cdef89ab 45670123 32117654 ba98fedc) .*
 #pass
--- a/gas/testsuite/gas/all/quad.s
+++ b/gas/testsuite/gas/all/quad.s
@@ -8,5 +8,8 @@
 	.quad	-0x87654321
 	.quad	-0xffffffff
 
+	.quad	0 - 0x80000000
+	.quad	0 - 0xffffffff
+
 	.quad	0x123456789abcdef
 	.quad	-0x123456789abcdef
--- /dev/null
+++ b/gas/testsuite/gas/all/quad2.d
@@ -0,0 +1,8 @@
+#objdump : -s -j .data -j "\$DATA\$"
+#name : .quad binary-not tests
+
+.*: .*
+
+Contents of section (\.data|\$DATA\$):
+ 0000 (ffffffff 7fffffff ffffffff 00000000|ffffff7f ffffffff 00000000 ffffffff) .*
+#pass
--- /dev/null
+++ b/gas/testsuite/gas/all/quad2.s
@@ -0,0 +1,3 @@
+	.data
+	.quad	~0x80000000
+	.quad	~0xffffffff


  parent reply	other threads:[~2023-05-05 13:02 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-05-05 13:00 [PATCH 0/4] improve range checking for certain constants on x86 Jan Beulich
2023-05-05 13:01 ` [PATCH 1/4] x86: tighten extend-to-32bit-address conditions Jan Beulich
2023-05-05 13:01 ` Jan Beulich [this message]
2023-05-05 13:03 ` [PATCH 3/4] gas: invoke md_optimize_expr() also for unary expressions Jan Beulich
2023-05-05 13:04 ` [PATCH 4/4] x86: further adjust extend-to-32bit-address conditions Jan Beulich

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=905e0a5d-d49d-03a6-d441-302bc5c2da6a@suse.com \
    --to=jbeulich@suse.com \
    --cc=binutils@sourceware.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).