Page MenuHomePhabricator

echristo (Eric Christopher)
User

Projects

User Details

User Since
Oct 15 2012, 2:12 PM (414 w, 4 d)

Recent Activity

Fri, Sep 18

echristo added a comment to rG549e55b3d563: Temporarily Revert "[clangd] Add Random Forest runtime for code completion.".

:)

Fri, Sep 18, 7:16 PM
echristo added a comment to rG549e55b3d563: Temporarily Revert "[clangd] Add Random Forest runtime for code completion.".

Which header do you think is missing? It built fine on several machines over here.

Fri, Sep 18, 6:22 PM
echristo added a reverting change for rGc8757ff3aa7d: RegAllocFast: Rewrite and improve: rGdbd53a1f0c93: Temporarily Revert "RegAllocFast: Rewrite and improve".
Fri, Sep 18, 6:11 PM
echristo committed rGdbd53a1f0c93: Temporarily Revert "RegAllocFast: Rewrite and improve" (authored by echristo).
Temporarily Revert "RegAllocFast: Rewrite and improve"
Fri, Sep 18, 6:11 PM
echristo added a reverting change for rG9b6765e784b3: [clangd] Add Random Forest runtime for code completion.: rG549e55b3d563: Temporarily Revert "[clangd] Add Random Forest runtime for code completion.".
Fri, Sep 18, 2:50 PM
echristo committed rG549e55b3d563: Temporarily Revert "[clangd] Add Random Forest runtime for code completion." (authored by echristo).
Temporarily Revert "[clangd] Add Random Forest runtime for code completion."
Fri, Sep 18, 2:50 PM
echristo added a reverting change for D83814: [clangd] Add Random Forest runtime for code completion.: rG549e55b3d563: Temporarily Revert "[clangd] Add Random Forest runtime for code completion.".
Fri, Sep 18, 2:50 PM · Restricted Project
echristo added a reverting change for rG455ca0ebb692: [SLP] Allow reordering of vectorization trees with reused instructions.: rGecfd8161bf43: Temporarily Revert "[SLP] Allow reordering of vectorization trees with reused….
Fri, Sep 18, 12:50 PM
echristo committed rGecfd8161bf43: Temporarily Revert "[SLP] Allow reordering of vectorization trees with reused… (authored by echristo).
Temporarily Revert "[SLP] Allow reordering of vectorization trees with reused…
Fri, Sep 18, 12:50 PM
echristo added a reverting change for D45263: [SLP] Allow reordering of vectorization trees with reused instructions.: rGecfd8161bf43: Temporarily Revert "[SLP] Allow reordering of vectorization trees with reused….
Fri, Sep 18, 12:50 PM · Restricted Project

Thu, Sep 17

echristo accepted D86485: [test] Fix FullUnroll.ll.
In D86485#2277714, @rnk wrote:

Here is the sequence of what happened as I understand it:

  • D71687 is intended to fix the full loop unrolling pragma with the NPM at -O1. The accompanying clang test covers this.
  • D71687 also adds an LLVM IR test. It is an integration test: it started from unoptimized IR and ran the full -O1 NPM pipeline. The IR happens to contain optnone. Maybe that was unintentional, since one used to be able to run clang foo.c -emit-llvm -o - | opt -O1 -S and get optimized IR. Now clang applies the optnone attribute, and that no longer works.
  • D85578 made the IR test more targeted: now it only runs the full unrolling pass instead of the O1 pipeline.

There is value in integration tests, so maybe the thing to do is to bring back the IR integration test in addition to this unit test. The unit test seems correct to me. If optnone is present, I would not expect the loop unroll metadata to be honored.

Thu, Sep 17, 11:09 AM · Restricted Project

Wed, Sep 16

echristo committed rGc14032281980: Use zu rather than llu format specifier for size_t (-Wformat warning fix). (authored by echristo).
Use zu rather than llu format specifier for size_t (-Wformat warning fix).
Wed, Sep 16, 7:28 PM

Wed, Sep 2

echristo updated subscribers of D86485: [test] Fix FullUnroll.ll.

I don't think your statement matches what I was seeing. I may have
constructed the testcase poorly, but there were differences being shown.
Also keep in mind that your clang run is testing what I've already fixed. :)

Wed, Sep 2, 9:46 AM · Restricted Project

Tue, Sep 1

echristo accepted D86839: [CMake] Remove -Wl,-allow-shlib-undefined which was added in rL221530.

I think this is the right thing to do here. In addition, we should not cherry pick this to any release branch so we have the opportunity to get it more fully evaluated by distros/vendors/etc over the next months.

