[PATCH] D81422: Change filecheck default to dump input on failure

[Mehdi AMINI via Phabricator via cfe-commits]

mehdi_amini marked an inline comment as done.
mehdi_amini added a comment.

In D81422#2096643 , @arsenm wrote:

> I would also expect a simple command line flag to llvm-lit to be able to 
> control this, rather than having to set an environment variable


Would you like a lit command line flag that set the environment variable? I 
think that's easily doable




Comment at: mlir/test/mlir-tblgen/op-format-spec.td:1
-// RUN: mlir-tblgen -gen-op-decls -asmformat-error-is-fatal=false -I 
%S/../../include %s -o=%t 2>&1 | FileCheck %s --dump-input-on-failure
+// RUN: mlir-tblgen -gen-op-decls -asmformat-error-is-fatal=false -I 
%S/../../include %s -o=%t 2>&1 | FileCheck %s
 

arsenm wrote:
> All of these MLIR tests are microscopic and I don't think this is a 
> representative sample across all the projects. Most testcases are 
> significantly larger, and have hundreds if not thousands of lines of output
Well, if this can act as an extra incentive to break-up such large tests, it 
see it as a win: debugging failing patterns in the middle of such a large test 
output is not nice regardless.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D81422/new/

https://reviews.llvm.org/D81422



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D81422: Change filecheck default to dump input on failure

[Matt Arsenault via Phabricator via cfe-commits]

arsenm added a comment.

I would also expect a simple command line flag to llvm-lit to be able to 
control this, rather than having to set an environment variable




Comment at: mlir/test/mlir-tblgen/op-format-spec.td:1
-// RUN: mlir-tblgen -gen-op-decls -asmformat-error-is-fatal=false -I 
%S/../../include %s -o=%t 2>&1 | FileCheck %s --dump-input-on-failure
+// RUN: mlir-tblgen -gen-op-decls -asmformat-error-is-fatal=false -I 
%S/../../include %s -o=%t 2>&1 | FileCheck %s
 

All of these MLIR tests are microscopic and I don't think this is a 
representative sample across all the projects. Most testcases are significantly 
larger, and have hundreds if not thousands of lines of output


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D81422/new/

https://reviews.llvm.org/D81422



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D81422: Change filecheck default to dump input on failure

[Joel E. Denny via Phabricator via cfe-commits]

jdenny added a comment.

Perhaps llvm-dev is the right place to discuss the default behavior.

Regardless of that default, perhaps there should be an option that limits the 
dump to N lines that always encloses the first error.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D81422/new/

https://reviews.llvm.org/D81422



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D81422: Change filecheck default to dump input on failure

[Arthur Eubanks via Phabricator via cfe-commits]

aeubanks added a comment.

FWIW I didn't see this change initially but was pleasantly surprised with the 
extra output when debugging a newly added (relatively small) test.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D81422/new/

https://reviews.llvm.org/D81422



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D81422: Change filecheck default to dump input on failure

[Mehdi AMINI via Phabricator via cfe-commits]

mehdi_amini added a comment.

In D81422#2093360 , @arsenm wrote:

> The amount of context printed previously was good enough for development use. 
> If I ever can't figure it out from the diff, I want to look at the output 
> completely separate from the terminal.


I have the totally opposite experience: I can't do any debugging efficiently 
without this option.  Empirically, most new-comers to writing tests with 
FileCheck (in MLIR, TensorFlow, XLA, IREE, etc.) have been manually adding 
-dump-input-on-failure hard-coded into the test themselves, which is a pretty 
good indication.

Having something contextual around the failures would work for me as well: but 
the previous situation was just not usable for me.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D81422/new/

https://reviews.llvm.org/D81422



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D81422: Change filecheck default to dump input on failure

[Matt Arsenault via Phabricator via cfe-commits]

arsenm added a comment.

I'm growing to dislike the new behavior even more as I use it. I don't think 
restricting the dump to CHECK-LABEL prefixes will even help. The amount of 
context printed previously was good enough for development use. If I ever can't 
figure it out from the diff, I want to look at the output completely separate 
from the terminal. This is adding a lot of noise that doesn't help show me the 
failure point, and often fills my terminal scrollback buffer.

For AMDGPU test cases for example, every function has a long chunk of metadata 
that is irrelevant to the majority of tests. This floods my terminal even if a 
single function failed between check labels. I think this should revert to the 
old behavior. I think dump full on input would only be a reasonable default on 
build bots (where even then it might need some restrictions to avoid gigantic 
build logs if a really broken patch were committed).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D81422/new/

https://reviews.llvm.org/D81422



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D81422: Change filecheck default to dump input on failure

[Joel E. Denny via Phabricator via cfe-commits]

jdenny added a comment.

In D81422#2090425 , @mehdi_amini wrote:

> I'm thinking about a possible improvement here: we could have FileCheck dump 
> the input for the current CHECK-LABEL section only: it seems like it could 
> reduce the output drastically while still preserving how useful the 
> information is?


One FileCheck invocation can report multiple errors, one per CHECK-LABEL 
section, and I prefer to see all errors every time.

I also prefer to see the entire input dump (with annotations produced by -vv) 
as sometimes the error FileCheck reports is far away from the actual error.  
For example, maybe the CHECK-LABEL directives matched text at unexpected 
locations in the input, and then the directives in their sections failed as a 
result. It might be hard to determine that the CHECK-LABEL directives were at 
fault without seeing the text it should have matched outside of its (incorrect) 
section.

If you choose to limit the dump to one CHECK-LABEL section, please make that 
behavior optional via a command-line option (which can be set via 
FILECHECK_OPTS).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D81422/new/

https://reviews.llvm.org/D81422



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D81422: Change filecheck default to dump input on failure

[Mehdi AMINI via Phabricator via cfe-commits]

mehdi_amini added a comment.

I'm thinking about a possible improvement here: we could have FileCheck dump 
the input for the current CHECK-LABEL section only: it seems like it could 
reduce the output drastically while still preserving how useful the information 
is?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D81422/new/

https://reviews.llvm.org/D81422



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D81422: Change filecheck default to dump input on failure

[Joel E. Denny via Phabricator via cfe-commits]

jdenny added a comment.

In D81422#2090725 , @mehdi_amini wrote:

