This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[RFA:] CGEN sim operation extension fix; frv adjustment needed.


(CGEN for the stricter semantics, gdb-patches for containing a
sim patch, binutils for being the project "owning" src/cpu.)

Recent gcc (for example, gcc-4.3) has started to optimize for
signed operations having undefined overflow semantics (i.e. it
validly optimizes for the overflow not happening, a restricted
domain of the input operands; please don't question the validity
of the optimization here, go see the gcc archives if you're
interested).

For both cris-elf and frv-elf on a i686-pc-linux-gnu host with
4.3.0, you get the FAILs as below, because it gets problematic
to check e.g. the sign-bit of the result of:

 (mul DI (zext siop1) (zext siop2))

which, as you know, is 1 for siop1=0xffffffff siop2=0xffffffff...

Also, suddenly it's much harder to make (abs SI 0x80000000) stay
0x80000000 and has bit 31 on.

cris-elf:
Running /home/hp/sim/src/sim/testsuite/sim/cris/asm/asm.exp ...
FAIL: crisv10 mulx.ms (execution)
FAIL: crisv32 abs.ms (execution)
FAIL: crisv32 mulx.ms (execution)

frv-elf:
Running /home/hp/sim/src/sim/testsuite/sim/frv/allinsn.exp ...
FAIL: frv umulcc.cgs (execution)
FAIL: fr500 umulcc.cgs (execution)
FAIL: fr550 umulcc.cgs (execution)
FAIL: fr400 umulcc.cgs (execution)
FAIL: fr405 umulcc.cgs (execution)
FAIL: fr450 umulcc.cgs (execution)
FAIL: frv umulicc.cgs (execution)
FAIL: fr500 umulicc.cgs (execution)
FAIL: fr550 umulicc.cgs (execution)
FAIL: fr400 umulicc.cgs (execution)
FAIL: fr405 umulicc.cgs (execution)
FAIL: fr450 umulicc.cgs (execution)

CGEN operations are supposed to be sign-agnostic and are
supposed to handle all bit-values as input(!), so to yield
defined consistent and testable results, it should perform the
operations internally using unsigned operands, which has the
desired (and defined) wrap-semantics.  In order to be consistent
with the signedness of e.g. temporary variables, this requires
casting the values back to the original type (that is, signed).

So, besides accommodating current optimizations, it's an
improvement in consistency.  Currently, the signedness of the
result of an operation is C-ish due to the operator
implementation in cgen-ops.h.  Because cgen allows
sloppy^wimplicit size conversion (no explicit ext/zext
required), the result depends on the type of the operands, which
are implicit in the case of literal numbers.  In an operation
like:

 (add HI op1 65535)

the operands are promoted to int and the signedness carries from
that.  This means that if you assign the result as in e.g.:

(sequence ((DI tmp) (HI tmp2)) (set tmp (add HI op1 65535)))

you'll get positive results for all positive op1 - as opposed to
assigning it first to a temporary variable:

(sequence ((DI tmp) (HI tmp2))
 (set tmp2 (add HI op1 65535))
 (set tmp tmp2))

I really don't think that's how it's intended to be.

Unfortunately, the frv sim depends on the current inconsistency
in it's comparisons for saturation.  For example, with *only*
the cgen-ops.h patch below only, you'd see:

Running /tmp/hpautotest-sim/src/sim/testsuite/sim/frv/allinsn.exp ...
FAIL: frv cmaddhss.cgs (execution)
FAIL: fr500 cmaddhss.cgs (execution)
FAIL: fr400 cmaddhss.cgs (execution)
FAIL: frv cmaddhus.cgs (execution)
FAIL: fr500 cmaddhus.cgs (execution)
FAIL: fr400 cmaddhus.cgs (execution)
FAIL: frv cmsubhss.cgs (execution)
FAIL: fr500 cmsubhss.cgs (execution)
FAIL: fr400 cmsubhss.cgs (execution)
FAIL: frv cmsubhus.cgs (execution)
FAIL: fr500 cmsubhus.cgs (execution)
FAIL: fr400 cmsubhus.cgs (execution)
FAIL: fr400 mabshs.cgs (execution)
FAIL: frv maddhss.cgs (execution)
FAIL: fr500 maddhss.cgs (execution)
FAIL: fr400 maddhss.cgs (execution)
FAIL: frv maddhus.cgs (execution)
FAIL: fr500 maddhus.cgs (execution)
FAIL: fr400 maddhus.cgs (execution)
FAIL: frv msubhss.cgs (execution)
FAIL: fr500 msubhss.cgs (execution)
FAIL: fr400 msubhss.cgs (execution)
FAIL: frv msubhus.cgs (execution)
FAIL: fr500 msubhus.cgs (execution)
FAIL: fr400 msubhus.cgs (execution)
FAIL: frv mtrap.cgs (execution)
FAIL: fr500 mtrap.cgs (execution)
FAIL: fr400 mtrap.cgs (execution)
(more errors elided)

