[PATCH] D86695: [RFC] Call ParseLangArgs for all inputs

hvdijk created this revision. Herald added subscribers: cfe-commits, atanasyan, arichardson, sdardis. Herald added a project: clang. hvdijk requested review of this revision. clang/lib/CodeGen/BackendUtil.cpp contains multiple checks for attributes of LangOpts. The option processing that sets up

[PATCH] D91913: Suppress non-conforming GNU paste extension in all standard-conforming modes

hvdijk added a comment. Ping 3 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91913/new/ https://reviews.llvm.org/D91913 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-

[PATCH] D91913: Suppress non-conforming GNU paste extension in all standard-conforming modes

hvdijk added a comment. Ping 4 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91913/new/ https://reviews.llvm.org/D91913 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-

[PATCH] D91913: Suppress non-conforming GNU paste extension in all standard-conforming modes

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGf4537935dcdb: Suppress non-conforming GNU paste extension in all standard-conforming modes (authored by hvdijk). Repository: rG LLVM Github Monore

[PATCH] D91913: Suppress non-conforming GNU paste extension in all standard-conforming modes

hvdijk added a comment. In D91913#2518448 , @rsmith wrote: > Looks like this change matches the behavior of GCC all the way back to at > least GCC 4.1. Yes, it's worked this way in GCC ever since it was first introduced back when 3.3 was in development:

[PATCH] D91913: Suppress non-conforming GNU paste extension in all standard-conforming modes

hvdijk added a comment. In D91913#2520244 , @dmajor wrote: > We have a downstream build break due to this commit. One of our files has > some convoluted arg-counting logic that now returns a different count, which > does not match gcc: https://godbolt.or

[PATCH] D91913: Suppress non-conforming GNU paste extension in all standard-conforming modes

hvdijk added a comment. Okay, GCC enables the standards-mandated behaviour with `-std=c++17 -fms-extensions` as well, but GCC doesn't have any `-fms-compatibility` option to compare. It makes sense to me for `clang -std=c++17 -fms-compatibility` to match MSVC, and MSVC supports this GCC extensi

[PATCH] D91913: Suppress non-conforming GNU paste extension in all standard-conforming modes

hvdijk added a comment. In D91913#2520399 , @zequanwu wrote: > This change also breaks many chrome ToT bots(not just windows bot), example > build: > https://ci.chromium.org/ui/p/chrome/builders/ci/ToTLinuxOfficial/10524/steps?succeeded=true&debug=false

[PATCH] D91913: Suppress non-conforming GNU paste extension in all standard-conforming modes

hvdijk added a comment. In D91913#2520432 , @zequanwu wrote: > In D91913#2520414 , @hvdijk wrote: > >> In D91913#2520399 , @zequanwu wrote: >> >>> This change also breaks man

[PATCH] D95392: Restore GNU , ## __VA_ARGS__ behavior in MSVC mode too

hvdijk created this revision. hvdijk added a reviewer: rsmith. hvdijk added a project: LLVM. hvdijk requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. As noted in D91913 , MSVC implements the GNU behavior for

[PATCH] D91913: Suppress non-conforming GNU paste extension in all standard-conforming modes

hvdijk added a comment. In D91913#2521031 , @thakis wrote: > We (and every other project out there) needs some incremental rollout plan > for this. Cut the hyperbole please, this change does not affect "every other project out there". This change makes

[PATCH] D95392: Restore GNU , ## __VA_ARGS__ behavior in MSVC mode

This revision was automatically updated to reflect the committed changes. Closed by commit rGb43c26d036dc: Restore GNU , ## __VA_ARGS__ behavior in MSVC mode (authored by hvdijk). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D95392/new/ https://revi

[PATCH] D91913: Suppress non-conforming GNU paste extension in all standard-conforming modes

hvdijk added a comment. Restoring the old behaviour for LLVM 12 may or may not help the Chrome/Chromium people: if they also regularly build with clang from git, they would need to handle this change anyway. However, if it does help, then it sounds like a good idea to me. > So we should be wor

[PATCH] D91913: Suppress non-conforming GNU paste extension in all standard-conforming modes

hvdijk added a comment. @rnk Taking it upon yourself to revert this without approval and without communication on all branches, especially given the earlier suggestion by @rsmith to only revert this on the LLVM 12 branch, is an abuse of your commit privileges as far as I am concerned. Reposit

