jdenny (Joel E. Denny)
User

Projects

User does not belong to any projects.

User Details

User Since
Nov 2 2017, 3:15 PM (54 w, 6 d)

Recent Activity

Yesterday

jdenny added a comment to D54769: [FileCheck] New option -warn.

This is small enough that I'd prefer to see it incorporated into the patch with the "real" warning, rather than have this temporary placeholder diagnostic.

Wed, Nov 21, 8:44 AM
jdenny added inline comments to D54769: [FileCheck] New option -warn.
Wed, Nov 21, 7:31 AM

Tue, Nov 20

jdenny committed rL347351: [OpenMP] Update CHECK-DAG usage in target_parallel_codegen.cpp.
[OpenMP] Update CHECK-DAG usage in target_parallel_codegen.cpp
Tue, Nov 20, 2:08 PM
jdenny committed rC347351: [OpenMP] Update CHECK-DAG usage in target_parallel_codegen.cpp.
[OpenMP] Update CHECK-DAG usage in target_parallel_codegen.cpp
Tue, Nov 20, 2:08 PM
jdenny closed D54765: [OpenMP] Update CHECK-DAG usage in target_parallel_codegen.cpp.
Tue, Nov 20, 2:08 PM
jdenny committed rL347350: [OpenMP] Update CHECK-DAG usage in for_codegen.cpp.
[OpenMP] Update CHECK-DAG usage in for_codegen.cpp
Tue, Nov 20, 2:07 PM
jdenny committed rC347350: [OpenMP] Update CHECK-DAG usage in for_codegen.cpp.
[OpenMP] Update CHECK-DAG usage in for_codegen.cpp
Tue, Nov 20, 2:07 PM
jdenny closed D54764: [OpenMP] Update CHECK-DAG usage in for_codegen.cpp.
Tue, Nov 20, 2:07 PM
jdenny added a comment to D54769: [FileCheck] New option -warn.

Thanks for working on this. I do not believe DAG overlaps should be a warning. I know from experience that they occur very frequently.

Tue, Nov 20, 12:45 PM
jdenny added a comment to D54764: [OpenMP] Update CHECK-DAG usage in for_codegen.cpp.

I'm fine with the patch,

Tue, Nov 20, 12:25 PM
jdenny created D54765: [OpenMP] Update CHECK-DAG usage in target_parallel_codegen.cpp.
Tue, Nov 20, 11:57 AM
jdenny created D54764: [OpenMP] Update CHECK-DAG usage in for_codegen.cpp.
Tue, Nov 20, 11:52 AM
jdenny added a comment to D53710: [FileCheck] Warn if a prefix is only used in LABEL checks.

I'm back :-) Catching up on the last messages.... I will post a patch for review soon that adds an option -warn to FileCheck, and will do some initial plumbing for that. If I understood correctly, it looks like we think that is best. After that initial plumbing, I will then return to this patch. Is that a plan?

Tue, Nov 20, 9:18 AM
jdenny updated the diff for D53899: [FileCheck] Annotate input dump (7/7).

Update for changes earlier in patch series.

Tue, Nov 20, 8:51 AM
jdenny updated the diff for D53898: [FileCheck] Annotate input dump (6/7).

Update for changes earlier in patch series.

Tue, Nov 20, 8:51 AM
jdenny updated the diff for D53897: [FileCheck] Annotate input dump (5/7).

Update for changes earlier in patch series.

Tue, Nov 20, 8:51 AM
jdenny updated the diff for D53896: [FileCheck] Annotate input dump (4/7).

Update for changes earlier in patch series.

Tue, Nov 20, 8:51 AM
jdenny updated the diff for D53894: [FileCheck] Annotate input dump (3/7).

Update for changes earlier in patch series.

Tue, Nov 20, 8:51 AM
jdenny updated the diff for D53893: [FileCheck] Annotate input dump (2/7).

Update for changes earlier in patch series.

Tue, Nov 20, 8:51 AM
jdenny updated the diff for D52999: [FileCheck] Annotate input dump (1/7).
  • Rebase, and extend for the new CHECK-COUNT-<num> directive.
  • Convert DiagList and AnnotationList to std::vector, as probinson recommended.
Tue, Nov 20, 8:46 AM

Fri, Nov 16

jdenny added a comment to D52999: [FileCheck] Annotate input dump (1/7).

