Re: web ICEs on subreg
On May 16, 2013, at 5:26 PM, David Edelsohn dje@gmail.com wrote:
This patch is creating new segfaults for 32 bit POWER AIX.
Thanks for the heads up. Fixed in r199030.
2013-05-17 Mike Stump mikest...@comcast.net
PR rtl-optimization/57304
* web.c (union_match_dups): Ensure that DF_REF_LOC exists before
accessing DF_REF_REAL_LOC.
Index: web.c
===
--- web.c (revision 199016)
+++ web.c (working copy)
@@ -133,9 +133,10 @@ union_match_dups (rtx insn, struct web_e
entry = type == OP_IN ? use_entry : def_entry;
for (; *ref; ref++)
{
- if (DF_REF_LOC (*ref) == recog_data.operand_loc[op])
+ rtx *l = DF_REF_LOC (*ref);
+ if (l == recog_data.operand_loc[op])
break;
- if (DF_REF_REAL_LOC (*ref) == recog_data.operand_loc[op])
+ if (l DF_REF_REAL_LOC (*ref) == recog_data.operand_loc[op])
break;
}
@@ -143,9 +144,10 @@ union_match_dups (rtx insn, struct web_e
{
for (ref = use_link, entry = use_entry; *ref; ref++)
{
- if (DF_REF_LOC (*ref) == recog_data.operand_loc[op])
+ rtx *l = DF_REF_LOC (*ref);
+ if (l == recog_data.operand_loc[op])
break;
- if (DF_REF_REAL_LOC (*ref) == recog_data.operand_loc[op])
+ if (l DF_REF_REAL_LOC (*ref) == recog_data.operand_loc[op])
break;
}
}
--
Re: web ICEs on subreg
Mike, This patch is creating new segfaults for 32 bit POWER AIX. Was this patch tested with PowerPC? Program received signal SIGSEGV, Segmentation fault. 0x10a1db88 in _ZL8web_mainv () at /nasfarm/dje/src/src/gcc/web.c:138 138 if (DF_REF_REAL_LOC (*ref) == recog_data.operand_loc[op]) This fails for libgomp vla1.f90, vla2.f90, vla4.f90, vla5.f90, vla6.f90, vla8.f90 -O3 -funroll-all-loops -fopenmp and gcc.dg/torture/stackalign/setjmp-1.c and gcc.c-torture/execute/built-in-setjmp.c -O3 -funroll-all-loops Thanks, David
Re: web ICEs on subreg
On May 16, 2013, at 5:26 PM, David Edelsohn dje@gmail.com wrote: This patch is creating new segfaults for 32 bit POWER AIX. Was this patch tested with PowerPC? No, x86_64. I've added a patch to http://gcc.gnu.org/bugzilla/show_bug.cgi?id=57304 If you could let us know if that fixes the problem you've seen, that would be a great help, thanks.
Re: web ICEs on subreg
On May 10, 2013, at 5:27 PM, Kenneth Zadeck zad...@naturalbridge.com wrote:
Assuming the patch has been tested on a public port, it is ok for commit.
Thanks. It turns out that my patch is necessary, but not sufficient, the code
that exists must be left in place, as there are pre-existing test cases in the
test suite that do depend upon the existing code. In the below code, I leave
the existing code alone, and merely add the additional code. I reviewed the
way the code gets here, and reasonably it can get here in either of the two
ways, so having two checks is required.
Here is the version I checked in.
2013-05-14 Mike Stump mikest...@comcast.net
* web.c (union_match_dups): Also check DF_REF_REAL_LOC.
Index: web.c
===
--- web.c (revision 198796)
+++ web.c (working copy)
@@ -132,14 +132,22 @@ union_match_dups (rtx insn, struct web_e
ref = type == OP_IN ? use_link : def_link;
entry = type == OP_IN ? use_entry : def_entry;
for (; *ref; ref++)
- if (DF_REF_LOC (*ref) == recog_data.operand_loc[op])
- break;
+ {
+ if (DF_REF_LOC (*ref) == recog_data.operand_loc[op])
+ break;
+ if (DF_REF_REAL_LOC (*ref) == recog_data.operand_loc[op])
+ break;
+ }
if (!*ref type == OP_INOUT)
{
for (ref = use_link, entry = use_entry; *ref; ref++)
- if (DF_REF_LOC (*ref) == recog_data.operand_loc[op])
- break;
+ {
+ if (DF_REF_LOC (*ref) == recog_data.operand_loc[op])
+ break;
+ if (DF_REF_REAL_LOC (*ref) == recog_data.operand_loc[op])
+ break;
+ }
}
gcc_assert (*ref);
--
Re: web ICEs on subreg
On Fri, May 10, 2013 at 11:51 PM, Mike Stump wrote:
- if (DF_REF_LOC (*ref) == recog_data.operand_loc[op])
- break;
+ {
+ if (DF_REF_LOC (*ref) == recog_data.operand_loc[op])
+ break;
+ /* DF_REF_LOC can be (subreg:DI (reg:TI 5) 8) and
+recog_data.operand_loc[op] can be (reg:TI 5), and the above
+won't find it. */
+ if (GET_CODE (*DF_REF_LOC (*ref)) == SUBREG
+ SUBREG_REG (*DF_REF_LOC (*ref)) == recog_data.operand_loc[op])
Congrats, you've re-invented DF_REF_REAL_LOC.
Ciao!
Steven
Re: web ICEs on subreg
On Fri, May 10, 2013 at 11:51 PM, Mike Stump wrote: I have a instruction pattern for my port that reads in part: [(parallel [(set (subreg:DI (match_operand:TI 0 register_operand =r) 0) (mem:DI (plus:DI (match_operand:DI 1 register_operand r) (match_operand:DI 2 const_int_operand (set (subreg:DI (match_dup 0) 8) (mem:DI (plus:DI (match_dup 1) (match_operand:DI 3 const_int_operand])] The subreg seems to give web.c fits. web is looking for the match_dup and failing to find it, due to the fact that DF_REF_LOC is of the form (subreg:DI (reg:TI 5) 8), not (reg:TI 5). If I add a case for SUBREG like in the below, we can then find and process the subreg. So, the question is, is the port completely wrong for trying to use subreg in this fashion? If not, is processing subregs in this way the right way? If yes, Ok? I don't think this port is completely wrong. I've looked at some other ports and there are comparable patterns, e.g. in m68k: ;; We need a separate DEFINE_EXPAND for u?mulsidi3 to be able to use the ;; proper matching constraint. This is because the matching is between ;; the high-numbered word of the DImode operand[0] and operand[1]. (define_expand umulsidi3 [(parallel [(set (subreg:SI (match_operand:DI 0 register_operand ) 4) (mult:SI (match_operand:SI 1 register_operand ) (match_operand:SI 2 register_operand ))) (set (subreg:SI (match_dup 0) 0) (truncate:SI (lshiftrt:DI (mult:DI (zero_extend:DI (match_dup 1)) (zero_extend:DI (match_dup 2))) (const_int 32])] TARGET_68020 !TUNE_68060 !TARGET_COLDFIRE ) Your web.c patch looks correct to me, but I can't approve it. Ciao! Steven
Re: web ICEs on subreg
On May 10, 2013, at 3:29 PM, Steven Bosscher stevenb@gmail.com wrote:
Your web.c patch looks correct to me, but I can't approve it.
Thanks. Now that you point out the DF accessor, it all makes perfect sense.
:-) I've fixed one other instance that has to be as wrong for all the same
reasons. Though my port doesn't trip the other case from the test suite, if my
port used an IN/OUT, I bet I could make it trip it.
Ok?
2013-05-10 Mike Stump mikest...@comcast.net
* web.c (union_match_dups): Use DF_REF_REAL_LOC instead.
Doing diffs in web.c.~1~:
--- web.c.~1~ 2013-01-14 10:39:36.0 -0800
+++ web.c 2013-05-10 15:45:02.0 -0700
@@ -132,13 +132,13 @@ union_match_dups (rtx insn, struct web_e
ref = type == OP_IN ? use_link : def_link;
entry = type == OP_IN ? use_entry : def_entry;
for (; *ref; ref++)
- if (DF_REF_LOC (*ref) == recog_data.operand_loc[op])
+ if (DF_REF_REAL_LOC (*ref) == recog_data.operand_loc[op])
break;
if (!*ref type == OP_INOUT)
{
for (ref = use_link, entry = use_entry; *ref; ref++)
- if (DF_REF_LOC (*ref) == recog_data.operand_loc[op])
+ if (DF_REF_REAL_LOC (*ref) == recog_data.operand_loc[op])
break;
}
--
Re: web ICEs on subreg
Assuming the patch has been tested on a public port, it is ok for commit. kenny On 05/10/2013 06:52 PM, Mike Stump wrote: On May 10, 2013, at 3:29 PM, Steven Bosscher stevenb@gmail.com wrote: Your web.c patch looks correct to me, but I can't approve it. Thanks. Now that you point out the DF accessor, it all makes perfect sense. :-) I've fixed one other instance that has to be as wrong for all the same reasons. Though my port doesn't trip the other case from the test suite, if my port used an IN/OUT, I bet I could make it trip it. Ok?
