[PATCH] D140722: [OpenMP] Prefix outlined and reduction func names with original func's name

[Itay Bookstein via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG782c59a4eef0: [OpenMP] Prefix outlined and reduction func 
names with original func's name (authored by nextsilicon-itay-bookstein).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D140722/new/

https://reviews.llvm.org/D140722

Files:
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/lib/CodeGen/CGOpenMPRuntime.h
  clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp
  clang/lib/CodeGen/CGOpenMPRuntimeGPU.h
  clang/lib/CodeGen/CGStmtOpenMP.cpp
  clang/test/CodeGen/PowerPC/ppc64le-varargs-f128.c
  clang/test/CoverageMapping/openmp.c
  clang/test/OpenMP/amdgpu_target_with_aligned_attribute.c
  clang/test/OpenMP/bug54082.c
  clang/test/OpenMP/bug60602.cpp
  clang/test/OpenMP/cancel_codegen.cpp
  clang/test/OpenMP/cancellation_point_codegen.cpp
  clang/test/OpenMP/debug-info-complex-byval.cpp
  clang/test/OpenMP/debug-info-openmp-array.cpp
  clang/test/OpenMP/debug_threadprivate_copyin.c
  clang/test/OpenMP/declare_target_codegen_globalization.cpp
  clang/test/OpenMP/distribute_codegen.cpp
  clang/test/OpenMP/distribute_firstprivate_codegen.cpp
  clang/test/OpenMP/distribute_lastprivate_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_firstprivate_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_if_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_lastprivate_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_num_threads_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_private_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_proc_bind_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_reduction_task_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_simd_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_simd_firstprivate_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_simd_if_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_simd_lastprivate_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_simd_num_threads_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_simd_private_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_simd_proc_bind_codegen.cpp
  clang/test/OpenMP/distribute_private_codegen.cpp
  clang/test/OpenMP/distribute_simd_codegen.cpp
  clang/test/OpenMP/distribute_simd_firstprivate_codegen.cpp
  clang/test/OpenMP/distribute_simd_lastprivate_codegen.cpp
  clang/test/OpenMP/distribute_simd_private_codegen.cpp
  clang/test/OpenMP/distribute_simd_reduction_codegen.cpp
  clang/test/OpenMP/for_firstprivate_codegen.cpp
  clang/test/OpenMP/for_lastprivate_codegen.cpp
  clang/test/OpenMP/for_linear_codegen.cpp
  clang/test/OpenMP/for_private_codegen.cpp
  clang/test/OpenMP/for_reduction_codegen.cpp
  clang/test/OpenMP/for_reduction_codegen_UDR.cpp
  clang/test/OpenMP/for_reduction_task_codegen.cpp
  clang/test/OpenMP/function-attr.cpp
  clang/test/OpenMP/master_taskloop_in_reduction_codegen.cpp
  clang/test/OpenMP/master_taskloop_simd_in_reduction_codegen.cpp
  clang/test/OpenMP/metadirective_device_arch_codegen.cpp
  clang/test/OpenMP/metadirective_device_isa_codegen.cpp
  clang/test/OpenMP/metadirective_implementation_codegen.c
  clang/test/OpenMP/nested_loop_codegen.cpp
  clang/test/OpenMP/nvptx_SPMD_codegen.cpp
  clang/test/OpenMP/nvptx_allocate_codegen.cpp
  clang/test/OpenMP/nvptx_data_sharing.cpp
  clang/test/OpenMP/nvptx_distribute_parallel_generic_mode_codegen.cpp
  clang/test/OpenMP/nvptx_lambda_capturing.cpp
  clang/test/OpenMP/nvptx_multi_target_parallel_codegen.cpp
  clang/test/OpenMP/nvptx_nested_parallel_codegen.cpp
  clang/test/OpenMP/nvptx_parallel_codegen.cpp
  clang/test/OpenMP/nvptx_parallel_for_codegen.cpp
  clang/test/OpenMP/nvptx_target_codegen.cpp
  clang/test/OpenMP/nvptx_target_parallel_codegen.cpp
  clang/test/OpenMP/nvptx_target_parallel_num_threads_codegen.cpp
  clang/test/OpenMP/nvptx_target_parallel_proc_bind_codegen.cpp
  clang/test/OpenMP/nvptx_target_parallel_reduction_codegen.cpp
  clang/test/OpenMP/nvptx_target_parallel_reduction_codegen_tbaa_PR46146.cpp
  clang/test/OpenMP/nvptx_target_teams_codegen.cpp
  clang/test/OpenMP/nvptx_target_teams_distribute_codegen.cpp
  clang/test/OpenMP/nvptx_target_teams_distribute_parallel_for_codegen.cpp
  
clang/test/OpenMP/nvptx_target_teams_distribute_parallel_for_generic_mode_codegen.cpp
  clang/test/OpenMP/nvptx_target_teams_distribute_parallel_for_simd_codegen.cpp
  clang/test/OpenMP/nvptx_target_teams_distribute_simd_codegen.cpp
  clang/test/OpenMP/nvptx_teams_codegen.cpp
  clang/test/OpenMP/nvptx_teams_reduction_codegen.cpp
  clang/test/OpenMP/omp_with_loop_pragma_instr_profile.c
  clang/test/OpenMP/openmp_win_codegen.cpp
  clang/test/OpenMP/outlined_artificial.c
  clang/test/OpenMP/parallel_codegen.cpp
  clang/test/OpenMP/parallel_copyin_codegen.cpp
  clang/test/OpenMP/parallel_copyin_combined_codegen.c
  clang/tes

[PATCH] D140722: [OpenMP] Prefix outlined and reduction func names with original func's name

[Itay Bookstein via Phabricator via cfe-commits]

nextsilicon-itay-bookstein updated this revision to Diff 515025.
nextsilicon-itay-bookstein edited the summary of this revision.
nextsilicon-itay-bookstein added a comment.

Updated a couple of tests that were introduced in the interim


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D140722/new/

https://reviews.llvm.org/D140722

Files:
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/lib/CodeGen/CGOpenMPRuntime.h
  clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp
  clang/lib/CodeGen/CGOpenMPRuntimeGPU.h
  clang/lib/CodeGen/CGStmtOpenMP.cpp
  clang/test/CodeGen/PowerPC/ppc64le-varargs-f128.c
  clang/test/CoverageMapping/openmp.c
  clang/test/OpenMP/amdgpu_target_with_aligned_attribute.c
  clang/test/OpenMP/bug54082.c
  clang/test/OpenMP/bug60602.cpp
  clang/test/OpenMP/cancel_codegen.cpp
  clang/test/OpenMP/cancellation_point_codegen.cpp
  clang/test/OpenMP/debug-info-complex-byval.cpp
  clang/test/OpenMP/debug-info-openmp-array.cpp
  clang/test/OpenMP/debug_threadprivate_copyin.c
  clang/test/OpenMP/declare_target_codegen_globalization.cpp
  clang/test/OpenMP/distribute_codegen.cpp
  clang/test/OpenMP/distribute_firstprivate_codegen.cpp
  clang/test/OpenMP/distribute_lastprivate_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_firstprivate_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_if_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_lastprivate_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_num_threads_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_private_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_proc_bind_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_reduction_task_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_simd_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_simd_firstprivate_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_simd_if_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_simd_lastprivate_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_simd_num_threads_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_simd_private_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_simd_proc_bind_codegen.cpp
  clang/test/OpenMP/distribute_private_codegen.cpp
  clang/test/OpenMP/distribute_simd_codegen.cpp
  clang/test/OpenMP/distribute_simd_firstprivate_codegen.cpp
  clang/test/OpenMP/distribute_simd_lastprivate_codegen.cpp
  clang/test/OpenMP/distribute_simd_private_codegen.cpp
  clang/test/OpenMP/distribute_simd_reduction_codegen.cpp
  clang/test/OpenMP/for_firstprivate_codegen.cpp
  clang/test/OpenMP/for_lastprivate_codegen.cpp
  clang/test/OpenMP/for_linear_codegen.cpp
  clang/test/OpenMP/for_private_codegen.cpp
  clang/test/OpenMP/for_reduction_codegen.cpp
  clang/test/OpenMP/for_reduction_codegen_UDR.cpp
  clang/test/OpenMP/for_reduction_task_codegen.cpp
  clang/test/OpenMP/function-attr.cpp
  clang/test/OpenMP/master_taskloop_in_reduction_codegen.cpp
  clang/test/OpenMP/master_taskloop_simd_in_reduction_codegen.cpp
  clang/test/OpenMP/metadirective_device_arch_codegen.cpp
  clang/test/OpenMP/metadirective_device_isa_codegen.cpp
  clang/test/OpenMP/metadirective_implementation_codegen.c
  clang/test/OpenMP/nested_loop_codegen.cpp
  clang/test/OpenMP/nvptx_SPMD_codegen.cpp
  clang/test/OpenMP/nvptx_allocate_codegen.cpp
  clang/test/OpenMP/nvptx_data_sharing.cpp
  clang/test/OpenMP/nvptx_distribute_parallel_generic_mode_codegen.cpp
  clang/test/OpenMP/nvptx_lambda_capturing.cpp
  clang/test/OpenMP/nvptx_multi_target_parallel_codegen.cpp
  clang/test/OpenMP/nvptx_nested_parallel_codegen.cpp
  clang/test/OpenMP/nvptx_parallel_codegen.cpp
  clang/test/OpenMP/nvptx_parallel_for_codegen.cpp
  clang/test/OpenMP/nvptx_target_codegen.cpp
  clang/test/OpenMP/nvptx_target_parallel_codegen.cpp
  clang/test/OpenMP/nvptx_target_parallel_num_threads_codegen.cpp
  clang/test/OpenMP/nvptx_target_parallel_proc_bind_codegen.cpp
  clang/test/OpenMP/nvptx_target_parallel_reduction_codegen.cpp
  clang/test/OpenMP/nvptx_target_parallel_reduction_codegen_tbaa_PR46146.cpp
  clang/test/OpenMP/nvptx_target_teams_codegen.cpp
  clang/test/OpenMP/nvptx_target_teams_distribute_codegen.cpp
  clang/test/OpenMP/nvptx_target_teams_distribute_parallel_for_codegen.cpp
  
clang/test/OpenMP/nvptx_target_teams_distribute_parallel_for_generic_mode_codegen.cpp
  clang/test/OpenMP/nvptx_target_teams_distribute_parallel_for_simd_codegen.cpp
  clang/test/OpenMP/nvptx_target_teams_distribute_simd_codegen.cpp
  clang/test/OpenMP/nvptx_teams_codegen.cpp
  clang/test/OpenMP/nvptx_teams_reduction_codegen.cpp
  clang/test/OpenMP/omp_with_loop_pragma_instr_profile.c
  clang/test/OpenMP/openmp_win_codegen.cpp
  clang/test/OpenMP/outlined_artificial.c
  clang/test/OpenMP/parallel_codegen.cpp
  clang/test/OpenMP/parallel_copyin_codegen.cpp
  clang/test/OpenMP/parallel_copyin_combined_codegen.c
  clang/test/OpenMP/parallel_firstprivate_codegen.cpp
  

[PATCH] D140722: [OpenMP] Prefix outlined and reduction func names with original func's name

[Itay Bookstein via Phabricator via cfe-commits]

nextsilicon-itay-bookstein reopened this revision.
nextsilicon-itay-bookstein marked 2 inline comments as done.
nextsilicon-itay-bookstein added a comment.
This revision is now accepted and ready to land.

Reopening due to revert, will update with a fixed patch


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D140722/new/

https://reviews.llvm.org/D140722

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D140722: [OpenMP] Prefix outlined and reduction func names with original func's name

[Itay Bookstein via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG029bfc311d4d: [OpenMP] Prefix outlined and reduction func 
names with original func's name (authored by nextsilicon-itay-bookstein).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D140722/new/

https://reviews.llvm.org/D140722

Files:
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/lib/CodeGen/CGOpenMPRuntime.h
  clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp
  clang/lib/CodeGen/CGOpenMPRuntimeGPU.h
  clang/lib/CodeGen/CGStmtOpenMP.cpp
  clang/test/CodeGen/PowerPC/ppc64le-varargs-f128.c
  clang/test/CoverageMapping/openmp.c
  clang/test/OpenMP/amdgpu_target_with_aligned_attribute.c
  clang/test/OpenMP/bug54082.c
  clang/test/OpenMP/bug60602.cpp
  clang/test/OpenMP/cancel_codegen.cpp
  clang/test/OpenMP/cancellation_point_codegen.cpp
  clang/test/OpenMP/debug-info-complex-byval.cpp
  clang/test/OpenMP/debug-info-openmp-array.cpp
  clang/test/OpenMP/debug_threadprivate_copyin.c
  clang/test/OpenMP/declare_target_codegen_globalization.cpp
  clang/test/OpenMP/distribute_codegen.cpp
  clang/test/OpenMP/distribute_firstprivate_codegen.cpp
  clang/test/OpenMP/distribute_lastprivate_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_firstprivate_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_if_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_lastprivate_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_num_threads_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_private_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_proc_bind_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_reduction_task_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_simd_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_simd_firstprivate_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_simd_if_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_simd_lastprivate_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_simd_num_threads_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_simd_private_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_simd_proc_bind_codegen.cpp
  clang/test/OpenMP/distribute_private_codegen.cpp
  clang/test/OpenMP/distribute_simd_codegen.cpp
  clang/test/OpenMP/distribute_simd_firstprivate_codegen.cpp
  clang/test/OpenMP/distribute_simd_lastprivate_codegen.cpp
  clang/test/OpenMP/distribute_simd_private_codegen.cpp
  clang/test/OpenMP/distribute_simd_reduction_codegen.cpp
  clang/test/OpenMP/for_firstprivate_codegen.cpp
  clang/test/OpenMP/for_lastprivate_codegen.cpp
  clang/test/OpenMP/for_linear_codegen.cpp
  clang/test/OpenMP/for_private_codegen.cpp
  clang/test/OpenMP/for_reduction_codegen.cpp
  clang/test/OpenMP/for_reduction_codegen_UDR.cpp
  clang/test/OpenMP/for_reduction_task_codegen.cpp
  clang/test/OpenMP/function-attr.cpp
  clang/test/OpenMP/master_taskloop_in_reduction_codegen.cpp
  clang/test/OpenMP/master_taskloop_simd_in_reduction_codegen.cpp
  clang/test/OpenMP/metadirective_device_arch_codegen.cpp
  clang/test/OpenMP/metadirective_device_isa_codegen.cpp
  clang/test/OpenMP/metadirective_implementation_codegen.c
  clang/test/OpenMP/nested_loop_codegen.cpp
  clang/test/OpenMP/nvptx_SPMD_codegen.cpp
  clang/test/OpenMP/nvptx_allocate_codegen.cpp
  clang/test/OpenMP/nvptx_data_sharing.cpp
  clang/test/OpenMP/nvptx_distribute_parallel_generic_mode_codegen.cpp
  clang/test/OpenMP/nvptx_lambda_capturing.cpp
  clang/test/OpenMP/nvptx_multi_target_parallel_codegen.cpp
  clang/test/OpenMP/nvptx_nested_parallel_codegen.cpp
  clang/test/OpenMP/nvptx_parallel_codegen.cpp
  clang/test/OpenMP/nvptx_parallel_for_codegen.cpp
  clang/test/OpenMP/nvptx_target_codegen.cpp
  clang/test/OpenMP/nvptx_target_parallel_codegen.cpp
  clang/test/OpenMP/nvptx_target_parallel_num_threads_codegen.cpp
  clang/test/OpenMP/nvptx_target_parallel_proc_bind_codegen.cpp
  clang/test/OpenMP/nvptx_target_parallel_reduction_codegen.cpp
  clang/test/OpenMP/nvptx_target_parallel_reduction_codegen_tbaa_PR46146.cpp
  clang/test/OpenMP/nvptx_target_teams_codegen.cpp
  clang/test/OpenMP/nvptx_target_teams_distribute_codegen.cpp
  clang/test/OpenMP/nvptx_target_teams_distribute_parallel_for_codegen.cpp
  
clang/test/OpenMP/nvptx_target_teams_distribute_parallel_for_generic_mode_codegen.cpp
  clang/test/OpenMP/nvptx_target_teams_distribute_parallel_for_simd_codegen.cpp
  clang/test/OpenMP/nvptx_target_teams_distribute_simd_codegen.cpp
  clang/test/OpenMP/nvptx_teams_codegen.cpp
  clang/test/OpenMP/nvptx_teams_reduction_codegen.cpp
  clang/test/OpenMP/omp_with_loop_pragma_instr_profile.c
  clang/test/OpenMP/openmp_win_codegen.cpp
  clang/test/OpenMP/outlined_artificial.c
  clang/test/OpenMP/parallel_codegen.cpp
  clang/test/OpenMP/parallel_copyin_codegen.cpp
  clang/test/OpenMP/parallel_copyin_combined_codegen.c
  clang/test/OpenMP/parallel_firstprivate_codegen.cpp
  clang/test/

[PATCH] D140722: [OpenMP] Prefix outlined and reduction func names with original func's name

[Itay Bookstein via Phabricator via cfe-commits]

nextsilicon-itay-bookstein added a comment.

Alright, cool, thanks! I'll land it tomorrow evening if there are no objections.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D140722/new/

https://reviews.llvm.org/D140722

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D140722: [OpenMP] Prefix outlined and reduction func names with original func's name

[Itay Bookstein via Phabricator via cfe-commits]

nextsilicon-itay-bookstein added a comment.

Ping (:


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D140722/new/

https://reviews.llvm.org/D140722

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D140722: [OpenMP] Prefix outlined and reduction func names with original func's name

[Itay Bookstein via Phabricator via cfe-commits]

nextsilicon-itay-bookstein added a comment.

Ping :)


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D140722/new/

https://reviews.llvm.org/D140722

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D140722: [OpenMP] Prefix outlined and reduction func names with original func's name

[Itay Bookstein via Phabricator via cfe-commits]

nextsilicon-itay-bookstein added a comment.

I don't have non-cumbersome access to GPU targets at the moment.
I have run the following locally with the patch, and it seems to pass:

  > ninja check-libomptarget
  [0/1] Running libomptarget tests
  
  Testing Time: 9.69s
Unsupported:  57
Passed : 205


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D140722/new/

https://reviews.llvm.org/D140722

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D140722: [OpenMP] Prefix outlined and reduction func names with original func's name

[Itay Bookstein via Phabricator via cfe-commits]

nextsilicon-itay-bookstein added a comment.

Passes CI. How should I proceed w.r.t. libomptarget tests?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D140722/new/

https://reviews.llvm.org/D140722

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D140722: [OpenMP] Prefix outlined and reduction func names with original func's name

[Itay Bookstein via Phabricator via cfe-commits]

nextsilicon-itay-bookstein updated this revision to Diff 508116.
nextsilicon-itay-bookstein added a comment.

Another minor tweak to clang/test/CodeGen/PowerPC/ppc64le-varargs-f128.c.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D140722/new/

https://reviews.llvm.org/D140722

Files:
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/lib/CodeGen/CGOpenMPRuntime.h
  clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp
  clang/lib/CodeGen/CGOpenMPRuntimeGPU.h
  clang/lib/CodeGen/CGStmtOpenMP.cpp
  clang/test/CodeGen/PowerPC/ppc64le-varargs-f128.c
  clang/test/CoverageMapping/openmp.c
  clang/test/OpenMP/amdgpu_target_with_aligned_attribute.c
  clang/test/OpenMP/bug54082.c
  clang/test/OpenMP/bug60602.cpp
  clang/test/OpenMP/cancel_codegen.cpp
  clang/test/OpenMP/cancellation_point_codegen.cpp
  clang/test/OpenMP/debug-info-complex-byval.cpp
  clang/test/OpenMP/debug-info-openmp-array.cpp
  clang/test/OpenMP/debug_threadprivate_copyin.c
  clang/test/OpenMP/declare_target_codegen_globalization.cpp
  clang/test/OpenMP/distribute_codegen.cpp
  clang/test/OpenMP/distribute_firstprivate_codegen.cpp
  clang/test/OpenMP/distribute_lastprivate_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_firstprivate_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_if_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_lastprivate_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_num_threads_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_private_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_proc_bind_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_reduction_task_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_simd_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_simd_firstprivate_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_simd_if_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_simd_lastprivate_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_simd_num_threads_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_simd_private_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_simd_proc_bind_codegen.cpp
  clang/test/OpenMP/distribute_private_codegen.cpp
  clang/test/OpenMP/distribute_simd_codegen.cpp
  clang/test/OpenMP/distribute_simd_firstprivate_codegen.cpp
  clang/test/OpenMP/distribute_simd_lastprivate_codegen.cpp
  clang/test/OpenMP/distribute_simd_private_codegen.cpp
  clang/test/OpenMP/distribute_simd_reduction_codegen.cpp
  clang/test/OpenMP/for_firstprivate_codegen.cpp
  clang/test/OpenMP/for_lastprivate_codegen.cpp
  clang/test/OpenMP/for_linear_codegen.cpp
  clang/test/OpenMP/for_private_codegen.cpp
  clang/test/OpenMP/for_reduction_codegen.cpp
  clang/test/OpenMP/for_reduction_codegen_UDR.cpp
  clang/test/OpenMP/for_reduction_task_codegen.cpp
  clang/test/OpenMP/function-attr.cpp
  clang/test/OpenMP/master_taskloop_in_reduction_codegen.cpp
  clang/test/OpenMP/master_taskloop_simd_in_reduction_codegen.cpp
  clang/test/OpenMP/metadirective_device_arch_codegen.cpp
  clang/test/OpenMP/metadirective_device_isa_codegen.cpp
  clang/test/OpenMP/metadirective_implementation_codegen.c
  clang/test/OpenMP/nested_loop_codegen.cpp
  clang/test/OpenMP/nvptx_SPMD_codegen.cpp
  clang/test/OpenMP/nvptx_allocate_codegen.cpp
  clang/test/OpenMP/nvptx_data_sharing.cpp
  clang/test/OpenMP/nvptx_distribute_parallel_generic_mode_codegen.cpp
  clang/test/OpenMP/nvptx_lambda_capturing.cpp
  clang/test/OpenMP/nvptx_multi_target_parallel_codegen.cpp
  clang/test/OpenMP/nvptx_nested_parallel_codegen.cpp
  clang/test/OpenMP/nvptx_parallel_codegen.cpp
  clang/test/OpenMP/nvptx_parallel_for_codegen.cpp
  clang/test/OpenMP/nvptx_target_codegen.cpp
  clang/test/OpenMP/nvptx_target_parallel_codegen.cpp
  clang/test/OpenMP/nvptx_target_parallel_num_threads_codegen.cpp
  clang/test/OpenMP/nvptx_target_parallel_proc_bind_codegen.cpp
  clang/test/OpenMP/nvptx_target_parallel_reduction_codegen.cpp
  clang/test/OpenMP/nvptx_target_parallel_reduction_codegen_tbaa_PR46146.cpp
  clang/test/OpenMP/nvptx_target_teams_codegen.cpp
  clang/test/OpenMP/nvptx_target_teams_distribute_codegen.cpp
  clang/test/OpenMP/nvptx_target_teams_distribute_parallel_for_codegen.cpp
  
clang/test/OpenMP/nvptx_target_teams_distribute_parallel_for_generic_mode_codegen.cpp
  clang/test/OpenMP/nvptx_target_teams_distribute_parallel_for_simd_codegen.cpp
  clang/test/OpenMP/nvptx_target_teams_distribute_simd_codegen.cpp
  clang/test/OpenMP/nvptx_teams_codegen.cpp
  clang/test/OpenMP/nvptx_teams_reduction_codegen.cpp
  clang/test/OpenMP/omp_with_loop_pragma_instr_profile.c
  clang/test/OpenMP/openmp_win_codegen.cpp
  clang/test/OpenMP/outlined_artificial.c
  clang/test/OpenMP/parallel_codegen.cpp
  clang/test/OpenMP/parallel_copyin_codegen.cpp
  clang/test/OpenMP/parallel_copyin_combined_codegen.c
  clang/test/OpenMP/parallel_firstprivate_codegen.cpp
  clang/test/OpenMP/parallel_for_codegen.cpp
  clang/test/OpenMP/parallel_for_lastprivate_con

[PATCH] D140722: [OpenMP] Prefix outlined and reduction func names with original func's name

[Itay Bookstein via Phabricator via cfe-commits]

nextsilicon-itay-bookstein updated this revision to Diff 508059.
nextsilicon-itay-bookstein added a comment.

Retry, wrong patch uploaded last time.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D140722/new/

https://reviews.llvm.org/D140722

Files:
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/lib/CodeGen/CGOpenMPRuntime.h
  clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp
  clang/lib/CodeGen/CGOpenMPRuntimeGPU.h
  clang/lib/CodeGen/CGStmtOpenMP.cpp
  clang/test/CodeGen/PowerPC/ppc64le-varargs-f128.c
  clang/test/CoverageMapping/openmp.c
  clang/test/OpenMP/amdgpu_target_with_aligned_attribute.c
  clang/test/OpenMP/bug54082.c
  clang/test/OpenMP/bug60602.cpp
  clang/test/OpenMP/cancel_codegen.cpp
  clang/test/OpenMP/cancellation_point_codegen.cpp
  clang/test/OpenMP/debug-info-complex-byval.cpp
  clang/test/OpenMP/debug-info-openmp-array.cpp
  clang/test/OpenMP/debug_threadprivate_copyin.c
  clang/test/OpenMP/declare_target_codegen_globalization.cpp
  clang/test/OpenMP/distribute_codegen.cpp
  clang/test/OpenMP/distribute_firstprivate_codegen.cpp
  clang/test/OpenMP/distribute_lastprivate_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_firstprivate_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_if_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_lastprivate_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_num_threads_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_private_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_proc_bind_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_reduction_task_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_simd_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_simd_firstprivate_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_simd_if_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_simd_lastprivate_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_simd_num_threads_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_simd_private_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_simd_proc_bind_codegen.cpp
  clang/test/OpenMP/distribute_private_codegen.cpp
  clang/test/OpenMP/distribute_simd_codegen.cpp
  clang/test/OpenMP/distribute_simd_firstprivate_codegen.cpp
  clang/test/OpenMP/distribute_simd_lastprivate_codegen.cpp
  clang/test/OpenMP/distribute_simd_private_codegen.cpp
  clang/test/OpenMP/distribute_simd_reduction_codegen.cpp
  clang/test/OpenMP/for_firstprivate_codegen.cpp
  clang/test/OpenMP/for_lastprivate_codegen.cpp
  clang/test/OpenMP/for_linear_codegen.cpp
  clang/test/OpenMP/for_private_codegen.cpp
  clang/test/OpenMP/for_reduction_codegen.cpp
  clang/test/OpenMP/for_reduction_codegen_UDR.cpp
  clang/test/OpenMP/for_reduction_task_codegen.cpp
  clang/test/OpenMP/function-attr.cpp
  clang/test/OpenMP/master_taskloop_in_reduction_codegen.cpp
  clang/test/OpenMP/master_taskloop_simd_in_reduction_codegen.cpp
  clang/test/OpenMP/metadirective_device_arch_codegen.cpp
  clang/test/OpenMP/metadirective_device_isa_codegen.cpp
  clang/test/OpenMP/metadirective_implementation_codegen.c
  clang/test/OpenMP/nested_loop_codegen.cpp
  clang/test/OpenMP/nvptx_SPMD_codegen.cpp
  clang/test/OpenMP/nvptx_allocate_codegen.cpp
  clang/test/OpenMP/nvptx_data_sharing.cpp
  clang/test/OpenMP/nvptx_distribute_parallel_generic_mode_codegen.cpp
  clang/test/OpenMP/nvptx_lambda_capturing.cpp
  clang/test/OpenMP/nvptx_multi_target_parallel_codegen.cpp
  clang/test/OpenMP/nvptx_nested_parallel_codegen.cpp
  clang/test/OpenMP/nvptx_parallel_codegen.cpp
  clang/test/OpenMP/nvptx_parallel_for_codegen.cpp
  clang/test/OpenMP/nvptx_target_codegen.cpp
  clang/test/OpenMP/nvptx_target_parallel_codegen.cpp
  clang/test/OpenMP/nvptx_target_parallel_num_threads_codegen.cpp
  clang/test/OpenMP/nvptx_target_parallel_proc_bind_codegen.cpp
  clang/test/OpenMP/nvptx_target_parallel_reduction_codegen.cpp
  clang/test/OpenMP/nvptx_target_parallel_reduction_codegen_tbaa_PR46146.cpp
  clang/test/OpenMP/nvptx_target_teams_codegen.cpp
  clang/test/OpenMP/nvptx_target_teams_distribute_codegen.cpp
  clang/test/OpenMP/nvptx_target_teams_distribute_parallel_for_codegen.cpp
  
clang/test/OpenMP/nvptx_target_teams_distribute_parallel_for_generic_mode_codegen.cpp
  clang/test/OpenMP/nvptx_target_teams_distribute_parallel_for_simd_codegen.cpp
  clang/test/OpenMP/nvptx_target_teams_distribute_simd_codegen.cpp
  clang/test/OpenMP/nvptx_teams_codegen.cpp
  clang/test/OpenMP/nvptx_teams_reduction_codegen.cpp
  clang/test/OpenMP/omp_with_loop_pragma_instr_profile.c
  clang/test/OpenMP/openmp_win_codegen.cpp
  clang/test/OpenMP/outlined_artificial.c
  clang/test/OpenMP/parallel_codegen.cpp
  clang/test/OpenMP/parallel_copyin_codegen.cpp
  clang/test/OpenMP/parallel_copyin_combined_codegen.c
  clang/test/OpenMP/parallel_firstprivate_codegen.cpp
  clang/test/OpenMP/parallel_for_codegen.cpp
  clang/test/OpenMP/parallel_for_lastprivate_conditional.cpp
  clang/test/OpenMP/pa

[PATCH] D140722: [OpenMP] Prefix outlined and reduction func names with original func's name

[Itay Bookstein via Phabricator via cfe-commits]

nextsilicon-itay-bookstein updated this revision to Diff 508054.
nextsilicon-itay-bookstein added a comment.

Minor fix to the clang/CodeGen/ppc64le-varargs-f128.c test.

@jdoerfert Does the PR CI run these, or are there build bots that cover the 
different target-offloading variants? Can I somehow check this prior to merging 
without having all hardware variants on-hand?


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D140722/new/

https://reviews.llvm.org/D140722

Files:
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/lib/CodeGen/CGOpenMPRuntime.h
  clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp
  clang/lib/CodeGen/CGOpenMPRuntimeGPU.h
  clang/lib/CodeGen/CGStmtOpenMP.cpp
  clang/test/CodeGen/PowerPC/ppc64le-varargs-f128.c
  clang/test/CoverageMapping/openmp.c
  clang/test/OpenMP/amdgpu_target_with_aligned_attribute.c
  clang/test/OpenMP/bug54082.c
  clang/test/OpenMP/bug60602.cpp
  clang/test/OpenMP/cancel_codegen.cpp
  clang/test/OpenMP/cancellation_point_codegen.cpp
  clang/test/OpenMP/debug-info-complex-byval.cpp
  clang/test/OpenMP/debug-info-openmp-array.cpp
  clang/test/OpenMP/debug_threadprivate_copyin.c
  clang/test/OpenMP/declare_target_codegen_globalization.cpp
  clang/test/OpenMP/distribute_codegen.cpp
  clang/test/OpenMP/distribute_firstprivate_codegen.cpp
  clang/test/OpenMP/distribute_lastprivate_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_firstprivate_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_if_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_lastprivate_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_num_threads_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_private_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_proc_bind_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_reduction_task_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_simd_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_simd_firstprivate_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_simd_if_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_simd_lastprivate_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_simd_num_threads_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_simd_private_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_simd_proc_bind_codegen.cpp
  clang/test/OpenMP/distribute_private_codegen.cpp
  clang/test/OpenMP/distribute_simd_codegen.cpp
  clang/test/OpenMP/distribute_simd_firstprivate_codegen.cpp
  clang/test/OpenMP/distribute_simd_lastprivate_codegen.cpp
  clang/test/OpenMP/distribute_simd_private_codegen.cpp
  clang/test/OpenMP/distribute_simd_reduction_codegen.cpp
  clang/test/OpenMP/for_firstprivate_codegen.cpp
  clang/test/OpenMP/for_lastprivate_codegen.cpp
  clang/test/OpenMP/for_linear_codegen.cpp
  clang/test/OpenMP/for_private_codegen.cpp
  clang/test/OpenMP/for_reduction_codegen.cpp
  clang/test/OpenMP/for_reduction_codegen_UDR.cpp
  clang/test/OpenMP/for_reduction_task_codegen.cpp
  clang/test/OpenMP/function-attr.cpp
  clang/test/OpenMP/master_taskloop_in_reduction_codegen.cpp
  clang/test/OpenMP/master_taskloop_simd_in_reduction_codegen.cpp
  clang/test/OpenMP/metadirective_device_arch_codegen.cpp
  clang/test/OpenMP/metadirective_device_isa_codegen.cpp
  clang/test/OpenMP/metadirective_implementation_codegen.c
  clang/test/OpenMP/nested_loop_codegen.cpp
  clang/test/OpenMP/nvptx_SPMD_codegen.cpp
  clang/test/OpenMP/nvptx_allocate_codegen.cpp
  clang/test/OpenMP/nvptx_data_sharing.cpp
  clang/test/OpenMP/nvptx_distribute_parallel_generic_mode_codegen.cpp
  clang/test/OpenMP/nvptx_lambda_capturing.cpp
  clang/test/OpenMP/nvptx_multi_target_parallel_codegen.cpp
  clang/test/OpenMP/nvptx_nested_parallel_codegen.cpp
  clang/test/OpenMP/nvptx_parallel_codegen.cpp
  clang/test/OpenMP/nvptx_parallel_for_codegen.cpp
  clang/test/OpenMP/nvptx_target_codegen.cpp
  clang/test/OpenMP/nvptx_target_parallel_codegen.cpp
  clang/test/OpenMP/nvptx_target_parallel_num_threads_codegen.cpp
  clang/test/OpenMP/nvptx_target_parallel_proc_bind_codegen.cpp
  clang/test/OpenMP/nvptx_target_parallel_reduction_codegen.cpp
  clang/test/OpenMP/nvptx_target_parallel_reduction_codegen_tbaa_PR46146.cpp
  clang/test/OpenMP/nvptx_target_teams_codegen.cpp
  clang/test/OpenMP/nvptx_target_teams_distribute_codegen.cpp
  clang/test/OpenMP/nvptx_target_teams_distribute_parallel_for_codegen.cpp
  
clang/test/OpenMP/nvptx_target_teams_distribute_parallel_for_generic_mode_codegen.cpp
  clang/test/OpenMP/nvptx_target_teams_distribute_parallel_for_simd_codegen.cpp
  clang/test/OpenMP/nvptx_target_teams_distribute_simd_codegen.cpp
  clang/test/OpenMP/nvptx_teams_codegen.cpp
  clang/test/OpenMP/nvptx_teams_reduction_codegen.cpp
  clang/test/OpenMP/omp_with_loop_pragma_instr_profile.c
  clang/test/OpenMP/openmp_win_codegen.cpp
  clang/test/OpenMP/outlined_artificial.c
  clang/test/OpenMP/parallel_codegen.cpp
  clang/test/OpenMP/parallel_copyin_codegen.cpp
  clang/tes

[PATCH] D140722: [OpenMP] Prefix outlined and reduction func names with original func's name

[Itay Bookstein via Phabricator via cfe-commits]

nextsilicon-itay-bookstein updated this revision to Diff 507532.
nextsilicon-itay-bookstein added a comment.
Herald added subscribers: kosarev, kerbowa, jvesely.

Removed the empty string in the `getName()` invocation for the outlined 
helper's name.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D140722/new/

https://reviews.llvm.org/D140722

Files:
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/lib/CodeGen/CGOpenMPRuntime.h
  clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp
  clang/lib/CodeGen/CGOpenMPRuntimeGPU.h
  clang/lib/CodeGen/CGStmtOpenMP.cpp
  clang/test/CodeGen/PowerPC/ppc64le-varargs-f128.c
  clang/test/CoverageMapping/openmp.c
  clang/test/OpenMP/amdgpu_target_with_aligned_attribute.c
  clang/test/OpenMP/bug54082.c
  clang/test/OpenMP/bug60602.cpp
  clang/test/OpenMP/cancel_codegen.cpp
  clang/test/OpenMP/cancellation_point_codegen.cpp
  clang/test/OpenMP/debug-info-complex-byval.cpp
  clang/test/OpenMP/debug-info-openmp-array.cpp
  clang/test/OpenMP/debug_threadprivate_copyin.c
  clang/test/OpenMP/declare_target_codegen_globalization.cpp
  clang/test/OpenMP/distribute_codegen.cpp
  clang/test/OpenMP/distribute_firstprivate_codegen.cpp
  clang/test/OpenMP/distribute_lastprivate_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_firstprivate_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_if_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_lastprivate_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_num_threads_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_private_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_proc_bind_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_reduction_task_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_simd_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_simd_firstprivate_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_simd_if_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_simd_lastprivate_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_simd_num_threads_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_simd_private_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_simd_proc_bind_codegen.cpp
  clang/test/OpenMP/distribute_private_codegen.cpp
  clang/test/OpenMP/distribute_simd_codegen.cpp
  clang/test/OpenMP/distribute_simd_firstprivate_codegen.cpp
  clang/test/OpenMP/distribute_simd_lastprivate_codegen.cpp
  clang/test/OpenMP/distribute_simd_private_codegen.cpp
  clang/test/OpenMP/distribute_simd_reduction_codegen.cpp
  clang/test/OpenMP/for_firstprivate_codegen.cpp
  clang/test/OpenMP/for_lastprivate_codegen.cpp
  clang/test/OpenMP/for_linear_codegen.cpp
  clang/test/OpenMP/for_private_codegen.cpp
  clang/test/OpenMP/for_reduction_codegen.cpp
  clang/test/OpenMP/for_reduction_codegen_UDR.cpp
  clang/test/OpenMP/for_reduction_task_codegen.cpp
  clang/test/OpenMP/function-attr.cpp
  clang/test/OpenMP/master_taskloop_in_reduction_codegen.cpp
  clang/test/OpenMP/master_taskloop_simd_in_reduction_codegen.cpp
  clang/test/OpenMP/metadirective_device_arch_codegen.cpp
  clang/test/OpenMP/metadirective_device_isa_codegen.cpp
  clang/test/OpenMP/metadirective_implementation_codegen.c
  clang/test/OpenMP/nested_loop_codegen.cpp
  clang/test/OpenMP/nvptx_SPMD_codegen.cpp
  clang/test/OpenMP/nvptx_allocate_codegen.cpp
  clang/test/OpenMP/nvptx_data_sharing.cpp
  clang/test/OpenMP/nvptx_distribute_parallel_generic_mode_codegen.cpp
  clang/test/OpenMP/nvptx_lambda_capturing.cpp
  clang/test/OpenMP/nvptx_multi_target_parallel_codegen.cpp
  clang/test/OpenMP/nvptx_nested_parallel_codegen.cpp
  clang/test/OpenMP/nvptx_parallel_codegen.cpp
  clang/test/OpenMP/nvptx_parallel_for_codegen.cpp
  clang/test/OpenMP/nvptx_target_codegen.cpp
  clang/test/OpenMP/nvptx_target_parallel_codegen.cpp
  clang/test/OpenMP/nvptx_target_parallel_num_threads_codegen.cpp
  clang/test/OpenMP/nvptx_target_parallel_proc_bind_codegen.cpp
  clang/test/OpenMP/nvptx_target_parallel_reduction_codegen.cpp
  clang/test/OpenMP/nvptx_target_parallel_reduction_codegen_tbaa_PR46146.cpp
  clang/test/OpenMP/nvptx_target_teams_codegen.cpp
  clang/test/OpenMP/nvptx_target_teams_distribute_codegen.cpp
  clang/test/OpenMP/nvptx_target_teams_distribute_parallel_for_codegen.cpp
  
clang/test/OpenMP/nvptx_target_teams_distribute_parallel_for_generic_mode_codegen.cpp
  clang/test/OpenMP/nvptx_target_teams_distribute_parallel_for_simd_codegen.cpp
  clang/test/OpenMP/nvptx_target_teams_distribute_simd_codegen.cpp
  clang/test/OpenMP/nvptx_teams_codegen.cpp
  clang/test/OpenMP/nvptx_teams_reduction_codegen.cpp
  clang/test/OpenMP/omp_with_loop_pragma_instr_profile.c
  clang/test/OpenMP/openmp_win_codegen.cpp
  clang/test/OpenMP/outlined_artificial.c
  clang/test/OpenMP/parallel_codegen.cpp
  clang/test/OpenMP/parallel_copyin_codegen.cpp
  clang/test/OpenMP/parallel_copyin_combined_codegen.c
  clang/test/OpenMP/parallel_firstprivate_codegen.cpp
  clang/test/OpenMP/parall

[PATCH] D140722: [OpenMP] Prefix outlined and reduction func names with original func's name

[Itay Bookstein via Phabricator via cfe-commits]

nextsilicon-itay-bookstein added a comment.

I had to cut context lines to 2 to fit in the 8MB limit. It looks like there 
are a few files that absorb way more line diff than makes sense, but I haven't 
investigated way. One particular offender is 
`clang/test/OpenMP/nvptx_SPMD_codegen.cpp`, which loses 23KLOC. I think some of 
the NVPTX files are not idempotent under update_cc_test_checks in master, but I 
haven't checked again to make sure.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D140722/new/

https://reviews.llvm.org/D140722

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D140722: [OpenMP] Prefix outlined and reduction func names with original func's name

[Itay Bookstein via Phabricator via cfe-commits]

nextsilicon-itay-bookstein updated this revision to Diff 507121.
nextsilicon-itay-bookstein added a comment.
Herald added subscribers: mattd, asavonic, zzheng, kbarton, nemanjai.

Update relevant test outputs.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D140722/new/

https://reviews.llvm.org/D140722

Files:
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/lib/CodeGen/CGOpenMPRuntime.h
  clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp
  clang/lib/CodeGen/CGOpenMPRuntimeGPU.h
  clang/lib/CodeGen/CGStmtOpenMP.cpp
  clang/test/CodeGen/PowerPC/ppc64le-varargs-f128.c
  clang/test/OpenMP/bug54082.c
  clang/test/OpenMP/bug60602.cpp
  clang/test/OpenMP/cancel_codegen.cpp
  clang/test/OpenMP/cancellation_point_codegen.cpp
  clang/test/OpenMP/debug-info-complex-byval.cpp
  clang/test/OpenMP/debug-info-openmp-array.cpp
  clang/test/OpenMP/debug_threadprivate_copyin.c
  clang/test/OpenMP/declare_target_codegen_globalization.cpp
  clang/test/OpenMP/distribute_codegen.cpp
  clang/test/OpenMP/distribute_firstprivate_codegen.cpp
  clang/test/OpenMP/distribute_lastprivate_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_firstprivate_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_if_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_lastprivate_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_num_threads_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_private_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_proc_bind_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_reduction_task_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_simd_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_simd_firstprivate_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_simd_if_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_simd_lastprivate_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_simd_num_threads_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_simd_private_codegen.cpp
  clang/test/OpenMP/distribute_parallel_for_simd_proc_bind_codegen.cpp
  clang/test/OpenMP/distribute_private_codegen.cpp
  clang/test/OpenMP/distribute_simd_codegen.cpp
  clang/test/OpenMP/distribute_simd_firstprivate_codegen.cpp
  clang/test/OpenMP/distribute_simd_lastprivate_codegen.cpp
  clang/test/OpenMP/distribute_simd_private_codegen.cpp
  clang/test/OpenMP/distribute_simd_reduction_codegen.cpp
  clang/test/OpenMP/for_firstprivate_codegen.cpp
  clang/test/OpenMP/for_lastprivate_codegen.cpp
  clang/test/OpenMP/for_linear_codegen.cpp
  clang/test/OpenMP/for_private_codegen.cpp
  clang/test/OpenMP/for_reduction_codegen.cpp
  clang/test/OpenMP/for_reduction_codegen_UDR.cpp
  clang/test/OpenMP/for_reduction_task_codegen.cpp
  clang/test/OpenMP/function-attr.cpp
  clang/test/OpenMP/master_taskloop_in_reduction_codegen.cpp
  clang/test/OpenMP/master_taskloop_simd_in_reduction_codegen.cpp
  clang/test/OpenMP/metadirective_device_arch_codegen.cpp
  clang/test/OpenMP/metadirective_device_isa_codegen.cpp
  clang/test/OpenMP/metadirective_implementation_codegen.c
  clang/test/OpenMP/nested_loop_codegen.cpp
  clang/test/OpenMP/nvptx_SPMD_codegen.cpp
  clang/test/OpenMP/nvptx_allocate_codegen.cpp
  clang/test/OpenMP/nvptx_data_sharing.cpp
  clang/test/OpenMP/nvptx_distribute_parallel_generic_mode_codegen.cpp
  clang/test/OpenMP/nvptx_lambda_capturing.cpp
  clang/test/OpenMP/nvptx_multi_target_parallel_codegen.cpp
  clang/test/OpenMP/nvptx_nested_parallel_codegen.cpp
  clang/test/OpenMP/nvptx_parallel_codegen.cpp
  clang/test/OpenMP/nvptx_parallel_for_codegen.cpp
  clang/test/OpenMP/nvptx_target_codegen.cpp
  clang/test/OpenMP/nvptx_target_parallel_codegen.cpp
  clang/test/OpenMP/nvptx_target_parallel_num_threads_codegen.cpp
  clang/test/OpenMP/nvptx_target_parallel_proc_bind_codegen.cpp
  clang/test/OpenMP/nvptx_target_parallel_reduction_codegen.cpp
  clang/test/OpenMP/nvptx_target_parallel_reduction_codegen_tbaa_PR46146.cpp
  clang/test/OpenMP/nvptx_target_teams_codegen.cpp
  clang/test/OpenMP/nvptx_target_teams_distribute_codegen.cpp
  clang/test/OpenMP/nvptx_target_teams_distribute_parallel_for_codegen.cpp
  
clang/test/OpenMP/nvptx_target_teams_distribute_parallel_for_generic_mode_codegen.cpp
  clang/test/OpenMP/nvptx_target_teams_distribute_parallel_for_simd_codegen.cpp
  clang/test/OpenMP/nvptx_target_teams_distribute_simd_codegen.cpp
  clang/test/OpenMP/nvptx_teams_codegen.cpp
  clang/test/OpenMP/nvptx_teams_reduction_codegen.cpp
  clang/test/OpenMP/omp_with_loop_pragma_instr_profile.c
  clang/test/OpenMP/openmp_win_codegen.cpp
  clang/test/OpenMP/outlined_artificial.c
  clang/test/OpenMP/parallel_codegen.cpp
  clang/test/OpenMP/parallel_copyin_codegen.cpp
  clang/test/OpenMP/parallel_copyin_combined_codegen.c
  clang/test/OpenMP/parallel_firstprivate_codegen.cpp
  clang/test/OpenMP/parallel_for_codegen.cpp
  clang/test/OpenMP/parallel_for_lastprivate_conditional.cpp
  clang/test/OpenMP/parallel_for_linear_codegen.cpp
  clan

[PATCH] D140722: [OpenMP] Prefix outlined and reduction func names with original func's name

[Itay Bookstein via Phabricator via cfe-commits]

nextsilicon-itay-bookstein created this revision.
nextsilicon-itay-bookstein added a reviewer: jdoerfert.
Herald added subscribers: guansong, yaxunl.
Herald added a project: All.
nextsilicon-itay-bookstein requested review of this revision.
Herald added subscribers: cfe-commits, sstefan1.
Herald added a project: clang.

This patch attempts to prefix omp outlined helpers and reduction funcs
with the original function's name.

It does not yet update all the CodeGen tests; I'm still trying, but it
seems that some of them are not idempotent under update_cc_test_checks
when I run it on master.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D140722

Files:
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/lib/CodeGen/CGOpenMPRuntime.h
  clang/lib/CodeGen/CGOpenMPRuntimeGPU.cpp
  clang/lib/CodeGen/CGOpenMPRuntimeGPU.h
  clang/lib/CodeGen/CGStmtOpenMP.cpp

Index: clang/lib/CodeGen/CGStmtOpenMP.cpp
===
--- clang/lib/CodeGen/CGStmtOpenMP.cpp
+++ clang/lib/CodeGen/CGStmtOpenMP.cpp
@@ -1545,7 +1545,8 @@
   llvm::Value *NumThreads = nullptr;
   llvm::Function *OutlinedFn =
   CGF.CGM.getOpenMPRuntime().emitParallelOutlinedFunction(
-  S, *CS->getCapturedDecl()->param_begin(), InnermostKind, CodeGen);
+  CGF, S, *CS->getCapturedDecl()->param_begin(), InnermostKind,
+  CodeGen);
   if (const auto *NumThreadsClause = S.getSingleClause()) {
 CodeGenFunction::RunCleanupsScope NumThreadsScope(CGF);
 NumThreads = CGF.EmitScalarExpr(NumThreadsClause->getNumThreads(),
@@ -6745,7 +6746,8 @@
   const CapturedStmt *CS = S.getCapturedStmt(OMPD_teams);
   llvm::Function *OutlinedFn =
   CGF.CGM.getOpenMPRuntime().emitTeamsOutlinedFunction(
-  S, *CS->getCapturedDecl()->param_begin(), InnermostKind, CodeGen);
+  CGF, S, *CS->getCapturedDecl()->param_begin(), InnermostKind,
+  CodeGen);
 
   const auto *NT = S.getSingleClause();
   const auto *TL = S.getSingleClause();
Index: clang/lib/CodeGen/CGOpenMPRuntimeGPU.h
===
--- clang/lib/CodeGen/CGOpenMPRuntimeGPU.h
+++ clang/lib/CodeGen/CGOpenMPRuntimeGPU.h
@@ -142,13 +142,6 @@
 const Expr *IfCond);
 
 protected:
-  /// Get the function name of an outlined region.
-  //  The name can be customized depending on the target.
-  //
-  StringRef getOutlinedHelperName() const override {
-return "__omp_outlined__";
-  }
-
   /// Check if the default location must be constant.
   /// Constant for NVPTX for better optimization.
   bool isDefaultLocationConstant() const override { return true; }
@@ -197,31 +190,31 @@
   //  directive.
   /// \a D. This outlined function has type void(*)(kmp_int32 *ThreadID,
   /// kmp_int32 BoundID, struct context_vars*).
+  /// \param CGF Reference to current CodeGenFunction.
   /// \param D OpenMP directive.
   /// \param ThreadIDVar Variable for thread id in the current OpenMP region.
   /// \param InnermostKind Kind of innermost directive (for simple directives it
   /// is a directive itself, for combined - its innermost directive).
   /// \param CodeGen Code generation sequence for the \a D directive.
-  llvm::Function *
-  emitParallelOutlinedFunction(const OMPExecutableDirective &D,
-   const VarDecl *ThreadIDVar,
-   OpenMPDirectiveKind InnermostKind,
-   const RegionCodeGenTy &CodeGen) override;
+  llvm::Function *emitParallelOutlinedFunction(
+  CodeGenFunction &CGF, const OMPExecutableDirective &D,
+  const VarDecl *ThreadIDVar, OpenMPDirectiveKind InnermostKind,
+  const RegionCodeGenTy &CodeGen) override;
 
   /// Emits inlined function for the specified OpenMP teams
   //  directive.
   /// \a D. This outlined function has type void(*)(kmp_int32 *ThreadID,
   /// kmp_int32 BoundID, struct context_vars*).
+  /// \param CGF Reference to current CodeGenFunction.
   /// \param D OpenMP directive.
   /// \param ThreadIDVar Variable for thread id in the current OpenMP region.
   /// \param InnermostKind Kind of innermost directive (for simple directives it
   /// is a directive itself, for combined - its innermost directive).
   /// \param CodeGen Code generation sequence for the \a D directive.
-  llvm::Function *
-  emitTeamsOutlinedFunction(const OMPExecutableDirective &D,
-const VarDecl *ThreadIDVar,
-OpenMPDirectiveKind InnermostKind,
-const RegionCodeGenTy &CodeGen) override;
+  llvm::Function *emitTeamsOutlinedFunction(
+  CodeGenFunction &CGF, const OMPExecutableDirective &D,
+  const VarDecl *ThreadIDVar, OpenMPDirectiveKind InnermostKind,
+  const RegionCodeGenTy &CodeGen) override;
 
   /// Emits code for teams call of the \a OutlinedFn with
   /// variables captured in a record which address is stored in \a
Index: clang/

[PATCH] D120266: [clang][CodeGen] Avoid emitting ifuncs with undefined resolvers

[Itay Bookstein via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGf3480390be61: [clang][CodeGen] Avoid emitting ifuncs with 
undefined resolvers (authored by ibookstein).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D120266/new/

https://reviews.llvm.org/D120266

Files:
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/test/CodeGen/attr-cpuspecific.c

Index: clang/test/CodeGen/attr-cpuspecific.c
===
--- clang/test/CodeGen/attr-cpuspecific.c
+++ clang/test/CodeGen/attr-cpuspecific.c
@@ -8,9 +8,12 @@
 #endif // _MSC_VER
 
 // Each version should have an IFunc and an alias.
+// LINUX: @SingleVersion = weak_odr alias void (), void ()* @SingleVersion.ifunc
 // LINUX: @TwoVersions = weak_odr alias void (), void ()* @TwoVersions.ifunc
+// LINUX: @OrderDispatchUsageSpecific = weak_odr alias void (), void ()* @OrderDispatchUsageSpecific.ifunc
 // LINUX: @TwoVersionsSameAttr = weak_odr alias void (), void ()* @TwoVersionsSameAttr.ifunc
 // LINUX: @ThreeVersionsSameAttr = weak_odr alias void (), void ()* @ThreeVersionsSameAttr.ifunc
+// LINUX: @OrderSpecificUsageDispatch = weak_odr alias void (), void ()* @OrderSpecificUsageDispatch.ifunc
 // LINUX: @NoSpecifics = weak_odr alias void (), void ()* @NoSpecifics.ifunc
 // LINUX: @HasGeneric = weak_odr alias void (), void ()* @HasGeneric.ifunc
 // LINUX: @HasParams = weak_odr alias void (i32, double), void (i32, double)* @HasParams.ifunc
@@ -18,10 +21,12 @@
 // LINUX: @GenericAndPentium = weak_odr alias i32 (i32, double), i32 (i32, double)* @GenericAndPentium.ifunc
 // LINUX: @DispatchFirst = weak_odr alias i32 (), i32 ()* @DispatchFirst.ifunc
 
-// LINUX: @TwoVersions.ifunc = weak_odr ifunc void (), void ()* ()* @TwoVersions.resolver
 // LINUX: @SingleVersion.ifunc = weak_odr ifunc void (), void ()* ()* @SingleVersion.resolver
+// LINUX: @TwoVersions.ifunc = weak_odr ifunc void (), void ()* ()* @TwoVersions.resolver
+// LINUX: @OrderDispatchUsageSpecific.ifunc = weak_odr ifunc void (), void ()* ()* @OrderDispatchUsageSpecific.resolver
 // LINUX: @TwoVersionsSameAttr.ifunc = weak_odr ifunc void (), void ()* ()* @TwoVersionsSameAttr.resolver
 // LINUX: @ThreeVersionsSameAttr.ifunc = weak_odr ifunc void (), void ()* ()* @ThreeVersionsSameAttr.resolver
+// LINUX: @OrderSpecificUsageDispatch.ifunc = weak_odr ifunc void (), void ()* ()* @OrderSpecificUsageDispatch.resolver
 // LINUX: @NoSpecifics.ifunc = weak_odr ifunc void (), void ()* ()* @NoSpecifics.resolver
 // LINUX: @HasGeneric.ifunc = weak_odr ifunc void (), void ()* ()* @HasGeneric.resolver
 // LINUX: @HasParams.ifunc = weak_odr ifunc void (i32, double), void (i32, double)* ()* @HasParams.resolver
@@ -34,6 +39,21 @@
 // LINUX: define{{.*}} void @SingleVersion.S() #[[S:[0-9]+]]
 // WINDOWS: define dso_local void @SingleVersion.S() #[[S:[0-9]+]]
 
+ATTR(cpu_dispatch(ivybridge))
+void SingleVersion(void);
+// LINUX: define weak_odr void ()* @SingleVersion.resolver()
+// LINUX: call void @__cpu_indicator_init
+// LINUX: ret void ()* @SingleVersion.S
+// LINUX: call void @llvm.trap
+// LINUX: unreachable
+
+// WINDOWS: define weak_odr dso_local void @SingleVersion() comdat
+// WINDOWS: call void @__cpu_indicator_init()
+// WINDOWS: call void @SingleVersion.S()
+// WINDOWS-NEXT: ret void
+// WINDOWS: call void @llvm.trap
+// WINDOWS: unreachable
+
 ATTR(cpu_specific(ivybridge))
 void NotCalled(void){}
 // LINUX: define{{.*}} void @NotCalled.S() #[[S]]
@@ -80,6 +100,31 @@
 // CHECK: define {{.*}}void @ThreeVersionsSameAttr.S() #[[S]]
 // CHECK: define {{.*}}void @ThreeVersionsSameAttr.Z() #[[K]]
 
+ATTR(cpu_specific(knl))
+void CpuSpecificNoDispatch(void) {}
+// CHECK: define {{.*}}void @CpuSpecificNoDispatch.Z() #[[K:[0-9]+]]
+
+ATTR(cpu_dispatch(knl))
+void OrderDispatchUsageSpecific(void);
+// LINUX: define weak_odr void ()* @OrderDispatchUsageSpecific.resolver()
+// LINUX: call void @__cpu_indicator_init
+// LINUX: ret void ()* @OrderDispatchUsageSpecific.Z
+// LINUX: call void @llvm.trap
+// LINUX: unreachable
+
+// WINDOWS: define weak_odr dso_local void @OrderDispatchUsageSpecific() comdat
+// WINDOWS: call void @__cpu_indicator_init()
+// WINDOWS: call void @OrderDispatchUsageSpecific.Z()
+// WINDOWS-NEXT: ret void
+// WINDOWS: call void @llvm.trap
+// WINDOWS: unreachable
+
+// CHECK: define {{.*}}void @OrderDispatchUsageSpecific.Z()
+
+ATTR(cpu_specific(knl))
+void OrderSpecificUsageDispatch(void) {}
+// CHECK: define {{.*}}void @OrderSpecificUsageDispatch.Z() #[[K:[0-9]+]]
+
 void usages(void) {
   SingleVersion();
   // LINUX: @SingleVersion.ifunc()
@@ -93,8 +138,19 @@
   ThreeVersionsSameAttr();
   // LINUX: @ThreeVersionsSameAttr.ifunc()
   // WINDOWS: @ThreeVersionsSameAttr()
+  CpuSpecificNoDispatch();
+  // LINUX: @CpuSpecificNoDispatch.ifunc()
+  // WINDOWS: @CpuSpecificNoDispatch()
+  OrderDispatchUsageSpecific();
+  

[PATCH] D120266: [clang][CodeGen] Avoid emitting ifuncs with undefined resolvers

[Itay Bookstein via Phabricator via cfe-commits]

ibookstein updated this revision to Diff 411521.
ibookstein added a comment.

Add tests for specific>usage>dispatch and dispatch>usage>specific
orderings.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D120266/new/

https://reviews.llvm.org/D120266

Files:
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/test/CodeGen/attr-cpuspecific.c

Index: clang/test/CodeGen/attr-cpuspecific.c
===
--- clang/test/CodeGen/attr-cpuspecific.c
+++ clang/test/CodeGen/attr-cpuspecific.c
@@ -8,9 +8,12 @@
 #endif // _MSC_VER
 
 // Each version should have an IFunc and an alias.
+// LINUX: @SingleVersion = weak_odr alias void (), void ()* @SingleVersion.ifunc
 // LINUX: @TwoVersions = weak_odr alias void (), void ()* @TwoVersions.ifunc
+// LINUX: @OrderDispatchUsageSpecific = weak_odr alias void (), void ()* @OrderDispatchUsageSpecific.ifunc
 // LINUX: @TwoVersionsSameAttr = weak_odr alias void (), void ()* @TwoVersionsSameAttr.ifunc
 // LINUX: @ThreeVersionsSameAttr = weak_odr alias void (), void ()* @ThreeVersionsSameAttr.ifunc
+// LINUX: @OrderSpecificUsageDispatch = weak_odr alias void (), void ()* @OrderSpecificUsageDispatch.ifunc
 // LINUX: @NoSpecifics = weak_odr alias void (), void ()* @NoSpecifics.ifunc
 // LINUX: @HasGeneric = weak_odr alias void (), void ()* @HasGeneric.ifunc
 // LINUX: @HasParams = weak_odr alias void (i32, double), void (i32, double)* @HasParams.ifunc
@@ -18,10 +21,12 @@
 // LINUX: @GenericAndPentium = weak_odr alias i32 (i32, double), i32 (i32, double)* @GenericAndPentium.ifunc
 // LINUX: @DispatchFirst = weak_odr alias i32 (), i32 ()* @DispatchFirst.ifunc
 
-// LINUX: @TwoVersions.ifunc = weak_odr ifunc void (), void ()* ()* @TwoVersions.resolver
 // LINUX: @SingleVersion.ifunc = weak_odr ifunc void (), void ()* ()* @SingleVersion.resolver
+// LINUX: @TwoVersions.ifunc = weak_odr ifunc void (), void ()* ()* @TwoVersions.resolver
+// LINUX: @OrderDispatchUsageSpecific.ifunc = weak_odr ifunc void (), void ()* ()* @OrderDispatchUsageSpecific.resolver
 // LINUX: @TwoVersionsSameAttr.ifunc = weak_odr ifunc void (), void ()* ()* @TwoVersionsSameAttr.resolver
 // LINUX: @ThreeVersionsSameAttr.ifunc = weak_odr ifunc void (), void ()* ()* @ThreeVersionsSameAttr.resolver
+// LINUX: @OrderSpecificUsageDispatch.ifunc = weak_odr ifunc void (), void ()* ()* @OrderSpecificUsageDispatch.resolver
 // LINUX: @NoSpecifics.ifunc = weak_odr ifunc void (), void ()* ()* @NoSpecifics.resolver
 // LINUX: @HasGeneric.ifunc = weak_odr ifunc void (), void ()* ()* @HasGeneric.resolver
 // LINUX: @HasParams.ifunc = weak_odr ifunc void (i32, double), void (i32, double)* ()* @HasParams.resolver
@@ -34,6 +39,21 @@
 // LINUX: define{{.*}} void @SingleVersion.S() #[[S:[0-9]+]]
 // WINDOWS: define dso_local void @SingleVersion.S() #[[S:[0-9]+]]
 
+ATTR(cpu_dispatch(ivybridge))
+void SingleVersion(void);
+// LINUX: define weak_odr void ()* @SingleVersion.resolver()
+// LINUX: call void @__cpu_indicator_init
+// LINUX: ret void ()* @SingleVersion.S
+// LINUX: call void @llvm.trap
+// LINUX: unreachable
+
+// WINDOWS: define weak_odr dso_local void @SingleVersion() comdat
+// WINDOWS: call void @__cpu_indicator_init()
+// WINDOWS: call void @SingleVersion.S()
+// WINDOWS-NEXT: ret void
+// WINDOWS: call void @llvm.trap
+// WINDOWS: unreachable
+
 ATTR(cpu_specific(ivybridge))
 void NotCalled(void){}
 // LINUX: define{{.*}} void @NotCalled.S() #[[S]]
@@ -80,6 +100,31 @@
 // CHECK: define {{.*}}void @ThreeVersionsSameAttr.S() #[[S]]
 // CHECK: define {{.*}}void @ThreeVersionsSameAttr.Z() #[[K]]
 
+ATTR(cpu_specific(knl))
+void CpuSpecificNoDispatch(void) {}
+// CHECK: define {{.*}}void @CpuSpecificNoDispatch.Z() #[[K:[0-9]+]]
+
+ATTR(cpu_dispatch(knl))
+void OrderDispatchUsageSpecific(void);
+// LINUX: define weak_odr void ()* @OrderDispatchUsageSpecific.resolver()
+// LINUX: call void @__cpu_indicator_init
+// LINUX: ret void ()* @OrderDispatchUsageSpecific.Z
+// LINUX: call void @llvm.trap
+// LINUX: unreachable
+
+// WINDOWS: define weak_odr dso_local void @OrderDispatchUsageSpecific() comdat
+// WINDOWS: call void @__cpu_indicator_init()
+// WINDOWS: call void @OrderDispatchUsageSpecific.Z()
+// WINDOWS-NEXT: ret void
+// WINDOWS: call void @llvm.trap
+// WINDOWS: unreachable
+
+// CHECK: define {{.*}}void @OrderDispatchUsageSpecific.Z()
+
+ATTR(cpu_specific(knl))
+void OrderSpecificUsageDispatch(void) {}
+// CHECK: define {{.*}}void @OrderSpecificUsageDispatch.Z() #[[K:[0-9]+]]
+
 void usages(void) {
   SingleVersion();
   // LINUX: @SingleVersion.ifunc()
@@ -93,8 +138,19 @@
   ThreeVersionsSameAttr();
   // LINUX: @ThreeVersionsSameAttr.ifunc()
   // WINDOWS: @ThreeVersionsSameAttr()
+  CpuSpecificNoDispatch();
+  // LINUX: @CpuSpecificNoDispatch.ifunc()
+  // WINDOWS: @CpuSpecificNoDispatch()
+  OrderDispatchUsageSpecific();
+  // LINUX: @OrderDispatchUsageSpecific.ifunc()
+  // WINDOWS: @OrderDispatchUsageSpecific()
+  Orde

[PATCH] D120266: [clang][CodeGen] Avoid emitting ifuncs with undefined resolvers

[Itay Bookstein via Phabricator via cfe-commits]

ibookstein added a comment.

Ah, I saw your comment just now, good thing I didn't continue down that 
plain-alias-name route then!
The change now satisfies that requirement in a way that binding against the 
alias name indeed would not: TU1 will have the callsite in `caller` bind 
against `foo.ifunc`, which is not a symbol that TU2 defines.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D120266/new/

https://reviews.llvm.org/D120266

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D120266: [clang][CodeGen] Avoid emitting ifuncs with undefined resolvers

[Itay Bookstein via Phabricator via cfe-commits]

ibookstein updated this revision to Diff 411409.
ibookstein edited the summary of this revision.
ibookstein added a comment.

Changed code to generating a function declaration and 'upgrading'
it to an ifunc instead of generating and ifunc and 'downgrading'
it to a function declaration. I decided against changing it to
bind against the unsuffixed alias instead of the ifunc, though,
because that seems to be more involved for little benefit.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D120266/new/

https://reviews.llvm.org/D120266

Files:
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/test/CodeGen/attr-cpuspecific.c

Index: clang/test/CodeGen/attr-cpuspecific.c
===
--- clang/test/CodeGen/attr-cpuspecific.c
+++ clang/test/CodeGen/attr-cpuspecific.c
@@ -8,6 +8,7 @@
 #endif // _MSC_VER
 
 // Each version should have an IFunc and an alias.
+// LINUX: @SingleVersion = weak_odr alias void (), void ()* @SingleVersion.ifunc
 // LINUX: @TwoVersions = weak_odr alias void (), void ()* @TwoVersions.ifunc
 // LINUX: @TwoVersionsSameAttr = weak_odr alias void (), void ()* @TwoVersionsSameAttr.ifunc
 // LINUX: @ThreeVersionsSameAttr = weak_odr alias void (), void ()* @ThreeVersionsSameAttr.ifunc
@@ -18,8 +19,8 @@
 // LINUX: @GenericAndPentium = weak_odr alias i32 (i32, double), i32 (i32, double)* @GenericAndPentium.ifunc
 // LINUX: @DispatchFirst = weak_odr alias i32 (), i32 ()* @DispatchFirst.ifunc
 
-// LINUX: @TwoVersions.ifunc = weak_odr ifunc void (), void ()* ()* @TwoVersions.resolver
 // LINUX: @SingleVersion.ifunc = weak_odr ifunc void (), void ()* ()* @SingleVersion.resolver
+// LINUX: @TwoVersions.ifunc = weak_odr ifunc void (), void ()* ()* @TwoVersions.resolver
 // LINUX: @TwoVersionsSameAttr.ifunc = weak_odr ifunc void (), void ()* ()* @TwoVersionsSameAttr.resolver
 // LINUX: @ThreeVersionsSameAttr.ifunc = weak_odr ifunc void (), void ()* ()* @ThreeVersionsSameAttr.resolver
 // LINUX: @NoSpecifics.ifunc = weak_odr ifunc void (), void ()* ()* @NoSpecifics.resolver
@@ -34,6 +35,21 @@
 // LINUX: define{{.*}} void @SingleVersion.S() #[[S:[0-9]+]]
 // WINDOWS: define dso_local void @SingleVersion.S() #[[S:[0-9]+]]
 
+ATTR(cpu_dispatch(ivybridge))
+void SingleVersion(void);
+// LINUX: define weak_odr void ()* @SingleVersion.resolver()
+// LINUX: call void @__cpu_indicator_init
+// LINUX: ret void ()* @SingleVersion.S
+// LINUX: call void @llvm.trap
+// LINUX: unreachable
+
+// WINDOWS: define weak_odr dso_local void @SingleVersion() comdat
+// WINDOWS: call void @__cpu_indicator_init()
+// WINDOWS: call void @SingleVersion.S()
+// WINDOWS-NEXT: ret void
+// WINDOWS: call void @llvm.trap
+// WINDOWS: unreachable
+
 ATTR(cpu_specific(ivybridge))
 void NotCalled(void){}
 // LINUX: define{{.*}} void @NotCalled.S() #[[S]]
@@ -80,6 +96,10 @@
 // CHECK: define {{.*}}void @ThreeVersionsSameAttr.S() #[[S]]
 // CHECK: define {{.*}}void @ThreeVersionsSameAttr.Z() #[[K]]
 
+ATTR(cpu_specific(knl))
+int CpuSpecificNoDispatch(void) { return 1; }
+// CHECK: define {{.*}}i32 @CpuSpecificNoDispatch.Z() #[[K:[0-9]+]]
+
 void usages(void) {
   SingleVersion();
   // LINUX: @SingleVersion.ifunc()
@@ -93,6 +113,9 @@
   ThreeVersionsSameAttr();
   // LINUX: @ThreeVersionsSameAttr.ifunc()
   // WINDOWS: @ThreeVersionsSameAttr()
+  CpuSpecificNoDispatch();
+  // LINUX: @CpuSpecificNoDispatch.ifunc()
+  // WINDOWS: @CpuSpecificNoDispatch()
 }
 
 // has an extra config to emit!
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -3593,16 +3593,28 @@
   CGF.EmitMultiVersionResolver(ResolverFunc, Options);
 
   if (getTarget().supportsIFunc()) {
+llvm::GlobalValue::LinkageTypes Linkage = getMultiversionLinkage(*this, GD);
+auto *IFunc = cast(
+GetOrCreateMultiVersionResolver(GD, DeclTy, FD));
+
+// Fix up function declarations that were created for cpu_specific before
+// cpu_dispatch was known
+if (!dyn_cast(IFunc)) {
+  assert(cast(IFunc)->isDeclaration());
+  auto *GI = llvm::GlobalIFunc::create(DeclTy, 0, Linkage, "", ResolverFunc,
+   &getModule());
+  GI->takeName(IFunc);
+  IFunc->replaceAllUsesWith(GI);
+  IFunc->eraseFromParent();
+  IFunc = GI;
+}
+
 std::string AliasName = getMangledNameImpl(
 *this, GD, FD, /*OmitMultiVersionMangling=*/true);
 llvm::Constant *AliasFunc = GetGlobalValue(AliasName);
 if (!AliasFunc) {
-  auto *IFunc = cast(GetOrCreateLLVMFunction(
-  AliasName, DeclTy, GD, /*ForVTable=*/false, /*DontDefer=*/true,
-  /*IsThunk=*/false, llvm::AttributeList(), NotForDefinition));
-  auto *GA = llvm::GlobalAlias::create(DeclTy, 0,
-   getMultiversionLinkage(*this, GD),
- 

[PATCH] D120567: [clang][CodeGen] Avoid emitting ifuncs with undefined resolvers

[Itay Bookstein via Phabricator via cfe-commits]

ibookstein created this revision.
ibookstein requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

The purpose of this change is to fix the following codegen bug:

  // main.c
  __attribute__((cpu_specific(generic)))
  int *foo(void) { static int z; return &z;}
  int main() { return *foo() = 5; }
  
  // other.c
  __attribute__((cpu_dispatch(generic))) int *foo(void);
  
  // run:
  clang main.c other.c -o main; ./main

This will segfault prior to the change, and return the correct
exit code 5 after the change.

The underlying cause is that when a translation unit contains
a cpu_specific function without the corresponding cpu_dispatch
the generated code binds the reference to foo() against a
GlobalIFunc whose resolver is undefined. This is invalid: the
resolver must be defined in the same translation unit as the
ifunc, but historically the LLVM bitcode verifier did not check
that. The generated code then binds against the resolver rather
than the ifunc, so it ends up calling the resolver rather than
the resolvee. In the example above it treats its return value as
an int *, therefore trying to write to program text.

The root issue at the representation level is that GlobalIFunc,
like GlobalAlias, does not support a "declaration" state. The
object which provides the correct semantics in these cases
is a Function declaration, but unlike Functions, changing a
declaration to a definition in the GlobalIFunc case constitutes
a change of the object type, as opposed to simply emitting code
into a Function.

I think this limitation is unlikely to change, so I implemented
the fix by returning a function declaration rather than an ifunc
when encountering cpu_specific, and upgrading it to an ifunc
when emitting cpu_dispatch.
This uses `takeName` + `replaceAllUsesWith` in similar vein to
other places where the correct IR object type cannot be known
locally/up-front, like in `CodeGenModule::EmitAliasDefinition`.

Signed-off-by: Itay Bookstein 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D120567

Files:
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/test/CodeGen/attr-cpuspecific.c

Index: clang/test/CodeGen/attr-cpuspecific.c
===
--- clang/test/CodeGen/attr-cpuspecific.c
+++ clang/test/CodeGen/attr-cpuspecific.c
@@ -8,6 +8,7 @@
 #endif // _MSC_VER
 
 // Each version should have an IFunc and an alias.
+// LINUX: @SingleVersion = weak_odr alias void (), void ()* @SingleVersion.ifunc
 // LINUX: @TwoVersions = weak_odr alias void (), void ()* @TwoVersions.ifunc
 // LINUX: @TwoVersionsSameAttr = weak_odr alias void (), void ()* @TwoVersionsSameAttr.ifunc
 // LINUX: @ThreeVersionsSameAttr = weak_odr alias void (), void ()* @ThreeVersionsSameAttr.ifunc
@@ -18,8 +19,8 @@
 // LINUX: @GenericAndPentium = weak_odr alias i32 (i32, double), i32 (i32, double)* @GenericAndPentium.ifunc
 // LINUX: @DispatchFirst = weak_odr alias i32 (), i32 ()* @DispatchFirst.ifunc
 
-// LINUX: @TwoVersions.ifunc = weak_odr ifunc void (), void ()* ()* @TwoVersions.resolver
 // LINUX: @SingleVersion.ifunc = weak_odr ifunc void (), void ()* ()* @SingleVersion.resolver
+// LINUX: @TwoVersions.ifunc = weak_odr ifunc void (), void ()* ()* @TwoVersions.resolver
 // LINUX: @TwoVersionsSameAttr.ifunc = weak_odr ifunc void (), void ()* ()* @TwoVersionsSameAttr.resolver
 // LINUX: @ThreeVersionsSameAttr.ifunc = weak_odr ifunc void (), void ()* ()* @ThreeVersionsSameAttr.resolver
 // LINUX: @NoSpecifics.ifunc = weak_odr ifunc void (), void ()* ()* @NoSpecifics.resolver
@@ -34,6 +35,21 @@
 // LINUX: define{{.*}} void @SingleVersion.S() #[[S:[0-9]+]]
 // WINDOWS: define dso_local void @SingleVersion.S() #[[S:[0-9]+]]
 
+ATTR(cpu_dispatch(ivybridge))
+void SingleVersion(void);
+// LINUX: define weak_odr void ()* @SingleVersion.resolver()
+// LINUX: call void @__cpu_indicator_init
+// LINUX: ret void ()* @SingleVersion.S
+// LINUX: call void @llvm.trap
+// LINUX: unreachable
+
+// WINDOWS: define weak_odr dso_local void @SingleVersion() comdat
+// WINDOWS: call void @__cpu_indicator_init()
+// WINDOWS: call void @SingleVersion.S()
+// WINDOWS-NEXT: ret void
+// WINDOWS: call void @llvm.trap
+// WINDOWS: unreachable
+
 ATTR(cpu_specific(ivybridge))
 void NotCalled(void){}
 // LINUX: define{{.*}} void @NotCalled.S() #[[S]]
@@ -80,6 +96,10 @@
 // CHECK: define {{.*}}void @ThreeVersionsSameAttr.S() #[[S]]
 // CHECK: define {{.*}}void @ThreeVersionsSameAttr.Z() #[[K]]
 
+ATTR(cpu_specific(knl))
+int CpuSpecificNoDispatch(void) { return 1; }
+// CHECK: define {{.*}}i32 @CpuSpecificNoDispatch.Z() #[[K:[0-9]+]]
+
 void usages(void) {
   SingleVersion();
   // LINUX: @SingleVersion.ifunc()
@@ -93,6 +113,9 @@
   ThreeVersionsSameAttr();
   // LINUX: @ThreeVersionsSameAttr.ifunc()
   // WINDOWS: @ThreeVersionsSameAttr()
+  CpuSpecificNoDispatch();
+  // LINUX: @CpuSpecificNoDispatch.ifunc()
+  // WINDOWS: @CpuSpecificNoDispatch()
 }
 
 // has 

[PATCH] D120266: [clang][CodeGen] Avoid emitting ifuncs with undefined resolvers

[Itay Bookstein via Phabricator via cfe-commits]

ibookstein added a comment.

Yeah, that's what happens with this patch; Reference binds against an 
`llvm::Function` declaration, linker resolves it to the actual ifunc in another 
translation unit and therefore emits IFUNC relocation.

Thinking about it more, this is inelegant. I would have liked the reference 
against the `cpu_specific` to bind against a plain "FOO" function declaration 
and not "FOO.ifunc", and 'upgrade' it later once a cpu_dispatch is encountered.
To my understanding, this is actually the reason 
https://reviews.llvm.org/D67058 added the plain-name alias.

I'll try to see if I can rework that.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D120266/new/

https://reviews.llvm.org/D120266

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D120266: [clang][CodeGen] Avoid emitting ifuncs with undefined resolvers

[Itay Bookstein via Phabricator via cfe-commits]

ibookstein updated this revision to Diff 410360.
ibookstein added a comment.

clang-format + description wording


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D120266/new/

https://reviews.llvm.org/D120266

Files:
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/CodeGenModule.h
  clang/test/CodeGen/attr-cpuspecific.c

Index: clang/test/CodeGen/attr-cpuspecific.c
===
--- clang/test/CodeGen/attr-cpuspecific.c
+++ clang/test/CodeGen/attr-cpuspecific.c
@@ -8,6 +8,7 @@
 #endif // _MSC_VER
 
 // Each version should have an IFunc and an alias.
+// LINUX: @SingleVersion = weak_odr alias void (), void ()* @SingleVersion.ifunc
 // LINUX: @TwoVersions = weak_odr alias void (), void ()* @TwoVersions.ifunc
 // LINUX: @TwoVersionsSameAttr = weak_odr alias void (), void ()* @TwoVersionsSameAttr.ifunc
 // LINUX: @ThreeVersionsSameAttr = weak_odr alias void (), void ()* @ThreeVersionsSameAttr.ifunc
@@ -18,8 +19,8 @@
 // LINUX: @GenericAndPentium = weak_odr alias i32 (i32, double), i32 (i32, double)* @GenericAndPentium.ifunc
 // LINUX: @DispatchFirst = weak_odr alias i32 (), i32 ()* @DispatchFirst.ifunc
 
-// LINUX: @TwoVersions.ifunc = weak_odr ifunc void (), void ()* ()* @TwoVersions.resolver
 // LINUX: @SingleVersion.ifunc = weak_odr ifunc void (), void ()* ()* @SingleVersion.resolver
+// LINUX: @TwoVersions.ifunc = weak_odr ifunc void (), void ()* ()* @TwoVersions.resolver
 // LINUX: @TwoVersionsSameAttr.ifunc = weak_odr ifunc void (), void ()* ()* @TwoVersionsSameAttr.resolver
 // LINUX: @ThreeVersionsSameAttr.ifunc = weak_odr ifunc void (), void ()* ()* @ThreeVersionsSameAttr.resolver
 // LINUX: @NoSpecifics.ifunc = weak_odr ifunc void (), void ()* ()* @NoSpecifics.resolver
@@ -34,6 +35,21 @@
 // LINUX: define{{.*}} void @SingleVersion.S() #[[S:[0-9]+]]
 // WINDOWS: define dso_local void @SingleVersion.S() #[[S:[0-9]+]]
 
+ATTR(cpu_dispatch(ivybridge))
+void SingleVersion(void);
+// LINUX: define weak_odr void ()* @SingleVersion.resolver()
+// LINUX: call void @__cpu_indicator_init
+// LINUX: ret void ()* @SingleVersion.S
+// LINUX: call void @llvm.trap
+// LINUX: unreachable
+
+// WINDOWS: define weak_odr dso_local void @SingleVersion() comdat
+// WINDOWS: call void @__cpu_indicator_init()
+// WINDOWS: call void @SingleVersion.S()
+// WINDOWS-NEXT: ret void
+// WINDOWS: call void @llvm.trap
+// WINDOWS: unreachable
+
 ATTR(cpu_specific(ivybridge))
 void NotCalled(void){}
 // LINUX: define{{.*}} void @NotCalled.S() #[[S]]
@@ -80,6 +96,10 @@
 // CHECK: define {{.*}}void @ThreeVersionsSameAttr.S() #[[S]]
 // CHECK: define {{.*}}void @ThreeVersionsSameAttr.Z() #[[K]]
 
+ATTR(cpu_specific(knl))
+int CpuSpecificNoDispatch(void) { return 1; }
+// CHECK: define {{.*}}i32 @CpuSpecificNoDispatch.Z() #[[K:[0-9]+]]
+
 void usages(void) {
   SingleVersion();
   // LINUX: @SingleVersion.ifunc()
@@ -93,6 +113,9 @@
   ThreeVersionsSameAttr();
   // LINUX: @ThreeVersionsSameAttr.ifunc()
   // WINDOWS: @ThreeVersionsSameAttr()
+  CpuSpecificNoDispatch();
+  // LINUX: @CpuSpecificNoDispatch.ifunc()
+  // WINDOWS: @CpuSpecificNoDispatch()
 }
 
 // has an extra config to emit!
Index: clang/lib/CodeGen/CodeGenModule.h
===
--- clang/lib/CodeGen/CodeGenModule.h
+++ clang/lib/CodeGen/CodeGenModule.h
@@ -42,6 +42,7 @@
 class Constant;
 class ConstantInt;
 class Function;
+class GlobalIFunc;
 class GlobalValue;
 class DataLayout;
 class FunctionType;
@@ -352,6 +353,10 @@
   /// we properly emit the iFunc.
   std::vector MultiVersionFuncs;
 
+  /// List of multiversion IFuncs we have emitted. Used to downgrade into
+  /// function declarations when we do not emit a definition for the resolver.
+  std::vector MultiVersionIFuncs;
+
   typedef llvm::StringMap > ReplacementsTy;
   ReplacementsTy Replacements;
 
@@ -1555,6 +1560,10 @@
 
   void emitMultiVersionFunctions();
 
+  /// Replace multiversion IFuncs whose resolver is undefined with function
+  /// declarations.
+  void replaceUndefinedMultiVersionIFuncs();
+
   /// Emit any vtables which we deferred and still have a use for.
   void EmitDeferredVTables();
 
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -508,6 +508,7 @@
   applyReplacements();
   checkAliases();
   emitMultiVersionFunctions();
+  replaceUndefinedMultiVersionIFuncs();
   EmitCXXGlobalInitFunc();
   EmitCXXGlobalCleanUpFunc();
   registerGlobalDtorsWithAtExit();
@@ -3289,6 +3290,22 @@
 EmitGlobalFunctionDefinition(GD, GV);
 }
 
+void CodeGenModule::replaceUndefinedMultiVersionIFuncs() {
+  for (llvm::GlobalIFunc *GIF : MultiVersionIFuncs) {
+llvm::Function *Resolver = GIF->getResolverFunction();
+if (!Resolver->isDeclaration())
+  continue;
+
+auto *DeclTy = cast(GIF->getValueTyp

[PATCH] D120266: [clang][CodeGen] Avoid emitting ifuncs with undefined resolvers

[Itay Bookstein via Phabricator via cfe-commits]

ibookstein created this revision.
ibookstein added reviewers: erichkeane, rsmith, MaskRay.
ibookstein requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

The purpose of this change is to fix the following codegen bug:

// main.c
__attribute__((cpu_specific(generic)))
int *foo(void) { static int z; return &z;}
int main() { return *foo() = 5; }

// other.c
__attribute__((cpu_dispatch(generic))) int *foo(void);

// run:
clang main.c other.c -o main; ./main

This will segfault prior to the change, and return the correct
exit code 5 after the change.

The underlying cause is that when a translation unit contains
a cpu_specific function without the corresponding cpu_dispatch
(which is a valid use-case, they can be put into different
translation units), the generated code binds the reference to
foo() against a GlobalIFunc whose resolver is undefined. This
is invalid (the resolver must be defined in the same translation
unit as the ifunc), but historically the LLVM bitcode verifier
did not check that. The generated code then binds against the
resolver rather than the ifunc, so it ends up calling the
resolver rather than the resolvee. In the example above it treats
its return value as an int *, therefore trying to write to program
text.

The root issue at the representation level is that GlobalIFunc,
like GlobalAlias, does not support a "declaration" state. The
object which provides the correct semantics in these cases
is a Function declaration, but unlike Functions, changing a
declaration to a definition in the GlobalIFunc case constitutes
a change of the object type (as opposed to simply emitting code
into the function).

I think this limitation is unlikely to change, so I implemented
the fix by rewriting the generated IR to use a function
declaration instead of an ifunc if the resolver ends up undefined.
This uses takeName + replaceAllUsesWith in similar vein to
other places where the correct IR object type cannot be known
up front locally, like in CodeGenModule::EmitAliasDefinition.
In this case, we don't know whether the translation unit
will contain the cpu_dispatch when generating code for a reference
bound against a cpu_specific symbol.

It is also possible to generate the reference as a Function
declaration first, and 'upgrade' it to a GlobalIFunc once a
cpu_dispatch is encountered, which is somewhat more 'natural'.
That would involve a larger code change, though, so I wanted to
get feedback on the viability of the approach first.

Signed-off-by: Itay Bookstein 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D120266

Files:
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/CodeGenModule.h
  clang/test/CodeGen/attr-cpuspecific.c

Index: clang/test/CodeGen/attr-cpuspecific.c
===
--- clang/test/CodeGen/attr-cpuspecific.c
+++ clang/test/CodeGen/attr-cpuspecific.c
@@ -8,6 +8,7 @@
 #endif // _MSC_VER
 
 // Each version should have an IFunc and an alias.
+// LINUX: @SingleVersion = weak_odr alias void (), void ()* @SingleVersion.ifunc
 // LINUX: @TwoVersions = weak_odr alias void (), void ()* @TwoVersions.ifunc
 // LINUX: @TwoVersionsSameAttr = weak_odr alias void (), void ()* @TwoVersionsSameAttr.ifunc
 // LINUX: @ThreeVersionsSameAttr = weak_odr alias void (), void ()* @ThreeVersionsSameAttr.ifunc
@@ -18,8 +19,8 @@
 // LINUX: @GenericAndPentium = weak_odr alias i32 (i32, double), i32 (i32, double)* @GenericAndPentium.ifunc
 // LINUX: @DispatchFirst = weak_odr alias i32 (), i32 ()* @DispatchFirst.ifunc
 
-// LINUX: @TwoVersions.ifunc = weak_odr ifunc void (), void ()* ()* @TwoVersions.resolver
 // LINUX: @SingleVersion.ifunc = weak_odr ifunc void (), void ()* ()* @SingleVersion.resolver
+// LINUX: @TwoVersions.ifunc = weak_odr ifunc void (), void ()* ()* @TwoVersions.resolver
 // LINUX: @TwoVersionsSameAttr.ifunc = weak_odr ifunc void (), void ()* ()* @TwoVersionsSameAttr.resolver
 // LINUX: @ThreeVersionsSameAttr.ifunc = weak_odr ifunc void (), void ()* ()* @ThreeVersionsSameAttr.resolver
 // LINUX: @NoSpecifics.ifunc = weak_odr ifunc void (), void ()* ()* @NoSpecifics.resolver
@@ -34,6 +35,21 @@
 // LINUX: define{{.*}} void @SingleVersion.S() #[[S:[0-9]+]]
 // WINDOWS: define dso_local void @SingleVersion.S() #[[S:[0-9]+]]
 
+ATTR(cpu_dispatch(ivybridge))
+void SingleVersion(void);
+// LINUX: define weak_odr void ()* @SingleVersion.resolver()
+// LINUX: call void @__cpu_indicator_init
+// LINUX: ret void ()* @SingleVersion.S
+// LINUX: call void @llvm.trap
+// LINUX: unreachable
+
+// WINDOWS: define weak_odr dso_local void @SingleVersion() comdat
+// WINDOWS: call void @__cpu_indicator_init()
+// WINDOWS: call void @SingleVersion.S()
+// WINDOWS-NEXT: ret void
+// WINDOWS: call void @llvm.trap
+// WINDOWS: unreachable
+
 ATTR(cpu_specific(ivybridge))
 void NotCalled(void){}
 // LINUX: define{{.*}} void @NotCalled.S() #[[S]]
@@ -80,6 +96,10 @@
 // CHECK: define {{.*}

[PATCH] D112349: [Verifier] Add verification logic for GlobalIFuncs

[Itay Bookstein via Phabricator via cfe-commits]

ibookstein added a comment.

I now realize that the type check isn't correct for the platforms which pass 
arguments to the resolver. Unfortunate that the glibc wiki doesn't mention this 
(as far as I can tell)...
I thought that the bitcast-to-"expected"-type should shield from that error, 
but maybe something drops the bitcast along the way. That reminded me of 
https://github.com/llvm/llvm-project/blob/f6ee45e94391ef8cee67e2a4ad6d61c614985de9/llvm/lib/Transforms/IPO/LowerTypeTests.cpp#L388-L391.
In addition, it sounds to me that the resolver type check will be made 
redundant by the opaque pointer work, so maybe it makes sense to remove it 
altogether now? I'm not in the details enough with respect to the migration 
plan to know.
Also, I recall there are some outstanding issues with respect to thinlto+ifunc: 
https://reviews.llvm.org/D82745 which may be of interest to your use-case as 
well.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D112349/new/

https://reviews.llvm.org/D112349

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D118619: [clang][CodeGen][NFC] Remove unused CodeGenModule fields

[Itay Bookstein via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG2a868802a372: [clang][CodeGen][NFC] Remove unused 
CodeGenModule fields (authored by ibookstein).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D118619/new/

https://reviews.llvm.org/D118619

Files:
  clang/lib/CodeGen/CodeGenModule.h


Index: clang/lib/CodeGen/CodeGenModule.h
===
--- clang/lib/CodeGen/CodeGenModule.h
+++ clang/lib/CodeGen/CodeGenModule.h
@@ -394,13 +394,6 @@
   llvm::MapVector MangledDeclNames;
   llvm::StringMap Manglings;
 
-  // An ordered map of canonical GlobalDecls paired with the cpu-index for
-  // cpu-specific name manglings.
-  llvm::MapVector, StringRef>
-  CPUSpecificMangledDeclNames;
-  llvm::StringMap, llvm::BumpPtrAllocator>
-  CPUSpecificManglings;
-
   /// Global annotations.
   std::vector Annotations;
 


Index: clang/lib/CodeGen/CodeGenModule.h
===
--- clang/lib/CodeGen/CodeGenModule.h
+++ clang/lib/CodeGen/CodeGenModule.h
@@ -394,13 +394,6 @@
   llvm::MapVector MangledDeclNames;
   llvm::StringMap Manglings;
 
-  // An ordered map of canonical GlobalDecls paired with the cpu-index for
-  // cpu-specific name manglings.
-  llvm::MapVector, StringRef>
-  CPUSpecificMangledDeclNames;
-  llvm::StringMap, llvm::BumpPtrAllocator>
-  CPUSpecificManglings;
-
   /// Global annotations.
   std::vector Annotations;
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D118619: [clang][CodeGen][NFC] Remove unused CodeGenModule fields

[Itay Bookstein via Phabricator via cfe-commits]

ibookstein added a comment.

Thanks!
CI failures seem to be in completely unrelated libarcher, which are shared by 
other runs in the past few hours; landing this, then.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D118619/new/

https://reviews.llvm.org/D118619

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D118619: [clang][CodeGen][NFC] Remove unused CodeGenModule fields

[Itay Bookstein via Phabricator via cfe-commits]

ibookstein created this revision.
ibookstein added a reviewer: erichkeane.
ibookstein requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Signed-off-by: Itay Bookstein 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D118619

Files:
  clang/lib/CodeGen/CodeGenModule.h


Index: clang/lib/CodeGen/CodeGenModule.h
===
--- clang/lib/CodeGen/CodeGenModule.h
+++ clang/lib/CodeGen/CodeGenModule.h
@@ -394,13 +394,6 @@
   llvm::MapVector MangledDeclNames;
   llvm::StringMap Manglings;
 
-  // An ordered map of canonical GlobalDecls paired with the cpu-index for
-  // cpu-specific name manglings.
-  llvm::MapVector, StringRef>
-  CPUSpecificMangledDeclNames;
-  llvm::StringMap, llvm::BumpPtrAllocator>
-  CPUSpecificManglings;
-
   /// Global annotations.
   std::vector Annotations;
 


Index: clang/lib/CodeGen/CodeGenModule.h
===
--- clang/lib/CodeGen/CodeGenModule.h
+++ clang/lib/CodeGen/CodeGenModule.h
@@ -394,13 +394,6 @@
   llvm::MapVector MangledDeclNames;
   llvm::StringMap Manglings;
 
-  // An ordered map of canonical GlobalDecls paired with the cpu-index for
-  // cpu-specific name manglings.
-  llvm::MapVector, StringRef>
-  CPUSpecificMangledDeclNames;
-  llvm::StringMap, llvm::BumpPtrAllocator>
-  CPUSpecificManglings;
-
   /// Global annotations.
   std::vector Annotations;
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D113352: [clang] Run LLVM Verifier in modes without CodeGen too

[Itay Bookstein via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG9efce0baee4b: [clang] Run LLVM Verifier in modes without 
CodeGen too (authored by ibookstein).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D113352/new/

https://reviews.llvm.org/D113352

Files:
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/test/CodeGen/lto-newpm-pipeline.c


Index: clang/test/CodeGen/lto-newpm-pipeline.c
===
--- clang/test/CodeGen/lto-newpm-pipeline.c
+++ clang/test/CodeGen/lto-newpm-pipeline.c
@@ -32,6 +32,8 @@
 // CHECK-FULL-O0-NEXT: Running pass: CoroCleanupPass
 // CHECK-FULL-O0-NEXT: Running pass: CanonicalizeAliasesPass
 // CHECK-FULL-O0-NEXT: Running pass: NameAnonGlobalPass
+// CHECK-FULL-O0-NEXT: Running pass: VerifierPass
+// CHECK-FULL-O0-NEXT: Running analysis: VerifierAnalysis
 // CHECK-FULL-O0-NEXT: Running pass: BitcodeWriterPass
 
 // CHECK-THIN-O0: Running pass: AlwaysInlinerPass
@@ -40,6 +42,8 @@
 // CHECK-THIN-O0: Running pass: CoroCleanupPass
 // CHECK-THIN-O0-NEXT: Running pass: CanonicalizeAliasesPass
 // CHECK-THIN-O0-NEXT: Running pass: NameAnonGlobalPass
+// CHECK-THIN-O0-NEXT: Running pass: VerifierPass
+// CHECK-THIN-O0-NEXT: Running analysis: VerifierAnalysis
 // CHECK-THIN-O0-NEXT: Running pass: ThinLTOBitcodeWriterPass
 
 // TODO: The LTO pre-link pipeline currently invokes
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -5034,7 +5034,7 @@
   llvm::Type *DeclTy = getTypes().ConvertTypeForMem(D->getType());
   llvm::Type *ResolverTy = llvm::GlobalIFunc::getResolverFunctionType(DeclTy);
   llvm::Constant *Resolver =
-  GetOrCreateLLVMFunction(IFA->getResolver(), ResolverTy, GD,
+  GetOrCreateLLVMFunction(IFA->getResolver(), ResolverTy, {},
   /*ForVTable=*/false);
   llvm::GlobalIFunc *GIF =
   llvm::GlobalIFunc::create(DeclTy, 0, llvm::Function::ExternalLinkage,
Index: clang/lib/CodeGen/BackendUtil.cpp
===
--- clang/lib/CodeGen/BackendUtil.cpp
+++ clang/lib/CodeGen/BackendUtil.cpp
@@ -487,6 +487,11 @@
   }
 }
 
+static bool actionRequiresCodeGen(BackendAction Action) {
+  return Action != Backend_EmitNothing && Action != Backend_EmitBC &&
+ Action != Backend_EmitLL;
+}
+
 static bool initTargetOptions(DiagnosticsEngine &Diags,
   llvm::TargetOptions &Options,
   const CodeGenOptions &CodeGenOpts,
@@ -977,9 +982,7 @@
 
   setCommandLineOpts(CodeGenOpts);
 
-  bool UsesCodeGen = (Action != Backend_EmitNothing &&
-  Action != Backend_EmitBC &&
-  Action != Backend_EmitLL);
+  bool UsesCodeGen = actionRequiresCodeGen(Action);
   CreateTargetMachine(UsesCodeGen);
 
   if (UsesCodeGen && !TM)
@@ -1006,6 +1009,12 @@
 
   CreatePasses(PerModulePasses, PerFunctionPasses);
 
+  // Add a verifier pass if requested. We don't have to do this if the action
+  // requires code generation because there will already be a verifier pass in
+  // the code-generation pipeline.
+  if (!UsesCodeGen && CodeGenOpts.VerifyModule)
+PerModulePasses.add(createVerifierPass());
+
   legacy::PassManager CodeGenPasses;
   CodeGenPasses.add(
   createTargetTransformInfoWrapperPass(getTargetIRAnalysis()));
@@ -1425,6 +1434,13 @@
   MPM.addPass(ModuleMemProfilerPass());
 }
   }
+
+  // Add a verifier pass if requested. We don't have to do this if the action
+  // requires code generation because there will already be a verifier pass in
+  // the code-generation pipeline.
+  if (!actionRequiresCodeGen(Action) && CodeGenOpts.VerifyModule)
+MPM.addPass(VerifierPass());
+
   switch (Action) {
   case Backend_EmitBC:
 if (CodeGenOpts.PrepareForThinLTO && !CodeGenOpts.DisableLLVMPasses) {
@@ -1514,8 +1530,7 @@
   TimeRegion Region(CodeGenOpts.TimePasses ? &CodeGenerationTime : nullptr);
   setCommandLineOpts(CodeGenOpts);
 
-  bool RequiresCodeGen = (Action != Backend_EmitNothing &&
-  Action != Backend_EmitBC && Action != 
Backend_EmitLL);
+  bool RequiresCodeGen = actionRequiresCodeGen(Action);
   CreateTargetMachine(RequiresCodeGen);
 
   if (RequiresCodeGen && !TM)


Index: clang/test/CodeGen/lto-newpm-pipeline.c
===
--- clang/test/CodeGen/lto-newpm-pipeline.c
+++ clang/test/CodeGen/lto-newpm-pipeline.c
@@ -32,6 +32,8 @@
 // CHECK-FULL-O0-NEXT: Running pass: CoroCleanupPass
 // CHECK-FULL-O0-NEXT: Running pass: CanonicalizeAliasesPass
 // CHECK-FULL-O0-NEXT: Running pass: NameAnonGlobalPass
+// CHECK-FULL-O0-NEXT: Running pass: VerifierPass
+// CHECK-FULL-O0-NEXT: Running analysis: VerifierAnalysis
 // 

[PATCH] D112868: [CodeGen] Diagnose and reject non-function ifunc resolvers

[Itay Bookstein via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG3b1fd19357be: [CodeGen] Diagnose and reject non-function 
ifunc resolvers (authored by ibookstein).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D112868/new/

https://reviews.llvm.org/D112868

Files:
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/test/CodeGen/attr-ifunc.c

Index: clang/test/CodeGen/attr-ifunc.c
===
--- clang/test/CodeGen/attr-ifunc.c
+++ clang/test/CodeGen/attr-ifunc.c
@@ -13,8 +13,7 @@
 void f1() __attribute__((ifunc("f1_ifunc")));
 // expected-error@-1 {{ifunc must point to a defined function}}
 
-void *f2_a() __attribute__((ifunc("f2_b")));
-// expected-error@-1 {{ifunc definition is part of a cycle}}
+void *f2_a() __attribute__((alias("f2_b")));
 void *f2_b() __attribute__((ifunc("f2_a")));
 // expected-error@-1 {{ifunc definition is part of a cycle}}
 
@@ -27,6 +26,15 @@
 void f4() __attribute__((ifunc("f4_ifunc")));
 // expected-error@-1 {{ifunc resolver function must return a pointer}}
 
+int f5_resolver_gvar;
+void f5() __attribute__((ifunc("f5_resolver_gvar")));
+// expected-error@-1 {{ifunc must point to a defined function}}
+
+void *f6_resolver_resolver() { return 0; }
+void *f6_resolver() __attribute__((ifunc("f6_resolver_resolver")));
+void f6() __attribute__((ifunc("f6_resolver")));
+// expected-error@-1 {{ifunc must point to a defined function}}
+
 #else
 void f1a() __asm("f1");
 void f1a() {}
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -313,21 +313,57 @@
 // This is only used in aliases that we created and we know they have a
 // linear structure.
 static const llvm::GlobalValue *getAliasedGlobal(const llvm::GlobalValue *GV) {
-  llvm::SmallPtrSet Visited;
-  for (;;) {
-if (!GV || !Visited.insert(GV).second)
-  return nullptr;
-
-const llvm::Constant *C;
-if (auto *GA = dyn_cast(GV))
-  C = GA->getAliasee();
-else if (auto *GI = dyn_cast(GV))
-  C = GI->getResolver();
-else
-  return GV;
+  const llvm::Constant *C;
+  if (auto *GA = dyn_cast(GV))
+C = GA->getAliasee();
+  else if (auto *GI = dyn_cast(GV))
+C = GI->getResolver();
+  else
+return GV;
+
+  const auto *AliaseeGV = dyn_cast(C->stripPointerCasts());
+  if (!AliaseeGV)
+return nullptr;
+
+  const llvm::GlobalValue *FinalGV = AliaseeGV->getAliaseeObject();
+  if (FinalGV == GV)
+return nullptr;
+
+  return FinalGV;
+}
+
+static bool checkAliasedGlobal(DiagnosticsEngine &Diags,
+   SourceLocation Location, bool IsIFunc,
+   const llvm::GlobalValue *Alias,
+   const llvm::GlobalValue *&GV) {
+  GV = getAliasedGlobal(Alias);
+  if (!GV) {
+Diags.Report(Location, diag::err_cyclic_alias) << IsIFunc;
+return false;
+  }
+
+  if (GV->isDeclaration()) {
+Diags.Report(Location, diag::err_alias_to_undefined) << IsIFunc << IsIFunc;
+return false;
+  }
+
+  if (IsIFunc) {
+// Check resolver function type.
+const auto *F = dyn_cast(GV);
+if (!F) {
+  Diags.Report(Location, diag::err_alias_to_undefined)
+  << IsIFunc << IsIFunc;
+  return false;
+}
 
-GV = dyn_cast(C->stripPointerCasts());
+llvm::FunctionType *FTy = F->getFunctionType();
+if (!FTy->getReturnType()->isPointerTy()) {
+  Diags.Report(Location, diag::err_ifunc_resolver_return);
+  return false;
+}
   }
+
+  return true;
 }
 
 void CodeGenModule::checkAliases() {
@@ -344,23 +380,13 @@
   Location = A->getLocation();
 else
   llvm_unreachable("Not an alias or ifunc?");
+
 StringRef MangledName = getMangledName(GD);
 llvm::GlobalValue *Alias = GetGlobalValue(MangledName);
-const llvm::GlobalValue *GV = getAliasedGlobal(Alias);
-if (!GV) {
-  Error = true;
-  Diags.Report(Location, diag::err_cyclic_alias) << IsIFunc;
-} else if (GV->isDeclaration()) {
+const llvm::GlobalValue *GV = nullptr;
+if (!checkAliasedGlobal(Diags, Location, IsIFunc, Alias, GV)) {
   Error = true;
-  Diags.Report(Location, diag::err_alias_to_undefined)
-  << IsIFunc << IsIFunc;
-} else if (IsIFunc) {
-  // Check resolver function type.
-  llvm::FunctionType *FTy = dyn_cast(
-  GV->getType()->getPointerElementType());
-  assert(FTy);
-  if (!FTy->getReturnType()->isPointerTy())
-Diags.Report(Location, diag::err_ifunc_resolver_return);
+  continue;
 }
 
 llvm::Constant *Aliasee =
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D113431: [clang][test][NFC] Move attr-ifunc.c test from Sema to CodeGen

[Itay Bookstein via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGce91540beeff: [clang][test][NFC] Move attr-ifunc.c test from 
Sema to CodeGen (authored by ibookstein).

Changed prior to commit:
  https://reviews.llvm.org/D113431?vs=385923&id=385966#toc

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D113431/new/

https://reviews.llvm.org/D113431

Files:
  clang/test/CodeGen/attr-ifunc.c
  clang/test/Sema/attr-ifunc.c




___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D113504: [clang][test][NFC] clang-format attr-ifunc.c test

[Itay Bookstein via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG4f04f7d816f3: [clang][test][NFC] clang-format attr-ifunc.c 
test (authored by ibookstein).

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D113504/new/

https://reviews.llvm.org/D113504

Files:
  clang/test/Sema/attr-ifunc.c


Index: clang/test/Sema/attr-ifunc.c
===
--- clang/test/Sema/attr-ifunc.c
+++ clang/test/Sema/attr-ifunc.c
@@ -5,39 +5,39 @@
 #if defined(_WIN32)
 void foo() {}
 void bar() __attribute__((ifunc("foo")));
-//expected-warning@-1 {{unknown attribute 'ifunc' ignored}}
+// expected-warning@-1 {{unknown attribute 'ifunc' ignored}}
 
 #else
 #if defined(CHECK_ALIASES)
-void* f1_ifunc();
+void *f1_ifunc();
 void f1() __attribute__((ifunc("f1_ifunc")));
-//expected-error@-1 {{ifunc must point to a defined function}}
+// expected-error@-1 {{ifunc must point to a defined function}}
 
-void* f2_a() __attribute__((ifunc("f2_b")));
-//expected-error@-1 {{ifunc definition is part of a cycle}}
-void* f2_b() __attribute__((ifunc("f2_a")));
-//expected-error@-1 {{ifunc definition is part of a cycle}}
+void *f2_a() __attribute__((ifunc("f2_b")));
+// expected-error@-1 {{ifunc definition is part of a cycle}}
+void *f2_b() __attribute__((ifunc("f2_a")));
+// expected-error@-1 {{ifunc definition is part of a cycle}}
 
-void* f3_a() __attribute__((ifunc("f3_b")));
-//expected-warning@-1 {{ifunc will always resolve to f3_c even if weak 
definition of f3_b is overridden}}
-void* f3_b() __attribute__((weak, alias("f3_c")));
-void* f3_c() { return 0; }
+void *f3_a() __attribute__((ifunc("f3_b")));
+// expected-warning@-1 {{ifunc will always resolve to f3_c even if weak 
definition of f3_b is overridden}}
+void *f3_b() __attribute__((weak, alias("f3_c")));
+void *f3_c() { return 0; }
 
 void f4_ifunc() {}
 void f4() __attribute__((ifunc("f4_ifunc")));
-//expected-error@-1 {{ifunc resolver function must return a pointer}}
+// expected-error@-1 {{ifunc resolver function must return a pointer}}
 
 #else
 void f1a() __asm("f1");
 void f1a() {}
-//expected-note@-1 {{previous definition is here}}
+// expected-note@-1 {{previous definition is here}}
 void f1() __attribute__((ifunc("f1_ifunc")));
-//expected-error@-1 {{definition with same mangled name 'f1' as another 
definition}}
-void* f1_ifunc() { return 0; }
+// expected-error@-1 {{definition with same mangled name 'f1' as another 
definition}}
+void *f1_ifunc() { return 0; }
 
-void* f6_ifunc(int i);
+void *f6_ifunc(int i);
 void __attribute__((ifunc("f6_ifunc"))) f6() {}
-//expected-error@-1 {{definition 'f6' cannot also be an ifunc}}
+// expected-error@-1 {{definition 'f6' cannot also be an ifunc}}
 
 #endif
 #endif


Index: clang/test/Sema/attr-ifunc.c
===
--- clang/test/Sema/attr-ifunc.c
+++ clang/test/Sema/attr-ifunc.c
@@ -5,39 +5,39 @@
 #if defined(_WIN32)
 void foo() {}
 void bar() __attribute__((ifunc("foo")));
-//expected-warning@-1 {{unknown attribute 'ifunc' ignored}}
+// expected-warning@-1 {{unknown attribute 'ifunc' ignored}}
 
 #else
 #if defined(CHECK_ALIASES)
-void* f1_ifunc();
+void *f1_ifunc();
 void f1() __attribute__((ifunc("f1_ifunc")));
-//expected-error@-1 {{ifunc must point to a defined function}}
+// expected-error@-1 {{ifunc must point to a defined function}}
 
-void* f2_a() __attribute__((ifunc("f2_b")));
-//expected-error@-1 {{ifunc definition is part of a cycle}}
-void* f2_b() __attribute__((ifunc("f2_a")));
-//expected-error@-1 {{ifunc definition is part of a cycle}}
+void *f2_a() __attribute__((ifunc("f2_b")));
+// expected-error@-1 {{ifunc definition is part of a cycle}}
+void *f2_b() __attribute__((ifunc("f2_a")));
+// expected-error@-1 {{ifunc definition is part of a cycle}}
 
-void* f3_a() __attribute__((ifunc("f3_b")));
-//expected-warning@-1 {{ifunc will always resolve to f3_c even if weak definition of f3_b is overridden}}
-void* f3_b() __attribute__((weak, alias("f3_c")));
-void* f3_c() { return 0; }
+void *f3_a() __attribute__((ifunc("f3_b")));
+// expected-warning@-1 {{ifunc will always resolve to f3_c even if weak definition of f3_b is overridden}}
+void *f3_b() __attribute__((weak, alias("f3_c")));
+void *f3_c() { return 0; }
 
 void f4_ifunc() {}
 void f4() __attribute__((ifunc("f4_ifunc")));
-//expected-error@-1 {{ifunc resolver function must return a pointer}}
+// expected-error@-1 {{ifunc resolver function must return a pointer}}
 
 #else
 void f1a() __asm("f1");
 void f1a() {}
-//expected-note@-1 {{previous definition is here}}
+// expected-note@-1 {{previous definition is here}}
 void f1() __attribute__((ifunc("f1_ifunc")));
-//expected-error@-1 {{definition with same mangled name 'f1' as another definition}}
-void* f1_ifunc() { return 0; }
+// expected-error@-1 {{definition with same mangled name 'f1' as another definition}}
+void *f1_ifunc() { return 0

[PATCH] D113431: [clang][test][NFC] Move attr-ifunc.c test from Sema to CodeGen

[Itay Bookstein via Phabricator via cfe-commits]

ibookstein added a comment.

Right; well, thanks!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D113431/new/

https://reviews.llvm.org/D113431

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D112868: [CodeGen] Diagnose and reject non-function ifunc resolvers

[Itay Bookstein via Phabricator via cfe-commits]

ibookstein added a comment.

Yeah, it's real ugly from a CFE/LLVM separation POV. I also can't avoid seeing 
it as a duplication of the verifier's logic. But emitting diagnoses is better 
than asserting/crashing...
For the recursion, maybe an equivalent of `getAliaseeObject` on `GlobalDecl`-s 
could work? For the rest - not steeped in the code enough to say...


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D112868/new/

https://reviews.llvm.org/D112868

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D113431: [clang][test][NFC] Move attr-ifunc.c test from Sema to CodeGen

[Itay Bookstein via Phabricator via cfe-commits]

ibookstein added a comment.

If you meant fusing the clang-format commit with this one, doing it in the same 
commit results in git no longer detecting the connection between them 
(similarity too low), so it loses the history. When searching I found a 
recommendation  to avoid combining 
renames with modifications where git history matters, so I thought I might as 
well :)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D113431/new/

https://reviews.llvm.org/D113431

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D112868: [CodeGen] Diagnose and reject non-function ifunc resolvers

[Itay Bookstein via Phabricator via cfe-commits]

ibookstein updated this revision to Diff 385926.
ibookstein added a comment.

rebase fix after adding parent NFC commits


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D112868/new/

https://reviews.llvm.org/D112868

Files:
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/test/CodeGen/attr-ifunc.c

Index: clang/test/CodeGen/attr-ifunc.c
===
--- clang/test/CodeGen/attr-ifunc.c
+++ clang/test/CodeGen/attr-ifunc.c
@@ -13,8 +13,7 @@
 void f1() __attribute__((ifunc("f1_ifunc")));
 // expected-error@-1 {{ifunc must point to a defined function}}
 
-void *f2_a() __attribute__((ifunc("f2_b")));
-// expected-error@-1 {{ifunc definition is part of a cycle}}
+void *f2_a() __attribute__((alias("f2_b")));
 void *f2_b() __attribute__((ifunc("f2_a")));
 // expected-error@-1 {{ifunc definition is part of a cycle}}
 
@@ -27,6 +26,15 @@
 void f4() __attribute__((ifunc("f4_ifunc")));
 // expected-error@-1 {{ifunc resolver function must return a pointer}}
 
+int f5_resolver_gvar;
+void f5() __attribute__((ifunc("f5_resolver_gvar")));
+// expected-error@-1 {{ifunc must point to a defined function}}
+
+void *f6_resolver_resolver() { return 0; }
+void *f6_resolver() __attribute__((ifunc("f6_resolver_resolver")));
+void f6() __attribute__((ifunc("f6_resolver")));
+// expected-error@-1 {{ifunc must point to a defined function}}
+
 #else
 void f1a() __asm("f1");
 void f1a() {}
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -318,21 +318,57 @@
 // This is only used in aliases that we created and we know they have a
 // linear structure.
 static const llvm::GlobalValue *getAliasedGlobal(const llvm::GlobalValue *GV) {
-  llvm::SmallPtrSet Visited;
-  for (;;) {
-if (!GV || !Visited.insert(GV).second)
-  return nullptr;
-
-const llvm::Constant *C;
-if (auto *GA = dyn_cast(GV))
-  C = GA->getAliasee();
-else if (auto *GI = dyn_cast(GV))
-  C = GI->getResolver();
-else
-  return GV;
+  const llvm::Constant *C;
+  if (auto *GA = dyn_cast(GV))
+C = GA->getAliasee();
+  else if (auto *GI = dyn_cast(GV))
+C = GI->getResolver();
+  else
+return GV;
+
+  const auto *AliaseeGV = dyn_cast(C->stripPointerCasts());
+  if (!AliaseeGV)
+return nullptr;
+
+  const llvm::GlobalValue *FinalGV = AliaseeGV->getAliaseeObject();
+  if (FinalGV == GV)
+return nullptr;
+
+  return FinalGV;
+}
+
+static bool checkAliasedGlobal(DiagnosticsEngine &Diags,
+   SourceLocation Location, bool IsIFunc,
+   const llvm::GlobalValue *Alias,
+   const llvm::GlobalValue *&GV) {
+  GV = getAliasedGlobal(Alias);
+  if (!GV) {
+Diags.Report(Location, diag::err_cyclic_alias) << IsIFunc;
+return false;
+  }
+
+  if (GV->isDeclaration()) {
+Diags.Report(Location, diag::err_alias_to_undefined) << IsIFunc << IsIFunc;
+return false;
+  }
+
+  if (IsIFunc) {
+// Check resolver function type.
+const auto *F = dyn_cast(GV);
+if (!F) {
+  Diags.Report(Location, diag::err_alias_to_undefined)
+  << IsIFunc << IsIFunc;
+  return false;
+}
 
-GV = dyn_cast(C->stripPointerCasts());
+llvm::FunctionType *FTy = F->getFunctionType();
+if (!FTy->getReturnType()->isPointerTy()) {
+  Diags.Report(Location, diag::err_ifunc_resolver_return);
+  return false;
+}
   }
+
+  return true;
 }
 
 void CodeGenModule::checkAliases() {
@@ -349,23 +385,13 @@
   Location = A->getLocation();
 else
   llvm_unreachable("Not an alias or ifunc?");
+
 StringRef MangledName = getMangledName(GD);
 llvm::GlobalValue *Alias = GetGlobalValue(MangledName);
-const llvm::GlobalValue *GV = getAliasedGlobal(Alias);
-if (!GV) {
-  Error = true;
-  Diags.Report(Location, diag::err_cyclic_alias) << IsIFunc;
-} else if (GV->isDeclaration()) {
+const llvm::GlobalValue *GV = nullptr;
+if (!checkAliasedGlobal(Diags, Location, IsIFunc, Alias, GV)) {
   Error = true;
-  Diags.Report(Location, diag::err_alias_to_undefined)
-  << IsIFunc << IsIFunc;
-} else if (IsIFunc) {
-  // Check resolver function type.
-  llvm::FunctionType *FTy = dyn_cast(
-  GV->getType()->getPointerElementType());
-  assert(FTy);
-  if (!FTy->getReturnType()->isPointerTy())
-Diags.Report(Location, diag::err_ifunc_resolver_return);
+  continue;
 }
 
 llvm::Constant *Aliasee =
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D113431: [clang][test][NFC] Move attr-ifunc.c test from Sema to CodeGen

[Itay Bookstein via Phabricator via cfe-commits]

ibookstein updated this revision to Diff 385923.
ibookstein added a comment.

add parent clang-format commit to pass CI


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D113431/new/

https://reviews.llvm.org/D113431

Files:
  clang/test/CodeGen/attr-ifunc.c
  clang/test/Sema/attr-ifunc.c


Index: clang/test/Sema/attr-ifunc.c
===
--- /dev/null
+++ clang/test/Sema/attr-ifunc.c
@@ -1,43 +0,0 @@
-// RUN: %clang_cc1 -triple x86_64-windows -fsyntax-only -verify %s
-// RUN: %clang_cc1 -triple x86_64-linux -fsyntax-only -verify -emit-llvm-only 
-DCHECK_ALIASES %s
-// RUN: %clang_cc1 -triple x86_64-linux -fsyntax-only -verify -emit-llvm-only 
%s
-
-#if defined(_WIN32)
-void foo() {}
-void bar() __attribute__((ifunc("foo")));
-// expected-warning@-1 {{unknown attribute 'ifunc' ignored}}
-
-#else
-#if defined(CHECK_ALIASES)
-void *f1_ifunc();
-void f1() __attribute__((ifunc("f1_ifunc")));
-// expected-error@-1 {{ifunc must point to a defined function}}
-
-void *f2_a() __attribute__((ifunc("f2_b")));
-// expected-error@-1 {{ifunc definition is part of a cycle}}
-void *f2_b() __attribute__((ifunc("f2_a")));
-// expected-error@-1 {{ifunc definition is part of a cycle}}
-
-void *f3_a() __attribute__((ifunc("f3_b")));
-// expected-warning@-1 {{ifunc will always resolve to f3_c even if weak 
definition of f3_b is overridden}}
-void *f3_b() __attribute__((weak, alias("f3_c")));
-void *f3_c() { return 0; }
-
-void f4_ifunc() {}
-void f4() __attribute__((ifunc("f4_ifunc")));
-// expected-error@-1 {{ifunc resolver function must return a pointer}}
-
-#else
-void f1a() __asm("f1");
-void f1a() {}
-// expected-note@-1 {{previous definition is here}}
-void f1() __attribute__((ifunc("f1_ifunc")));
-// expected-error@-1 {{definition with same mangled name 'f1' as another 
definition}}
-void *f1_ifunc() { return 0; }
-
-void *f6_ifunc(int i);
-void __attribute__((ifunc("f6_ifunc"))) f6() {}
-// expected-error@-1 {{definition 'f6' cannot also be an ifunc}}
-
-#endif
-#endif


Index: clang/test/Sema/attr-ifunc.c
===
--- /dev/null
+++ clang/test/Sema/attr-ifunc.c
@@ -1,43 +0,0 @@
-// RUN: %clang_cc1 -triple x86_64-windows -fsyntax-only -verify %s
-// RUN: %clang_cc1 -triple x86_64-linux -fsyntax-only -verify -emit-llvm-only -DCHECK_ALIASES %s
-// RUN: %clang_cc1 -triple x86_64-linux -fsyntax-only -verify -emit-llvm-only %s
-
-#if defined(_WIN32)
-void foo() {}
-void bar() __attribute__((ifunc("foo")));
-// expected-warning@-1 {{unknown attribute 'ifunc' ignored}}
-
-#else
-#if defined(CHECK_ALIASES)
-void *f1_ifunc();
-void f1() __attribute__((ifunc("f1_ifunc")));
-// expected-error@-1 {{ifunc must point to a defined function}}
-
-void *f2_a() __attribute__((ifunc("f2_b")));
-// expected-error@-1 {{ifunc definition is part of a cycle}}
-void *f2_b() __attribute__((ifunc("f2_a")));
-// expected-error@-1 {{ifunc definition is part of a cycle}}
-
-void *f3_a() __attribute__((ifunc("f3_b")));
-// expected-warning@-1 {{ifunc will always resolve to f3_c even if weak definition of f3_b is overridden}}
-void *f3_b() __attribute__((weak, alias("f3_c")));
-void *f3_c() { return 0; }
-
-void f4_ifunc() {}
-void f4() __attribute__((ifunc("f4_ifunc")));
-// expected-error@-1 {{ifunc resolver function must return a pointer}}
-
-#else
-void f1a() __asm("f1");
-void f1a() {}
-// expected-note@-1 {{previous definition is here}}
-void f1() __attribute__((ifunc("f1_ifunc")));
-// expected-error@-1 {{definition with same mangled name 'f1' as another definition}}
-void *f1_ifunc() { return 0; }
-
-void *f6_ifunc(int i);
-void __attribute__((ifunc("f6_ifunc"))) f6() {}
-// expected-error@-1 {{definition 'f6' cannot also be an ifunc}}
-
-#endif
-#endif
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D113504: [clang][test][NFC] clang-format attr-ifunc.c test

[Itay Bookstein via Phabricator via cfe-commits]

ibookstein created this revision.
ibookstein added reviewers: erichkeane, aaron.ballman.
ibookstein requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Signed-off-by: Itay Bookstein 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D113504

Files:
  clang/test/Sema/attr-ifunc.c


Index: clang/test/Sema/attr-ifunc.c
===
--- clang/test/Sema/attr-ifunc.c
+++ clang/test/Sema/attr-ifunc.c
@@ -5,39 +5,39 @@
 #if defined(_WIN32)
 void foo() {}
 void bar() __attribute__((ifunc("foo")));
-//expected-warning@-1 {{unknown attribute 'ifunc' ignored}}
+// expected-warning@-1 {{unknown attribute 'ifunc' ignored}}
 
 #else
 #if defined(CHECK_ALIASES)
-void* f1_ifunc();
+void *f1_ifunc();
 void f1() __attribute__((ifunc("f1_ifunc")));
-//expected-error@-1 {{ifunc must point to a defined function}}
+// expected-error@-1 {{ifunc must point to a defined function}}
 
-void* f2_a() __attribute__((ifunc("f2_b")));
-//expected-error@-1 {{ifunc definition is part of a cycle}}
-void* f2_b() __attribute__((ifunc("f2_a")));
-//expected-error@-1 {{ifunc definition is part of a cycle}}
+void *f2_a() __attribute__((ifunc("f2_b")));
+// expected-error@-1 {{ifunc definition is part of a cycle}}
+void *f2_b() __attribute__((ifunc("f2_a")));
+// expected-error@-1 {{ifunc definition is part of a cycle}}
 
-void* f3_a() __attribute__((ifunc("f3_b")));
-//expected-warning@-1 {{ifunc will always resolve to f3_c even if weak 
definition of f3_b is overridden}}
-void* f3_b() __attribute__((weak, alias("f3_c")));
-void* f3_c() { return 0; }
+void *f3_a() __attribute__((ifunc("f3_b")));
+// expected-warning@-1 {{ifunc will always resolve to f3_c even if weak 
definition of f3_b is overridden}}
+void *f3_b() __attribute__((weak, alias("f3_c")));
+void *f3_c() { return 0; }
 
 void f4_ifunc() {}
 void f4() __attribute__((ifunc("f4_ifunc")));
-//expected-error@-1 {{ifunc resolver function must return a pointer}}
+// expected-error@-1 {{ifunc resolver function must return a pointer}}
 
 #else
 void f1a() __asm("f1");
 void f1a() {}
-//expected-note@-1 {{previous definition is here}}
+// expected-note@-1 {{previous definition is here}}
 void f1() __attribute__((ifunc("f1_ifunc")));
-//expected-error@-1 {{definition with same mangled name 'f1' as another 
definition}}
-void* f1_ifunc() { return 0; }
+// expected-error@-1 {{definition with same mangled name 'f1' as another 
definition}}
+void *f1_ifunc() { return 0; }
 
-void* f6_ifunc(int i);
+void *f6_ifunc(int i);
 void __attribute__((ifunc("f6_ifunc"))) f6() {}
-//expected-error@-1 {{definition 'f6' cannot also be an ifunc}}
+// expected-error@-1 {{definition 'f6' cannot also be an ifunc}}
 
 #endif
 #endif


Index: clang/test/Sema/attr-ifunc.c
===
--- clang/test/Sema/attr-ifunc.c
+++ clang/test/Sema/attr-ifunc.c
@@ -5,39 +5,39 @@
 #if defined(_WIN32)
 void foo() {}
 void bar() __attribute__((ifunc("foo")));
-//expected-warning@-1 {{unknown attribute 'ifunc' ignored}}
+// expected-warning@-1 {{unknown attribute 'ifunc' ignored}}
 
 #else
 #if defined(CHECK_ALIASES)
-void* f1_ifunc();
+void *f1_ifunc();
 void f1() __attribute__((ifunc("f1_ifunc")));
-//expected-error@-1 {{ifunc must point to a defined function}}
+// expected-error@-1 {{ifunc must point to a defined function}}
 
-void* f2_a() __attribute__((ifunc("f2_b")));
-//expected-error@-1 {{ifunc definition is part of a cycle}}
-void* f2_b() __attribute__((ifunc("f2_a")));
-//expected-error@-1 {{ifunc definition is part of a cycle}}
+void *f2_a() __attribute__((ifunc("f2_b")));
+// expected-error@-1 {{ifunc definition is part of a cycle}}
+void *f2_b() __attribute__((ifunc("f2_a")));
+// expected-error@-1 {{ifunc definition is part of a cycle}}
 
-void* f3_a() __attribute__((ifunc("f3_b")));
-//expected-warning@-1 {{ifunc will always resolve to f3_c even if weak definition of f3_b is overridden}}
-void* f3_b() __attribute__((weak, alias("f3_c")));
-void* f3_c() { return 0; }
+void *f3_a() __attribute__((ifunc("f3_b")));
+// expected-warning@-1 {{ifunc will always resolve to f3_c even if weak definition of f3_b is overridden}}
+void *f3_b() __attribute__((weak, alias("f3_c")));
+void *f3_c() { return 0; }
 
 void f4_ifunc() {}
 void f4() __attribute__((ifunc("f4_ifunc")));
-//expected-error@-1 {{ifunc resolver function must return a pointer}}
+// expected-error@-1 {{ifunc resolver function must return a pointer}}
 
 #else
 void f1a() __asm("f1");
 void f1a() {}
-//expected-note@-1 {{previous definition is here}}
+// expected-note@-1 {{previous definition is here}}
 void f1() __attribute__((ifunc("f1_ifunc")));
-//expected-error@-1 {{definition with same mangled name 'f1' as another definition}}
-void* f1_ifunc() { return 0; }
+// expected-error@-1 {{definition with same mangled name 'f1' as another definition}}
+void *f1_ifunc() { return 0; }
 
-void*

[PATCH] D112349: [Verifier] Add verification logic for GlobalIFuncs

[Itay Bookstein via Phabricator via cfe-commits]

ibookstein added a comment.

Who is the Clang CFE maintainer that we need to involve?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D112349/new/

https://reviews.llvm.org/D112349

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D113431: [clang][test][NFC] Move attr-ifunc.c test from Sema to CodeGen

[Itay Bookstein via Phabricator via cfe-commits]

ibookstein created this revision.
ibookstein published this revision for review.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Signed-off-by: Itay Bookstein 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D113431

Files:
  clang/test/CodeGen/attr-ifunc.c
  clang/test/Sema/attr-ifunc.c


Index: clang/test/Sema/attr-ifunc.c
===
--- /dev/null
+++ clang/test/Sema/attr-ifunc.c
@@ -1,43 +0,0 @@
-// RUN: %clang_cc1 -triple x86_64-windows -fsyntax-only -verify %s
-// RUN: %clang_cc1 -triple x86_64-linux -fsyntax-only -verify -emit-llvm-only 
-DCHECK_ALIASES %s
-// RUN: %clang_cc1 -triple x86_64-linux -fsyntax-only -verify -emit-llvm-only 
%s
-
-#if defined(_WIN32)
-void foo() {}
-void bar() __attribute__((ifunc("foo")));
-//expected-warning@-1 {{unknown attribute 'ifunc' ignored}}
-
-#else
-#if defined(CHECK_ALIASES)
-void* f1_ifunc();
-void f1() __attribute__((ifunc("f1_ifunc")));
-//expected-error@-1 {{ifunc must point to a defined function}}
-
-void* f2_a() __attribute__((ifunc("f2_b")));
-//expected-error@-1 {{ifunc definition is part of a cycle}}
-void* f2_b() __attribute__((ifunc("f2_a")));
-//expected-error@-1 {{ifunc definition is part of a cycle}}
-
-void* f3_a() __attribute__((ifunc("f3_b")));
-//expected-warning@-1 {{ifunc will always resolve to f3_c even if weak 
definition of f3_b is overridden}}
-void* f3_b() __attribute__((weak, alias("f3_c")));
-void* f3_c() { return 0; }
-
-void f4_ifunc() {}
-void f4() __attribute__((ifunc("f4_ifunc")));
-//expected-error@-1 {{ifunc resolver function must return a pointer}}
-
-#else
-void f1a() __asm("f1");
-void f1a() {}
-//expected-note@-1 {{previous definition is here}}
-void f1() __attribute__((ifunc("f1_ifunc")));
-//expected-error@-1 {{definition with same mangled name 'f1' as another 
definition}}
-void* f1_ifunc() { return 0; }
-
-void* f6_ifunc(int i);
-void __attribute__((ifunc("f6_ifunc"))) f6() {}
-//expected-error@-1 {{definition 'f6' cannot also be an ifunc}}
-
-#endif
-#endif