It really seems like DiagList and AnnotationList ought to be vectors, not lists. They are append-only, and AnnotationList gets converted to a vector anyway to sort it. The code doesn't depend on the element-pointer stability guarantee of a list, except in one place noted below which can be fixed.
It's quite possible a vector would perform less well, in the face of many diags/annotations, but as the diags/annotations are the unusual case, performance is not really a big consideration.

Fri, Nov 16, 11:40 AM

Fri, Nov 9

jdenny added a comment to D53710: [FileCheck] Warn if a prefix is only used in LABEL checks.

I hadn't thought about -v; I don't think the new diag is currently under -v? The first test isn't using that option...

I think we should encourage people to have warnings on when they develop their tests. Having that separate from -v seems like a good thing, as -v obviously dumps a lot of info, most of which would ordinarily describe a test behaving exactly as intended.

Fri, Nov 9, 10:13 AM
jdenny added a comment to D53710: [FileCheck] Warn if a prefix is only used in LABEL checks.

Of course, let's see what Paul thinks before proceeding.

Fri, Nov 9, 8:33 AM
jdenny added a comment to D53710: [FileCheck] Warn if a prefix is only used in LABEL checks.

Agreed with all previous comments! And yes, I messed up my last lit test example :-( and I looked at yours and Paul's, and agree. But anyway, we have a plan...

Fri, Nov 9, 8:30 AM

Thu, Nov 8

jdenny added a comment to D53710: [FileCheck] Warn if a prefix is only used in LABEL checks.
we can't reliably report all typos.
Thu, Nov 8, 11:36 AM
jdenny added a comment to D53710: [FileCheck] Warn if a prefix is only used in LABEL checks.

Hi Paul and Joel, thanks for your help here, appreciate it! I thought this was
a nice little addition, low-hanging fruit, but it looks like it turns into
proper project! :-)

Thu, Nov 8, 8:11 AM

Wed, Nov 7

jdenny updated the diff for D53898: [FileCheck] Annotate input dump (6/7).

Rebased onto updated patches earlier in series.

Wed, Nov 7, 3:15 PM
jdenny updated the diff for D53899: [FileCheck] Annotate input dump (7/7).

Rebased onto updated patches earlier in series.

Wed, Nov 7, 3:15 PM
jdenny updated the diff for D53897: [FileCheck] Annotate input dump (5/7).

Rebased onto updated patches earlier in series.

Wed, Nov 7, 3:15 PM
jdenny updated the diff for D53894: [FileCheck] Annotate input dump (3/7).

Rebased onto updated patches earlier in series.

Wed, Nov 7, 3:15 PM
jdenny updated the diff for D53896: [FileCheck] Annotate input dump (4/7).

Rebased onto updated patches earlier in series.

Wed, Nov 7, 3:15 PM
jdenny updated the diff for D53893: [FileCheck] Annotate input dump (2/7).

Rebased onto updated patches earlier in series.

Wed, Nov 7, 3:15 PM
jdenny updated the diff for D52999: [FileCheck] Annotate input dump (1/7).
  • Rebased.
  • Removed FILECHECK_DUMP_INPUT env var. The new FILECHECK_OPTS is sufficient.
  • Removed unsetting of FILECHECK_* env vars in tests. That should be handled more carefully in a separate patch.
Wed, Nov 7, 2:59 PM
jdenny added a comment to D53710: [FileCheck] Warn if a prefix is only used in LABEL checks.

This seems like a reasonable use-case to me.

Wed, Nov 7, 12:20 PM
jdenny added a comment to D53710: [FileCheck] Warn if a prefix is only used in LABEL checks.

AMDGPU does this a lot intentionally for a shared label prefix and unique per run like ones for different subtargets

Yep. Briefly looked at AMDGPU warnings. The very first 4 warnings, in tests basic-loop.ll, basic-call-return.ll, bug-vopc-commute.ll, call-return-types.ll, shows I think that they are dodgy tests, but then I see indeed the intentional use of the shared label prefix.

Wed, Nov 7, 9:50 AM

Tue, Nov 6

jdenny committed rL346277: [FileCheck] Try to fix windows bots broken by r346272.
[FileCheck] Try to fix windows bots broken by r346272
Tue, Nov 6, 2:46 PM
jdenny added a comment to D53517: [FileCheck] Parse command-line options from FILECHECK_OPTS.

LGTM.
A patch to report an error on multiple -D of the same variable is probably a good thing, particularly since it doesn't work properly.

