[PATCH] D79210: Let clang print registered targets for --version

2020-04-30 Thread Eric Christopher via Phabricator via cfe-commits
echristo added a comment. LGTM if it works? Can you post the output into the commit message so we can review that here? I guess clang -v or something? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D79210/new/ https://reviews.llvm.org/D79210 ___

[PATCH] D79210: Let clang print registered targets for --version

2020-05-01 Thread Eric Christopher via Phabricator via cfe-commits
echristo accepted this revision. echristo added a comment. This revision is now accepted and ready to land. Yeah, it's fine. We might want to put it behind a "long information" option, but for now it's ok. Keep an eye open if someone complains please. CHANGES SINCE LAST ACTION https://reviews

[PATCH] D83273: [X86] Remove the feature dependency handling in X86TargetInfo::setFeatureEnabledImpl to a table based lookup in X86TargetParser.cpp

2020-08-04 Thread Eric Christopher via Phabricator via cfe-commits
echristo added a comment. In D83273#2194842 , @nickdesaulniers wrote: > In D83273#2194832 , @craig.topper > wrote: > >> It's a pretty nasty revert in our downstream tree where we have prototyped >> future ISAs. S

[PATCH] D84068: AMDGPU/clang: Search resource directory for device libraries

2020-08-11 Thread Eric Christopher via Phabricator via cfe-commits
echristo added a comment. Hi All, I'd really like to avoid depending on any program other than clang installing something into clang's working directory. I think this needs to be written from the perspective of specifying another directory to look for things. -eric CHANGES SINCE LAST ACTION

[PATCH] D71687: Fix full loop unrolling initialization in new pass manager

2020-05-04 Thread Eric Christopher via Phabricator via cfe-commits
echristo updated this revision to Diff 261928. echristo edited the summary of this revision. echristo added a comment. Herald added a subscriber: zzheng. Add a testcase with opt and command line option so we can enable it. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https:

[PATCH] D71687: Fix full loop unrolling initialization in new pass manager

2020-05-06 Thread Eric Christopher via Phabricator via cfe-commits
echristo updated this revision to Diff 262458. echristo added a comment. Update and reduce testcase a bit. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71687/new/ https://reviews.llvm.org/D71687 Files: clang/test/Misc/loop-opt-setup.c llvm/li

[PATCH] D71687: Fix full loop unrolling initialization in new pass manager

2020-05-06 Thread Eric Christopher via Phabricator via cfe-commits
echristo marked 6 inline comments as done. echristo added a comment. OK, ready again :) Comment at: llvm/test/Transforms/LoopUnroll/FullUnroll.ll:4-6 +; We don't end up deleting the loop, but we remove everything inside of it so checking for any +; reasonable instruction from

[PATCH] D79916: Map -O to -O1 instead of -O2

2020-05-15 Thread Eric Christopher via Phabricator via cfe-commits
echristo accepted this revision. echristo added a reviewer: dexonsmith. echristo added a comment. This revision is now accepted and ready to land. I'm totally down, but you knew that already :) Duncan: Do you have any concerns? I doubt it, but just checking. Repository: rG LLVM Github Monorep

[PATCH] D127812: [AArch64] Function multiversioning support added.

2022-07-13 Thread Eric Christopher via Phabricator via cfe-commits
echristo added a comment. In D127812#3616603 , @danielkiss wrote: > In D127812#3602688 , @aaron.ballman > wrote: > >> In D127812#3602645 , @erichkeane >> wrote: >> >>>

[PATCH] D36249: Mark tests that need intel 80-bit floats as x86-only

2017-08-03 Thread Eric Christopher via Phabricator via cfe-commits
echristo accepted this revision. echristo added a comment. This revision is now accepted and ready to land. I'm ok with this. Repository: rL LLVM https://reviews.llvm.org/D36249 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists

[PATCH] D35449: [X86] Implement __builtin_cpu_is

2017-08-10 Thread Eric Christopher via Phabricator via cfe-commits
echristo accepted this revision. echristo added a comment. This revision is now accepted and ready to land. SGTM. -eric https://reviews.llvm.org/D35449 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/li

[PATCH] D63607: [clang][driver] Add basic --driver-mode=flang support for fortran

2020-01-08 Thread Eric Christopher via Phabricator via cfe-commits
echristo added a comment. Sadly I'm just noticing this: I don't really agree with the rationale for adding fortran support to the clang driver. My proposed design here would be to move the aspects you need out of clang and into a separate library in llvm that you can use instead. I don't neces

[PATCH] D89184: Support complex target features combinations

2020-10-13 Thread Eric Christopher via Phabricator via cfe-commits
echristo added a comment. Hi Peng, Looks interesting, but I think the language support needs some more comments about what's expected and in addition through everything else please? I've added some inline comments with places I think could use it for sure. -eric Comment at:

[PATCH] D89184: Support complex target features combinations

2020-10-13 Thread Eric Christopher via Phabricator via cfe-commits
echristo requested changes to this revision. echristo added a comment. This revision now requires changes to proceed. Mark as requesting changes :) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89184/new/ https://reviews.llvm.org/D89184 ___ c

[PATCH] D63852: [Clang] Move assembler into a separate file

2020-12-01 Thread Eric Christopher via Phabricator via cfe-commits
echristo accepted this revision. echristo added a comment. This revision is now accepted and ready to land. Thanks for the explanation, lgtm. -eric Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D63852/new/ https://reviews.llvm.org/D63852 _

[PATCH] D89184: Support complex target features combinations

2020-10-28 Thread Eric Christopher via Phabricator via cfe-commits
echristo added a comment. I'll take a look tomorrow, sorry for the delay. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D89184/new/ https://reviews.llvm.org/D89184 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/

[PATCH] D89184: Support complex target features combinations

2020-10-29 Thread Eric Christopher via Phabricator via cfe-commits
echristo accepted this revision. echristo added a comment. This revision is now accepted and ready to land. Let's go ahead and unblock you, but getting a lot of this refactored would be great if you can. I think it's hitting the limits of the original design. :) CHANGES SINCE LAST ACTION http

[PATCH] D89158: [NewPM] Run callbacks added via registerPipelineStartEPCallback under -O0

2020-10-29 Thread Eric Christopher via Phabricator via cfe-commits
echristo added a comment. In D89158#2363220 , @aeubanks wrote: > Looking at BackendUtil.cpp in Clang as well as the Rust code, I'm back to > thinking that we should provide a way to to all callbacks. Then in the case > of passes added via TargetMachine,

[PATCH] D103125: [Clang][WIP] Allow renaming of "clang"

2021-05-26 Thread Eric Christopher via Phabricator via cfe-commits
echristo added a comment. I'm also not a fan of this change. From a project perspective the binary is clang and while people may wish to change the name for their own product teams it seems like that onus should be on them. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION htt

[PATCH] D96203: [clang][patch] Modify sanitizer options names: renaming blacklist to blocklist

2021-02-08 Thread Eric Christopher via Phabricator via cfe-commits
echristo added a reviewer: vitalybuka. echristo added a comment. In D96203#2548495 , @aaron.ballman wrote: > In D96203#2548471 , @mibintc wrote: > >> In D96203#2546856 , @aa

[PATCH] D96906: [AMDGPU] gfx90a support

2021-02-19 Thread Eric Christopher via Phabricator via cfe-commits
echristo added a comment. In D96906#2572842 , @msearles wrote: > In D96906#2572749 , @kzhuravl wrote: > >>> The point is that nobody upstream even got a chance to chime in. >> >> We are and will be taking care of an

[PATCH] D96203: [clang][patch] Modify sanitizer options names: renaming blacklist to blocklist

2021-02-25 Thread Eric Christopher via Phabricator via cfe-commits
echristo added a comment. Exclusion list? or exclude? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D96203/new/ https://reviews.llvm.org/D96203 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http

[PATCH] D51650: Implement target_clones multiversioning

2021-11-05 Thread Eric Christopher via Phabricator via cfe-commits
echristo added a comment. FWIW I'm a bit rusty in this area myself, but thanks for doing this. Let's see if we can't get Aaron to continue reviewing :) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D51650/new/ https://reviews.llvm.org/D51650

[PATCH] D97736: [Driver] Add a experimental option to link to LLVM libc.

2021-03-04 Thread Eric Christopher via Phabricator via cfe-commits
echristo added a comment. In addition to the bikeshed below, I'm not a huge fan of this in general. I think we should assume that the libc we link is complete and thus it would just be named "libc." and in a sysroot somewhere. Another option is perhaps looking at the rtlib option, but I'd want

[PATCH] D110549: [HIPSPV][1/4] Refactor HIP tool chain

2021-11-22 Thread Eric Christopher via Phabricator via cfe-commits
echristo added inline comments. Comment at: clang/lib/Driver/ToolChains/HIPUtility.cpp:119-133 + // Add MC directives to embed target binaries. We ensure that each + // section and image is 16-byte aligned. This is not mandatory, but + // increases the likelihood of data to be

[PATCH] D141547: Fix format for `case` in .proto files

2023-01-11 Thread Eric Christopher via Phabricator via cfe-commits
echristo accepted this revision. echristo 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/D141547/new/ https://reviews.llvm.org/D141547 __

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

2023-10-09 Thread Eric Christopher via Phabricator via cfe-commits
echristo accepted this revision. echristo added a comment. Explicitly still ok with this as well. Thanks for continuing here. :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D86310/new/ https://reviews.llvm.org/D86310 _

