[PATCH] D105191: [Clang][OpenMP] Add support for Static Device Libraries

Ye Luo via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Sep 15 07:23:20 PDT 2021


ye-luo added a comment.

> The option of adding sm_XX in Bundle Entry ID when user hasn't used -march flag, comes under command line simplification. I have a bunch of upcoming patches which will significantly simplify OpenMP command line for GPU offloading. But, don't you think this feature is different than supporting static device libraries and should be dealt separately?

Simplifying command line options is a different topic and we have not agreed upon how to do the simplification. In general, that is unrelated to this broken test case. But it can probably be fixed later with a separate patch.

> Our plan was always to support SDLs using -l and -L options, as described in Greg Rodgers's presentation about static device libraries <https://www.youtube.com/watch?v=3FsYwEhtCaM> in last year's LLVM-CTH Workshop. It was also discussed in multi company meetings. We can explore supporting direct linking of SDLs as described by you, but it seems to me that it is out of scope of this patch. What do you think?

To me, supporting both -L/-l and libXXX.a are both required for static libraries. It is not reasonable to request users to use one way not the other. AOMP had no issue working in both ways.

> PS: We (mulit-company OpenMP-dev meetings) have been tracking this feature for a while and we would very much like it to be picked for llvm-13.

Although we wanted to deliver this feature to users, I don't think we should tell users that linking static archive works with the current quality. So in my view, It is not suitable to backport it o 13.

This patch is good. It makes part of the static linking feature work but we can not claim "Static Device Libraries" is fully working. So please update the tile and also in the description saying what is working and what is not. Also need a test case for nvptx64.
In addition, I'd like to see commitment of addressing both above failed test cases before accepting this patch to help incremental development.

by the way, I found offload to x86 with static linking doesn't work with -L. -lmylib
https://github.com/ye-luo/openmp-target/blob/master/tests/linking/link_static_fat_bin/compile-x86.sh
I think this can also be addressed separately. FYI, AOMP happily makes it working.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105191



More information about the cfe-commits mailing list