Re: Cleaning up expand optabs code

[Richard Henderson]

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.


r~

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;