[PATCH 3/7] use rtx_insn * more

Trevor Saunders tbsaunde@tbsaunde.org
Tue Oct 18 13:45:00 GMT 2016


On Tue, Oct 18, 2016 at 01:18:42PM +0200, Bernd Schmidt wrote:
> On 10/17/2016 09:46 PM, tbsaunde+gcc@tbsaunde.org wrote:
> >      {
> > -      rtx r0, r16, eqv, tga, tp, insn, dest, seq;
> > +      rtx r0, r16, eqv, tga, tp, dest, seq;
> > +      rtx_insn *insn;
> > 
> >        switch (tls_symbolic_operand_type (x))
> >  	{
> > @@ -1025,66 +1026,70 @@ alpha_legitimize_address_1 (rtx x, rtx scratch, machine_mode mode)
> >  	  break;
> > 
> >  	case TLS_MODEL_GLOBAL_DYNAMIC:
> > -	  start_sequence ();
> > +	  {
> > +	    start_sequence ();
> > 
> > -	  r0 = gen_rtx_REG (Pmode, 0);
> > -	  r16 = gen_rtx_REG (Pmode, 16);
> > -	  tga = get_tls_get_addr ();
> > -	  dest = gen_reg_rtx (Pmode);
> > -	  seq = GEN_INT (alpha_next_sequence_number++);
> > +	    r0 = gen_rtx_REG (Pmode, 0);
> > +	    r16 = gen_rtx_REG (Pmode, 16);
> > +	    tga = get_tls_get_addr ();
> > +	    dest = gen_reg_rtx (Pmode);
> > +	    seq = GEN_INT (alpha_next_sequence_number++);
> > 
> > -	  emit_insn (gen_movdi_er_tlsgd (r16, pic_offset_table_rtx, x, seq));
> > -	  insn = gen_call_value_osf_tlsgd (r0, tga, seq);
> > -	  insn = emit_call_insn (insn);
> > -	  RTL_CONST_CALL_P (insn) = 1;
> > -	  use_reg (&CALL_INSN_FUNCTION_USAGE (insn), r16);
> > +	    emit_insn (gen_movdi_er_tlsgd (r16, pic_offset_table_rtx, x, seq));
> > +	    rtx val = gen_call_value_osf_tlsgd (r0, tga, seq);
> 
> Since this doesn't consistently declare variables at the point of
> initialization, might as well put val into the list of variables at the top,
> and avoid reindentation that way. There are several such reindented blocks,
> and the patch would be a lot easier to review without this.

I do really prefer reading code where variables are declared at first
use, but I'll agree with the tools we are using this can be hard to
review, sorry about that.

> Alternatively, split it up a bit more into obvious/nonobvious parts.

yeah, I'll try to get that done soon.  fwiw a -b diff is below if you
find that better.

> > diff --git a/gcc/config/arc/arc.c b/gcc/config/arc/arc.c
> > index 21bba0c..8e8fff4 100644
> > --- a/gcc/config/arc/arc.c
> > +++ b/gcc/config/arc/arc.c
> > @@ -4829,7 +4829,6 @@ static rtx
> >  arc_emit_call_tls_get_addr (rtx sym, int reloc, rtx eqv)
> >  {
> >    rtx r0 = gen_rtx_REG (Pmode, R0_REG);
> > -  rtx insns;
> >    rtx call_fusage = NULL_RTX;
> > 
> >    start_sequence ();
> > @@ -4846,7 +4845,7 @@ arc_emit_call_tls_get_addr (rtx sym, int reloc, rtx eqv)
> >    RTL_PURE_CALL_P (call_insn) = 1;
> >    add_function_usage_to (call_insn, call_fusage);
> > 
> > -  insns = get_insns ();
> > +  rtx_insn *insns = get_insns ();
> >    end_sequence ();
> 
> For example, stuff like this looks obvious enough that it can go in.

yeah, I think Jeff preapproved stuff like that a while back, but I just
lumped it in though really that wasn't very nice to review, and there
isn't really a reason for a human to review what a compiler can check
for us.

Thanks!

Trev

> 
> 
> Bernd



More information about the Gcc-patches mailing list