Tue, Sep 1, 7:47 PM · Restricted Project

Sat, Aug 29

echristo accepted D86846: [NFC] Adding pythonenv* to .gitignore.
Sat, Aug 29, 9:37 PM · Restricted Project

Fri, Aug 28

echristo accepted D86505: [RFC][Target] Add a new triple called Triple::csky.
Fri, Aug 28, 9:27 AM · Restricted Project

Aug 26 2020

echristo committed rGea7b1c79f73d: Add cmake test support for LLJITWithThinLTOSummaries to make sure (authored by echristo).
Add cmake test support for LLJITWithThinLTOSummaries to make sure
Aug 26 2020, 11:47 AM

Aug 24 2020

echristo added a reverting change for rG589ce5f7050d: [DebugInfo] Move constructor homing case in shouldOmitDefinition.: rG05777ab94106: Temporarily Revert "[DebugInfo] Move constructor homing case in….
Aug 24 2020, 9:54 PM
echristo committed rG05777ab94106: Temporarily Revert "[DebugInfo] Move constructor homing case in… (authored by echristo).
Temporarily Revert "[DebugInfo] Move constructor homing case in…
Aug 24 2020, 9:54 PM
echristo added a reverting change for D86491: [DebugInfo] Move constructor homing case in shouldOmitDefinition.: rG05777ab94106: Temporarily Revert "[DebugInfo] Move constructor homing case in….
Aug 24 2020, 9:54 PM · Restricted Project
echristo committed rG1a2a34a38a7d: Add NDEBUG checks around debug only loop to avoid unused variable (authored by echristo).
Add NDEBUG checks around debug only loop to avoid unused variable
Aug 24 2020, 5:59 PM
echristo added a comment to D86485: [test] Fix FullUnroll.ll.

The code has a pragma to turn on loop unrolling. I actually did want to check with optnone.

Aug 24 2020, 2:24 PM · Restricted Project

Aug 23 2020

echristo accepted D86387: Fix frame pointer layout on AArch64 Linux..

LGTM from maintainability etc. Haven't really checked more so might want to wait for others.

Aug 23 2020, 12:35 PM · Restricted Project

Aug 21 2020

echristo added inline comments to D86387: Fix frame pointer layout on AArch64 Linux..
Aug 21 2020, 4:45 PM · Restricted Project
echristo added a comment to D84108: [SimplifyCFG][LoopRotate] SimplifyCFG: disable common instruction hoisting by default, enable late in pipeline.

I would have been using old, the default. I think we still see some performance/codesize problems with switch to new.

That might be the pattern then, just another old-pm vs new-pm difference for the switch to track.
But i'm not seeing ab umbrella bug for performance regressions on https://bugs.llvm.org/show_bug.cgi?id=46649?

One more follow up here:

One set of benchmarks we're seeing this in are also in the llvm test harness:

llvm_multisource_tsvc.TSVC-ControlFlow-flt and llvm_multisource_misc.Ptrdist-yacr2

in an FDO mode (so we have some decent branch hints for the code).

You should be able to duplicate at least some of the slowdown there.

That sadly doesn't tell me much.
Can you please provide the reproduction steps, and ideally the IR with/without this change?

It should be as straightforward as -O3 -fexperimental-new-pass-manager, if that isn't then we'll look a bit more. Those tests are in the llvm test-suite so they should be easy to get as part of your checkout.


Uhm, for N=9, i'm afraid i'm not seeing anything horrible like that:

$ /repositories/llvm-test-suite/utils/compare.py --lhs-name old-newpm --rhs-name new-newpm llvm-test-suite-old-NewPM/res-*.json vs llvm-test-suite-new-NewPM/res-*.json --merge-average --filter-hash
Tests: 41
Metric: exec_time

Program                                        old-newpm new-newpm diff 
 test-suite...marks/Ptrdist/yacr2/yacr2.test     0.77      0.74    -4.0%
 test-suite...s/Ptrdist/anagram/anagram.test     0.76      0.78     3.0%
 test-suite...flt/InductionVariable-flt.test     2.90      2.95     1.9%
 test-suite...lFlow-flt/ControlFlow-flt.test     3.48      3.44    -1.4%
 test-suite...ing-dbl/NodeSplitting-dbl.test     3.09      3.13     1.1%
 test-suite...dbl/InductionVariable-dbl.test     4.02      4.05     1.0%
 test-suite...pansion-dbl/Expansion-dbl.test     2.49      2.51     0.7%
 test-suite...pansion-flt/Expansion-flt.test     1.56      1.57     0.5%
 test-suite...lt/CrossingThresholds-flt.test     2.86      2.87     0.5%
 test-suite...lFlow-dbl/ControlFlow-dbl.test     4.05      4.03    -0.5%
 test-suite...ing-flt/Equivalencing-flt.test     1.13      1.14     0.3%
 test-suite...ow-dbl/GlobalDataFlow-dbl.test     3.20      3.21     0.3%
 test-suite.../Benchmarks/Ptrdist/bc/bc.test     0.40      0.40     0.3%
 test-suite...C/Packing-flt/Packing-flt.test     2.90      2.91     0.3%
 test-suite.../Benchmarks/Ptrdist/ft/ft.test     1.05      1.05     0.3%
 Geomean difference                                                 0.1%
       old-newpm  new-newpm       diff