Tue, Nov 6, 2:11 PM
jdenny committed rL346272: [FileCheck] Parse command-line options from FILECHECK_OPTS.
[FileCheck] Parse command-line options from FILECHECK_OPTS
Tue, Nov 6, 2:09 PM
jdenny closed D53517: [FileCheck] Parse command-line options from FILECHECK_OPTS.
Tue, Nov 6, 2:09 PM
jdenny updated the diff for D53517: [FileCheck] Parse command-line options from FILECHECK_OPTS.

Address reviewer comments:

Tue, Nov 6, 12:03 PM
jdenny added inline comments to D53517: [FileCheck] Parse command-line options from FILECHECK_OPTS.
Tue, Nov 6, 11:59 AM
jdenny added a comment to D53710: [FileCheck] Warn if a prefix is only used in LABEL checks.

Friendly ping.

Does this look somewhat reasonable, and do what we want?

Tue, Nov 6, 7:08 AM

Mon, Nov 5

jdenny updated the diff for D53517: [FileCheck] Parse command-line options from FILECHECK_OPTS.

This is a small patch to FileCheck that I think could be beneficial throughout LLVM. Does it need an RFC?

Mon, Nov 5, 2:42 PM

Wed, Oct 31

jdenny updated the diff for D52999: [FileCheck] Annotate input dump (1/7).
  • Address zturner's comment: prefer FILECHECK_DUMP_INPUT=never over -dump-input=never when avoiding user environment variables.
  • Set FILECHECK_DUMP_INPUT=never in test/FileCheck/verbose.txt as well. Somehow I didn't notice this one was failing before.
Wed, Oct 31, 1:05 PM
jdenny added reviewers for D53898: [FileCheck] Annotate input dump (6/7): george.karpenkov, probinson, hfinkel.
Wed, Oct 31, 12:00 PM
jdenny added reviewers for D53897: [FileCheck] Annotate input dump (5/7): george.karpenkov, probinson, hfinkel.
Wed, Oct 31, 12:00 PM
jdenny added reviewers for D53899: [FileCheck] Annotate input dump (7/7): george.karpenkov, probinson, hfinkel.
Wed, Oct 31, 12:00 PM
jdenny added reviewers for D53896: [FileCheck] Annotate input dump (4/7): george.karpenkov, probinson, hfinkel.
Wed, Oct 31, 12:00 PM
jdenny added reviewers for D53893: [FileCheck] Annotate input dump (2/7): george.karpenkov, probinson, hfinkel.
Wed, Oct 31, 12:00 PM
jdenny added reviewers for D53894: [FileCheck] Annotate input dump (3/7): george.karpenkov, probinson, hfinkel.
Wed, Oct 31, 12:00 PM

Tue, Oct 30

