This is an archive of the discontinued LLVM Phabricator instance.

Differential D121888

[LTO][ELF] Require asserts for --stats-file= tests.
ClosedPublic

Authored by MTC on Mar 17 2022, 1:43 AM.

Details

Reviewers
tejohnson
MaskRay
Commits
rG948d05324a15: [LTO][ELF] Require asserts for --stats-file= tests.
Summary

https://reviews.llvm.org/D121809 causes the build bot failure, add the REQUIRES: asserts to fix it.

Diff Detail

Event Timeline

MTC created this revision.Mar 17 2022, 1:43 AM
Herald added a project: Restricted Project. · View Herald TranscriptMar 17 2022, 1:43 AM
Herald added subscribers: ormris, steven_wu, hiraditya and 2 others. · View Herald Transcript
MTC requested review of this revision.Mar 17 2022, 1:43 AM
MTC added a comment.EditedMar 17 2022, 1:47 AM

Sorry for the patch abuse, the pre-merge build bot didn't catch the failures, like https://lab.llvm.org/buildbot/#/builders/124/builds/3948 and https://lab.llvm.org/buildbot/#/builders/67/builds/6214. Hopefully, this patch will fix it.

Harbormaster completed remote builds in B154783: Diff 416093.Mar 17 2022, 1:56 AM
tejohnson accepted this revision.Mar 17 2022, 8:01 AM

lgtm

In D121888#3388554, @MTC wrote:

Sorry for the patch abuse, the pre-merge build bot didn't catch the failures, like https://lab.llvm.org/buildbot/#/builders/124/builds/3948 and https://lab.llvm.org/buildbot/#/builders/67/builds/6214. Hopefully, this patch will fix it.

No worries, this is not an uncommon situation. Also, you don't need a patch review for a small bot fix like this. Generally just go ahead and commit these types of fixes. Alternatively, if the fix is more complicated than something like this, you could revert the original patch and upload the fixed version of the patch for re-review.

This revision is now accepted and ready to land.Mar 17 2022, 8:01 AM
MTC added a comment.Mar 17 2022, 8:15 AM

I appreciate your guidance and the feedback, thanks : )

Closed by commit rG948d05324a15: [LTO][ELF] Require asserts for --stats-file= tests. (authored by MTC). · Explain WhyMar 17 2022, 8:57 AM
This revision was automatically updated to reflect the committed changes.
henry wong <wanliushuai@bytedance.com> added a commit: rG948d05324a15: [LTO][ELF] Require asserts for --stats-file= tests..
Herald added a project: Restricted Project. · View Herald TranscriptMar 17 2022, 8:57 AM
Herald added a subscriber: llvm-commits. · View Herald Transcript

Revision Contents

PathSize
lld/
test/
ELF/
lto/
2 lines

Diff 416198

lld/test/ELF/lto/stats-file-option.ll

; REQUIRES: asserts
; RUN: llvm-as -o %t.bc %s ; RUN: llvm-as -o %t.bc %s
;; Try to save statistics to file. ;; Try to save statistics to file.
; RUN: ld.lld --stats-file=%t2.stats -m elf_x86_64 -r -o %t.o %t.bc ; RUN: ld.lld --stats-file=%t2.stats -m elf_x86_64 -r -o %t.o %t.bc
; RUN: FileCheck --input-file=%t2.stats %s ; RUN: FileCheck --input-file=%t2.stats %s
; CHECK: { ; CHECK: {
; CHECK: "asm-printer.EmittedInsts": ; CHECK: "asm-printer.EmittedInsts":
Show All 20 Lines