[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. The buildbot found one more test that needed updating, that was disabled on my system. Created https://github.com/llvm/llvm-project/pull/68781 for that. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86310/new/ https://revie

[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

[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. 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 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#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 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#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#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 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#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#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#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#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-64 datalayout

hvdijk added a comment. In D86310#4501240 , @rnk wrote: > In D86310#4501170 , @hvdijk wrote: > >> For example, it would generally be better if long double were >> 8-byte-aligned, but the x86 32-bit ABI specifies th

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

hvdijk added a comment. @JohnReagan That is a valid concern, and I hope it reassures you that if things were working before, I would never be on board with this change. For example, it would generally be better if long double were 8-byte-aligned, but the x86 32-bit ABI specifies that it is 4-by

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

hvdijk added a comment. In D86310#4498575 , @efriedma wrote: > https://reviews.llvm.org/D86310#2231136 has an example where IR generated by > clang breaks. clang bases it on the data layout, so when the change here is applied, clang already generates co

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

hvdijk added a comment. In D86310#4498551 , @rnk wrote: > Given that most non-clang frontends want the bug fix (ABI break), who exactly > is asking for this level of IR ABI stability? You were, I thought, or at least that's how I interpreted your earlier

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

hvdijk added a comment. In D86310#4496582 , @nikic wrote: > The main problem with that is that we can't have multiple data layouts for > one module, so linking old and new bitcode together would fail. Good point, but it's worth pointing out that this onl

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

hvdijk added a comment. A thought occurs: in older versions of LLVM, the data layout mechanism worked differently and permitted targets to declare that they supported multiple different data layout strings, by overriding `isCompatibleDataLayout`. This mechanism was removed in D67631

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

hvdijk added a comment. In D86310#4485837 , @tmgross wrote: > What is the current status of this patch? Are the reviewers here OK with this > fix in general but just need to see changes to autoupgrade? > @craig.topper or @hvdijk since you worked on it,

[PATCH] D140315: [AMDGCN] Update search path for device libraries

hvdijk added a comment. In D140315#4077611 , @scchan wrote: > In D140315#4076614 , @mgorny wrote: > >> I'm sorry for reporting it this late (clang's been broken for over two >> weeks, so we couldn't periodically

[PATCH] D147097: [SYCL] Always set NoUnwind attribute for SYCL.

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG6b868139458d: [SYCL] Always set NoUnwind attribute for SYCL. (authored by hvdijk). Changed prior to commit: https://reviews.llvm.org/D147097?vs=50

[PATCH] D147097: [SYCL] Always set NoUnwind attribute for SYCL.

hvdijk updated this revision to Diff 509355. hvdijk added a comment. Forgot to update the call to `foo()` to call `bar()` instead. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D147097/new/ https://reviews.llvm.org/D147097 Files: clang/lib/CodeGe

[PATCH] D147097: [SYCL] Always set NoUnwind attribute for SYCL.

hvdijk updated this revision to Diff 509335. hvdijk added a comment. Apparently the test was already passing without my change, so I updated the test to make it a function that did not previously get the nounwind attribute. This also revealed that the test was more fragile than I realised: it de

[PATCH] D147097: [SYCL] Always set NoUnwind attribute for SYCL.

hvdijk added a comment. > That's a good question. I haven't looked into this issue deep enough, but I > think using -fexceptions requires using delayed diagnostics to avoid false > diagnostics during host code analysis. I am assuming you mean `-fno-exceptions` (or, in `clang -cc1`, the absence

[PATCH] D147097: [SYCL] Always set NoUnwind attribute for SYCL.

hvdijk added a comment. Is the rationale I gave in the description correct, or would it be better for SYCL device code to unconditionally build without `-fexceptions` and get the `nounwind` attribute added that way? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://rev

[PATCH] D147097: [SYCL] Always set NoUnwind attribute for SYCL.

hvdijk created this revision. hvdijk added a reviewer: bader. hvdijk added a project: clang. Herald added subscribers: Naghasan, Anastasia, ebevhan, yaxunl. Herald added a project: All. hvdijk requested review of this revision. Herald added a reviewer: jdoerfert. Herald added subscribers: cfe-commi

[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

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 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

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#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#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#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 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 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 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 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 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 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 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 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 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] 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] 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] 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] 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] 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] 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] 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] D86310: [X86] Align i128 to 16 bytes in x86-64 datalayout

hvdijk added a comment. In D86310#3226142 , @rnk wrote: > In D86310#2736983 , @hvdijk wrote: > >> There is a risk of bitcode incompatibilities with this change, but we >> already have that the code we generate now

[PATCH] D111367: [Driver] Change -dumpmachine to respect --target/LLVM_DEFAULT_TARGET_TRIPLE verbatim

hvdijk added a comment. In D111367#3050062 , @MaskRay wrote: > If GCC installation has library files under `lib/x86_64-redhat-linux`, I > think inferred `LLVM_DEFAULT_TARGET_TRIPLE` should be `x86_64-redhat-linux`, > instead of `x86_64-redhat-linux-gnu`

[PATCH] D111367: [Driver] Change -dumpmachine to respect --target/LLVM_DEFAULT_TARGET_TRIPLE verbatim

hvdijk added a comment. Does Red Hat actually use `x86_64-redhat-linux` with LLVM (other than for finding the GCC installation)? I didn't see this on Fedora, I think they use the config.guess-determined target name, and I think that's the right thing for them to do. That only affects whether yo

[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] 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] 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] 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] 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] 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] 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] 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.

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 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] 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] 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] 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#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#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. 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] 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 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 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 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] 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] 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 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

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 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 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: 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: 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 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 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 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] 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 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] 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 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#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. @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. 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] 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. 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 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#2520432 , @zequanwu wrote: > In D91913#2520414 , @hvdijk wrote: > >> In D91913#2520399 , @zequanwu wrote: >> >>> This change also breaks man

  1   2   >