[PATCH] D91913: Suppress non-conforming GNU paste extension in all standard-conforming modes

hvdijk added a comment. In D91913#2526117 , @rsmith wrote: > I think @rnk's observation that `__VA_OPT__` isn't actually available in any > compilation mode other than C++20 (which I hadn't previously realized) is > important here: we'd left longstanding

[PATCH] D91913: Suppress non-conforming GNU paste extension in all standard-conforming modes

hvdijk added a comment. In D91913#2526403 , @rnk wrote: > Anyway, I apologize for the misunderstanding. I'm doing my best to operate in > good faith with LLVM project policies. Hopefully you feel that you have a > path forward here. Thank you, I appreci

[PATCH] D91913: Suppress non-conforming GNU paste extension in all standard-conforming modes

hvdijk added a comment. In D91913#2526866 , @dmajor wrote: > If I'm reading git correctly, the change is still present on the 12.x branch. > Should it be reverted there too? I could have sworn that I saw it already reverted on the 12.x branch too, but I

[PATCH] D91913: Suppress non-conforming GNU paste extension in all standard-conforming modes

hvdijk created this revision. hvdijk added a reviewer: rsmith. hvdijk added a project: clang. Herald added a subscriber: cfe-commits. hvdijk requested review of this revision. The GNU token paste extension that removes the comma in , ## __VA_ARGS__ conflicts with C99/C++11's requirements when a v

[PATCH] D91913: Suppress non-conforming GNU paste extension in all standard-conforming modes

hvdijk added a comment. In D91913#2409688 , @lebedev.ri wrote: > Then the comment needs to be fixed too i would think? It's the comment right above the code that I changed; I did fix that too. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST AC

[PATCH] D52050: [Driver] Fix architecture triplets and search paths for Linux x32

hvdijk added a comment. In D52050#2415723 , @glaubitz wrote: > Yeah, @hvdijk has made multiple other improvements which should finally allow > the backend to be usable. > > We still disagree on the search paths for libraries and headers though if I > rem

[PATCH] D52050: [Driver] Fix architecture triplets and search paths for Linux x32

hvdijk added a comment. In D52050#2428560 , @glaubitz wrote: > Any idea how it could make clang default to `-mx32` if this condition is met: > `getToolChain().getTriple().getEnvironment() == llvm::Triple::GNUX32`? That sounds like it's one thing in http

[PATCH] D52050: [Driver] Fix architecture triplets and search paths for Linux x32

hvdijk added a comment. In D52050#2428693 , @glaubitz wrote: > Maybe `sys::getDefaultTargetTriple()` needs to be adjusted. I'll have a look. `sys::getDefaultTargetTriple()` defaults to `LLVM_DEFAULT_TARGET_TRIPLE`, which in turn defaults to `LLVM_HOST_TR

[PATCH] D52050: [Driver] Fix architecture triplets and search paths for Linux x32

hvdijk added a comment. In D52050#2428735 , @glaubitz wrote: > Hmm, I was pretty sure that autoconf can deal with x32 inside an x32 chroot. Most autoconf-using software won't be dealing with a copy of `config.guess` that was last updated in 2011. Updatin

[PATCH] D91913: Suppress non-conforming GNU paste extension in all standard-conforming modes

hvdijk added a comment. Ping Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91913/new/ https://reviews.llvm.org/D91913 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bi

[PATCH] D52050: [Driver] Fix architecture triplets and search paths for Linux x32

hvdijk added a comment. I've been able to check what Ubuntu 20.10 offers in terms of x32 support. Its kernel supports x32 binaries, it provides x32 versions of core system libraries in separate packages (e.g. libc6-x32, libx32stdc++6, libx32z1), and it provides a compiler that targets x32 by de

[PATCH] D91913: Suppress non-conforming GNU paste extension in all standard-conforming modes

hvdijk added a comment. Ping 2 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D91913/new/ https://reviews.llvm.org/D91913 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-

[PATCH] D52050: [Driver] Fix architecture triplets and search paths for Linux x32

hvdijk added a comment. In D52050#2466874 , @glaubitz wrote: > However, that's not the same as whether we're on an x86_64 system or on an > x32 system determines which GNU triplet to use and which include and library > search paths are our primary ones.

