[PATCH] D135171: FreeBSD: enable __float128 on x86 and powerpc64le

arichardson added a comment. In D135171#4657080 , @brad wrote: > You can close this. The submitted patch https://github.com/llvm/llvm-project/commit/23c47eba879769a29772c999be2991201c2fe399 was not the same since it omitted ppc64. So I guess this

[PATCH] D155456: [RISCV] Support -m[no-]strict-align options

arichardson added inline comments. Comment at: clang/test/Driver/riscv-features.c:41 +// DEFAULT: "-target-feature" "-unaligned-scalar-mem" +// DEFAULT-NOT: "-target-feature" "+unaligned-scalar-mem" + This looks a bit fragile, can we just check all

[PATCH] D153989: [compiler-rt] Move crt into builtins

arichardson added a comment. In D153989#4492342 , @phosek wrote: > In D153989#4492229 , @arichardson > wrote: > >> I think reduplication of build logic makes sense but these are conceptually >> separate things.

[PATCH] D153989: [compiler-rt] Move crt into builtins

arichardson added a comment. I think reduplication of build logic makes sense but these are conceptually separate things. The compiler rt builtins are needed in many cases where the C start-up code isn't (e.g. baremetal OS code). Would moving it to a subdirectory of builtins/ work too? That

[PATCH] D152285: Add support for the NO_COLOR environment variable

arichardson added a comment. >>> [...] I don't know what compelling use case there is for forcing colors >>> *on*, [...] until we know why users need to force-enable colors. >> >> The reason is that adding `-fcolor-diagnostics` to the command line is not >> always feasible, e.g. most build

[PATCH] D142934: clang: Use ptrmask for pointer alignment

arichardson added a comment. The `__builtin_align_{up,down}` code generation could also make use of this IIRC. I think the reason I didn't do this initially was concerns about ptrmask not being optimized as well. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D142934/new/

[PATCH] D144341: [Driver][FreeBSD] Correct driver behavior if a triple is provided without a version

arichardson added a comment. LGTM, but I'll let @dim and @emaste decide if FreeBSD 8 code is still needed. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D144341/new/ https://reviews.llvm.org/D144341 ___

[PATCH] D142048: [Phabricator] Fix __ptr32 arguments passed to builtins

arichardson added a comment. It sounds to me that the logic from b919c7d should have been restricted to actual LLVM intrinsics that are overloaded on the pointer args ( e.g. llvm.memcpy which was the test in that commit). Builtins that are just library calls with known semantics probably need

[PATCH] D139114: [Clang][Sema] Enabled implicit conversion warning for CompoundAssignment operator.

arichardson added inline comments. Comment at: clang/lib/Sema/SemaChecking.cpp:13402 + // intergral operands. + if (E->getLHS()->getType()->isIntegerType() && + E->getRHS()->getType()->isIntegerType() && !E->isShiftAssignOp()) Why is this restricted to

[PATCH] D140218: [update_cc_test_checks] Default to --function-signature for new tests

arichardson added a comment. In D140218#4001835 , @jdoerfert wrote: > I don't understand why we would remove the flag for _cc_. I feel all these > patches are going exactly in the opposite direction I would think one would > go. The flag is not being

[PATCH] D140218: [update_cc_test_checks] Default to --function-signature for new tests

arichardson created this revision. arichardson added reviewers: nikic, lebedev.ri, jdoerfert, spatel, sebastian-ne. Herald added a project: All. arichardson requested review of this revision. Herald added subscribers: llvm-commits, cfe-commits, sstefan1. Herald added projects: clang, LLVM. This

[PATCH] D139006: [UpdateTestChecks] Match define for labels

arichardson added a comment. I like the idea of defaulting to --function-signature for new tests a lot, so I've implemented that in D140212 Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D139006/new/

[PATCH] D138316: [ASTContext] Avoid duplicating address space map. NFCI

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rGc4c23527d6c9: [ASTContext] Avoid duplicating address space map. NFCI (authored by arichardson). Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D138316: [ASTContext] Avoid duplicating address space map. NFCI

arichardson added a comment. As this is a rather simple cleanup, I'll go ahead and merge this next week unless there is any objection until then. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D138316/new/ https://reviews.llvm.org/D138316