jdenny added inline comments to D52999: [FileCheck] Annotate input dump (1/7).
Tue, Oct 30, 3:11 PM
jdenny added a dependent revision for D53898: [FileCheck] Annotate input dump (6/7): D53899: [FileCheck] Annotate input dump (7/7).
Tue, Oct 30, 2:22 PM
jdenny created D53899: [FileCheck] Annotate input dump (7/7).
Tue, Oct 30, 2:22 PM
jdenny added a dependency for D53899: [FileCheck] Annotate input dump (7/7): D53898: [FileCheck] Annotate input dump (6/7).
Tue, Oct 30, 2:22 PM
jdenny added a dependency for D53898: [FileCheck] Annotate input dump (6/7): D53897: [FileCheck] Annotate input dump (5/7).
Tue, Oct 30, 2:19 PM
jdenny added a dependent revision for D53897: [FileCheck] Annotate input dump (5/7): D53898: [FileCheck] Annotate input dump (6/7).
Tue, Oct 30, 2:19 PM
jdenny created D53898: [FileCheck] Annotate input dump (6/7).
Tue, Oct 30, 2:18 PM
jdenny added a dependency for D53897: [FileCheck] Annotate input dump (5/7): D53896: [FileCheck] Annotate input dump (4/7).
Tue, Oct 30, 2:17 PM
jdenny added a dependent revision for D53896: [FileCheck] Annotate input dump (4/7): D53897: [FileCheck] Annotate input dump (5/7).
Tue, Oct 30, 2:17 PM
jdenny created D53897: [FileCheck] Annotate input dump (5/7).
Tue, Oct 30, 2:17 PM
jdenny added a dependency for D53896: [FileCheck] Annotate input dump (4/7): D53894: [FileCheck] Annotate input dump (3/7).
Tue, Oct 30, 2:16 PM
jdenny added a dependent revision for D53894: [FileCheck] Annotate input dump (3/7): D53896: [FileCheck] Annotate input dump (4/7).
Tue, Oct 30, 2:16 PM
jdenny created D53896: [FileCheck] Annotate input dump (4/7).
Tue, Oct 30, 2:16 PM
jdenny added a dependency for D53894: [FileCheck] Annotate input dump (3/7): D53893: [FileCheck] Annotate input dump (2/7).
Tue, Oct 30, 2:10 PM
jdenny added a dependent revision for D53893: [FileCheck] Annotate input dump (2/7): D53894: [FileCheck] Annotate input dump (3/7).
Tue, Oct 30, 2:10 PM
jdenny created D53894: [FileCheck] Annotate input dump (3/7).
Tue, Oct 30, 2:10 PM
jdenny added a dependency for D53893: [FileCheck] Annotate input dump (2/7): D52999: [FileCheck] Annotate input dump (1/7).
Tue, Oct 30, 2:06 PM
jdenny added a dependent revision for D52999: [FileCheck] Annotate input dump (1/7): D53893: [FileCheck] Annotate input dump (2/7).
Tue, Oct 30, 2:06 PM
jdenny created D53893: [FileCheck] Annotate input dump (2/7).
Tue, Oct 30, 2:05 PM
jdenny set the repository for D52999: [FileCheck] Annotate input dump (1/7) to rL LLVM.
Tue, Oct 30, 2:05 PM
jdenny updated the summary of D52999: [FileCheck] Annotate input dump (1/7).
Tue, Oct 30, 1:57 PM
jdenny updated the diff for D52999: [FileCheck] Annotate input dump (1/7).
  • Split up the patch. This is now the first in the series. Each patch has a description and example of the diagnostics for which it implements input annotations.
  • Add the input file name and check file name to the annotations description to further address a reviewer concern.
  • Don't use the word "key" to identify the annotations description because technically that's not what it is.
Tue, Oct 30, 1:56 PM

Mon, Oct 29

jdenny added a comment to D52999: [FileCheck] Annotate input dump (1/7).
Mon, Oct 29, 10:39 AM
jdenny added a comment to D53517: [FileCheck] Parse command-line options from FILECHECK_OPTS.

Ping.

Mon, Oct 29, 10:01 AM
jdenny added a comment to D50485: [lit] Add LIT_DEBUG environment variable to support passing options though ninja.

@george.karpenkov: I don't know what your current plans are for this patch. I recall you once mentioned wanting it to set FileCheck's -v. D53517 could help with that, and it addresses other uses cases, such as color.

Mon, Oct 29, 7:31 AM

Fri, Oct 26

jdenny added a comment to D53710: [FileCheck] Warn if a prefix is only used in LABEL checks.

Why isn't that just CHECK? Is that a legitimate use case?

CHECK-LABEL will complain if it doesn't find a matching string, so the test would fail if the label text does not appear.
However, it's not really a proper use of LABEL, and it would be better to have this test just use CHECK.

Fri, Oct 26, 9:07 AM
jdenny added a comment to D53710: [FileCheck] Warn if a prefix is only used in LABEL checks.

Thanks for looking at this! Answering this one first:

Have you run existing test suites to see how many warnings you get?

I just did, and it came back with some interesting results. For example test/CodeGen/ARM/Windows/mangling.ll:

; RUN: llc -mtriple=thumbv7-windows -mcpu=cortex-a9 -o - %s | FileCheck %s
define void @function() nounwind {
entry:
  ret void
}
; CHECK-LABEL: function

This shows that just checking the label is actually a use-case, which is why I thought a warning would be better; looks like we can't promote it to an error (which is okay I think).

Fri, Oct 26, 7:07 AM

Thu, Oct 25

jdenny added inline comments to D53710: [FileCheck] Warn if a prefix is only used in LABEL checks.
Thu, Oct 25, 9:27 AM
jdenny added a reviewer for D53710: [FileCheck] Warn if a prefix is only used in LABEL checks: probinson.
Thu, Oct 25, 9:22 AM
jdenny added a comment to D53710: [FileCheck] Warn if a prefix is only used in LABEL checks.

