[PATCH] D64597: CodeGet: Init 32bit pointers with 0xFFFFFFFF

2019-07-12 Thread Hubert Tong via Phabricator via cfe-commits
hubert.reinterpretcast added a comment. In D64597#1581605 , @pcc wrote: > The problem with `0x` on 32-bit is that it is likely to be a valid > address. > > When I discussed this with JF I proposed a pointer initialization of > `0x` which

[PATCH] D64597: CodeGet: Init 32bit pointers with 0xFFFFFFFF

2019-07-12 Thread Hubert Tong via Phabricator via cfe-commits
hubert.reinterpretcast added a comment. In D64597#1582950 , @jfb wrote: > On 32-bit platforms the only region that's likely to trap is the zero page. > This seems to be the best choice, but platforms with other regions should be > able to tune this value

[PATCH] D64222: [sanitizers] Use covering ObjectFormatType switches

2019-07-19 Thread Hubert Tong via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL366544: [sanitizers] Use covering ObjectFormatType switches (authored by hubert.reinterpretcast, committed by ). Changed prior to commit: https://reviews.llvm.org/D64222?vs=208085&id=210772#toc Reposit

[PATCH] D64301: Use `ln -n` to prevent forming a symlink cycle, instead of rm'ing the source

2019-07-26 Thread Hubert Tong via Phabricator via cfe-commits
hubert.reinterpretcast added a comment. `ln -n` is not a standard option that is portably supported. I'll be restoring the use of `rm` (with `-f` instead of `-rf`) and the associated comment for removing `%t.fake`. Repository: rL LLVM CHANGES SINCE LAST ACTION https://reviews.llvm.org/D64

[PATCH] D64301: Use `ln -n` to prevent forming a symlink cycle, instead of rm'ing the source

2019-07-26 Thread Hubert Tong via Phabricator via cfe-commits
hubert.reinterpretcast added a comment. In D64301#1603499 , @thakis wrote: > Which platforms doesn't it work on? `ln -n` means something different on AIX: https://www.ibm.com/support/knowledgecenter/en/ssw_aix_72/l_commands/ln.html `-n` is not documente

[PATCH] D65668: [Driver][test] Avoid undefined grep in darwin-ld.c

2019-08-02 Thread Hubert Tong via Phabricator via cfe-commits
hubert.reinterpretcast created this revision. hubert.reinterpretcast added reviewers: rnk, daltenty, xingxue, jasonliu. Herald added a project: clang. question-mark is not a BRE special character. POSIX.1-2017 XBD Section 9.3.2 indicates that the interpretation of `\?` as used by rC366282

[PATCH] D65668: [Driver][test] Avoid undefined grep in darwin-ld.c

2019-08-02 Thread Hubert Tong via Phabricator via cfe-commits
hubert.reinterpretcast added a comment. In D65668#1612711 , @rnk wrote: > I assume this is affecting a real platform that you care about, but I'm > curious what it is. Yes, this is affecting our tests on AIX. `\?` there matches a literal `?` character.

[PATCH] D65668: [Driver][test] Avoid undefined grep in darwin-ld.c

