[PING v2][PATCH] Make function clone name numbering independent.

Michael Ploujnikov michael.ploujnikov@oracle.com
Fri Aug 31 18:49:00 GMT 2018


On 2018-08-13 07:58 PM, Michael Ploujnikov wrote:
> Ping and I've updated the patch since last time as follows:
> 
>   - unittest scans assembly rather than the constprop dump because its
>     forward changed
>   - unittests should handle different hosts where any of
>     NO_DOT_IN_LABEL, NO_DOLLAR_IN_LABEL or __USER_LABEL_PREFIX__ may
>     be defined
>   - not 100% it's safe to change DECL_NAME to DECL_ASSEMBLER_NAME in
>     cgraph_node::create_virtual_clone, but I've attempted to reduce
>     some code duplication
>   - lto-partition.c: privatize_symbol_name_1 *does* need numbered
>     names
>   - but cold sections don't
>   - Expecting an IDENTIFIER_NODE in clone_function_name_1 avoids
>     unreliable string pointer use as pointed out in the first review
>   - renamed clone_function_name_1 and clone_function_name to
>     numbered_clone_function_name_1 and numbered_clone_function_name to
>     clarify purpose and discourage future unintended uses
> 
> Also bootstrapped and regtested.

Ping.

I've done some more digging into the current uses of
numbered_clone_function_name and checked if any tests fail if I change
it to suffixed_function_name:

  - gcc/cgraphclones.c:  DECL_NAME (new_decl) = numbered_clone_function_name (thunk->decl, "artificial_thunk");
    - no new tests fail, inconclusive
    - and despite the comment on redirect_callee_duplicating_thunks
      about "one or more" duplicates it doesn't seem like
      duplicate_thunk_for_node would be called more than once for each
      node, assuming each node is named uniquely, but I'm far from an
      expert in this area
  - gcc/cgraphclones.c:  SET_DECL_ASSEMBLER_NAME (new_decl, numbered_clone_function_name (old_decl, suffix));
    - called by cgraph_node::create_virtual_clone, definitely needs it
    - this is where clones like .constprop come from
  - gcc/cgraphclones.c:  DECL_NAME (new_decl) = numbered_clone_function_name (old_decl, suffix);
    - more tests fail
    - this is where clones like .simdclone come from
    - called by cgraph_node::create_version_clone_with_body, most likely needs it
  - gcc/config/rs6000/rs6000.c:  tree decl_name = numbered_clone_function_name (default_decl, "resolver");
    - doesn't look like this needs the numbering as there should only
      be one resolver per multi-version function, but need someone
      with an rs/6000 to confirm
  - gcc/lto/lto-partition.c:                                      numbered_clone_function_name_1 (identifier,
    - definitely needed for disambiguation, shown by unit tests failing
  - gcc/multiple_target.c:                                numbered_clone_function_name (node->decl,
    - create_dispatcher_calls says it only creates the dispatcher once
    - no new tests fail, inconclusive
  - gcc/multiple_target.c:                                    numbered_clone_function_name (node->decl,
    - I have a feeling this isn't necessary
    - no new tests fail, inconclusive
  - gcc/omp-expand.c:  DECL_NAME (kern_fndecl) = numbered_clone_function_name (kern_fndecl, "kernel");
    - no new tests fail, inconclusive
    - I didn't see (and couldn't figure out a way to get) any of the
      existing omp/acc tests actually exercise this codeptah
  - gcc/omp-low.c:  return numbered_clone_function_name (current_function_decl,
    - definitely needed based on
      gcc/testsuite/c-c++-common/goacc/kernels-loop-2.c and others
  - gcc/omp-simd-clone.c:      DECL_NAME (new_decl) = numbered_clone_function_name (old_decl, "simdclone");
    - no tests fail, inconclusive
    - can definitely have more than one simdclone per function as above, but
      maybe not these external types
    - some tests, like declare-simd.c actually exercise this codepath,
      but I couldn't find the resulting .simdclone decl the the
      simdclone pass dump nor in any of the other dumps to verify
  - gcc/symtab.c:  DECL_NAME (new_decl) = numbered_clone_function_name (node->decl, "localalias");
    - no tests fail, inconclusive
    - my understanding of function_and_variable_visibility says that
      there can only be one per function so maybe this isn't; hubicka?

I'll add patches to switch these once someone with expertise in these
areas can confirm that the numbering isn't needed in the respective
decl names.


- Michael

-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Make-function-assembly-more-independent.patch
Type: text/x-patch
Size: 5898 bytes
Desc: not available
URL: <http://gcc.gnu.org/pipermail/gcc-patches/attachments/20180831/5f4135ac/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-Rename-clone_function_name_1-and-clone_function_name.patch
Type: text/x-patch
Size: 8681 bytes
Desc: not available
URL: <http://gcc.gnu.org/pipermail/gcc-patches/attachments/20180831/5f4135ac/attachment-0001.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0003-Cold-sections-don-t-need-to-be-numbered.patch
Type: text/x-patch
Size: 5654 bytes
Desc: not available
URL: <http://gcc.gnu.org/pipermail/gcc-patches/attachments/20180831/5f4135ac/attachment-0002.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0004-Use-suffixed_function_name-in-cgraph_node-create_vir.patch
Type: text/x-patch
Size: 1675 bytes
Desc: not available
URL: <http://gcc.gnu.org/pipermail/gcc-patches/attachments/20180831/5f4135ac/attachment-0003.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 473 bytes
Desc: OpenPGP digital signature
URL: <http://gcc.gnu.org/pipermail/gcc-patches/attachments/20180831/5f4135ac/attachment.sig>


More information about the Gcc-patches mailing list