[PATCH] D114099: Enable `_Float16` type support on X86 without the avx512fp16 flag

zahiraam added a comment. In D114099#3148665 , @zahiraam wrote: > In D114099#3148631 , > @sylvestre.ledru wrote: > >> Actually, it breaks on all Debian. >> Could you please revert it? > > Done. I have reverted

[PATCH] D114483: Add support for sycl_special_class attribute

zahiraam created this revision. zahiraam added reviewers: bader, Naghasan, keryell, Fznamznon, aaron.ballman, erichkeane. Herald added subscribers: jdoerfert, Anastasia, ebevhan, yaxunl. zahiraam requested review of this revision. Herald added a project: clang. Special classes such as accessor,

[PATCH] D114099: Enable `_Float16` type support on X86 without the avx512fp16 flag

zahiraam added a comment. In D114099#3148631 , @sylvestre.ledru wrote: > Actually, it breaks on all Debian. > Could you please revert it? Done. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D114099/new/ https://reviews.llvm.org/D114099

[PATCH] D113107: Support of expression granularity for _Float16.

zahiraam added a comment. Question about this comment: 3. “ Approaches #1 and #2 require a lot of intermediate conversions when hardware isn't available. In our example, a + b + c has to be calculated as (_Float16) ((float) (_Float16) ((float) a + (float) b) + (float) c), where the result of

[PATCH] D71016: [SYCL] Implement OpenCL kernel function generation

zahiraam added a comment. Added in the sycl_special_class attribute. This is still work in progress as I still have a few LIT tests failing and didn't address the issue of the separating sema from codegen work. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71016/new/

[PATCH] D114099: Enable `_Float16` type support on X86 without the avx512fp16 flag

zahiraam added a comment. rG6623c02d70c3 CHANGES SINCE LAST ACTION https://reviews.llvm.org/D114099/new/ https://reviews.llvm.org/D114099 ___ cfe-commits mailing list

[PATCH] D114099: Enable `_Float16` type support on X86 without the avx512fp16 flag

zahiraam updated this revision to Diff 388273. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D114099/new/ https://reviews.llvm.org/D114099 Files: clang/docs/LanguageExtensions.rst clang/docs/ReleaseNotes.rst clang/lib/Basic/Targets/X86.cpp

[PATCH] D114099: Enable `_Float16` type support on X86 without the avx512fp16 flag

zahiraam updated this revision to Diff 388189. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D114099/new/ https://reviews.llvm.org/D114099 Files: clang/docs/LanguageExtensions.rst clang/docs/ReleaseNotes.rst clang/lib/Basic/Targets/X86.cpp

[PATCH] D114099: Enable `_Float16` type support on X86 without the avx512fp16 flag

zahiraam updated this revision to Diff 388179. zahiraam marked 7 inline comments as done. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D114099/new/ https://reviews.llvm.org/D114099 Files: clang/docs/LanguageExtensions.rst clang/docs/ReleaseNotes.rst clang/lib/Basic/Targets/X86.cpp

[PATCH] D113107: Support of expression granularity for _Float16.

zahiraam added a comment. In D113107#3138415 , @rjmccall wrote: > In D113107#3137921 , @zahiraam > wrote: > >> In D113107#3136464 , @rjmccall >> wrote: >> >>> Does GCC

[PATCH] D113107: Support of expression granularity for _Float16.

zahiraam added a comment. In D113107#3136464 , @rjmccall wrote: > Does GCC actually change the formal types of expressions to `float`, or is it > doing this "no intermediate casts thing" as some sort of fp_contract-style > custom emission of trees of

[PATCH] D114099: Enable `_Float16` type support on X86 without the avx512fp16 flag

zahiraam created this revision. zahiraam added reviewers: rjmccall, pengfei, andrew.w.kaylor. zahiraam requested review of this revision. Herald added a project: clang. The _Float16 type is supported on x86 systems with SSE2 enabled. Operations are emulated by software emulation and “float”

[PATCH] D113107: Support of expression granularity for _Float16.

