Page MenuHomePhabricator

greened (David Greene)
User

Projects

User does not belong to any projects.

User Details

User Since
Jul 1 2015, 10:19 AM (273 w, 4 d)

Recent Activity

Wed, Sep 23

greened added a comment to D83004: [UpdateCCTestChecks] Include generated functions if asked.

The expensive-checks bots have been red for several days. Could you please take a look or revert?

Working on it...

Wed, Sep 23, 9:30 AM · Restricted Project, Restricted Project
greened committed rGb8779337841b: [UpdateTestChecks] Remove bug-exposing test (authored by greened).
[UpdateTestChecks] Remove bug-exposing test
Wed, Sep 23, 9:29 AM
greened added a comment to D83004: [UpdateCCTestChecks] Include generated functions if asked.

The expensive-checks bots have been red for several days. Could you please take a look or revert?

Wed, Sep 23, 8:01 AM · Restricted Project, Restricted Project

Fri, Sep 18

greened committed rG7c8bb409f31e: [UpdateCCTestChecks] Include generated functions if asked (authored by greened).
[UpdateCCTestChecks] Include generated functions if asked
Fri, Sep 18, 4:36 AM
greened closed D83004: [UpdateCCTestChecks] Include generated functions if asked.
Fri, Sep 18, 4:35 AM · Restricted Project, Restricted Project

Thu, Sep 17

greened updated the diff for D83004: [UpdateCCTestChecks] Include generated functions if asked.

Fix one inadvertent change to the original algorithm in update_test_checks.py. This line:

output_lines.append(input_line)

had an extra rstrip in it compared to the original. It was probably an innocuous change but fixing this makes the diff a bit cleaner.

Thu, Sep 17, 2:57 PM · Restricted Project, Restricted Project
greened added inline comments to D83004: [UpdateCCTestChecks] Include generated functions if asked.
Thu, Sep 17, 2:52 PM · Restricted Project, Restricted Project
greened added a comment to D83004: [UpdateCCTestChecks] Include generated functions if asked.

Maybe add another test that checks for the @__cxx_global_var_init() constructor function?
E.g. something like this:

int init_func(int arg) {
    return arg + 1;
}

int my_global = init_func(2);
Thu, Sep 17, 2:42 PM · Restricted Project, Restricted Project
greened updated the diff for D83004: [UpdateCCTestChecks] Include generated functions if asked.

Refactored to pull out most of the --include-generated-funcs processing into common.py.

Thu, Sep 17, 2:40 PM · Restricted Project, Restricted Project
greened updated the diff for D83004: [UpdateCCTestChecks] Include generated functions if asked.

Enhanced update_test_checks.py to honor --include-generated-funcs and added tests. For fun, did the same for update_llc_test_checks.py. The latter doesn't have complete coverage for all architectures because the machine outliner doesn't run on every backend. I didn't look further into this but we can do so later if desired.

Thu, Sep 17, 12:25 PM · Restricted Project, Restricted Project

Wed, Sep 16

greened committed rGce0eb81c7274: [UpdateTestChecks] Allow $ in function names (authored by greened).
[UpdateTestChecks] Allow $ in function names
Wed, Sep 16, 12:36 PM
greened closed D82995: [UpdateTestChecks] Allow $ in function names.
Wed, Sep 16, 12:36 PM · Restricted Project
greened added a comment to D83004: [UpdateCCTestChecks] Include generated functions if asked.
Wed, Sep 16, 12:26 PM · Restricted Project, Restricted Project
greened updated the diff for D82995: [UpdateTestChecks] Allow $ in function names.

Added tests.

Wed, Sep 16, 7:41 AM · Restricted Project

Tue, Sep 15

greened added a comment to D82995: [UpdateTestChecks] Allow $ in function names.

@greened Any luck with a test case?

Tue, Sep 15, 9:31 AM · Restricted Project
greened added a comment to D83004: [UpdateCCTestChecks] Include generated functions if asked.

Due to D82995 I realized we should have a test of this in llvm/test/tools/UpdateTestChecks as well.