> In D81422#2090618 , @jdenny wrote:
>
> > In D81422#2090425 , @mehdi_amini 
> > wrote:
> >
> > > I'm thinking about a possible improvement here: we could have FileCheck 
> > > dump the input for the current CHECK-LABEL section only: it seems like it 
> > > could reduce the output drastically while still preserving how useful the 
> > > information is?
> >
> >
> > One FileCheck invocation can report multiple errors, one per CHECK-LABEL 
> > section, and I prefer to see all errors every time.
>
>
> Yeah I had in mind to show all the errors for the current section, then the 
> input for the section, before moving to the next section.


I'm not sure I understand. Currently, (1) FileCheck prints all errors up front 
in case that's sufficient without the dump, and then (2) FileCheck prints the 
dump with annotations showing the errors again (and successful directives, 
including labels, if -vv).  Are you talking about interleaving 1 and 2?  Does 
that actually make it easier to read?  Maybe just omit lines from the dump 
instead?

>> I also prefer to see the entire input dump (with annotations produced by 
>> -vv) as sometimes the error FileCheck reports is far away from the actual 
>> error.  For example, maybe the CHECK-LABEL directives matched text at 
>> unexpected locations in the input, and then the directives in their sections 
>> failed as a result. It might be hard to determine that the CHECK-LABEL 
>> directives were at fault without seeing the text it should have matched 
>> outside of its (incorrect) section.
> 
> Sure: but that seems like not the most common case to me: if we start by 
> showing the section we matched it provides already a pretty good indication.
> 
>> If you choose to limit the dump to one CHECK-LABEL section, please make that 
>> behavior optional via a command-line option (which can be set via 
>> FILECHECK_OPTS).
> 
> I thought it would be a better default actually, leaving the full-full as 
> opt-in

I'm not sure how common it is, and I don't know what the default should be.  
From the command line, I can always run again with different options to get 
missing info.  From the bots, it's harder to get missing info (which is why I 
suggested -vv for the bots).


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D81422/new/

https://reviews.llvm.org/D81422



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D81422: Change filecheck default to dump input on failure

[Mehdi AMINI via Phabricator via cfe-commits]

mehdi_amini added a comment.

In D81422#2090618 , @jdenny wrote:

> In D81422#2090425 , @mehdi_amini 
> wrote:
>
> > I'm thinking about a possible improvement here: we could have FileCheck 
> > dump the input for the current CHECK-LABEL section only: it seems like it 
> > could reduce the output drastically while still preserving how useful the 
> > information is?
>
>
> One FileCheck invocation can report multiple errors, one per CHECK-LABEL 
> section, and I prefer to see all errors every time.


Yeah I had in mind to show all the errors for the current section, then the 
input for the section, before moving to the next section.

> I also prefer to see the entire input dump (with annotations produced by -vv) 
> as sometimes the error FileCheck reports is far away from the actual error.  
> For example, maybe the CHECK-LABEL directives matched text at unexpected 
> locations in the input, and then the directives in their sections failed as a 
> result. It might be hard to determine that the CHECK-LABEL directives were at 
> fault without seeing the text it should have matched outside of its 
> (incorrect) section.

Sure: but that seems like not the most common case to me: if we start by 
showing the section we matched it provides already a pretty good indication.

> If you choose to limit the dump to one CHECK-LABEL section, please make that 
> behavior optional via a command-line option (which can be set via 
> FILECHECK_OPTS).

I thought it would be a better default actually, leaving the full-full as opt-in


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D81422/new/

https://reviews.llvm.org/D81422



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D81422: Change filecheck default to dump input on failure

[Matt Arsenault via Phabricator via cfe-commits]

arsenm added a comment.

In D81422#2090425 , @mehdi_amini wrote:

> I'm thinking about a possible improvement here: we could have FileCheck dump 
> the input for the current CHECK-LABEL section only: it seems like it could 
> reduce the output drastically while still preserving how useful the 
> information is?


That would help some. In my situation I've been making a lot of changes that 
touch dozens of generated MIR tests, and change most of the functions. If the 
dump constrained itself to just the first -LABEL section it would be more 
manageable


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D81422/new/

https://reviews.llvm.org/D81422



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D81422: Change filecheck default to dump input on failure

[Mehdi AMINI via Phabricator via cfe-commits]

mehdi_amini added a comment.

In D81422#2083761 , @arsenm wrote:

> I think this is a worse default for development for large tests.


Maybe the issue is with large tests that needs to be broken up?
To me this is a general improvement during development at my desk and reading a 
test output to understand the failure: this goes beyond build bots.

(you have the environment variable you can set locally if you don't want the 
behavior though)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D81422/new/

https://reviews.llvm.org/D81422



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D81422: Change filecheck default to dump input on failure

[Matt Arsenault via Phabricator via cfe-commits]

arsenm added a comment.

In D81422#2083808 , @mehdi_amini wrote:

> In D81422#2083761 , @arsenm wrote:
>
> > I think this is a worse default for development for large tests.
>
>
> Maybe the issue is with large tests that needs to be broken up?


This isn't really manageable, especially with the trend of using update_* test 
checks scripts. Stuff like legalization tests just have to stress every 
combination of inputs. The -dump-input-on-failure could also be smarter about 
how much context it prints

> To me this is a general improvement during development at my desk and reading 
> a test output to understand the failure: this goes beyond build bots.
> 
> (you have the environment variable you can set locally if you don't want the 
> behavior though)

The environment variable was removed though? I would also at least expect this 
to be an option I can set at cmake time and never have to think about again


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D81422/new/

https://reviews.llvm.org/D81422



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D81422: Change filecheck default to dump input on failure

[Mehdi AMINI via Phabricator via cfe-commits]

mehdi_amini added a comment.

In D81422#2083882 , @arsenm wrote:

> In D81422#2083808 , @mehdi_amini 
> wrote:
>
> > In D81422#2083761 , @arsenm wrote:
> >
> > > I think this is a worse default for development for large tests.
> >
> >
> > Maybe the issue is with large tests that needs to be broken up?
>
>
> This isn't really manageable, especially with the trend of using update_* 
> test checks scripts. Stuff like legalization tests just have to stress every 
> combination of inputs.