zahiraam added inline comments. Comment at: clang/lib/CodeGen/CGExprScalar.cpp:1315 + if ((SrcType->isHalfType() || iSFloat16Allowed) && + !CGF.getContext().getLangOpts().NativeHalfType) { // Cast to FP using the intrinsic if the half type itself isn't supported.

[PATCH] D113107: Support of expression granularity for _Float16.

zahiraam added inline comments. Comment at: clang/lib/CodeGen/CGExprScalar.cpp:1315 + if ((SrcType->isHalfType() || iSFloat16Allowed) && + !CGF.getContext().getLangOpts().NativeHalfType) { // Cast to FP using the intrinsic if the half type itself isn't supported.

[PATCH] D113107: Support of expression granularity for _Float16.

zahiraam added inline comments. Comment at: clang/test/CodeGen/X86/avx512fp16-complex.c:123 // X86-NOT: fdiv - // X86: call {{.*}} @__divhc3( + // X86: call {{.*}} @__divsc3( // X86: ret zahiraam wrote: > pengfei wrote: > > andrew.w.kaylor wrote: > > >

[PATCH] D113107: Support of expression granularity for _Float16.

zahiraam updated this revision to Diff 387387. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D113107/new/ https://reviews.llvm.org/D113107 Files: clang/lib/Basic/Targets/X86.cpp clang/lib/CodeGen/CGExprScalar.cpp clang/lib/Sema/SemaExpr.cpp

[PATCH] D107994: Making the code compliant to the documentation about Floating Point support default values for C/C++. FPP-MODEL=PRECISE enables FFP-CONTRACT (FMA is enabled).

zahiraam updated this revision to Diff 387337. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D107994/new/ https://reviews.llvm.org/D107994 Files: clang/docs/ReleaseNotes.rst Index: clang/docs/ReleaseNotes.rst === ---

[PATCH] D107994: Making the code compliant to the documentation about Floating Point support default values for C/C++. FPP-MODEL=PRECISE enables FFP-CONTRACT (FMA is enabled).

zahiraam added a comment. In D107994#3130494 , @wristow wrote: > The Release Note change here says: > > Floating Point Support in Clang > --- > - The -ffp-model=precise now implies -ffp-contract=on rather than >

[PATCH] D107994: Making the code compliant to the documentation about Floating Point support default values for C/C++. FPP-MODEL=PRECISE enables FFP-CONTRACT (FMA is enabled).

zahiraam added a comment. In D107994#3115589 , @thakis wrote: > This fixes tests on macOS, but I don't know if the space is there > intentionally to check for presence of any attributes: > > % git diff > diff --git

[PATCH] D107994: Making the code compliant to the documentation about Floating Point support default values for C/C++. FPP-MODEL=PRECISE enables FFP-CONTRACT (FMA is enabled).

zahiraam updated this revision to Diff 386279. zahiraam marked 2 inline comments as done. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D107994/new/ https://reviews.llvm.org/D107994 Files: clang/docs/ReleaseNotes.rst clang/docs/UsersManual.rst clang/lib/Driver/ToolChains/Clang.cpp

[PATCH] D107994: Making the code compliant to the documentation about Floating Point support default values for C/C++. FPP-MODEL=PRECISE enables FFP-CONTRACT (FMA is enabled).

zahiraam added inline comments. Comment at: clang/test/CodeGen/ffp-model.c:4 +// RUN: | FileCheck %s \ +// RUN: --check-prefixes=CHECK,CHECK-FAST --strict-whitespace + andrew.w.kaylor wrote: > Why did you add the strict-whitespace option? Because on some

[PATCH] D107994: Making the code compliant to the documentation about Floating Point support default values for C/C++. FPP-MODEL=PRECISE enables FFP-CONTRACT (FMA is enabled).

zahiraam updated this revision to Diff 386124. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D107994/new/ https://reviews.llvm.org/D107994 Files: clang/docs/ReleaseNotes.rst clang/docs/UsersManual.rst clang/lib/Driver/ToolChains/Clang.cpp clang/test/CodeGen/ffp-contract-option.c

[PATCH] D107994: Making the code compliant to the documentation about Floating Point support default values for C/C++. FPP-MODEL=PRECISE enables FFP-CONTRACT (FMA is enabled).

zahiraam updated this revision to Diff 386014. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D107994/new/ https://reviews.llvm.org/D107994 Files: clang/docs/ReleaseNotes.rst clang/docs/UsersManual.rst clang/lib/Driver/ToolChains/Clang.cpp clang/test/CodeGen/ffp-contract-option.c

[PATCH] D112094: Add support for floating-point option `ffp-eval-method` and for `pragma clang fp eval_method`.

zahiraam added a comment. In D112094#3087531 , @aaron.ballman wrote: > One thing I think we need additional test coverage for is constant expression > evaluation. Should the pragma impact the resulting value stored into `f`: > > constexpr long double

[PATCH] D112094: Add support for floating-point option `ffp-eval-method` and for `pragma clang fp eval_method`.

zahiraam updated this revision to Diff 382663. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112094/new/ https://reviews.llvm.org/D112094 Files: clang/docs/LanguageExtensions.rst clang/docs/UsersManual.rst clang/include/clang/Basic/FPOptions.def

[PATCH] D112094: Add support for floating-point option `ffp-eval-method` and for `pragma clang fp eval_method`.

zahiraam added inline comments. Comment at: clang/docs/LanguageExtensions.rst:3608-3609 +within a language linkage specification or at the start of a compound statement +(excluding comments). When used within a compound statement, the pragma is +active within the scope of the

[PATCH] D112094: Add support for floating-point option `ffp-eval-method` and for `pragma clang fp eval_method`.

zahiraam updated this revision to Diff 382392. zahiraam marked 6 inline comments as done. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112094/new/ https://reviews.llvm.org/D112094 Files: clang/docs/LanguageExtensions.rst clang/docs/UsersManual.rst

[PATCH] D93769: [clang] Add support for option -ffp-eval-method and extend #pragma float_control similarly

zahiraam added a comment. In D93769#2970097 , @matthewtff wrote: > This CL breaks -E flow. I've created a repro: https://pastebin.com/fFuUdsfp > If you build it on linux with ToT clang like this: clang++ -target > i386-unknown-linux-gnu repro.cc -o

[PATCH] D74436: Change clang option -ffp-model=precise to select ffp-contract=on

zahiraam added a comment. This patch had to be reverted because SPEC2006 and SPEC2017 are failing. More information here: https://bugs.llvm.org/show_bug.cgi?id=51346 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74436/new/

[PATCH] D93769: [clang] Add support for option -ffp-eval-method and extend #pragma float_control similarly

zahiraam added a comment. This patch was reverted because SPEC2006 and SPEC2017 are failing. More information here: https://bugs.llvm.org/show_bug.cgi?id=51346 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D93769/new/

[PATCH] D74436: Change clang option -ffp-model=precise to select ffp-contract=on

zahiraam added a comment. Files bug to edit the release not as suggested by @xbolva00 here: https://bugs.llvm.org/show_bug.cgi?id=51347 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D74436/new/ https://reviews.llvm.org/D74436

[PATCH] D99005: [clang] Implement P2266 Simpler implicit move

zahiraam added a comment. In D99005#2844365 , @mizvekov wrote: > In D99005#2844332 , @zahiraam wrote: > >> This change has made this snippet fail. >> https://godbolt.org/z/3ehK784hY Pass >>

[PATCH] D99005: [clang] Implement P2266 Simpler implicit move

zahiraam added a comment. This change has made this snippet fail. https://godbolt.org/z/3ehK784hY Pass https://godbolt.org/z/9q48WvsP7 fails. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D99005/new/ https://reviews.llvm.org/D99005

[PATCH] D103664: [Windows SEH]: Fix -O2 crash for Windows -EHa

zahiraam added a comment. In D103664#2799944 , @tentzen wrote: > since I cannot repro it locally, let's have this patch in to resolve -EHa -O2 > crashes for now. > I will add more -O2 tests in following patches. Sounds good to me! Repository: rG

[PATCH] D103664: [Windows SEH]: Fix -O2 crash for Windows -EHa

zahiraam added a comment. In D103664#2799852 , @tentzen wrote: > @zahiraam, are you removing all those CHECKs: > > - CHECK: invoke void @llvm.seh.scope.** > > those are placed there to ensure SEH scope semantic is preserved for Od.. I was just showing

[PATCH] D103664: [Windows SEH]: Fix -O2 crash for Windows -EHa

zahiraam added a comment. In D103664#2799725 , @tentzen wrote: > In D103664#2798732 , @aganea wrote: > >> Thanks for the quick fix! Would you mind fixing the two failing tests >> please? (see above) > > Hmm, I

[PATCH] D103664: [Windows SEH]: Fix -O2 crash for Windows -EHa

zahiraam added a comment. wco Comment at: clang/lib/Driver/ToolChains/Clang.cpp:7155 -if (EH.Asynch) - CmdArgs.push_back("-fasync-exceptions"); } Not really sure I understand this change. Isn't the case that if I compile with -EHa, I want the

[PATCH] D80344: [Windows SEH]: HARDWARE EXCEPTION HANDLING (MSVC -EHa) - Part 1

zahiraam added a comment. Actually adding -O2 to all the LIT tests in this patch will trigger the same assertion at CGCleanup.cpp:1326. The code's assuming there is always an invoke destination in the program, but in these cases the InvokeDest is null. According to Microsoft's documentation,

[PATCH] D80344: [Windows SEH]: HARDWARE EXCEPTION HANDLING (MSVC -EHa) - Part 1

zahiraam added a comment. In D80344#2796066 , @pengfei wrote: > We found another crush: https://godbolt.org/z/vcbvddEKr Even smaller https://godbolt.org/z/dbrGjGbaf Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D91089: [dllexport] Instantiate default ctor default args for explicit specializations (PR45811)

zahiraam added a comment. In D91089#2385821 , @hans wrote: >> clang.exe -c test.cpp >> Assertion failed: !hasUninstantiatedDefaultArg() && "Default argument is not >> yet instantiated!", file >>

[PATCH] D91089: [dllexport] Instantiate default ctor default args for explicit specializations (PR45811)

zahiraam added a comment. In D91089#2383433 , @hans wrote: > Please take a look. @hans Thanks for looking at this. This test case is still crashing with the patch: ksh-3.2$ cat test.cpp template class __declspec(dllexport) foo { foo(int x = 0); };

[PATCH] D90165: Clang is crashing after generating the right diagnostic for a re-declaration of a friend method - Fix for PR47544.

zahiraam created this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. zahiraam requested review of this revision. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D90165 Files: clang/lib/Parse/ParseCXXInlineMethods.cpp

[PATCH] D89960: Testing the use of arc.

zahiraam created this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. zahiraam requested review of this revision. Change-Id: I94cb7ac295acc8de9aa2c60bb31a1f5ee7d86fde Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D89960 Files:

[PATCH] D84780: Setting the /bigobj option globally for Windows debug build. https://bugs.llvm.org/show_bug.cgi?id=46733

zahiraam created this revision. Herald added subscribers: llvm-commits, lldb-commits, cfe-commits, AlexeySotkin, msifontes, jurahul, Kayjukh, grosul1, bader, Joonsoo, liufengdb, aartbik, lucyrfox, mgester, arpith-jacob, antiagainst, shauheen, jpienaar, rriddle, mehdi_amini, hiraditya, mgorny.

[PATCH] D43576: Solution to fix PR27066 - Redefinition with same mangled name as another definition (dllexport and uuid)

zahiraam updated this revision to Diff 207882. zahiraam marked 14 inline comments as done. zahiraam added a comment. Thanks for the review. I think and hope that I have responded to every issue you raised. Let me know if there are still pending issues. Happy 4th! CHANGES SINCE LAST ACTION

[PATCH] D43576: Solution to fix PR27066 - Redefinition with same mangled name as another definition (dllexport and uuid)

zahiraam added inline comments. Comment at: include/clang/AST/Decl.h:4303 + + StringLiteral *getSTLUuid() { return STLUuid; } +}; rsmith wrote: > What does "STL" mean here? Renamed it. Comment at: lib/CodeGen/CodeGenModule.cpp:1071-1073 +