Tue, Sep 15, 9:30 AM · Restricted Project, Restricted Project
greened added a comment to D83004: [UpdateCCTestChecks] Include generated functions if asked.

reverse ping?

Tue, Sep 15, 9:25 AM · Restricted Project, Restricted Project

Jul 22 2020

greened updated the diff for D83004: [UpdateCCTestChecks] Include generated functions if asked.

Updated to move the option into common.py. Also had to rework the output loop to account for differences between python 2 and 3.

Jul 22 2020, 1:26 PM · Restricted Project, Restricted Project
greened added inline comments to D83004: [UpdateCCTestChecks] Include generated functions if asked.
Jul 22 2020, 11:33 AM · Restricted Project, Restricted Project
greened added a comment to D82995: [UpdateTestChecks] Allow $ in function names.

We need a test.

If it helps, OpenMP begin/end declare variant mangles the names with $, so do we for CUDA (sometimes).

Jul 22 2020, 11:29 AM · Restricted Project

Jul 7 2020

greened added inline comments to D83004: [UpdateCCTestChecks] Include generated functions if asked.
Jul 7 2020, 8:01 AM · Restricted Project, Restricted Project

Jul 6 2020

greened added inline comments to D83004: [UpdateCCTestChecks] Include generated functions if asked.
Jul 6 2020, 6:21 PM · Restricted Project, Restricted Project
greened added a comment to D83004: [UpdateCCTestChecks] Include generated functions if asked.
Jul 6 2020, 6:18 PM · Restricted Project, Restricted Project
greened added a comment to D83004: [UpdateCCTestChecks] Include generated functions if asked.

This is great! Just a few days ago I added a TODO in one of the tests I created asking for this: D82722 :)

Jul 6 2020, 2:42 PM · Restricted Project, Restricted Project
greened added inline comments to D83004: [UpdateCCTestChecks] Include generated functions if asked.
Jul 6 2020, 2:38 PM · Restricted Project, Restricted Project
greened updated the diff for D83004: [UpdateCCTestChecks] Include generated functions if asked.

Fixed various bugs, added tests.

Jul 6 2020, 2:34 PM · Restricted Project, Restricted Project
greened added inline comments to D82995: [UpdateTestChecks] Allow $ in function names.
Jul 6 2020, 7:02 AM · Restricted Project
greened added a comment to D82995: [UpdateTestChecks] Allow $ in function names.