Have you run existing test suites to see how many warnings you get? That would help to determine if there are use cases we haven't thought of. If there are no use cases, we might consider making this an error.

Thu, Oct 25, 9:22 AM

Wed, Oct 24

jdenny committed rL345202: [SourceMgr][FileCheck] Obey -color by extending WithColor.
[SourceMgr][FileCheck] Obey -color by extending WithColor
Wed, Oct 24, 2:49 PM
jdenny closed D53419: [SourceMgr][FileCheck] Obey -color by extending WithColor.
Wed, Oct 24, 2:48 PM

Tue, Oct 23

jdenny added a comment to D53419: [SourceMgr][FileCheck] Obey -color by extending WithColor.

Seems ok to me, my only concern is that something could fail on Windows 8 or Windows 7, but I don't have a system to test that on.

Tue, Oct 23, 2:46 PM
jdenny added a reviewer for D53517: [FileCheck] Parse command-line options from FILECHECK_OPTS: gaeke.
Tue, Oct 23, 2:27 PM
jdenny requested review of D53419: [SourceMgr][FileCheck] Obey -color by extending WithColor.
Tue, Oct 23, 2:14 PM
jdenny reopened D53419: [SourceMgr][FileCheck] Obey -color by extending WithColor.
Tue, Oct 23, 2:12 PM
jdenny updated the diff for D53419: [SourceMgr][FileCheck] Obey -color by extending WithColor.

The patch was reverted because the test it introduces fails on some Windows bots where color isn't supported as expected.

Tue, Oct 23, 2:12 PM

Oct 22 2018

jdenny created D53517: [FileCheck] Parse command-line options from FILECHECK_OPTS.
Oct 22 2018, 11:38 AM
jdenny added a comment to D52999: [FileCheck] Annotate input dump (1/7).

@jdenny

I'm willing to split up the patch more if it helps the review

Yes please, to be honest I can't still fully understand the feature description, let alone the code.
I have asked my colleague to take a glance, and his verdict was the same, so I'm not alone there.

You have a lot of new features in your FileCheck output; could you think of a way to separate them into simple, small, understandable chunks?

Oct 22 2018, 11:26 AM
jdenny committed rL344930: [SourceMgr][FileCheck] Obey -color by extending WithColor.
[SourceMgr][FileCheck] Obey -color by extending WithColor
Oct 22 2018, 11:03 AM
jdenny closed D53419: [SourceMgr][FileCheck] Obey -color by extending WithColor.
Oct 22 2018, 11:02 AM

Oct 19 2018

jdenny added a dependency for D52999: [FileCheck] Annotate input dump (1/7): D53419: [SourceMgr][FileCheck] Obey -color by extending WithColor.
Oct 19 2018, 11:18 AM
jdenny added a dependent revision for D53419: [SourceMgr][FileCheck] Obey -color by extending WithColor: D52999: [FileCheck] Annotate input dump (1/7).
Oct 19 2018, 11:18 AM
jdenny updated the diff for D52999: [FileCheck] Annotate input dump (1/7).

Adjusted to take advantage of D53419, which extends WithColor.

Oct 19 2018, 11:17 AM
jdenny added a comment to D53419: [SourceMgr][FileCheck] Obey -color by extending WithColor.

LGTM.

Oct 19 2018, 9:36 AM
jdenny created D53419: [SourceMgr][FileCheck] Obey -color by extending WithColor.
Oct 19 2018, 12:41 AM

Oct 11 2018

jdenny updated the diff for D52999: [FileCheck] Annotate input dump (1/7).

Address reviewer concerns:

Oct 11 2018, 3:34 PM

Oct 10 2018

jdenny added a comment to D52999: [FileCheck] Annotate input dump (1/7).

OK that's somewhat more clear, but I'm still somewhat confused. Line by line:

$ FileCheck -v -dump-input=always checks1 < input1 |& sed -n '/^Key for/,$p'

I assume sed is there to suppress all output before the legend is printed?

Oct 10 2018, 12:24 PM

Oct 9 2018

jdenny added a comment to D52999: [FileCheck] Annotate input dump (1/7).

@jdenny Sorry I'm still struggling to understand what exactly are you doing.

Oct 9 2018, 7:30 PM
jdenny updated the summary of D52999: [FileCheck] Annotate input dump (1/7).
Oct 9 2018, 7:28 PM