Page MenuHomePhabricator

[docs][Bugpoint]Add notes about multiple crashes
ClosedPublic

Authored by jsji on Aug 27 2019, 2:49 PM.

Details

Summary
When reducing case for a CodeGenCrash, bugpoint may generate a new
reduced
testcase that exposes/causes another crash or break something due to
limitation.

Bugpoint does not distiguish different crashes currently,
so when this happens, bugpoint will go on reducing for the new crash,
or just abort, we can't get the case reduced for the origial crash.

An advice is added into usage doc to connect to recommend checking error
message with scripts and `-compile-command`.

Diff Detail

Repository
rL LLVM

Event Timeline

jsji created this revision.Aug 27 2019, 2:49 PM
Herald added a project: Restricted Project. · View Herald TranscriptAug 27 2019, 2:49 PM
jsji updated this revision to Diff 217503.Aug 27 2019, 2:52 PM

Fix typos in testcase.

Harbormaster completed remote builds in B37383: Diff 217503.

Ping..
and add the most recent code modifier & patch reviewers . Thanks!

vsk added a comment.Sep 9 2019, 5:21 PM

I think you've identified a real, pernicious problem, but am not sure that this is the best approach for addressing it. One potential problem here is that slight variations in the error message can confuse the reducer. If e.g. some location information changes in between messages, the reduction will stop too early.

Maybe we need better documentation/examples about how to write check scripts that validate the error message?

jsji added a comment.Sep 10 2019, 10:44 AM

Thanks @vsk for the valuable comments.

In D66832#1664105, @vsk wrote:

I think you've identified a real, pernicious problem, but am not sure that this is the best approach for addressing it. One potential problem here is that slight variations in the error message can confuse the reducer. If e.g. some location information changes in between messages, the reduction will stop too early.

Maybe we need better documentation/examples about how to write check scripts that validate the error message?

Yes, you are absolutely right!
In some complex cases, the change might looks trivial, eg: location information, or call stack change.
And, yes, we should be able to use -custom-command and write a simple script to validate.

eg, for the testcase in llvm/test/BugPoint/crash-sameerror.ll, we should be able to use a custom-command line like:

$ cat t1.sh
#!/bin/bash
!(llc < $1 2>&1|grep "LLVM ERROR: Cannot select: intrinsic %llvm.ppc.altivec.dss" )

$ bugpoint -compile-custom -compile-command ./t1.sh t.ll

I think the major point of this patch is to make it easier for beginners, for common scenarios:
for most simple code gen crash, we can simply use follow command to reduce the testcases.

$ bugpoint --llc-safe t.ll -same-error 
jsji added a comment.EditedSep 10 2019, 10:48 AM

I think it would be great to make the powerful bugpoint tool easier to use, especially for beginners, for common simple scenarios.

But if we don't think this is really necessary or in the wrong direction of design, I am OK to abandon this patch.

I think it would be great to make the powerful bugpoint tool easier to use, especially for beginners, for common simple scenarios.

But if we don't think this is really necessary or in the wrong direction of design, I am OK to abandon this patch.

I see the need to make error validation with bugpoint more beginner-friendly, but am worried about situations in which the mode proposed here could confuse beginners (what if the first error isn't the interesting one? or reductions stop too early?). I don't have a better alternative to suggest. Perhaps we could repost @mehdi_amini's writeup in the "Search for a string in the output" section within http://blog.llvm.org/2015/11/reduce-your-testcases-with-bugpoint-and.html, or link to it from the bugpoint docs.

jsji updated this revision to Diff 219606.Sep 10 2019, 2:18 PM

Abandon the --same-error change, update docs instead.

jsji retitled this revision from [Bugpoint][CrashDebugger] Add --same-error to skip non-relevant CodeGenCrash to [NFC][Bugpoint][CrashDebugger] Add notes about multiple crashes.Sep 10 2019, 2:20 PM
jsji edited the summary of this revision. (Show Details)
jsji added a comment.Sep 10 2019, 2:24 PM

> I see the need to make error validation with bugpoint more beginner-friendly, but am worried about situations in which the mode proposed here could confuse beginners (what if the first error isn't the interesting one? or reductions stop too early?). I don't have a better alternative to suggest. Perhaps we could repost @mehdi_amini's writeup in the "Search for a string in the output" section within http://blog.llvm.org/2015/11/reduce-your-testcases-with-bugpoint-and.html, or link to it from the bugpoint docs.

Make sense, the more flag, the more confusion. So I abandon the original change.

I am not sure whether it is appropriate to point to a writeup in blog, so I just simple refer it to the command in the official documents.
@vsk Any comments?

jsji retitled this revision from [NFC][Bugpoint][CrashDebugger] Add notes about multiple crashes to [docs][Bugpoint]Add notes about multiple crashes.Sep 12 2019, 9:03 AM
jsji added a comment.Tue, Sep 17, 7:05 AM

Ping? @vsk Do you think it worth updating Bugpoint.rst? If so, any comments? Thanks.

vsk accepted this revision.Tue, Sep 17, 11:02 AM

Thanks, this is quite helpful.

This revision is now accepted and ready to land.Tue, Sep 17, 11:02 AM
This revision was automatically updated to reflect the committed changes.