| Message ID | 4D8CCE3D.7010100@redhat.com |
|---|---|
| State | New |
| Headers | show |
Richard Henderson <rth@redhat.com> writes: > This is due to a miscommunication between the middle-end and the backend > about how many arguments the setmemhi pattern takes. > > (define_expand "setmemhi" > [(parallel [(set (match_operand:BLK 0 "memory_operand" "") > (match_operand 2 "const_int_operand" "")) > (use (match_operand:HI 1 "const_int_operand" "")) > (use (match_operand:HI 3 "const_int_operand" "n")) > (clobber (match_scratch:HI 4 "")) > (clobber (match_dup 5))])] > > The match_scratch is counted in .n_operands, which makes the count of > operands not equal 4, so we assume 6 operands are necessary. We can > fix this for the special case of avr by only assuming 6 operands when > there are in fact 6 operands, but of course this could fail just as > easily if there were two scratches. > > All of which suggests that optional arguments to a named optab is a > mistake that ought to be rectified. > > I plan to commit the following after bootstrap and check. Thanks. I think it needs to be s/!= 4/>= 6/ though, so that match_scratches still work when 6 operands really are passed in. Fully agreed on the optional args thing. Or maybe insn_data should have a separate "num_args" field. Richard
Richard Sandiford <richard.sandiford@linaro.org> writes: > Richard Henderson <rth@redhat.com> writes: >> This is due to a miscommunication between the middle-end and the backend >> about how many arguments the setmemhi pattern takes. >> >> (define_expand "setmemhi" >> [(parallel [(set (match_operand:BLK 0 "memory_operand" "") >> (match_operand 2 "const_int_operand" "")) >> (use (match_operand:HI 1 "const_int_operand" "")) >> (use (match_operand:HI 3 "const_int_operand" "n")) >> (clobber (match_scratch:HI 4 "")) >> (clobber (match_dup 5))])] >> >> The match_scratch is counted in .n_operands, which makes the count of >> operands not equal 4, so we assume 6 operands are necessary. We can >> fix this for the special case of avr by only assuming 6 operands when >> there are in fact 6 operands, but of course this could fail just as >> easily if there were two scratches. >> >> All of which suggests that optional arguments to a named optab is a >> mistake that ought to be rectified. >> >> I plan to commit the following after bootstrap and check. > > Thanks. I think it needs to be s/!= 4/>= 6/ though, so that > match_scratches still work when 6 operands really are passed in. Er, >= 4 even... Richard
Richard Sandiford <richard.sandiford@linaro.org> writes:
> Er, >= 4 even...
Or not, *sigh*. Time I went home...
Richard
On 03/25/2011 10:51 AM, Richard Sandiford wrote: > Thanks. I think it needs to be s/!= 4/>= 6/ though, so that > match_scratches still work when 6 operands really are passed in. For the record, I audited all setmem and movmem patterns. There are is only one that uses 6 operands: i386. There are two that use 4 operands, but have 1 scratch: avr, pdp11. All the rest have exactly 4 operands. I'll leave the test as == 6 for now. > Fully agreed on the optional args thing. Or maybe insn_data should > have a separate "num_args" field. This is sounds like a good thing. It's probably worth doing some checking in some genfoo (opinit?) that the named patterns have exactly the number of operands expected. r~
Richard Henderson schrieb: > On 03/25/2011 05:41 AM, Georg-Johann Lay wrote: >>> On 03/22/2011 06:48 PM, Richard Henderson wrote: >>> >>>> Ok. Watch out for other target problems this week. >> libgcc fails to build for avr (SVN 171446) >> >> ../../../../../gcc.gnu.org/trunk/libgcc/../gcc/libgcc2.c: In function >> '__negdi2': >> ../../../../../gcc.gnu.org/trunk/libgcc/../gcc/libgcc2.c:68:17: >> internal compiler error: in maybe_gen_insn, at optabs.c:7123 > > This is due to a miscommunication between the middle-end and the backend > about how many arguments the setmemhi pattern takes. > > (define_expand "setmemhi" > [(parallel [(set (match_operand:BLK 0 "memory_operand" "") > (match_operand 2 "const_int_operand" "")) > (use (match_operand:HI 1 "const_int_operand" "")) > (use (match_operand:HI 3 "const_int_operand" "n")) > (clobber (match_scratch:HI 4 "")) > (clobber (match_dup 5))])] > > The match_scratch is counted in .n_operands, which makes the count of > operands not equal 4, so we assume 6 operands are necessary. We can > fix this for the special case of avr by only assuming 6 operands when > there are in fact 6 operands, but of course this could fail just as > easily if there were two scratches. > > All of which suggests that optional arguments to a named optab is a > mistake that ought to be rectified. > > I plan to commit the following after bootstrap and check. > > Hi, there is still trouble with "setmemhi" on avr, again for the negdi2 from libgcc: #1 0x0839ccd7 in maybe_legitimize_operand (icode=CODE_FOR_setmemhi, opno=4, op=0xbfffd22c) at ../../../gcc.gnu.org/trunk/gcc/optabs.c:7024 (gdb) p *op $11 = {type = EXPAND_OUTPUT, unsigned_p = 0, unused = 0, mode = VOIDmode, value = 0xb7d63360} (gdb) p op->value $12 = (rtx) 0xb7d63360 (gdb) pr (use:CC (nil)) i.e. there is garbage in op->value (gdb) (gdb) frame 0 #0 fancy_abort (file=0x88099f0 "../../../gcc.gnu.org/trunk/gcc/optabs.c", line=7024, function=0x880a280 "maybe_legitimize_operand") at ../../../gcc.gnu.org/trunk/gcc/diagnostic.c:893 (gdb) Sources are as of svn trunk 4.7 (SVN 171773) Reading specs from /mnt/nfs/home/georg/gnu/build/gcc-4.6-avr/gcc/specs COLLECT_GCC=/mnt/nfs/home/georg/gnu/build/gcc-4.6-avr/gcc/xgcc COLLECT_LTO_WRAPPER=/mnt/nfs/home/georg/gnu/build/gcc-4.6-avr/gcc/lto-wrapper Target: avr Configured with: ../../gcc.gnu.org/trunk/configure --target=avr --prefix=/local/gnu/install/gcc-4.6 --enable-languages=c,c++ --disable-libssp --disable-libada --disable-nls --disable-shared Johann
Georg-Johann Lay <avr@gjlay.de> writes: > Richard Henderson schrieb: >> On 03/25/2011 05:41 AM, Georg-Johann Lay wrote: >>>> On 03/22/2011 06:48 PM, Richard Henderson wrote: >>>> >>>>> Ok. Watch out for other target problems this week. >>> libgcc fails to build for avr (SVN 171446) >>> >>> ../../../../../gcc.gnu.org/trunk/libgcc/../gcc/libgcc2.c: In function >>> '__negdi2': >>> ../../../../../gcc.gnu.org/trunk/libgcc/../gcc/libgcc2.c:68:17: >>> internal compiler error: in maybe_gen_insn, at optabs.c:7123 >> >> This is due to a miscommunication between the middle-end and the backend >> about how many arguments the setmemhi pattern takes. >> >> (define_expand "setmemhi" >> [(parallel [(set (match_operand:BLK 0 "memory_operand" "") >> (match_operand 2 "const_int_operand" "")) >> (use (match_operand:HI 1 "const_int_operand" "")) >> (use (match_operand:HI 3 "const_int_operand" "n")) >> (clobber (match_scratch:HI 4 "")) >> (clobber (match_dup 5))])] >> >> The match_scratch is counted in .n_operands, which makes the count of >> operands not equal 4, so we assume 6 operands are necessary. We can >> fix this for the special case of avr by only assuming 6 operands when >> there are in fact 6 operands, but of course this could fail just as >> easily if there were two scratches. >> >> All of which suggests that optional arguments to a named optab is a >> mistake that ought to be rectified. >> >> I plan to commit the following after bootstrap and check. >> >> > > Hi, there is still trouble with "setmemhi" on avr, again for the > negdi2 from libgcc: > > #1 0x0839ccd7 in maybe_legitimize_operand (icode=CODE_FOR_setmemhi, > opno=4, op=0xbfffd22c) at ../../../gcc.gnu.org/trunk/gcc/optabs.c:7024 > (gdb) p *op > $11 = {type = EXPAND_OUTPUT, unsigned_p = 0, unused = 0, mode = > VOIDmode, value = 0xb7d63360} > (gdb) p op->value > $12 = (rtx) 0xb7d63360 > (gdb) pr > (use:CC (nil)) > > > i.e. there is garbage in op->value > > (gdb) > (gdb) frame 0 > #0 fancy_abort (file=0x88099f0 > "../../../gcc.gnu.org/trunk/gcc/optabs.c", line=7024, > function=0x880a280 "maybe_legitimize_operand") at > ../../../gcc.gnu.org/trunk/gcc/diagnostic.c:893 > (gdb) Yeah, as things stand, we need to set nops to 4 if was originally 5. I'm testing a series of patches to make the number of generator arguments available in insn_data. I'll post them once they pass (hopefully later today). Richard
Richard Sandiford schrieb: > Georg-Johann Lay <avr@gjlay.de> writes: >> Richard Henderson schrieb: >>> On 03/25/2011 05:41 AM, Georg-Johann Lay wrote: >>>>> On 03/22/2011 06:48 PM, Richard Henderson wrote: >>>>> >>>>>> Ok. Watch out for other target problems this week. >>>> libgcc fails to build for avr (SVN 171446) >>>> >>>> ../../../../../gcc.gnu.org/trunk/libgcc/../gcc/libgcc2.c: In function >>>> '__negdi2': >>>> ../../../../../gcc.gnu.org/trunk/libgcc/../gcc/libgcc2.c:68:17: >>>> internal compiler error: in maybe_gen_insn, at optabs.c:7123 >>> This is due to a miscommunication between the middle-end and the backend >>> about how many arguments the setmemhi pattern takes. >>> >>> (define_expand "setmemhi" >>> [(parallel [(set (match_operand:BLK 0 "memory_operand" "") >>> (match_operand 2 "const_int_operand" "")) >>> (use (match_operand:HI 1 "const_int_operand" "")) >>> (use (match_operand:HI 3 "const_int_operand" "n")) >>> (clobber (match_scratch:HI 4 "")) >>> (clobber (match_dup 5))])] >>> >>> The match_scratch is counted in .n_operands, which makes the count of >>> operands not equal 4, so we assume 6 operands are necessary. We can >>> fix this for the special case of avr by only assuming 6 operands when >>> there are in fact 6 operands, but of course this could fail just as >>> easily if there were two scratches. >>> >>> All of which suggests that optional arguments to a named optab is a >>> mistake that ought to be rectified. >>> >>> I plan to commit the following after bootstrap and check. >>> >>> >> Hi, there is still trouble with "setmemhi" on avr, again for the >> negdi2 from libgcc: >> >> #1 0x0839ccd7 in maybe_legitimize_operand (icode=CODE_FOR_setmemhi, >> opno=4, op=0xbfffd22c) at ../../../gcc.gnu.org/trunk/gcc/optabs.c:7024 >> (gdb) p *op >> $11 = {type = EXPAND_OUTPUT, unsigned_p = 0, unused = 0, mode = >> VOIDmode, value = 0xb7d63360} >> (gdb) p op->value >> $12 = (rtx) 0xb7d63360 >> (gdb) pr >> (use:CC (nil)) >> >> >> i.e. there is garbage in op->value >> >> (gdb) >> (gdb) frame 0 >> #0 fancy_abort (file=0x88099f0 >> "../../../gcc.gnu.org/trunk/gcc/optabs.c", line=7024, >> function=0x880a280 "maybe_legitimize_operand") at >> ../../../gcc.gnu.org/trunk/gcc/diagnostic.c:893 >> (gdb) > > Yeah, as things stand, we need to set nops to 4 if was originally 5. > > I'm testing a series of patches to make the number of generator > arguments available in insn_data. I'll post them once they pass > (hopefully later today). > > Richard Thanks, I can build avr-gcc again. Johann
diff --git a/gcc/expr.c b/gcc/expr.c index 4db1c77..0a95aa7 100644 --- a/gcc/expr.c +++ b/gcc/expr.c @@ -1299,11 +1299,10 @@ emit_block_move_via_movmem (rtx x, rtx y, rtx size, unsigned int align, /* The check above guarantees that this size conversion is valid. */ create_convert_operand_to (&ops[2], size, mode, true); create_integer_operand (&ops[3], align / BITS_PER_UNIT); - if (nops != 4) + if (nops == 6) { create_integer_operand (&ops[4], expected_align / BITS_PER_UNIT); create_integer_operand (&ops[5], expected_size); - nops = 6; } if (maybe_expand_insn (code, nops, ops)) { @@ -2721,11 +2720,10 @@ set_storage_via_setmem (rtx object, rtx size, rtx val, unsigned int align, create_convert_operand_to (&ops[1], size, mode, true); create_convert_operand_from (&ops[2], val, byte_mode, true); create_integer_operand (&ops[3], align / BITS_PER_UNIT); - if (nops != 4) + if (nops == 6) { create_integer_operand (&ops[4], expected_align / BITS_PER_UNIT); create_integer_operand (&ops[5], expected_size); - nops = 6; } if (maybe_expand_insn (code, nops, ops)) return true;