Page MenuHomePhabricator

Fixed a typo in the test s/CEHCK/CHECK/
ClosedPublic

Authored by gribozavr on Feb 25 2019, 5:35 AM.

Details

Summary

Turns out the test was not correct, I had to adjust the test to work. I
also added CHECK-LABELs for better error messages from FileCheck while
I'm here.

Diff Detail

Repository
rL LLVM

Event Timeline

gribozavr created this revision.Feb 25 2019, 5:35 AM
Herald added a project: Restricted Project. · View Herald TranscriptFeb 25 2019, 5:35 AM
jsji accepted this revision.Feb 25 2019, 6:58 AM
jsji added a subscriber: HLJ2009.

LGTM. Thanks for catching this, I am surprised that those typos slipped through ... FYI. @HLJ2009

This revision is now accepted and ready to land.Feb 25 2019, 6:58 AM

LGTM. Thanks for catching this, I am surprised that those typos slipped through ... FYI. @HLJ2009

Two things here.

  1. Why is CHECK-NEXT not being used?
  2. Why is ./utils/update_llc_test_checks.py not being used? That dramatically reduces chances of things like this, and spares one from actually having to manually write proper check lines. (thus improves test coverage. one will of course still need to verify that the checks are correct.)
jsji added a comment.Feb 25 2019, 12:52 PM

Why is CHECK-NEXT not being used?
Why is ./utils/update_llc_test_checks.py not being used? That dramatically reduces chances of things like this, and spares one from actually having to manually write proper check lines. (thus improves test coverage. one will of course still need to verify that the checks are correct.)

Good questions, I think this was tricky and it should be due to "\0A" in the inline asm here.
The output will have empty lines after each assembly line.

#APP
xxsldwi vs0, v2, v2, 3

xscvspdp f0, f0

fctiw f0, f0

mffprd  r3, f0


#NO_APP

So, we can NOT use CHECK-NEXT,
and the script ./utils/update_llc_test_checks.py also can NOT handle it well either:
the output of scripts will have empty string CHECK-NEXT, then causing failures.

inlineasm-vsx-reg.ll:8:15: error: found empty check string with prefix 'CHECK:'
; CHECK-NEXT: 
              ^

But agree that we should recommend using CHECK-NEXT and ./utils/update_llc_test_checks.py if possible.

LGTM. Thanks for catching this, I am surprised that those typos slipped through ... FYI. @HLJ2009

Sorry for this error, I am very grateful to point this out.

This revision was automatically updated to reflect the committed changes.