This is an archive of the discontinued LLVM Phabricator instance.

[MsgPack] Attempt to fix failure on windows expensive checks bot
ClosedPublic

Authored by tpr on Mar 14 2019, 4:26 PM.

Details

Summary

My theory is that the failure was caused by me forgetting to flush a
raw_string_ostream.

Change-Id: I9c6208325503b3ee0786b4b688e13fc24a15babf

Diff Detail

Repository
rL LLVM

Event Timeline

tpr created this revision.Mar 14 2019, 4:26 PM
Herald added a project: Restricted Project. · View Herald TranscriptMar 14 2019, 4:26 PM
gkistanova requested changes to this revision.Mar 14 2019, 10:58 PM

This makes things better, but did not resolve the problem completely.
The following 2 tests are still failing:

********************
Failing Tests (2):
    LLVM :: CodeGen/MIR/AMDGPU/machine-function-info-no-ir.mir
    LLVM :: CodeGen/MIR/AMDGPU/machine-function-info.ll

Both failures has the same reason behind:

>c:\ps4-buildslave2\llvm-clang-x86_64-expensive-checks-win\build\bin\llc.exe -run-pass=none -verify-machineinstrs C:\ps4-buildslave2\llvm-clang-x86_64-expensive-checks-win\build\test\CodeGen\MIR\AMDGPU\Output\machine-function-info.ll.tmp.mir -o -
error: C:\ps4-buildslave2\llvm-clang-x86_64-expensive-checks-win\build\test\CodeGen\MIR\AMDGPU\Output\machine-function-info.ll.tmp.mir:114:21: expected a named register
  scratchRSrcReg:  ''
                    ^

I'll send you the build\test\CodeGen\MIR\AMDGPU\Output\machine-function-info.ll.tmp.mir by e-mail, so you could reproduce the problem locally and play with that.

This revision now requires changes to proceed.Mar 14 2019, 10:58 PM
tpr updated this revision to Diff 190794.Mar 15 2019, 2:19 AM

V2: Also fix the same bug in Matt's AMDGPU MIR roundtrip commit.

tpr added a reviewer: arsenm.Mar 15 2019, 2:19 AM
arsenm added inline comments.Mar 15 2019, 9:36 AM
lib/BinaryFormat/MsgPackDocumentYAML.cpp
64 ↗(On Diff #190794)

Why is this necessary?

lib/Target/AMDGPU/SIMachineFunctionInfo.cpp
326–327 ↗(On Diff #190794)

Maybe this should be scoped instead of the explicit flush?

tpr marked 2 inline comments as done.Mar 16 2019, 5:18 AM
tpr added inline comments.
lib/BinaryFormat/MsgPackDocumentYAML.cpp
64 ↗(On Diff #190794)

str() flushes the raw_string_ostream before returning the string.

tpr updated this revision to Diff 190955.Mar 16 2019, 5:25 AM
tpr marked an inline comment as done.

V3: Adjusted the MIR roundtrip fix to use scoping instead of explicit flush.

tpr marked an inline comment as done.Mar 16 2019, 5:26 AM
arsenm accepted this revision.Mar 16 2019, 9:28 AM

LGTM

gkistanova accepted this revision.Mar 16 2019, 5:29 PM

This version would keep the llvm-clang-x86_64-expensive-checks-win bot happy.
Thanks, Tim!

This revision is now accepted and ready to land.Mar 16 2019, 5:29 PM
This revision was automatically updated to reflect the committed changes.