Re: [PATCH 01/11] Initial TI PRU GCC port

[Jeff Law <law at redhat dot com>]

On 06/23/2018 09:42 AM, Dimitar Dimitrov wrote:

>>
>> I've already asked about your copyright assignment status.  So you know,
>> we can't go forward with any nontrivial contributions until the
>> assignment is in place.
> Yes, FSF has my assignment since November 2016.
Great.  Thanks.




>> So it looks like you're supporting libgloss/newlib.  Does it work with
>> the current libgloss/newlib trunk?  I've had troubles over the last few
>> months with 16 bit targets.
> I have not detected issues with top-of-tree. But keep in mind that PRU is 8-
> bit only for the GCC internal representation. The port declares efficient 32-
> bit ops for SImode. From newlib's point of view, PRU is native 32-bit.
The issues are with targets that have 16 bit ints.  At least that's
that's my recollection.  They're not something I worry about too deeply
anymore, so I didn't dig into the details of what broke within newlib.

It sounds like you're lucky in that what you're exposing avoids these
recent breakages in newlib.

> 
> Some history here: http://gcc.gnu.org/ml/gcc/2017-01/msg00217.html
Thanks.  As Nathan mentioned, PRU is a little extreme in its register
handling.  There's always a tradeoff for what level of granularity to
expose on these oddball architectures.

Typically we have the register size match a natural word on the given
architecture and we don't try to allocate the sub-register objects
independently (ie, ah/al on x86 or something like the left/right
registers on the h8).

Given what little I know about the PRU, the actual operations on the
processor are 32 bits wide.  So I probably wouldn't have bothered
exposing byte level operations, except for byte loads/stores and
zero/sign extension from byte to 32bit words.  SImilarly for half-words.

Furthermore I would have restricted the byte and half-word operations to
the lowest covering sub-object.

But that'd just be my choice.  You're obviously allowed to do something
different.

>>
>>> diff --git a/gcc/common/config/pru/pru-common.c
>>> b/gcc/common/config/pru/pru-common.c new file mode 100644
>>> index 00000000000..e87d70ce9ca
>>> --- /dev/null
>>> +++ b/gcc/common/config/pru/pru-common.c
>>
>> [ ... ]
>>
>>> @@ -0,0 +1,36 @@
>>> +
>>> +#undef TARGET_EXCEPT_UNWIND_INFO
>>> +#define TARGET_EXCEPT_UNWIND_INFO sjlj_except_unwind_info
>>
>> SJLJ exceptions?  Is there some reason you're not using dwarf2 unwind
>> info?  It may mean adding some notes in the prologue generation code,
>> but otherwise I'd expect it to "just work".
> I would like to avoid increasing memory footprint. I saw that AVR folks use 
> it, too. My understanding is that eh_frame data will have to be included for 
> each function when using dwarf2 unwind.
> 
> For reference, typical data memory size for PRU is 4kb.
Understood.  Of course one could argue that trying to do C++ in a 4kb
footprint is on the absurd side. :-)


>>> +
>>> +/* ABI variant for code generation.  */
>>> +enum pru_abi {
>>> +    PRU_ABI_GNU,
>>> +    PRU_ABI_TI
>>
>> Is there really any value in having two ABIs?  If there's another
>> toolchain out there it'd be best if we were just compatible with that
>> rather than defining a GNU ABI.  I guess it might be helpful if the TI
>> ABI is crippled enough that it's hard to run the testsuite..
> 
> TI ABI defines data pointers as 32-bit and function pointers as 16-bit. I 
> could not implement this reliably for the PRU GCC port. And it doesn't seem to 
> be a good idea anyway:
>   http://gcc.gnu.org/ml/gcc/2012-04/msg00870.html
Ah.  Yea, we've never supported multiple pointer sizes like that.
While we do have special data segments on some architectures, that's
more about the ability to use short offsets from specific base pointers
rather than different sized pointers.

More recently we have added some limited address space support, but
again I don't think it's really a good match for this problem.


> 
> My GCC implementation for TI ABI does not support function pointers and large 
> return values. Hence I defined GNU ABI where those are supported, users can 
> enjoy full C language capabilities, and a decent testsuite coverage is 
> achieved.
ACK.  Thanks for explaining.

>>
>>> +
>>> +/* Say that the epilogue uses the return address register.  Note that
>>> +   in the case of sibcalls, the values "used by the epilogue" are
>>> +   considered live at the start of the called function.  */
>>> +#define EPILOGUE_USES(REGNO) (epilogue_completed &&	      \
>>> +			      (((REGNO) == RA_REGNO)	      \
>>> +			       || (REGNO) == (RA_REGNO + 1)))
>>
>> Is this really needed?  If you properly describe the dataflow for your
>> epilogues I don't think you need this hack.
> Yes it is needed. I saw that nios2 port uses it, and I had no idea it is a 
> hack.
It looks like most ports are using EPILOGUE_USES these days by way of
the df infrastructure which went in a few years ago.  So I probably
shouldn't call it a hack anymore.  Don't worry about trying to eliminate
it.  Sorry for steering you the wrong direction on this.

Jeff