[PATCH] D43576: Solution to fix PR27066 - Redefinition with same mangled name as another definition (dllexport and uuid)

zahiraam added a comment. A review please :-) Thanks. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D43576/new/ https://reviews.llvm.org/D43576 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D43576: Solution to fix PR27066 - Redefinition with same mangled name as another definition (dllexport and uuid)

zahiraam updated this revision to Diff 200484. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D43576/new/ https://reviews.llvm.org/D43576 Files: include/clang/AST/Decl.h include/clang/AST/RecursiveASTVisitor.h include/clang/Basic/Attr.td include/clang/Basic/DeclNodes.td

[PATCH] D43576: Solution to fix PR27066 - Redefinition with same mangled name as another definition (dllexport and uuid)

zahiraam added a comment. And this patch actually fixes the bug. Thanks. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D43576/new/ https://reviews.llvm.org/D43576 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D43576: Solution to fix PR27066 - Redefinition with same mangled name as another definition (dllexport and uuid)

zahiraam updated this revision to Diff 200258. zahiraam marked 2 inline comments as done. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D43576/new/ https://reviews.llvm.org/D43576 Files: include/clang/AST/Decl.h include/clang/AST/RecursiveASTVisitor.h include/clang/Basic/Attr.td

[PATCH] D43576: Solution to fix PR27066 - Redefinition with same mangled name as another definition (dllexport and uuid)

zahiraam marked 12 inline comments as done. zahiraam added a comment. @rsmith I think I have made all the changes you have pointed out to. But please note that this new patch only impements an explicit AST representation of uuid in template arguments. It **does not** fix the bug for which this

[PATCH] D43576: Solution to fix PR27066 - Redefinition with same mangled name as another definition (dllexport and uuid)

zahiraam added a comment. There are still a few things I haven't addressed yet. Mostly because not sure there is another solution like getting rid of the map from StringRef to expr. The other issue is not adding new kind to ParsedAttr. There may be another way of doing it, but didn't look at

[PATCH] D43576: Solution to fix PR27066 - Redefinition with same mangled name as another definition (dllexport and uuid)

zahiraam updated this revision to Diff 193258. zahiraam marked 15 inline comments as done. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D43576/new/ https://reviews.llvm.org/D43576 Files: include/clang/AST/Decl.h include/clang/AST/RecursiveASTVisitor.h include/clang/Basic/Attr.td

[PATCH] D43576: Solution to fix PR27066 - Redefinition with same mangled name as another definition (dllexport and uuid)

zahiraam updated this revision to Diff 191371. Herald added a subscriber: jdoerfert. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D43576/new/ https://reviews.llvm.org/D43576 Files: include/clang/AST/Decl.h include/clang/AST/RecursiveASTVisitor.h include/clang/Basic/Attr.td