count  41.000000  41.000000  41.000000
mean   2.761491   2.765264   0.001355 
std    1.382334   1.383351   0.009172 
min    0.400356   0.401544  -0.040124 
25%    2.158678   2.161544  -0.000219 
50%    2.836133   2.839078   0.000974 
75%    3.204467   3.214600   0.002932 
max    6.865056   6.881911   0.029819 

@echristo or does that match what you were seeing for test-suite? is the Ptrdist/anagram regression the one?

Aug 21 2020, 8:41 AM · Restricted Project
echristo added a comment to D84108: [SimplifyCFG][LoopRotate] SimplifyCFG: disable common instruction hoisting by default, enable late in pipeline.

I would have been using old, the default. I think we still see some performance/codesize problems with switch to new.

Aug 21 2020, 8:29 AM · Restricted Project
echristo added a comment to D84108: [SimplifyCFG][LoopRotate] SimplifyCFG: disable common instruction hoisting by default, enable late in pipeline.

I think you're still in scientific computing yes?

Embedded computing actually. Low power Arm devices, but I'm always interested in all uses of Arm devices. I have been working in the vectorizer a lot lately, and a number of the benchmarks we usually run are DSP style codes, so quite similar to HPC. But the improvements I saw included CPU's without any vectorization. Codesize was also flat in all, which I took as a good sign that the patch wasn't bad for general codegen.

Aug 21 2020, 8:29 AM · Restricted Project

Aug 20 2020

echristo added a comment to D84108: [SimplifyCFG][LoopRotate] SimplifyCFG: disable common instruction hoisting by default, enable late in pipeline.

We saw performance improvements from this patch. They were a little bit up-and-down, but definitely an improvement overall.

Aug 20 2020, 7:56 AM · Restricted Project
echristo added a comment to D84108: [SimplifyCFG][LoopRotate] SimplifyCFG: disable common instruction hoisting by default, enable late in pipeline.

One more follow up here:

One set of benchmarks we're seeing this in are also in the llvm test harness:

llvm_multisource_tsvc.TSVC-ControlFlow-flt and llvm_multisource_misc.Ptrdist-yacr2

in an FDO mode (so we have some decent branch hints for the code).

You should be able to duplicate at least some of the slowdown there.

That sadly doesn't tell me much.
Can you please provide the reproduction steps, and ideally the IR with/without this change?

Aug 20 2020, 7:53 AM · Restricted Project

Aug 19 2020

echristo added a comment to D84108: [SimplifyCFG][LoopRotate] SimplifyCFG: disable common instruction hoisting by default, enable late in pipeline.

One more follow up here:

Aug 19 2020, 4:12 PM · Restricted Project
echristo added a comment to D86045: [llvm-dwarfdump] --statistics: switch to json::OStream. NFC.

I feel pretty OK approving this.