Changing to extend the operands explicitly helps.

These FSF sims are CGEN-generated: cris, frv, iq2000, m32r, sh64.
All but iq2000 have testsuites, supposedly covering edge cases
like the ones we're talking about here.  The iq2000 has none.

I reviewed the iq2000 port which at a glance seems pretty
trivial and not affected by this bug.  For the record, ado162
and ado16 looks like they have a sign-extension-related bug;
consider what happens in the last "or" operation when "low" has
bit 15 set.  You'd get 0xffff in the upper 16 bits, right?
Still, that's an implicit sign-extension from a local variable,
not from an operation, hence unaffected by this patch.

No regressions with the patches below for any of the other sims.
I've started gcc regression tests for frv, m32r and sh64 to
raise confidence and will make sure there are no regressions
before committing.

Anyway, over to the patches.  First, there's the frv.cpu patch,
making the implicit promotions explicit and the result defined
and as expected, independently of the cgen-ops.h patch.  There
are other ways, like assigning through a temporary of the
correct mode or to pass the extend operator as a parameter or
maybe the maitainer has another preferred route.  In any case, I
don't have commit authority over src/cpu which is defined as
belonging to the binutils domain, so...  Ok to commit?

src/cpu:
	* frv.cpu (mabshs): Explicitly sign-extend arguments of abs to DI.
	(DI-ext-HI, DI-ext-UHI, DI-ext-DI): New pmacros.
	(media-arith-sat-semantics): Explicitly sign- or zero-extend
	arguments of "operation" to DI using "mode" and the new pmacros.

Index: frv.cpu
===================================================================
RCS file: /cvs/src/src/cpu/frv.cpu,v
retrieving revision 1.24
diff -u -p -r1.24 frv.cpu
--- frv.cpu	5 Jul 2007 09:49:03 -0000	1.24
+++ frv.cpu	23 Dec 2008 21:11:06 -0000
@@ -8229,18 +8229,28 @@
 	       (set FRintk (c-raw-call SI "frv_ref_SI" FRintk))
 	       (set arghi (halfword hi FRintj 0))
 	       (set arglo (halfword lo FRintj 0))
-	       (saturate-v (abs arghi) 32767 -32768 (msr-sie-fri-hi)
+	       ; We extend the argument before the abs operation so we can
+	       ; notice -32768 overflowing as 32768.
+	       (saturate-v (abs (ext DI arghi)) 32767 -32768 (msr-sie-fri-hi)
 			   (halfword hi FRintk 0))
-	       (saturate-v (abs arglo) 32767 -32768 (msr-sie-fri-lo)
+	       (saturate-v (abs (ext DI arglo)) 32767 -32768 (msr-sie-fri-lo)
 			   (halfword lo FRintk 0)))
      ((fr400 (unit u-media-1)) (fr450 (unit u-media-1))
       (fr550 (unit u-media)))
 )
 
+; How to extend from a mode to get the intended signedness.
+(define-pmacro (DI-ext-HI x) (ext DI x))
+(define-pmacro (DI-ext-UHI x) (zext DI x))
+(define-pmacro (DI-ext-DI x) x)
+
 (define-pmacro (media-arith-sat-semantics
 		operation arg1 arg2 res mode max min sie)
   (sequence ((DI tmp))
-	    (set tmp (operation arg1 arg2))
+	    ; Make sure we saturate at max/min against a value that is
+	    ; sign- or zero-extended appropriately from "mode".
+	    (set tmp (operation DI
+		      ((.sym DI-ext- mode) arg1) ((.sym DI-ext- mode) arg2)))
 	    (saturate-v tmp max min sie res))
 )
 

