Re: [PATCH SH]: Addition of push pop instructions for GBR register

[Kaz Kojima <kkojima at rr dot iij4u dot or dot jp>]

Kaushik Phatak <Kaushik.Phatak@kpitcummins.com> wrote:
> The following patch allows the compiler to save and restore the GBR
> register on the SH target. These instructions can be executed in user
> mode. They have been added to fix the internal compiler error caused
> when the '-fcall-saved-gbr' option is passed, where a function is 
> modifying the GBR register. Regression done for sh-elf-* and no new 
> regressions found.

The description of the problem would be too vague.  One can get
an ICE for

__thread int a; int foo() { return a; }

with -fcall-saved-gbr, but SH's TLS ABI uses gbr as the thread
pointer and -fcall-saved-gbr is just pointless for such cases
in the first place.  Adding push/pop insns for gbr will silence
that ICE and hides the real issue. The right thing here would
be to warn the use of -fcall-saved- for thread pointer register.
Perhaps your problem may be different at all.  Anyway it's better
to file your problem into gcc bugzilla with a test case.

BTW if you want to add new push/pop insns, they should be added
just after push_fpul insn with an appropriate comment.

> +(define_insn "push_gbr"
> +  [(set (mem:SI (pre_dec:SI (reg:SI SP_REG)))
> +	(reg:SI GBR_REG))]
> +  "TARGET_SH1 && ! TARGET_SH5"
> +  "stc.l	gbr,@-r15"
> +  [(set_attr "type" "tls_load")])

Why do you use "tls_load"?  It should be "store", shouldn't it?

Regards,
	kaz