[PATCH] D43576: Solution to fix PR27066 - Redefinition with same mangled name as another definition (dllexport and uuid)

zahiraam added a comment. It would be nice to have a review for this year old (updated) patch. Thanks. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D43576/new/ https://reviews.llvm.org/D43576 ___ cfe-commits mailing list

[PATCH] D45978: dllexport const variables must have external linkage.

zahiraam added a comment. In D45978#1431057 , @aaron.ballman wrote: > LGTM! Would you mind committing the changes please? Thanks. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D45978/new/ https://reviews.llvm.org/D45978

[PATCH] D45978: dllexport const variables must have external linkage.

zahiraam updated this revision to Diff 190124. zahiraam marked an inline comment as done. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D45978/new/ https://reviews.llvm.org/D45978 Files: lib/Sema/SemaDecl.cpp test/CodeGen/dllexport-1.c test/Sema/dllexport-1.cpp

[PATCH] D45978: dllexport const variables must have external linkage.

zahiraam marked 6 inline comments as done. zahiraam added inline comments. Comment at: test/SemaCXX/dllexport.cpp:72-74 +#ifndef MS namespace{ __declspec(dllexport) int InternalGlobal; } // expected-error{{'(anonymous namespace)::InternalGlobal' must have external linkage

[PATCH] D45978: dllexport const variables must have external linkage.