The (for me) more interesting patch, as mentioned, keeps the
result of an additive and multiplicative operation, staying in
the mode of the operation.  I didn't touch logical operators
because they don't "overflow" and I also didn't touch div and
mod, because they're too loosely defined in corner cases, so
they require domain-specific testing of the input operands
anyway (consider 0x80..01 / -1 on a x86 host).

Committing this patch as-is would expose the longer list of
FAILs seen above for frv-elf sim, so I'll wait until the frv
issue is resolved and gcc regtests for frv, m32r and sh64 have
completed - or of course if someone has a strong argument
against it.

src/sim/common:
	* cgen-ops.h (ADDQI, SUBQI, MULQI, NEGQI, ABSQI, ADDHI, SUBHI)
	(MULHI, NEGHI, ABSHI, ADDSI, SUBSI, MULSI, NEGSI, ABSSI, ADDDI)
	(SUBDI, MULDI, NEGDI, ABSDI): Cast arguments to the unsigned type
	variant; UQI, UHI, USI, UDI, and cast the result to the signed
	type, QI, HI, SI, DI.

Index: cgen-ops.h
===================================================================
RCS file: /cvs/src/src/sim/common/cgen-ops.h,v
retrieving revision 1.9
diff -p -u -r1.9 cgen-ops.h
--- cgen-ops.h	1 Jan 2008 22:53:23 -0000	1.9
+++ cgen-ops.h	23 Dec 2008 02:35:47 -0000
@@ -59,9 +59,9 @@ along with this program.  If not, see <h
 #define GTUBI(x, y) ((BI) (x) > (BI) (y))
 #define GEUBI(x, y) ((BI) (x) >= (BI) (y))
 
-#define ADDQI(x, y) ((x) + (y))
-#define SUBQI(x, y) ((x) - (y))
-#define MULQI(x, y) ((x) * (y))
+#define ADDQI(x, y) ((QI) ((UQI) (x) + (UQI) (y)))
+#define SUBQI(x, y) ((QI) ((UQI) (x) - (UQI) (y)))
+#define MULQI(x, y) ((QI) ((UQI) (x) * (UQI) (y)))
 #define DIVQI(x, y) ((QI) (x) / (QI) (y))
 #define UDIVQI(x, y) ((UQI) (x) / (UQI) (y))
 #define MODQI(x, y) ((QI) (x) % (QI) (y))
@@ -74,10 +74,10 @@ extern QI ROLQI (QI, int);
 #define ANDQI(x, y) ((x) & (y))
 #define ORQI(x, y) ((x) | (y))
 #define XORQI(x, y) ((x) ^ (y))
-#define NEGQI(x) (- (x))
+#define NEGQI(x) ((QI) (- (UQI) (x)))
 #define NOTQI(x) (! (QI) (x))
 #define INVQI(x) (~ (x))
-#define ABSQI(x) ((x) < 0 ? -(x) : (x))
+#define ABSQI(x) ((QI) ((QI) (x) < 0 ? -(UQI) (x) : (UQI) (x)))
 #define EQQI(x, y) ((QI) (x) == (QI) (y))
 #define NEQI(x, y) ((QI) (x) != (QI) (y))
 #define LTQI(x, y) ((QI) (x) < (QI) (y))
@@ -89,9 +89,9 @@ extern QI ROLQI (QI, int);
 #define GTUQI(x, y) ((UQI) (x) > (UQI) (y))
 #define GEUQI(x, y) ((UQI) (x) >= (UQI) (y))
 
-#define ADDHI(x, y) ((x) + (y))
-#define SUBHI(x, y) ((x) - (y))
-#define MULHI(x, y) ((x) * (y))
+#define ADDHI(x, y) ((HI) ((UHI) (x) + (UHI) (y)))
+#define SUBHI(x, y) ((HI) ((UHI) (x) - (UHI) (y)))
+#define MULHI(x, y) ((HI) ((UHI) (x) * (UHI) (y)))
 #define DIVHI(x, y) ((HI) (x) / (HI) (y))
 #define UDIVHI(x, y) ((UHI) (x) / (UHI) (y))
 #define MODHI(x, y) ((HI) (x) % (HI) (y))
@@ -104,10 +104,10 @@ extern HI ROLHI (HI, int);
 #define ANDHI(x, y) ((x) & (y))
 #define ORHI(x, y) ((x) | (y))
 #define XORHI(x, y) ((x) ^ (y))
