[PATCH] D80947: Add to the Coding Standard our that single-line bodies omit braces

hubert.reinterpretcast added inline comments. Comment at: llvm/docs/CodingStandards.rst:1578 +unnecessary and otherwise meaningless code. Braces should be used however +in cases where it significantly improves readability, such as when the single +statement is accompanied by a

[PATCH] D80947: Add to the Coding Standard our that single-line bodies omit braces

hubert.reinterpretcast added inline comments. Comment at: llvm/docs/CodingStandards.rst:1582 +clear that it is a single line. Note that comments should only be hoisted for loops and +'if', and not in 'else if' or 'else', where it would be unclear whether the comment +belonged

[PATCH] D80947: Add to the Coding Standard our that single-line bodies omit braces

hubert.reinterpretcast added inline comments. Comment at: llvm/docs/CodingStandards.rst:1580 +statement is accompanied by a comment that loses its meaning if hoisted above the if +or loop statement, or where the single statement is complex enough that it stops being +clear

[PATCH] D80300: [Driver] Add DEFAULT_DYLD_PREFIX and DEFAULT_RPATH to complement DEFAULT_SYSROOT

hubert.reinterpretcast marked 3 inline comments as done. hubert.reinterpretcast added inline comments. Comment at: clang/lib/Driver/ToolChains/Gnu.cpp:452 CmdArgs.push_back("-dynamic-linker"); - CmdArgs.push_back(Args.MakeArgString(Loader)); +

[PATCH] D72736: [AIX] Add improved interface for retrieving load module paths

hubert.reinterpretcast added inline comments. Comment at: llvm/lib/Support/Unix/Path.inc:243 +#elif defined(_AIX) + return getLoadModuleFilenameForFunction(MainAddr)->c_str(); #elif defined(__linux__) || defined(__CYGWIN__) || defined(__gnu_hurd__)

[PATCH] D80532: [NFC] Fix formatting for the 'aix-ld.c' test case.

hubert.reinterpretcast accepted this revision. hubert.reinterpretcast 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/D80532/new/ https://reviews.llvm.org/D80532

[PATCH] D80300: [Driver] Add DEFAULT_DYLD_PREFIX and DEFAULT_RPATH to complement DEFAULT_SYSROOT

hubert.reinterpretcast added a comment. @joerg, are you okay with this patch landing? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D80300/new/ https://reviews.llvm.org/D80300 ___ cfe-commits mailing

[PATCH] D80532: [NFC] Fix formatting for the 'aix-ld.c' test case.

hubert.reinterpretcast marked an inline comment as done. hubert.reinterpretcast added a comment. LGTM with minor comment. Comment at: clang/test/Driver/aix-ld.c:8 -// RUN: --sysroot %S/Inputs/aix_ppc_tree \ -// RUN: | FileCheck --check-prefix=CHECK-LD32 %s //

[PATCH] D80417: Fix Darwin 'constinit thread_local' variables.

hubert.reinterpretcast added subscribers: rsmith, hubert.reinterpretcast. hubert.reinterpretcast added inline comments. Comment at: clang/lib/CodeGen/CodeGenModule.cpp:4145 + // referenced directly, without calling the thread-wrapper, so the linkage + // must not be changed. +

[PATCH] D80415: [AIX] Add '-bcdtors:all:0:s' to linker to gather static init functions

hubert.reinterpretcast accepted this revision. hubert.reinterpretcast added a comment. This revision is now accepted and ready to land. Thanks. LGTM with minor nit. In D80415#2051772 , @stevewan wrote: > I'm planning to post an NFC patch after this to

[PATCH] D80415: [AIX] Add '-bcdtors:all:0:s' to linker to gather static init functions

hubert.reinterpretcast added inline comments. Comment at: clang/test/Driver/aix-ld.cpp:2 +// Check powerpc-ibm-aix7.1.0.0, 32-bit. 'bcdtors' and argument order. +// RUN: %clangxx -no-canonical-prefixes %s 2>&1 -### \ +// RUN: -Wl,-bnocdtors \ stevewan

[PATCH] D80415: [AIX] Add '-bcdtors:all:0:s' to linker to gather static init functions

hubert.reinterpretcast added inline comments. Comment at: clang/test/Driver/aix-ld.cpp:2 +// Check powerpc-ibm-aix7.1.0.0, 32-bit. 'bcdtors' and argument order. +// RUN: %clangxx -no-canonical-prefixes %s 2>&1 -### \ +// RUN: -Wl,-bnocdtors \ I am