zahiraam added inline comments. Comment at: lib/Sema/SemaDecl.cpp:5975-5977 +if ((!ND.isExternallyVisible() && + (!isAnonymousNS || !(VD && VD->hasInit( || + (VD && VD->isStaticLocal())) { aaron.ballman wrote: > This used to unconditionally

[PATCH] D45978: dllexport const variables must have external linkage.

zahiraam updated this revision to Diff 189843. zahiraam marked 10 inline comments as done. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D45978/new/ https://reviews.llvm.org/D45978 Files: lib/Sema/SemaDecl.cpp test/CodeGen/dllexport-1.c test/Sema/dllexport-1.cpp

[PATCH] D45978: dllexport const variables must have external linkage.

zahiraam updated this revision to Diff 189659. zahiraam marked 3 inline comments as done. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D45978/new/ https://reviews.llvm.org/D45978 Files: lib/Sema/SemaDecl.cpp test/CodeGen/dllexport-1.c test/Sema/dllexport-1.cpp

[PATCH] D45978: dllexport const variables must have external linkage.

zahiraam updated this revision to Diff 188047. zahiraam marked 2 inline comments as done. Herald added subscribers: jdoerfert, jfb, mgrang, srhines. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D45978/new/ https://reviews.llvm.org/D45978 Files: lib/Sema/SemaDecl.cpp mypatch.patch

[PATCH] D45978: dllexport const variables must have external linkage.

zahiraam added a comment. Let's see if I have included every thing mentioned. Thanks. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D45978/new/ https://reviews.llvm.org/D45978 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D45978: dllexport const variables must have external linkage.

zahiraam marked an inline comment as done. zahiraam added inline comments. Comment at: lib/Sema/SemaDecl.cpp:11370 +// In Microsoft C++ mode, a const variable defined in namespace scope has +// external linkage by default if the variable is declared with

[PATCH] D45978: dllexport const variables must have external linkage.

zahiraam added a comment. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D45978/new/ https://reviews.llvm.org/D45978 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D45978: dllexport const variables must have external linkage.

zahiraam updated this revision to Diff 186641. zahiraam marked 5 inline comments as done. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D45978/new/ https://reviews.llvm.org/D45978 Files: lib/Sema/SemaDecl.cpp test/CodeGen/dllexport-1.c test/Sema/dllexport-1.cpp

[PATCH] D45978: dllexport const variables must have external linkage.

zahiraam updated this revision to Diff 186460. zahiraam marked 3 inline comments as done. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D45978/new/ https://reviews.llvm.org/D45978 Files: lib/Sema/SemaDecl.cpp test/CodeGen/dllexport.c test/Sema/dllexport-1.cpp

[PATCH] D45978: dllexport const variables must have external linkage.

zahiraam added a subscriber: z. zahiraam added inline comments. Comment at: test/Sema/dllexport-1.c:8 + +// CHECK: @y = common dso_local dllexport global i32 0, align 4 + aaron.ballman wrote: > Are x and z also exported as expected? Only x and y are exported.

[PATCH] D45978: dllexport const variables must have external linkage.

zahiraam updated this revision to Diff 186275. Herald added a subscriber: mstorsjo. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D45978/new/ https://reviews.llvm.org/D45978 Files: lib/Sema/SemaDecl.cpp test/Sema/dllexport-1.c test/Sema/dllexport-1.cpp test/Sema/dllexport-2.cpp

[PATCH] D45978: dllexport const variables must have external linkage.

zahiraam added a comment. In D45978#1392855 , @aaron.ballman wrote: > It looks like the patch got mucked up somehow, I only see three testing files > in the patch now? Oops! Sorry about that. CHANGES SINCE LAST ACTION

[PATCH] D45978: dllexport const variables must have external linkage.

zahiraam updated this revision to Diff 185920. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D45978/new/ https://reviews.llvm.org/D45978 Files: test/Sema/dllexport-1.cpp test/Sema/dllexport-2.cpp test/Sema/dllexport.c Index: test/Sema/dllexport.c

[PATCH] D45978: dllexport const variables must have external linkage.

zahiraam added a comment. In D45978#1387385 , @aaron.ballman wrote: > Can you add tests for C mode as well, as it seems the behavior differs there. Aaron, Let me know if that is enough. Thanks. CHANGES SINCE LAST ACTION

[PATCH] D45978: dllexport const variables must have external linkage.

zahiraam added a comment. > That seems like a reasonable place to try, to me. Ok. Let's see if this does the trick. Thanks. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D45978/new/ https://reviews.llvm.org/D45978 ___ cfe-commits mailing

[PATCH] D45978: dllexport const variables must have external linkage.

zahiraam updated this revision to Diff 185541. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D45978/new/ https://reviews.llvm.org/D45978 Files: lib/Sema/SemaDecl.cpp test/Sema/dllexport-1.cpp test/Sema/dllexport-2.cpp Index: test/Sema/dllexport-2.cpp

[PATCH] D45978: dllexport const variables must have external linkage.

zahiraam added a comment. In D45978#1379901 , @rnk wrote: > I'm still not sure this is the best place to make this change, but the > functionality is important. There are still unaddressed comments (no need to > check MSVCCompatibility, formatting), and

[PATCH] D45978: dllexport const variables must have external linkage.

zahiraam marked an inline comment as done. zahiraam added a comment. Aaron, Please advise. Thanks. Comment at: test/Sema/dllexport.c:70 +// const variables +__declspec(dllexport) int const x = 3; aaron.ballman wrote: > Can you think of any redeclaration

[PATCH] D43576: Solution to fix PR27066 - Redefinition with same mangled name as another definition (dllexport and uuid)

zahiraam updated this revision to Diff 146642. https://reviews.llvm.org/D43576 Files: include/clang/AST/Decl.h include/clang/AST/RecursiveASTVisitor.h include/clang/Basic/Attr.td include/clang/Basic/DeclNodes.td include/clang/Sema/AttributeList.h include/clang/Sema/Sema.h

[PATCH] D43576: Solution to fix PR27066 - Redefinition with same mangled name as another definition (dllexport and uuid)

zahiraam added a comment. Richard, Please let me know if I have answered to all the issues you raised. Thanks. https://reviews.llvm.org/D43576 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D43576: Solution to fix PR27066 - Redefinition with same mangled name as another definition (dllexport and uuid)

zahiraam updated this revision to Diff 143321. https://reviews.llvm.org/D43576 Files: include/clang/AST/Decl.h include/clang/Basic/Attr.td include/clang/Sema/AttributeList.h include/clang/Sema/Sema.h lib/AST/Decl.cpp lib/AST/DeclCXX.cpp lib/CodeGen/CGDecl.cpp

[PATCH] D43576: Solution to fix PR27066 - Redefinition with same mangled name as another definition (dllexport and uuid)

zahiraam marked 3 inline comments as done. zahiraam added inline comments. Comment at: lib/Sema/SemaDeclAttr.cpp:4937-4938 - return nullptr; -Diag(UA->getLocation(), diag::err_mismatched_uuid); -Diag(Range.getBegin(), diag::note_previous_uuid); -D->dropAttr();

[PATCH] D43576: Solution to fix PR27066 - Redefinition with same mangled name as another definition (dllexport and uuid)

zahiraam marked 4 inline comments as done. zahiraam added inline comments. Comment at: lib/Parse/ParseDecl.cpp:572 +DeclSpecUuidDecl *ArgDecl = + DeclSpecUuidDecl::Create(Actions.getASTContext(), + Actions.getFunctionLevelDeclContext(),

[PATCH] D43576: Solution to fix PR27066 - Redefinition with same mangled name as another definition (dllexport and uuid)

zahiraam updated this revision to Diff 142516. https://reviews.llvm.org/D43576 Files: include/clang/AST/Decl.h include/clang/AST/RecursiveASTVisitor.h include/clang/Basic/Attr.td include/clang/Basic/DeclNodes.td include/clang/Sema/AttributeList.h include/clang/Sema/Sema.h

[PATCH] D43576: Solution to fix PR27066 - Redefinition with same mangled name as another definition (dllexport and uuid)

zahiraam added a comment. In https://reviews.llvm.org/D43576#1016295, @majnemer wrote: > We should really, really avoid making this sort of change without first > trying to desugar uuidof into a reference to a variable. That would solve a > ton of problems, problems like this one. Not sure I

[PATCH] D44505: [MS] Always use base dtors in place of complete/vbase dtors when possible

zahiraam added a comment. LGTM 2. It fixes PR32990. https://reviews.llvm.org/D44505 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D43576: Solution to fix PR27066 - Redefinition with same mangled name as another definition (dllexport and uuid)

zahiraam added a comment. In https://reviews.llvm.org/D43576#1016588, @rsmith wrote: > In https://reviews.llvm.org/D43576#1016561, @majnemer wrote: > > > Here's my thinking: the `__uuidof` expression literally declares a variable > > called `_GUID_ddb47a6a_0f23_11d5_9109_00e0296b75d3` of type

[PATCH] D43576: Solution to fix PR27066 - Redefinition with same mangled name as another definition (dllexport and uuid)

zahiraam added a comment. In https://reviews.llvm.org/D43576#1016295, @majnemer wrote: > We should really, really avoid making this sort of change without first > trying to desugar uuidof into a reference to a variable. That would solve a > ton of problems, problems like this one. Not sure I

[PATCH] D43576: Solution to fix PR27066 - Redefinition with same mangled name as another definition (dllexport and uuid)

zahiraam updated this revision to Diff 135502. https://reviews.llvm.org/D43576 Files: test/CodeGenCXX/instantiate-uuid.cpp test/Sema/member-reference-dll.cpp Index: test/Sema/member-reference-dll.cpp === ---

[PATCH] D43576: Solution to fix PR27066 - Redefinition with same mangled name as another definition (dllexport and uuid)

zahiraam added a comment. Adding test case. https://reviews.llvm.org/D43576 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D40621: MS ABI: Treat explicit instantiation definitions of dllimport function templates as explicit instantiation decls (PR35435)

zahiraam added a comment. Looks good to me 2. Repository: rL LLVM https://reviews.llvm.org/D40621 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D37308: Interface class with uuid base record

zahiraam added a comment. MSVC and xmain compile this: struct __declspec(uuid("---C000-0046")) IUnknown {}; struct PageBase : public IUnknown {}; struct Page3 : public PageBase {}; struct Page4 : public PageBase {}; __interface PropertyPage : public Page4 {}; But MSVC

[PATCH] D37308: Interface class with uuid base record

zahiraam added a comment. Erich, The IsOrInheritsFromIUnknown function is needed. Wh Comment at: lib/Sema/SemaDeclCXX.cpp:2377 +/// \brief Tests if the __interface base is public. +static bool IsBasePublicInterface(const CXXRecordDecl *RD, +

[PATCH] D37308: Interface class with uuid base record

zahiraam updated this revision to Diff 115125. zahiraam added a comment. Hi have made all the changes requested. https://reviews.llvm.org/D37308 Files: lib/Sema/SemaDeclCXX.cpp Index: lib/Sema/SemaDeclCXX.cpp === ---

[PATCH] D37308: Interface class with uuid base record

zahiraam updated this revision to Diff 114935. zahiraam added a comment. Erich, Aaron, Please review at your convenience. Thanks. https://reviews.llvm.org/D37308 Files: lib/Sema/SemaDeclCXX.cpp test/SemaCXX/ms-uuid.cpp Index: test/SemaCXX/ms-uuid.cpp

[PATCH] D37308: Interface class with uuid base record

zahiraam updated this revision to Diff 114497. zahiraam added a comment. Erich, Addressed your comments. Thanks. https://reviews.llvm.org/D37308 Files: lib/Sema/SemaDeclCXX.cpp test/SemaCXX/ms-uuid.cpp Index: test/SemaCXX/ms-uuid.cpp

[PATCH] D37308: Interface class with uuid base record

zahiraam updated this revision to Diff 114428. zahiraam added a comment. Responding to Erich 's and Aaron's reviews. Thanks. https://reviews.llvm.org/D37308 Files: lib/Sema/SemaDeclCXX.cpp test/SemaCXX/ms-uuid.cpp Index: test/SemaCXX/ms-uuid.cpp

[PATCH] D37308: Interface class with uuid base record

zahiraam updated this revision to Diff 114201. zahiraam added a comment. Just upload a new patch. Please review. https://reviews.llvm.org/D37308 Files: lib/Sema/SemaDeclCXX.cpp test/SemaCXX/ms-uuid.cpp Index: test/SemaCXX/ms-uuid.cpp

[PATCH] D37308: Interface class with uuid base record

zahiraam added a comment. Aaron, Yes I want to this to succeed: struct __declspec(uuid("---C000-0046")) IUnknown {}; __interface ISfFileIOPropertyPage : public IUnknown {}; But I also want this to succeed: struct __declspec(uuid("---C000-0046"))

[PATCH] D37308: Interface class with uuid base record

zahiraam added a comment. That's both of the code snip-its are supposed to fail (take the diagnostic). If I remove the ClasshasAttrs() condition, from line #2459, these snip-its will pass. I was trying to explain the need for that part of the condition. https://reviews.llvm.org/D37308

[PATCH] D37308: Interface class with uuid base record

zahiraam added a comment. The test case you gave doesn't compile. It returns the diagnostic. CL has the same behavior. I don't think it is because of the dllimport. https://reviews.llvm.org/D37308 ___ cfe-commits mailing list

[PATCH] D37308: Interface class with uuid base record

zahiraam updated this revision to Diff 113387. zahiraam added a comment. Removed the helper function. If RD (base class) has uuid attribute, we want to ensure that the interface doesn't have attributes. Otherwise cases like: class __declspec(uuid("---C000-0046"))

<    1   2   3   4   5   6   >