This is an archive of the discontinued LLVM Phabricator instance.

Differential D20371

[llc] Remove exit-on-error flag from MIR tests (PR27770)
ClosedPublic

Authored by rovka on May 18 2016, 8:25 AM.

Details

Reviewers
qcolombet
rengolin
arphaman
Commits
rGdb2aff0ab48a: [llc] Remove exit-on-error flag from MIR tests (PR27770)
rL272254: [llc] Remove exit-on-error flag from MIR tests (PR27770)
Summary

This is made possible by removing an assert in llc that assumed MIRParser::parseLLVMModule would exit on error. MIRParser's documentation states that it returns null if a parsing error occurs, so there's no reason to assert. We can instead just fall through to where the check for a module is performed and exit if it is null.

This commit is part of the clean-up after r269655.

Fixes PR27770

Diff Detail

Repository
rL LLVM

Event Timeline

rovka updated this revision to Diff 57629.May 18 2016, 8:25 AM
rovka retitled this revision from to [llc] Remove exit-on-error flag from MIR tests (PR27770).
rovka updated this object.
rovka added a reviewer: arphaman.
rovka added subscribers: llvm-commits, rengolin.
Herald added a subscriber: qcolombet. · View Herald TranscriptMay 18 2016, 8:25 AM
rovka added a comment.May 25 2016, 7:11 AM

Ping?

rovka added a comment.Jun 7 2016, 5:07 AM

Ping again?