[PATCH] D80415: [AIX] Add '-bcdtors:all:0:s' to linker to gather static init functions

hubert.reinterpretcast added inline comments. Comment at: clang/test/Driver/aix-ld.cpp:18 +// CHECK-LD32-ARG-ORDER: "-bnocdtors" +// CHECK-LD32-ARG-ORDER: "-L[[SYSROOT]]/usr/lib" +// CHECK-LD32-ARG-ORDER: "-lc" The main point of the test being that

[PATCH] D80415: [AIX] Add '-bcdtors:all:0:s' to linker to gather static init functions

hubert.reinterpretcast added inline comments. Comment at: clang/test/Driver/aix-ld.cpp:9 +// CHECK-LD32-ARG-ORDER-NOT: warning: +// CHECK-LD32-ARG-ORDER: {{.*}}clang{{(.exe)?}}" "-cc1" "-triple" "powerpc-ibm-aix7.1.0.0" +// CHECK-LD32-ARG-ORDER: "-isysroot"

[PATCH] D80415: [AIX] Add '-bcdtors:all:0:s' to linker to gather static init functions

hubert.reinterpretcast added inline comments. Comment at: clang/test/Driver/aix-ld.cpp:1 +// Check powerpc-ibm-aix7.1.0.0, 32-bit. 'bcdtors' and Arguemnt order. +// // RUN: %clang++ -no-canonical-prefixes %s -### -o %t.o 2>&1 \ ZarkoCA wrote: >

[PATCH] D80300: [Driver] Add DEFAULT_DYLD_PREFIX and DEFAULT_RPATH to complement DEFAULT_SYSROOT

hubert.reinterpretcast added a comment. In D80300#2049679 , @joerg wrote: > So the general idea is that for turnkey toolchains, > we want to allow customizing the "default" target of the toolchain to > hard-code options like --sysroot, --target,

[PATCH] D80300: [Driver] Add DEFAULT_DYLD_PREFIX and DEFAULT_RPATH to complement DEFAULT_SYSROOT

hubert.reinterpretcast added a comment. In D80300#2049181 , @joerg wrote: > It doesn't seem to work well with the cross-compiling support in the driver > as clearly shown by the amount of tests that need patching. I don't see the test changes as

[PATCH] D80300: [Driver] Add DEFAULT_DYLD_PREFIX and DEFAULT_RPATH to complement DEFAULT_SYSROOT

hubert.reinterpretcast added inline comments. Comment at: clang/lib/Driver/ToolChains/Gnu.cpp:452 CmdArgs.push_back("-dynamic-linker"); - CmdArgs.push_back(Args.MakeArgString(Loader)); + CmdArgs.push_back(Args.MakeArgString(Twine(D.DyldPrefix) + +

[PATCH] D80300: [Driver] Add DEFAULT_DYLD_PREFIX and DEFAULT_RPATH to complement DEFAULT_SYSROOT

hubert.reinterpretcast created this revision. hubert.reinterpretcast added reviewers: nemanjai, jsji, daltenty, stevewan, jasonliu. Herald added subscribers: atanasyan, jrtc27, mgorny. Herald added a project: clang. hubert.reinterpretcast edited the summary of this revision. Some target

[PATCH] D74166: [AIX][Frontend] Static init implementation for AIX considering no priority

hubert.reinterpretcast added inline comments. Comment at: clang/test/CodeGen/static-init.cpp:14 +// CHECK: @llvm.global_dtors = appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 65535, void ()* @__sterm8000_clang_b2e4830f1c9d2d063e5ea946868f3bfd,

[PATCH] D79694: [tests][Driver] Set `--sysroot=""` to allow `DEFAULT_SYSROOT` build

This revision was automatically updated to reflect the committed changes. Closed by commit rG15f0f824b36e: [tests][Driver] Set `--sysroot=` to allow `DEFAULT_SYSROOT` build (authored by hubert.reinterpretcast). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D79693: [test][ARM][CMSE] Use clang_cc1 in arm_cmse.h tests

This revision was automatically updated to reflect the committed changes. Closed by commit rG3f5fc73a9d52: [test][ARM][CMSE] Use clang_cc1 in arm_cmse.h tests (authored by hubert.reinterpretcast). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D79693: [test][ARM][CMSE] Use clang_cc1 in arm_cmse.h tests

hubert.reinterpretcast updated this revision to Diff 264149. hubert.reinterpretcast added a comment. - Use clang_cc1 in arm_cmse.h tests Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79693/new/ https://reviews.llvm.org/D79693 Files:

[PATCH] D79693: [test][ARM][CMSE] Use clang_cc1 in arm_cmse.h tests

hubert.reinterpretcast added a comment. In D79693#2036710 , @chill wrote: > I can also count (`grep -rn '#include.* standard headers. If they don't create a problem, wouldn't it be better to > rewrite this test to use `%clang_cc1` ? @chill, thanks for

[PATCH] D79693: [test][ARM][CMSE] Use -ffreestanding for arm_cmse.h tests

hubert.reinterpretcast added a comment. In D79693#2036599 , @chill wrote: > How do you trigger a problem? If your build was configured with `DEFAULT_SYSROOT` to a toolchain path that has a `/include` directory, then the contents of that directory would

[PATCH] D79693: [test][ARM][CMSE] Use -ffreestanding for arm_cmse.h tests

hubert.reinterpretcast added a comment. Ping, @chill. It seems you checked these files in under D70817 . The way they were written has issues as indicated by the patch description. Please take a look; thanks. Repository: rG LLVM Github Monorepo CHANGES

[PATCH] D79694: [tests][Driver] Set `--sysroot=""` to allow `DEFAULT_SYSROOT` build

hubert.reinterpretcast created this revision. hubert.reinterpretcast added reviewers: broadwaylamb, sepavloff, jyknight, mstorsjo. Herald added a project: clang. If `DEFAULT_SYSROOT` is configured to some path, some tests would fail. This patch overrides `sysroot` to be the empty string in the

[PATCH] D79693: [test][ARM][CMSE] Use -ffreestanding for arm_cmse.h tests

hubert.reinterpretcast created this revision. hubert.reinterpretcast added reviewers: chill, dmgreen. Herald added subscribers: danielkiss, kristof.beyls. Herald added a project: clang. The `arm_cmse.h` header includes standard headers, but some tests that include this header explicitly specify

[PATCH] D79044: [AIX] Avoid structor alias; die before bad alias codegen

This revision was automatically updated to reflect the committed changes. Closed by commit rGb116ded57da3: [AIX] Avoid structor alias; die before bad alias codegen (authored by hubert.reinterpretcast). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D79290: Update suffix check and cast non-suffix types

hubert.reinterpretcast added a comment. In D79290#2016608 , @v.g.vassilev wrote: > @hubert.reinterpretcast, this patch is part of our internal forks which we > would like to put upstream. The author of the previous patch does not have > bandwidth to

[PATCH] D79290: Update suffix check and cast non-suffix types

hubert.reinterpretcast added a comment. In D79290#2016278 , @reikdas wrote: > In D79290#2016273 , > @hubert.reinterpretcast wrote: > > > @reikdas, please upload the full copy of your combined changes to D77598 >

[PATCH] D76932: [AIX] emit .extern and .weak directive linkage

hubert.reinterpretcast accepted this revision. hubert.reinterpretcast added a comment. LGTM with minor comment. Comment at: llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp:2143 + case GlobalValue::AvailableExternallyLinkage: +report_fatal_error("Unhandled

[PATCH] D78350: [AST] Build recovery expressions by default for C++.

hubert.reinterpretcast added a comment. In D78350#2007065 , @hokein wrote: > In D78350#2006469 , > @hubert.reinterpretcast wrote: > > > Got it. I'll put together a build. > > > Thank you! Look forward to the

[PATCH] D79033: [NFC][clang] Replace raw new/delete with unique_ptr to store ABIInfo in TargetCodeGenInfo

hubert.reinterpretcast added a comment. Funny that it's the front-end code that this patch makes more C++11 after so many years. LGTM; thanks. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79033/new/ https://reviews.llvm.org/D79033 ___

[PATCH] D79044: [AIX] Avoid structor alias; die before bad alias codegen

hubert.reinterpretcast created this revision. hubert.reinterpretcast added reviewers: cebowleratibm, jasonliu, sfertile, daltenty, DiggerLin. Herald added subscribers: kbarton, hiraditya, nemanjai. Herald added projects: clang, LLVM. hubert.reinterpretcast edited the summary of this revision.

[PATCH] D79033: [NFC][clang] Replace raw new/delete with unique_ptr to store ABIInfo in TargetCodeGenInfo

hubert.reinterpretcast added inline comments. Comment at: clang/lib/CodeGen/TargetInfo.h:51 - TargetCodeGenInfo(ABIInfo *info = nullptr) : Info(info) {} - virtual ~TargetCodeGenInfo(); I'm not sure removing a virtual destructor is a good idea. The use of

[PATCH] D78938: Fixing all comparisons for C++20 compilation.

hubert.reinterpretcast added a comment. In D78938#2006715 , @BRevzin wrote: > Wtf, where'd my other changes go? I've hit this before, use `arc diff --update D78938 `. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D78350: [AST] Build recovery expressions by default for C++.

hubert.reinterpretcast added a comment. In D78350#1988416 , @hokein wrote: > @ebevhan, @hubert.reinterpretcast, the patch is based on > fd7a34186137168064ffe2ca536823559b92d939 > , it

[PATCH] D76932: [AIX] emit .extern and .weak directive linkage

hubert.reinterpretcast accepted this revision. hubert.reinterpretcast added a comment. LGTM with minor comment. Comment at: llvm/test/CodeGen/PowerPC/aix-extern-weak.ll:8 +; RUN: llc -verify-machineinstrs -mtriple powerpc-ibm-aix-xcoff -mcpu=pwr4 \ +; RUN: -mattr=-altivec < %s

[PATCH] D76932: [AIX] emit .extern and .weak directive linkage

hubert.reinterpretcast added inline comments. Comment at: llvm/test/CodeGen/PowerPC/aix-extern-weak.ll:174 +; CHECKSYM-NEXT: Index: [[#Index+11]] +; CHECKSYM-NEXT: ContainingCsectSymbolIndex: 8 +; CHECKSYM-NEXT: ParameterHashIndex: 0x0 This

[PATCH] D76932: [AIX] emit .extern and .weak directive linkage

hubert.reinterpretcast added inline comments. Comment at: llvm/test/CodeGen/PowerPC/aix-extern-weak.ll:9 +; RUN: -mattr=-altivec -filetype=obj -o %t.o < %s +; RUN: llvm-readobj --symbols %t.o | FileCheck --check-prefix=CHECKSYM %s + No need for two consecutive

[PATCH] D76932: [AIX] emit .extern and .weak directive linkage

hubert.reinterpretcast added inline comments. Comment at: llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp:1509 + +MCSymbol *Name = getSymbol(); +if (TM.getTargetTriple().isOSBinFormatXCOFF() && !F.isIntrinsic()) { Add a comment that this gives us the function

[PATCH] D77168: Add a flag to debug automatic variable initialization

hubert.reinterpretcast added a comment. In D77168#1989973 , @jfb wrote: > I'd also like to see the pragma attribute approach, as well as byte-pattern > variability as I described. I don't think auto-narrowing is the only approach > we should push people

[PATCH] D76932: [AIX] emit .extern and .weak directive linkage

hubert.reinterpretcast added inline comments. Comment at: llvm/lib/CodeGen/TargetLoweringObjectFileImpl.cpp:2144 report_fatal_error( -"Unhandled linkage when mapping linkage to StorageClass."); +"AvailableExternallyLinkage is for its first instance of

[PATCH] D76932: [AIX] emit .extern and .weak directive linkage

hubert.reinterpretcast added inline comments. Comment at: llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp:1496 + +MCSymbol *Name = getSymbol(); +if (TM.getTargetTriple().isOSBinFormatXCOFF() && !F.isIntrinsic()) { DiggerLin wrote: > jasonliu wrote: > > This

[PATCH] D78172: [www] Update make_cxx_dr_status for v10; regenerate cxx_dr_status.html

This revision was automatically updated to reflect the committed changes. Closed by commit rGa73a81dce5bc: [www] Update make_cxx_dr_status for v10; regenerate cxx_dr_status.html (authored by hubert.reinterpretcast). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D78172: [www] Update make_cxx_dr_status for v10; regenerate cxx_dr_status.html

hubert.reinterpretcast marked an inline comment as done. hubert.reinterpretcast added inline comments. Comment at: clang/www/cxx_dr_status.html:14486 +https://wg21.link/cwg2445;>2445 + +Partial ordering with rewritten candidates The status is blank

[PATCH] D78068: [www] Turn 'Clang 10' boxes green in C++ status pages to reflect release

hubert.reinterpretcast marked 3 inline comments as done. hubert.reinterpretcast added inline comments. Comment at: clang/www/cxx_dr_status.html:3 "http://www.w3.org/TR/html4/strict.dtd;> hubert.reinterpretcast wrote: > rsmith wrote: > > Please

[PATCH] D78172: [www] Update make_cxx_dr_status for v10; regenerate cxx_dr_status.html

hubert.reinterpretcast created this revision. hubert.reinterpretcast added a reviewer: rsmith. Herald added a project: clang. hubert.reinterpretcast marked an inline comment as done. hubert.reinterpretcast added inline comments. Comment at: clang/www/cxx_dr_status.html:14486 +

[PATCH] D78068: [www] Turn 'Clang 10' boxes green in C++ status pages to reflect release

hubert.reinterpretcast added inline comments. Comment at: clang/www/cxx_dr_status.html:3 "http://www.w3.org/TR/html4/strict.dtd;> rsmith wrote: > Please heed this comment, and check in a matching change to > `make_cxx_dr_status` =) Thanks for

[PATCH] D78068: [www] Turn 'Clang 10' boxes green in C++ status pages to reflect release

This revision was automatically updated to reflect the committed changes. Closed by commit rG021a333bfca3: [www] Turn Clang 10 boxes green in C++ status pages to reflect release (authored by hubert.reinterpretcast). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D78068: [www] Turn 'Clang 10' boxes green in C++ status pages to reflect release

hubert.reinterpretcast created this revision. hubert.reinterpretcast added reviewers: rsmith, aaron.ballman. Herald added a project: clang. The 'Clang 10' boxes should be green since Clang 10 has been released. Repository: rG LLVM Github Monorepo https://reviews.llvm.org/D78068 Files:

[PATCH] D76932: [AIX] emit .extern and .weak directive linkage

hubert.reinterpretcast added inline comments. Comment at: llvm/lib/CodeGen/AsmPrinter/AsmPrinter.cpp:403 switch (Linkage) { case GlobalValue::CommonLinkage: case GlobalValue::LinkOnceAnyLinkage: I have my doubts that `CommonLinkage` should produce

[PATCH] D77683: [Docs] Make code review policy clearer about requested pre-commit reviews

hubert.reinterpretcast added inline comments. Comment at: llvm/docs/CodeReview.rst:34 +uncertainty, a patch should be reviewed prior to being committed. If pre-commit +code reviewes in a particular area have been requested, code should clear a +significantly higher bar, e.g.,

[PATCH] D77683: [Docs] Make code review policy clearer about requested pre-commit reviews

hubert.reinterpretcast added inline comments. Comment at: llvm/docs/CodeReview.rst:34 +uncertainty, a patch should be reviewed prior to being committed. If pre-commit +code reviewes in a particular area have been requested, code should clear a +significantly higher bar, e.g.,

[PATCH] D77598: Integral template argument suffix printing

hubert.reinterpretcast added inline comments. Comment at: clang/test/CodeGenCXX/debug-info-template.cpp:34 -// CHECK: ![[TC]] = distinct !DICompositeType(tag: DW_TAG_structure_type, name: "TC" +// CHECK: ![[TC]] = distinct !DICompositeType(tag: DW_TAG_structure_type, name:

[PATCH] D77598: Integral template argument suffix printing

hubert.reinterpretcast added a comment. Is this actually appropriate for an argument to `template ` or should the type be reflected in more than the suffix? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D77598/new/ https://reviews.llvm.org/D77598

[PATCH] D73307: Unique Names for Functions with Internal Linkage

hubert.reinterpretcast added inline comments. Comment at: clang/docs/UsersManual.rst:1684 + linkage symbols. The unique name is obtained by appending the hash of the + full module name to the original symbol. This option is particularly useful + in attributing profile

[PATCH] D68115: Zero initialize padding in unions

hubert.reinterpretcast added a comment. In D68115#1962892 , @rsmith wrote: > In D68115#1962863 , @jfb wrote: > > > In D68115#1962833 , @rsmith wrote: > > > > > I think the

[PATCH] D76572: Replace `T(x)` with `reinterpret_cast(x)` everywhere it means reinterpret_cast. No functional change

hubert.reinterpretcast added a comment. I am guessing @DiggerLin was pinged with regards to `XCOFFObjectFile.cpp`. @jhenderson already reviewed and approved the changes to that file (it falls under libObject). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D76932: [AIX] emit .extern and .weak directive linkage

hubert.reinterpretcast added inline comments. Comment at: llvm/include/llvm/MC/MCDirectives.h:47 + MCSA_WeakReference, ///< .weak_reference (MachO) + MCSA_WeakDefAutoPrivate ///< .weak_def_can_be_hidden (MachO) }; DiggerLin wrote: >

[PATCH] D68115: Zero initialize padding in unions

hubert.reinterpretcast added a comment. In D68115#1946757 , @rsmith wrote: > 3. After doing (1), extend `__builtin_bit_cast` support to properly handle > padding bits. > 4. After doing (1) and (2), extend constant aggregate emission to always zero >

[PATCH] D68115: Zero initialize padding in unions

hubert.reinterpretcast added a comment. In D68115#1946612 , @jfb wrote: > What's the verdict then? It sounds like we are looking for `-fzero-union-padding`. That's been where the discussion has left off twice for months. Repository: rG LLVM Github

[PATCH] D76696: [AST] Build recovery expressions by default for C++.

hubert.reinterpretcast added a comment. In D76696#1944784 , @sammccall wrote: > The general scheme is probably common: unresolved expr -> ??? -> an > expression is dependent but not marked as such -> constant evaluation crashes. > > But the ??? matters,

[PATCH] D76696: [AST] Build recovery expressions by default for C++.

hubert.reinterpretcast added a comment. > In D76696#1944014 , > @hubert.reinterpretcast wrote: > >> We have also encountered crashes in downstream testing caused by this using >> just the vanilla source from trunk. When there is a proposed fix, please

[PATCH] D76696: [AST] Build recovery expressions by default for C++.

hubert.reinterpretcast added a comment. We have also encountered crashes in downstream testing caused by this using just the vanilla source from trunk. When there is a proposed fix, please let us know so we can test. Thanks. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D73307: Unique Names for Functions with Internal Linkage

hubert.reinterpretcast added inline comments. Comment at: clang/lib/CodeGen/CodeGenModule.cpp:1135 +llvm::MD5 Md5; +Md5.update(getModule().getSourceFileName()); +llvm::MD5::MD5Result R; tmsriram wrote: > hubert.reinterpretcast wrote: > > davidxl

[PATCH] D73307: Unique Names for Functions with Internal Linkage

hubert.reinterpretcast added inline comments. Comment at: clang/lib/CodeGen/CodeGenModule.cpp:1135 +llvm::MD5 Md5; +Md5.update(getModule().getSourceFileName()); +llvm::MD5::MD5Result R; davidxl wrote: > rnk wrote: > > davidxl wrote: > > > Source

[PATCH] D76360: [PPC][AIX] Emit correct Vaarg for 32BIT-AIX in clang

hubert.reinterpretcast added inline comments. Comment at: clang/lib/CodeGen/TargetInfo.cpp:4176 +/// PPC32_SVR4_ABIInfo - The 32-bit PowerPC ABI information, used by PowerPC ELF +/// (SVR4), Darwin and AIX. class PPC32_SVR4_ABIInfo : public DefaultABIInfo { I

[PATCH] D76291: [Support] Fix formatted_raw_ostream for UTF-8

hubert.reinterpretcast added a comment. I have no further concerns with this patch (although I would suggest editing the description to use "Unicode" capitalized). I will leave approval of the patch to one of the requested reviewers. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST

[PATCH] D76291: [Support] Fix formatted_raw_ostream for UTF-8

hubert.reinterpretcast added inline comments. Comment at: llvm/lib/Support/FormattedStream.cpp:59 +size_t NumBytes = getNumBytesForUTF8(PartialUTF8Char[0]); +if (NumBytes > (PartialUTF8Char.size() + Size)) { + // If we still don't have enough bytes for a complete

[PATCH] D76360: [PPC][AIX] Emit correct Vaarg for 32BIT-AIX in clang

hubert.reinterpretcast added inline comments. Comment at: clang/lib/CodeGen/TargetInfo.cpp:4232 QualType Ty) const { - if (getTarget().getTriple().isOSDarwin()) { + // TODO: Add AIX ABI Info. Currently we are relying on

[PATCH] D76291: [Support] Fix formatted_raw_ostream for UTF-8

hubert.reinterpretcast added inline comments. Comment at: llvm/lib/Support/FormattedStream.cpp:64 +} else { + // The first few bytes from the buffer will complete the code-point. + // Concatenate them and process their effect on the line ane column

[PATCH] D76291: [Support] Fix formatted_raw_ostream for UTF-8

hubert.reinterpretcast added inline comments. Comment at: llvm/include/llvm/Support/FormattedStream.h:44 + /// PartialUTF8Char - Either empty or a prefix of a UTF-8 character which + /// should be prepended to the buffer for the next call to ComputePosition.

[PATCH] D69825: [Clang][Driver] Re-use the calling process instead of creating a new process for the cc1 invocation

hubert.reinterpretcast added a comment. In D69825#1916865 , @daltenty wrote: > On AIX and PPC LE Linux after this change we are seeing invalid accesses when > the backend asserts/fatal_errors. Looks like the driver and CC1 now share > some global

[PATCH] D75969: [clangd] Link libClangDaemonTweaks to libClangFormat

hubert.reinterpretcast added a comment. Thanks for the prompt response. It seems our fix attempts happened "concurrently" and we ended up with pretty much the same fix. I had committed rG48121a5743b684def33d158391c5424626de28e2

[PATCH] D75969: [clangd] Link libClangDaemonTweaks to libClangFormat

This revision was not accepted when it landed; it landed in state "Needs Review". This revision was automatically updated to reflect the committed changes. Closed by commit rG48121a5743b6 (authored by hubert.reinterpretcast). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D75716: [clangd] Have visibleNamespaces() and getEligiblePoints() take a LangOptions rather than a FormatStyle

hubert.reinterpretcast added a comment. In D75716#1916032 , @hubert.reinterpretcast wrote: > It looks like this is causing bot failures > (http://lab.llvm.org:8011/builders/clang-ppc64le-rhel/builds/1881/steps/build%20stage%201/logs/stdio): Fix

[PATCH] D75716: [clangd] Have visibleNamespaces() and getEligiblePoints() take a LangOptions rather than a FormatStyle

hubert.reinterpretcast added a comment. It looks like this is causing bot failures (http://lab.llvm.org:8011/builders/clang-ppc64le-rhel/builds/1881/steps/build%20stage%201/logs/stdio): : && /home/buildbots/clang.9.0.0/bin/clang++ --gcc-toolchain=/opt/rh/devtoolset-7/root/usr -fPIC -fPIC

[PATCH] D75524: [www] cxx_status: Update Reflection TS to Cologne draft

This revision was automatically updated to reflect the committed changes. Closed by commit rGdfaafbab4687: [www] cxx_status: Update Reflection TS to Cologne draft (authored by hubert.reinterpretcast). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION

[PATCH] D75523: [www] cxx_status: Update title to mention C++20

This revision was automatically updated to reflect the committed changes. Closed by commit rGf4076ad64075: [www] cxx_status: Update title to mention C++20 (authored by hubert.reinterpretcast). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75523/new/

[PATCH] D75523: [www] cxx_status: Update title to mention C++20

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

[PATCH] D75524: [www] cxx_status: Update Reflection TS to Cologne draft

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

[PATCH] D75524: [www] cxx_status: Update Reflection TS to Cologne draft

hubert.reinterpretcast created this revision. hubert.reinterpretcast added reviewers: rsmith, aaron.ballman. Herald added a project: clang. As of the 2019 Cologne meeting, according to its minutes (N4826), N4818 is the draft of the Reflection TS. Repository: rG LLVM Github Monorepo

[PATCH] D75523: [www] cxx_status: Update title to mention C++20

hubert.reinterpretcast created this revision. hubert.reinterpretcast added reviewers: rsmith, aaron.ballman. Herald added a project: clang. The document covers the Clang implementation status of the "upcoming C++20 standard". Update the title to match. Repository: rG LLVM Github Monorepo

[PATCH] D75056: [Driver] Default to -fno-common for all targets

hubert.reinterpretcast added inline comments. Comment at: clang/docs/ReleaseNotes.rst:87 +- -fno-common has been enabled as the default for all targets. jyknight wrote: > Might be nice to expand upon this somewhat, to give users a clue what it > means to

[PATCH] D75056: [Driver] Default to -fno-common

hubert.reinterpretcast added a comment. I have no objections; however, it may help for the title of the patch to clearly indicate "for all targets". CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75056/new/ https://reviews.llvm.org/D75056

[PATCH] D72736: [AIX] Add improved interface for retrieving load module paths

hubert.reinterpretcast requested changes to this revision. hubert.reinterpretcast added inline comments. This revision now requires changes to proceed. Comment at: llvm/lib/Support/Unix/Path.inc:243 +#elif defined(_AIX) + return

[PATCH] D65761: Add Windows Control Flow Guard checks (/guard:cf).

hubert.reinterpretcast added a comment. In D65761#1814968 , @ajpaverd wrote: > The CFGuard library shouldn't be needed for targets other than ARM, AArch64, > and X86, and it's only being built for these targets. However, it looks like > `llvm-build` is

[PATCH] D72736: [AIX] Add improved interface for retrieving load module paths

hubert.reinterpretcast added inline comments. Comment at: llvm/include/llvm/Support/FileUtilities.h:38 +#if defined(_AIX) + SmallString<128> getLoadModuleFilenameForFunctionAIX(void (*Func)(void)); +#endif This should be an internal function in the `.cpp` file

[PATCH] D72736: [AIX] Add improved interface for retrieving load module paths

hubert.reinterpretcast added inline comments. Comment at: llvm/include/llvm/Support/FileUtilities.h:44 +if (!FuncAddr) { + return ""; +} I'm not sure that this function should handle failure by returning an empty string. The error condition can be

[PATCH] D68115: Zero initialize padding in unions

hubert.reinterpretcast added a comment. In D68115#1820462 , @vitalybuka wrote: > So if I understand this the proposal is to have something like > -fzero-union-padding which is off by default. > When it's OFF compiler will continue to do whatever it does

[PATCH] D65761: Add Windows Control Flow Guard checks (/guard:cf).

hubert.reinterpretcast added a comment. In D65761#1804302 , @hubert.reinterpretcast wrote: > I have confirmed that the case I mentioned fails with rGd157a9b > . @ajpaverd, is a fix

[PATCH] D68115: Zero initialize padding in unions

hubert.reinterpretcast added a comment. In D68115#1810891 , @lebedev.ri wrote: > Does this have to be an unilateral change, > likely penalizing non-`-ftrivial-auto-var-init=` cases, > i.e. [why] can't it be **only** done for when

[PATCH] D65761: Add Windows Control Flow Guard checks (/guard:cf).

hubert.reinterpretcast added a comment. Is building `check-all` immediately after configuration expected to be clean? I have a build configured to build LLVM with just `clang` and just with the `PowerPC` target, and it seems `check-all` (under such a configuration) does not trigger the

[PATCH] D69620: Add AIX assembler support

hubert.reinterpretcast accepted this revision. hubert.reinterpretcast added a comment. LGTM with minor change request to a comment. Thanks. Comment at: clang/lib/Driver/ToolChains/AIX.cpp:68 + if (Inputs.size() != 1) +llvm_unreachable("Invalid number of input files."); +

[PATCH] D69620: Add AIX assembler support

hubert.reinterpretcast added inline comments. Comment at: clang/lib/Driver/ToolChains/AIX.cpp:70 + const InputInfo = Inputs[0]; + if (II.isFilename()) +CmdArgs.push_back(II.getFilename()); Should is also be checked as being either a filename or "nothing"?

[PATCH] D69620: Add AIX assembler support

hubert.reinterpretcast added inline comments. Comment at: clang/lib/Driver/ToolChains/AIX.cpp:68 + if (Inputs.size() != 1) +llvm_unreachable("Invalid number of input files."); + const InputInfo = Inputs[0]; `llvm_unreachable` is not the right solution if

[PATCH] D70675: [AIX] Disable clang python binding tests

hubert.reinterpretcast accepted this revision. hubert.reinterpretcast added a comment. Agreed. LGTM as well. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D70675/new/ https://reviews.llvm.org/D70675

[PATCH] D69620: Add AIX assembler support

hubert.reinterpretcast added inline comments. Comment at: clang/lib/Driver/ToolChains/AIX.cpp:37 + + // Specify the mode in which the as command operates. + if (IsArch32Bit) { Refer to the other comment regarding confusion of "as" with the English word.

<    1   2   3   4   5   6   7   8   >