Re: [PATCH,mmix] convert to constraints.md
On Wed, 12 Sep 2012, Hans-Peter Nilsson wrote: On Wed, 12 Sep 2012, Nathan Froyd wrote: - Keeping old layout of mmix_reg_or_8bit_operand. That looked like a spurious change and I prefer the ior construct to the if_then_else. ISTR without this change, there were lots of assembly changes like: I'll try with your original patch and see it I can spot something. Nope, I see no differences in the generated code before/after the patch-patch below (applied to your original patch, except edited as if using --no-prefix, to fit with my other patches). Case closed: I don't think gen* mishandled neither construct. --- patch.nathanorig.adjusted 2012-09-12 12:33:34.0 +0200 +++ patch3 2012-09-14 14:42:31.0 +0200 @@ -364,7 +364,7 @@ diff --git a/gcc/config/mmix/predicates. index b5773b8..7fa3bf1 100644 --- gcc/config/mmix/predicates.md +++ gcc/config/mmix/predicates.md -@@ -149,7 +149,13 @@ +@@ -149,7 +149,14 @@ ;; True if this is a register or an int 0..255. (define_predicate mmix_reg_or_8bit_operand @@ -372,9 +372,10 @@ index b5773b8..7fa3bf1 100644 - (match_operand 0 register_operand) - (and (match_code const_int) - (match_test CONST_OK_FOR_LETTER_P (INTVAL (op), 'I') -+ (if_then_else (match_code const_int) -+(match_test satisfies_constraint_I (op)) -+(match_operand 0 register_operand))) ++ (ior ++ (match_operand 0 register_operand) ++ (and (match_code const_int) ++ (match_test satisfies_constraint_I (op) + +;; True if this is a memory address, possibly strictly. + brgds, H-P
Re: [PATCH,mmix] convert to constraints.md
- Original Message - Nathan, again thanks. There are a few minor tweaks compared to your version: Thanks for fixing this up! - Keeping old layout of mmix_reg_or_8bit_operand. That looked like a spurious change and I prefer the ior construct to the if_then_else. ISTR without this change, there were lots of assembly changes like: set rx, 6 cmp rz, ry, rx instead of the previous and better: cmp rz, ry, 6 (apologies if the assembly syntax isn't correct; hopefully the intent is clear) but if you double-checked that the assembly didn't change after your changes, maybe something else that you tweaked fixed this. - Replacing undefined-constraint-H with I instead of removing it. It was either renamed early or a genuine typo. Good catch. Hard not to see it; the gen* machinery complains about undefined constraints. :) -Nathan
Re: [PATCH,mmix] convert to constraints.md
On Wed, 12 Sep 2012, Nathan Froyd wrote: - Original Message - Nathan, again thanks. There are a few minor tweaks compared to your version: Thanks for fixing this up! - Keeping old layout of mmix_reg_or_8bit_operand. That looked like a spurious change and I prefer the ior construct to the if_then_else. ISTR without this change, there were lots of assembly changes like: set rx, 6 cmp rz, ry, rx instead of the previous and better: cmp rz, ry, 6 (apologies if the assembly syntax isn't correct; hopefully the intent is clear) Yes. My only guess is a typo in your previous edit, as the two constructs certainly should be equivalent in *what's* being recognized. If they aren't, we have a bug in the gen* machinery. If my construct is wrong, then that's a separate bug-fix, ehm. Seems worth it to double-check, not just for the sake of MMIX. but if you double-checked that the assembly didn't change after your changes, maybe something else that you tweaked fixed this. I have no clue; nothing in the patch below stands out - the missing I-constraint that may explain this (modulo that H wasn't recognizable either) was on an or operation, not a compare. But maybe one was close enough that it mattered. What revision was your baseline? My baseline was r190260 but configure-patches as posted (required to build, affecting crtstuff at most) and with: I'll try with your original patch and see it I can spot something. Index: gcc/config/mmix/predicates.md === --- gcc/config/mmix/predicates.md (revision 190682) +++ gcc/config/mmix/predicates.md (working copy) @@ -118,7 +118,7 @@ (define_predicate mmix_symbolic_or_addr return 1; /* Fall through. */ default: - return address_operand (op, mode); + return mmix_extra_constraint (op, 'U', reload_in_progress || reload_completed); } }) Index: gcc/config/mmix/mmix.md === --- gcc/config/mmix/mmix.md (revision 190682) +++ gcc/config/mmix/mmix.md (working copy) @@ -274,7 +275,7 @@ (define_insn anddi3 (define_insn iordi3 [(set (match_operand:DI 0 register_operand =r,r) (ior:DI (match_operand:DI 1 register_operand %r,0) - (match_operand:DI 2 mmix_reg_or_constant_operand rH,LS)))] + (match_operand:DI 2 mmix_reg_or_constant_operand rI,LS)))] @ OR %0,%1,%2 - Replacing undefined-constraint-H with I instead of removing it. It was either renamed early or a genuine typo. Good catch. Hard not to see it; the gen* machinery complains about undefined constraints. :) Exactly! :) You don't congratulate the machine, but the machinist - and sometimes the inventor of the machine. (Thank you, Zack!) brgds, H-P
Re: [PATCH,mmix] convert to constraints.md
On Fri, 31 Aug 2012, Hans-Peter Nilsson wrote: On Fri, 3 Aug 2012, Hans-Peter Nilsson wrote: Sorry, but I need more time to get test-results for a recent trunk revision to a state where testing patches makes sense. I'll test the patch and commit that or a variant. Thanks for your work. Nathan, again thanks. There are a few minor tweaks compared to your version: - U is a define_address_constraint, not plain define_constraint. - Keeping old layout of mmix_reg_or_8bit_operand. That looked like a spurious change and I prefer the ior construct to the if_then_else. - Replacing undefined-constraint-H with I instead of removing it. It was either renamed early or a genuine typo. Good catch. - Carrying over old comments at R constraint. Marking supposedly redundant constraints with FIXME:s. - Removing the now-unused MMIX_REG_OK_STRICT macros. - Using mmix_address_operand instead of address_operand in mmix_symbolic_or_address_operand. - Missing CL entry for mmix_emit_sp_add. You thought nobody reads them? ;) The redundant constraints and attempts to match const_double for integer scalars is an artefact from olden times when large integers (64 bits!) had to be modelled as const_doubles. A clean-up for later. Tested cross to mmix-knuth-mmixware and verified that the newlib libc and libm, libgcc and libstdc++ were the same after this patch as before (modulo a patch to fix H-I and the equivalent of the new mmix_address_operand in mmix_symbolic_or_address_operand). Committed. * config/mmix/mmix.h (MMIX_REG_OK_STRICT): Delete. (REG_CLASS_FROM_LETTER, CONST_OK_FOR_LETTER_P): Delete. (CONST_DOUBLE_OK_FOR_LETTER_P, EXTRA_CONSTRAINT): Delete. * config/mmix/mmix-protos.h (mmix_intval): Declare. (mmix_const_ok_for_letter_p, mmix_extra_constraint): Delete. (mmix_const_double_ok_for_letter_p): Delete. * config/mmix/constraints.md: New file. * config/mmix/mmix.md: Include it. (iordi3): Fix typo; use I instead of undefined H constraint. (*call_real): Update comment about not using the p constraint. * config/mmix/predicates.md (mmix_reg_or_8bit_operand): Use satisfies_constraint_I. (mmix_address_operand): New predicate. (mmix_symbolic_or_address_operand): Use it instead of address_operand. * config/mmix/mmix.c: #include tm-constrs.h. (mmix_intval): Delete declaration. Make non-static. (mmix_const_ok_for_letter_p, mmix_extra_constraint): Delete. (mmix_const_double_ok_for_letter_p): Delete. (mmix_legitimate_address_p): Use satisfies_constraint_I. (mmix_print_operand_address): Likewise. (mmix_emit_sp_add): Adjust to use insn_const_int_ok_for_constraint when matching L constraint. Index: gcc/config/mmix/mmix.h === --- gcc/config/mmix/mmix.h (revision 190682) +++ gcc/config/mmix/mmix.h (working copy) @@ -72,12 +72,6 @@ along with GCC; see the file COPYING3. untouched by the epilogue. */ #define MMIX_EH_RETURN_STACKADJ_REGNUM MMIX_STATIC_CHAIN_REGNUM -#ifdef REG_OK_STRICT -# define MMIX_REG_OK_STRICT 1 -#else -# define MMIX_REG_OK_STRICT 0 -#endif - #define MMIX_FUNCTION_ARG_SIZE(MODE, TYPE) \ ((MODE) != BLKmode ? GET_MODE_SIZE (MODE) : int_size_in_bytes (TYPE)) @@ -439,11 +433,6 @@ enum reg_class #define INDEX_REG_CLASS GENERAL_REGS -#define REG_CLASS_FROM_LETTER(CHAR)\ - ((CHAR) == 'x' ? SYSTEM_REGS \ - : (CHAR) == 'y' ? REMAINDER_REG \ - : (CHAR) == 'z' ? HIMULT_REG : NO_REGS) - #define REGNO_OK_FOR_BASE_P(REGNO) \ ((REGNO) = MMIX_LAST_GENERAL_REGISTER\ || (REGNO) == MMIX_ARG_POINTER_REGNUM\ @@ -460,16 +449,6 @@ enum reg_class #define CLASS_MAX_NREGS(CLASS, MODE) HARD_REGNO_NREGS (CLASS, MODE) -#define CONST_OK_FOR_LETTER_P(VALUE, C)\ - mmix_const_ok_for_letter_p (VALUE, C) - -#define EXTRA_CONSTRAINT(VALUE, C) \ - mmix_extra_constraint (VALUE, C, MMIX_REG_OK_STRICT) - -/* Do we need anything serious here? Yes, any FLOT constant. */ -#define CONST_DOUBLE_OK_FOR_LETTER_P(VALUE, C) \ - mmix_const_double_ok_for_letter_p (VALUE, C) - /* Node: Frame Layout */ Index: gcc/config/mmix/predicates.md === --- gcc/config/mmix/predicates.md (revision 190682) +++ gcc/config/mmix/predicates.md (working copy) @@ -118,7 +118,7 @@ (define_predicate mmix_symbolic_or_addr return 1; /* Fall through. */ default: - return address_operand (op, mode); + return mmix_address_operand (op, mode); } }) @@ -152,4 +152,12 @@ (define_predicate mmix_reg_or_8bit_oper (ior (match_operand 0 register_operand) (and (match_code const_int) - (match_test CONST_OK_FOR_LETTER_P (INTVAL
Re: [PATCH,mmix] convert to constraints.md
On Fri, 3 Aug 2012, Hans-Peter Nilsson wrote: On Thu, 2 Aug 2012, Nathan Froyd wrote: H-P, if you'd like to test beforehand, that'd be great. ...yes, I'd like to test this before, please. Thanks for your work. (Let's set a timeout at the end of August; ok to commit Sep 1 have I not got back to you.) This is that call. Sorry, but I need more time to get test-results for a recent trunk revision to a state where testing patches makes sense. I'll test the patch and commit that or a variant. Thanks for your work. If you have general patches depending on this, e.g. that gets rid of the old constraint methods, don't let this hold you back. brgds, H-P
Re: [PATCH,mmix] convert to constraints.md
On Thu, 2 Aug 2012, Nathan Froyd wrote: As $SUBJECT says. There's not too much interesting here. I did a fairly literal-minded conversion, so it's possible there's smarter ways to do some things. Doesn't look too bad though, but ... Compiled with cross to mmix-knuth-mmixware and spot-checked by comparing libgcc object files. I have no idea how to set up a simulator environment. Find the MMIXware simulator (mmix), install it. Following the links from our further readings page will take you to http://www-cs-faculty.stanford.edu/~knuth/mmix-news.html or http://mmix.cs.hm.edu/src/index.html which both have links to the simulator. At time of this reading, the links point to identical tarballs, for example is http://mmix.cs.hm.edu/src/mmix-20110831.tar.gz. A dejagnu baseboard was in dejagnu-1.4.4 already, use RUNTESTFLAGS=--target_board=mmixware-sim, which should not be a surprise to you, have you ever tested using a simulator before. Though if nothing changed in GCC in the last month (ha-ha), expect a lot of timeouts, and lots of LTO FAILs. Also, better wrap the executable in a script that does e.g. ulimit -v 30, as the simulator allocates memory on access (kind of like stack on modern systems), making debugging a bit harder than expected and letting the oomkiller run around out of control otherwise. (On my virtual todo-list to have MMIXware patches for controlled memory allocation; right after looking into the LTO FAILs.) H-P, if you'd like to test beforehand, that'd be great. ...yes, I'd like to test this before, please. Thanks for your work. (Let's set a timeout at the end of August; ok to commit Sep 1 have I not got back to you.) (Now, from where did I get that H constraint?) brgds, H-P
[PATCH,mmix] convert to constraints.md
As $SUBJECT says. There's not too much interesting here. I did a fairly literal-minded conversion, so it's possible there's smarter ways to do some things. Compiled with cross to mmix-knuth-mmixware and spot-checked by comparing libgcc object files. I have no idea how to set up a simulator environment. H-P, if you'd like to test beforehand, that'd be great. OK to commit? -Nathan * config/mmix/mmix.h (REG_CLASS_FROM_LETTER): Delete. (CONST_OK_FOR_LETTER_P, CONST_DOUBLE_OK_FOR_LETTER_P): Delete. (EXTRA_CONSTRAINT): Delete. * config/mmix/mmix-protos.h (mmix_intval): Declare. (mmix_const_ok_for_letter_p, mmix_extra_constraint): Delete. (mmix_const_double_ok_for_letter_p): Delete. * config/mmix/constraints.md: New file. * config/mmix/mmix.md: Include it. (iordi3): Delete dead H constraint. * config/mmix/predicates.md (mmix_reg_or_8bit_operand): Use satisfies_constraint_I. (mmix_address_operand): New predicate. * config/mmix/mmix.c: #include tm-constrs.h. (mmix_intval): Delete declaration. Make non-static. (mmix_const_ok_for_letter_p, mmix_extra_constraint): Delete. (mmix_const_double_ok_for_letter_p): Delete. (mmix_legitimate_address_p): Use satisfies_constraint_I. (mmix_print_operand_address): Likewise. --- gcc/ChangeLog | 21 + gcc/config/mmix/constraints.md | 93 gcc/config/mmix/mmix-protos.h |4 +- gcc/config/mmix/mmix.c | 93 ++- gcc/config/mmix/mmix.h | 15 -- gcc/config/mmix/mmix.md|3 +- gcc/config/mmix/predicates.md | 14 -- 7 files changed, 132 insertions(+), 111 deletions(-) create mode 100644 gcc/config/mmix/constraints.md diff --git a/gcc/config/mmix/constraints.md b/gcc/config/mmix/constraints.md new file mode 100644 index 000..b8cc690 --- /dev/null +++ b/gcc/config/mmix/constraints.md @@ -0,0 +1,93 @@ +;; MMIX constraints +;; Copyright (C) 2012 Free Software Foundation, Inc. +;; +;; This file is part of GCC. +;; +;; GCC is free software; you can redistribute it and/or modify it +;; under the terms of the GNU General Public License as published by +;; the Free Software Foundation; either version 3, or (at your option) +;; any later version. +;; +;; GCC is distributed in the hope that it will be useful, but WITHOUT +;; ANY WARRANTY; without even the implied warranty of MERCHANTABILITY +;; or FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public +;; License for more details. +;; +;; You should have received a copy of the GNU General Public License +;; along with GCC; see the file COPYING3. If not see +;; http://www.gnu.org/licenses/. */ + +(define_register_constraint x SYSTEM_REGS + @internal) + +(define_register_constraint y REMAINDER_REG + @internal) + +(define_register_constraint z HIMULT_REG + @internal) + +(define_constraint I + A 8-bit unsigned integer + (and (match_code const_int) + (match_test IN_RANGE (ival, 0, 255 + +(define_constraint J + A 16-bit unsigned integer. + (and (match_code const_int) + (match_test IN_RANGE (ival, 0, 65535 + +(define_constraint K + An integer between -255 and 0. + (and (match_code const_int) + (match_test IN_RANGE (ival, -255, 0 + +(define_constraint L + @internal + (and (match_code const_int) + (match_test mmix_shiftable_wyde_value (ival + +(define_constraint M + The value 0. + (and (match_code const_int) + (match_test ival == 0))) + +(define_constraint N + @internal + (and (match_code const_int) + (match_test mmix_shiftable_wyde_value (~ival + +(define_constraint O + The value 3, 5, 9, or 17. + (and (match_code const_int) + (ior (match_test ival == 3) + (match_test ival == 5) + (match_test ival == 9) + (match_test ival == 17 + +(define_constraint G + Floating-point zero. + (and (match_code const_double) + (match_test op == CONST0_RTX (mode + +(define_constraint R + @internal + (and (not (match_code const_int,const_double)) + (match_test mmix_constant_address_p (op)) + (ior (match_test !TARGET_BASE_ADDRESSES) + (match_code LABEL_REF) + (and (match_code SYMBOL_REF) +(match_test SYMBOL_REF_FLAG (op)) + +(define_constraint S + @internal + (and (match_code const_int,const_double) + (match_test mmix_shiftable_wyde_value (mmix_intval (op) + +(define_constraint T + @internal + (and (match_code const_int,const_double) + (match_test mmix_shiftable_wyde_value (~mmix_intval (op) + +(define_constraint U + @internal + (match_operand 0 mmix_address_operand)) diff --git a/gcc/config/mmix/mmix-protos.h b/gcc/config/mmix/mmix-protos.h index 4e8c338..62cdbae 100644 --- a/gcc/config/mmix/mmix-protos.h +++ b/gcc/config/mmix/mmix-protos.h @@ -40,6 +40,7 @@
