[PATCH] D108421: Mark openmp internal global dso_local

2021-09-13 Thread kamlesh kumar via Phabricator via cfe-commits
kamleshbhalui added a comment. ping? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D108421/new/ https://reviews.llvm.org/D108421 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.

[PATCH] D108421: Mark openmp internal global dso_local

2021-09-07 Thread kamlesh kumar via Phabricator via cfe-commits
kamleshbhalui added a comment. ping? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D108421/new/ https://reviews.llvm.org/D108421 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.

[PATCH] D108421: Mark openmp internal global dso_local

2021-09-05 Thread kamlesh kumar via Phabricator via cfe-commits
kamleshbhalui updated this revision to Diff 370837. kamleshbhalui added a comment. mark dso_local only when non-pic Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D108421/new/ https://reviews.llvm.org/D108421 Files: clang/lib/CodeGen/CGOpenMPRunti

[PATCH] D108421: Mark openmp internal global dso_local

2021-09-02 Thread Umesh Kalappa via Phabricator via cfe-commits
umesh.kalappa0 added a comment. In D108421#2977216 , @MaskRay wrote: > In D108421#2977107 , @kamleshbhalui > wrote: > >> In D108421#2958848 , @MaskRay >> wrote: >> >>> I

[PATCH] D108421: Mark openmp internal global dso_local

2021-09-01 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay requested changes to this revision. MaskRay added a comment. This revision now requires changes to proceed. In D108421#2977107 , @kamleshbhalui wrote: > In D108421#2958848 , @MaskRay wrote: > >> If you re

[PATCH] D108421: Mark openmp internal global dso_local

2021-09-01 Thread kamlesh kumar via Phabricator via cfe-commits
kamleshbhalui added a comment. In D108421#2958848 , @MaskRay wrote: > If you read the comment in TargetMachine::shouldAssumeDSOLocal: this is the > wrong direction. dso_local is assumed to be marked by the frontend. > > Direct accesses and GOT accesses a

[PATCH] D108421: Mark openmp internal global dso_local

2021-08-24 Thread Alexey Bataev via Phabricator via cfe-commits
ABataev added a comment. In D108421#2962199 , @lebedev.ri wrote: > In D108421#2962160 , @ABataev wrote: > >> In D108421#2962151 , @lebedev.ri >> wrote: >> >>> In D108421

[PATCH] D108421: Mark openmp internal global dso_local

2021-08-24 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. In D108421#2962160 , @ABataev wrote: > In D108421#2962151 , @lebedev.ri > wrote: > >> In D108421#2961611 , >> @kamleshbhalui wrote: >> >>> up

[PATCH] D108421: Mark openmp internal global dso_local

2021-08-24 Thread Alexey Bataev via Phabricator via cfe-commits
ABataev added a comment. In D108421#2962151 , @lebedev.ri wrote: > In D108421#2961611 , @kamleshbhalui > wrote: > >> updated test and make changes local to auto generated global vars for lock. > > I do not under

[PATCH] D108421: Mark openmp internal global dso_local

2021-08-24 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. In D108421#2961611 , @kamleshbhalui wrote: > updated test and make changes local to auto generated global vars for lock. I do not understand why the most original diff here was wrong. Is is the wrong default for `getOrCreateI

[PATCH] D108421: Mark openmp internal global dso_local

2021-08-23 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:2194 + getOrCreateInternalVariable(KmpCriticalNameTy, Name)); + if (!GV->isDSOLocal()) +GV->setDSOLocal(true); MaskRay wrote: > Can be variable be preemptible on ELF? (i.e.

[PATCH] D108421: Mark openmp internal global dso_local

2021-08-23 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay added inline comments. Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:2194 + getOrCreateInternalVariable(KmpCriticalNameTy, Name)); + if (!GV->isDSOLocal()) +GV->setDSOLocal(true); Can be variable be preemptible on ELF? (i.e. default visibili

[PATCH] D108421: Mark openmp internal global dso_local

2021-08-23 Thread kamlesh kumar via Phabricator via cfe-commits
kamleshbhalui updated this revision to Diff 368261. kamleshbhalui added a comment. updated test and make changes local to auto generated global vars for lock. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D108421/new/ https://reviews.llvm.org/D10842

[PATCH] D108421: Mark openmp internal global dso_local

2021-08-21 Thread Fangrui Song via Phabricator via cfe-commits
MaskRay requested changes to this revision. MaskRay added a comment. This revision now requires changes to proceed. If you read the comment in TargetMachine::shouldAssumeDSOLocal: this is the wrong direction. dso_local is assumed to marked by the frontend. Direct accesses and GOT accesses are tr

[PATCH] D108421: Mark openmp internal global dso_local

2021-08-21 Thread kamlesh kumar via Phabricator via cfe-commits
kamleshbhalui added a comment. In D108421#2958745 , @lebedev.ri wrote: > In D108421#2958742 , @kamleshbhalui > wrote: > >> assume dso local if relocation model static > > I think these are two separate fixes. `T

[PATCH] D108421: Mark openmp internal global dso_local

2021-08-21 Thread Roman Lebedev via Phabricator via cfe-commits
lebedev.ri added a comment. In D108421#2958742 , @kamleshbhalui wrote: > assume dso local if relocation model static I think these are two separate fixes. `TargetMachine::shouldAssumeDSOLocal()` change might make sense, but as the comment there notes,

[PATCH] D108421: Mark openmp internal global dso_local

2021-08-21 Thread kamlesh kumar via Phabricator via cfe-commits
kamleshbhalui updated this revision to Diff 367954. kamleshbhalui added a comment. assume dso local if relocation model static Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D108421/new/ https://reviews.llvm.org/D108421 Files: llvm/lib/Target/Targ

[PATCH] D108421: Mark openmp internal global dso_local

2021-08-21 Thread kamlesh kumar via Phabricator via cfe-commits
kamleshbhalui updated this revision to Diff 367953. kamleshbhalui added a comment. Herald added subscribers: llvm-commits, hiraditya. Herald added a project: LLVM. updated test and make changes local to auto generated global vars for lock. Repository: rG LLVM Github Monorepo CHANGES SINCE LAS

[PATCH] D108421: Mark openmp internal global dso_local

2021-08-20 Thread Alexey Bataev via Phabricator via cfe-commits
ABataev added inline comments. Comment at: clang/lib/CodeGen/CGOpenMPRuntime.cpp:2182-2189 + llvm::GlobalVariable *GV = new llvm::GlobalVariable( + CGM.getModule(), Ty, /*IsConstant*/ false, + llvm::GlobalValue::CommonLinkage, llvm::Constant::getNullValue(Ty), + E

[PATCH] D108421: Mark openmp internal global dso_local

2021-08-19 Thread kamlesh kumar via Phabricator via cfe-commits
kamleshbhalui updated this revision to Diff 367700. kamleshbhalui added a comment. clang formatted Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D108421/new/ https://reviews.llvm.org/D108421 Files: clang/lib/CodeGen/CGOpenMPRuntime.cpp Index: c

[PATCH] D108421: Mark openmp internal global dso_local

2021-08-19 Thread kamlesh kumar via Phabricator via cfe-commits
kamleshbhalui created this revision. kamleshbhalui added reviewers: MaskRay, pengfei. kamleshbhalui added a project: OpenMP. Herald added subscribers: guansong, yaxunl. kamleshbhalui requested review of this revision. Herald added a reviewer: jdoerfert. Herald added subscribers: cfe-commits, sstefa