less use of optabs/insn-codes, 2/n

[Zack Weinberg]

On Sat, Aug 18, 2001 at 05:21:22PM -0700, Richard Henderson wrote:
> > 	* stmt.c (check_for_full_enumeration_handling, emit_case_nodes):
> > 	Zap #if 0 blocks.
> > 	(is_call_to_builtin_classify_type): New function.
> > 	(expand_end_case): All low-level code to generate a casesi
> > 	or a tablejump split out to routines in expr.c.
> > 	* expr.c (do_tablejump): Make static and defined always.
> > 	(case_values_threshold, try_casesi, try_tablejump): New functions,
> > 	code formerly in stmt.c (expand_end_case).
> > 	* expr.h: Prototype try_casesi, try_tablejump, case_values_threshold;
> > 	not do_tablejump.
> 
> Ok if you build cross compilers to one target that doesn't have casesi
> (e.g. alpha) and one that doesn't have tablejump (e.g. s390 or vax).

alpha works with tweaks (see below), vax doesn't.  This test program:

$ cat test.c
int foo(int x)
{
        switch (x) {
        case 1: return 2;
        case 2: return 4;
        case 3: return 6;
        case 4: return 8;
        default: return 0;
        }
}
$ ./cc1 -O0 test.c
 foo
test.c: In function `foo':
test.c:10: Internal compiler error in instantiate_virtual_regs_1,
	   at function.c:4057
$ ./cc1 -O2 test.c
 foo
test.c: In function `foo':
test.c:10: verify_flow_info: Incorrect fallthru 0->1
test.c:10: Wrong insn in the fallthru edge
(jump_insn 49 48 61 (addr_diff_vec:HI (label_ref:SI 48)
        [ 
            (label_ref:SI 11)
            (label_ref:SI 18)
            (label_ref:SI 25)
            (label_ref:SI 32)
        ] 
        (const_int 0 [0x0])
        (const_int 0 [0x0])) -1 (nil)
    (nil))
test.c:10: confused by earlier errors, bailing out

[stupid ICE suppression...]

This is the problematic RTL:

(jump_insn 47 4 48 (set (pc)
        (if_then_else (leu (minus:SI (reg/v:SI 22)
                    (const_int 1 [0x1]))
                (const_int 3 [0x3]))
            (plus:SI (sign_extend:SI (mem:HI (plus:SI (pc)
                            (mult:SI (minus:SI (reg/v:SI 22)
                                    (const_int 1 [0x1]))
                                (const_int 2 [0x2]))) 0))
                (label_ref:SI 48))
            (pc))) -1 (nil)
    (nil))

(code_label 48 47 49 8 "" "" [0 uses])

(jump_insn 49 48 50 (addr_diff_vec:HI (label_ref:SI 48)
        [ 
            (label_ref:SI 11)
            (label_ref:SI 18)
            (label_ref:SI 25)
            (label_ref:SI 32)
        ] 
        (const_int 0 [0x0])
        (const_int 0 [0x0])) -1 (nil)
    (nil))

(jump_insn 50 49 51 (set (pc)
        (label_ref 39)) -1 (nil)
    (nil))

(barrier 51 50 11)

It's fairly clear what's going on at -O2.  The first jump_insn ends basic
block 0, but vax defines CASE_DROPS_THROUGH, so we have a fallthru edge
to basic block 1 which consists only of jump_insn 50.  verify_flow_info
wants to ensure that there's nothing on the insn chain between blocks
except code_labels, barriers, and notes.  The jump_insn containing the
addr_diff_vec does not qualify.

I tried having expand_end_case put the jump to the default label before the
addr_diff_vec in the insn chain.  That makes flow happy, but then the final
assembly output has a jbr betwen the casel and its jump table.  I don't
exactly speak VAX assembler, but I'm fairly sure that casel wants the
jump table to be the very next thing in the instruction stream.

I'm not sure what's going wrong at -O0.  We hit this abort:

4054                  emit_insns_before (seq, object);
4055                  if (! validate_change (object, loc, temp, 0)
4056                      && ! validate_replace_rtx (x, temp, object))
4057                    abort ();

(gdb) call debug_rtx (object)
(jump_insn 46 59 47 (set (pc)
        (if_then_else (leu (minus:SI (mem/f:SI (reg/f:SI 16 virtual-incoming-args) 0)
                    (const_int 1 [0x1]))
                (const_int 3 [0x3]))
            (plus:SI (sign_extend:SI (mem:HI (plus:SI (pc)
                            (mult:SI (minus:SI (mem/f:SI (reg/f:SI 16 virtual-incoming-args) 0)
                                    (const_int 1 [0x1]))
                                (const_int 2 [0x2]))) 0))
                (label_ref:SI 47))
            (pc))) -1 (nil)
    (nil))
(gdb) call debug_rtx (*loc)
(reg/f:SI 16 virtual-incoming-args)
(gdb) call debug_rtx (x)
(reg/f:SI 16 virtual-incoming-args)
(gdb) call debug_rtx (temp)
(reg:SI 27)
(gdb) call debug_rtx (seq)
(insn 59 3 46 (set (reg:SI 27)
        (plus:SI (reg/f:SI 12 ap)
            (const_int 4 [0x4]))) -1 (nil)
    (nil))

I do not see why this transformation has been rejected.

> > +static inline int
> > +is_call_to_builtin_classify_type (expr)
> > +     tree expr;
> 
> Ideally this would be handled in fold_builtin, so that this builtin
> wouldn't get this far.

I've got a revised version of the patch that does just that.  That bit
works great.

> > +#ifndef HAVE_casesi
> > +#define HAVE_casesi 0
> > +#endif
> 
> Don't you need a gen_casesi stub here?

yep, and a dummy CODE_FOR_casesi too.  Included in revisions.

If you could give me some advice on what to do about the vax, then I'll
roll that in and resubmit.

zw