[PATCH] D103777: [X32] Add Triple::isX32(), use it.

hvdijk created this revision. hvdijk added reviewers: MaskRay, craig.topper. Herald added subscribers: dexonsmith, pengfei, hiraditya, dschuff. hvdijk requested review of this revision. Herald added projects: clang, LLVM. Herald added subscribers: llvm-commits, cfe-commits. So far, support for x86

[PATCH] D103777: [X32] Add Triple::isX32(), use it.

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG75521bd9d8d1: [X32] Add Triple::isX32(), use it. (authored by hvdijk). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://re

[PATCH] D52050: WIP: [Driver] Fix architecture triplets and search paths for Linux x32

hvdijk added a comment. I think the problem is actually the other thing covered before (don't provide un-normalised triples as cmake arguments), though the wrong detection of LLVM_HOST_TRIPLE will cause other issues too. If we would just update config.guess, the default configuration (not speci

[PATCH] D52050: WIP: [Driver] Fix architecture triplets and search paths for Linux x32

hvdijk commandeered this revision. hvdijk edited reviewers, added: glaubitz; removed: hvdijk. hvdijk added a comment. > Feel free to update the diff here with your suggested patch. Alright, thanks. Updating this requires me to 'commandeer' it, doing that now. Repository: rG LLVM Github Monore

[PATCH] D52050: WIP: [Driver] Fix architecture triplets and search paths for Linux x32

hvdijk updated this revision to Diff 334365. hvdijk added a comment. This updates the diff as described, plus includes test suite changes needed to make `check-clang` pass. The changes to `clang/test/Driver/Inputs/basic_cross_linux_tree/usr` are because we now correctly detect that a x86_64 GCC

[PATCH] D52050: WIP: [Driver] Fix architecture triplets and search paths for Linux x32

hvdijk added inline comments. Comment at: clang/test/Driver/baremetal.cpp:37 // CHECK-V6M-DEFAULTCXX: "{{[^"]*}}ld{{(\.(lld|bfd|gold))?}}{{(\.exe)?}}" "{{.*}}.o" "-Bstatic" -// CHECK-V6M-DEFAULTCXX-SAME: "-L{{[^"]*}}{{[/\\]+}}lib{{(64)?}}{{[/\\]+}}clang{{[/\\]+}}{{.*}}{{[/\\]+

[PATCH] D52050: WIP: [Driver] Fix architecture triplets and search paths for Linux x32

hvdijk added inline comments. Comment at: clang/test/Driver/baremetal.cpp:37 // CHECK-V6M-DEFAULTCXX: "{{[^"]*}}ld{{(\.(lld|bfd|gold))?}}{{(\.exe)?}}" "{{.*}}.o" "-Bstatic" -// CHECK-V6M-DEFAULTCXX-SAME: "-L{{[^"]*}}{{[/\\]+}}lib{{(64)?}}{{[/\\]+}}clang{{[/\\]+}}{{.*}}{{[/\\]+

[PATCH] D52050: WIP: [Driver] Fix architecture triplets and search paths for Linux x32

hvdijk added inline comments. Comment at: clang/test/Driver/baremetal.cpp:37 // CHECK-V6M-DEFAULTCXX: "{{[^"]*}}ld{{(\.(lld|bfd|gold))?}}{{(\.exe)?}}" "{{.*}}.o" "-Bstatic" -// CHECK-V6M-DEFAULTCXX-SAME: "-L{{[^"]*}}{{[/\\]+}}lib{{(64)?}}{{[/\\]+}}clang{{[/\\]+}}{{.*}}{{[/\\]+

[PATCH] D52050: WIP: [Driver] Fix architecture triplets and search paths for Linux x32

hvdijk added inline comments. Comment at: clang/test/Driver/baremetal.cpp:37 // CHECK-V6M-DEFAULTCXX: "{{[^"]*}}ld{{(\.(lld|bfd|gold))?}}{{(\.exe)?}}" "{{.*}}.o" "-Bstatic" -// CHECK-V6M-DEFAULTCXX-SAME: "-L{{[^"]*}}{{[/\\]+}}lib{{(64)?}}{{[/\\]+}}clang{{[/\\]+}}{{.*}}{{[/\\]+

[PATCH] D52050: [Driver] Fix architecture triplets and search paths for Linux x32

hvdijk updated this revision to Diff 334505. hvdijk retitled this revision from "WIP: [Driver] Fix architecture triplets and search paths for Linux x32" to "[Driver] Fix architecture triplets and search paths for Linux x32". hvdijk added a comment. Tests now updated differently, avoiding the nee

[PATCH] D52050: [Driver] Fix architecture triplets and search paths for Linux x32

hvdijk added a comment. In D52050#2662372 , @MaskRay wrote: > Mostly looks good. > >> `clang/test/Driver/Inputs/basic_cross_linux_tree/usr/lib/gcc/x86_64-unknown-linux-gnu/4.6.0/x32/crtbegin.o` > > Worth bumping the version. 4.6 is quite old and as a host

[PATCH] D52050: [Driver] Fix architecture triplets and search paths for Linux x32

hvdijk added a comment. In D52050#2662570 , @glaubitz wrote: > Since I am a big fan of consistency, I would rather leave it as is (4.6) and > then bump to 10.2.0 in a follow-up commit. You are right that including the bump in this commit would either for

[PATCH] D52050: [Driver] Fix architecture triplets and search paths for Linux x32

hvdijk updated this revision to Diff 334557. hvdijk edited the summary of this revision. hvdijk added a comment. I have also updated the summary to provide a more complete explanation of the changes, and hope the revised summary will answer @MaskRay's questions. Repository: rG LLVM Github Mon

[PATCH] D52050: [Driver] Fix architecture triplets and search paths for Linux x32

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG1d463c2a3860: [Driver] Fix architecture triplets and search paths for Linux x32 (authored by hvdijk). Changed prior to commit: https://reviews.llv

[PATCH] D52050: [Driver] Fix architecture triplets and search paths for Linux x32

hvdijk added a comment. In D52050#2663205 , @glaubitz wrote: > I think, however, we should bump the rest of the paths to 10.2.0 if possible. I updated all the Linux trees that were on 4.6.0. The only remaining 4.6.0 trees are for Hurd, which seems to me

[PATCH] D100148: [Driver] Fix compiler-rt lookup for x32

hvdijk created this revision. hvdijk added a reviewer: glaubitz. hvdijk added a project: clang. Herald added subscribers: pengfei, dberris. hvdijk requested review of this revision. Herald added a subscriber: cfe-commits. x86_64-linux-gnu and x86_64-linux-gnux32 use different ABIs and objects buil

[PATCH] D100148: [Driver] Fix compiler-rt lookup for x32

hvdijk added inline comments. Comment at: clang/test/Driver/sanitizer-ld.c:309 // CHECK-UBSAN-LINUX: "-lpthread" // RUN: %clang -fsanitize=undefined -fno-sanitize-link-runtime %s -### -o %t.o 2>&1 \ glaubitz wrote: > Do we need want to run the test for i386

[PATCH] D100148: [Driver] Fix compiler-rt lookup for x32

hvdijk added a comment. In D100148#2683325 , @glaubitz wrote: > Hi! Can we get this patch merged as-is or do we need anything else? Sorry, miscommunication. I was thinking you could use this to update D99988 to actually

[PATCH] D100148: [Driver] Fix compiler-rt lookup for x32

hvdijk added a comment. In D100148#2683748 , @glaubitz wrote: > Understood. However, I'm not really sure what else we need. Do we just take > the architecture definition from here to pass the proper flags to the > compiler or do we also need to add > so

[PATCH] D100148: [Driver] Fix compiler-rt lookup for x32

hvdijk updated this revision to Diff 359016. hvdijk edited the summary of this revision. hvdijk added a reviewer: MaskRay. hvdijk added a comment. Updated to use `Triple.isX32()`. Should we perhaps just merge this as is, ahead of the update to compiler-rt to create x32 objects? For non-x32, this

[PATCH] D100148: [Driver] Fix compiler-rt lookup for x32

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG66ab8568c485: [Driver] Fix compiler-rt lookup for x32 (authored by hvdijk). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https

[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

hvdijk added a comment. It's bad enough that this warns for #define A 2 #define B 0 int f() { return A ^ B; } where as far as the users of A are concerned, A is just some arbitrary value, it's just random chance it happens to be two and there is no chance of it being misinterpreted as exponent

[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

hvdijk added a comment. In D63423#2732158 , @Quuxplusone wrote: > In D63423#2732152 , @hvdijk wrote: > >> It's bad enough that this warns for >> >> #define A 2 >> int f() { return A ^ 1; } >> >> where as far as the

[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

hvdijk added a comment. In D63423#2732186 , @xbolva00 wrote: > I remember that was interest to support macros too :) tbh I cant remember now > such real world case where “macro ^ cst” was an issue but.. it was a long > time ago ;) > > If you are want, yo

[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

hvdijk added a comment. In D63423#2732199 , @xbolva00 wrote: > No, we want to preserve warning for 2 ^ MACRO or 10 ^ MACRO. Perhaps that should warn even if the RHS is in hex form, or is an enumerator constant, or is not even constant at all. libpng: #i

[PATCH] D63423: [Diagnostics] Diagnose misused xor as pow

hvdijk added a comment. In D63423#2732209 , @xbolva00 wrote: >>> Perhaps that should warn even if the RHS is in hex form > > It would be kinda strange, since in one clang release we ask users to silence > warning with hex form and newer release would warn

[PATCH] D86310: [X86] Align i128 to 16 bytes in x86-64 datalayout

hvdijk added a comment. Herald added a subscriber: pengfei. There is a risk of bitcode incompatibilities with this change, but we already have that the code we generate now is incompatible with GCC and results in crashes that way too, I don't think there's a perfect fix, I'd like it if we could

[PATCH] D101851: Make clangd CompletionModel not depend on directory layout.

hvdijk created this revision. hvdijk added reviewers: sammccall, usaxena95. hvdijk added a project: clang-tools-extra. Herald added subscribers: kadircet, arphaman, mgorny. hvdijk requested review of this revision. Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov. The current code acc

[PATCH] D101851: Make clangd CompletionModel not depend on directory layout.

hvdijk updated this revision to Diff 342881. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D101851/new/ https://reviews.llvm.org/D101851 Files: clang-tools-extra/clangd/quality/CompletionModel.cmake Index: clang-tools-extra/clangd/quality/Complet

[PATCH] D101851: Make clangd CompletionModel not depend on directory layout.

hvdijk marked an inline comment as done. hvdijk added inline comments. Comment at: clang-tools-extra/clangd/quality/CompletionModel.cmake:7 # namespace-qualified class name. +set(CLANGD_QUALITY_DIR ${CMAKE_CURRENT_LIST_DIR}) function(gen_decision_forest model filename cpp_class

[PATCH] D101851: Make clangd CompletionModel not depend on directory layout.

This revision was automatically updated to reflect the committed changes. Closed by commit rG7907c46fe619: Make clangd CompletionModel not depend on directory layout. (authored by hvdijk). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D101851/new/ ht

[PATCH] D100625: [clang-tools-extra] Do not use `LLVM_EXTERNAL_CLANG_TOOLS_EXTRA_SOURCE_DIR`

hvdijk added a comment. Herald added a subscriber: cfe-commits. Apparently Phabricator detected the presence of "revert D100625 " in my alternative D101851 's description and marked that as reverting this one. Of course, as thi

[PATCH] D98574: [Sparc] Define the same macros for -mcpu=v9 as GCC on Linux and the BSDs

hvdijk added inline comments. Comment at: clang/lib/Basic/Targets/Sparc.cpp:246-256 + if (getTriple().getOS() == llvm::Triple::Linux) { Builder.defineMacro("__sparc_v9__"); -Builder.defineMacro("__sparcv9__"); + } else { +Builder.defineMacro("__sparcv9"); +// S

[PATCH] D52050: WIP: [Driver] Fix architecture triplets and search paths for Linux x32

hvdijk added a comment. I may be missing something, but I do not understand the problem. What systems, other than Debian multi-arch, are you looking to also add support for? My own native x32 system uses `(/usr)/libx32` for x32 libraries. Debian uses `(/usr)/lib/x86_64-linux-gnux32`. I can unde

[PATCH] D52050: WIP: [Driver] Fix architecture triplets and search paths for Linux x32

hvdijk added a comment. I am testing the below, on top of c8e56f394af0b9e32c413d62a0e7aebbba3e6b70 , both in a Debian chroot and in my non-Debian system. Initial testing in the Debian chroot suggests that this works for simple

[PATCH] D52050: WIP: [Driver] Fix architecture triplets and search paths for Linux x32

hvdijk added a comment. I am building with cmake -G Ninja ../llvm -DCMAKE_BUILD_TYPE=Release -DLLVM_ENABLE_PROJECTS='clang;libcxx;libcxxabi' -DLLVM_HOST_TRIPLE=x86_64-pc-linux-gnux32 -DBUILD_SHARED_LIBS=ON -DLLVM_LIBDIR_SUFFIX=x32 -DLLVM_ENABLE_RTTI=ON -DLLVM_BUILD_TESTS=ON -DLLVM_TARGETS_T

[PATCH] D118935: [SYCL] Disallow explicit casts between mismatching address spaces

hvdijk added a comment. Herald added a project: All. Hi, what is the rationale here? This reuses the logic that was written for OpenCL mode, but in OpenCL mode, it was made an error with the idea that a new keyword `addrspace_cast` could be used in those cases where the user actually wants an a

[PATCH] D118935: [SYCL] Disallow explicit casts between mismatching address spaces

hvdijk added a comment. In D118935#3399095 , @Fznamznon wrote: > I was under impression that the change is small and therefore easy to > understand. Would some comment like "SYCL re-uses OpenCL mode diagnostics to > emit errors in case the cast happens

[PATCH] D122663: Mark identifier prefixes as substitutable

hvdijk added subscribers: urnathan, erichkeane. hvdijk added a comment. ping. Apologies, I don't know who to add as a reviewer, there is no one specifically listed as code owner and it does not seem to be handled by anyone in particular. @urnathan, @erichkeane, you two appear to be the most rece

[PATCH] D122663: Mark identifier prefixes as substitutable

hvdijk updated this revision to Diff 423426. hvdijk added a comment. Herald added a subscriber: dexonsmith. Allow the old mangling with suitable -fclang-abi-compat= value Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122663/new/ https://reviews.llv

[PATCH] D122663: Mark identifier prefixes as substitutable

hvdijk added a comment. In D122663#3456507 , @erichkeane wrote: > I believe the answer here is 'yes'. We also likely need release notes. Thanks for the feedback. -fclang-abi-compat= support added. Other mangling bugfixes appear to not have made it int

[PATCH] D122663: Mark identifier prefixes as substitutable

hvdijk added a comment. In D122663#3457131 , @erichkeane wrote: > Our most recent direction is to document any non-NFC patches in the release > notes if at all sensible, so I think this meets those requirements. Thanks, I'm not finding documentation of

[PATCH] D122663: Mark identifier prefixes as substitutable

hvdijk updated this revision to Diff 423445. hvdijk added a comment. Add to release notes. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122663/new/ https://reviews.llvm.org/D122663 Files: clang/docs/ReleaseNotes.rst clang/include/clang/Basic/

[PATCH] D122663: Mark identifier prefixes as substitutable

hvdijk updated this revision to Diff 423482. hvdijk added a comment. Extend test, add assert. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122663/new/ https://reviews.llvm.org/D122663 Files: clang/docs/ReleaseNotes.rst clang/include/clang/Bas

[PATCH] D122663: Mark identifier prefixes as substitutable

hvdijk marked 2 inline comments as done. hvdijk added inline comments. Comment at: clang/lib/AST/ItaniumMangle.cpp:6031 +bool CXXNameMangler::mangleSubstitution(NestedNameSpecifier *NNS) { + NNS = Context.getASTContext().getCanonicalNestedNameSpecifier(NNS); + return mangleSubs

[PATCH] D122663: Mark identifier prefixes as substitutable

hvdijk added inline comments. Comment at: clang/docs/ReleaseNotes.rst:240 + GCC. This breaks binary compatibility with code compiled with earlier versions + of clang; use the `-fclang-abi-compat=14` option to get the old mangling. This is incorrect rst synta

[PATCH] D122663: Mark identifier prefixes as substitutable

hvdijk updated this revision to Diff 423879. hvdijk added a comment. Fixed release notes to use correct RST syntax. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122663/new/ https://reviews.llvm.org/D122663 Files: clang/docs/ReleaseNotes.rst c

[PATCH] D122663: Mark identifier prefixes as substitutable

hvdijk added a comment. In D122663#3457330 , @erichkeane wrote: > LGTM! I would like @rjmccall to take a pass if he ends up having time in the > next day or two (perhaps tack on an extra day or two because of Easter), else > I'll be willing to approve

[PATCH] D122663: Mark identifier prefixes as substitutable

hvdijk added a comment. In D122663#3471741 , @erichkeane wrote: > Ping me EOW if @rsmith doesn't respond in the meantime. It is also not clear > to me whether you were able to capture/fix the issue he had with the > clang-abi-compat.cpp test. Will do

[PATCH] D122663: Mark identifier prefixes as substitutable

hvdijk added a comment. In D122663#3471763 , @erichkeane wrote: > I DID see that and am REASONABLY sure you do, but sometimes folks will ask > for test coverage because they suspect the resulting behavior will > demonstrate some sort of problem with th

[PATCH] D122663: Mark identifier prefixes as substitutable

hvdijk added a comment. In D122663#3471866 , @erichkeane wrote: > This actually wouldn't be necessary in this case: cxxfilt is already 'right', > this is just for humans to be able to see "we changed this from mangling as > to ". That's a good point.

[PATCH] D122663: Mark identifier prefixes as substitutable

hvdijk added a comment. In D122663#3471741 , @erichkeane wrote: > Ping me EOW if @rsmith doesn't respond in the meantime. ping Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D122663/new/ https://reviews.ll

[PATCH] D122663: Mark identifier prefixes as substitutable

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGfed7be096f8e: Mark identifier prefixes as substitutable (authored by hvdijk). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION htt

[PATCH] D122663: Mark identifier prefixes as substitutable

hvdijk added inline comments. Comment at: clang/lib/AST/ItaniumMangle.cpp:6031 +bool CXXNameMangler::mangleSubstitution(NestedNameSpecifier *NNS) { + NNS = Context.getASTContext().getCanonicalNestedNameSpecifier(NNS); + return mangleSubstitution(reinterpret_cast(NNS));

[PATCH] D122663: Mark identifier prefixes as substitutable

hvdijk created this revision. hvdijk added a reviewer: rsmith. hvdijk added a project: clang. Herald added a project: All. hvdijk requested review of this revision. Herald added a subscriber: cfe-commits. The Itanium C++ ABI says prefixes are substitutable. For most prefixes we already handle thi

[PATCH] D122663: Mark identifier prefixes as substitutable

hvdijk added a comment. Question: does this need to be covered by `-fclang-abi-compat=` when the prior mangling resulted in names that even llvm-cxxfilt agreed made no sense? (If it is needed, it should be an easy change.) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION http

[PATCH] D120254: [OpenCL] Align subgroup builtin guards

hvdijk added a comment. Herald added a project: All. In D120254#3342551 , @dyung wrote: > Hi, our internal release build bots are showing failures in two clang-tidy > tests that I bisected back to your commit, > clang-tidy/checkers/altera-id-dependent-b

[PATCH] D120254: [OpenCL] Align subgroup builtin guards

hvdijk added a comment. In D120254#3419369 , @svenvh wrote: > In D120254#3419221 , @hvdijk wrote: > >> The problem is that this change enables certain built-ins in OpenCL 1.2 that >> take a memory_scope argument,

[PATCH] D121097: [C++20][Modules][HU 3/5] Emit module macros for header units.

hvdijk added inline comments. Comment at: clang/test/Modules/cxx20-hu-04.cpp:22 +// RUN: %clang_cc1 -std=c++20 -emit-module-interface importer-01.cpp \ +// RUN: -fmodule-file=hu-02.pcm -o B.pcm -DTDIR=%t -verify + On Windows, when the path starts with `C:\Users\

[PATCH] D86310: [X86] Align i128 to 16 bytes in x86 datalayouts

hvdijk added a comment. In D86310#4656024 , @Fznamznon wrote: > Hi there, > > This change seems to be causing assertion failure in clang when a struct > contains a _BitInt with length longer than 128 - > https://godbolt.org/z/4jTrW4fcP . Thanks for the

[PATCH] D86310: [X86] Align i128 to 16 bytes in x86 datalayouts

hvdijk added a comment. I do not think there is a sensible way to keep [`upgrade-datalayout2.ll`](https://github.com/llvm/llvm-project/blob/main/llvm/test/Bitcode/upgrade-datalayout2.ll) working, with the way the upgrade logic is structured, and we should rethink that test. The change here inte

[PATCH] D86310: [X86] Align i128 to 16 bytes in x86 datalayouts

hvdijk added a comment. In D86310#4605475 , @tmgross wrote: > In D86310#4597359 , @hvdijk wrote: > >> In D86310#4596841 , @tmgross wrote: >> >>> I think that D158169

[PATCH] D86310: [X86] Align i128 to 16 bytes in x86 datalayouts

hvdijk added a comment. In D86310#4648446 , @tmgross wrote: > Is this just waiting for a review? Yes, I think so. Valid concerns over compatibility were raised, but now that strict compatibility with `i128` has already been broken anyway, I no longer be

[PATCH] D86310: [X86] Align i128 to 16 bytes in x86 datalayouts

hvdijk added a comment. In D86310#4648634 , @nikic wrote: > I'm happy to sign off on the x86-64 part here, but I'm less sure about > x86-32. If I understood correctly, the i128 alignment is raised there > exclusively to fix the "f128 legalized to i128 li

[PATCH] D86310: [X86] Align i128 to 16 bytes in x86 datalayouts

hvdijk added inline comments. Comment at: llvm/lib/IR/AutoUpgrade.cpp:5233 + SmallVector Groups; + Regex R("(.*-i64:64)(-.*)"); + if (R.match(Res, &Groups)) nikic wrote: > I don't think this will work for the 32-bit targets that don't have `-i64:64

[PATCH] D86310: [X86] Align i128 to 16 bytes in x86 datalayouts

hvdijk added a comment. In D86310#4516184 , @tmgross wrote: > Is the compatibility note here only meant to address calls between LLVM and > GCC, not generated code? Because of course, struct layout and pass-in-memory > function calls are incompatible. T

[PATCH] D86310: [X86] Align i128 to 16 bytes in x86 datalayouts

hvdijk added a comment. In D86310#4516876 , @pengfei wrote: > Just FYI. There are a few reports about the compatibility issues, e.g., > #41784 . Thanks. This is a case where clang breaks up `__int128` in

[PATCH] D86310: [X86] Align i128 to 16 bytes in x86 datalayouts

hvdijk added a comment. In D86310#4519549 , @tmgross wrote: > Does this happen on the clang side or the LLVM side? Definitely on the clang side, but... > I built rustc against LLVM with your patch ([link to > source](llvm.org/docs/LangRef.html#floating-

[PATCH] D86310: [X86] Align i128 to 16 bytes in x86 datalayouts

hvdijk added a comment. In D86310#4595996 , @tmgross wrote: > @nikic posted a patch that fixes the register passing at > https://reviews.llvm.org/D158169. I think that patch plus this one should > resolve all the problems we have Thanks for the link, th

[PATCH] D86310: [X86] Align i128 to 16 bytes in x86 datalayouts

hvdijk added a comment. In D86310#4596712 , @tmgross wrote: > Is clang still doing something wrong? From my testing, it seems like clang > and GCC now agree with each other, I am not sure what would still be incorrect My understanding is that the code cl

[PATCH] D86310: [X86] Align i128 to 16 bytes in x86 datalayouts

hvdijk added a comment. In D86310#4596932 , @tmgross wrote: > Was your failure in https://bugs.llvm.org/show_bug.cgi?id=50198 fixed with > these patches? Yes, it was (at least it was at the time that I initially commented). > I cannot reproduce that fai

[PATCH] D86310: [X86] Align i128 to 16 bytes in x86 datalayouts

hvdijk added inline comments. Comment at: llvm/lib/IR/AutoUpgrade.cpp:5233 + SmallVector Groups; + Regex R("(.*-i64:64)(-.*)"); + if (R.match(Res, &Groups)) hvdijk wrote: > nikic wrote: > > I don't think this will work for the 32-bit targets that d

[PATCH] D86310: [X86] Align i128 to 16 bytes in x86 datalayouts

hvdijk added a comment. In D86310#4653550 , @tmgross wrote: > Probably would be good to add https://bugs.llvm.org/show_bug.cgi?id=50198 to > the test suite if it isn't there already. That test would not work as an LLVM test directly, but we do already ha

  1   2   >