This is something that the script generating every combination could manage to 
split as well?
Alternatively, can these very large test be appended `--dump-input=never` on 
the RUN line? (maybe the test generator can do this?)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D81422/new/

https://reviews.llvm.org/D81422



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D81422: Change filecheck default to dump input on failure

[Joel E. Denny via Phabricator via cfe-commits]

jdenny added a comment.

In D81422#2083882 , @arsenm wrote:

> The environment variable was removed though? I would also at least expect 
> this to be an option I can set at cmake time and never have to think about 
> again


Could you set `FILECHECK_OPTS=-dump-input=never` in your `~/.profile`?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D81422/new/

https://reviews.llvm.org/D81422



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D81422: Change filecheck default to dump input on failure

[Mehdi AMINI via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGd31c9e5a46ee: Change filecheck default to dump input on 
failure (authored by mehdi_amini).

Changed prior to commit:
  https://reviews.llvm.org/D81422?vs=269581&id=269631#toc

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D81422/new/

https://reviews.llvm.org/D81422

Files:
  clang/test/CodeGenObjC/externally-retained.m
  clang/test/Driver/rocm-device-libs.cl
  compiler-rt/test/fuzzer/fork.test
  debuginfo-tests/llvm-prettyprinters/gdb/llvm-support.gdb
  llvm/docs/CommandGuide/FileCheck.rst
  llvm/test/CodeGen/AArch64/speculation-hardening-dagisel.ll
  llvm/test/CodeGen/AArch64/speculation-hardening-loads.ll
  llvm/test/CodeGen/AArch64/speculation-hardening.ll
  llvm/test/CodeGen/AArch64/speculation-hardening.mir
  llvm/test/FileCheck/comment/after-words.txt
  llvm/test/FileCheck/comment/blank-comments.txt
  llvm/test/FileCheck/comment/suffixes.txt
  llvm/test/FileCheck/comment/suppresses-checks.txt
  llvm/test/FileCheck/comment/unused-comment-prefixes.txt
  llvm/test/FileCheck/dump-input-enable.txt
  llvm/test/FileCheck/envvar-opts.txt
  llvm/test/FileCheck/lit.local.cfg
  llvm/test/FileCheck/match-full-lines.txt
  llvm/test/FileCheck/verbose.txt
  llvm/test/Transforms/InstCombine/fortify-folding.ll
  llvm/utils/FileCheck/FileCheck.cpp
  llvm/utils/lit/lit/TestingConfig.py
  llvm/utils/lit/tests/lit.cfg
  mlir/test/Analysis/test-callgraph.mlir
  mlir/test/Analysis/test-dominance.mlir
  mlir/test/Analysis/test-liveness.mlir
  mlir/test/Conversion/GPUToNVVM/gpu-to-nvvm.mlir
  mlir/test/Conversion/GPUToROCDL/gpu-to-rocdl.mlir
  mlir/test/Conversion/SCFToGPU/no_blocks_no_threads.mlir
  mlir/test/Conversion/SCFToGPU/parallel_loop.mlir
  mlir/test/Conversion/ShapeToStandard/shape-to-standard.mlir
  mlir/test/Dialect/GPU/outlining.mlir
  mlir/test/Dialect/Linalg/fusion-tensor.mlir
  mlir/test/Dialect/Linalg/fusion.mlir
  mlir/test/Dialect/Linalg/fusion_indexed_generic.mlir
  mlir/test/Dialect/Linalg/parallel_loops.mlir
  mlir/test/Dialect/Linalg/tensors-to-buffers.mlir
  mlir/test/Dialect/Linalg/tile_conv_padding.mlir
  mlir/test/Dialect/Linalg/tile_parallel.mlir
  mlir/test/Dialect/SCF/ops.mlir
  mlir/test/Dialect/SCF/parallel-loop-fusion.mlir
  mlir/test/Dialect/SCF/parallel-loop-specialization.mlir
  mlir/test/Dialect/SCF/parallel-loop-tiling.mlir
  mlir/test/Dialect/Shape/ops.mlir
  mlir/test/Dialect/Shape/shape-to-shape.mlir
  mlir/test/Dialect/Standard/expand-atomic.mlir
  mlir/test/Dialect/Vector/vector-contract-transforms.mlir
  mlir/test/Dialect/Vector/vector-flat-transforms.mlir
  mlir/test/EDSC/builder-api-test.cpp
  mlir/test/IR/print-op-local-scope.mlir
  mlir/test/Transforms/buffer-placement-preparation-allowed-memref-results.mlir
  mlir/test/Transforms/buffer-placement-preparation.mlir
  mlir/test/Transforms/buffer-placement.mlir
  mlir/test/Transforms/canonicalize.mlir
  mlir/test/Transforms/sccp-callgraph.mlir
  mlir/test/mlir-tblgen/op-attribute.td
  mlir/test/mlir-tblgen/op-decl.td
  mlir/test/mlir-tblgen/op-derived-attribute.mlir
  mlir/test/mlir-tblgen/op-format-spec.td
  mlir/test/mlir-tblgen/op-interface.td
  mlir/test/mlir-tblgen/pattern.mlir
  mlir/test/mlir-tblgen/predicate.td
  mlir/test/mlir-tblgen/return-types.mlir

Index: mlir/test/mlir-tblgen/return-types.mlir
===
--- mlir/test/mlir-tblgen/return-types.mlir
+++ mlir/test/mlir-tblgen/return-types.mlir
@@ -1,4 +1,4 @@
-// RUN: mlir-opt %s -test-return-type -split-input-file -verify-diagnostics | FileCheck %s --dump-input-on-failure
+// RUN: mlir-opt %s -test-return-type -split-input-file -verify-diagnostics | FileCheck %s
 
 // CHECK-LABEL: testCreateFunctions
 // This function tests invoking the create method with different inference
Index: mlir/test/mlir-tblgen/predicate.td
===
--- mlir/test/mlir-tblgen/predicate.td
+++ mlir/test/mlir-tblgen/predicate.td
@@ -1,4 +1,4 @@
-// RUN: mlir-tblgen -gen-op-defs -I %S/../../include %s | FileCheck %s --dump-input-on-failure
+// RUN: mlir-tblgen -gen-op-defs -I %S/../../include %s | FileCheck %s
 
 include "mlir/IR/OpBase.td"
 
Index: mlir/test/mlir-tblgen/pattern.mlir
===
--- mlir/test/mlir-tblgen/pattern.mlir
+++ mlir/test/mlir-tblgen/pattern.mlir
@@ -1,4 +1,4 @@
-// RUN: mlir-opt -test-patterns -mlir-print-debuginfo %s | FileCheck %s --dump-input-on-failure
+// RUN: mlir-opt -test-patterns -mlir-print-debuginfo %s | FileCheck %s
 
 // CHECK-LABEL: verifyFusedLocs
 func @verifyFusedLocs(%arg0 : i32) -> i32 {
Index: mlir/test/mlir-tblgen/op-interface.td
===
--- mlir/test/mlir-tblgen/op-interface.td
+++ mlir/test/mlir-tblgen/op-interface.td
@@ -1,5 +1,5 @@
-// RUN: mlir-tblgen -gen-op-interface-decls -I 

[PATCH] D81422: Change filecheck default to dump input on failure

[Matt Arsenault via Phabricator via cfe-commits]

arsenm added a comment.

I think this is a worse default for development for large tests. For some 
generated check lines, I'm seeing many thousand of line dumps on failure, which 
just makes it harder to see the point it actually failed at. Can we move this 
default into the buildbot defaults or something so the log is clearer when a 
handful of tests fails? When working on patches, there are often a larger 
number of failures


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D81422/new/

https://reviews.llvm.org/D81422



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D81422: Change filecheck default to dump input on failure

[Joel E. Denny via Phabricator via cfe-commits]

jdenny accepted this revision.
jdenny added a comment.

LGTM.  Thanks!


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D81422/new/

https://reviews.llvm.org/D81422



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D81422: Change filecheck default to dump input on failure

[Mehdi AMINI via Phabricator via cfe-commits]

mehdi_amini updated this revision to Diff 269581.
mehdi_amini marked 4 inline comments as done.
mehdi_amini edited the summary of this revision.
mehdi_amini added a comment.

Address @jdenny's comments:

- fix the example in lit.local.cfg
- Test the default value for dump-input


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D81422/new/

https://reviews.llvm.org/D81422

Files:
  clang/test/CodeGenObjC/externally-retained.m
  clang/test/Driver/rocm-device-libs.cl
  compiler-rt/test/fuzzer/fork.test
  debuginfo-tests/llvm-prettyprinters/gdb/llvm-support.gdb
  llvm/docs/CommandGuide/FileCheck.rst
  llvm/test/CodeGen/AArch64/speculation-hardening-dagisel.ll
  llvm/test/CodeGen/AArch64/speculation-hardening-loads.ll
  llvm/test/CodeGen/AArch64/speculation-hardening.ll
  llvm/test/CodeGen/AArch64/speculation-hardening.mir
  llvm/test/FileCheck/comment/after-words.txt
  llvm/test/FileCheck/comment/blank-comments.txt
  llvm/test/FileCheck/comment/suffixes.txt
  llvm/test/FileCheck/comment/suppresses-checks.txt
  llvm/test/FileCheck/comment/unused-comment-prefixes.txt
  llvm/test/FileCheck/dump-input-enable.txt
  llvm/test/FileCheck/envvar-opts.txt
  llvm/test/FileCheck/lit.local.cfg
  llvm/test/FileCheck/match-full-lines.txt
  llvm/test/FileCheck/verbose.txt
  llvm/test/Transforms/InstCombine/fortify-folding.ll
  llvm/utils/FileCheck/FileCheck.cpp
  llvm/utils/lit/lit/TestingConfig.py
  llvm/utils/lit/tests/lit.cfg
  mlir/test/Analysis/test-callgraph.mlir
  mlir/test/Analysis/test-dominance.mlir
  mlir/test/Analysis/test-liveness.mlir
  mlir/test/Conversion/GPUToNVVM/gpu-to-nvvm.mlir
  mlir/test/Conversion/GPUToROCDL/gpu-to-rocdl.mlir
  mlir/test/Conversion/SCFToGPU/no_blocks_no_threads.mlir
  mlir/test/Conversion/SCFToGPU/parallel_loop.mlir
  mlir/test/Conversion/ShapeToStandard/shape-to-standard.mlir
  mlir/test/Dialect/GPU/outlining.mlir
  mlir/test/Dialect/Linalg/fusion-tensor.mlir
  mlir/test/Dialect/Linalg/fusion.mlir
  mlir/test/Dialect/Linalg/fusion_indexed_generic.mlir
  mlir/test/Dialect/Linalg/parallel_loops.mlir
  mlir/test/Dialect/Linalg/tensors-to-buffers.mlir
  mlir/test/Dialect/Linalg/tile_conv_padding.mlir
  mlir/test/Dialect/Linalg/tile_parallel.mlir
  mlir/test/Dialect/SCF/ops.mlir
  mlir/test/Dialect/SCF/parallel-loop-fusion.mlir
  mlir/test/Dialect/SCF/parallel-loop-specialization.mlir
  mlir/test/Dialect/SCF/parallel-loop-tiling.mlir
  mlir/test/Dialect/Shape/ops.mlir
  mlir/test/Dialect/Shape/shape-to-shape.mlir
  mlir/test/Dialect/Standard/expand-atomic.mlir
  mlir/test/Dialect/Vector/vector-contract-transforms.mlir
  mlir/test/Dialect/Vector/vector-flat-transforms.mlir
  mlir/test/EDSC/builder-api-test.cpp
  mlir/test/IR/print-op-local-scope.mlir
  mlir/test/Transforms/buffer-placement-preparation-allowed-memref-results.mlir
  mlir/test/Transforms/buffer-placement-preparation.mlir
  mlir/test/Transforms/buffer-placement.mlir
  mlir/test/Transforms/canonicalize.mlir
  mlir/test/Transforms/sccp-callgraph.mlir
  mlir/test/mlir-tblgen/op-attribute.td
  mlir/test/mlir-tblgen/op-decl.td
  mlir/test/mlir-tblgen/op-derived-attribute.mlir
  mlir/test/mlir-tblgen/op-format-spec.td
  mlir/test/mlir-tblgen/op-interface.td
  mlir/test/mlir-tblgen/pattern.mlir
  mlir/test/mlir-tblgen/predicate.td
  mlir/test/mlir-tblgen/return-types.mlir

Index: mlir/test/mlir-tblgen/return-types.mlir
===
--- mlir/test/mlir-tblgen/return-types.mlir
+++ mlir/test/mlir-tblgen/return-types.mlir
@@ -1,4 +1,4 @@
-// RUN: mlir-opt %s -test-return-type -split-input-file -verify-diagnostics | FileCheck %s --dump-input-on-failure
+// RUN: mlir-opt %s -test-return-type -split-input-file -verify-diagnostics | FileCheck %s
 
 // CHECK-LABEL: testCreateFunctions
 // This function tests invoking the create method with different inference
Index: mlir/test/mlir-tblgen/predicate.td
===
--- mlir/test/mlir-tblgen/predicate.td
+++ mlir/test/mlir-tblgen/predicate.td
@@ -1,4 +1,4 @@
-// RUN: mlir-tblgen -gen-op-defs -I %S/../../include %s | FileCheck %s --dump-input-on-failure
+// RUN: mlir-tblgen -gen-op-defs -I %S/../../include %s | FileCheck %s
 
 include "mlir/IR/OpBase.td"
 
Index: mlir/test/mlir-tblgen/pattern.mlir
===
--- mlir/test/mlir-tblgen/pattern.mlir
+++ mlir/test/mlir-tblgen/pattern.mlir
@@ -1,4 +1,4 @@
-// RUN: mlir-opt -test-patterns -mlir-print-debuginfo %s | FileCheck %s --dump-input-on-failure
+// RUN: mlir-opt -test-patterns -mlir-print-debuginfo %s | FileCheck %s
 
 // CHECK-LABEL: verifyFusedLocs
 func @verifyFusedLocs(%arg0 : i32) -> i32 {
Index: mlir/test/mlir-tblgen/op-interface.td
===
--- mlir/test/mlir-tblgen/op-interface.td
+++ mlir/test/mlir-tblgen/op-interface.td
@@ -1,5 +1,5 @@
-// RUN: mlir-tblgen -gen-op-interfac

[PATCH] D81422: Change filecheck default to dump input on failure

[Mehdi AMINI via Phabricator via cfe-commits]

mehdi_amini added inline comments.



Comment at: llvm/test/FileCheck/lit.local.cfg:42
 #   ; RUN: %ProtectFileCheckOutput FILECHECK_OPTS=-v \
 #   ; RUN: FileCheck -input-file %s %s 2>&1 \
 #   ; RUN: | FileCheck -check-prefix TRACE %s

jdenny wrote:
> Please add `-dump-input=never` to this first FileCheck call.  Otherwise, the 
> example is broken because FileCheck never prints `expected string found in 
> input` when `-dump-input=fail` regardless of `-v`.
Well spotted!



Comment at: llvm/utils/FileCheck/FileCheck.cpp:117
- "FILECHECK_DUMP_INPUT_ON_FAILURE environment variable.\n"
- "This option is deprecated in favor of -dump-input=fail.\n"));
 

jdenny wrote:
> jdenny wrote:
> > Please mention in the patch summary that `-dump-input-on-failure` and 
> > `FILECHECK_DUMP_INPUT_ON_FAILURE` are being dropped.
> This is marked as done, but I don't see the change.
That's because I amended my commit locally but `arc` does not propagate the 
update to Phabricator apparently: https://phabricator.kde.org/T7711 ; I'll do 
it manually


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D81422/new/

https://reviews.llvm.org/D81422



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D81422: Change filecheck default to dump input on failure

[Joel E. Denny via Phabricator via cfe-commits]

jdenny added inline comments.



Comment at: llvm/test/FileCheck/dump-input-enable.txt:77
 ;--
-; Check no -dump-input, which defaults to never.
 ;--

The previous section already covers `-dump-input=never`. This section was 
covering the case where `-dump-input` isn't specified.  Maybe you should just 
update it to check that the default is now `fail` instead of `never`.



Comment at: llvm/test/FileCheck/lit.local.cfg:42
 #   ; RUN: %ProtectFileCheckOutput FILECHECK_OPTS=-v \
 #   ; RUN: FileCheck -input-file %s %s 2>&1 \
 #   ; RUN: | FileCheck -check-prefix TRACE %s

Please add `-dump-input=never` to this first FileCheck call.  Otherwise, the 
example is broken because FileCheck never prints `expected string found in 
input` when `-dump-input=fail` regardless of `-v`.



Comment at: llvm/utils/FileCheck/FileCheck.cpp:117
- "FILECHECK_DUMP_INPUT_ON_FAILURE environment variable.\n"
- "This option is deprecated in favor of -dump-input=fail.\n"));
 