It would be good to check in a test example alongside this change, so we know it works. (And we'll know what we are losing if this has to be reverted for some reason.)

Jul 6 2020, 6:56 AM · Restricted Project

Jul 2 2020

greened added a comment to D83004: [UpdateCCTestChecks] Include generated functions if asked.

Do we have any tests for these tools? I vaguely recall some discussion about tests but I couldn't find any. I'd like to add tests for this feature if the infrastructure is already there.

Yes. See clang/test/utils/update_cc_test_checks/ (thanks to @arichardson)

Jul 2 2020, 10:14 AM · Restricted Project, Restricted Project

Jul 1 2020

greened added a comment to D83004: [UpdateCCTestChecks] Include generated functions if asked.

Do we have any tests for these tools? I vaguely recall some discussion about tests but I couldn't find any. I'd like to add tests for this feature if the infrastructure is already there.

Jul 1 2020, 3:40 PM · Restricted Project, Restricted Project
greened created D83004: [UpdateCCTestChecks] Include generated functions if asked.
Jul 1 2020, 3:40 PM · Restricted Project, Restricted Project
greened created D82995: [UpdateTestChecks] Allow $ in function names.
Jul 1 2020, 2:04 PM · Restricted Project
greened added a comment to D68230: [UpdateCCTestChecks] Allow asm in output with --allow-asm.

Ping?

Jul 1 2020, 1:32 PM · Restricted Project, Restricted Project

Jun 15 2020

greened added a comment to D68230: [UpdateCCTestChecks] Allow asm in output with --allow-asm.
Jun 15 2020, 7:51 PM · Restricted Project, Restricted Project

Jun 8 2020

greened added a comment to D68230: [UpdateCCTestChecks] Allow asm in output with --allow-asm.

Ok, so if test-suite seems.like a good place for such tests, can this change proceed?

Jun 8 2020, 6:16 PM · Restricted Project, Restricted Project

May 22 2020

greened added a comment to D68230: [UpdateCCTestChecks] Allow asm in output with --allow-asm.

In general I have a pretty strong objection to end to end tests in the clang repository unless there's no other way to test them.

Can you give an idea of what you plan on testing with this?

May 22 2020, 9:38 AM · Restricted Project, Restricted Project

May 18 2020

greened added a comment to D68230: [UpdateCCTestChecks] Allow asm in output with --allow-asm.

Getting back to this. From the (very long!) thread on end-to-end testing, people are generally supportive of the idea. The main question is where tests should be located. That question is othogonal to this patch. We currently use the functionality of this patch downstream and I'm guessing others would also find it useful. Since the idea of end-to-end testing seems to be supported in the community, can we merge this patch and then have a follow-up discussion about where such tests should live upstream? Getting this merged now would allow others to experiment with/use the functionality as discussion about where to place end-to-end tests continues.

May 18 2020, 5:52 AM · Restricted Project, Restricted Project

Apr 22 2020

greened added a comment to D78565: [clang][doc] Clang ARM CPU command line argument reference.

Fair enough, perhaps the audience is too small here on llvm.org for this and this is too niche. In A-profile we have the same problem, so could the exercise for an A-core here, but can't spend time on that now, so will abandon this.

I would find this extremely valuable in the clang documentation. Not sure why this should be put on developer.arm.com as it is compiler-specific as wel as (sub-)target-specific.

Apr 22 2020, 10:52 AM
greened added a comment to D78565: [clang][doc] Clang ARM CPU command line argument reference.

Fair enough, perhaps the audience is too small here on llvm.org for this and this is too niche. In A-profile we have the same problem, so could the exercise for an A-core here, but can't spend time on that now, so will abandon this.

Apr 22 2020, 10:52 AM

Mar 2 2020

greened added inline comments to D75385: [TargetLowering] Avoid infinite iteration on setcc fold.
Mar 2 2020, 6:52 AM · Restricted Project

Feb 28 2020

greened added a comment to D75385: [TargetLowering] Avoid infinite iteration on setcc fold.

This appeared using a quite old version of LLVM with an out-of-tree target and I'm having trouble getting a valid testcase out that runs with the latest LLVM/bugpoint. The code in question is exacly the same in the old and current LLVM so the bug still exists currently but I haven't yet been able to generate a testcase. Thought I would at least put this up for review. I'm not entirely confident current master could get into this situation but it seems wise to guard against it.

Feb 28 2020, 2:00 PM · Restricted Project
greened created D75385: [TargetLowering] Avoid infinite iteration on setcc fold.
Feb 28 2020, 2:00 PM · Restricted Project

Feb 18 2020

greened added a comment to D71618: [System Model] Introduce system model classes.
Feb 18 2020, 11:48 AM · Restricted Project
greened added a comment to D68793: [System Model] [TTI] Add TTI interfaces for write-combining buffers.

@jdoerfert @hfinkel should I consider Johannes' comment a LGTM? This has been waiting for two months now.

Feb 18 2020, 11:39 AM · Restricted Project

Feb 3 2020

greened added a comment to D73708: [update_cc_test_checks] Don't attach CHECK lines to function declarations.

LGTM. I did something similar in my local copy that I was going to upstream but I added a switch for the user to choose where to place checks (declaration, definition or both). I only added the switch because I wasn't sure whether the changed behavior was intended. I'm fine just placing checks on definitions as I don't have a need to put them on declarations.

Feb 3 2020, 6:14 PM · Restricted Project
greened accepted D73708: [update_cc_test_checks] Don't attach CHECK lines to function declarations.
Feb 3 2020, 6:14 PM · Restricted Project

Dec 17 2019

greened created D71618: [System Model] Introduce system model classes.
Dec 17 2019, 9:45 AM · Restricted Project
greened added a comment to D69088: [Lex] #pragma clang transform.

This is a major new language feature, and code review is probably not the right venue for reviewing it; there should be a thread on cfe-dev. My apologies if that's already been discussed and I missed it.

Dec 17 2019, 9:37 AM · Restricted Project

Dec 16 2019

greened committed rG055aeb527515: [Bugpoint] Do not create illegal function attribute combos (authored by greened).
[Bugpoint] Do not create illegal function attribute combos
Dec 16 2019, 8:43 AM
greened closed D69522: [Bugpoint] Do not create illegal function attribute combos.
Dec 16 2019, 8:43 AM · Restricted Project
greened updated the diff for D68793: [System Model] [TTI] Add TTI interfaces for write-combining buffers.

Updated to latest master.

Dec 16 2019, 8:43 AM · Restricted Project
greened added a comment to D69534: [X86] Lower frame references with large offsets.

Ping.

Dec 16 2019, 8:06 AM · Restricted Project
greened added a comment to D68793: [System Model] [TTI] Add TTI interfaces for write-combining buffers.

Ping. This is ready for another review.

Dec 16 2019, 7:48 AM · Restricted Project

Dec 3 2019

greened updated the diff for D68793: [System Model] [TTI] Add TTI interfaces for write-combining buffers.

Added comment explaining what a write-combining buffer is and one possibility of how to use this information.

Dec 3 2019, 10:33 AM · Restricted Project
greened added a comment to D68793: [System Model] [TTI] Add TTI interfaces for write-combining buffers.

Ok, I think I've addressed all the comments so this is ready for another review. Thanks!

Dec 3 2019, 10:33 AM · Restricted Project

Nov 1 2019

greened added inline comments to D69534: [X86] Lower frame references with large offsets.
Nov 1 2019, 10:19 AM · Restricted Project

Oct 30 2019

greened accepted D68819: [Utils] Allow update_test_checks to check function arguments.

LGTM. This is useful functionality.

Oct 30 2019, 2:30 PM · Restricted Project

Oct 29 2019

greened updated the diff for D69534: [X86] Lower frame references with large offsets.

This revision avoids the more general use of register scavenging, opting instead for a local register scavenger which can operate in backward mode, which doesn't need up-to-date liveness information. This seems safer to me, avoiding lots of potentially unknown pitfalls in the X86 backend.

Oct 29 2019, 2:33 PM · Restricted Project
greened added a comment to D69534: [X86] Lower frame references with large offsets.

This causes a couple of tests to fail due to the X86 backend not keeping liveness information up-to-date. My understanding is that the liveness bits should go away. Currently PEI uses RegisterScavenging::forward which is commented as being not preferred. The better option is to use RegisterScavenging::backward which doesn't need to rely on liveness information. However, PEI iterates through the basic block in a forward manner, so using RegisterScavenging::backward could be expensive.

Oct 29 2019, 11:13 AM · Restricted Project

Oct 28 2019

greened added a comment to D69534: [X86] Lower frame references with large offsets.

This likely needs some work. I wanted to get it up early because it includes a rather significant change, the first use of register scavenging in the X86 backend as far as I'm aware. If there is a better way to do this I'm all ears.

Oct 28 2019, 2:35 PM · Restricted Project
greened created D69534: [X86] Lower frame references with large offsets.
Oct 28 2019, 2:34 PM · Restricted Project
greened created D69522: [Bugpoint] Do not create illegal function attribute combos.
Oct 28 2019, 11:42 AM · Restricted Project

Oct 17 2019

greened accepted D67550: [AArch64][SVE] Implement unpack intrinsics.

LGTM.

Oct 17 2019, 8:49 AM · Restricted Project

Oct 10 2019

greened added a comment to D68819: [Utils] Allow update_test_checks to check function arguments.

We can, or should, combine D68153 and this, either in one or two patches.

Oct 10 2019, 9:02 PM · Restricted Project
greened committed rG7c562f12869f: [System Model] [TTI] Move default cache/prefetch implementations (authored by greened).
[System Model] [TTI] Move default cache/prefetch implementations
Oct 10 2019, 1:46 PM
greened closed D68804: [System Model] [TTI] Move default cache/prefetch implementations.
Oct 10 2019, 1:46 PM · Restricted Project
greened committed rL374446: [System Model] [TTI] Move default cache/prefetch implementations.
[System Model] [TTI] Move default cache/prefetch implementations
Oct 10 2019, 1:37 PM
greened added a comment to D68819: [Utils] Allow update_test_checks to check function arguments.

Does this subsume the goal of D68153? If so I am happy to abandon that revision. D68153 attempts to solve the problem of a CHECK-LABEL matching a function call instead of the start of a function definition. It looks like with --function-signature the CHECK-LABEL will include the arguments in the label pattern which should be enough to disambiguate it from a call to the function. Do I have that right?

Oct 10 2019, 1:25 PM · Restricted Project
greened added a comment to D68793: [System Model] [TTI] Add TTI interfaces for write-combining buffers.

we might say something like:

\return the number of write-combining buffers. A write-combining buffer is a per-core resource used for collecting writes to a particular cache line before further processing those writes using other parts of the memory subsystem.
Oct 10 2019, 1:16 PM · Restricted Project
greened added a comment to D68793: [System Model] [TTI] Add TTI interfaces for write-combining buffers.

How do you imagine that we'd use this? Do we need some kind of size to go along with this?

Oct 10 2019, 11:50 AM · Restricted Project
greened added a comment to D67551: [AArch64][SVE] Implement sdot and udot (lane) intrinsics.

LGTM.

Oct 10 2019, 9:18 AM · Restricted Project
greened added inline comments to D67550: [AArch64][SVE] Implement unpack intrinsics.
Oct 10 2019, 9:08 AM · Restricted Project
greened created D68804: [System Model] [TTI] Move default cache/prefetch implementations.
Oct 10 2019, 8:50 AM · Restricted Project
greened created D68793: [System Model] [TTI] Add TTI interfaces for write-combining buffers.
Oct 10 2019, 8:23 AM · Restricted Project

Oct 9 2019

greened added a comment to D63614: [System Model] [TTI] Update cache and prefetch TTI interfaces.

This patch causes lots of warning spews because it adds virtual methods to a class that doesn't have a virtual destructor.

Oct 9 2019, 2:34 PM · Restricted Project
greened committed rL374205: [System Model] [TTI] Update cache and prefetch TTI interfaces.
[System Model] [TTI] Update cache and prefetch TTI interfaces
Oct 9 2019, 12:51 PM
greened committed rG2e6f6b4dadbf: [System Model] [TTI] Update cache and prefetch TTI interfaces (authored by greened).
[System Model] [TTI] Update cache and prefetch TTI interfaces
Oct 9 2019, 12:51 PM

Oct 8 2019

greened added a comment to D67728: Scrub FileCheck regex delimiters from test checks.

I could mock-up a testcase but where does it go?

As you mentioned SCEV, you can add new/update test for SCEV. But I dont think this "step" is required.

So, that leaves me with a question. What else needs to be done before getting this committed?

Oct 8 2019, 10:57 AM · Restricted Project
greened committed rGeb6698572623: [UpdateCCTestChecks] Detect function mangled name on separate line (authored by greened).
[UpdateCCTestChecks] Detect function mangled name on separate line
Oct 8 2019, 9:27 AM
greened closed D68272: [UpdateCCTestChecks] Detect function mangled name on separate line.
Oct 8 2019, 9:26 AM · Restricted Project
greened committed rL374078: [UpdateCCTestChecks] Detect function mangled name on separate line.
[UpdateCCTestChecks] Detect function mangled name on separate line
Oct 8 2019, 9:24 AM

Oct 7 2019

greened committed rGa14ffc7eb741: Allow update_test_checks.py to not scrub names. (authored by greened).
Allow update_test_checks.py to not scrub names.
Oct 7 2019, 10:14 PM
greened committed rL373912: Allow update_test_checks.py to not scrub names..
Allow update_test_checks.py to not scrub names.
Oct 7 2019, 10:14 PM
greened added a comment to D68153: Make IR labels more precise.

Wouldn't this mean that every regeneration would see this change?

Oct 7 2019, 7:23 PM · Restricted Project
greened closed D68081: Allow update_test_checks.py to not scrub names.

For some reason the commit did not auto-update this. Closing with commit r373912/a14ffc7eb741de4fd7484350d11947dea40991fd.

Oct 7 2019, 5:52 PM · Restricted Project
greened added a comment to D68272: [UpdateCCTestChecks] Detect function mangled name on separate line.

Can you give an example demonstrating the issue?

Oct 7 2019, 9:07 AM · Restricted Project

Oct 4 2019

greened added a comment to D68230: [UpdateCCTestChecks] Allow asm in output with --allow-asm.

It then becomes a problem of maintenance - if a developer is just building/testing the llvm project for backend work are they responsible for keeping that test project passing? If not then it prevents us using CI bots to test and breaks are likely to linger for long lengths of time.

Oct 4 2019, 12:32 PM · Restricted Project, Restricted Project
greened accepted D68138: [utils] Fix incompatibility of bisect[-skip-count] with Python 3.

LGTM, thanks!

Oct 4 2019, 9:23 AM · Restricted Project

Oct 1 2019

greened created D68272: [UpdateCCTestChecks] Detect function mangled name on separate line.
Oct 1 2019, 7:29 AM · Restricted Project
greened added a comment to D68230: [UpdateCCTestChecks] Allow asm in output with --allow-asm.

My thinking on this is now that we have the monorepo and that the monorepo will become the canonical source after the conference this month, it's much more feasible to create end-to-end tests. They could live in a subdirectory of the monorepo root. They wouldn't be largish things like in test-suite but rather more focused tests that ensure some behavior works in the context of the entire tool pipeline.

Oct 1 2019, 7:22 AM · Restricted Project, Restricted Project
greened added inline comments to D68230: [UpdateCCTestChecks] Allow asm in output with --allow-asm.
Oct 1 2019, 7:20 AM · Restricted Project, Restricted Project
greened updated the diff for D68230: [UpdateCCTestChecks] Allow asm in output with --allow-asm.

We also need to use the asm check generator so that LABEL expressions are generated correctly.

Oct 1 2019, 7:20 AM · Restricted Project, Restricted Project

Sep 30 2019

greened added a comment to D68230: [UpdateCCTestChecks] Allow asm in output with --allow-asm.

For the record, is this for the use in upstream clang tests?

Sep 30 2019, 10:55 AM · Restricted Project, Restricted Project
greened added a comment to D68230: [UpdateCCTestChecks] Allow asm in output with --allow-asm.

Can you show simple example? Is it something like

int foo(void) {
; CHECK: xor eax, eax
return 0;
}

Than great, +1

Sep 30 2019, 10:13 AM · Restricted Project, Restricted Project
greened created D68230: [UpdateCCTestChecks] Allow asm in output with --allow-asm.
Sep 30 2019, 9:30 AM · Restricted Project, Restricted Project
greened requested changes to D68138: [utils] Fix incompatibility of bisect[-skip-count] with Python 3.

The more I think about this, the more I do think this should use from __future__ import print_function.

Sep 30 2019, 8:58 AM · Restricted Project
greened added a comment to D68138: [utils] Fix incompatibility of bisect[-skip-count] with Python 3.

Should this use from __future__ import print_function to avoid someone accidentially printing arguments as a tuple?

Sep 30 2019, 8:56 AM · Restricted Project
greened accepted D68138: [utils] Fix incompatibility of bisect[-skip-count] with Python 3.

LGTM.

Sep 30 2019, 8:53 AM · Restricted Project
greened added a comment to D68153: Make IR labels more precise.

Is there an existing test file in trunk that you can regenerate to the show the diff?

Sep 30 2019, 8:49 AM · Restricted Project
greened updated the summary of D68153: Make IR labels more precise.
Sep 30 2019, 6:50 AM · Restricted Project