Index: clang/test/Sema/attr-ifunc.c
===
--- /dev/null
+++ clang/test/Sema/attr-ifunc.c
@@ -1,43 +0,0 @@
-// RUN: %clang_cc1 -triple x86_64-windows -fsyntax-only -verify %s
-// RUN: %clang_cc1 -triple x86_64-linux -fsyntax-only -verify -emit-llvm-only -DCHECK_ALIASES %s
-// RUN: %clang_cc1 -triple x86_64-linux -fsyntax-only -verify -emit-llvm-only %s
-
-#if defined(_WIN32)
-void foo() {}
-void bar() __attribute__((ifunc("foo")));
-//expected-warning@-1 {{unknown attribute 'ifunc' ignored}}
-
-#else
-#if defined(CHECK_ALIASES)
-void* f1_ifunc();
-void f1() __attribute__((ifunc("f1_ifunc")));
-//expected-error@-1 {{ifunc must point to a defined function}}
-
-void* f2_a() __attribute__((ifunc("f2_b")));
-//expected-error@-1 {{ifunc definition is part of a cycle}}
-void* f2_b() __attribute__((ifunc("f2_a")));
-//expected-error@-1 {{ifunc definition is part of a cycle}}
-
-void* f3_a() __attribute__((ifunc("f3_b")));
-//expected-warning@-1 {{ifunc will always resolve to f3_c even if weak definition of f3_b is overridden}}
-void* f3_b() __attribute__((weak, alias("f3_c")));
-void* f3_c() { return 0; }
-
-void f4_ifunc() {}
-void f4() __attribute__((ifunc("f4_ifunc")));
-//expected-error@-1 {{ifunc resolver function must return a pointer}}
-
-#else
-void f1a() __asm("f1");
-void f1a() {}
-//expected-note@-1 {{previous definition is here}}
-void f1() __attribute__((ifunc("f1_ifunc")));
-//expected-error@-1 {{definition with same mangled name 'f1' as another definition}}
-void* f1_ifunc() { return 0; }
-
-void* f6_ifunc(int i);
-void __attribute__((ifunc("f6_ifunc"))) f6() {}
-//expected-error@-1 {{definition 'f6' cannot also be an ifunc}}
-
-#endif
-#endif
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D112868: [CodeGen] Diagnose and reject non-function ifunc resolvers