jdenny wrote:
> Please mention in the patch summary that `-dump-input-on-failure` and 
> `FILECHECK_DUMP_INPUT_ON_FAILURE` are being dropped.
This is marked as done, but I don't see the change.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D81422/new/

https://reviews.llvm.org/D81422



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D81422: Change filecheck default to dump input on failure

[Joel E. Denny via Phabricator via cfe-commits]

jdenny added a comment.

By the way, `-vv` combined with `-dump-input=fail` provides a lot of additional 
information that can be helpful when debugging.  Bots could pass that via 
`FILECHECK_OPTS`, or it could become the default when `-dump-input` is not 
`never`.  In the latter case, I suggest a separate patch to make the review 
easier.  What do people think?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D81422/new/

https://reviews.llvm.org/D81422



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D81422: Change filecheck default to dump input on failure

[Mehdi AMINI via Phabricator via cfe-commits]

mehdi_amini added inline comments.



Comment at: llvm/test/FileCheck/dump-input-enable.txt:78
 ; Check no -dump-input, which defaults to never.
 ;--
 

jdenny wrote:
> Are the tests in this section passing for you now that the default is fail?
Now they do :)
I originally pushed to Phab to get the pre-merge testing running while I was 
still fixing a few things locally, I didn't expect such fast reviews!
Should be all good now.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D81422/new/