[PATCH] D75152: Fix typo

2020-02-25 Thread Eric Christopher via Phabricator via cfe-commits
echristo accepted this revision. echristo added a comment. This revision is now accepted and ready to land. This probably qualifies as "obvious" :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75152/new/ https://reviews.llvm.org/D75152

[PATCH] D65410: [PassManager] First Pass implementation at -O1 pass pipeline

2019-11-07 Thread Eric Christopher via Phabricator via cfe-commits
echristo updated this revision to Diff 228338. echristo marked an inline comment as done. echristo added a comment. Herald added subscribers: aheejin, sbc100, nhaehnle, jvesely. I've gone ahead and enabled SROA here. In the testing I've done so far it's helped execute time quite a bit and compile

[PATCH] D65410: [PassManager] First Pass implementation at -O1 pass pipeline

2019-11-07 Thread Eric Christopher via Phabricator via cfe-commits
echristo updated this revision to Diff 228339. echristo added a comment. Update to remove comments around SROA addition. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65410/new/ https://reviews.llvm.org/D65410 Files: clang/test/CodeGen/2008-07-30-implicit-initialization.c clang/tes

[PATCH] D65410: [PassManager] First Pass implementation at -O1 pass pipeline

2019-11-16 Thread Eric Christopher via Phabricator via cfe-commits
echristo added a comment. Ping :) CHANGES SINCE LAST ACTION https://reviews.llvm.org/D65410/new/ https://reviews.llvm.org/D65410 ___ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D62731: Add support for options -frounding-math, -ftrapping-math, -ffp-model=, and -ffp-exception-behavior=, : Specify floating point behavior

2019-11-18 Thread Eric Christopher via Phabricator via cfe-commits
echristo added a comment. In D62731#1749916 , @mibintc wrote: > Thanks I see it, I'm working on a patch. Previously there was no support for > frounding-math (unimplemented). This patch enables the option. In the IR > builder, there's a call to a runtime

[PATCH] D65410: [PassManager] First Pass implementation at -O1 pass pipeline

2019-11-25 Thread Eric Christopher via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes. Closed by commit rG8ff85ed905a7: As a follow-up to my initial mail to llvm-dev here's a first pass at the O1… (authored by echristo). Changed prior to commit: https://reviews.llvm.org/D65410?vs=228339&id=230987#toc Repos

[PATCH] D65410: [PassManager] First Pass implementation at -O1 pass pipeline

2019-11-26 Thread Eric Christopher via Phabricator via cfe-commits
This revision was not accepted when it landed; it landed in state "Needs Revision". This revision was automatically updated to reflect the committed changes. Closed by commit rGfd39b1bb20ce: Revert "Revert "As a follow-up to my initial mail to llvm-dev here's a first… (authored by echristo). Hera

[PATCH] D71320: [IR] Split out target specific intrinsic enums into separate headers

2019-12-11 Thread Eric Christopher via Phabricator via cfe-commits
echristo accepted this revision. echristo added a comment. SGTM and thank you :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71320/new/ https://reviews.llvm.org/D71320 ___ cfe-commits mailing list cf

[PATCH] D71393: Default to -fno-use-init-array

2019-12-11 Thread Eric Christopher via Phabricator via cfe-commits
echristo added a comment. Fan of this change, but let's definitely wait for more reviews :) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71393/new/ https://reviews.llvm.org/D71393 ___ cfe-commits mail

[PATCH] D71486: [clang-tools-extra] Fix switch coverage warning

2019-12-13 Thread Eric Christopher via Phabricator via cfe-commits
echristo created this revision. echristo added reviewers: Anastasia, sammccall. echristo added a project: clang-tools-extra. Herald added subscribers: cfe-commits, mcrosier. Herald added a project: clang. echristo added a subscriber: rdhindsa. ... I think this is right? Add support in the switch

[PATCH] D71486: [clang-tools-extra] Fix switch coverage warning

2019-12-13 Thread Eric Christopher via Phabricator via cfe-commits
echristo updated this revision to Diff 233857. echristo added a comment. Fix for some slightly better API uses. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71486/new/ https://reviews.llvm.org/D71486 Files: clang-tools-extra/clang-tidy/ClangTid

[PATCH] D71486: [clang-tools-extra] Fix switch coverage warning

2019-12-13 Thread Eric Christopher via Phabricator via cfe-commits
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 rG5623bd52acd3: Fix -Wswitch-coverage warning in clang-tidy after ak_addrspace introduction. (authored by echristo). Reposi

[PATCH] D71687: Fix full loop unrolling initialization in new pass manager

2019-12-18 Thread Eric Christopher via Phabricator via cfe-commits
echristo updated this revision to Diff 234656. echristo added a comment. Formatting and parens changes. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71687/new/ https://reviews.llvm.org/D71687 Files: clang/test/Misc/loop-opt-setup.c llvm/lib/P

[PATCH] D71687: Fix full loop unrolling initialization in new pass manager

2019-12-18 Thread Eric Christopher via Phabricator via cfe-commits
echristo created this revision. echristo added reviewers: chandlerc, hfinkel, asbirlea. Herald added subscribers: llvm-commits, cfe-commits, hiraditya, mcrosier. Herald added projects: clang, LLVM. echristo updated this revision to Diff 234656. echristo added a comment. Formatting and parens chang

[PATCH] D71687: Fix full loop unrolling initialization in new pass manager

2019-12-26 Thread Eric Christopher via Phabricator via cfe-commits
echristo added a comment. Ping ping goes the trolley. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71687/new/ https://reviews.llvm.org/D71687 ___ cfe-commits mailing list cfe-commits@lists.llvm.org ht

[PATCH] D71687: Fix full loop unrolling initialization in new pass manager

2020-03-13 Thread Eric Christopher via Phabricator via cfe-commits
echristo updated this revision to Diff 250229. echristo added a comment. Fixed the clang test. Tried to get something that I could reduce down and duplicate with just opt but it's been... difficult. Even the small clang testcase in isolation won't duplicate via something like: clang -O0 -fexpe

[PATCH] D71687: Fix full loop unrolling initialization in new pass manager

2020-03-13 Thread Eric Christopher via Phabricator via cfe-commits
echristo updated this revision to Diff 250232. echristo added a comment. Fix comments around full unroller. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D71687/new/ https://reviews.llvm.org/D71687 Files: clang/test/Misc/loop-opt-setup.c llvm/l

[PATCH] D76297: [clang][AArch64] readd support for 'p' inline asm constraint

2020-03-17 Thread Eric Christopher via Phabricator via cfe-commits
echristo added inline comments. Comment at: clang/test/CodeGen/inline-asm-p-constraint.c:1 +// RUN: %clang_cc1 -emit-llvm %s -o - -triple aarch64-linux-gnu | FileCheck %s +void foo(void* ptr) { nickdesaulniers wrote: > Does this need `REQUIRES: aarch64-registered

[PATCH] D70172: [CUDA][HIP][OpenMP] Emit deferred diagnostics by a post-parsing AST travese

2020-02-18 Thread Eric Christopher via Phabricator via cfe-commits
echristo added a comment. In D70172#1879481 , @jpienaar wrote: > This seems to result in triggering clang/lib/CodeGen/CGExpr.cpp:2626 when > compiling mlir/lib/Transforms/AffineDataCopyGeneration.cpp with clang build > with assertions on (clean build at

[PATCH] D44970: [XRay][clang] Add flag to choose instrumentation bundles

2018-03-28 Thread Eric Christopher via Phabricator via cfe-commits
echristo added inline comments. Comment at: clang/include/clang/Frontend/CodeGenOptions.h:113 +XRay_None,// Emit none of the instrumentation points. +XRay_FunctionExtents, // Only emit function entry/exit instrumentation + // points. -

[PATCH] D45061: [NVPTX, CUDA] Use custom feature detection to handle NVPTX target builtins.

2018-03-31 Thread Eric Christopher via Phabricator via cfe-commits
echristo added a comment. The llvm change and corresponding switch from satom->sm_60 in the front end is fine. Let's talk about the rest of it more. I'm not sure I'm seeing the need here rather than the annotations that are already here. Can you elaborate more here on why we need an additional

[PATCH] D45109: Remove -cc1 option "-backend-option"

2018-04-09 Thread Eric Christopher via Phabricator via cfe-commits
echristo accepted this revision. echristo added a comment. This revision is now accepted and ready to land. Uh sure. :) Though I'd almost do it the other way around. That said, -mllvm probably has more uptake. Repository: rC Clang https://reviews.llvm.org/D45109 _

[PATCH] D45061: [NVPTX, CUDA] Improved feature constraints on NVPTX target builtins.

2018-04-10 Thread Eric Christopher via Phabricator via cfe-commits
echristo accepted this revision. echristo added a comment. Guessing that SM_60 (etc) are defines? Anyhow LGTM. I'm not sure you can split up the satom part of the patch, but if you can that'd be great. -eric https://reviews.llvm.org/D45061 ___ cf

[PATCH] D45474: [XRay][clang+compiler-rt] Support build-time mode selection

2018-04-10 Thread Eric Christopher via Phabricator via cfe-commits
echristo accepted this revision. echristo added a comment. This revision is now accepted and ready to land. Sure. Comment at: clang/lib/Driver/XRayArgs.cpp:168 - for (const auto&AttrFile : AttrListFiles) { + for (const auto &AttrFile : AttrListFiles) { SmallString<64>

[PATCH] D44970: [XRay][clang] Add flag to choose instrumentation bundles

2018-04-10 Thread Eric Christopher via Phabricator via cfe-commits
echristo added inline comments. Comment at: clang/include/clang/Frontend/CodeGenOptions.h:110 + enum XRayInstrumentationPointBundle { +XRay_All, // Always emit all the instrumentation points. dberris wrote: > pelikan wrote: > > To me, this feel

[PATCH] D39341: [X86][Driver] Move all of the X86 feature flags to one spot in the Options.td file and pair them up with their negations.

2017-10-26 Thread Eric Christopher via Phabricator via cfe-commits
echristo accepted this revision. echristo added a comment. This revision is now accepted and ready to land. LGTM. Thanks. https://reviews.llvm.org/D39341 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/l

[PATCH] D40228: [Target] Keep the TargetOptions feature list sorted instead of sorting during CodeGen

2017-11-20 Thread Eric Christopher via Phabricator via cfe-commits
echristo accepted this revision. echristo added a comment. This revision is now accepted and ready to land. This seems strictly more difficult to keep under control? Though I guess the assert helps. Feel free to go ahead, but... https://reviews.llvm.org/D40228 __

[PATCH] D40228: [Target] Keep the TargetOptions feature list sorted instead of sorting during CodeGen

2017-11-20 Thread Eric Christopher via Phabricator via cfe-commits
echristo added a comment. No strong opinion. I think I like that one more though. https://reviews.llvm.org/D40228 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D28037: [PowerPC, DAGCombiner] Change vec_sl to a << (b % (sizeof(a) * 8)), and fold it back to a << b.

2017-01-03 Thread Eric Christopher via Phabricator via cfe-commits
echristo added a comment. One inline comment for the clang part and then let's split the llvm part out into a separate patch please. Thanks! -eric Comment at: clang/lib/Headers/altivec.h:8045 /* vec_sl */ static __inline__ vector unsigned char __ATTRS_o_ai --

[PATCH] D28037: [Altivec] Change vec_sl to a << (b % (sizeof(a) * 8))

2017-01-04 Thread Eric Christopher via Phabricator via cfe-commits
echristo added a comment. LGTM. Thanks! -eric https://reviews.llvm.org/D28037 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D27872: [APFloat] Switch from (PPCDoubleDoubleImpl, IEEEdouble) layout to (IEEEdouble, IEEEdouble)

2017-01-04 Thread Eric Christopher via Phabricator via cfe-commits
echristo added a comment. In https://reviews.llvm.org/D27872#628212, @timshen wrote: > I changed type style to early return. > > For constructors and destructors, I use: > > if (...) { > // statement; > return; > } > > > For normal functions that returns void, I chose: > > if (..

[PATCH] D27872: [APFloat] Switch from (PPCDoubleDoubleImpl, IEEEdouble) layout to (IEEEdouble, IEEEdouble)

2017-01-04 Thread Eric Christopher via Phabricator via cfe-commits
echristo added a comment. In https://reviews.llvm.org/D27872#636147, @timshen wrote: > In https://reviews.llvm.org/D27872#636130, @echristo wrote: > > > Looks pretty weird. Typically I'd suggest just: > > > > if (foo) { > > > > Foo(); > > return; > > > > } > > > > since that will keep cogniti

[PATCH] D28409: Use CodegenOpts::less when creating a TargetMachine for clang `-O1`

2017-01-06 Thread Eric Christopher via Phabricator via cfe-commits
echristo accepted this revision. echristo added a comment. This revision is now accepted and ready to land. Sounds fine to me. I want to modify https://reviews.llvm.org/owners/package/1/ at some point, but there's nothing wrong with consistency for now. https://reviews.llvm.org/D28409 __

[PATCH] D27872: [APFloat] Switch from (PPCDoubleDoubleImpl, IEEEdouble) layout to (IEEEdouble, IEEEdouble)

2017-01-06 Thread Eric Christopher via Phabricator via cfe-commits
echristo added a reviewer: scanon. echristo added a comment. Adding Steve in an attempt to get him to review :) https://reviews.llvm.org/D27872 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cf

[PATCH] D25435: Add -fdebug-info-for-profiling to emit more debug info for sample pgo profile collection

2017-01-09 Thread Eric Christopher via Phabricator via cfe-commits
echristo added a comment. I don't think this ever was hashed out in the llvm-dev thread? https://reviews.llvm.org/D25435 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D30415: Fix -mno-altivec cannot overwrite -maltivec option

2017-03-16 Thread Eric Christopher via Phabricator via cfe-commits
echristo requested changes to this revision. echristo added a comment. This revision now requires changes to proceed. Different suggestion: Remove the faltivec option. Even gcc doesn't support it anymore afaict. (Go ahead and commit the zvector part if you'd like). -eric https://reviews.llvm.

[PATCH] D30415: Fix -mno-altivec cannot overwrite -maltivec option

2017-03-18 Thread Eric Christopher via Phabricator via cfe-commits
echristo added a comment. In https://reviews.llvm.org/D30415#703652, @uweigand wrote: > In https://reviews.llvm.org/D30415#703442, @hfinkel wrote: > > > In https://reviews.llvm.org/D30415#703398, @echristo wrote: > > > > > Different suggestion: > > > > > > Remove the faltivec option. Even gcc doe

[PATCH] D30415: Fix -mno-altivec cannot overwrite -maltivec option

2017-03-20 Thread Eric Christopher via Phabricator via cfe-commits
echristo added a comment. In https://reviews.llvm.org/D30415#705196, @uweigand wrote: > In https://reviews.llvm.org/D30415#704761, @echristo wrote: > > > In https://reviews.llvm.org/D30415#703652, @uweigand wrote: > > > > > I'm a bit confused by this discussion. -faltivec and -maltivec are > >

[PATCH] D30415: Fix -mno-altivec cannot overwrite -maltivec option

2017-03-20 Thread Eric Christopher via Phabricator via cfe-commits
echristo added a comment. In https://reviews.llvm.org/D30415#705889, @echristo wrote: > In https://reviews.llvm.org/D30415#705196, @uweigand wrote: > > > In https://reviews.llvm.org/D30415#704761, @echristo wrote: > > > > > In https://reviews.llvm.org/D30415#703652, @uweigand wrote: > > > > > > >

[PATCH] D30760: Record command lines in objects built by clang

2017-03-21 Thread Eric Christopher via Phabricator via cfe-commits
echristo requested changes to this revision. echristo added a comment. This revision now requires changes to proceed. Why aren't we passing the flags down as a string in the IR? Needs a testcase: you should only check for IR in clang tests. To make sure something is propagated to the other end u

[PATCH] D31213: Add support for -fno-auto-profile and -fno-profile-sample-use

2017-03-21 Thread Eric Christopher via Phabricator via cfe-commits
echristo added a comment. Can you add a test for fno-auto-profile? https://reviews.llvm.org/D31213 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D31213: Add support for -fno-auto-profile and -fno-profile-sample-use

2017-03-21 Thread Eric Christopher via Phabricator via cfe-commits
echristo accepted this revision. echristo added a comment. This revision is now accepted and ready to land. LGTM. -eric https://reviews.llvm.org/D31213 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/li

[PATCH] D30415: Fix -mno-altivec cannot overwrite -maltivec option

2017-03-21 Thread Eric Christopher via Phabricator via cfe-commits
echristo added a comment. In https://reviews.llvm.org/D30415#706370, @sfertile wrote: > > I have a patch to do this now. I'll plan on committing it in a bit. > > Is there a way to mark the -f form of the option as deprecated? We should > warn and suggest users switch to the -maltivec option for

[PATCH] D30760: Record command lines in objects built by clang, Clang part

2017-03-23 Thread Eric Christopher via Phabricator via cfe-commits
echristo added a comment. Needs more testing. Might want to make sure that you actually are recording some useful command line options and that you're looking at the cc1 command line. This should also be a Driver test and not CodeGen. You can then use -### to inspect the command lines as passe

[PATCH] D30760: Record command lines in objects built by clang, Clang part

2017-03-24 Thread Eric Christopher via Phabricator via cfe-commits
echristo added inline comments. Comment at: test/Driver/debug-options.c:201-202 // +// GRECORD: "-dwarf-debug-flags" +// GRECORD: -### -c -grecord-gcc-switches +// This seems a little light on the testing, would you mind adding some more interesting lines here?

[PATCH] D30760: Record command lines in objects built by clang, Clang part

2017-03-29 Thread Eric Christopher via Phabricator via cfe-commits
echristo added inline comments. Comment at: test/Driver/debug-options.c:201-202 // +// GRECORD: "-dwarf-debug-flags" +// GRECORD: -### -c -grecord-gcc-switches +// george.burgess.iv wrote: > echristo wrote: > > This seems a little light on the testing, would you

[PATCH] D30760: Record command lines in objects built by clang, Clang part

2017-03-29 Thread Eric Christopher via Phabricator via cfe-commits
echristo accepted this revision. echristo added a comment. This revision is now accepted and ready to land. Sounds good. Do you have commit access? https://reviews.llvm.org/D30760 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.

[PATCH] D30760: Record command lines in objects built by clang, Clang part

2017-03-29 Thread Eric Christopher via Phabricator via cfe-commits
echristo added a comment. Committed thusly: echristo@athyra ~/s/l/t/clang> git svn dcommit Committing to https://llvm.org/svn/llvm-project/cfe/trunk ... M lib/Driver/ToolChains/Clang.cpp M test/Driver/debug-options.c Committed r299037 https://reviews.llvm.org/D30760

[PATCH] D31440: PR32382: Adapt to LLVM changes in DIExpression.

2017-04-11 Thread Eric Christopher via Phabricator via cfe-commits
echristo requested changes to this revision. echristo added a comment. This revision now requires changes to proceed. Sounds like Dave is asking for changes so I'll put it down as Request Changes to get it out of my queue. :) https://reviews.llvm.org/D31440 __

[PATCH] D25435: Add -fdebug-info-for-profiling to emit more debug info for sample pgo profile collection

2017-01-18 Thread Eric Christopher via Phabricator via cfe-commits
echristo accepted this revision. echristo added a comment. Thanks for explaining all of this and going through it Dehao. LGTM. -eric https://reviews.llvm.org/D25435 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-

[PATCH] D28037: [Altivec] Change vec_sl to a << (b % (sizeof(a) * 8))

2017-01-20 Thread Eric Christopher via Phabricator via cfe-commits
echristo added a comment. Going to commit this? https://reviews.llvm.org/D28037 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D27872: [APFloat] Switch from (PPCDoubleDoubleImpl, IEEEdouble) layout to (IEEEdouble, IEEEdouble)

2017-01-20 Thread Eric Christopher via Phabricator via cfe-commits
echristo added a comment. Few comments inline. Generally looks OK, I do share Hal's comment on finding some way of simplifying the if (someSemantics) ... if (otherSemantics) ... llvm_unreachable(...) pattern. What did you have in mind? Comment at: llvm/include/llvm/ADT/APFlo

[PATCH] D27872: [APFloat] Switch from (PPCDoubleDoubleImpl, IEEEdouble) layout to (IEEEdouble, IEEEdouble)

2017-01-23 Thread Eric Christopher via Phabricator via cfe-commits
echristo accepted this revision. echristo added a comment. This revision is now accepted and ready to land. This looks fine to me now, might be good to get someone else to ack as well though. https://reviews.llvm.org/D27872 ___ cfe-commits mailing

[PATCH] D27872: [APFloat] Switch from (PPCDoubleDoubleImpl, IEEEdouble) layout to (IEEEdouble, IEEEdouble)

2017-01-23 Thread Eric Christopher via Phabricator via cfe-commits
echristo added a comment. Hal has given an ack (offline) as well. Go ahead Tim. https://reviews.llvm.org/D27872 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D26546: [PPC] Add vec_insert4b/vec_extract4b to altivec.h

2017-01-30 Thread Eric Christopher via Phabricator via cfe-commits
echristo closed this revision. echristo added a comment. This was committed: commit d65cd1f9424369c4ae7f945fac7fd9e4357451b2 Author: Sean Fertile Date: Thu Jan 5 21:43:30 2017 + Add vec_insert4b and vec_extract4b functions to altivec.h Add builtins for the functions and custom code

[PATCH] D29770: [Assembler] Inline assembly diagnostics test.

2017-02-09 Thread Eric Christopher via Phabricator via cfe-commits
echristo added a comment. Can we not get llc to use the diags interfaces here? https://reviews.llvm.org/D29770 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33820: [PowerPC] Pass CPU to assembler with -no-integrated-as

2017-06-29 Thread Eric Christopher via Phabricator via cfe-commits
echristo accepted this revision. echristo added a comment. This revision is now accepted and ready to land. Sorry, when I say "One inline comment otherwise LGTM" feel free to commit after fixing :) Since you have, then LGTM. -eric Repository: rL LLVM https://reviews.llvm.org/D33820

[PATCH] D34425: Unified ARM logic for computing target ABI.

2017-06-29 Thread Eric Christopher via Phabricator via cfe-commits
echristo added a comment. I've committed this for you as: M lib/Driver/ToolChains/Clang.cpp M test/CodeGen/arm-v8.1a-neon-intrinsics.c M test/CodeGen/named_reg_global.c M test/CodeGen/neon-immediate-ubsan.c M test/CodeGen/xray-attribut

[PATCH] D35131: Prevent ClangTools from generating dependency files.D34304 created a way for ToolInvocations to conditionally generatedependency files, and updated call sites to preserve the old behav

2017-07-14 Thread Eric Christopher via Phabricator via cfe-commits
echristo accepted this revision. echristo added a comment. This revision is now accepted and ready to land. This sort of thing seems obvious in the future :) -eric https://reviews.llvm.org/D35131 ___ cfe-commits mailing list cfe-commits@lists.llvm.

[PATCH] D35519: Add SEMA checking to attribute 'target'

2017-07-18 Thread Eric Christopher via Phabricator via cfe-commits
echristo added a comment. If you wouldn't mind I'd like to see this separated out a bit - a) I think we can make the attribute info a struct rather than a pair as a simple change first b) setCPU split c) let's see where we are after that? Also the description makes it sound like you're adding su

[PATCH] D35449: [X86] Implement __builtin_cpu_is

2017-07-18 Thread Eric Christopher via Phabricator via cfe-commits
echristo added a subscriber: erichkeane. echristo added a comment. Adding Erich Keane here on this since he's working on something similar for the target attribute. -eric https://reviews.llvm.org/D35449 ___ cfe-commits mailing list cfe-commits@lis

[PATCH] D35449: [X86] Implement __builtin_cpu_is

2017-07-18 Thread Eric Christopher via Phabricator via cfe-commits
echristo added a comment. From my perspective here once you and Erich get some agreement on the checking between your two bits I'm fine :) -eric https://reviews.llvm.org/D35449 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.l

[PATCH] D35449: [X86] Implement __builtin_cpu_is

2017-07-18 Thread Eric Christopher via Phabricator via cfe-commits
echristo added a comment. FWIW the duplication in CGCall.cpp of the enum set is painful if you can come up with anything else it'd be awesome. I don't have any good ideas, just a fond wish :) https://reviews.llvm.org/D35449 ___ cfe-commits mailing

[PATCH] D35572: Add isValidCPUName and isValidFeature to TargetInfo

2017-07-18 Thread Eric Christopher via Phabricator via cfe-commits
echristo added a comment. Can you update the code in CGBuiltin to use this as part of this patch? :) Thanks! https://reviews.llvm.org/D35572 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-

[PATCH] D35573: Improve SEMA for attribute-target

2017-07-18 Thread Eric Christopher via Phabricator via cfe-commits
echristo accepted this revision. echristo added a comment. This revision is now accepted and ready to land. LGTM pending dependency approval https://reviews.llvm.org/D35573 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.or

[PATCH] D35574: Convert attribute 'target' parsing from a 'pair' to a 'struct' to make further improvements easier

2017-07-18 Thread Eric Christopher via Phabricator via cfe-commits
echristo accepted this revision. echristo added a comment. This revision is now accepted and ready to land. LGTM. https://reviews.llvm.org/D35574 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/

[PATCH] D35572: Add isValidCPUName and isValidFeature to TargetInfo

2017-07-19 Thread Eric Christopher via Phabricator via cfe-commits
echristo added a comment. In https://reviews.llvm.org/D35572#813520, @erichkeane wrote: > In https://reviews.llvm.org/D35572#813369, @echristo wrote: > > > Can you update the code in CGBuiltin to use this as part of this patch? :) > > > > Thanks! > > > I'm not sure which you're referring to? I d

[PATCH] D35572: Add isValidCPUName and isValidFeature to TargetInfo

2017-07-19 Thread Eric Christopher via Phabricator via cfe-commits
echristo added a comment. LGTM. -eric https://reviews.llvm.org/D35572 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D35708: Remove Bitrig: Clang Changes

2017-07-20 Thread Eric Christopher via Phabricator via cfe-commits
echristo accepted this revision. echristo added a comment. This revision is now accepted and ready to land. LGTM https://reviews.llvm.org/D35708 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/c

[PATCH] D35709: Remove Bitrig: CompilerRT Changes

2017-07-20 Thread Eric Christopher via Phabricator via cfe-commits
echristo accepted this revision. echristo added a comment. LGTM. https://reviews.llvm.org/D35709 ___ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D35577: Add -flookup-tables and -fno-lookup-tables flags

2017-07-20 Thread Eric Christopher via Phabricator via cfe-commits
echristo added a comment. So, what's the overall logical idea behind the need for this option? While I understand that you don't want people just doing this via the -mllvm set of options, but if you're just doing this to provide a temporary option rather than turning it off in the backend I'm n

[PATCH] D35701: Break up Targets.cpp into a header/impl pair per target type[NFCI]

2017-07-21 Thread Eric Christopher via Phabricator via cfe-commits
echristo accepted this revision. echristo added a comment. This revision is now accepted and ready to land. I'm going to say this ahead of time without looking into it "LGTM", but wait for ctopper (or someone else) to ack it for style etc since I'm unlikely to get to it any time shortly. :) Als

[PATCH] D35577: Add -flookup-tables and -fno-lookup-tables flags

2017-07-21 Thread Eric Christopher via Phabricator via cfe-commits
echristo added a comment. In https://reviews.llvm.org/D35577#817267, @sgundapa wrote: > The discussion is scattered across these patches > https://reviews.llvm.org/D35578 and https://reviews.llvm.org/D35579. > I will provide a brief summary here: > > The idea is to control the generation of dat

<    3   4   5   6   7   8   9   >