On Wed, Jan 18, 2012 at 03:03:47PM +0000, Ramana Radhakrishnan wrote:
> +;; operand1 is the memory address to go into pic_load_addr_32bit.
> +;; operand2 is the PIC label to be emitted from pic_add_dot_plus_*.
> +;; We do this to allow hoisting of the entire c
> +(define_insn_and_split "pic_load_addr_unified"
> + [(set (match_operand:SI 0 "s_register_operand" "=r,r,l")
> + (unspec:SI [(match_operand:SI 1 "" "mX,mX,mX")
> + (match_operand:SI 2 "" "")]
> + UNSPEC_PIC_UNIFIED))]
> + "flag_pic"
> + "#"
> + "flag_pic"
What's the reason to duplicate the condition here? If flag_pic isn't
non-zero, then the insn wouldn't match, therefore wouldn't be split...
Also, while this fixes the testcase, at least if you split it before reload
e.g. first scheduling pass might move away the two insns far away from each
other, register allocation might decide to spill the pseudo set by the first
insn to the stack and the same problem could reappear.
That doesn't mean your patch isn't an incremental improvement.
> + [(set (match_dup 3) (unspec:SI [(match_dup 1)] UNSPEC_PIC_SYM))
> + (set (match_dup 0) (unspec:SI [(match_dup 3) (match_dup 4)
> + (match_dup 2)] UNSPEC_PIC_BASE))]
> + "operands[3] = can_create_pseudo_p () ? gen_reg_rtx (SImode) : operands[0];
> + operands[4] = TARGET_THUMB ? GEN_INT (4) : GEN_INT (8);"
> + [(set_attr "type" "load1,load1,load1")
> + (set_attr "pool_range" "4096,4096,1024")
> + (set_attr "neg_pool_range" "0,4084,0")
> + (set_attr "arch" "a,t2,t1")
> + (set_attr "length" "8,6,4")]
> +)
> +
Jakub