2019-08-02 Thread Hubert Tong via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL367709: [Driver][test] Avoid undefined grep in darwin-ld.c (authored by hubert.reinterpretcast, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Repository: rL LLVM

[PATCH] D65688: [AIX][test/Index] Set/propagate AIXTHREAD_STK for AIX

2019-08-02 Thread Hubert Tong via Phabricator via cfe-commits
hubert.reinterpretcast created this revision. hubert.reinterpretcast added reviewers: xingxue, daltenty, jasonliu. Herald added subscribers: jfb, arphaman. Herald added a project: clang. Some tests perform deep recursion, which requires a larger pthread stack size than the relatively low default

[PATCH] D65688: [AIX][test/Index] Set/propagate AIXTHREAD_STK for AIX

2019-08-13 Thread Hubert Tong via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL368690: [AIX][test/Index] Set/propagate AIXTHREAD_STK for AIX (authored by hubert.reinterpretcast, committed by ). Herald added a project: LLVM. Herald added a subscriber: llvm-commits. Repository: rL L

[PATCH] D63767: [NFC] Make some ObjectFormatType switches covering

2019-06-25 Thread Hubert Tong via Phabricator via cfe-commits
hubert.reinterpretcast created this revision. hubert.reinterpretcast added reviewers: sfertile, jasonliu, daltenty. Herald added subscribers: jsji, aheejin. Herald added projects: clang, LLVM. This patch removes the `default` case from some switches on `llvm::Triple::ObjectFormatType`, and cases

[PATCH] D63786: Print NULL as "(null)" in diagnostic message

2019-06-25 Thread Hubert Tong via Phabricator via cfe-commits
hubert.reinterpretcast accepted this revision. hubert.reinterpretcast added a comment. This revision is now accepted and ready to land. LGTM. I have some minor style comments that could be fixed as part of the commit. Comment at: clang/tools/c-index-test/c-index-test.c:4648

[PATCH] D63767: [NFC] Make some ObjectFormatType switches covering

2019-07-03 Thread Hubert Tong via Phabricator via cfe-commits
hubert.reinterpretcast added a comment. Ping. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63767/new/ https://reviews.llvm.org/D63767 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://li

[PATCH] D63767: [NFC] Make some ObjectFormatType switches covering

2019-07-04 Thread Hubert Tong via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rL365160: [NFC] Make some ObjectFormatType switches covering (authored by hubert.reinterpretcast, committed by ). Herald added a subscriber: kristina. Changed prior to commit: https://reviews.llvm.org/D63

[PATCH] D64222: [sanitizers] Use covering ObjectFormatType switches

2019-07-04 Thread Hubert Tong via Phabricator via cfe-commits
hubert.reinterpretcast created this revision. hubert.reinterpretcast added reviewers: sfertile, jasonliu, daltenty. Herald added subscribers: sunfish, aheejin, hiraditya. Herald added projects: clang, LLVM. This patch removes the `default` case from some switches on `llvm::Triple::ObjectFormatTyp

[PATCH] D59253: [AIX][libcxx] AIX system headers need stdint.h and inttypes.h to be re-enterable when macro _STD_TYPES_T is defined

2019-05-23 Thread Hubert Tong via Phabricator via cfe-commits
hubert.reinterpretcast added inline comments. Comment at: libcxx/include/stdint.h:16 +#endif // _STD_TYPES_T /* mclow.lists wrote: > I don't think that this will do what you want it to. > Is this a supported use case? > > ``` > #include > #define _STD_TYPES_

[PATCH] D62533: Build with _XOPEN_SOURCE defined on AIX

2019-05-28 Thread Hubert Tong via Phabricator via cfe-commits
hubert.reinterpretcast added inline comments. Comment at: llvm/CMakeLists.txt:862 +# Build with _XOPEN_SOURCE on AIX, as stray macros in _ALL_SOURCE mode tend to +# break things. In this case we need to enable the LARGE FILE API as well +if (UNIX AND ${CMAKE_SYSTEM_NAME} MATCHES

[PATCH] D55510: [ExprConstant] Improve memchr/memcmp for type mismatch and multibyte element types

2018-12-11 Thread Hubert Tong via Phabricator via cfe-commits
hubert.reinterpretcast updated this revision to Diff 177796. hubert.reinterpretcast added a comment. Recast representation-sensitive tests as SemaCXX tests using array bounds Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55510/new/ https://reviews.llvm.org/D55510

[PATCH] D55510: [ExprConstant] Improve memchr/memcmp for type mismatch and multibyte element types

2018-12-11 Thread Hubert Tong via Phabricator via cfe-commits
hubert.reinterpretcast marked 3 inline comments as done. hubert.reinterpretcast added inline comments. Comment at: lib/AST/ExprConstant.cpp:6147-6148 + return ZeroInitialization(E); +if (!Result.checkNullPointerForFoldAccess(Info, E, AK_Read)) + return false; +Q

[PATCH] D55510: [ExprConstant] Improve memchr/memcmp for type mismatch and multibyte element types

2018-12-12 Thread Hubert Tong via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. hubert.reinterpretcast marked an inline comment as done. Closed by commit rL348938: [ExprConstant] Improve memchr/memcmp for type mismatch and multibyte element… (authored by hubert.reinterpretcast, committed by ). Herald a

[PATCH] D68115: Zero initialize padding in unions

2019-09-26 Thread Hubert Tong via Phabricator via cfe-commits
hubert.reinterpretcast added inline comments. Comment at: clang/test/CodeGen/init.c:197 // CHECK-LABEL: @nonzeroPaddedUnionMemset( - // CHECK-NOT: store - // CHECK-NOT: memcpy - // CHECK: call void @llvm.memset.p0i8.i32(i8* {{.*}}, i8 -16, i32 36, i1 false) + // CHECK: ca

[PATCH] D68115: Zero initialize padding in unions

2019-09-27 Thread Hubert Tong via Phabricator via cfe-commits
hubert.reinterpretcast added inline comments. Comment at: clang/test/CodeGen/init.c:197 // CHECK-LABEL: @nonzeroPaddedUnionMemset( - // CHECK-NOT: store - // CHECK-NOT: memcpy - // CHECK: call void @llvm.memset.p0i8.i32(i8* {{.*}}, i8 -16, i32 36, i1 false) + // CHECK: ca

[PATCH] D68115: Zero initialize padding in unions

2019-09-27 Thread Hubert Tong via Phabricator via cfe-commits
hubert.reinterpretcast added inline comments. Comment at: clang/test/CodeGen/init.c:197 // CHECK-LABEL: @nonzeroPaddedUnionMemset( - // CHECK-NOT: store - // CHECK-NOT: memcpy - // CHECK: call void @llvm.memset.p0i8.i32(i8* {{.*}}, i8 -16, i32 36, i1 false) + // CHECK: ca

[PATCH] D68115: Zero initialize padding in unions

2019-09-27 Thread Hubert Tong via Phabricator via cfe-commits
hubert.reinterpretcast added a comment. In D68115#1686837 , @jfb wrote: > The entire point of this feature is to add guardrails to the language. I agree, and guardrails have a tendency to scratch paint if one drives against them. > What do people expec

[PATCH] D68115: Zero initialize padding in unions

2019-09-27 Thread Hubert Tong via Phabricator via cfe-commits
hubert.reinterpretcast added a comment. In D68115#1686887 , @vitalybuka wrote: > I would be happy to update the patch to enable it only for > -ftrivial-auto-var-init=pattern, if we want "bumper" version. It seems to be a separable feature (although it d

[PATCH] D68340: Add AIX toolchain and basic linker functionality

2019-10-12 Thread Hubert Tong via Phabricator via cfe-commits
hubert.reinterpretcast added inline comments. Comment at: clang/lib/Driver/ToolChains/AIX.cpp:1 +//===--- AIX.cpp - AIX ToolChain Implementations *- C++ -*-===// +// See Jason's comment about the line length. Comment at: clang/lib/Drive

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

2020-05-20 Thread Hubert Tong via Phabricator via cfe-commits
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 toolchain

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

2020-05-21 Thread Hubert Tong via Phabricator via cfe-commits
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

2020-05-21 Thread Hubert Tong via Phabricator via cfe-commits
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 reflect

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

2020-05-21 Thread Hubert Tong via Phabricator via cfe-commits
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, -Wl,-rpa

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

2020-05-22 Thread Hubert Tong via Phabricator via cfe-commits
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: > s/Arguemnt/Argumen

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

2020-05-22 Thread Hubert Tong via Phabricator via cfe-commits
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" "[[SYSROOT:[^

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

2020-05-22 Thread Hubert Tong via Phabricator via cfe-commits
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

2020-05-22 Thread Hubert Tong via Phabricator via cfe-commits
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 wonder

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

2020-05-22 Thread Hubert Tong via Phabricator via cfe-commits
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 wr

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

2020-05-22 Thread Hubert Tong via Phabricator via cfe-commits
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 fix

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

2020-05-22 Thread Hubert Tong via Phabricator via cfe-commits
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] D80532: [NFC] Fix formatting for the 'aix-ld.c' test case.

2020-05-25 Thread Hubert Tong via Phabricator via cfe-commits
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 // CHECK-L

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

2020-05-26 Thread Hubert Tong via Phabricator via cfe-commits
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 li

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

2020-05-26 Thread Hubert Tong via Phabricator via cfe-commits
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] D72736: [AIX] Add improved interface for retrieving load module paths

2020-05-29 Thread Hubert Tong via Phabricator via cfe-commits
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__) hubert.reint

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

2020-05-31 Thread Hubert Tong via Phabricator via cfe-commits
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)); + CmdArgs.push_back(A

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

2020-06-01 Thread Hubert Tong via Phabricator via cfe-commits
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 that

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

2020-06-01 Thread Hubert Tong via Phabricator via cfe-commits
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 t

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

2020-06-02 Thread Hubert Tong via Phabricator via cfe-commits
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 co

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

2020-06-02 Thread Hubert Tong via Phabricator via cfe-commits
hubert.reinterpretcast marked an inline comment as done. hubert.reinterpretcast added a comment. I believe I have responded to @joerg's comments with rationale to support the current direction. I intend to commit this patch within a week or two. Repository: rG LLVM Github Monorepo CHANGES SI

[PATCH] D80835: [AIX] Change the default target CPU to power4 for AIX on Power

2020-06-02 Thread Hubert Tong via Phabricator via cfe-commits
hubert.reinterpretcast accepted this revision. hubert.reinterpretcast added a comment. This revision is now accepted and ready to land. LGTM; thanks. Comment at: clang/lib/Driver/ToolChains/CommonArgs.cpp:301 +TargetCPUName = "pwr4"; + else if (!T.isOSDarwin()) { +

[PATCH] D83915: [PowerPC] Remove QPX/A2Q BGQ/BGP support

2020-07-16 Thread Hubert Tong via Phabricator via cfe-commits
hubert.reinterpretcast added a comment. Should we indicate planned removal in the Release Notes for version 11 and actual removal in those for version 12? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83915/new/ https://reviews.llvm.org/D83915

[PATCH] D83974: [AIX] report_fatal_error on `-fregister_global_dtors_with_atexit` for static init

2020-07-16 Thread Hubert Tong via Phabricator via cfe-commits
hubert.reinterpretcast added inline comments. Comment at: clang/lib/CodeGen/CodeGenModule.cpp:1212 if (CodeGenOpts.RegisterGlobalDtorsWithAtExit) { +if (getContext().getTargetInfo().getTriple().isOSAIX()) + llvm::report_fatal_error( This should query

[PATCH] D83974: [AIX] report_fatal_error on `-fregister_global_dtors_with_atexit` for static init

2020-07-17 Thread Hubert Tong via Phabricator via cfe-commits
hubert.reinterpretcast accepted this revision. hubert.reinterpretcast added a comment. LGTM; thanks. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D83974/new/ https://reviews.llvm.org/D83974 ___ cfe-commits mailing list cfe-commits@lists.llv

[PATCH] D79719: [AIX] Implement AIX special alignment rule about double/long double

2020-07-20 Thread Hubert Tong via Phabricator via cfe-commits
hubert.reinterpretcast added a comment. I've done another pass over the code (but did not get through the tests). I have no comments about the code at this time. My understanding is that @jasonliu will be doing another pass over this patch, so he can approve while I'm away on vacation. CHANGE

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

2020-04-03 Thread Hubert Tong via Phabricator via cfe-commits
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 https://reviews.l

[PATCH] D68115: Zero initialize padding in unions

2020-04-06 Thread Hubert Tong via Phabricator via cfe-commits
hubert.reinterpretcast added a comment. In D68115#1962892 , @rsmith wrote: > In D68115#1962863 , @jfb wrote: > > > In D68115#1962833 , @rsmith wrote: > > > > > I think the ma

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

2020-04-06 Thread Hubert Tong via Phabricator via cfe-commits
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 infor

[PATCH] D77598: Integral template argument suffix printing

2020-04-06 Thread Hubert Tong via Phabricator via cfe-commits
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] D77598: Integral template argument suffix printing

2020-04-07 Thread Hubert Tong via Phabricator via cfe-commits
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: "T

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

2020-04-07 Thread Hubert Tong via Phabricator via cfe-commits
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., fix

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

2020-04-07 Thread Hubert Tong via Phabricator via cfe-commits
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., fix

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

2020-04-08 Thread Hubert Tong via Phabricator via cfe-commits
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 `.weak

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

2020-04-13 Thread Hubert Tong via Phabricator via cfe-commits
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: clan

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

2020-04-14 Thread Hubert Tong via Phabricator via cfe-commits
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 https://revi

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

2020-04-14 Thread Hubert Tong via Phabricator via cfe-commits
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 poi

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

2020-04-14 Thread Hubert Tong via Phabricator via cfe-commits
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

2020-04-14 Thread Hubert Tong via Phabricator via cfe-commits
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 hee

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

2020-04-14 Thread Hubert Tong via Phabricator via cfe-commits
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 o

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

2020-04-15 Thread Hubert Tong via Phabricator via cfe-commits
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 https://review

[PATCH] D81115: [PowerPC] Do not check for non-Darwin in PowerPC target cpu handling

2020-06-03 Thread Hubert Tong via Phabricator via cfe-commits
hubert.reinterpretcast added a comment. Calling it the "Darwin factor" gives it a certain je ne sais quoi. :) Comment at: clang/lib/Driver/ToolChains/CommonArgs.cpp:298 +// each architecture. (except on AIX) if (TargetCPUName.empty()) { if (T.isOSAIX())

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

2020-06-03 Thread Hubert Tong via Phabricator via cfe-commits
hubert.reinterpretcast added inline comments. Comment at: llvm/docs/CodingStandards.rst:1576 + +When writing the body of an `if`, `else`, or loop statement, omit the braces to avoid +unnecessary and otherwise meaningless code. However, braces should be used Ther

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

2020-06-04 Thread Hubert Tong via Phabricator via cfe-commits
hubert.reinterpretcast accepted this revision. hubert.reinterpretcast added a comment. This revision is now accepted and ready to land. This LGTM. I believe we have not heard back from @arsenm on the response to some of their comments though. Comment at: llvm/docs/CodingStanda

[PATCH] D81115: [PowerPC] Do not special case Darwin on PowerPC in target cpu handling

2020-06-04 Thread Hubert Tong via Phabricator via cfe-commits
hubert.reinterpretcast accepted this revision. hubert.reinterpretcast added a comment. This revision is now accepted and ready to land. LGTM; thanks. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D81115/new/ https://reviews.llvm.org/D81115 __

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

2020-06-10 Thread Hubert Tong via Phabricator via cfe-commits
hubert.reinterpretcast added inline comments. Comment at: clang/lib/CodeGen/ItaniumCXXABI.cpp:4447 + + // Create __dtor function for the var decl. + llvm::Function *dtorStub = CGF.createAtExitStub(D, dtor, addr); We should probably report_fatal_error if `CXAAtE

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

2020-06-10 Thread Hubert Tong via Phabricator via cfe-commits
hubert.reinterpretcast added inline comments. Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:562 +static std::string getTransformedFileName(llvm::Module &M) { + SmallString<128> FileName = llvm::sys::path::filename(M.getName()); Consider having the `SmallString<1

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

2020-06-10 Thread Hubert Tong via Phabricator via cfe-commits
hubert.reinterpretcast added inline comments. Comment at: clang/include/clang/AST/Mangle.h:178 + virtual void mangleDynamicDestructor(const VarDecl *D, raw_ostream &Out) = 0; + I am not sure "destructor" is the right term here. This seems to be an analogue to

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

2020-06-11 Thread Hubert Tong via Phabricator via cfe-commits
hubert.reinterpretcast added inline comments. Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:305 + if (llvm::Function *unatexitFn = + dyn_cast(unatexit.getCallee())) +unatexitFn->setDoesNotThrow(); Xiangling_L wrote: > jasonliu wrote: > > Is there a va

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

2020-06-11 Thread Hubert Tong via Phabricator via cfe-commits
hubert.reinterpretcast added inline comments. Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:293 +CodeGenFunction::unregisterGlobalDtorWithUnAtExit(llvm::Function *dtorStub) { + // extern "C" int unatexit(void (*f)(void)); + assert(dtorStub->getFunctionType() == P

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

2020-06-11 Thread Hubert Tong via Phabricator via cfe-commits
hubert.reinterpretcast added inline comments. Comment at: clang/lib/AST/ItaniumMangle.cpp:5231 + CXXNameMangler Mangler(*this, Out); + Mangler.getStream() << "__cxx_global_var_destruct_"; + if (shouldMangleDeclName(D)) I believe these are actually paired with

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

2020-06-11 Thread Hubert Tong via Phabricator via cfe-commits
hubert.reinterpretcast added inline comments. Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:824 std::tie(CalleeTy, Callee, Arg) = DtorsAndObjects[e - i - 1]; - llvm::CallInst *CI = Builder.CreateCall(CalleeTy, Callee, Arg); + llvm::CallInst *CI = (Arg == nullptr) +

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

2020-06-11 Thread Hubert Tong via Phabricator via cfe-commits
hubert.reinterpretcast added inline comments. Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:682 void CodeGenModule::EmitCXXGlobalDtorFunc() { if (CXXGlobalDtors.empty()) return; hubert.reinterpretcast wrote: > Following from my previous comments on `CXXGlo

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

2020-06-12 Thread Hubert Tong via Phabricator via cfe-commits
hubert.reinterpretcast added inline comments. Comment at: clang/test/CodeGen/static-init.cpp:12 ~test(); -} t; +} t1, t2; I suggest adding also one each of the following: - a dynamic initialization of a non-local variable of type `int` - a `constinit` i

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

2020-06-12 Thread Hubert Tong via Phabricator via cfe-commits
hubert.reinterpretcast added inline comments. Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:807 void CodeGenFunction::GenerateCXXGlobalDtorsFunc( llvm::Function *Fn, This function is to be renamed. Comment at: clang/lib/CodeGen/CodeGenMod

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

2020-06-12 Thread Hubert Tong via Phabricator via cfe-commits
hubert.reinterpretcast marked an inline comment as done. hubert.reinterpretcast added inline comments. Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:700 + +Fn = CreateGlobalInitOrDestructFunction( +FTy, llvm::Twine("__sterm8000_clang_") + GlobalUniqueModuleId, FI, -

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

2020-06-12 Thread Hubert Tong via Phabricator via cfe-commits
hubert.reinterpretcast added inline comments. Comment at: clang/test/CodeGen/static-init.cpp:31 +// CHECK: @llvm.global_ctors = appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 65535, void ()* @__sinit8000_clang_510e898aa8d263cac999dd03eeed5b51, i

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

2020-06-12 Thread Hubert Tong via Phabricator via cfe-commits
hubert.reinterpretcast added inline comments. Comment at: clang/test/CodeGen/static-init.cpp:8 +// RUN: FileCheck %s struct test { Xiangling_L wrote: > jasonliu wrote: > > Looks like the non-inline comments are easier to get ignored and missed, so > > I wil

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

2020-06-12 Thread Hubert Tong via Phabricator via cfe-commits
hubert.reinterpretcast added inline comments. Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:277 + llvm::FunctionType::get(CGM.VoidTy, false) && + "atexit has wrong parameter type."); + s/atexit has wrong parameter type/Argument to atexit has a w

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

2020-06-13 Thread Hubert Tong via Phabricator via cfe-commits
hubert.reinterpretcast added inline comments. Comment at: clang/lib/CodeGen/ItaniumCXXABI.cpp:4482 + llvm::Value *NeedsDestruct = + CGF.Builder.CreateIsNull(V, "needsDestruct"); + There are uses of `CreateIsNull` with `snake_case`; this is the only `camelC

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

2020-06-15 Thread Hubert Tong via Phabricator via cfe-commits
hubert.reinterpretcast added inline comments. Comment at: clang/test/CodeGen/static-init.cpp:31 +// CHECK: @llvm.global_ctors = appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 65535, void ()* @__sinit8000_clang_510e898aa8d263cac999dd03eeed5b51, i

[PATCH] D79719: [AIX] Implement AIX special alignment rule about double/long double

2020-06-15 Thread Hubert Tong via Phabricator via cfe-commits
hubert.reinterpretcast added inline comments. Comment at: clang/include/clang/Basic/TargetInfo.h:1389 + /// Whether target supports the special power alignment rules of AIX. + virtual bool supportsAIXPowerAlignment() const { return false; } Minor nit: Use bac

[PATCH] D79719: [AIX] Implement AIX special alignment rule about double/long double

2020-06-16 Thread Hubert Tong via Phabricator via cfe-commits
hubert.reinterpretcast added inline comments. Comment at: clang/lib/AST/RecordLayoutBuilder.cpp:660 + /// When there are OverlappingEmptyFields existing in the aggregate, the + /// flag shows if the following first non-overlappingEmptyField has been + /// handled, if any.

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

2020-06-16 Thread Hubert Tong via Phabricator via cfe-commits
hubert.reinterpretcast added inline comments. Comment at: clang/lib/CodeGen/ItaniumCXXABI.cpp:4489 + // DestructCallBlock, otherwise jump to EndBlock directly. + CGF.EmitCXXGuardedInitBranch(NeedsDestruct, DestructCallBlock, EndBlock, + CodeGenFunc

[PATCH] D81972: [NFC] Cleanup of EmitCXXGlobalInitFunc() and EmitCXXGlobalDtorFunc()

2020-06-17 Thread Hubert Tong via Phabricator via cfe-commits
hubert.reinterpretcast added inline comments. Comment at: clang/lib/CodeGen/CGDeclCXX.cpp:600 + // with priority emitted above. + SmallString<128> FileName; llvm::Function *Fn = CreateGlobalInitOrDestructFunction( In the less complex context of this patch, i

[PATCH] D79719: [AIX] Implement AIX special alignment rule about double/long double

2020-07-01 Thread Hubert Tong via Phabricator via cfe-commits
hubert.reinterpretcast added inline comments. Comment at: clang/lib/AST/ASTContext.cpp:2416 - // Double and long long should be naturally aligned if possible. + // Double, long double (only when the target supports AIX power alignment) and + // long long should be naturally

[PATCH] D82677: [Clang] Handle AIX Include management in the driver

2020-07-02 Thread Hubert Tong via Phabricator via cfe-commits
hubert.reinterpretcast added inline comments. Comment at: clang/lib/Driver/ToolChains/AIX.cpp:179 + } + addSystemInclude(DriverArgs, CC1Args, "/usr/include"); +} This does not observe `--sysroot`. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTI

[PATCH] D82677: [Clang] Handle AIX Include management in the driver

2020-07-02 Thread Hubert Tong via Phabricator via cfe-commits
hubert.reinterpretcast added inline comments. Comment at: clang/lib/Driver/ToolChains/AIX.cpp:172 +return; + + const Driver &D = getDriver(); Please investigate the handling of `OPT_nostdlibinc`. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST AC

[PATCH] D79719: [AIX] Implement AIX special alignment rule about double/long double

2020-07-02 Thread Hubert Tong via Phabricator via cfe-commits
hubert.reinterpretcast added inline comments. Comment at: clang/test/Layout/aix-double-struct-member.cpp:1 +// RUN: %clang_cc1 -emit-llvm-only -triple powerpc-ibm-aix-xcoff \ +// RUN: -fdump-record-layouts -fsyntax-only %s 2>/dev/null | \ Xiangling_L wrote: >

[PATCH] D82677: [Clang] Handle AIX Include management in the driver

2020-07-02 Thread Hubert Tong via Phabricator via cfe-commits
hubert.reinterpretcast added inline comments. Comment at: clang/lib/Driver/ToolChains/AIX.cpp:172 +return; + + const Driver &D = getDriver(); daltenty wrote: > hubert.reinterpretcast wrote: > > Please investigate the handling of `OPT_nostdlibinc`. > If we ar

[PATCH] D82677: [Clang] Handle AIX Include management in the driver

2020-07-02 Thread Hubert Tong via Phabricator via cfe-commits
hubert.reinterpretcast added inline comments. Comment at: clang/lib/Driver/ToolChains/AIX.cpp:179 +ArgStringList &CC1Args) const { + // return if -nostdinc/-nostdlibinc is specified as a driver option. + if (DriverArgs.hasArg(options::OPT_nos

[PATCH] D79719: [AIX] Implement AIX special alignment rule about double/long double

2020-07-03 Thread Hubert Tong via Phabricator via cfe-commits
hubert.reinterpretcast added inline comments. Comment at: clang/lib/AST/RecordLayoutBuilder.cpp:1235 + // Do not use AIX special alignment if current base is not the first member or + // the struct is not a union. hubert.reinterpretcast wrote: > Suggestion:

[PATCH] D79719: [AIX] Implement AIX special alignment rule about double/long double

2020-07-03 Thread Hubert Tong via Phabricator via cfe-commits
hubert.reinterpretcast added inline comments. Comment at: clang/lib/AST/RecordLayoutBuilder.cpp:1871 + // AIX ABI has this special rule that in aggregates, the first member of + // floating point data type(or aggregate type contains floating point data Sugges

[PATCH] D82677: [Clang] Handle AIX Include management in the driver

2020-07-03 Thread Hubert Tong via Phabricator via cfe-commits
hubert.reinterpretcast accepted this revision. hubert.reinterpretcast added a comment. Aside from one comment; LGTM. Thanks. Comment at: clang/test/Driver/aix-toolchain-include.cpp:4 +// Check powerpc-ibm-aix, 32-bit/64-bit. +// RUN: %clangxx -### -no-canonical-prefixes %s 2>&1

[PATCH] D79719: [AIX] Implement AIX special alignment rule about double/long double

2020-07-03 Thread Hubert Tong via Phabricator via cfe-commits
hubert.reinterpretcast added inline comments. Comment at: clang/lib/AST/RecordLayoutBuilder.cpp:1778 +if (FoundNonOverlappingEmptyField) + HandledFirstNonOverlappingEmptyField = true; + See my other comment for rationale on why `HandledFirstNonOverlappi

<    1   2   3   4   5   6   7   8   >