-#define NEGHI(x) (- (x))
+#define NEGHI(x) ((HI) (- (UHI) (x)))
 #define NOTHI(x) (! (HI) (x))
 #define INVHI(x) (~ (x))
-#define ABSHI(x) ((x) < 0 ? -(x) : (x))
+#define ABSHI(x) ((HI) ((HI) (x) < 0 ? -(UHI) (x) : (UHI) (x)))
 #define EQHI(x, y) ((HI) (x) == (HI) (y))
 #define NEHI(x, y) ((HI) (x) != (HI) (y))
 #define LTHI(x, y) ((HI) (x) < (HI) (y))
@@ -119,9 +119,9 @@ extern HI ROLHI (HI, int);
 #define GTUHI(x, y) ((UHI) (x) > (UHI) (y))
 #define GEUHI(x, y) ((UHI) (x) >= (UHI) (y))
 
-#define ADDSI(x, y) ((x) + (y))
-#define SUBSI(x, y) ((x) - (y))
-#define MULSI(x, y) ((x) * (y))
+#define ADDSI(x, y) ((SI) ((USI) (x) + (USI) (y)))
+#define SUBSI(x, y) ((SI) ((USI) (x) - (USI) (y)))
+#define MULSI(x, y) ((SI) ((USI) (x) * (USI) (y)))
 #define DIVSI(x, y) ((SI) (x) / (SI) (y))
 #define UDIVSI(x, y) ((USI) (x) / (USI) (y))
 #define MODSI(x, y) ((SI) (x) % (SI) (y))
@@ -134,10 +134,10 @@ extern SI ROLSI (SI, int);
 #define ANDSI(x, y) ((x) & (y))
 #define ORSI(x, y) ((x) | (y))
 #define XORSI(x, y) ((x) ^ (y))
-#define NEGSI(x) (- (x))
+#define NEGSI(x) ((SI) (- (USI) (x)))
 #define NOTSI(x) (! (SI) (x))
 #define INVSI(x) (~ (x))
-#define ABSSI(x) ((x) < 0 ? -(x) : (x))
+#define ABSSI(x) ((SI) ((SI) (x) < 0 ? -(USI) (x) : (USI) (x)))
 #define EQSI(x, y) ((SI) (x) == (SI) (y))
 #define NESI(x, y) ((SI) (x) != (SI) (y))
 #define LTSI(x, y) ((SI) (x) < (SI) (y))
@@ -179,9 +179,9 @@ extern int LEUDI (UDI, UDI);
 extern int GTUDI (UDI, UDI);
 extern int GEUDI (UDI, UDI);
 #else /* ! DI_FN_SUPPORT */
-#define ADDDI(x, y) ((x) + (y))
-#define SUBDI(x, y) ((x) - (y))
-#define MULDI(x, y) ((x) * (y))
+#define ADDDI(x, y) ((DI) ((UDI) (x) + (UDI) (y)))
+#define SUBDI(x, y) ((DI) ((UDI) (x) - (UDI) (y)))
+#define MULDI(x, y) ((DI) ((UDI) (x) * (UDI) (y)))
 #define DIVDI(x, y) ((DI) (x) / (DI) (y))
 #define UDIVDI(x, y) ((UDI) (x) / (UDI) (y))
 #define MODDI(x, y) ((DI) (x) % (DI) (y))
@@ -194,10 +194,10 @@ extern DI ROLDI (DI, int);
 #define ANDDI(x, y) ((x) & (y))
 #define ORDI(x, y) ((x) | (y))
 #define XORDI(x, y) ((x) ^ (y))
-#define NEGDI(x) (- (x))
+#define NEGDI(x) ((DI) (- (UDI) (x)))
 #define NOTDI(x) (! (DI) (x))
 #define INVDI(x) (~ (x))
-#define ABSDI(x) ((x) < 0 ? -(x) : (x))
+#define ABSDI(x) ((DI) ((DI) (x) < 0 ? -(UDI) (x) : (UDI) (x)))
 #define EQDI(x, y) ((DI) (x) == (DI) (y))
 #define NEDI(x, y) ((DI) (x) != (DI) (y))
 #define LTDI(x, y) ((DI) (x) < (DI) (y))

Happy Holidays and seasonal greetings.
brgds, H-P


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]