https://reviews.llvm.org/D81422



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D81422: Change filecheck default to dump input on failure

[Mehdi AMINI via Phabricator via cfe-commits]

mehdi_amini added a comment.

In D81422#2080758 , @probinson wrote:

> I don't remember the exact reasoning but I believe it had something to do 
> with bot logs?  @jdenny or @thopre might remember.


It'd be interesting to hear about it: having the bot log including the input on 
failure could indeed help debugging when something fails in flaky way, or is 
dependent on the environment (like the path where the code is checked out, 
etc.). 
At least in the past it could have saved me trouble when only a single bot was 
failing in mysterious way and the bot owner had to be involved to help figure 
out.

(also right now we're inconsistent: many tests are adding the flag explicitly)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D81422/new/

https://reviews.llvm.org/D81422



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D81422: Change filecheck default to dump input on failure

[Thomas Preud'homme via Phabricator via cfe-commits]

thopre accepted this revision.
thopre added a comment.

In D81422#2080758 , @probinson wrote:

> I don't remember the exact reasoning but I believe it had something to do 
> with bot logs?  @jdenny or @thopre might remember.


I guess -dump-input just followed the existing default behavior when 
-dump-input-on-failure was introduced. There was several mentions of whether to 
make it the default when that was introduced [1][2] and my understanding was 
the worry was on whether it would make some FileCheck test fail. I think with 
FileCheckOpt this is no longer a concern. I would have personally benefited 
from having this the default so I support this change.

[1] https://reviews.llvm.org/D49328?id=155527#inline-433596
[2] https://reviews.llvm.org/D49328#1170186


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D81422/new/

https://reviews.llvm.org/D81422



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D81422: Change filecheck default to dump input on failure

[Evgenii Stepanov via Phabricator via cfe-commits]

eugenis accepted this revision.
eugenis added a comment.

This would help debugging sanitizer failures on the bots a lot.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D81422/new/

https://reviews.llvm.org/D81422



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D81422: Change filecheck default to dump input on failure

[Joel E. Denny via Phabricator via cfe-commits]

jdenny added inline comments.



Comment at: llvm/utils/FileCheck/FileCheck.cpp:117
- "FILECHECK_DUMP_INPUT_ON_FAILURE environment variable.\n"
- "This option is deprecated in favor of -dump-input=fail.\n"));
 