(Tradeoffs I'd contemplated and discarded was renaming OS rather than OS->J everywhere. I ended up liking the readability of J more. JS might be nice as well?)

-eric

Do you mean removing the OS->J rename? That would be a bit ambiguous because people usually use OS to refer to raw_ostream and its derivatives. The JSON stream is special.

Aug 19 2020, 4:07 PM · Restricted Project
echristo added a comment to D84108: [SimplifyCFG][LoopRotate] SimplifyCFG: disable common instruction hoisting by default, enable late in pipeline.

So, we're seeing several significant (20-30%) regressions due to this in various different library benchmarks. Usually around things that are compression/decompression loops, but also other places.

Aug 19 2020, 4:02 PM · Restricted Project
echristo accepted D86045: [llvm-dwarfdump] --statistics: switch to json::OStream. NFC.

I feel pretty OK approving this.

Aug 19 2020, 2:48 PM · Restricted Project

Aug 18 2020

echristo updated subscribers of D85384: [X86] Add basic support for -mtune command line option in clang.

This would be because at that point the default cpu was that and it
probably had something to do with fallbacks.

Aug 18 2020, 10:17 PM · Restricted Project
echristo accepted D86132: [clang][driver]Add quotation mark in test/fortran.f95 to avoid false positive.
Aug 18 2020, 12:56 PM · Restricted Project

Aug 17 2020

echristo committed rGc6464c819e66: Fix another Wsign-comparison warning. (authored by echristo).
Fix another Wsign-comparison warning.
Aug 17 2020, 2:33 PM
echristo committed rG1bf0732443ee: Fix Wsign-compare warnings in test. (authored by echristo).
Fix Wsign-compare warnings in test.
Aug 17 2020, 2:00 PM

Aug 11 2020

echristo added a comment to D84068: AMDGPU/clang: Search resource directory for device libraries.

Hi All,

Aug 11 2020, 10:56 AM
echristo committed rG8155cb27a232: Fold Opcode into assert uses to fix an unused variable warning without asserts. (authored by echristo).
Fold Opcode into assert uses to fix an unused variable warning without asserts.
Aug 11 2020, 9:31 AM

Aug 10 2020

echristo committed rG9564821144f8: Test requires a debug build to pass. (authored by echristo).
Test requires a debug build to pass.
Aug 10 2020, 11:58 PM
echristo updated subscribers of D84334: [flang] Version information in flang/f18.

I'm also not sure this is passing?

Aug 10 2020, 11:17 PM · Restricted Project, Restricted Project

Aug 9 2020

echristo committed rGb529c5270c99: Add override to fix -Winconsistent-missing-override warning. (authored by echristo).
Add override to fix -Winconsistent-missing-override warning.
Aug 9 2020, 10:22 PM

Aug 6 2020

echristo updated subscribers of D84250: [InstSimplify] ExpandBinOp should fold an expression only when it's safe.

I think that we should revert the original patch for now since we have a
known miscompile and a chain of fixes are required to fix it.

Aug 6 2020, 4:35 PM · Restricted Project
echristo updated subscribers of D84825: [ELF] Change tombstone values to (.debug_ranges/.debug_loc) 1 and (other .debug_*) 0.

*thumbs up on ok for release*

Aug 6 2020, 3:33 PM · Restricted Project
echristo accepted D84825: [ELF] Change tombstone values to (.debug_ranges/.debug_loc) 1 and (other .debug_*) 0.

This looks good to me. Thanks for seeing this through Ray!

Aug 6 2020, 3:21 PM · Restricted Project

Aug 5 2020

echristo added a comment to D84825: [ELF] Change tombstone values to (.debug_ranges/.debug_loc) 1 and (other .debug_*) 0.

At which point we are at an impasse and the default behavior applies. Please revert completely back to the original behavior immediately.

Thanks.

With respect I think the "request for changes" blocked the change and I am not inclined to agree we would otherwise be at an impasse without the "request for changes".
I would likely get an "Accept Revision" or a just textual "LGTM", even if there is a conditional request that ".debug_* should use 0 as well, like GNU ld". I have mentioned that my position has been weakened. (I made the reply after 5 days, not my usual style as you can tell, because the related threads have caused so much stress/pain to me. I would otherwise not be able to focus on real work. In some sense, I feel that I am fighting a really meaningless battle, even the original proposers have not said something on this patch - Why do I still care about the resolution so much? The all things make me feel I can't really make a fair decision on this patch. I regret that I was involved in the original implementation. I would definitely better if I were just a reviewer)

I sent the patch instead of reverting 3 or 4 intertwined dependent patches because I think reverting will just make the situation more confusing. I have experienced the review processes of all relevant patches and have read all the context,
so I very responsibly state again that "a simple revert of 3 or 4 dependent patches will just make the situation more confusing".

I will very appreciate feedback from other people, not my colleague, especially LLD contributors such as @psmith @grimar @jhenderson, whether we really need a flag in this patch or can we simply defer it to another thread.

Aug 5 2020, 11:38 AM · Restricted Project

Aug 4 2020

echristo added a comment to D84825: [ELF] Change tombstone values to (.debug_ranges/.debug_loc) 1 and (other .debug_*) 0.

Please implement a clear flag for

  • BFD semantics
  • New (-1/-2) semantics

And default to the BFD semantics for now.

The change in semantics is not ready to ship by default - there are lots of DWARF consumers out there and lots of people don't update them in lock-step with their compiler.

I don't know what the request is. The patch implemented:

  • .debug_ranges & .debug_loc: -2
  • .debug_*: 0 (GNU ld semantics)

If you meant full BFD semantics:

  • .debug_ranges: 1
  • .debug_*: 0

Yes, that's what I meant. (though probably applying the debug_ranges workaround to debug_loc as well would be suitable but not necessary, I think)

Did you mean

- * .debug_ranges & .debug_loc: -2
+ * .debug_ranges & .debug_loc: 1

?
I think this has gone too far a step. We really need more evidence that 1 can make DWARF consumers happier.

That's fairly easy - 0, 0 terminates a location list the same way it terminates a range list - llvm-dwarfdump, for instance, then tries to parse the following expression as the start of a new location list and errors. binutils objdump only dumps location lists that are referenced from debug_info, printint out "holes" when the location list terminates early.

eg:

$ g++-tot loc.cpp -g -ffunction-sections -Wl,-gc-sections -O3 -fuse-ld=bfd
$ llvm-dwarfdump-tot a.out
.debug_loc contents:
0x00000000: 
            (0x0040102000040402, 0x0040102000000000): 
            (0x10209f3300020000, 0x1023000000000040): <decoding error> 00 00 00 00 02 00 37 9f 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 02 04 04 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 02 00 33 9f 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
            (0x000000009f370002, 0x0000000000000000): error: unexpected end of data at offset 0x78 while reading [0x76, 0x7e)
$ objdump -g a.out | grep -A50 debug_loc
objdump: Warning: Hole and overlap detection requires adjacent view lists and loclists.
objdump: Warning: Contents of the .debug_loc section (loaded from a.out):
There are 40 unused bytes at the end of section .debug_loc

    Offset   Begin            End              Expression

    00000000 v000000000000002 v000000000000004 location view pair
    00000002 v000000000000004 v000000000000000 location view pair

    00000004 v000000000000002 v000000000000004 views at 00000000 for:
             0000000000401020 0000000000401020 (DW_OP_lit3; DW_OP_stack_value)
    00000018 v000000000000004 v000000000000000 views at 00000002 for:
             0000000000401020 0000000000401023 (DW_OP_lit7; DW_OP_stack_value)
    0000002c <End of list>

    0000003c v000000000000002 v000000000000004 location view pair
    0000003e v000000000000004 v000000000000000 location view pair

    00000040 <End of list>

(have to use GCC to produce the input here, since Clang's current use of base address specifiers in debug_loc doesn't trip over the issue - that doesn't diminish the problem, though)

Can you share loc.cpp?

Oh, right, sorry:

__attribute__((noinline)) void f1() { }
void f2() {
  int i = 3;
  f1();
  i = 7;
  f1();
}
int main() {
  int i = 3;
  f1();
  i = 7;
  f1();
}

That's fairly easy - 0, 0 terminates a location list the same way it terminates a range list - llvm-dwarfdump, for instance, then tries to parse the following expression as the start of a new location list and errors. binutils objdump only dumps location lists that are referenced from debug_info, printint out "holes" when the location list terminates early.

So how is BFD's choice for .debug_loc (1) better than -2? Additional warning objdump: Warning: Hole and overlap detection requires adjacent view lists and loclists. objdump: Warning: There are 40 unused bytes at the end of section .debug_loc and unneeded fffffffffffffffe ranges cannot be filtered out?

Aug 4 2020, 11:12 PM · Restricted Project
echristo added a comment to D83273: [X86] Remove the feature dependency handling in X86TargetInfo::setFeatureEnabledImpl to a table based lookup in X86TargetParser.cpp.

It's a pretty nasty revert in our downstream tree where we have prototyped future ISAs. So I'd like a little time to take a look.

@nickdesaulniers what cpu and fetaures are on your command lines. getImpliedDisabledFeatures should only be called if some feature is explicitly being disabled.

Likely, as the kernel is very explicit to disable the use of all FP extensions, see https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/arch/x86/Makefile#n57. Maybe we could just set fewer flags, but there's still probably room for improvement here, too.

Aug 4 2020, 4:26 PM · Restricted Project, Restricted Project
echristo updated subscribers of D83273: [X86] Remove the feature dependency handling in X86TargetInfo::setFeatureEnabledImpl to a table based lookup in X86TargetParser.cpp.

Ow. Can revert and reapply after we fix the caching problem perhaps?

Aug 4 2020, 3:49 PM · Restricted Project, Restricted Project
echristo updated subscribers of D85231: Protect against filenames with no extension at all..

Could maybe add an assert along with the patch as well as an assert only
test?

Aug 4 2020, 2:00 PM · Restricted Project

Jul 28 2020

echristo added a comment to D84793: [X86] Add assembler support for {disp8} and {disp32} to control the size of displacement used for memory operands..

I think I'd find the conditionals easier to "read" if they were positive rather than negative? It'd help if the comments spelled out the conditions a little more as well.

Jul 28 2020, 8:47 PM · Restricted Project

Jul 27 2020

D18036: Enabling multiple letters and digits in constraints now requires changes to proceed.

This could probably use some tests. If no current upstream port uses the feature though it makes it hard to accept. :(

Jul 27 2020, 10:07 PM
D26344: Small Data SectionKind and PPC-EABI SDA Relocation now requires changes to proceed.
Jul 27 2020, 10:05 PM
echristo accepted D84709: [X86] Add support for {disp32} to control size of jmp and jcc instructions in the assembler.
Jul 27 2020, 6:14 PM · Restricted Project

Jul 26 2020

echristo requested changes to D84519: [llvm-objdump][AMDGPU] Detect CPU string.

I'd really like all of the cpu specific bits here to not reside in the objdump binary. Ideally it should be in include/ and lib/ somewhere.

Jul 26 2020, 10:42 PM · Restricted Project

Jul 25 2020

echristo committed rG4b14ef33e81c: Temporarily Revert "Unify the return value of GetByteSize to an llvm… (authored by echristo).
Temporarily Revert "Unify the return value of GetByteSize to an llvm…
Jul 25 2020, 6:43 PM
echristo added a reverting change for rG1d9b860fb6a8: Unify the return value of GetByteSize to an llvm::Optional<uint64_t> (NFC-ish): rG4b14ef33e81c: Temporarily Revert "Unify the return value of GetByteSize to an llvm….
Jul 25 2020, 6:43 PM
echristo committed rG18975762c197: Fold StatepointBB into checks as it's only used from an NDEBUG or ASSERT… (authored by echristo).
Fold StatepointBB into checks as it's only used from an NDEBUG or ASSERT…
Jul 25 2020, 6:37 PM

Jul 23 2020

echristo committed rG3ac828b8f7a8: Use llvm::size rather than an empty loop to get the number of top level loops. (authored by echristo).
Use llvm::size rather than an empty loop to get the number of top level loops.
Jul 23 2020, 3:00 PM
echristo committed rG3a75466f41be: Temporarily Revert "Reland [lldb] Unify type name matching in… (authored by echristo).
Temporarily Revert "Reland [lldb] Unify type name matching in…
Jul 23 2020, 12:47 AM
echristo added a reverting change for rG074b121642b2: Reland [lldb] Unify type name matching in FormattersContainer: rG3a75466f41be: Temporarily Revert "Reland [lldb] Unify type name matching in….
Jul 23 2020, 12:47 AM

Jul 17 2020

echristo committed rG020545d386cf: Temporarily Revert "[OpenMP] Add Additional Function Attribute Information to… (authored by echristo).
Temporarily Revert "[OpenMP] Add Additional Function Attribute Information to…
Jul 17 2020, 3:07 PM
echristo added a reverting change for rG09fe0c5ab9ca: [OpenMP] Add Additional Function Attribute Information to OMPKinds.def: rG020545d386cf: Temporarily Revert "[OpenMP] Add Additional Function Attribute Information to….
Jul 17 2020, 3:07 PM
echristo committed rGae08dbc67326: Temporarily Revert "[InlineAdvisor] New inliner advisor to replay inlining from… (authored by echristo).
Temporarily Revert "[InlineAdvisor] New inliner advisor to replay inlining from…
Jul 17 2020, 2:59 PM
echristo added a reverting change for rG029946b11268: [InlineAdvisor] New inliner advisor to replay inlining from optimization remarks: rGae08dbc67326: Temporarily Revert "[InlineAdvisor] New inliner advisor to replay inlining from….
Jul 17 2020, 2:59 PM
echristo accepted D84058: Pass -rtlib=libgcc in tests conditioned on the default..
Jul 17 2020, 11:25 AM · Restricted Project
echristo added a reviewer for D84058: Pass -rtlib=libgcc in tests conditioned on the default.: saugustine.

(Adding Sterling as well)
I think this will be an OK workaround for now, go ahead and reply on the main thread as I think they'll want to set the default runtime library as part of the toolchain. I'm surprised at this behavior as well.

Jul 17 2020, 11:25 AM · Restricted Project

Jul 16 2020

echristo accepted D83996: [X86] Change the scheduler model for 'pentium4' to SandyBridgeModel..

Appears so, but ok.

Jul 16 2020, 7:34 PM · Restricted Project
echristo committed rG7bfaa4008635: Temporarily Revert "[AssumeBundles] Use operand bundles to encode alignment… (authored by echristo).
Temporarily Revert "[AssumeBundles] Use operand bundles to encode alignment…
Jul 16 2020, 11:54 AM
echristo added a reverting change for rG8d09f20798ac: [AssumeBundles] Use operand bundles to encode alignment assumptions: rG7bfaa4008635: Temporarily Revert "[AssumeBundles] Use operand bundles to encode alignment….
Jul 16 2020, 11:54 AM

Jul 15 2020

echristo added a comment to D83360: [InstSimplify] Remove select ?, undef, X -> X and select ?, X, undef -> X.

It's that even before the msan instrumentation the IR doesn't look correct - thus a miscompile.

I'll share a prototype of the InstSimplify patch on Phabricator, in a day or two; it would be great if the miscompilation is fixed with the patch.

Jul 15 2020, 6:30 PM · Restricted Project, Restricted Project
echristo added a comment to D83360: [InstSimplify] Remove select ?, undef, X -> X and select ?, X, undef -> X.

It's that even before the msan instrumentation the IR doesn't look correct - thus a miscompile.

Jul 15 2020, 4:36 PM · Restricted Project, Restricted Project
echristo added a comment to D83360: [InstSimplify] Remove select ?, undef, X -> X and select ?, X, undef -> X.

We're starting to see miscompiles as we do more testing as well, just nothing smaller at the moment.

Jul 15 2020, 3:47 PM · Restricted Project, Restricted Project
echristo added a comment to D83360: [InstSimplify] Remove select ?, undef, X -> X and select ?, X, undef -> X.

This seems like something that we should then revert until we know that instsimplify can be updated with a fix?

Jul 15 2020, 3:41 PM · Restricted Project, Restricted Project
echristo accepted D83913: [X86] Change the tuning settings for pentium4 to be more modern since its the default 32-bit cpu in clang.

LGTM.

Jul 15 2020, 3:15 PM · Restricted Project
echristo added a comment to D83897: [X86] Add a generic 32-bit CPU with sse2 with modern tuning flags to be used as the default for the 32-bit targets instead of pentium4.

Did you want to just change the pentium4 tuning? Otherwise naming things is hard and this feels awkward, but I don't have any better ideas :)

Jul 15 2020, 12:40 PM · Restricted Project

Jul 14 2020

echristo committed rG368eb7712f9f: Fix a -Wunused-variable warning. (authored by echristo).
Fix a -Wunused-variable warning.
Jul 14 2020, 12:41 PM

Jul 13 2020

echristo committed rGe958379581e5: Fold the opt size check into the assert to silence an unused variable warning. (authored by echristo).
Fold the opt size check into the assert to silence an unused variable warning.
Jul 13 2020, 4:05 PM

Jul 10 2020

echristo committed rG256e4d46a675: Fix signed vs unsigned comparison warnings a different way. (authored by echristo).
Fix signed vs unsigned comparison warnings a different way.
Jul 10 2020, 10:53 PM
echristo committed rGcc28058c13e8: Temporarily revert "[NFC] Separate bitcode reading for FUNC_CODE_INST_CMPXCHG… (authored by echristo).
Temporarily revert "[NFC] Separate bitcode reading for FUNC_CODE_INST_CMPXCHG…
Jul 10 2020, 3:21 PM
echristo added a reverting change for rG30582457b470: [NFC] Separate bitcode reading for FUNC_CODE_INST_CMPXCHG(_OLD): rGcc28058c13e8: Temporarily revert "[NFC] Separate bitcode reading for FUNC_CODE_INST_CMPXCHG….
Jul 10 2020, 3:21 PM

Jul 9 2020

echristo added a comment to D83525: Remove optnone from FullUnroll.ll.

I may need to comment it better, but in this case part of it is that it's designed to only have the forced loop unroller run on it.

Jul 9 2020, 5:37 PM · Restricted Project
echristo committed rG98eec7700c3f: Temporarily Revert "Fix [-Werror,-Wsign-compare] warnings arising from… (authored by echristo).
Temporarily Revert "Fix [-Werror,-Wsign-compare] warnings arising from…
Jul 9 2020, 2:00 PM
echristo added a reverting change for rGc2827083166c: Fix [-Werror,-Wsign-compare] warnings arising from subsection symbols patch.: rG98eec7700c3f: Temporarily Revert "Fix [-Werror,-Wsign-compare] warnings arising from….
Jul 9 2020, 2:00 PM
echristo committed rGce1e4853b5a1: Temporarily Revert "[PowerPC] Split s34imm into two types" as it was failing in… (authored by echristo).
Temporarily Revert "[PowerPC] Split s34imm into two types" as it was failing in…
Jul 9 2020, 1:37 PM
echristo added a reverting change for rGbd2068031121: [PowerPC] Split s34imm into two types: rGce1e4853b5a1: Temporarily Revert "[PowerPC] Split s34imm into two types" as it was failing in….
Jul 9 2020, 1:37 PM
echristo added a comment to D83013: [LPM] Port CGProfilePass from NPM to LPM.

Some inline nits. I see you've already committed and that's fine - I still don't think we should do it, but we can delete it again soon :)

Jul 9 2020, 1:08 PM · Restricted Project, Restricted Project
echristo committed rGc2827083166c: Fix [-Werror,-Wsign-compare] warnings arising from subsection symbols patch. (authored by echristo).
Fix [-Werror,-Wsign-compare] warnings arising from subsection symbols patch.
Jul 9 2020, 11:19 AM
echristo added a comment to D83462: [DWARF] Avoid entry_values production for SCE.

So the tuning here for SCE is also a "does not support" or something else?

Jul 9 2020, 9:52 AM · Restricted Project, debug-info

Jul 8 2020

echristo committed rG371c94fca039: Fix a typo in an error message. (authored by echristo).
Fix a typo in an error message.
Jul 8 2020, 8:46 PM
echristo added a comment to D83397: [flang] Replace uses of _Complex with std::complex.

You also still haven't replied with "what bot/compiler/etc is going to break by making this change". Would you please do that?

Locally, we build flang with our changes with g++ and clang++ on a variety of Linux and MacOS.

Jul 8 2020, 9:02 AM · Restricted Project, Restricted Project
echristo added a comment to D83397: [flang] Replace uses of _Complex with std::complex.

I've reverted these models from the upstreamed code for now. It will avoid the warnings.

Jul 8 2020, 9:01 AM · Restricted Project, Restricted Project
echristo added a comment to D83397: [flang] Replace uses of _Complex with std::complex.

Hi Eric,

There is an active development branch for the flang middle end. https://github.com/flang-compiler/f18-llvm-project/tree/fir-dev

That's not part of the llvm project.

That code base is being upstreamed piecemeal. Not all of the code is upstreamed at this point. It is simply a false impression that code in the middle of being upstreamed is "unused" or "unnecessary". Since it not all of it is upstreamed, changing interfaces and support code in llvm-project directly is going to cause problems that can become hard to track and resolve while the upstreaming is ongoing.

It very much is unused and unnecessary as there are no pieces of that code in the repository.

We are stuck between a rock and a hard place. Flang needs a middle end, as it currently can't generate code. Work on that middle end is being done. We are following the rules and trying to upstream that code in small "reviewable" chunks as quickly as possible. It's just simply going to be the case because of the interdependencies involved and the small chunks process that some code will merely appear to be temporally unused.

If you have a better solution to how to change the upstream process, then our group would be happy to hear it.

Jul 8 2020, 8:12 AM · Restricted Project, Restricted Project
echristo added a comment to D83397: [flang] Replace uses of _Complex with std::complex.

Hi Eric,

There is an active development branch for the flang middle end. https://github.com/flang-compiler/f18-llvm-project/tree/fir-dev

Jul 8 2020, 7:49 AM · Restricted Project, Restricted Project
echristo added a comment to D83397: [flang] Replace uses of _Complex with std::complex.

In whose builds and why? What changes would you like? What compiler are you using? Can you provide any more information?

Jul 8 2020, 7:34 AM · Restricted Project, Restricted Project
Herald added a project to D83397: [flang] Replace uses of _Complex with std::complex: Restricted Project.
Jul 8 2020, 7:20 AM · Restricted Project, Restricted Project

Jul 7 2020

echristo accepted D83368: [NewPM][opt] Share -disable-loop-unrolling between pass managers.

If this works then awesome. :)

Jul 7 2020, 9:52 PM · Restricted Project

Jul 6 2020

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

Works for me :)

Jul 6 2020, 6:48 PM · Restricted Project, Restricted Project
echristo committed rG4029f8ede42f: Temporarily Revert "[llvm-install-name-tool] Merge install-name options" as it… (authored by echristo).
Temporarily Revert "[llvm-install-name-tool] Merge install-name options" as it…
Jul 6 2020, 3:40 PM
echristo added a reverting change for rGc143900a0851: [llvm-install-name-tool] Merge install-name options: rG4029f8ede42f: Temporarily Revert "[llvm-install-name-tool] Merge install-name options" as it….
Jul 6 2020, 3:40 PM