rengolin added inline comments.Jun 8 2016, 5:01 AM
tools/llc/llc.cpp
264 ↗(On Diff #57629)

Can you guarantee parseLLVMModule won't return a null pointer?

I mean, it's pretty unlikely, but I'm surprised that this was asserting and being the only reason the *whole* test was "passing".

rovka added inline comments.Jun 8 2016, 5:06 AM
tools/llc/llc.cpp
264 ↗(On Diff #57629)

No, in fact parseLLVMModule is supposed to return null if it finds any errors. But when it does, we don't need to assert, we can just fall through to lines 267-270, where we print those errors and return.

rengolin accepted this revision.Jun 8 2016, 5:20 AM
rengolin added a reviewer: rengolin.

Right, not only this is a no-brainer, but the current behaviour of the tests ("passing" on an unrelated assert) is dangerous.

LGTM, thanks!

tools/llc/llc.cpp
264 ↗(On Diff #57629)

D'oh! Of course!

This revision is now accepted and ready to land.Jun 8 2016, 5:20 AM
qcolombet accepted this revision.Jun 8 2016, 3:08 PM
qcolombet added a reviewer: qcolombet.

LGTM as well.

Closed by commit rL272254: [llc] Remove exit-on-error flag from MIR tests (PR27770) (authored by rovka). · Explain WhyJun 9 2016, 3:37 AM
This revision was automatically updated to reflect the committed changes.

Revision Contents

Diff 60150

llvm/trunk/test/CodeGen/MIR/Generic/invalid-jump-table-kind.mir

# RUN: not llc -start-after branch-folder -stop-after branch-folder -exit-on-error -o /dev/null %s 2>&1 | FileCheck %s # RUN: not llc -start-after branch-folder -stop-after branch-folder -o /dev/null %s 2>&1 | FileCheck %s
--- | --- |
define i32 @test_jumptable(i32 %in) { define i32 @test_jumptable(i32 %in) {
entry: entry:
switch i32 %in, label %def [ switch i32 %in, label %def [
i32 0, label %lbl1 i32 0, label %lbl1
i32 1, label %lbl2 i32 1, label %lbl2
▲ Show 20 Lines • Show All 44 Lines • Show Last 20 Lines

llvm/trunk/test/CodeGen/MIR/Generic/llvm-ir-error-reported.mir

# RUN: not llc -start-after branch-folder -stop-after branch-folder -exit-on-error -o /dev/null %s 2>&1 | FileCheck %s # RUN: not llc -start-after branch-folder -stop-after branch-folder -o /dev/null %s 2>&1 | FileCheck %s
# This test ensures an error is reported if the embedded LLVM IR contains an # This test ensures an error is reported if the embedded LLVM IR contains an
# error. # error.
--- | --- |
; CHECK: [[@LINE+3]]:15: use of undefined value '%a' ; CHECK: [[@LINE+3]]:15: use of undefined value '%a'
define i32 @foo(i32 %x, i32 %y) { define i32 @foo(i32 %x, i32 %y) {
%z = alloca i32, align 4 %z = alloca i32, align 4
Show All 13 Lines

llvm/trunk/test/CodeGen/MIR/Generic/machine-function-missing-function.mir

# RUN: not llc -start-after branch-folder -stop-after branch-folder -exit-on-error -o /dev/null %s 2>&1 | FileCheck %s # RUN: not llc -start-after branch-folder -stop-after branch-folder -o /dev/null %s 2>&1 | FileCheck %s
# This test ensures that an error is reported when the mir file has LLVM IR and # This test ensures that an error is reported when the mir file has LLVM IR and
# one of the machine functions has a name that doesn't match any function in # one of the machine functions has a name that doesn't match any function in
# the LLVM IR. # the LLVM IR.
--- | --- |
define i32 @foo() { define i32 @foo() {
ret i32 0 ret i32 0
Show All 14 Lines

llvm/trunk/test/CodeGen/MIR/Generic/machine-function-missing-name.mir

# RUN: not llc -start-after branch-folder -stop-after branch-folder -exit-on-error -o /dev/null %s 2>&1 | FileCheck %s # RUN: not llc -start-after branch-folder -stop-after branch-folder -o /dev/null %s 2>&1 | FileCheck %s
# This test ensures that an error is reported when a machine function doesn't # This test ensures that an error is reported when a machine function doesn't
# have a name attribute. # have a name attribute.
--- | --- |
define i32 @foo() { define i32 @foo() {
ret i32 0 ret i32 0
} }
Show All 17 Lines

llvm/trunk/test/CodeGen/MIR/Generic/machine-function-redefinition-error.mir

# RUN: not llc -start-after branch-folder -stop-after branch-folder -exit-on-error -o /dev/null %s 2>&1 | FileCheck %s # RUN: not llc -start-after branch-folder -stop-after branch-folder -o /dev/null %s 2>&1 | FileCheck %s
# This test ensures that the machine function errors are reported correctly. # This test ensures that the machine function errors are reported correctly.
--- ---
name: foo name: foo
... ...
--- ---
# CHECK: redefinition of machine function 'foo' # CHECK: redefinition of machine function 'foo'
name: foo name: foo
... ...

llvm/trunk/test/CodeGen/MIR/X86/spill-slot-fixed-stack-object-aliased.mir

# RUN: not llc -march=x86-64 -start-after branch-folder -stop-after branch-folder -exit-on-error -o /dev/null %s 2>&1 | FileCheck %s # RUN: not llc -march=x86-64 -start-after branch-folder -stop-after branch-folder -o /dev/null %s 2>&1 | FileCheck %s
--- | --- |
define i32 @test(i32 %a) #0 { define i32 @test(i32 %a) #0 {
entry: entry:
%b = alloca i32 %b = alloca i32
store i32 %a, i32* %b store i32 %a, i32* %b
%c = load i32, i32* %b %c = load i32, i32* %b
Show All 21 Lines

llvm/trunk/test/CodeGen/MIR/X86/spill-slot-fixed-stack-object-immutable.mir

# RUN: not llc -march=x86-64 -start-after branch-folder -stop-after branch-folder -exit-on-error -o /dev/null %s 2>&1 | FileCheck %s # RUN: not llc -march=x86-64 -start-after branch-folder -stop-after branch-folder -o /dev/null %s 2>&1 | FileCheck %s
--- | --- |
define i32 @test(i32 %a) #0 { define i32 @test(i32 %a) #0 {
entry: entry:
%b = alloca i32 %b = alloca i32
store i32 %a, i32* %b store i32 %a, i32* %b
%c = load i32, i32* %b %c = load i32, i32* %b
Show All 21 Lines

llvm/trunk/test/CodeGen/MIR/X86/variable-sized-stack-object-size-error.mir

# RUN: not llc -march=x86-64 -start-after branch-folder -stop-after branch-folder -exit-on-error -o /dev/null %s 2>&1 | FileCheck %s # RUN: not llc -march=x86-64 -start-after branch-folder -stop-after branch-folder -o /dev/null %s 2>&1 | FileCheck %s
--- | --- |
define i32 @test(i32 %a) { define i32 @test(i32 %a) {
entry: entry:
%b = alloca i32 %b = alloca i32
%x = alloca i64 %x = alloca i64
%y = alloca i32, i32 %a %y = alloca i32, i32 %a
store i32 %a, i32* %b store i32 %a, i32* %b
Show All 25 Lines

llvm/trunk/tools/llc/llc.cpp

Show First 20 Lines • Show All 254 Lines • ▼ Show 20 Linesstatic int compileModule(char **argv, LLVMContext &Context) {
bool SkipModule = MCPU == "help" || bool SkipModule = MCPU == "help" ||
(!MAttrs.empty() && MAttrs.front() == "help"); (!MAttrs.empty() && MAttrs.front() == "help");
// If user just wants to list available options, skip module loading // If user just wants to list available options, skip module loading
if (!SkipModule) { if (!SkipModule) {
if (StringRef(InputFilename).endswith_lower(".mir")) { if (StringRef(InputFilename).endswith_lower(".mir")) {
MIR = createMIRParserFromFile(InputFilename, Err, Context); MIR = createMIRParserFromFile(InputFilename, Err, Context);
if (MIR) { if (MIR)
M = MIR->parseLLVMModule(); M = MIR->parseLLVMModule();
assert(M && "parseLLVMModule should exit on failure");
}
} else } else
M = parseIRFile(InputFilename, Err, Context); M = parseIRFile(InputFilename, Err, Context);
if (!M) { if (!M) {
Err.print(argv[0], errs()); Err.print(argv[0], errs());
return 1; return 1;
} }
// Verify module immediately to catch problems before doInitialization() is // Verify module immediately to catch problems before doInitialization() is
▲ Show 20 Lines • Show All 224 Lines • Show Last 20 Lines