[PATCH] D138316: [ASTContext] Avoid duplicating address space map. NFCI

arichardson updated this revision to Diff 481664. arichardson added a comment. rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D138316/new/ https://reviews.llvm.org/D138316 Files: clang/include/clang/AST/ASTContext.h

[PATCH] D138722: Overload all llvm.annotation intrinsics for globals argument

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. arichardson marked 2 inline comments as done. Closed by commit rG9114ac67a986: Overload all llvm.annotation intrinsics for globals argument (authored by arichardson).

[PATCH] D138722: Overload all llvm.annotation intrinsics for globals argument

arichardson updated this revision to Diff 480900. arichardson added a comment. use opaque pointers in the new test Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D138722/new/ https://reviews.llvm.org/D138722 Files: clang/lib/CodeGen/CGBuiltin.cpp

[PATCH] D139305: [clang][driver] Support option '-mcpu' on target AVR

arichardson added inline comments. Comment at: clang/lib/Driver/ToolChains/CommonArgs.cpp:387 return A->getValue(); +if (const Arg *A = Args.getLastArg(options::OPT_mcpu_EQ)) + return A->getValue(); It looks to me that -mcpu will always take

[PATCH] D138296: [clang] Avoid duplicating ProgramAddressSpace in TargetInfo. NFCI

This revision was automatically updated to reflect the committed changes. Closed by commit rGf3a17d059509: [clang] Avoid duplicating ProgramAddressSpace in TargetInfo. NFCI (authored by arichardson). Changed prior to commit: https://reviews.llvm.org/D138296?vs=476473=479387#toc Repository:

[PATCH] D138296: [clang] Avoid duplicating ProgramAddressSpace in TargetInfo. NFCI

arichardson added a comment. In D138296#3937599 , @rjmccall wrote: > In D138296#3937486 , @arichardson > wrote: > >> In D138296#3937224 , @eandrews >> wrote: >> >>>

[PATCH] D138295: [clang][TargetInfo] Use LangAS for getPointer{Width,Align}()

arichardson added inline comments. Comment at: clang/lib/AST/ASTContext.cpp:2228-2230 + Width = Target->getPointerWidth( + LangAS::Default); // C++ 3.9.1p11: sizeof(nullptr_t) + Align = Target->getPointerAlign(LangAS::Default); // == sizeof(void*)

[PATCH] D138295: [clang][TargetInfo] Use LangAS for getPointer{Width,Align}()

This revision was automatically updated to reflect the committed changes. arichardson marked an inline comment as done. Closed by commit rGa602f76a2406: [clang][TargetInfo] Use LangAS for getPointer{Width,Align}() (authored by arichardson). Changed prior to commit:

[PATCH] D139006: [UpdateTestChecks] Match define for labels