Please mention in the patch summary that `-dump-input-on-failure` and 
`FILECHECK_DUMP_INPUT_ON_FAILURE` are being dropped.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D81422/new/

https://reviews.llvm.org/D81422



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D81422: Change filecheck default to dump input on failure

[Joel E. Denny via Phabricator via cfe-commits]

jdenny added a comment.

In D81422#2080861 , @mehdi_amini wrote:

> In D81422#2080758 , @probinson wrote:
>
> > I don't remember the exact reasoning but I believe it had something to do 
> > with bot logs?  @jdenny or @thopre might remember.
>


The only problem I've seen is the extreme verbosity when there are many 
failures.  For example, I have experienced cases where that verbosity hung an 
IDE that was collecting the output.  But if people have problems, they can set 
`FILECHECK_OPTS=-dump-input=never`, so it's probably fine.

> It'd be interesting to hear about it: having the bot log including the input 
> on failure could indeed help debugging when something fails in flaky way, or 
> is dependent on the environment (like the path where the code is checked out, 
> etc.). 
>  At least in the past it could have saved me trouble when only a single bot 
> was failing in mysterious way and the bot owner had to be involved to help 
> figure out.
> 
> (also right now we're inconsistent: many tests are adding the flag explicitly)

I imagined more and more bots would set this eventually via FILECHECK_OPTS.  
Changing the default seems fine too.




Comment at: llvm/test/FileCheck/dump-input-enable.txt:78
 ; Check no -dump-input, which defaults to never.
 ;--
 

Are the tests in this section passing for you now that the default is fail?



Comment at: llvm/test/FileCheck/dump-input-enable.txt:127
+; Check -dump-input=fail
 ;--
 

The preceding section checks `-dump-input=fail`, so you can probably just 
remove this `-dump-input-on-failure` section.



Comment at: llvm/test/FileCheck/dump-input-enable.txt:142
+; RUN:   -match-full-lines  -v 2>&1 \
+; RUN: | FileCheck %s -dump-input=never -match-full-lines \
 ; RUN:-check-prefixes=NOTRACE,ERR,DUMP-ERR,DUMP-ERR-V

Why is `-dump-input=never` needed here?


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D81422/new/

https://reviews.llvm.org/D81422



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D81422: Change filecheck default to dump input on failure

[Mehdi AMINI via Phabricator via cfe-commits]

mehdi_amini updated this revision to Diff 269385.
mehdi_amini marked 3 inline comments as done.
mehdi_amini added a comment.
Herald added a subscriber: delcypher.

Fix more tests


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D81422/new/

https://reviews.llvm.org/D81422

Files:
  clang/test/CodeGenObjC/externally-retained.m
  clang/test/Driver/rocm-device-libs.cl
  compiler-rt/test/fuzzer/fork.test
  debuginfo-tests/llvm-prettyprinters/gdb/llvm-support.gdb
  llvm/docs/CommandGuide/FileCheck.rst
  llvm/test/CodeGen/AArch64/speculation-hardening-dagisel.ll
  llvm/test/CodeGen/AArch64/speculation-hardening-loads.ll
  llvm/test/CodeGen/AArch64/speculation-hardening.ll
  llvm/test/CodeGen/AArch64/speculation-hardening.mir
  llvm/test/FileCheck/comment/after-words.txt
  llvm/test/FileCheck/comment/blank-comments.txt
  llvm/test/FileCheck/comment/suffixes.txt
  llvm/test/FileCheck/comment/suppresses-checks.txt
  llvm/test/FileCheck/comment/unused-comment-prefixes.txt
  llvm/test/FileCheck/dump-input-enable.txt
  llvm/test/FileCheck/envvar-opts.txt
  llvm/test/FileCheck/lit.local.cfg
  llvm/test/FileCheck/match-full-lines.txt
  llvm/test/FileCheck/verbose.txt
  llvm/test/Transforms/InstCombine/fortify-folding.ll
  llvm/utils/FileCheck/FileCheck.cpp
  llvm/utils/lit/lit/TestingConfig.py
  llvm/utils/lit/tests/lit.cfg
  mlir/test/Analysis/test-callgraph.mlir
  mlir/test/Analysis/test-dominance.mlir
  mlir/test/Analysis/test-liveness.mlir
  mlir/test/Conversion/GPUToNVVM/gpu-to-nvvm.mlir
  mlir/test/Conversion/GPUToROCDL/gpu-to-rocdl.mlir
  mlir/test/Conversion/SCFToGPU/no_blocks_no_threads.mlir
  mlir/test/Conversion/SCFToGPU/parallel_loop.mlir
  mlir/test/Conversion/ShapeToStandard/shape-to-standard.mlir
  mlir/test/Dialect/GPU/outlining.mlir
  mlir/test/Dialect/Linalg/fusion-tensor.mlir
  mlir/test/Dialect/Linalg/fusion.mlir
  mlir/test/Dialect/Linalg/fusion_indexed_generic.mlir
  mlir/test/Dialect/Linalg/parallel_loops.mlir
  mlir/test/Dialect/Linalg/tensors-to-buffers.mlir
  mlir/test/Dialect/Linalg/tile_conv_padding.mlir
  mlir/test/Dialect/Linalg/tile_parallel.mlir
  mlir/test/Dialect/SCF/ops.mlir
  mlir/test/Dialect/SCF/parallel-loop-fusion.mlir
  mlir/test/Dialect/SCF/parallel-loop-specialization.mlir
  mlir/test/Dialect/SCF/parallel-loop-tiling.mlir
  mlir/test/Dialect/Shape/ops.mlir
  mlir/test/Dialect/Shape/shape-to-shape.mlir
  mlir/test/Dialect/Standard/expand-atomic.mlir
  mlir/test/Dialect/Vector/vector-contract-transforms.mlir
  mlir/test/Dialect/Vector/vector-flat-transforms.mlir
  mlir/test/EDSC/builder-api-test.cpp
  mlir/test/IR/print-op-local-scope.mlir
  mlir/test/Transforms/buffer-placement-preparation-allowed-memref-results.mlir
  mlir/test/Transforms/buffer-placement-preparation.mlir
  mlir/test/Transforms/buffer-placement.mlir
  mlir/test/Transforms/canonicalize.mlir
  mlir/test/Transforms/sccp-callgraph.mlir
  mlir/test/mlir-tblgen/op-attribute.td
  mlir/test/mlir-tblgen/op-decl.td
  mlir/test/mlir-tblgen/op-derived-attribute.mlir
  mlir/test/mlir-tblgen/op-format-spec.td
  mlir/test/mlir-tblgen/op-interface.td
  mlir/test/mlir-tblgen/pattern.mlir
  mlir/test/mlir-tblgen/predicate.td
  mlir/test/mlir-tblgen/return-types.mlir

Index: mlir/test/mlir-tblgen/return-types.mlir
===
--- mlir/test/mlir-tblgen/return-types.mlir
+++ mlir/test/mlir-tblgen/return-types.mlir
@@ -1,4 +1,4 @@
-// RUN: mlir-opt %s -test-return-type -split-input-file -verify-diagnostics | FileCheck %s --dump-input-on-failure
+// RUN: mlir-opt %s -test-return-type -split-input-file -verify-diagnostics | FileCheck %s
 
 // CHECK-LABEL: testCreateFunctions
 // This function tests invoking the create method with different inference
Index: mlir/test/mlir-tblgen/predicate.td
===
--- mlir/test/mlir-tblgen/predicate.td
+++ mlir/test/mlir-tblgen/predicate.td
@@ -1,4 +1,4 @@
-// RUN: mlir-tblgen -gen-op-defs -I %S/../../include %s | FileCheck %s --dump-input-on-failure
+// RUN: mlir-tblgen -gen-op-defs -I %S/../../include %s | FileCheck %s
 
 include "mlir/IR/OpBase.td"
 
Index: mlir/test/mlir-tblgen/pattern.mlir
===
--- mlir/test/mlir-tblgen/pattern.mlir
+++ mlir/test/mlir-tblgen/pattern.mlir
@@ -1,4 +1,4 @@
-// RUN: mlir-opt -test-patterns -mlir-print-debuginfo %s | FileCheck %s --dump-input-on-failure
+// RUN: mlir-opt -test-patterns -mlir-print-debuginfo %s | FileCheck %s
 
 // CHECK-LABEL: verifyFusedLocs
 func @verifyFusedLocs(%arg0 : i32) -> i32 {
Index: mlir/test/mlir-tblgen/op-interface.td
===
--- mlir/test/mlir-tblgen/op-interface.td
+++ mlir/test/mlir-tblgen/op-interface.td
@@ -1,5 +1,5 @@
-// RUN: mlir-tblgen -gen-op-interface-decls -I %S/../../include %s | FileCheck %s --check-prefix=DECL --dump-input-on-failure
-// RUN: m

[PATCH] D81422: Change filecheck default to dump input on failure

[Jacques Pienaar via Phabricator via cfe-commits]

jpienaar accepted this revision.
jpienaar added a comment.

I think this is a good default given it provides very useful info in failure 
case. For cases where folks expect it to fail + don't want it logged, then 
setting never (if that is a concern) seems better behavior IMHO.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D81422/new/

https://reviews.llvm.org/D81422



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D81422: Change filecheck default to dump input on failure

[Mehdi AMINI via Phabricator via cfe-commits]

mehdi_amini updated this revision to Diff 269342.
mehdi_amini added a comment.
Herald added subscribers: Sanitizers, cfe-commits, msifontes, jurahul, Kayjukh, 
frgossen, grosul1, Joonsoo, stephenneuendorffer, liufengdb, aartbik, lucyrfox, 
mgester, arpith-jacob, csigg, nicolasvasilache, antiagainst, shauheen, 
jpienaar, rriddle, jfb, arphaman, jholewinski.
Herald added a reviewer: nicolasvasilache.
Herald added a reviewer: herhut.
Herald added a reviewer: aartbik.
Herald added a reviewer: silvas.
Herald added projects: clang, Sanitizers, MLIR.

Update tests


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D81422/new/

https://reviews.llvm.org/D81422

Files:
  clang/test/CodeGenObjC/externally-retained.m
  clang/test/Driver/rocm-device-libs.cl
  compiler-rt/test/fuzzer/fork.test
  debuginfo-tests/llvm-prettyprinters/gdb/llvm-support.gdb
  llvm/docs/CommandGuide/FileCheck.rst
  llvm/test/CodeGen/AArch64/speculation-hardening-dagisel.ll
  llvm/test/CodeGen/AArch64/speculation-hardening-loads.ll
  llvm/test/CodeGen/AArch64/speculation-hardening.ll
  llvm/test/CodeGen/AArch64/speculation-hardening.mir
  llvm/test/FileCheck/dump-input-enable.txt
  llvm/test/Transforms/InstCombine/fortify-folding.ll
  llvm/utils/FileCheck/FileCheck.cpp
  mlir/test/Analysis/test-callgraph.mlir
  mlir/test/Analysis/test-dominance.mlir
  mlir/test/Analysis/test-liveness.mlir
  mlir/test/Conversion/GPUToNVVM/gpu-to-nvvm.mlir
  mlir/test/Conversion/GPUToROCDL/gpu-to-rocdl.mlir
  mlir/test/Conversion/SCFToGPU/no_blocks_no_threads.mlir
  mlir/test/Conversion/SCFToGPU/parallel_loop.mlir
  mlir/test/Conversion/ShapeToStandard/shape-to-standard.mlir
  mlir/test/Dialect/GPU/outlining.mlir
  mlir/test/Dialect/Linalg/fusion-tensor.mlir
  mlir/test/Dialect/Linalg/fusion.mlir
  mlir/test/Dialect/Linalg/fusion_indexed_generic.mlir
  mlir/test/Dialect/Linalg/parallel_loops.mlir
  mlir/test/Dialect/Linalg/tensors-to-buffers.mlir
  mlir/test/Dialect/Linalg/tile_conv_padding.mlir
  mlir/test/Dialect/Linalg/tile_parallel.mlir
  mlir/test/Dialect/SCF/ops.mlir
  mlir/test/Dialect/SCF/parallel-loop-fusion.mlir
  mlir/test/Dialect/SCF/parallel-loop-specialization.mlir
  mlir/test/Dialect/SCF/parallel-loop-tiling.mlir
  mlir/test/Dialect/Shape/ops.mlir
  mlir/test/Dialect/Shape/shape-to-shape.mlir
  mlir/test/Dialect/Standard/expand-atomic.mlir
  mlir/test/Dialect/Vector/vector-contract-transforms.mlir
  mlir/test/Dialect/Vector/vector-flat-transforms.mlir
  mlir/test/EDSC/builder-api-test.cpp
  mlir/test/IR/print-op-local-scope.mlir
  mlir/test/Transforms/buffer-placement-preparation-allowed-memref-results.mlir
  mlir/test/Transforms/buffer-placement-preparation.mlir
  mlir/test/Transforms/buffer-placement.mlir
  mlir/test/Transforms/canonicalize.mlir
  mlir/test/Transforms/sccp-callgraph.mlir
  mlir/test/mlir-tblgen/op-attribute.td
  mlir/test/mlir-tblgen/op-decl.td
  mlir/test/mlir-tblgen/op-derived-attribute.mlir
  mlir/test/mlir-tblgen/op-format-spec.td
  mlir/test/mlir-tblgen/op-interface.td
  mlir/test/mlir-tblgen/pattern.mlir
  mlir/test/mlir-tblgen/predicate.td
  mlir/test/mlir-tblgen/return-types.mlir

Index: mlir/test/mlir-tblgen/return-types.mlir
===
--- mlir/test/mlir-tblgen/return-types.mlir
+++ mlir/test/mlir-tblgen/return-types.mlir
@@ -1,4 +1,4 @@
-// RUN: mlir-opt %s -test-return-type -split-input-file -verify-diagnostics | FileCheck %s --dump-input-on-failure
+// RUN: mlir-opt %s -test-return-type -split-input-file -verify-diagnostics | FileCheck %s
 
 // CHECK-LABEL: testCreateFunctions
 // This function tests invoking the create method with different inference
Index: mlir/test/mlir-tblgen/predicate.td
===
--- mlir/test/mlir-tblgen/predicate.td
+++ mlir/test/mlir-tblgen/predicate.td
@@ -1,4 +1,4 @@
-// RUN: mlir-tblgen -gen-op-defs -I %S/../../include %s | FileCheck %s --dump-input-on-failure
+// RUN: mlir-tblgen -gen-op-defs -I %S/../../include %s | FileCheck %s
 
 include "mlir/IR/OpBase.td"
 
Index: mlir/test/mlir-tblgen/pattern.mlir
===
--- mlir/test/mlir-tblgen/pattern.mlir
+++ mlir/test/mlir-tblgen/pattern.mlir
@@ -1,4 +1,4 @@
-// RUN: mlir-opt -test-patterns -mlir-print-debuginfo %s | FileCheck %s --dump-input-on-failure
+// RUN: mlir-opt -test-patterns -mlir-print-debuginfo %s | FileCheck %s
 
 // CHECK-LABEL: verifyFusedLocs
 func @verifyFusedLocs(%arg0 : i32) -> i32 {
Index: mlir/test/mlir-tblgen/op-interface.td
===
--- mlir/test/mlir-tblgen/op-interface.td
+++ mlir/test/mlir-tblgen/op-interface.td
@@ -1,5 +1,5 @@
-// RUN: mlir-tblgen -gen-op-interface-decls -I %S/../../include %s | FileCheck %s --check-prefix=DECL --dump-input-on-failure
-// RUN: mlir-tblgen -gen-op-decls -I %S/../../include %s | FileCheck %s --check-prefix=OP_DE

[PATCH] D81422: Change filecheck default to dump input on failure

[Joel E. Denny via Phabricator via cfe-commits]

jdenny added a comment.

Also, please grep for `FILECHECK_DUMP_INPUT_ON_FAILURE`.  There are a few more 
occurrences to remove from the repo.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D81422/new/

https://reviews.llvm.org/D81422



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D81422: Change filecheck default to dump input on failure

[Aart Bik via Phabricator via cfe-commits]

aartbik accepted this revision.
aartbik added a comment.
This revision is now accepted and ready to land.

Yes, assuming nobody remembers an adverse side-effect, I think this change will 
help debugging future failures.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D81422/new/

https://reviews.llvm.org/D81422



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits