[PATCH] -fpic problems on PPC, a different approach

Franz Sirl Franz.Sirl-kernel@lauterbach.com
Wed Jun 9 10:25:00 GMT 1999


Am Wed, 09 Jun 1999 schrieb Richard Henderson:
>On Wed, Jun 09, 1999 at 02:23:17AM +0200, Franz Sirl wrote:
>> It turned out that simply checking regs_ever_live[PIC_OFFSET_TABLE_REGNUM]
>> is not enough, as this information may no longer be correct after reload,
>> if reload was the first instance to produce pic_offset_table_rtx references.
>
>It should have been valid again at flow2.  Oh, wait, horrible
>post-reload lifeness hacks for saved registers.  Ho hum.  Ok.

Ok, so nothing to worry about for me. Should I add a comment to
rs6000_got_register, explaining that setting regs_ever_live here can go away
when flow2 is fixed lifetimewise?

>> Is it possible to use something like "frame_pointer_needed ? 30 : 31" for
>> PIC_OFFSET_TABLE_REGNUM? This would make the unnecessary saving of r31 go
>> away.
>
>Maybe.  There are several register choice optimizations that
>are possible, including not using a call-saved register at all
>under certain circumstances.  Don't fool with this right now.

Ok, I'll leave it for now.

>> @@ -2540,7 +2427,7 @@ rs6000_reorg (insn)
>>  {
>>    if (flag_pic && (DEFAULT_ABI == ABI_V4 || DEFAULT_ABI == ABI_SOLARIS))
>>      {
>> -      rtx got_reg = gen_rtx_REG (Pmode, GOT_TOC_REGNUM);
>> +      rtx got_reg = gen_rtx_REG (Pmode, 2);
>>        for ( ; insn != NULL_RTX; insn = NEXT_INSN (insn))
>>  	if (GET_RTX_CLASS (GET_CODE (insn)) == 'i'
>>  	    && reg_mentioned_p (got_reg, PATTERN (insn)))
>
>This function should go away too.  It's sole purpose was for making
>sure we'd done the right thing with converting pic_offset_table_rtx
>on svr4.
>
>> -      fputs (TARGET_MINIMAL_TOC ? reg_names[30] : reg_names[2], file);
>> +      fputs (TARGET_MINIMAL_TOC ? reg_names[30] : reg_names[2 \
>> /* PIC_OFFSET_TABLE_REGNUM? */ ], file);
>
>This is AIX TOC related.  It should not change.
>
>> -	fprintf (file, "(%s)", reg_names[ TARGET_MINIMAL_TOC ? 30 : 2 ]);
>> +	fprintf (file, "(%s)", reg_names[ TARGET_MINIMAL_TOC ? 30 : 2 \
>> /* PIC_OFFSET_TABLE_REGNUM? */ ]);
>
>Same here.

Uhm, I'm not so sure about this. The TOC approach is used on svr4/ppc for
-fPIC. And there's a switch -mminimal-toc that sets TARGET_MINIMAL_TOC and
it's valid for -fpic. But I haven't been able to get an overview on all the ABI
interactions so far, so I left the reorg in and marked (at least in this
first version of the patch) the questionable places in the code.

>> +      /* we save all the condition registers as if they are a single 
>> +         register. The are physically, so this should work fine. */
>> +      if (dwarf2out_do_frame ())
>> +        {
>> +          SET_DWARF_LABEL (dw2_label);
>> +          dwarf2out_reg_save (dw2_label, 70, info->cr_save_offset);
>> +	}
>
>A stray change?

Oops, sorry. I thought I deleted all of the DWARF EH/PPC stuff from the patch.

>> +  /* If we need PIC_OFFSET_TABLE_REGNUM, initialize it now */
>> +  if (TARGET_ELF && flag_pic == 1 && current_function_uses_pic_offset_table)
>
>The `== 1' is wrong -- `-fPIC' is flag_pic == 2.
>
>> +  if (TARGET_ELF && flag_pic == 1)					\
>> +    fixed_regs[PIC_OFFSET_TABLE_REGNUM]				\
>
>Same here.

Don't think so, -fpic looks fundamentally different from -fPIC on PPC/SYSV. I
don't know what the reason behind that is, maybe -fPIC can be merged into -fpic
now, but I think this would be a change more suitable for the mainline than the
branch.

>> -/* Register to use as a placeholder for the GOT/allocated TOC register. 
>> -   FINALIZE_PIC will change all uses of this register to a an appropriate 
>> -  pseudo register when it adds the code to setup the GOT.  We use r2
>> -  because it is a reserved register in all of the ABI's.  */ 
>> -#define GOT_TOC_REGNUM 2 >
>Due to AIX usage, you might just keep this definition as TOC_REGNUM,
>and adjust the comment appropriately.  Tho maybe not -- it's not used.

Yeah, I've noticed that there's a bad practice of intermixing real and
macrofied versions of registernumbers.

>> +/*
>>  #define PREFERRED_RELOAD_CLASS(X,CLASS)			\
>>    ((GET_CODE (X) == CONST_DOUBLE			\
>>      && GET_MODE_CLASS (GET_MODE (X)) == MODE_FLOAT)	\
>>     ? NO_REGS : (CLASS))
>> +*/
>> +#define PREFERRED_RELOAD_CLASS(X,CLASS)                \
>> +  (CONSTANT_P (X)                                      \
>> +   && ((CLASS) == FLOAT_REGS                           \
>> +       || (GET_MODE_CLASS (GET_MODE (X)) == MODE_FLOAT \
>> +          && (HOST_FLOAT_FORMAT != IEEE_FLOAT_FORMAT   \
>> +              || HOST_BITS_PER_INT != BITS_PER_WORD))) \
>> +   ? NO_REGS : (CLASS))
>
>Another stray change?

Hmm, I overlooked this one too, but maybe someone should comment on the
proposed change. It's a change from Geoff that is independent of his old reload
patch and he wrote back then that he adapted some code from sparc.h. But AFAIK
nobody ever commented on this.


>> -;; V.4 specific code to initialize the PIC register
>> -
>> -(define_insn "init_v4_pic"
>> -  [(set (match_operand:SI 0 "register_operand" "=l")
>> -	(unspec [(const_int 0)] 7))]
>> -  "DEFAULT_ABI == ABI_V4 || DEFAULT_ABI == ABI_SOLARIS"
>> -  "bl _GLOBAL_OFFSET_TABLE_@local-4"
>> -  [(set_attr "type" "branch")
>> -   (set_attr "length" "4")])
>
>Keep this -- at some point in the (hopefuly near) future, Meissner
>will be able to merge prologue-epilogue-as-rtl changes.

OK, I will do so, adding a comment that it's temporarily unused.

Thanks for looking into the patch,
Franz.


More information about the Gcc-patches mailing list