On Fri, 2015-10-02 at 11:18 -0400, Rich Felker wrote:
> Thanks! This is very helpful. gcc style has changed a lot since the
> old patch was submitted so I think it makes sense to update it to
> match current practices rather than just making it work. I'll try to
> focus on any functional problems first though so as to keep a working
> patch against 5.2 as well and ease backporting to earlier versions (if
> anyone wants to do that on their own; certainly I don't expect it to
> happen in upstream gcc).
Let's see what the final patch will look like.
> There are a few call sites where the symbol returned is actually used.
> Would you want me to just do something like:
>
> struct function_symbol_result funcsym = function_symbol(...);
>
> then use funcsym.symbol and funcsym.label?
If you need both return values, then yes. But without the "struct". If
"function_symbol_result" is too long feel free to come up with a shorter
name.
>
> Would you object to shorter member names .sym and .lab?
No, that's OK, too.
> Uhg, not sure how I missed that. (Well, yes I am -- it's C++'s
> fault;-) I'll try to figure out what's going on.
I think the overloaded function from your patch is simply not invoked by
anything. You'd probably have to merge it into the already existing
one.
> > > +(define_insn "sibcalli_fdpic"
> > > + [(call (mem:SI (match_operand:SI 0 "register_operand" "k"))
> > > + (match_operand 1 "" ""))
> > > + (use (reg:SI FPSCR_MODES_REG))
> > > + (use (reg:SI PIC_REG))
> > > + (return)]
> > > + "TARGET_SH1 && TARGET_FDPIC"
> > > ^^^
> >
> > This is maybe slightly impossible, because of ..
>
> Because SH5 is deprecated?
No, because ...
> > > + if (TARGET_FDPIC
> > > + && (TARGET_SHMEDIA || TARGET_SHCOMPACT || !TARGET_SH2))
> > > + sorry ("non-SH2 FDPIC");
> > > +
... this refuses operation if FDPIC is used with anything "less than"
SH2, i.e. SH1. I think the condition above should be "TARGET_SH2 &&
TARGET_FDPIC".
> OK, but I'm not really familiar with this part of the code; I just
> adapted the patch by pattern. There are a lot of places with
> (match_operand N "" ""); should the empty strings be dropped for all
> of them?
Yes, there are several places with empty predicate/constraint strings.
They could be removed with a big patch, but for the moment, just don't
add new ones.
> > Maybe it could be useful to replace all "gen_rtx_REG (Pmode, PIC_REG)"
> > in the patch with something like 'get_t_reg_rtx'. Depends on how many
> > times this gen_rtx_REG is invoked.
>
> I'm fairly indifferent to this. Neither is significantly shorter or
> more readable.
It's about the amount of rtx objects generated. But that can be checked
out later.
> I'm a bit confused by the fact that basically ALL of the traffic on
> the linux-sh list is "shmedia" stuff. Is that unrelated to the actual
> SH5/SHMEDIA and just a brand name that got co-opted for an ARM-based
> SoC?
Yes, looks like.
> If so, is there anything that can be done to get it off the
> linux-sh list so that it doesn't bury mail about the actual SH
> ISA/platform?
Ask there. It doesn't show up here :)
Cheers,
Oleg