[Itay Bookstein via Phabricator via cfe-commits]

ibookstein updated this revision to Diff 385606.
ibookstein added a comment.

move test from Sema to CodeGen (trying to make arcanist send multiple
separate commits..?)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D112868/new/

https://reviews.llvm.org/D112868

Files:
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/test/CodeGen/attr-ifunc.c

Index: clang/test/CodeGen/attr-ifunc.c
===
--- clang/test/CodeGen/attr-ifunc.c
+++ clang/test/CodeGen/attr-ifunc.c
@@ -13,8 +13,7 @@
 void f1() __attribute__((ifunc("f1_ifunc")));
 //expected-error@-1 {{ifunc must point to a defined function}}
 
-void* f2_a() __attribute__((ifunc("f2_b")));
-//expected-error@-1 {{ifunc definition is part of a cycle}}
+void *f2_a() __attribute__((alias("f2_b")));
 void* f2_b() __attribute__((ifunc("f2_a")));
 //expected-error@-1 {{ifunc definition is part of a cycle}}
 
@@ -27,6 +26,15 @@
 void f4() __attribute__((ifunc("f4_ifunc")));
 //expected-error@-1 {{ifunc resolver function must return a pointer}}
 
+int f5_resolver_gvar;
+void f5() __attribute__((ifunc("f5_resolver_gvar")));
+// expected-error@-1 {{ifunc must point to a defined function}}
+
+void *f6_resolver_resolver() { return 0; }
+void *f6_resolver() __attribute__((ifunc("f6_resolver_resolver")));
+void f6() __attribute__((ifunc("f6_resolver")));
+// expected-error@-1 {{ifunc must point to a defined function}}
+
 #else
 void f1a() __asm("f1");
 void f1a() {}
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -318,21 +318,57 @@
 // This is only used in aliases that we created and we know they have a
 // linear structure.
 static const llvm::GlobalValue *getAliasedGlobal(const llvm::GlobalValue *GV) {
-  llvm::SmallPtrSet Visited;
-  for (;;) {
-if (!GV || !Visited.insert(GV).second)
-  return nullptr;
-
-const llvm::Constant *C;
-if (auto *GA = dyn_cast(GV))
-  C = GA->getAliasee();
-else if (auto *GI = dyn_cast(GV))
-  C = GI->getResolver();
-else
-  return GV;
+  const llvm::Constant *C;
+  if (auto *GA = dyn_cast(GV))
+C = GA->getAliasee();
+  else if (auto *GI = dyn_cast(GV))
+C = GI->getResolver();
+  else
+return GV;
+
+  const auto *AliaseeGV = dyn_cast(C->stripPointerCasts());
+  if (!AliaseeGV)
+return nullptr;
+
+  const llvm::GlobalValue *FinalGV = AliaseeGV->getAliaseeObject();
+  if (FinalGV == GV)
+return nullptr;
+
+  return FinalGV;
+}
+
+static bool checkAliasedGlobal(DiagnosticsEngine &Diags,
+   SourceLocation Location, bool IsIFunc,
+   const llvm::GlobalValue *Alias,
+   const llvm::GlobalValue *&GV) {
+  GV = getAliasedGlobal(Alias);
+  if (!GV) {
+Diags.Report(Location, diag::err_cyclic_alias) << IsIFunc;
+return false;
+  }
+
+  if (GV->isDeclaration()) {
+Diags.Report(Location, diag::err_alias_to_undefined) << IsIFunc << IsIFunc;
+return false;
+  }
+
+  if (IsIFunc) {
+// Check resolver function type.
+const auto *F = dyn_cast(GV);
+if (!F) {
+  Diags.Report(Location, diag::err_alias_to_undefined)
+  << IsIFunc << IsIFunc;
+  return false;
+}
 
-GV = dyn_cast(C->stripPointerCasts());
+llvm::FunctionType *FTy = F->getFunctionType();
+if (!FTy->getReturnType()->isPointerTy()) {
+  Diags.Report(Location, diag::err_ifunc_resolver_return);
+  return false;
+}
   }
+
+  return true;
 }
 
 void CodeGenModule::checkAliases() {
@@ -349,23 +385,13 @@
   Location = A->getLocation();
 else
   llvm_unreachable("Not an alias or ifunc?");
+
 StringRef MangledName = getMangledName(GD);
 llvm::GlobalValue *Alias = GetGlobalValue(MangledName);
-const llvm::GlobalValue *GV = getAliasedGlobal(Alias);
-if (!GV) {
-  Error = true;
-  Diags.Report(Location, diag::err_cyclic_alias) << IsIFunc;
-} else if (GV->isDeclaration()) {
+const llvm::GlobalValue *GV = nullptr;
+if (!checkAliasedGlobal(Diags, Location, IsIFunc, Alias, GV)) {
   Error = true;
-  Diags.Report(Location, diag::err_alias_to_undefined)
-  << IsIFunc << IsIFunc;
-} else if (IsIFunc) {
-  // Check resolver function type.
-  llvm::FunctionType *FTy = dyn_cast(
-  GV->getType()->getPointerElementType());
-  assert(FTy);
-  if (!FTy->getReturnType()->isPointerTy())
-Diags.Report(Location, diag::err_ifunc_resolver_return);
+  continue;
 }
 
 llvm::Constant *Aliasee =
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D112868: [CodeGen] Diagnose and reject non-function ifunc resolvers

[Itay Bookstein via Phabricator via cfe-commits]

ibookstein updated this revision to Diff 385603.
ibookstein retitled this revision from "[Sema] Diagnose and reject non-function 
ifunc resolvers" to "[CodeGen] Diagnose and reject non-function ifunc 
resolvers".
ibookstein added a comment.

move attr-ifunc.c test to codegen


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D112868/new/

https://reviews.llvm.org/D112868

Files:
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/test/CodeGen/attr-ifunc.c

Index: clang/test/CodeGen/attr-ifunc.c
===
--- clang/test/CodeGen/attr-ifunc.c
+++ clang/test/CodeGen/attr-ifunc.c
@@ -13,8 +13,7 @@
 void f1() __attribute__((ifunc("f1_ifunc")));
 //expected-error@-1 {{ifunc must point to a defined function}}
 
-void* f2_a() __attribute__((ifunc("f2_b")));
-//expected-error@-1 {{ifunc definition is part of a cycle}}
+void *f2_a() __attribute__((alias("f2_b")));
 void* f2_b() __attribute__((ifunc("f2_a")));
 //expected-error@-1 {{ifunc definition is part of a cycle}}
 
@@ -27,6 +26,15 @@
 void f4() __attribute__((ifunc("f4_ifunc")));
 //expected-error@-1 {{ifunc resolver function must return a pointer}}
 
+int f5_resolver_gvar;
+void f5() __attribute__((ifunc("f5_resolver_gvar")));
+// expected-error@-1 {{ifunc must point to a defined function}}
+
+void *f6_resolver_resolver() { return 0; }
+void *f6_resolver() __attribute__((ifunc("f6_resolver_resolver")));
+void f6() __attribute__((ifunc("f6_resolver")));
+// expected-error@-1 {{ifunc must point to a defined function}}
+
 #else
 void f1a() __asm("f1");
 void f1a() {}
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -318,21 +318,57 @@
 // This is only used in aliases that we created and we know they have a
 // linear structure.
 static const llvm::GlobalValue *getAliasedGlobal(const llvm::GlobalValue *GV) {
-  llvm::SmallPtrSet Visited;
-  for (;;) {
-if (!GV || !Visited.insert(GV).second)
-  return nullptr;
-
-const llvm::Constant *C;
-if (auto *GA = dyn_cast(GV))
-  C = GA->getAliasee();
-else if (auto *GI = dyn_cast(GV))
-  C = GI->getResolver();
-else
-  return GV;
+  const llvm::Constant *C;
+  if (auto *GA = dyn_cast(GV))
+C = GA->getAliasee();
+  else if (auto *GI = dyn_cast(GV))
+C = GI->getResolver();
+  else
+return GV;
+
+  const auto *AliaseeGV = dyn_cast(C->stripPointerCasts());
+  if (!AliaseeGV)
+return nullptr;
+
+  const llvm::GlobalValue *FinalGV = AliaseeGV->getAliaseeObject();
+  if (FinalGV == GV)
+return nullptr;
+
+  return FinalGV;
+}
+
+static bool checkAliasedGlobal(DiagnosticsEngine &Diags,
+   SourceLocation Location, bool IsIFunc,
+   const llvm::GlobalValue *Alias,
+   const llvm::GlobalValue *&GV) {
+  GV = getAliasedGlobal(Alias);
+  if (!GV) {
+Diags.Report(Location, diag::err_cyclic_alias) << IsIFunc;
+return false;
+  }
+
+  if (GV->isDeclaration()) {
+Diags.Report(Location, diag::err_alias_to_undefined) << IsIFunc << IsIFunc;
+return false;
+  }
+
+  if (IsIFunc) {
+// Check resolver function type.
+const auto *F = dyn_cast(GV);
+if (!F) {
+  Diags.Report(Location, diag::err_alias_to_undefined)
+  << IsIFunc << IsIFunc;
+  return false;
+}
 
-GV = dyn_cast(C->stripPointerCasts());
+llvm::FunctionType *FTy = F->getFunctionType();
+if (!FTy->getReturnType()->isPointerTy()) {
+  Diags.Report(Location, diag::err_ifunc_resolver_return);
+  return false;
+}
   }
+
+  return true;
 }
 
 void CodeGenModule::checkAliases() {
@@ -349,23 +385,13 @@
   Location = A->getLocation();
 else
   llvm_unreachable("Not an alias or ifunc?");
+
 StringRef MangledName = getMangledName(GD);
 llvm::GlobalValue *Alias = GetGlobalValue(MangledName);
-const llvm::GlobalValue *GV = getAliasedGlobal(Alias);
-if (!GV) {
-  Error = true;
-  Diags.Report(Location, diag::err_cyclic_alias) << IsIFunc;
-} else if (GV->isDeclaration()) {
+const llvm::GlobalValue *GV = nullptr;
+if (!checkAliasedGlobal(Diags, Location, IsIFunc, Alias, GV)) {
   Error = true;
-  Diags.Report(Location, diag::err_alias_to_undefined)
-  << IsIFunc << IsIFunc;
-} else if (IsIFunc) {
-  // Check resolver function type.
-  llvm::FunctionType *FTy = dyn_cast(
-  GV->getType()->getPointerElementType());
-  assert(FTy);
-  if (!FTy->getReturnType()->isPointerTy())
-Diags.Report(Location, diag::err_ifunc_resolver_return);
+  continue;
 }
 
 llvm::Constant *Aliasee =
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D112349: [Verifier] Add verification logic for GlobalIFuncs

[Itay Bookstein via Phabricator via cfe-commits]

ibookstein added a comment.

> To me the 'not in the weeds' part is, "How do I get CPU-dispatch/CPU-specific 
> to work without RAUW, since that is offensive to the CFE code owner?  
> Additionally, I found that some of the examples without the defined resolver 
> actually DO work in my downstream, though I don't know what changes we make 
> to make that happen.  So adding this limitation in actually breaks my 
> downstream.

Regarding the examples that do work, I've provided explanations as to how they 
partially sort-of-work, but that the general semantics are broken (the 
'connection' to the resolver is 'severed' in each translation unit that has it 
as a declaration + all references from within that TU are borked).

The problems that the CFE currently uses RAUW to solve are //fundamental// to 
the specified behavior of the **language features** that use it, so a solution 
for these problems needs to arise from the CFE. Imagine the following 
incremental compilation use-case:

  > extern int x; // x is a GlobalVariable declaration (initializer == null).
  > int y; // y is a GlobalVariable definition (initializer = constant 0).
  > extern int x __attribute__((alias("y"))); // x needs to transform into a 
GlobalAlias now.
  >
  > extern void *my_memcpy(void *dest, void *src, size_t n); // my_memcpy is a 
Function declaration
  > void *my_memcpy(void *dest, void *src, unsigned long n) 
__attribute__((ifunc("my_memcpy_resolver"))); // my_memcpy_resolver is a 
Function declaration, my_memcpy is a GlobalIFunc with declaration for a 
resolver - whoops, incremental module is invalid at this point
  > static void *my_memcpy_impl(void *dest, void *src, unsigned long n) { 
return 0; }
  > static void *my_memcpy_resolver(void) { return &my_memcpy_impl; } // Now 
my_memcpy ifunc has defined resolver
  >
  > void CpuSpecific(void); // CpuSpecific is a Function declaration
  > void Foo(void) { CpuSpecific(); } // Foo calls a function declaration
  > __attribute__((cpu_specific(generic))) void CpuSpecific(void) { 
puts("generic"); } // We still don't know whether cpu_dispatch will be in this 
TU or not. Do we upgrade CpuSpecific from a function declaration to a 
definition? to an ifunc? Bind directly to this body?
  > __attribute__((cpu_dispatch(generic))) void CpuSpecific(void); // Now we 
know that we have to upgrade it to an ifunc, and how the resolver should look. 
But it's already a Function (declaration).

All the above //need// to work in non-incremental compilation, and the only 
existing way for them to work right now is RAUW or a module finalization step. 
If the CFE code owner(s) find that offensive but proposes no alternative, what 
is one to do? There are 3 possible states here:

1. Remain broken
2. Solve using RAUW/finalization
3. Solve using yet-unproposed non-RAUW solution (which solves all of the above 
use-cases simultaneously because they're essentially the same)

It is better to treat (2) as a way out of (1) which doesn't increase the cost 
of (3), than to just stay at (1) until (3) happens. As it currently stands, the 
somewhat similar issue of calling a declared-but-undefined function in the REPL 
is currently embodied as a JIT session error, with failure to materialize 
symbols. Perhaps a valid solution for the work-in-progress aliases and ifuncs 
is to transform them into declarations in the JIT module until they have 
definitions for their aliasees/resolvers. But we won't know without more 
context. I think it might be more effective that I'll write a patch up which 
does use RAUW together with a test for the breakage we discussed and we'll 
continue the discussion with the CFE code owner(s) there.

>> The LLVM bitcode verifier does not consider GlobalAliases which have either 
>> a null or an undefined aliasee to be valid. The same should have held for 
>> GlobalIFuncs and their resolvers since their inception, and the fact that it 
>> were not so is an oversight.
>
> Citation Needed here.  Is there a design document that specifies this, or is 
> this your opinion?  If its the latter, the implementation was the 
> documentation, so this is still a breaking change.

I'm assuming that you're talking about GlobalIFuncs, since for GlobalAliases 
you'll see both constraints encoded in the `Verifier::visitGlobalAlias` and 
`Verifier::visitAliaseeSubExpr` functions.
As for GlobalIFunc, I have no design document for the LLVM-IR-level 
representation of the feature, but:

1. @MaskRay has provided reference for the object-format constraint itself 
(`Requirement (a): Resolver must be defined in the same translation unit as the 
implementations.`). Bitcode Modules correspond to TUs - so it stands to reason 
that the same restriction apply to them, unless behavior is implemented to 
bridge the gap.
2. I've demonstrated that compiling a bitcode module containing an ifunc with 
an undefined resolver emits broken code for all usages of the 
ifunc-with-no-resolver, where I can probably massage that into a cras

[PATCH] D112868: [Sema] Diagnose and reject non-function ifunc resolvers

[Itay Bookstein via Phabricator via cfe-commits]

ibookstein updated this revision to Diff 385541.
ibookstein added a comment.

clang-format + fastforward rebase


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D112868/new/

https://reviews.llvm.org/D112868

Files:
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/test/Sema/attr-ifunc.c

Index: clang/test/Sema/attr-ifunc.c
===
--- clang/test/Sema/attr-ifunc.c
+++ clang/test/Sema/attr-ifunc.c
@@ -13,8 +13,7 @@
 void f1() __attribute__((ifunc("f1_ifunc")));
 //expected-error@-1 {{ifunc must point to a defined function}}
 
-void* f2_a() __attribute__((ifunc("f2_b")));
-//expected-error@-1 {{ifunc definition is part of a cycle}}
+void *f2_a() __attribute__((alias("f2_b")));
 void* f2_b() __attribute__((ifunc("f2_a")));
 //expected-error@-1 {{ifunc definition is part of a cycle}}
 
@@ -27,6 +26,15 @@
 void f4() __attribute__((ifunc("f4_ifunc")));
 //expected-error@-1 {{ifunc resolver function must return a pointer}}
 
+int f5_resolver_gvar;
+void f5() __attribute__((ifunc("f5_resolver_gvar")));
+// expected-error@-1 {{ifunc must point to a defined function}}
+
+void *f6_resolver_resolver() { return 0; }
+void *f6_resolver() __attribute__((ifunc("f6_resolver_resolver")));
+void f6() __attribute__((ifunc("f6_resolver")));
+// expected-error@-1 {{ifunc must point to a defined function}}
+
 #else
 void f1a() __asm("f1");
 void f1a() {}
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -318,21 +318,57 @@
 // This is only used in aliases that we created and we know they have a
 // linear structure.
 static const llvm::GlobalValue *getAliasedGlobal(const llvm::GlobalValue *GV) {
-  llvm::SmallPtrSet Visited;
-  for (;;) {
-if (!GV || !Visited.insert(GV).second)
-  return nullptr;
-
-const llvm::Constant *C;
-if (auto *GA = dyn_cast(GV))
-  C = GA->getAliasee();
-else if (auto *GI = dyn_cast(GV))
-  C = GI->getResolver();
-else
-  return GV;
+  const llvm::Constant *C;
+  if (auto *GA = dyn_cast(GV))
+C = GA->getAliasee();
+  else if (auto *GI = dyn_cast(GV))
+C = GI->getResolver();
+  else
+return GV;
+
+  const auto *AliaseeGV = dyn_cast(C->stripPointerCasts());
+  if (!AliaseeGV)
+return nullptr;
+
+  const llvm::GlobalValue *FinalGV = AliaseeGV->getAliaseeObject();
+  if (FinalGV == GV)
+return nullptr;
+
+  return FinalGV;
+}
+
+static bool checkAliasedGlobal(DiagnosticsEngine &Diags,
+   SourceLocation Location, bool IsIFunc,
+   const llvm::GlobalValue *Alias,
+   const llvm::GlobalValue *&GV) {
+  GV = getAliasedGlobal(Alias);
+  if (!GV) {
+Diags.Report(Location, diag::err_cyclic_alias) << IsIFunc;
+return false;
+  }
+
+  if (GV->isDeclaration()) {
+Diags.Report(Location, diag::err_alias_to_undefined) << IsIFunc << IsIFunc;
+return false;
+  }
+
+  if (IsIFunc) {
+// Check resolver function type.
+const auto *F = dyn_cast(GV);
+if (!F) {
+  Diags.Report(Location, diag::err_alias_to_undefined)
+  << IsIFunc << IsIFunc;
+  return false;
+}
 
-GV = dyn_cast(C->stripPointerCasts());
+llvm::FunctionType *FTy = F->getFunctionType();
+if (!FTy->getReturnType()->isPointerTy()) {
+  Diags.Report(Location, diag::err_ifunc_resolver_return);
+  return false;
+}
   }
+
+  return true;
 }
 
 void CodeGenModule::checkAliases() {
@@ -349,23 +385,13 @@
   Location = A->getLocation();
 else
   llvm_unreachable("Not an alias or ifunc?");
+
 StringRef MangledName = getMangledName(GD);
 llvm::GlobalValue *Alias = GetGlobalValue(MangledName);
-const llvm::GlobalValue *GV = getAliasedGlobal(Alias);
-if (!GV) {
-  Error = true;
-  Diags.Report(Location, diag::err_cyclic_alias) << IsIFunc;
-} else if (GV->isDeclaration()) {
+const llvm::GlobalValue *GV = nullptr;
+if (!checkAliasedGlobal(Diags, Location, IsIFunc, Alias, GV)) {
   Error = true;
-  Diags.Report(Location, diag::err_alias_to_undefined)
-  << IsIFunc << IsIFunc;
-} else if (IsIFunc) {
-  // Check resolver function type.
-  llvm::FunctionType *FTy = dyn_cast(
-  GV->getType()->getPointerElementType());
-  assert(FTy);
-  if (!FTy->getReturnType()->isPointerTy())
-Diags.Report(Location, diag::err_ifunc_resolver_return);
+  continue;
 }
 
 llvm::Constant *Aliasee =
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D113352: [clang] Run LLVM Verifier in modes without CodeGen too

[Itay Bookstein via Phabricator via cfe-commits]

ibookstein added a comment.

Thanks; Might I ask that you commit this on my behalf? :)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D113352/new/

https://reviews.llvm.org/D113352

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D113352: [clang] Run LLVM Verifier in modes without CodeGen too

[Itay Bookstein via Phabricator via cfe-commits]

ibookstein added inline comments.



Comment at: clang/lib/CodeGen/CodeGenModule.cpp:5016
   llvm::Constant *Resolver =
-  GetOrCreateLLVMFunction(IFA->getResolver(), ResolverTy, GD,
+  GetOrCreateLLVMFunction(IFA->getResolver(), ResolverTy, {},
   /*ForVTable=*/false);

rjmccall wrote:
> Hmm, what was going on here?
The `emitIFuncDefinition` fucntion incorrectly passes the GlobalDecl of the 
IFunc itself to the call to GetOrCreateLLVMFunction for creating the resolver, 
which causes it to be created with a wrong attribute list, which fails 
`Verifier::visitFunction` with "Attribute after last parameter!". You'll note 
that unlike the relationship between aliases and their aliasees, the resolver 
and the ifunc have different types - the resolver takes no parameters.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D113352/new/

https://reviews.llvm.org/D113352

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D113352: [clang] Run LLVM Verifier in modes without CodeGen too

[Itay Bookstein via Phabricator via cfe-commits]

ibookstein marked an inline comment as done.
ibookstein added a comment.

In D113352#3114009 , @dblaikie wrote:

> I guess CodeGenOpts::VerifyModule is on by default, but I'll admit that 
> surprises me - for API-built modules, I figured it was expected they were 
> valid by construction (ie: like an assertion - it's a bug in clang if the 
> resulting module isn't valid - so we wouldn't want to validate that invariant 
> on every user's compilation, etc) - so I'd expect we should have to opt-in to 
> verifying on output. (verifying on /input/ would be always done - because the 
> input is untrusted)

I agree that it is a bit surprising. It might make sense to change the default 
and make the clang llvm-lit tool substitutions opt-in.
However, it is a somewhat useful default when considered in its position as the 
entry into the codegen pipeline.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D113352/new/

https://reviews.llvm.org/D113352

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D113352: [clang] Run LLVM Verifier in modes without CodeGen too

[Itay Bookstein via Phabricator via cfe-commits]

ibookstein updated this revision to Diff 385361.
ibookstein edited the summary of this revision.
ibookstein added a comment.
Herald added subscribers: steven_wu, hiraditya.

- Fixed PR comment about documentation
- Amended clang/test/CodeGen/lto-newpm-pipeline.c to reflect change
- Fixed small bug in CodeGenModule::emitIFuncDefinition which made the 
clang/test/CodeGen/ifunc.c test fail verification


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D113352/new/

https://reviews.llvm.org/D113352

Files:
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/test/CodeGen/lto-newpm-pipeline.c


Index: clang/test/CodeGen/lto-newpm-pipeline.c
===
--- clang/test/CodeGen/lto-newpm-pipeline.c
+++ clang/test/CodeGen/lto-newpm-pipeline.c
@@ -32,6 +32,8 @@
 // CHECK-FULL-O0-NEXT: Running pass: CoroCleanupPass
 // CHECK-FULL-O0-NEXT: Running pass: CanonicalizeAliasesPass
 // CHECK-FULL-O0-NEXT: Running pass: NameAnonGlobalPass
+// CHECK-FULL-O0-NEXT: Running pass: VerifierPass
+// CHECK-FULL-O0-NEXT: Running analysis: VerifierAnalysis
 // CHECK-FULL-O0-NEXT: Running pass: BitcodeWriterPass
 
 // CHECK-THIN-O0: Running pass: AlwaysInlinerPass
@@ -40,6 +42,8 @@
 // CHECK-THIN-O0: Running pass: CoroCleanupPass
 // CHECK-THIN-O0-NEXT: Running pass: CanonicalizeAliasesPass
 // CHECK-THIN-O0-NEXT: Running pass: NameAnonGlobalPass
+// CHECK-THIN-O0-NEXT: Running pass: VerifierPass
+// CHECK-THIN-O0-NEXT: Running analysis: VerifierAnalysis
 // CHECK-THIN-O0-NEXT: Running pass: ThinLTOBitcodeWriterPass
 
 // TODO: The LTO pre-link pipeline currently invokes
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -5013,7 +5013,7 @@
   llvm::Type *DeclTy = getTypes().ConvertTypeForMem(D->getType());
   llvm::Type *ResolverTy = llvm::GlobalIFunc::getResolverFunctionType(DeclTy);
   llvm::Constant *Resolver =
-  GetOrCreateLLVMFunction(IFA->getResolver(), ResolverTy, GD,
+  GetOrCreateLLVMFunction(IFA->getResolver(), ResolverTy, {},
   /*ForVTable=*/false);
   llvm::GlobalIFunc *GIF =
   llvm::GlobalIFunc::create(DeclTy, 0, llvm::Function::ExternalLinkage,
Index: clang/lib/CodeGen/BackendUtil.cpp
===
--- clang/lib/CodeGen/BackendUtil.cpp
+++ clang/lib/CodeGen/BackendUtil.cpp
@@ -487,6 +487,11 @@
   }
 }
 
+static bool actionRequiresCodeGen(BackendAction Action) {
+  return Action != Backend_EmitNothing && Action != Backend_EmitBC &&
+ Action != Backend_EmitLL;
+}
+
 static bool initTargetOptions(DiagnosticsEngine &Diags,
   llvm::TargetOptions &Options,
   const CodeGenOptions &CodeGenOpts,
@@ -977,9 +982,7 @@
 
   setCommandLineOpts(CodeGenOpts);
 
-  bool UsesCodeGen = (Action != Backend_EmitNothing &&
-  Action != Backend_EmitBC &&
-  Action != Backend_EmitLL);
+  bool UsesCodeGen = actionRequiresCodeGen(Action);
   CreateTargetMachine(UsesCodeGen);
 
   if (UsesCodeGen && !TM)
@@ -1006,6 +1009,12 @@
 
   CreatePasses(PerModulePasses, PerFunctionPasses);
 
+  // Add a verifier pass if requested. We don't have to do this if the action
+  // requires code generation because there will already be a verifier pass in
+  // the code-generation pipeline.
+  if (!UsesCodeGen && CodeGenOpts.VerifyModule)
+PerModulePasses.add(createVerifierPass());
+
   legacy::PassManager CodeGenPasses;
   CodeGenPasses.add(
   createTargetTransformInfoWrapperPass(getTargetIRAnalysis()));
@@ -1425,6 +1434,13 @@
   MPM.addPass(ModuleMemProfilerPass());
 }
   }
+
+  // Add a verifier pass if requested. We don't have to do this if the action
+  // requires code generation because there will already be a verifier pass in
+  // the code-generation pipeline.
+  if (!actionRequiresCodeGen(Action) && CodeGenOpts.VerifyModule)
+MPM.addPass(VerifierPass());
+
   switch (Action) {
   case Backend_EmitBC:
 if (CodeGenOpts.PrepareForThinLTO && !CodeGenOpts.DisableLLVMPasses) {
@@ -1514,8 +1530,7 @@
   TimeRegion Region(CodeGenOpts.TimePasses ? &CodeGenerationTime : nullptr);
   setCommandLineOpts(CodeGenOpts);
 
-  bool RequiresCodeGen = (Action != Backend_EmitNothing &&
-  Action != Backend_EmitBC && Action != 
Backend_EmitLL);
+  bool RequiresCodeGen = actionRequiresCodeGen(Action);
   CreateTargetMachine(RequiresCodeGen);
 
   if (RequiresCodeGen && !TM)


Index: clang/test/CodeGen/lto-newpm-pipeline.c
===
--- clang/test/CodeGen/lto-newpm-pipeline.c
+++ clang/test/CodeGen/lto-newpm-pipeline.c
@@ -32,6 +32,8 @@
 // CHECK-FULL-O0-NEXT: Running pass: CoroCleanupPass
 // CHECK-FULL-O0-

[PATCH] D113352: [clang] Run LLVM Verifier in modes without CodeGen too

[Itay Bookstein via Phabricator via cfe-commits]

ibookstein updated this revision to Diff 385298.
ibookstein added a comment.

clang-format


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D113352/new/

https://reviews.llvm.org/D113352

Files:
  clang/lib/CodeGen/BackendUtil.cpp


Index: clang/lib/CodeGen/BackendUtil.cpp
===
--- clang/lib/CodeGen/BackendUtil.cpp
+++ clang/lib/CodeGen/BackendUtil.cpp
@@ -487,6 +487,11 @@
   }
 }
 
+static bool actionRequiresCodeGen(BackendAction Action) {
+  return Action != Backend_EmitNothing && Action != Backend_EmitBC &&
+ Action != Backend_EmitLL;
+}
+
 static bool initTargetOptions(DiagnosticsEngine &Diags,
   llvm::TargetOptions &Options,
   const CodeGenOptions &CodeGenOpts,
@@ -977,9 +982,7 @@
 
   setCommandLineOpts(CodeGenOpts);
 
-  bool UsesCodeGen = (Action != Backend_EmitNothing &&
-  Action != Backend_EmitBC &&
-  Action != Backend_EmitLL);
+  bool UsesCodeGen = actionRequiresCodeGen(Action);
   CreateTargetMachine(UsesCodeGen);
 
   if (UsesCodeGen && !TM)
@@ -1006,6 +1009,10 @@
 
   CreatePasses(PerModulePasses, PerFunctionPasses);
 
+  if (!UsesCodeGen && CodeGenOpts.VerifyModule)
+// Add it here so that it runs prior to the BC/LL emission pass
+PerModulePasses.add(createVerifierPass());
+
   legacy::PassManager CodeGenPasses;
   CodeGenPasses.add(
   createTargetTransformInfoWrapperPass(getTargetIRAnalysis()));
@@ -1425,6 +1432,11 @@
   MPM.addPass(ModuleMemProfilerPass());
 }
   }
+
+  if (!actionRequiresCodeGen(Action) && CodeGenOpts.VerifyModule)
+// Add it here so that it runs prior to the BC/LL emission pass
+MPM.addPass(VerifierPass());
+
   switch (Action) {
   case Backend_EmitBC:
 if (CodeGenOpts.PrepareForThinLTO && !CodeGenOpts.DisableLLVMPasses) {
@@ -1514,8 +1526,7 @@
   TimeRegion Region(CodeGenOpts.TimePasses ? &CodeGenerationTime : nullptr);
   setCommandLineOpts(CodeGenOpts);
 
-  bool RequiresCodeGen = (Action != Backend_EmitNothing &&
-  Action != Backend_EmitBC && Action != 
Backend_EmitLL);
+  bool RequiresCodeGen = actionRequiresCodeGen(Action);
   CreateTargetMachine(RequiresCodeGen);
 
   if (RequiresCodeGen && !TM)


Index: clang/lib/CodeGen/BackendUtil.cpp
===
--- clang/lib/CodeGen/BackendUtil.cpp
+++ clang/lib/CodeGen/BackendUtil.cpp
@@ -487,6 +487,11 @@
   }
 }
 
+static bool actionRequiresCodeGen(BackendAction Action) {
+  return Action != Backend_EmitNothing && Action != Backend_EmitBC &&
+ Action != Backend_EmitLL;
+}
+
 static bool initTargetOptions(DiagnosticsEngine &Diags,
   llvm::TargetOptions &Options,
   const CodeGenOptions &CodeGenOpts,
@@ -977,9 +982,7 @@
 
   setCommandLineOpts(CodeGenOpts);
 
-  bool UsesCodeGen = (Action != Backend_EmitNothing &&
-  Action != Backend_EmitBC &&
-  Action != Backend_EmitLL);
+  bool UsesCodeGen = actionRequiresCodeGen(Action);
   CreateTargetMachine(UsesCodeGen);
 
   if (UsesCodeGen && !TM)
@@ -1006,6 +1009,10 @@
 
   CreatePasses(PerModulePasses, PerFunctionPasses);
 
+  if (!UsesCodeGen && CodeGenOpts.VerifyModule)
+// Add it here so that it runs prior to the BC/LL emission pass
+PerModulePasses.add(createVerifierPass());
+
   legacy::PassManager CodeGenPasses;
   CodeGenPasses.add(
   createTargetTransformInfoWrapperPass(getTargetIRAnalysis()));
@@ -1425,6 +1432,11 @@
   MPM.addPass(ModuleMemProfilerPass());
 }
   }
+
+  if (!actionRequiresCodeGen(Action) && CodeGenOpts.VerifyModule)
+// Add it here so that it runs prior to the BC/LL emission pass
+MPM.addPass(VerifierPass());
+
   switch (Action) {
   case Backend_EmitBC:
 if (CodeGenOpts.PrepareForThinLTO && !CodeGenOpts.DisableLLVMPasses) {
@@ -1514,8 +1526,7 @@
   TimeRegion Region(CodeGenOpts.TimePasses ? &CodeGenerationTime : nullptr);
   setCommandLineOpts(CodeGenOpts);
 
-  bool RequiresCodeGen = (Action != Backend_EmitNothing &&
-  Action != Backend_EmitBC && Action != Backend_EmitLL);
+  bool RequiresCodeGen = actionRequiresCodeGen(Action);
   CreateTargetMachine(RequiresCodeGen);
 
   if (RequiresCodeGen && !TM)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D113352: [clang] Run LLVM Verifier in modes without CodeGen too

[Itay Bookstein via Phabricator via cfe-commits]

ibookstein created this revision.
ibookstein added reviewers: dblaikie, rjmccall.
Herald added a subscriber: ormris.
ibookstein requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Previously, the Backend_Emit{Nothing,BC,LL} modes did
not run the LLVM verifier since it is usually added via
the TargetMachine::addPassesToEmitFile method according
to the DisableVerify parameter. This is called from
EmitAssemblyHelper::AddEmitPasses, which is only relevant
for BackendAction-s that require CodeGen.

Note:

- In these particular situations the verifier is added to the optimization 
pipeline rather than the codegen pipeline so that it runs prior to the BC/LL 
emission pass.
- This change applies to both the old and the new PMs.
- Because the clang tests use -emit-llvm ubiquitously, this change will enable 
the verifier for them.

Signed-off-by: Itay Bookstein 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D113352

Files:
  clang/lib/CodeGen/BackendUtil.cpp


Index: clang/lib/CodeGen/BackendUtil.cpp
===
--- clang/lib/CodeGen/BackendUtil.cpp
+++ clang/lib/CodeGen/BackendUtil.cpp
@@ -487,6 +487,12 @@
   }
 }
 
+static bool actionRequiresCodeGen(BackendAction Action) {
+  return Action != Backend_EmitNothing &&
+ Action != Backend_EmitBC &&
+ Action != Backend_EmitLL;
+}
+
 static bool initTargetOptions(DiagnosticsEngine &Diags,
   llvm::TargetOptions &Options,
   const CodeGenOptions &CodeGenOpts,
@@ -977,9 +983,7 @@
 
   setCommandLineOpts(CodeGenOpts);
 
-  bool UsesCodeGen = (Action != Backend_EmitNothing &&
-  Action != Backend_EmitBC &&
-  Action != Backend_EmitLL);
+  bool UsesCodeGen = actionRequiresCodeGen(Action);
   CreateTargetMachine(UsesCodeGen);
 
   if (UsesCodeGen && !TM)
@@ -1006,6 +1010,10 @@
 
   CreatePasses(PerModulePasses, PerFunctionPasses);
 
+  if (!UsesCodeGen && CodeGenOpts.VerifyModule)
+// Add it here so that it runs prior to the BC/LL emission pass
+PerModulePasses.add(createVerifierPass());
+
   legacy::PassManager CodeGenPasses;
   CodeGenPasses.add(
   createTargetTransformInfoWrapperPass(getTargetIRAnalysis()));
@@ -1425,6 +1433,11 @@
   MPM.addPass(ModuleMemProfilerPass());
 }
   }
+
+  if (!actionRequiresCodeGen(Action) && CodeGenOpts.VerifyModule)
+// Add it here so that it runs prior to the BC/LL emission pass
+MPM.addPass(VerifierPass());
+
   switch (Action) {
   case Backend_EmitBC:
 if (CodeGenOpts.PrepareForThinLTO && !CodeGenOpts.DisableLLVMPasses) {
@@ -1514,8 +1527,7 @@
   TimeRegion Region(CodeGenOpts.TimePasses ? &CodeGenerationTime : nullptr);
   setCommandLineOpts(CodeGenOpts);
 
-  bool RequiresCodeGen = (Action != Backend_EmitNothing &&
-  Action != Backend_EmitBC && Action != 
Backend_EmitLL);
+  bool RequiresCodeGen = actionRequiresCodeGen(Action);
   CreateTargetMachine(RequiresCodeGen);
 
   if (RequiresCodeGen && !TM)


Index: clang/lib/CodeGen/BackendUtil.cpp
===
--- clang/lib/CodeGen/BackendUtil.cpp
+++ clang/lib/CodeGen/BackendUtil.cpp
@@ -487,6 +487,12 @@
   }
 }
 
+static bool actionRequiresCodeGen(BackendAction Action) {
+  return Action != Backend_EmitNothing &&
+ Action != Backend_EmitBC &&
+ Action != Backend_EmitLL;
+}
+
 static bool initTargetOptions(DiagnosticsEngine &Diags,
   llvm::TargetOptions &Options,
   const CodeGenOptions &CodeGenOpts,
@@ -977,9 +983,7 @@
 
   setCommandLineOpts(CodeGenOpts);
 
-  bool UsesCodeGen = (Action != Backend_EmitNothing &&
-  Action != Backend_EmitBC &&
-  Action != Backend_EmitLL);
+  bool UsesCodeGen = actionRequiresCodeGen(Action);
   CreateTargetMachine(UsesCodeGen);
 
   if (UsesCodeGen && !TM)
@@ -1006,6 +1010,10 @@
 
   CreatePasses(PerModulePasses, PerFunctionPasses);
 
+  if (!UsesCodeGen && CodeGenOpts.VerifyModule)
+// Add it here so that it runs prior to the BC/LL emission pass
+PerModulePasses.add(createVerifierPass());
+
   legacy::PassManager CodeGenPasses;
   CodeGenPasses.add(
   createTargetTransformInfoWrapperPass(getTargetIRAnalysis()));
@@ -1425,6 +1433,11 @@
   MPM.addPass(ModuleMemProfilerPass());
 }
   }
+
+  if (!actionRequiresCodeGen(Action) && CodeGenOpts.VerifyModule)
+// Add it here so that it runs prior to the BC/LL emission pass
+MPM.addPass(VerifierPass());
+
   switch (Action) {
   case Backend_EmitBC:
 if (CodeGenOpts.PrepareForThinLTO && !CodeGenOpts.DisableLLVMPasses) {
@@ -1514,8 +1527,7 @@
   TimeRegion Region(CodeGenOpts.TimePasses ? &CodeGenerationTime : nullptr);
   setCommandLineOpts(CodeGenOpts);
 
-  bool RequiresCodeGen = (Action != Backend_EmitNothing &&
-

[PATCH] D112868: [Sema] Diagnose and reject non-function ifunc resolvers

[Itay Bookstein via Phabricator via cfe-commits]

ibookstein updated this revision to Diff 385212.
ibookstein added a comment.

Nicer param order for checkAliasedGlobal..


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D112868/new/

https://reviews.llvm.org/D112868

Files:
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/test/Sema/attr-ifunc.c

Index: clang/test/Sema/attr-ifunc.c
===
--- clang/test/Sema/attr-ifunc.c
+++ clang/test/Sema/attr-ifunc.c
@@ -13,8 +13,7 @@
 void f1() __attribute__((ifunc("f1_ifunc")));
 //expected-error@-1 {{ifunc must point to a defined function}}
 
-void* f2_a() __attribute__((ifunc("f2_b")));
-//expected-error@-1 {{ifunc definition is part of a cycle}}
+void *f2_a() __attribute__((alias("f2_b")));
 void* f2_b() __attribute__((ifunc("f2_a")));
 //expected-error@-1 {{ifunc definition is part of a cycle}}
 
@@ -27,6 +26,15 @@
 void f4() __attribute__((ifunc("f4_ifunc")));
 //expected-error@-1 {{ifunc resolver function must return a pointer}}
 
+int f5_resolver_gvar;
+void f5() __attribute__((ifunc("f5_resolver_gvar")));
+// expected-error@-1 {{ifunc must point to a defined function}}
+
+void *f6_resolver_resolver() { return 0; }
+void *f6_resolver() __attribute__((ifunc("f6_resolver_resolver")));
+void f6() __attribute__((ifunc("f6_resolver")));
+// expected-error@-1 {{ifunc must point to a defined function}}
+
 #else
 void f1a() __asm("f1");
 void f1a() {}
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -318,21 +318,58 @@
 // This is only used in aliases that we created and we know they have a
 // linear structure.
 static const llvm::GlobalValue *getAliasedGlobal(const llvm::GlobalValue *GV) {
-  llvm::SmallPtrSet Visited;
-  for (;;) {
-if (!GV || !Visited.insert(GV).second)
-  return nullptr;
-
-const llvm::Constant *C;
-if (auto *GA = dyn_cast(GV))
-  C = GA->getAliasee();
-else if (auto *GI = dyn_cast(GV))
-  C = GI->getResolver();
-else
-  return GV;
+  const llvm::Constant *C;
+  if (auto *GA = dyn_cast(GV))
+C = GA->getAliasee();
+  else if (auto *GI = dyn_cast(GV))
+C = GI->getResolver();
+  else
+return GV;
+
+  const auto *AliaseeGV = dyn_cast(C->stripPointerCasts());
+  if (!AliaseeGV)
+return nullptr;
+
+  const llvm::GlobalValue *FinalGV = AliaseeGV->getAliaseeObject();
+  if (FinalGV == GV)
+return nullptr;
+
+  return FinalGV;
+}
+
+static bool checkAliasedGlobal(DiagnosticsEngine &Diags,
+   SourceLocation Location,
+   bool IsIFunc,
+   const llvm::GlobalValue *Alias,
+   const llvm::GlobalValue *&GV) {
+  GV = getAliasedGlobal(Alias);
+  if (!GV) {
+Diags.Report(Location, diag::err_cyclic_alias) << IsIFunc;
+return false;
+  }
+
+  if (GV->isDeclaration()) {
+Diags.Report(Location, diag::err_alias_to_undefined) << IsIFunc << IsIFunc;
+return false;
+  }
+
+  if (IsIFunc) {
+// Check resolver function type.
+const auto *F = dyn_cast(GV);
+if (!F) {
+  Diags.Report(Location, diag::err_alias_to_undefined)
+  << IsIFunc << IsIFunc;
+  return false;
+}
 
-GV = dyn_cast(C->stripPointerCasts());
+llvm::FunctionType *FTy = F->getFunctionType();
+if (!FTy->getReturnType()->isPointerTy()) {
+  Diags.Report(Location, diag::err_ifunc_resolver_return);
+  return false;
+}
   }
+
+  return true;
 }
 
 void CodeGenModule::checkAliases() {
@@ -349,23 +386,13 @@
   Location = A->getLocation();
 else
   llvm_unreachable("Not an alias or ifunc?");
+
 StringRef MangledName = getMangledName(GD);
 llvm::GlobalValue *Alias = GetGlobalValue(MangledName);
-const llvm::GlobalValue *GV = getAliasedGlobal(Alias);
-if (!GV) {
-  Error = true;
-  Diags.Report(Location, diag::err_cyclic_alias) << IsIFunc;
-} else if (GV->isDeclaration()) {
+const llvm::GlobalValue *GV = nullptr;
+if (!checkAliasedGlobal(Diags, Location, IsIFunc, Alias, GV)) {
   Error = true;
-  Diags.Report(Location, diag::err_alias_to_undefined)
-  << IsIFunc << IsIFunc;
-} else if (IsIFunc) {
-  // Check resolver function type.
-  llvm::FunctionType *FTy = dyn_cast(
-  GV->getType()->getPointerElementType());
-  assert(FTy);
-  if (!FTy->getReturnType()->isPointerTy())
-Diags.Report(Location, diag::err_ifunc_resolver_return);
+  continue;
 }
 
 llvm::Constant *Aliasee =
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D112868: [Sema] Diagnose and reject non-function ifunc resolvers

[Itay Bookstein via Phabricator via cfe-commits]

ibookstein updated this revision to Diff 385211.
ibookstein added a comment.

Changed to using D->hasAttr() and passing that
bool into the checkAliasedGlobal function.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D112868/new/

https://reviews.llvm.org/D112868

Files:
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/test/Sema/attr-ifunc.c

Index: clang/test/Sema/attr-ifunc.c
===
--- clang/test/Sema/attr-ifunc.c
+++ clang/test/Sema/attr-ifunc.c
@@ -13,8 +13,7 @@
 void f1() __attribute__((ifunc("f1_ifunc")));
 //expected-error@-1 {{ifunc must point to a defined function}}
 
-void* f2_a() __attribute__((ifunc("f2_b")));
-//expected-error@-1 {{ifunc definition is part of a cycle}}
+void *f2_a() __attribute__((alias("f2_b")));
 void* f2_b() __attribute__((ifunc("f2_a")));
 //expected-error@-1 {{ifunc definition is part of a cycle}}
 
@@ -27,6 +26,15 @@
 void f4() __attribute__((ifunc("f4_ifunc")));
 //expected-error@-1 {{ifunc resolver function must return a pointer}}
 
+int f5_resolver_gvar;
+void f5() __attribute__((ifunc("f5_resolver_gvar")));
+// expected-error@-1 {{ifunc must point to a defined function}}
+
+void *f6_resolver_resolver() { return 0; }
+void *f6_resolver() __attribute__((ifunc("f6_resolver_resolver")));
+void f6() __attribute__((ifunc("f6_resolver")));
+// expected-error@-1 {{ifunc must point to a defined function}}
+
 #else
 void f1a() __asm("f1");
 void f1a() {}
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -318,21 +318,58 @@
 // This is only used in aliases that we created and we know they have a
 // linear structure.
 static const llvm::GlobalValue *getAliasedGlobal(const llvm::GlobalValue *GV) {
-  llvm::SmallPtrSet Visited;
-  for (;;) {
-if (!GV || !Visited.insert(GV).second)
-  return nullptr;
-
-const llvm::Constant *C;
-if (auto *GA = dyn_cast(GV))
-  C = GA->getAliasee();
-else if (auto *GI = dyn_cast(GV))
-  C = GI->getResolver();
-else
-  return GV;
+  const llvm::Constant *C;
+  if (auto *GA = dyn_cast(GV))
+C = GA->getAliasee();
+  else if (auto *GI = dyn_cast(GV))
+C = GI->getResolver();
+  else
+return GV;
+
+  const auto *AliaseeGV = dyn_cast(C->stripPointerCasts());
+  if (!AliaseeGV)
+return nullptr;
+
+  const llvm::GlobalValue *FinalGV = AliaseeGV->getAliaseeObject();
+  if (FinalGV == GV)
+return nullptr;
+
+  return FinalGV;
+}
+
+static bool checkAliasedGlobal(bool IsIFunc,
+   DiagnosticsEngine &Diags,
+   SourceLocation Location,
+   const llvm::GlobalValue *Alias,
+   const llvm::GlobalValue *&GV) {
+  GV = getAliasedGlobal(Alias);
+  if (!GV) {
+Diags.Report(Location, diag::err_cyclic_alias) << IsIFunc;
+return false;
+  }
+
+  if (GV->isDeclaration()) {
+Diags.Report(Location, diag::err_alias_to_undefined) << IsIFunc << IsIFunc;
+return false;
+  }
+
+  if (IsIFunc) {
+// Check resolver function type.
+const auto *F = dyn_cast(GV);
+if (!F) {
+  Diags.Report(Location, diag::err_alias_to_undefined)
+  << IsIFunc << IsIFunc;
+  return false;
+}
 
-GV = dyn_cast(C->stripPointerCasts());
+llvm::FunctionType *FTy = F->getFunctionType();
+if (!FTy->getReturnType()->isPointerTy()) {
+  Diags.Report(Location, diag::err_ifunc_resolver_return);
+  return false;
+}
   }
+
+  return true;
 }
 
 void CodeGenModule::checkAliases() {
@@ -349,23 +386,13 @@
   Location = A->getLocation();
 else
   llvm_unreachable("Not an alias or ifunc?");
+
 StringRef MangledName = getMangledName(GD);
 llvm::GlobalValue *Alias = GetGlobalValue(MangledName);
-const llvm::GlobalValue *GV = getAliasedGlobal(Alias);
-if (!GV) {
-  Error = true;
-  Diags.Report(Location, diag::err_cyclic_alias) << IsIFunc;
-} else if (GV->isDeclaration()) {
+const llvm::GlobalValue *GV = nullptr;
+if (!checkAliasedGlobal(IsIFunc, Diags, Location, Alias, GV)) {
   Error = true;
-  Diags.Report(Location, diag::err_alias_to_undefined)
-  << IsIFunc << IsIFunc;
-} else if (IsIFunc) {
-  // Check resolver function type.
-  llvm::FunctionType *FTy = dyn_cast(
-  GV->getType()->getPointerElementType());
-  assert(FTy);
-  if (!FTy->getReturnType()->isPointerTy())
-Diags.Report(Location, diag::err_ifunc_resolver_return);
+  continue;
 }
 
 llvm::Constant *Aliasee =
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D112349: [Verifier] Add verification logic for GlobalIFuncs

[Itay Bookstein via Phabricator via cfe-commits]

ibookstein added a comment.

I feel like we're getting lost in the weeds here.

At the time a bitcode module is finalized, it is supposed to be in a valid 
state. 
The LLVM bitcode verifier does not consider GlobalAliases which have either a 
null or an undefined aliasee to be valid. The same should have held for 
GlobalIFuncs and their resolvers since their inception, and the fact that it 
were not so is an oversight.

There are two separate issues here which we need to decouple:

- IFuncs with undefined resolvers were never valid, the verification just 
happened to be missing. They also misbehave, as demonstrated by my example 
above where a TU contains both cpu_specific and a usage, but no cpu_dispatch. 
Therefore, we'd not be making anything worse by plugging the current hole in 
the verification of GlobalIFunc and keeping cpu_specific/cpu_dispatch working, 
using the same method we use to handle aliases or ifuncs that come after 
declarations, i.e. cpu_specific emits plain function declarations, cpu_dispatch 
upgrades them via takeName+RAUW+eraseFromParent if they already exist. We'll 
have made the verification more robust, fixed a bug when there's a TU where 
there's cpu_specific + usage but no cpu_dispatch, and not have incurred more 
tech debt by doing so (since that tech debt already exists, and a solution 
would need to address all these cases in exactly the same way).
- Clang-repl/CGM needs infrastructure for dealing with this sort of 
'backtracking' in incremental compilation use-cases (declaration gets upgraded 
to alias, declaration gets upgraded to ifunc). If it needs IR changes for that, 
then they should be designed, agreed upon, and integrated. We're not going to 
have a decision made in this closed PR discussion that either GlobalAliases or 
GlobalIFuncs support a declaration state all of a sudden. Such decisions could 
have cross-cutting ramifications in that there would all of a sudden be 3 ways 
to represent things that are equivalent to/get lowered to function 
declarations, rather than one. Limiting ourselves to not using the existing 
solution for these use-cases/bugs in normal compilation with the hope that it 
will ease the creation of a solution for incremental compilation of the same 
use-cases is self-defeating.

As for clang-repl, I've so far tried the following; I get the sense that I'm 
poking around in less well-specified places (asserts and null-deref crashes):

  itay ~/llvm-project/build (main)> bin/clang-repl
  clang-repl> #include 
  clang-repl> __attribute__((cpu_dispatch(generic))) void a(void);
  clang-repl> __attribute__((cpu_specific(generic))) void a(void) { puts("hi"); 
}
  In file included from <<< inputs >>>:1:
  input_line_2:1:55: warning: body of cpu_dispatch function will be ignored 
[-Wfunction-multiversion]
  __attribute__((cpu_specific(generic))) void a(void) { puts("hi"); }
^
  clang-repl> auto b = (a(), 5);
  clang-repl: 
/home/itay/llvm-project/llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp:1683: void 
llvm::AsmPrinter::emitGlobalIFunc(llvm::Module &, const llvm::GlobalIFunc &): 
Assertion `GI.hasLocalLinkage() && "Invalid ifunc linkage"' failed.
  fish: 'bin/clang-repl' terminated by signal SIGABRT (Abort)
  itay ~/llvm-project/build (main) [SIGABRT]> bin/clang-repl
  clang-repl> extern int g_a __attribute__((alias("g_b")));
  In file included from <<< inputs >>>:1:
  input_line_0:1:31: error: alias must point to a defined variable or function
  extern int g_a __attribute__((alias("g_b")));
^
  fish: 'bin/clang-repl' terminated by signal SIGSEGV (Address boundary error)
  itay ~/llvm-project/build (main) [SIGSEGV]> bin/clang-repl
  clang-repl> void foo(void) __attribute__((ifunc("foo_resolver")));
  In file included from <<< inputs >>>:1:
  input_line_1:1:31: error: ifunc must point to a defined function
  void foo(void) __attribute__((ifunc("foo_resolver")));
^
  fish: 'bin/clang-repl' terminated by signal SIGSEGV (Address boundary error)
  itay ~/llvm-project/build (main) [SIGSEGV]> bin/clang-repl
  clang-repl> void foo(void);
  clang-repl> void bar(void) __attribute__((alias("foo")));
  In file included from <<< inputs >>>:1:
  input_line_1:1:31: error: alias must point to a defined variable or function
  void bar(void) __attribute__((alias("foo")));
^
  fish: 'bin/clang-repl' terminated by signal SIGSEGV (Address boundary error)
  itay ~/llvm-project/build (main) [SIGSEGV]> bin/clang-repl
  clang-repl> void foo(void); void bar(void) __attribute__((alias("foo"))); 
void foo(void) {}
  In file included from <<< inputs >>>:1:
  input_line_0:1:47: error: alias must point to a defined variable or function
  void foo(void); void bar(void) __attribute__((alias("foo"))); void foo(void) 
{}
^
  fish: 'bin/clang-repl' terminated by signal SIGSEGV (Address bounda

[PATCH] D112349: [Verifier] Add verification logic for GlobalIFuncs

[Itay Bookstein via Phabricator via cfe-commits]

ibookstein added a comment.

And how is Cling expecting CFE to deal with partial knowledge situations at the 
implementation level? To deal with exactly the non-local cases that the current 
violations address?
If there's no prescriptive way forward to dealing with these cases (so they're 
tech debt without a remediation plan), then as far as I'm concerned this case 
sits exactly under the same tech-debt umbrella of the existing violations and 
the way forward is using the same violating solution for this case too. 
We definitely shouldn't block the IR verification indefinitely on this impasse. 
Once an acceptable solution is found, this will also be part of that refactor.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D112349/new/

https://reviews.llvm.org/D112349

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D112868: [Sema] Diagnose and reject non-function ifunc resolvers

[Itay Bookstein via Phabricator via cfe-commits]

ibookstein added inline comments.



Comment at: clang/lib/CodeGen/CodeGenModule.cpp:320
 // linear structure.
 static const llvm::GlobalValue *getAliasedGlobal(const llvm::GlobalValue *GV) {
+  const llvm::Constant *C;

erichkeane wrote:
> Can you explain a bit better how this change here works?  The previous 
> version appears to have done quite a bit of work in the loop to look through 
> aliases/ifuncs, and it seems we don't do that anymore?  
Previously this function would have drilled through an arbitrary 
ifunc->alias->ifunc->alias interleaving structure. This is sort of peculiar 
since for alias->ifunc it would return the resolver (rather than the aliasee, 
which is the ifunc itself), and for ifunc->ifunc (which is invalid anyway) it 
would return the second ifunc's resolver.
In this change I want it to stop at the first non-alias object it encounters, 
and for ifuncs diagnose when the type of such an object is incorrect.
That is exactly what the `GlobalValue::getAliaseeObject()` function does - it 
pierces through aliases in a loop.

In essence, in an alias->ifunc->alias->... chain there should always be 0 or 1 
ifuncs, and if there's an ifunc the last object should be a resolver function 
(and not a global variable or an ifunc).
If you were to try the following code snippet, you'll see that it 
asserts/crashes clang prior to this patch:
```
int g_var;
void foo(void) __attribute__((ifunc("g_var")));
```




Comment at: clang/lib/CodeGen/CodeGenModule.cpp:397
 
+bool IsIFunc = isa(Alias);
 llvm::Constant *Aliasee =

erichkeane wrote:
> What is the purpose of changing this from checking the declaration to the IR? 
>  It seems that this is something we should be able to catch at the AST level 
> and not have to revert to IR checks like this.
Inside `checkAliasedGlobal` I wanted to save on a parameter (`D`) since then I 
would just be passing it redundantly because the information is already 
available on the `Alias`. Here I wanted the check to be an exact copy of the 
check inside `checkAliasedGlobal` for consistency. I don't mind changing that 
at at all, I guess I just don't see why I should prefer one over the other :)



Comment at: clang/test/Sema/attr-ifunc.c:16
 
-void* f2_a() __attribute__((ifunc("f2_b")));
-//expected-error@-1 {{ifunc definition is part of a cycle}}
+void *f2_a() __attribute__((alias("f2_b")));
 void* f2_b() __attribute__((ifunc("f2_a")));

erichkeane wrote:
> Did we lose this previous error?  We still want that to happen, right?
One of the two errors (the one on `f2_b`) is enough, IMO.
There's some lopsidedness in this 'cycle' diagnosis because I consider the 
ifunc 'hop' to be special - it is no longer treated as a plain alias to loop 
into.
This case is the `if (FinalGV == GV)` in the `getAliasedGlobal` implementation 
above. It's also possible to diagnose this as "resolver of ifunc cannot be 
ifunc" (if I remove that condition).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D112868/new/

https://reviews.llvm.org/D112868

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D112349: [Verifier] Add verification logic for GlobalIFuncs

[Itay Bookstein via Phabricator via cfe-commits]

ibookstein added a comment.

I see. What is the guiding principle there, though? Generating correct IR "up 
front" / "the first time" rather than "fixing it up as you go via 
manipulations"? (could you give a link?)
I can see the engineering consideration in not letting IR manipulations creep 
into the CFE, I just want to make sure that's the principle that we're asked to 
follow.
In the end this isn't the first instance where the "streaming" design of 
GlobalDecl emission forces a fixup/rewrite of a previous decision, as can be 
evidenced by a close sibling of this feature, 
`CodeGenModule::EmitAliasDefinition` (which uses exactly that idiom, for a very 
similar use-case)...
It will always be either a fixup or an accumulate/commit idiom, since there 
will always be a GlobalDecl ordering where the information to make 'the perfect 
module-global decision' isn't available upon having to act on partial 
information.

In the end, I would like to have the verification of IFuncs having a defined 
resolver restored (and avoid more dependencies being taken on this being 
'allowed' at the IR level), since having LLVM emit an object file with an 
undefined STT_GNU_IFUNC is probably just trouble and confusion waiting to 
happen.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D112349/new/

https://reviews.llvm.org/D112349

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D112349: [Verifier] Add verification logic for GlobalIFuncs

[Itay Bookstein via Phabricator via cfe-commits]

nextsilicon-itay-bookstein added a comment.

That feature already exists - use a plain old function declaration :)

My mental model for this is like this:
memcpy one of the is the most widely popular APIs commonly implemented as an 
ifunc. In clients of this API, it's just a plain old function declaration. In 
the implementer of this API, it's an ifunc with a defined resolver. Nothing new 
here.
It's true that this usage usually crosses a dynamic-linking boundary (rather 
than static linking), but a lot of the times dynamic linking and static linking 
are set up to mirror each other in behavior.

What I'm proposing is as follows. I really haven't read the existing 
implementation yet, so I'm not sure if it makes 100% sense in terms of it, but 
bear with me:

- When processing cpu_specific, emit a plain old function declaration "x.ifunc";
- When processing cpu_dispatch:
  1. Create an unnamed ifunc (call it `GI`)
  2. Call CodeGenModule::GetGlobalValue
  3. If the result was null, set the name of the ifunc and continue
  4. If the result wasn't null (call it `F`), use `GI->takeName(F); 
F->replaceAllUsesWith(GI);`
- Throughout, references against the multiversioned symbol are bound against 
the correctly named global. It just so happens that it could either begin its 
life as an ifunc and remain that way, begin its life as function declaration 
and remain that way, or begin its life as a function declaration and get 
upgraded (by RAUW) to an ifunc.

You can think of this as mirroring the behavior of the IR linker - linking a 
bitcode module containing an ifunc definition into an existing module where 
there's a function declaration with the same name as the ifunc simply tramples 
over the function declaration (RAUWs it with the linked-in ifunc). This is 
exactly what happens in `llvm/test/Linker/ifunc.ll` for `bar`.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D112349/new/

https://reviews.llvm.org/D112349

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D112349: [Verifier] Add verification logic for GlobalIFuncs

[Itay Bookstein via Phabricator via cfe-commits]

nextsilicon-itay-bookstein added a comment.

> I don't know much about the ELF format... but this works today?  We can 
> define a resolver in a different TU and it WORKS thanks to the linker?  So 
> there is perhaps something?

The ifunc symbol that is emitted in the TU with the undefined resolver loses 
its connection to the resolver and the calls to the ifunc are instead bound 
against the resolver itself (which is absolutely not what you want).

  itay> cat specific.c
  #include 
  
  __attribute__((cpu_specific(generic)))
  void single_version(void){
puts("In single_version generic");
  }
  
  void useage() {
single_version();
  }
  
  itay> cat dispatch_main.c
  void useage(void);
  
  __attribute__((cpu_dispatch(generic)))
  void single_version(void);
  
  int main()
  {
useage();
single_version();
return 0;
  }
  
  itay> clang -c dispatch_main.c -o dispatch_main.c.o
  itay> clang -c specific.c -o specific.c.o
  itay> clang specific.c.o dispatch_main.c.o -o main
  itay> ./main
  In single_version generic

This line should have been printed twice, not once.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D112349/new/

https://reviews.llvm.org/D112349

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D112349: [Verifier] Add verification logic for GlobalIFuncs

[Itay Bookstein via Phabricator via cfe-commits]

nextsilicon-itay-bookstein added a comment.

It sort-of-works only because you define the ifunc in both translation units 
(with the same name). But looks like it behaves incorrectly for references to 
the ifunc in the translation unit where the resolver is only declared, not 
defined:

  > cat example1.ll
  @single_version_ifunc = weak_odr dso_local ifunc void (), void ()* ()* 
@single_version_resolver
  define void ()* @single_version_resolver() {
ret void ()* null
  }
  
  define dso_local void @useage() local_unnamed_addr {
  entry:
tail call void @single_version_ifunc()
ret void
  }
  > clang -c example1.ll -o example1.ll.o
  > llvm-readobj --relocations example1.ll.o
  
  File: example1.ll.o
  Format: elf64-x86-64
  Arch: x86_64
  AddressSize: 64bit
  LoadName: 
  Relocations [
Section (3) .rela.text {
  0x11 R_X86_64_PLT32 single_version_ifunc 0xFFFC <-- FINE
}
Section (6) .rela.eh_frame {
  0x20 R_X86_64_PC32 .text 0x0
  0x34 R_X86_64_PC32 .text 0x10
}
  ]
  
  > cat example2.ll
  @single_version_ifunc = weak_odr dso_local ifunc void (), void ()* ()* 
@single_version_resolver
  declare void ()* @single_version_resolver()
  
  define dso_local void @useage() local_unnamed_addr {
  entry:
tail call void @single_version_ifunc()
ret void
  }
  > clang -c example2.ll -o example2.ll.o
  > llvm-readobj --relocations example2.ll.o
  
  File: example2.ll.o
  Format: elf64-x86-64
  Arch: x86_64
  AddressSize: 64bit
  LoadName: 
  Relocations [
Section (3) .rela.text {
  0x1 R_X86_64_PLT32 single_version_resolver 0xFFFC  <-- WHOOPS
}
Section (6) .rela.eh_frame {
  0x20 R_X86_64_PC32 .text 0x0
}
  ]


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D112349/new/

https://reviews.llvm.org/D112349

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D112349: [Verifier] Add verification logic for GlobalIFuncs

[Itay Bookstein via Phabricator via cfe-commits]

nextsilicon-itay-bookstein added a comment.

I'm referring you again to the start of my explanation (the first three 
paragraphs); the object file format (ELF) literally cannot express the 
semantics you're asking for. You're asking for it to support a symbol in a 
special kind of undefined state: 
ABC = "undefined, but becomes defined to the value of a different symbol XYZ if 
it ever becomes defined".

In other words, the logical type of **the value** of a symbol is either 
**empty/undefined** or **offset-into-some-section**. It is never 
**name-of-another-symbol**.

That is, unless I'm completely missing some special-sauce in the 
`Elf32_Sym`/`Elf64_Sym` documentation which allows for `st_value` to somehow 
point at the name of a different undefined symbol (am I missing something? can 
you point me at documentation that allows for this use-case?).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D112349/new/

https://reviews.llvm.org/D112349

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D112349: [Verifier] Add verification logic for GlobalIFuncs

[Itay Bookstein via Phabricator via cfe-commits]

ibookstein added a comment.

I'll first explain my thought process about the representation of aliases and 
ifuncs in the IR, and why I think both aliasees and resolvers must always be 
defined; I hope I'm not completely off track and would love it if @MaskRay 
could weigh in as to whether I make sense.
Let's start at the level of the object file; My understanding is that aliases 
apply to ELF and MachO, and ifuncs apply only to ELF. I'm not at all acquainted 
with MachO, but on the ELF side, my understanding is that:

1. Aliases are simply lowered to additional symbols with the same st_value as 
their aliasee. As long as the value of a symbol has to be concrete/numeric and 
cannot express a way to refer to another symbol, for aliases to make sense and 
have the correct semantics at this level, their aliasee must be defined at the 
IR level. Otherwise all you're left with at the object file level is an 
undefined symbol with no way to express that it 'wants to' alias an external 
symbol with some specified name. In other words, symbols are either undefined 
(st_shndx == 0, st_value meaningless) or defined (st_shndx != 0, st_value 
meaningful and holds a section offset). If we were to allow aliases to have 
undefined aliasees, they would decay to simple undefined symbols and lose their 
aliasee information.
2. IFuncs are lowered to specially typed symbols whose st_value is the 
resolver. In much the same way as aliases, for this to actually have any 
meaning, the resolver must be defined (because you have no way to specify "the 
value is in another castle named 'XYZ'", only "defined at offset X" or 
"undefined"). When we allow ifuncs to have undefined resolvers, they decay to 
simple undefined symbols with the additional wart of having a special symbol 
type, but the desired resolver name is lost. Concretely, as long as the linker 
doesn't throw a fuss at said wart, for the references against that symbol from 
within the object file this will behave like a simple undefined external 
function. Because in your implementation one TU will have a cpu_dispatch and 
therefore a defined resolver, it will 'win' and intra-EXE/intra-DSO references 
against the ifunc will indeed be bound against the return value of the 
resolver. If no translation unit in the EXE/DSO had an ifunc with the same name 
and a defined resolver, you'd end up with a peculiar undefined symbol of type 
ifunc in the EXE/DSO (same as the .o).

It is my conclusion therefore that ifuncs with undefined resolvers behave 
exactly like function declarations (and lose the name of the resolver), as long 
as the linker is willing to accept such weird symbols.
Therefore, at the IR level, they're representational slack at best, and don't 
do what you want (possibly binding against a differently-named resolver) at 
worst, so they should not be allowed.

> My understanding is the frontend's semantic rules are/were different from the 
> IR rules, which is why we implemented it that way.

As I understand it, features like aliases and ifuncs consist mostly of vertical 
plumbing to expose low-level object-file semantics, and their design must be 
informed by them.

> I'm not sure what you mean here?  Are you suggesting that an undefined 
> resolver should instead just implement an undefined 'function' for the 
> .ifunc?  This doesn't seem right?  Wouldn't that cause ODR issues?

As I understand it, making the symbol your current implementation calls 
"x.ifunc" a function **declaration** which gets upgraded to an ifunc with a 
defined resolver on encountering cpu_dispatch would yield the correct behavior.

> I guess I still don't understand what the practical limitation that requires 
> ifuncs to have a defined resolver?  The resolver is just a normal function, 
> so it seems to me that allowing them to have normal linking rules makes 
> sense?  I personally think this is the least obtrusive change; this patch is 
> adding a limitation that didn't exist previously unnecessarily.

I //think// I've addressed this in the wall of text above

> It just seems odd I guess to name a function .ifunc, and not have it be an 
> ifunc?  What does our linker think about that?

Ah, the name is just a name :)
As far as the linker is concerned, it encounters an object file with an 
undefined symbol (of type STT_NOTYPE) and an object file with a defined symbol 
with the same name, of type STT_GNU_IFUNC. It will bind references in the 
former against the definition in the latter.
Here's my trying it out:

  itay@CTHULHU ~/tmp/ifuncdecl/tu> cat main.c
  int foo(void);
  int main() { return foo(); }
  
  itay@CTHULHU ~/tmp/ifuncdecl/tu> cat foo.c
  static int foo_impl(void) { return 42; }
  static void *foo_resolver(void) { return &foo_impl; }
  int foo(void) __attribute__((ifunc("foo_resolver")));
  
  itay@CTHULHU ~/tmp/ifuncdecl/tu> clang-14 -c main.c -o main.c.o
  itay@CTHULHU ~/tmp/ifuncdecl/tu> clang-14 -c foo.c -o foo.c.o
  itay@CTHULHU ~/tmp/ifuncdecl/tu> clang-14 main.c.o foo

[PATCH] D112349: [Verifier] Add verification logic for GlobalIFuncs

[Itay Bookstein via Phabricator via cfe-commits]

ibookstein added a comment.

Note that (as the examples demonstrate) clang self-verifies and checks among 
other things that ifuncs that it emits point to definitions; this happens in 
`CodeGenModule::checkAliases()`.
I haven't read the cpu_specific/cpu_dispatch-related code in CodeGenModule yet, 
but I'm guessing that it doesn't register the generated aliases/ifuncs into the 
`CodeGenModule::Aliases` vector for deferred verification, which is why this 
didn't trigger the same error that my ifunc example did so far.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D112349/new/

https://reviews.llvm.org/D112349

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D112349: [Verifier] Add verification logic for GlobalIFuncs

[Itay Bookstein via Phabricator via cfe-commits]

ibookstein added a comment.

Hmm. When I try to compile an object file where the resolver is a declaration, 
both clang-13, clang-14, and gcc-9.3 complain that the ifunc must point to a 
defined function:

  void *foo_resolver();
  void foo(void) __attribute__((ifunc("foo_resolver")));

clang (13 and 14) complain:

  obj.c:2:31: error: ifunc must point to a defined function
  void foo(void) __attribute__((ifunc("foo_resolver")));
^
  1 error generated.

gcc 9.3.0 complains:

  obj.c:3:6: error: ‘foo’ aliased to undefined symbol ‘foo_resolver’
  3 | void foo(void) __attribute__((ifunc("foo_resolver")));
|  ^~~

I realize that the fact that frontends reject this doesn't necessarily mean 
that the IR they would have hypothetically produced is invalid, I'm just 
wondering about the semantics.
Drawing some parallels to GlobalAliases, you'll see that they also check that 
the aliasee is a definition rather than a declaration 
(`Verifier::visitAliaseeSubExpr`), which was the reason I added the same check 
to `Verifier::visitGlobalIFunc`.
Should an object file be produced with an UND ifunc symbol in that case? 
Wouldn't it be more correct to emit a plain old function declaration? (see 
`llvm/test/Linker/ifunc.ll` for behavior of linking an ifunc on top of a 
function declaration, should work correctly).

At the very least to alleviate the breakage I think we can just rip out the 
`Assert(!Resolver->isDeclarationForLinker(), "...")` from the Verifier, but I 
have a feeling that this is not the right long-term solution.

The cpu_specific/cpu_dispatch attributes are completely new to me, so bear with 
me if I'm misunderstanding; wouldn't the following implementation both provide 
the correct semantics and avoid an ifunc-with-an-undefined-resolver situation?

- The cpu_specific attribute emits
  1. A Function Declaration with a computed name "x.ifunc"
  2. A single Function with the cpu-specific body
  3. Multiple GlobalAliases with computed named whose aliasee is the function 
from 2)
- The cpu_dispatch attribute emits a strongly-defined non-interposable ifunc 
with the same computed name "x.ifunc", and a hidden defined resolver. Both IR 
linking and regular linking should resolve the plain 
function-delcaration-references to the ifunc properly.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D112349/new/

https://reviews.llvm.org/D112349

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D112868: [Sema] Diagnose and reject non-function ifunc resolvers

[Itay Bookstein via Phabricator via cfe-commits]

ibookstein updated this revision to Diff 383661.
ibookstein added a comment.

clang-format


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D112868/new/

https://reviews.llvm.org/D112868

Files:
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/test/Sema/attr-ifunc.c

Index: clang/test/Sema/attr-ifunc.c
===
--- clang/test/Sema/attr-ifunc.c
+++ clang/test/Sema/attr-ifunc.c
@@ -13,8 +13,7 @@
 void f1() __attribute__((ifunc("f1_ifunc")));
 //expected-error@-1 {{ifunc must point to a defined function}}
 
-void* f2_a() __attribute__((ifunc("f2_b")));
-//expected-error@-1 {{ifunc definition is part of a cycle}}
+void *f2_a() __attribute__((alias("f2_b")));
 void* f2_b() __attribute__((ifunc("f2_a")));
 //expected-error@-1 {{ifunc definition is part of a cycle}}
 
@@ -27,6 +26,15 @@
 void f4() __attribute__((ifunc("f4_ifunc")));
 //expected-error@-1 {{ifunc resolver function must return a pointer}}
 
+int f5_resolver_gvar;
+void f5() __attribute__((ifunc("f5_resolver_gvar")));
+// expected-error@-1 {{ifunc must point to a defined function}}
+
+void *f6_resolver_resolver() { return 0; }
+void *f6_resolver() __attribute__((ifunc("f6_resolver_resolver")));
+void f6() __attribute__((ifunc("f6_resolver")));
+// expected-error@-1 {{ifunc must point to a defined function}}
+
 #else
 void f1a() __asm("f1");
 void f1a() {}
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -318,21 +318,58 @@
 // This is only used in aliases that we created and we know they have a
 // linear structure.
 static const llvm::GlobalValue *getAliasedGlobal(const llvm::GlobalValue *GV) {
-  llvm::SmallPtrSet Visited;
-  for (;;) {
-if (!GV || !Visited.insert(GV).second)
-  return nullptr;
-
-const llvm::Constant *C;
-if (auto *GA = dyn_cast(GV))
-  C = GA->getAliasee();
-else if (auto *GI = dyn_cast(GV))
-  C = GI->getResolver();
-else
-  return GV;
+  const llvm::Constant *C;
+  if (auto *GA = dyn_cast(GV))
+C = GA->getAliasee();
+  else if (auto *GI = dyn_cast(GV))
+C = GI->getResolver();
+  else
+return GV;
+
+  const auto *AliaseeGV = dyn_cast(C->stripPointerCasts());
+  if (!AliaseeGV)
+return nullptr;
+
+  const llvm::GlobalValue *FinalGV = AliaseeGV->getAliaseeObject();
+  if (FinalGV == GV)
+return nullptr;
+
+  return FinalGV;
+}
+
+static bool checkAliasedGlobal(const llvm::GlobalValue *Alias,
+   DiagnosticsEngine &Diags,
+   SourceLocation Location,
+   const llvm::GlobalValue *&GV) {
+  bool IsIFunc = isa(Alias);
+  GV = getAliasedGlobal(Alias);
+  if (!GV) {
+Diags.Report(Location, diag::err_cyclic_alias) << IsIFunc;
+return false;
+  }
+
+  if (GV->isDeclaration()) {
+Diags.Report(Location, diag::err_alias_to_undefined) << IsIFunc << IsIFunc;
+return false;
+  }
+
+  if (IsIFunc) {
+// Check resolver function type.
+const auto *F = dyn_cast(GV);
+if (!F) {
+  Diags.Report(Location, diag::err_alias_to_undefined)
+  << IsIFunc << IsIFunc;
+  return false;
+}
 
-GV = dyn_cast(C->stripPointerCasts());
+llvm::FunctionType *FTy = F->getFunctionType();
+if (!FTy->getReturnType()->isPointerTy()) {
+  Diags.Report(Location, diag::err_ifunc_resolver_return);
+  return false;
+}
   }
+
+  return true;
 }
 
 void CodeGenModule::checkAliases() {
@@ -344,30 +381,20 @@
   for (const GlobalDecl &GD : Aliases) {
 const auto *D = cast(GD.getDecl());
 SourceLocation Location;
-bool IsIFunc = D->hasAttr();
 if (const Attr *A = D->getDefiningAttr())
   Location = A->getLocation();
 else
   llvm_unreachable("Not an alias or ifunc?");
+
 StringRef MangledName = getMangledName(GD);
 llvm::GlobalValue *Alias = GetGlobalValue(MangledName);
-const llvm::GlobalValue *GV = getAliasedGlobal(Alias);
-if (!GV) {
-  Error = true;
-  Diags.Report(Location, diag::err_cyclic_alias) << IsIFunc;
-} else if (GV->isDeclaration()) {
+const llvm::GlobalValue *GV = nullptr;
+if (!checkAliasedGlobal(Alias, Diags, Location, GV)) {
   Error = true;
-  Diags.Report(Location, diag::err_alias_to_undefined)
-  << IsIFunc << IsIFunc;
-} else if (IsIFunc) {
-  // Check resolver function type.
-  llvm::FunctionType *FTy = dyn_cast(
-  GV->getType()->getPointerElementType());
-  assert(FTy);
-  if (!FTy->getReturnType()->isPointerTy())
-Diags.Report(Location, diag::err_ifunc_resolver_return);
+  continue;
 }
 
+bool IsIFunc = isa(Alias);
 llvm::Constant *Aliasee =
 IsIFunc ? cast(Alias)->getResolver()
 : cast(Alias)->getAliasee();
___
c

[PATCH] D112349: [Verifier] Add verification logic for GlobalIFuncs

[Itay Bookstein via Phabricator via cfe-commits]

ibookstein added a comment.

Could you commit this on my behalf? :)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D112349/new/

https://reviews.llvm.org/D112349

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D112868: [Sema] Diagnose and reject non-function ifunc resolvers

[Itay Bookstein via Phabricator via cfe-commits]

ibookstein created this revision.
ibookstein added reviewers: MaskRay, aaron.ballman.
ibookstein requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Signed-off-by: Itay Bookstein 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D112868

Files:
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/test/Sema/attr-ifunc.c

Index: clang/test/Sema/attr-ifunc.c
===
--- clang/test/Sema/attr-ifunc.c
+++ clang/test/Sema/attr-ifunc.c
@@ -13,8 +13,7 @@
 void f1() __attribute__((ifunc("f1_ifunc")));
 //expected-error@-1 {{ifunc must point to a defined function}}
 
-void* f2_a() __attribute__((ifunc("f2_b")));
-//expected-error@-1 {{ifunc definition is part of a cycle}}
+void* f2_a() __attribute__((alias("f2_b")));
 void* f2_b() __attribute__((ifunc("f2_a")));
 //expected-error@-1 {{ifunc definition is part of a cycle}}
 
@@ -27,6 +26,15 @@
 void f4() __attribute__((ifunc("f4_ifunc")));
 //expected-error@-1 {{ifunc resolver function must return a pointer}}
 
+int f5_resolver_gvar;
+void f5() __attribute__((ifunc("f5_resolver_gvar")));
+//expected-error@-1 {{ifunc must point to a defined function}}
+
+void *f6_resolver_resolver() { return 0; }
+void *f6_resolver() __attribute__((ifunc("f6_resolver_resolver")));
+void f6() __attribute__((ifunc("f6_resolver")));
+//expected-error@-1 {{ifunc must point to a defined function}}
+
 #else
 void f1a() __asm("f1");
 void f1a() {}
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -318,21 +318,58 @@
 // This is only used in aliases that we created and we know they have a
 // linear structure.
 static const llvm::GlobalValue *getAliasedGlobal(const llvm::GlobalValue *GV) {
-  llvm::SmallPtrSet Visited;
-  for (;;) {
-if (!GV || !Visited.insert(GV).second)
-  return nullptr;
-
-const llvm::Constant *C;
-if (auto *GA = dyn_cast(GV))
-  C = GA->getAliasee();
-else if (auto *GI = dyn_cast(GV))
-  C = GI->getResolver();
-else
-  return GV;
+  const llvm::Constant *C;
+  if (auto *GA = dyn_cast(GV))
+C = GA->getAliasee();
+  else if (auto *GI = dyn_cast(GV))
+C = GI->getResolver();
+  else
+return GV;
+
+  const auto *AliaseeGV = dyn_cast(C->stripPointerCasts());
+  if (!AliaseeGV)
+return nullptr;
+
+  const llvm::GlobalValue *FinalGV = AliaseeGV->getAliaseeObject();
+  if (FinalGV == GV)
+return nullptr;
+
+  return FinalGV;
+}
+
+static bool checkAliasedGlobal(const llvm::GlobalValue *Alias,
+   DiagnosticsEngine &Diags,
+   SourceLocation Location,
+   const llvm::GlobalValue *&GV) {
+  bool IsIFunc = isa(Alias);
+  GV = getAliasedGlobal(Alias);
+  if (!GV) {
+Diags.Report(Location, diag::err_cyclic_alias) << IsIFunc;
+return false;
+  }
+
+  if (GV->isDeclaration()) {
+Diags.Report(Location, diag::err_alias_to_undefined) << IsIFunc << IsIFunc;
+return false;
+  }
+
+  if (IsIFunc) {
+// Check resolver function type.
+const auto *F = dyn_cast(GV);
+if (!F) {
+  Diags.Report(Location, diag::err_alias_to_undefined)
+  << IsIFunc << IsIFunc;
+  return false;
+}
 
-GV = dyn_cast(C->stripPointerCasts());
+llvm::FunctionType *FTy = F->getFunctionType();
+if (!FTy->getReturnType()->isPointerTy()) {
+  Diags.Report(Location, diag::err_ifunc_resolver_return);
+  return false;
+}
   }
+
+  return true;
 }
 
 void CodeGenModule::checkAliases() {
@@ -344,30 +381,20 @@
   for (const GlobalDecl &GD : Aliases) {
 const auto *D = cast(GD.getDecl());
 SourceLocation Location;
-bool IsIFunc = D->hasAttr();
 if (const Attr *A = D->getDefiningAttr())
   Location = A->getLocation();
 else
   llvm_unreachable("Not an alias or ifunc?");
+
 StringRef MangledName = getMangledName(GD);
 llvm::GlobalValue *Alias = GetGlobalValue(MangledName);
-const llvm::GlobalValue *GV = getAliasedGlobal(Alias);
-if (!GV) {
-  Error = true;
-  Diags.Report(Location, diag::err_cyclic_alias) << IsIFunc;
-} else if (GV->isDeclaration()) {
+const llvm::GlobalValue *GV = nullptr;
+if (!checkAliasedGlobal(Alias, Diags, Location, GV)) {
   Error = true;
-  Diags.Report(Location, diag::err_alias_to_undefined)
-  << IsIFunc << IsIFunc;
-} else if (IsIFunc) {
-  // Check resolver function type.
-  llvm::FunctionType *FTy = dyn_cast(
-  GV->getType()->getPointerElementType());
-  assert(FTy);
-  if (!FTy->getReturnType()->isPointerTy())
-Diags.Report(Location, diag::err_ifunc_resolver_return);
+  continue;
 }
 
+bool IsIFunc = isa(Alias);
 llvm::Constant *Aliasee =
 IsIFunc ? cast(Alias)->getResolver()
 : c

[PATCH] D112349: [Verifier] Add verification logic for GlobalIFuncs

[Itay Bookstein via Phabricator via cfe-commits]

ibookstein updated this revision to Diff 381773.
ibookstein added a comment.

Change BitcodeReader to transparently fix up the type rather than returning an 
error because older formats use wrong type.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D112349/new/

https://reviews.llvm.org/D112349

Files:
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/test/CodeGen/ifunc.c
  clang/test/CodeGen/semantic-interposition.c
  llvm/include/llvm/IR/GlobalIFunc.h
  llvm/lib/Bitcode/Reader/BitcodeReader.cpp
  llvm/lib/IR/Globals.cpp
  llvm/lib/IR/Verifier.cpp
  llvm/test/Assembler/ifunc-asm.ll
  llvm/test/Assembler/ifunc-dsolocal.ll
  llvm/test/Assembler/ifunc-use-list-order.ll
  llvm/test/Bindings/llvm-c/echo.ll
  llvm/test/Bitcode/compatibility-3.9.ll
  llvm/test/Bitcode/compatibility-4.0.ll
  llvm/test/Bitcode/compatibility-5.0.ll
  llvm/test/Bitcode/compatibility-6.0.ll
  llvm/test/Bitcode/compatibility.ll
  llvm/test/Bitcode/dso_local_equivalent.ll
  llvm/test/Bitcode/dso_location.ll
  llvm/test/CodeGen/PowerPC/ifunc.ll
  llvm/test/CodeGen/X86/addrsig.ll
  llvm/test/CodeGen/X86/dso_local_equivalent.ll
  llvm/test/CodeGen/X86/ifunc-asm.ll
  llvm/test/CodeGen/X86/partition.ll
  llvm/test/LTO/Resolution/X86/Inputs/ifunc2.ll
  llvm/test/LTO/Resolution/X86/ifunc.ll
  llvm/test/LTO/Resolution/X86/ifunc2.ll
  llvm/test/Linker/ifunc.ll
  llvm/test/Object/X86/nm-ir.ll
  llvm/test/ThinLTO/X86/empty-module.ll
  llvm/test/Transforms/GlobalDCE/global-ifunc.ll

Index: llvm/test/Transforms/GlobalDCE/global-ifunc.ll
===
--- llvm/test/Transforms/GlobalDCE/global-ifunc.ll
+++ llvm/test/Transforms/GlobalDCE/global-ifunc.ll
@@ -2,12 +2,12 @@
 target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-unknown-linux-gnu"
 
-@if = ifunc void (), void ()* @fn
+@if = ifunc void (), void ()* ()* @fn
 
-define internal void @fn() {
+define internal void ()* @fn() {
 entry:
-  ret void
+  ret void ()* null
 }
 
-; CHECK-DAG: @if = ifunc void (), void ()* @fn
-; CHECK-DAG: define internal void @fn(
+; CHECK-DAG: @if = ifunc void (), void ()* ()* @fn
+; CHECK-DAG: define internal void ()* @fn(
Index: llvm/test/ThinLTO/X86/empty-module.ll
===
--- llvm/test/ThinLTO/X86/empty-module.ll
+++ llvm/test/ThinLTO/X86/empty-module.ll
@@ -10,9 +10,9 @@
 target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-unknown-linux-gnu"
 
-@foo = ifunc i32 (i32), i64 ()* @foo_ifunc
+@foo = ifunc i32 (i32), i32 (i32)* ()* @foo_ifunc
 
-define internal i64 @foo_ifunc() {
+define internal i32 (i32)* @foo_ifunc() {
 entry:
-  ret i64 0
+  ret i32 (i32)* null
 }
Index: llvm/test/Object/X86/nm-ir.ll
===
--- llvm/test/Object/X86/nm-ir.ll
+++ llvm/test/Object/X86/nm-ir.ll
@@ -32,12 +32,12 @@
 @a1 = alias i32, i32* @g1
 @a2 = internal alias i32, i32* @g1
 
-define void @f1() {
+define void ()* @f1() {
   call void @f5()
-  ret void
+  ret void ()* null
 }
 
-@ifunc_f1 = ifunc void (), void ()* @f1
+@ifunc_f1 = ifunc void (), void ()* ()* @f1
 
 define internal void @f2() {
   ret void
Index: llvm/test/Linker/ifunc.ll
===
--- llvm/test/Linker/ifunc.ll
+++ llvm/test/Linker/ifunc.ll
@@ -3,18 +3,18 @@
 
 ;; Check that ifuncs are linked in properly.
 
-; CHECK-DAG: @foo = ifunc void (), bitcast (void ()* ()* @foo_resolve to void ()*)
+; CHECK-DAG: @foo = ifunc void (), void ()* ()* @foo_resolve
 ; CHECK-DAG: define internal void ()* @foo_resolve() {
 
-; CHECK-DAG: @bar = ifunc void (), bitcast (void ()* ()* @bar_resolve to void ()*)
+; CHECK-DAG: @bar = ifunc void (), void ()* ()* @bar_resolve
 ; CHECK-DAG: define internal void ()* @bar_resolve() {
 
 ;--- a.ll
 declare void @bar()
 
 ;--- b.ll
-@foo = ifunc void (), bitcast (void ()* ()* @foo_resolve to void ()*)
-@bar = ifunc void (), bitcast (void ()* ()* @bar_resolve to void ()*)
+@foo = ifunc void (), void ()* ()* @foo_resolve
+@bar = ifunc void (), void ()* ()* @bar_resolve
 
 define internal void ()* @foo_resolve() {
   ret void ()* null
Index: llvm/test/LTO/Resolution/X86/ifunc2.ll
===
--- llvm/test/LTO/Resolution/X86/ifunc2.ll
+++ llvm/test/LTO/Resolution/X86/ifunc2.ll
@@ -6,14 +6,14 @@
 target datalayout = "e-p:64:64"
 target triple = "x86_64-unknown-linux-gnu"
 
-; CHECK: @foo = ifunc i32 (), i32 ()* @foo_resolver.2
-@foo = ifunc i32 (), i32 ()* @foo_resolver
+; CHECK: @foo = ifunc i32 (), i32 ()* ()* @foo_resolver.2
+@foo = ifunc i32 (), i32 ()* ()* @foo_resolver
 
-; CHECK: define internal i32 @foo_resolver.2() {
-; CHECK-NEXT: ret i32 1
-define weak i32 @foo_resolver() {
-  ret i32 1
+; CHECK: define internal i32 ()* @foo_resolver.2() {
+; CHECK-NEXT: ret i32 ()* intto

[PATCH] D112349: [Verifier] Add verification logic for GlobalIFuncs

[Itay Bookstein via Phabricator via cfe-commits]

ibookstein updated this revision to Diff 381760.
ibookstein edited the summary of this revision.
ibookstein added a comment.
Herald added a subscriber: nemanjai.

Added check to BitcodeReader, fixed clang tests, hoisted
logic to shared static function on GlobalIFunc.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D112349/new/

https://reviews.llvm.org/D112349

Files:
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/test/CodeGen/ifunc.c
  clang/test/CodeGen/semantic-interposition.c
  llvm/include/llvm/IR/GlobalIFunc.h
  llvm/lib/Bitcode/Reader/BitcodeReader.cpp
  llvm/lib/IR/Globals.cpp
  llvm/lib/IR/Verifier.cpp
  llvm/test/Assembler/ifunc-asm.ll
  llvm/test/Assembler/ifunc-dsolocal.ll
  llvm/test/Assembler/ifunc-use-list-order.ll
  llvm/test/Bindings/llvm-c/echo.ll
  llvm/test/Bitcode/compatibility-6.0.ll
  llvm/test/Bitcode/compatibility-6.0.ll.bc
  llvm/test/Bitcode/compatibility.ll
  llvm/test/Bitcode/dso_local_equivalent.ll
  llvm/test/Bitcode/dso_location.ll
  llvm/test/CodeGen/PowerPC/ifunc.ll
  llvm/test/CodeGen/X86/addrsig.ll
  llvm/test/CodeGen/X86/dso_local_equivalent.ll
  llvm/test/CodeGen/X86/ifunc-asm.ll
  llvm/test/CodeGen/X86/partition.ll
  llvm/test/LTO/Resolution/X86/Inputs/ifunc2.ll
  llvm/test/LTO/Resolution/X86/ifunc.ll
  llvm/test/LTO/Resolution/X86/ifunc2.ll
  llvm/test/Linker/ifunc.ll
  llvm/test/Object/X86/nm-ir.ll
  llvm/test/ThinLTO/X86/empty-module.ll
  llvm/test/Transforms/GlobalDCE/global-ifunc.ll

Index: llvm/test/Transforms/GlobalDCE/global-ifunc.ll
===
--- llvm/test/Transforms/GlobalDCE/global-ifunc.ll
+++ llvm/test/Transforms/GlobalDCE/global-ifunc.ll
@@ -2,12 +2,12 @@
 target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-unknown-linux-gnu"
 
-@if = ifunc void (), void ()* @fn
+@if = ifunc void (), void ()* ()* @fn
 
-define internal void @fn() {
+define internal void ()* @fn() {
 entry:
-  ret void
+  ret void ()* null
 }
 
-; CHECK-DAG: @if = ifunc void (), void ()* @fn
-; CHECK-DAG: define internal void @fn(
+; CHECK-DAG: @if = ifunc void (), void ()* ()* @fn
+; CHECK-DAG: define internal void ()* @fn(
Index: llvm/test/ThinLTO/X86/empty-module.ll
===
--- llvm/test/ThinLTO/X86/empty-module.ll
+++ llvm/test/ThinLTO/X86/empty-module.ll
@@ -10,9 +10,9 @@
 target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-unknown-linux-gnu"
 
-@foo = ifunc i32 (i32), i64 ()* @foo_ifunc
+@foo = ifunc i32 (i32), i32 (i32)* ()* @foo_ifunc
 
-define internal i64 @foo_ifunc() {
+define internal i32 (i32)* @foo_ifunc() {
 entry:
-  ret i64 0
+  ret i32 (i32)* null
 }
Index: llvm/test/Object/X86/nm-ir.ll
===
--- llvm/test/Object/X86/nm-ir.ll
+++ llvm/test/Object/X86/nm-ir.ll
@@ -32,12 +32,12 @@
 @a1 = alias i32, i32* @g1
 @a2 = internal alias i32, i32* @g1
 
-define void @f1() {
+define void ()* @f1() {
   call void @f5()
-  ret void
+  ret void ()* null
 }
 
-@ifunc_f1 = ifunc void (), void ()* @f1
+@ifunc_f1 = ifunc void (), void ()* ()* @f1
 
 define internal void @f2() {
   ret void
Index: llvm/test/Linker/ifunc.ll
===
--- llvm/test/Linker/ifunc.ll
+++ llvm/test/Linker/ifunc.ll
@@ -3,18 +3,18 @@
 
 ;; Check that ifuncs are linked in properly.
 
-; CHECK-DAG: @foo = ifunc void (), bitcast (void ()* ()* @foo_resolve to void ()*)
+; CHECK-DAG: @foo = ifunc void (), void ()* ()* @foo_resolve
 ; CHECK-DAG: define internal void ()* @foo_resolve() {
 
-; CHECK-DAG: @bar = ifunc void (), bitcast (void ()* ()* @bar_resolve to void ()*)
+; CHECK-DAG: @bar = ifunc void (), void ()* ()* @bar_resolve
 ; CHECK-DAG: define internal void ()* @bar_resolve() {
 
 ;--- a.ll
 declare void @bar()
 
 ;--- b.ll
-@foo = ifunc void (), bitcast (void ()* ()* @foo_resolve to void ()*)
-@bar = ifunc void (), bitcast (void ()* ()* @bar_resolve to void ()*)
+@foo = ifunc void (), void ()* ()* @foo_resolve
+@bar = ifunc void (), void ()* ()* @bar_resolve
 
 define internal void ()* @foo_resolve() {
   ret void ()* null
Index: llvm/test/LTO/Resolution/X86/ifunc2.ll
===
--- llvm/test/LTO/Resolution/X86/ifunc2.ll
+++ llvm/test/LTO/Resolution/X86/ifunc2.ll
@@ -6,14 +6,14 @@
 target datalayout = "e-p:64:64"
 target triple = "x86_64-unknown-linux-gnu"
 
-; CHECK: @foo = ifunc i32 (), i32 ()* @foo_resolver.2
-@foo = ifunc i32 (), i32 ()* @foo_resolver
+; CHECK: @foo = ifunc i32 (), i32 ()* ()* @foo_resolver.2
+@foo = ifunc i32 (), i32 ()* ()* @foo_resolver
 
-; CHECK: define internal i32 @foo_resolver.2() {
-; CHECK-NEXT: ret i32 1
-define weak i32 @foo_resolver() {
-  ret i32 1
+; CHECK: define internal i32 ()* @foo_resolver.2() {
+; CHECK-NEXT: ret i32 ()* inttoptr (i32 1 to

[PATCH] D112349: [Verifier] Add verification logic for GlobalIFuncs

[Itay Bookstein via Phabricator via cfe-commits]

ibookstein added a comment.

Because this commit changes an existing binary bitcode file, and because that 
file specifically tests backwards compatibility, does that mean I need to avoid 
changing it and instead add a backwards compatibility fix to the BitcodeReader? 
(Something like always bitcasting to the required type?)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D112349/new/

https://reviews.llvm.org/D112349

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D112349: [Verifier] Add verification logic for GlobalIFuncs

[Itay Bookstein via Phabricator via cfe-commits]

ibookstein updated this revision to Diff 381754.
ibookstein added a comment.

Now using arcanist because commit includes change to binary file.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D112349/new/

https://reviews.llvm.org/D112349

Files:
  clang/lib/CodeGen/CodeGenModule.cpp
  llvm/lib/IR/Globals.cpp
  llvm/lib/IR/Verifier.cpp
  llvm/test/Assembler/ifunc-asm.ll
  llvm/test/Assembler/ifunc-dsolocal.ll
  llvm/test/Assembler/ifunc-use-list-order.ll
  llvm/test/Bindings/llvm-c/echo.ll
  llvm/test/Bitcode/compatibility-6.0.ll
  llvm/test/Bitcode/compatibility-6.0.ll.bc
  llvm/test/Bitcode/compatibility.ll
  llvm/test/Bitcode/dso_local_equivalent.ll
  llvm/test/Bitcode/dso_location.ll
  llvm/test/CodeGen/X86/addrsig.ll
  llvm/test/CodeGen/X86/dso_local_equivalent.ll
  llvm/test/CodeGen/X86/ifunc-asm.ll
  llvm/test/CodeGen/X86/partition.ll
  llvm/test/LTO/Resolution/X86/Inputs/ifunc2.ll
  llvm/test/LTO/Resolution/X86/ifunc.ll
  llvm/test/LTO/Resolution/X86/ifunc2.ll
  llvm/test/Linker/ifunc.ll
  llvm/test/Object/X86/nm-ir.ll
  llvm/test/ThinLTO/X86/empty-module.ll
  llvm/test/Transforms/GlobalDCE/global-ifunc.ll

Index: llvm/test/Transforms/GlobalDCE/global-ifunc.ll
===
--- llvm/test/Transforms/GlobalDCE/global-ifunc.ll
+++ llvm/test/Transforms/GlobalDCE/global-ifunc.ll
@@ -2,12 +2,12 @@
 target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-unknown-linux-gnu"
 
-@if = ifunc void (), void ()* @fn
+@if = ifunc void (), void ()* ()* @fn
 
-define internal void @fn() {
+define internal void ()* @fn() {
 entry:
-  ret void
+  ret void ()* null
 }
 
-; CHECK-DAG: @if = ifunc void (), void ()* @fn
-; CHECK-DAG: define internal void @fn(
+; CHECK-DAG: @if = ifunc void (), void ()* ()* @fn
+; CHECK-DAG: define internal void ()* @fn(
Index: llvm/test/ThinLTO/X86/empty-module.ll
===
--- llvm/test/ThinLTO/X86/empty-module.ll
+++ llvm/test/ThinLTO/X86/empty-module.ll
@@ -10,9 +10,9 @@
 target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-unknown-linux-gnu"
 
-@foo = ifunc i32 (i32), i64 ()* @foo_ifunc
+@foo = ifunc i32 (i32), i32 (i32)* ()* @foo_ifunc
 
-define internal i64 @foo_ifunc() {
+define internal i32 (i32)* @foo_ifunc() {
 entry:
-  ret i64 0
+  ret i32 (i32)* null
 }
Index: llvm/test/Object/X86/nm-ir.ll
===
--- llvm/test/Object/X86/nm-ir.ll
+++ llvm/test/Object/X86/nm-ir.ll
@@ -32,12 +32,12 @@
 @a1 = alias i32, i32* @g1
 @a2 = internal alias i32, i32* @g1
 
-define void @f1() {
+define void ()* @f1() {
   call void @f5()
-  ret void
+  ret void ()* null
 }
 
-@ifunc_f1 = ifunc void (), void ()* @f1
+@ifunc_f1 = ifunc void (), void ()* ()* @f1
 
 define internal void @f2() {
   ret void
Index: llvm/test/Linker/ifunc.ll
===
--- llvm/test/Linker/ifunc.ll
+++ llvm/test/Linker/ifunc.ll
@@ -3,18 +3,18 @@
 
 ;; Check that ifuncs are linked in properly.
 
-; CHECK-DAG: @foo = ifunc void (), bitcast (void ()* ()* @foo_resolve to void ()*)
+; CHECK-DAG: @foo = ifunc void (), void ()* ()* @foo_resolve
 ; CHECK-DAG: define internal void ()* @foo_resolve() {
 
-; CHECK-DAG: @bar = ifunc void (), bitcast (void ()* ()* @bar_resolve to void ()*)
+; CHECK-DAG: @bar = ifunc void (), void ()* ()* @bar_resolve
 ; CHECK-DAG: define internal void ()* @bar_resolve() {
 
 ;--- a.ll
 declare void @bar()
 
 ;--- b.ll
-@foo = ifunc void (), bitcast (void ()* ()* @foo_resolve to void ()*)
-@bar = ifunc void (), bitcast (void ()* ()* @bar_resolve to void ()*)
+@foo = ifunc void (), void ()* ()* @foo_resolve
+@bar = ifunc void (), void ()* ()* @bar_resolve
 
 define internal void ()* @foo_resolve() {
   ret void ()* null
Index: llvm/test/LTO/Resolution/X86/ifunc2.ll
===
--- llvm/test/LTO/Resolution/X86/ifunc2.ll
+++ llvm/test/LTO/Resolution/X86/ifunc2.ll
@@ -6,14 +6,14 @@
 target datalayout = "e-p:64:64"
 target triple = "x86_64-unknown-linux-gnu"
 
-; CHECK: @foo = ifunc i32 (), i32 ()* @foo_resolver.2
-@foo = ifunc i32 (), i32 ()* @foo_resolver
+; CHECK: @foo = ifunc i32 (), i32 ()* ()* @foo_resolver.2
+@foo = ifunc i32 (), i32 ()* ()* @foo_resolver
 
-; CHECK: define internal i32 @foo_resolver.2() {
-; CHECK-NEXT: ret i32 1
-define weak i32 @foo_resolver() {
-  ret i32 1
+; CHECK: define internal i32 ()* @foo_resolver.2() {
+; CHECK-NEXT: ret i32 ()* inttoptr (i32 1 to i32 ()*)
+define weak i32 ()* @foo_resolver() {
+  ret i32 ()* inttoptr (i32 1 to i32 ()*)
 }
 
-; CHECK: define i32 @foo_resolver() {
-; CHECK-NEXT: ret i32 2
+; CHECK: define i32 ()* @foo_resolver() {
+; CHECK-NEXT: ret i32 ()* inttoptr (i32 2 to i32 ()*)
Index: llvm/test/LTO/Resolution/X86/ifunc.ll
=

[PATCH] D112349: [Verifier] Add verification logic for GlobalIFuncs

[Itay Bookstein via Phabricator via cfe-commits]

ibookstein updated this revision to Diff 381751.
ibookstein added a comment.

rebase


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D112349/new/

https://reviews.llvm.org/D112349

Files:
  clang/lib/CodeGen/CodeGenModule.cpp
  llvm/lib/IR/Globals.cpp
  llvm/lib/IR/Verifier.cpp
  llvm/test/Assembler/ifunc-asm.ll
  llvm/test/Assembler/ifunc-dsolocal.ll
  llvm/test/Assembler/ifunc-use-list-order.ll
  llvm/test/Bindings/llvm-c/echo.ll
  llvm/test/Bitcode/compatibility-6.0.ll
  llvm/test/Bitcode/compatibility-6.0.ll.bc
  llvm/test/Bitcode/compatibility.ll
  llvm/test/Bitcode/dso_local_equivalent.ll
  llvm/test/Bitcode/dso_location.ll
  llvm/test/CodeGen/X86/addrsig.ll
  llvm/test/CodeGen/X86/dso_local_equivalent.ll
  llvm/test/CodeGen/X86/ifunc-asm.ll
  llvm/test/CodeGen/X86/partition.ll
  llvm/test/LTO/Resolution/X86/Inputs/ifunc2.ll
  llvm/test/LTO/Resolution/X86/ifunc.ll
  llvm/test/LTO/Resolution/X86/ifunc2.ll
  llvm/test/Linker/ifunc.ll
  llvm/test/Object/X86/nm-ir.ll
  llvm/test/ThinLTO/X86/empty-module.ll
  llvm/test/Transforms/GlobalDCE/global-ifunc.ll

Index: llvm/test/Transforms/GlobalDCE/global-ifunc.ll
===
--- llvm/test/Transforms/GlobalDCE/global-ifunc.ll
+++ llvm/test/Transforms/GlobalDCE/global-ifunc.ll
@@ -2,12 +2,12 @@
 target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-unknown-linux-gnu"
 
-@if = ifunc void (), void ()* @fn
+@if = ifunc void (), void ()* ()* @fn
 
-define internal void @fn() {
+define internal void ()* @fn() {
 entry:
-  ret void
+  ret void ()* null
 }
 
-; CHECK-DAG: @if = ifunc void (), void ()* @fn
-; CHECK-DAG: define internal void @fn(
+; CHECK-DAG: @if = ifunc void (), void ()* ()* @fn
+; CHECK-DAG: define internal void ()* @fn(
Index: llvm/test/ThinLTO/X86/empty-module.ll
===
--- llvm/test/ThinLTO/X86/empty-module.ll
+++ llvm/test/ThinLTO/X86/empty-module.ll
@@ -10,9 +10,9 @@
 target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-unknown-linux-gnu"
 
-@foo = ifunc i32 (i32), i64 ()* @foo_ifunc
+@foo = ifunc i32 (i32), i32 (i32)* ()* @foo_ifunc
 
-define internal i64 @foo_ifunc() {
+define internal i32 (i32)* @foo_ifunc() {
 entry:
-  ret i64 0
+  ret i32 (i32)* null
 }
Index: llvm/test/Object/X86/nm-ir.ll
===
--- llvm/test/Object/X86/nm-ir.ll
+++ llvm/test/Object/X86/nm-ir.ll
@@ -32,12 +32,12 @@
 @a1 = alias i32, i32* @g1
 @a2 = internal alias i32, i32* @g1
 
-define void @f1() {
+define void ()* @f1() {
   call void @f5()
-  ret void
+  ret void ()* null
 }
 
-@ifunc_f1 = ifunc void (), void ()* @f1
+@ifunc_f1 = ifunc void (), void ()* ()* @f1
 
 define internal void @f2() {
   ret void
Index: llvm/test/Linker/ifunc.ll
===
--- llvm/test/Linker/ifunc.ll
+++ llvm/test/Linker/ifunc.ll
@@ -3,18 +3,18 @@
 
 ;; Check that ifuncs are linked in properly.
 
-; CHECK-DAG: @foo = ifunc void (), bitcast (void ()* ()* @foo_resolve to void ()*)
+; CHECK-DAG: @foo = ifunc void (), void ()* ()* @foo_resolve
 ; CHECK-DAG: define internal void ()* @foo_resolve() {
 
-; CHECK-DAG: @bar = ifunc void (), bitcast (void ()* ()* @bar_resolve to void ()*)
+; CHECK-DAG: @bar = ifunc void (), void ()* ()* @bar_resolve
 ; CHECK-DAG: define internal void ()* @bar_resolve() {
 
 ;--- a.ll
 declare void @bar()
 
 ;--- b.ll
-@foo = ifunc void (), bitcast (void ()* ()* @foo_resolve to void ()*)
-@bar = ifunc void (), bitcast (void ()* ()* @bar_resolve to void ()*)
+@foo = ifunc void (), void ()* ()* @foo_resolve
+@bar = ifunc void (), void ()* ()* @bar_resolve
 
 define internal void ()* @foo_resolve() {
   ret void ()* null
Index: llvm/test/LTO/Resolution/X86/ifunc2.ll
===
--- llvm/test/LTO/Resolution/X86/ifunc2.ll
+++ llvm/test/LTO/Resolution/X86/ifunc2.ll
@@ -6,14 +6,14 @@
 target datalayout = "e-p:64:64"
 target triple = "x86_64-unknown-linux-gnu"
 
-; CHECK: @foo = ifunc i32 (), i32 ()* @foo_resolver.2
-@foo = ifunc i32 (), i32 ()* @foo_resolver
+; CHECK: @foo = ifunc i32 (), i32 ()* ()* @foo_resolver.2
+@foo = ifunc i32 (), i32 ()* ()* @foo_resolver
 
-; CHECK: define internal i32 @foo_resolver.2() {
-; CHECK-NEXT: ret i32 1
-define weak i32 @foo_resolver() {
-  ret i32 1
+; CHECK: define internal i32 ()* @foo_resolver.2() {
+; CHECK-NEXT: ret i32 ()* inttoptr (i32 1 to i32 ()*)
+define weak i32 ()* @foo_resolver() {
+  ret i32 ()* inttoptr (i32 1 to i32 ()*)
 }
 
-; CHECK: define i32 @foo_resolver() {
-; CHECK-NEXT: ret i32 2
+; CHECK: define i32 ()* @foo_resolver() {
+; CHECK-NEXT: ret i32 ()* inttoptr (i32 2 to i32 ()*)
Index: llvm/test/LTO/Resolution/X86/ifunc.ll
===
--- llvm/test/LTO/Resolution/X86/ifunc.ll
+

[PATCH] D112349: [Verifier] Add verification logic for GlobalIFuncs

[Itay Bookstein via Phabricator via cfe-commits]

ibookstein updated this revision to Diff 381735.
ibookstein set the repository for this revision to rG LLVM Github Monorepo.
ibookstein added a comment.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Changed type verification to look at the resolver operand rather than the 
ultimate resolver function, added comments, fixed clang CodeGenModule to give 
the correct type to the resolver operand.
Might still need to fix some clang tests.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D112349/new/

https://reviews.llvm.org/D112349

Files:
  clang/lib/CodeGen/CodeGenModule.cpp
  llvm/lib/IR/Globals.cpp
  llvm/lib/IR/Verifier.cpp
  llvm/test/Assembler/ifunc-asm.ll
  llvm/test/Assembler/ifunc-dsolocal.ll
  llvm/test/Assembler/ifunc-use-list-order.ll
  llvm/test/Bindings/llvm-c/echo.ll
  llvm/test/Bitcode/compatibility-6.0.ll
  llvm/test/Bitcode/compatibility-6.0.ll.bc
  llvm/test/Bitcode/compatibility.ll
  llvm/test/Bitcode/dso_local_equivalent.ll
  llvm/test/Bitcode/dso_location.ll
  llvm/test/CodeGen/X86/addrsig.ll
  llvm/test/CodeGen/X86/dso_local_equivalent.ll
  llvm/test/CodeGen/X86/ifunc-asm.ll
  llvm/test/CodeGen/X86/partition.ll
  llvm/test/LTO/Resolution/X86/Inputs/ifunc2.ll
  llvm/test/LTO/Resolution/X86/ifunc.ll
  llvm/test/LTO/Resolution/X86/ifunc2.ll
  llvm/test/Linker/ifunc.ll
  llvm/test/Object/X86/nm-ir.ll
  llvm/test/ThinLTO/X86/empty-module.ll
  llvm/test/Transforms/GlobalDCE/global-ifunc.ll

Index: llvm/test/Transforms/GlobalDCE/global-ifunc.ll
===
--- llvm/test/Transforms/GlobalDCE/global-ifunc.ll
+++ llvm/test/Transforms/GlobalDCE/global-ifunc.ll
@@ -2,12 +2,12 @@
 target datalayout = "e-m:e-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-unknown-linux-gnu"
 
-@if = ifunc void (), void ()* @fn
+@if = ifunc void (), void ()* ()* @fn
 
-define internal void @fn() {
+define internal void ()* @fn() {
 entry:
-  ret void
+  ret void ()* null
 }
 
-; CHECK-DAG: @if = ifunc void (), void ()* @fn
-; CHECK-DAG: define internal void @fn(
+; CHECK-DAG: @if = ifunc void (), void ()* ()* @fn
+; CHECK-DAG: define internal void ()* @fn(
Index: llvm/test/ThinLTO/X86/empty-module.ll
===
--- llvm/test/ThinLTO/X86/empty-module.ll
+++ llvm/test/ThinLTO/X86/empty-module.ll
@@ -10,9 +10,9 @@
 target datalayout = "e-m:e-p270:32:32-p271:32:32-p272:64:64-i64:64-f80:128-n8:16:32:64-S128"
 target triple = "x86_64-unknown-linux-gnu"
 
-@foo = ifunc i32 (i32), i64 ()* @foo_ifunc
+@foo = ifunc i32 (i32), i32 (i32)* ()* @foo_ifunc
 
-define internal i64 @foo_ifunc() {
+define internal i32 (i32)* @foo_ifunc() {
 entry:
-  ret i64 0
+  ret i32 (i32)* null
 }
Index: llvm/test/Object/X86/nm-ir.ll
===
--- llvm/test/Object/X86/nm-ir.ll
+++ llvm/test/Object/X86/nm-ir.ll
@@ -32,12 +32,12 @@
 @a1 = alias i32, i32* @g1
 @a2 = internal alias i32, i32* @g1
 
-define void @f1() {
+define void ()* @f1() {
   call void @f5()
-  ret void
+  ret void ()* null
 }
 
-@ifunc_f1 = ifunc void (), void ()* @f1
+@ifunc_f1 = ifunc void (), void ()* ()* @f1
 
 define internal void @f2() {
   ret void
Index: llvm/test/Linker/ifunc.ll
===
--- llvm/test/Linker/ifunc.ll
+++ llvm/test/Linker/ifunc.ll
@@ -3,18 +3,18 @@
 
 ;; Check that ifuncs are linked in properly.
 
-; CHECK-DAG: @foo = ifunc void (), bitcast (void ()* ()* @foo_resolve to void ()*)
+; CHECK-DAG: @foo = ifunc void (), void ()* ()* @foo_resolve
 ; CHECK-DAG: define internal void ()* @foo_resolve() {
 
-; CHECK-DAG: @bar = ifunc void (), bitcast (void ()* ()* @bar_resolve to void ()*)
+; CHECK-DAG: @bar = ifunc void (), void ()* ()* @bar_resolve
 ; CHECK-DAG: define internal void ()* @bar_resolve() {
 
 ;--- a.ll
 declare void @bar()
 
 ;--- b.ll
-@foo = ifunc void (), bitcast (void ()* ()* @foo_resolve to void ()*)
-@bar = ifunc void (), bitcast (void ()* ()* @bar_resolve to void ()*)
+@foo = ifunc void (), void ()* ()* @foo_resolve
+@bar = ifunc void (), void ()* ()* @bar_resolve
 
 define internal void ()* @foo_resolve() {
   ret void ()* null
Index: llvm/test/LTO/Resolution/X86/ifunc2.ll
===
--- llvm/test/LTO/Resolution/X86/ifunc2.ll
+++ llvm/test/LTO/Resolution/X86/ifunc2.ll
@@ -6,14 +6,14 @@
 target datalayout = "e-p:64:64"
 target triple = "x86_64-unknown-linux-gnu"
 
-; CHECK: @foo = ifunc i32 (), i32 ()* @foo_resolver.2
-@foo = ifunc i32 (), i32 ()* @foo_resolver
+; CHECK: @foo = ifunc i32 (), i32 ()* ()* @foo_resolver.2
+@foo = ifunc i32 (), i32 ()* ()* @foo_resolver
 
-; CHECK: define internal i32 @foo_resolver.2() {
-; CHECK-NEXT: ret i32 1
-define weak i32 @foo_resolver() {
-  ret i32 1
+; CHECK: define internal i32 ()* @foo_resolver.2() {
+; CHECK-NEXT: ret i32 ()* inttoptr (i32 1