Re: The nvptx port [10/11+] Target files

[Jeff Law <law@redhat.com>]

On 10/20/14 08:33, Bernd Schmidt wrote:
> These are the main target files for the ptx port. t-nvptx is empty for
> now but will grow some content with follow up patches.
>
>
> Bernd
>
>
> 010-target.diff
>
>
> 	* configure.ac: Allow configuring lto for nvptx.
> 	* configure: Regenerate.
>
> 	gcc/
> 	* config/nvptx/nvptx.c: New file.
> 	* config/nvptx/nvptx.h: New file.
> 	* config/nvptx/nvptx-protos.h: New file.
> 	* config/nvptx/nvptx.md: New file.
> 	* config/nvptx/t-nvptx: New file.
> 	* config/nvptx/nvptx.opt: New file.
> 	* common/config/nvptx/nvptx-common.c: New file.
> 	* config.gcc: Handle nvptx-*-*.
>
> 	libgcc/
> 	* config.host: Handle nvptx-*-*.
> 	* config/nvptx/t-nvptx: New file.
> 	* config/nvptx/crt0.s: New file.
Please make sure all the functions in nvptx.c have function comments. 
nvptx_split_reg_p, write_as_kernel, nvptx_write_function_decl, 
write_function_decl_only, nvptx_function_incoming_arg, 
nvptx_promote_function_mode, nvptx_maybe_convert_symbolic_operand, etc.

There are many others..  A scan over that entire file would be appreciated.


>
> ------------------------------------------------------------------------
> +
> +/* TARGET_FUNCTION_VALUE implementation.  Returns an RTX representing the place
> +   where function FUNC returns or receives a value of data type TYPE.  */
> +
> +static rtx
> +nvptx_function_value (const_tree type, const_tree func ATTRIBUTE_UNUSED,
> +		      bool outgoing)
> +{
> +  int unsignedp = TYPE_UNSIGNED (type);
> +  enum machine_mode orig_mode = TYPE_MODE (type);
> +  enum machine_mode mode = promote_function_mode (type, orig_mode,
> +						  &unsignedp, NULL_TREE, 1);
> +  if (outgoing)
> +    return gen_rtx_REG (mode, 4);
> +  if (cfun->machine->start_call == NULL_RTX)
> +    /* Pretend to return in a hard reg for early uses before pseudos can be
> +       generated.  */
> +    return gen_rtx_REG (mode, 4);
> +  return gen_reg_rtx (mode);
Rather than magic register numbers, can you use something symbolic?

> +}
> +
> +/* Implement TARGET_LIBCALL_VALUE.  */
> +
> +static rtx
> +nvptx_libcall_value (enum machine_mode mode, const_rtx)
> +{
> +  if (cfun->machine->start_call == NULL_RTX)
> +    /* Pretend to return in a hard reg for early uses before pseudos can be
> +       generated.  */
> +    return gen_rtx_REG (mode, 4);
> +  return gen_reg_rtx (mode);
> +}
Similarly.


> +
> +/* Implement TARGET_FUNCTION_VALUE_REGNO_P.  */
> +
> +static bool
> +nvptx_function_value_regno_p (const unsigned int regno)
> +{
> +  return regno == 4;
> +}
Here too.


> +
> +bool
> +nvptx_hard_regno_mode_ok (int regno, enum machine_mode mode)
> +{
> +  if (regno != 4 || cfun == NULL || cfun->machine->ret_reg_mode == VOIDmode)
> +    return true;
> +  return mode == cfun->machine->ret_reg_mode;
> +}
Function comment.  Magic register #.


> +
> +const char *
> +nvptx_output_call_insn (rtx insn, rtx result, rtx callee)
If possible, promote first argument to rtx_insn *.

> +
> +/* Clean up subreg operands.  */
Which means what?  A little more descriptive here would be helpful.  I 
have a guess what you need to do here, but more commentary would be 
helpful for someone that hasn't read through the virtual PTX ISA.

The machine description is about what I would expect, in fact, it shows 
how "nice" a virtual ISA can be.

Overall it seems pretty reasonable.  Most of the difficulty appears to 
be interfacing with the 3rd party tools, but that's largely expected.

I'm surprised there's not more hair around the address space issues.  I 
expected more problems there.

I'm going to trust that all the ABI related stuff is correct.  I'm not 
going to second guess any of that stuff.

I think we've got a couple things to iterate on from yesterday and 
you've got some minor stuff to address as noted above, but this looks 
pretty close to being ready.


jeff