arichardson added a comment. Agreed that the churn is annoying, but at least unlike the function-signature flag (which I'd quite like to have on by default tbh) it only affects a single line rather than also including variable captures. Comment at:

[PATCH] D138295: [clang][TargetInfo] Use LangAS for getPointer{Width,Align}()

arichardson added inline comments. Comment at: clang/lib/AST/RecordLayoutBuilder.cpp:1917 +EffectiveFieldSize = FieldSize = TI.Width; +FieldAlign = TI.Align; } else { rjmccall wrote: > This is a nice cleanup, but I actually can't figure out why we

[PATCH] D138316: [ASTContext] Avoid duplicating address space map. NFCI

arichardson updated this revision to Diff 478870. arichardson added a comment. rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D138316/new/ https://reviews.llvm.org/D138316 Files: clang/include/clang/AST/ASTContext.h

[PATCH] D138295: [clang][TargetInfo] Use LangAS for getPointer{Width,Align}()

arichardson updated this revision to Diff 478867. arichardson marked 3 inline comments as done. arichardson added a comment. address review feedback Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D138295/new/ https://reviews.llvm.org/D138295 Files:

[PATCH] D135171: FreeBSD: enable __float128 on x86

arichardson added a comment. @brooks do you want me to commit this for you or do you have commit access? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D135171/new/ https://reviews.llvm.org/D135171 ___

[PATCH] D138722: Overload all llvm.annotation intrinsics for globals argument

arichardson created this revision. arichardson added reviewers: arsenm, bader, Tyker, nikic. Herald added subscribers: jrtc27, hiraditya. Herald added a project: All. arichardson requested review of this revision. Herald added subscribers: llvm-commits, cfe-commits, jdoerfert, wdng. Herald added

[PATCH] D138681: [AVR] Fix broken bitcast for aliases in non-zero address space

arichardson accepted this revision. arichardson added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D138681/new/ https://reviews.llvm.org/D138681

[PATCH] D119541: [RISCV] Fix RISCVTargetInfo::initFeatureMap, add non-ISA features back after implication

arichardson added a comment. Herald added subscribers: sunshaoce, StephenFan, shiva0217. Herald added a project: All. I just noticed that target features (e.g. -mrelax) are broken in all LLVM 14 releases due to D113336 . This should have been cherry-picked

[PATCH] D138284: Fix incorrect cast in VisitSYCLUniqueStableNameExpr

This revision was automatically updated to reflect the committed changes. Closed by commit rG0745b0c0354a: Fix incorrect cast in VisitSYCLUniqueStableNameExpr (authored by arichardson). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D138284/new/

[PATCH] D138316: [ASTContext] Avoid duplicating address space map. NFCI

arichardson created this revision. arichardson added reviewers: clang, pcc. Herald added a project: All. arichardson requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. ASTContext was holding onto a pointer to the Clang->LLVM address space map

[PATCH] D138296: [clang] Avoid duplicating ProgramAddressSpace in TargetInfo. NFCI

arichardson added a comment. In D138296#3937224 , @eandrews wrote: > Functionally this looks ok to me. However I am not sure if CodeGenTypes is > the 'right' place for this function to live, considering that other functions > with similar functionality

[PATCH] D138296: [clang] Avoid duplicating ProgramAddressSpace in TargetInfo. NFCI

arichardson updated this revision to Diff 476473. arichardson added a comment. clang-format Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D138296/new/ https://reviews.llvm.org/D138296 Files: clang/include/clang/AST/ASTContext.h

[PATCH] D138295: [clang][TargetInfo] Use LangAS for getPointer{Width,Align}()

arichardson updated this revision to Diff 476472. arichardson added a comment. clang-format I could also use {} instead of `LangAS::Default`, but the latter seems more obvious Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D138295/new/

[PATCH] D138284: Fix incorrect cast in VisitSYCLUniqueStableNameExpr

arichardson updated this revision to Diff 476470. arichardson added a comment. Fix to use addrspace(1) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D138284/new/ https://reviews.llvm.org/D138284 Files: clang/lib/CodeGen/CGExprScalar.cpp

[PATCH] D138284: Fix incorrect cast in VisitSYCLUniqueStableNameExpr

arichardson added inline comments. Comment at: clang/lib/CodeGen/CGExprScalar.cpp:1635 + Context.getTargetInfo().getConstantAddressSpace().value_or( + LangAS::Default)); llvm::Constant *GlobalConstStr = Builder.CreateGlobalStringPtr( arichardson

[PATCH] D138284: Fix incorrect cast in VisitSYCLUniqueStableNameExpr

arichardson added a comment. In D138284#3936846 , @erichkeane wrote: > In D138284#3936830 , @aaron.ballman > wrote: > >> Adding @bader as SYCL code owner. The changes look reasonable to me, but >> codegen is

[PATCH] D138296: [clang] Avoid duplicating ProgramAddressSpace in TargetInfo. NFCI

arichardson created this revision. arichardson added reviewers: eandrews, dylanmckay, bader, rjmccall. Herald added subscribers: jrtc27, luismarques, s.egerton, Jim, PkmX, atanasyan, simoncook, kristof.beyls, sdardis. Herald added a project: All. arichardson requested review of this revision.

[PATCH] D138295: [clang][TargetInfo] Use LangAS for getPointer{Width,Align}()

arichardson created this revision. arichardson added a reviewer: clang. Herald added subscribers: steakhal, kosarev, mattd, gchakrabarti, asavonic, martong, kerbowa, atanasyan, jrtc27, jvesely, sdardis. Herald added a reviewer: aaron.ballman. Herald added a reviewer: NoQ. Herald added a project:

[PATCH] D138284: Fix incorrect cast in VisitSYCLUniqueStableNameExpr

arichardson created this revision. arichardson added reviewers: erichkeane, rjmccall, aaron.ballman. Herald added subscribers: Naghasan, Anastasia, ebevhan, yaxunl. Herald added a project: All. arichardson requested review of this revision. Herald added a project: clang. Herald added a subscriber:

[PATCH] D111566: [SYCL] Fix function pointer address space

arichardson added inline comments. Herald added a project: All. Comment at: clang/lib/AST/ASTContext.cpp:11579 +unsigned ASTContext::getTargetAddressSpace(QualType T) const { + return T->isFunctionType() ? getTargetInfo().getProgramAddressSpace() + :

[PATCH] D135142: Use TI.hasBuiltinAtomic() when setting ATOMIC_*_LOCK_FREE values. NFCI

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG00b1f7a6ab2a: Use TI.hasBuiltinAtomic() when setting ATOMIC_*_LOCK_FREE values. NFCI (authored by arichardson). Repository: rG LLVM Github

[PATCH] D116735: [RISCV] Adjust RV64I data layout by using n32:64 in layout string

arichardson added inline comments. Comment at: llvm/docs/ReleaseNotes.rst:122 been removed. +* n32 was added to the RV64I datalayout string. Without additional context I don't think this makes much sense to most readers. Before looking at this patch

[PATCH] D135171: FreeBSD: enable __float128 on x86

arichardson accepted this revision. arichardson added a comment. Could you add a RUN: line to `clang/test/CodeGenCXX/float128-declarations.cpp? Code LGTM. // RUN: %clang_cc1 -no-opaque-pointers -emit-llvm -triple x86_64-unknown-freebsd -std=c++11 \ // RUN: %s -o - | FileCheck %s

[PATCH] D135142: Use TI.hasBuiltinAtomic() when setting ATOMIC_*_LOCK_FREE values. NFCI

arichardson created this revision. arichardson added reviewers: rprichard, efriedma, hfinkel. Herald added a project: All. arichardson requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. I noticed that the values for

[PATCH] D44604: Make stdarg.h compatible with FreeBSD

arichardson abandoned this revision. arichardson added a comment. Herald added a subscriber: jrtc27. Herald added a project: All. Hopefully no longer required. Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D44604/new/ https://reviews.llvm.org/D44604

[PATCH] D134650: [runtimes] Remove all traces of the legacy testing configuration system

arichardson added a comment. In case anyone else runs into this: It appears this change somehow broke incremental builds with `-DLLVM_ENABLE_RUNTIMES=libunwind` (even after deleting CMakeCache.txt in the main build dir): -- Using libunwind testing configuration:

[PATCH] D134337: [clang] [Driver] More flexible rules for loading default configs

arichardson added inline comments. Comment at: clang/test/Driver/config-file3.c:27 -//--- Invocation qqq-clang-g++ tries to find config file qqq-clang-g++.cfg first. +//--- Invocation x86_64-unknown-linux-gnu-clang-g++ tries x86_64-unknown-linux-gnu-clang++.cfg first. //

[PATCH] D134337: [clang] [Driver] More flexible rules for loading default configs

arichardson added inline comments. Comment at: clang/test/Driver/config-file3.c:27 -//--- Invocation qqq-clang-g++ tries to find config file qqq-clang-g++.cfg first. +//--- Invocation x86_64-unknown-linux-gnu-clang-g++ tries x86_64-unknown-linux-gnu-clang++.cfg first. //

[PATCH] D134671: [Driver] Prevent Mips specific code from claiming -mabi argument on other targets.

arichardson accepted this revision. arichardson added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134671/new/ https://reviews.llvm.org/D134671

[PATCH] D134671: [Driver] Prevent Mips specific code from claiming -mabi argument on other targets.

arichardson added inline comments. Comment at: clang/lib/Driver/Driver.cpp:637 // accordingly to provided ABI name. - A = Args.getLastArg(options::OPT_mabi_EQ); + A = Args.getLastArgNoClaim(options::OPT_mabi_EQ); if (A && Target.isMIPS()) { Would it make

[PATCH] D108212: Emit offsetof values as notes when a static_assert fails

arichardson abandoned this revision. arichardson added a comment. Herald added a project: All. No longer needed Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D108212/new/ https://reviews.llvm.org/D108212

[PATCH] D108211: Emit sizeof/alignof values as notes when a static_assert fails

arichardson abandoned this revision. arichardson added a comment. Would require significant work to still be useful now that we print the value of expressions (e.g. only print for more complex expressions). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D132608: [CMake] Clean up CMake binary dir handling

arichardson added inline comments. Comment at: cmake/Modules/GNUBinaryDirs.cmake:6 +if (NOT DEFINED CMAKE_BINARY_BINDIR) + set(CMAKE_BINARY_BINDIR "${CMAKE_BINARY_BINDIR}/bin") +endif() I find this name a bit confusing, maybe something like `CMAKE_BUILD_BINDIR`

[PATCH] D108211: Emit sizeof/alignof values as notes when a static_assert fails

arichardson added a reviewer: tbaeder. arichardson added a comment. This is less useful now that 09117b21890c652994f7ada0229d309b35b44259 / D130894 has landed, but it might still be worth

[PATCH] D108211: Emit sizeof/alignof values as notes when a static_assert fails

arichardson updated this revision to Diff 455258. arichardson marked 2 inline comments as done. arichardson added a comment. Herald added a project: All. rebase and address feedback Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D108211/new/

[PATCH] D131155: [clang] Expand array expressions if the filler expression's filler is element dependent

arichardson added inline comments. Comment at: clang/test/SemaCXX/constexpr-array-init.cpp:3 + + +/// expected-no-diagnostics Nit: It might be helpful to add a comment to this test explaining what it's testing. This makes it easier to diagnose tests failures

[PATCH] D130089: update-test-checks: safely handle tests with #if's

arichardson accepted this revision. arichardson added inline comments. This revision is now accepted and ready to land. Comment at: llvm/test/tools/UpdateTestChecks/update_llc_test_checks/Inputs/arm_function_name.ll:2 ; Check that we accept functions with '$' in the name. ;

[PATCH] D128625: [RISCV][Driver] Fix baremetal `GCCInstallation` paths

arichardson added inline comments. Comment at: clang/lib/Driver/ToolChains/RISCVToolchain.cpp:54 + + // Set alias for "riscv{64|32}-unknown-unknown-elf" + SmallVector TripleAliases; anton-afanasyev wrote: > arichardson wrote: > > This seems like the wrong

[PATCH] D128625: [RISCV][Driver] Fix baremetal `GCCInstallation` paths

arichardson added inline comments. Comment at: clang/lib/Driver/ToolChains/RISCVToolchain.cpp:54 + + // Set alias for "riscv{64|32}-unknown-unknown-elf" + SmallVector TripleAliases; This seems like the wrong place to add this workaround, shouldn't the change

[PATCH] D122335: [clang] Emit crash reproduction as a single tar file

arichardson added inline comments. Comment at: clang/CMakeLists.txt:520 + ON "${CMAKE_BUILD_TYPE} MATCHES Debug" OFF) +if(CLANG_CRASH_SAVE_TEMPS) + add_definitions(-DCLANG_CRASH_SAVE_TEMPS) Could you add this to the config.h header

[PATCH] D122335: [clang] Emit crash reproduction as a single tar file

arichardson added a comment. In D122335#3404358 , @jrtc27 wrote: > As a developer who often deals with crashes locally this is more annoying; > currently I can just point tools at the shell script and C file in /tmp and > let them go to work reducing,

[PATCH] D109611: Fix CLANG_ENABLE_STATIC_ANALYZER=OFF building all analyzer source

arichardson planned changes to this revision. arichardson added a comment. Have to fix `cmake -GNinja -DCLANG_ENABLE_STATIC_ANALYZER=OFF -DLLVM_ENABLE_PROJECTS="llvm;clang;clang-tools-extra" -DCLANG_ENABLE_ARCMT=OFF ../llvm` Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D109611: Fix CLANG_ENABLE_STATIC_ANALYZER=OFF building all analyzer source

arichardson reopened this revision. arichardson added a comment. This revision is now accepted and ready to land. I will try to get back to this soon, but it will probably not be this week. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D109611/new/

[PATCH] D100810: [llvm] Use `GNUInstallDirs` to support custom installation dirs

arichardson added inline comments. Comment at: llvm/CMakeLists.txt:5 +include(GNUInstallDirs) + arichardson wrote: > This seems to be causing the following warning for me: > > ``` > CMake Warning (dev) at >

[PATCH] D100810: [llvm] Use `GNUInstallDirs` to support custom installation dirs

arichardson added inline comments. Comment at: llvm/CMakeLists.txt:5 +include(GNUInstallDirs) + This seems to be causing the following warning for me: ``` CMake Warning (dev) at

[PATCH] D71026: Fix LLVM_ENABLE_MODULES=ON + BUILD_SHARED_LIBS=ON build

arichardson abandoned this revision. arichardson added a comment. I believe this is no longer necessary. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71026/new/ https://reviews.llvm.org/D71026 ___

[PATCH] D102669: [analyzer][ctu] Fix wrong 'multiple definitions' errors caused by space characters in lookup names when parsing the ctu index file

arichardson added inline comments. Comment at: clang/test/Analysis/ctu-lookup-name-with-space.cpp:13 +// RUN: -verify %s + +void importee(); OikawaKirie wrote: > Adding this line here. Disabling the test on non- Linux is not a good idea IMO since it means we

[PATCH] D113372: [Driver] Add CLANG_DEFAULT_PIE_ON_LINUX to emulate GCC --enable-default-pie

arichardson added inline comments. Comment at: clang/CMakeLists.txt:232 +if(CLANG_DEFAULT_PIE_ON_LINUX) + set(CLANG_DEFAULT_PIE_ON_LINUX 1) +endif() Can these 3 lines be removed after D115751? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D110257: [CFE][Codegen] Make sure to maintain the contiguity of all the static allocas

arichardson added inline comments. Comment at: clang/lib/CodeGen/CGExpr.cpp:115 +if (AllocaInsertedAtAllocaInsertPt) + AllocaAddrSpaceInsertPt = dyn_cast(V)->getIterator(); } Shouldn't this use `cast` instead? Repository: rG LLVM Github Monorepo

[PATCH] D110612: [Utils] Use common substs in update_cc_test_checks

arichardson added reviewers: jdoerfert, ggeorgakoudis. arichardson added inline comments. Comment at: llvm/utils/update_cc_test_checks.py:223 def exec_run_line(exe): popen = subprocess.Popen(exe, stdout=subprocess.PIPE, stderr=subprocess.PIPE, universal_newlines=True)

[PATCH] D109611: Fix CLANG_ENABLE_STATIC_ANALYZER=OFF building all analyzer source

arichardson added a comment. In D109611#3010353 , @thakis wrote: > This breaks the build for us: > > Running cmake -GNinja -DCMAKE_BUILD_TYPE=Release > -DLLVM_ENABLE_ASSERTIONS=ON >

[PATCH] D109611: Fix CLANG_ENABLE_STATIC_ANALYZER=OFF building all analyzer source

This revision was automatically updated to reflect the committed changes. Closed by commit rG6d7b3d6b3a8d: Fix CLANG_ENABLE_STATIC_ANALYZER=OFF building all analyzer source (authored by arichardson). Changed prior to commit: https://reviews.llvm.org/D109611?vs=371944=373554#toc Repository:

[PATCH] D109841: Fix vtbl field addr space

arichardson accepted this revision. arichardson added a comment. This revision is now accepted and ready to land. Thanks, I can confirm that this fixes the assertions we were seeing after merging D103835 . A few minor suggestions below: There's a typo in the

[PATCH] D103835: [CUDA][HIP] Fix store of vtbl in ctor

arichardson added a comment. In D103835#2999731 , @yaxunl wrote: > In D103835#2999545 , @arichardson > wrote: > >> Merging this change broke our out-of-tree CHERI targets (and Arm Morello). >> We use

[PATCH] D103835: [CUDA][HIP] Fix store of vtbl in ctor

arichardson added a comment. Merging this change broke our out-of-tree CHERI targets (and Arm Morello). We use addrspace(200) for *all* globals (including vtables). It would have been nice if I had been added to this review considering that I added line you are changing here. If vtables are

[PATCH] D109611: Fix CLANG_ENABLE_STATIC_ANALYZER=OFF building all analyzer source

arichardson added a comment. In D109611#2997236 , @thakis wrote: > Can you just set `CLANG_TIDY_ENABLE_STATIC_ANALYZER=OFF` too if you care > about this? That doesn't have any effect on the libraries linked into clang-shlib. I am not building

[PATCH] D87118: Add an explicit toggle for the static analyzer in clang-tidy

arichardson added inline comments. Comment at: clang/lib/CMakeLists.txt:24 add_subdirectory(IndexSerialization) -if(CLANG_ENABLE_STATIC_ANALYZER) - add_subdirectory(StaticAnalyzer) arichardson wrote: > thakis wrote: > > hans wrote: > > > Why does removing the

[PATCH] D109611: Fix CLANG_ENABLE_STATIC_ANALYZER=OFF building all analyzer source

arichardson created this revision. arichardson added reviewers: thakis, hans, tstellar. Herald added subscribers: manas, steakhal, ASDenysPetrov, martong, dkrupp, donat.nagy, Szelethus, a.sidorin, baloghadamsoftware, mgorny. arichardson requested review of this revision. Herald added projects:

[PATCH] D87118: Add an explicit toggle for the static analyzer in clang-tidy

arichardson added inline comments. Herald added subscribers: manas, steakhal. Herald added a project: clang-tools-extra. Comment at: clang/lib/CMakeLists.txt:24 add_subdirectory(IndexSerialization) -if(CLANG_ENABLE_STATIC_ANALYZER) - add_subdirectory(StaticAnalyzer)

[PATCH] D105972: Fix __attribute__((annotate("")) with non-zero globals AS

This revision was automatically updated to reflect the committed changes. Closed by commit rG7cab90a7b1c4: Fix __attribute__((annotate()) with non-zero globals AS (authored by arichardson). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105972/new/

[PATCH] D108110: Fix LLVM_ENABLE_THREADS check from 26a92d5852b2c6bf77efd26f6c0194c913f40285

This revision was automatically updated to reflect the committed changes. Closed by commit rGbf66b0eefcda: Fix LLVM_ENABLE_THREADS check from 26a92d5852b2c6bf77efd26f6c0194c913f40285 (authored by arichardson). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D105972: Fix __attribute__((annotate("")) with non-zero globals AS

arichardson updated this revision to Diff 366911. arichardson added a comment. rebase Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105972/new/ https://reviews.llvm.org/D105972 Files: clang/lib/CodeGen/CodeGenModule.cpp

[PATCH] D108212: Emit offsetof values as notes when a static_assert fails

arichardson created this revision. arichardson added reviewers: courbet, Quuxplusone, aaron.ballman. arichardson requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Trying to debug a static_assert(offset(foo, field) == ...) failure can be

[PATCH] D108211: Emit sizeof/alignof values as notes when a static_assert fails

arichardson created this revision. arichardson added reviewers: courbet, Quuxplusone, aaron.ballman. arichardson requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. Trying to debug a static_assert(sizeof(foo) == ...) failure can be rather

[PATCH] D108110: Fix LLVM_ENABLE_THREADS check from 26a92d5852b2c6bf77efd26f6c0194c913f40285

arichardson created this revision. arichardson added reviewers: aaron.ballman, rsmith. Herald added a subscriber: dexonsmith. arichardson requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. We should be using #if instead of #ifdef here since

[PATCH] D95583: Frontend: Add -f{, no-}implicit-modules-uses-lock and -Rmodule-lock

arichardson added inline comments. Comment at: clang/test/Modules/implicit-modules-use-lock.m:21 + +// CHECK-NO-LOCKS-NOT: remark: +// CHECK-LOCKS: remark: locking '{{.*}}.pcm' to build module 'X' [-Rmodule-lock] jansvoboda11 wrote: > dexonsmith wrote: > >

[PATCH] D105516: [clang][PassManager] Add -falways-mem2reg to run mem2reg at -O0

arichardson added a comment. I like this since the flag significantly improves readability for `update_cc_test_checks.py`-generated Clang test without having to use the `-disable-O0-optnone | opt` trick. Not sure what the best flag name is, but as long as it's a CC1 flag it shouldn't really

[PATCH] D105972: Fix __attribute__((annotate("")) with non-zero globals AS

arichardson added a comment. ping? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105972/new/ https://reviews.llvm.org/D105972 ___ cfe-commits mailing list cfe-commits@lists.llvm.org

[PATCH] D105555: [PoC][RISCV][Clang] Compute the default target-abi if it's empty.

arichardson added inline comments. Comment at: llvm/include/llvm/Support/TargetParser.h:177 StringRef resolveTuneCPUAlias(StringRef TuneCPU, bool IsRV64); +StringRef computeABIByArch(bool HasD, bool HasE, bool IsRV64); Maybe this should be

[PATCH] D106243: [Utils] Support class template specializations in update_cc_test_checks

arichardson accepted this revision. arichardson added a comment. This revision is now accepted and ready to land. LGTM Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D106243/new/ https://reviews.llvm.org/D106243

[PATCH] D105972: Fix __attribute__((annotate("")) with non-zero globals AS

arichardson created this revision. arichardson added reviewers: rjmccall, nhaehnle, Tyker. Herald added subscribers: jrtc27, luismarques, s.egerton, PkmX, simoncook, tpr. arichardson requested review of this revision. Herald added a project: clang. Herald added a subscriber: cfe-commits. The

[PATCH] D98113: [Driver] Also search FilePaths for compiler-rt before falling back

arichardson added a comment. In D98113#2872080 , @jroelofs wrote: >> compiler-rt depends on a libc, typically newlib, which then depends on your >> compiler > > The builtins should only depend on compiler-provided headers, and not on the > rest of libc.

[PATCH] D104714: [UpdateCCTestChecks] Support --check-globals

arichardson accepted this revision. arichardson added a comment. Thanks for working on this! We have some tests downstream that check globals and currently have to use `// UTC_ARGS: --disable` to manually retain them. The other update script tests compare to an expected output file instead of

[PATCH] D100762: [clang][cli] Extract AST dump format into extra option

arichardson added a comment. In D100762#2707812 , @jansvoboda11 wrote: > If `-ast-dump=json` was a driver flag, it would be trivial to pass `-ast-dump > -ast-dump-format json` to -cc1 instead. However, aliasing a single option to > two options within

[PATCH] D100762: [clang][cli] Extract AST dump format into extra option

arichardson requested changes to this revision. arichardson added a reviewer: aaron.ballman. arichardson added a comment. I'm not sure it's a good idea to remove the `-ast-dump=json` option. While this is -cc1 option, there do seem to be external consumers based on a quick search for

[PATCH] D98286: [Driver] Enable kernel address and memory sanitizers on FreeBSD

This revision was landed with ongoing or failed builds. This revision was automatically updated to reflect the committed changes. Closed by commit rG99eca1bd9c7a: [Driver] Enable kernel address and memory sanitizers on FreeBSD (authored by markj, committed by arichardson). Repository: rG LLVM

[PATCH] D55212: Handle alloc_size attribute on function pointers

This revision was automatically updated to reflect the committed changes. Closed by commit rGdc4abca7662b: Handle alloc_size attribute on function pointers (authored by arichardson). Herald added a project: clang. Changed prior to commit: https://reviews.llvm.org/D55212?vs=177519=336519#toc

[PATCH] D100054: Handle flags such as -m32 when computing the prefix for programs/runtime libs

arichardson updated this revision to Diff 336483. arichardson added a comment. - Fix Windows path regex Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D100054/new/ https://reviews.llvm.org/D100054 Files: clang/include/clang/Driver/Driver.h

  1   2   3   4   >