This is an archive of the discontinued LLVM Phabricator instance.

Allow llvm-dis to disassemble multiple files
ClosedPublic

Authored by ormris on Apr 22 2021, 2:13 PM.

Details

Summary

Allow llvm-dis to disassemble multiple files

Diff Detail

Event Timeline

ormris requested review of this revision.Apr 22 2021, 2:13 PM
ormris created this revision.
Herald added a project: Restricted Project. · View Herald TranscriptApr 22 2021, 2:13 PM

Can you add a test? Should -o be considered incompatible with multiple files?

llvm/tools/llvm-dis/llvm-dis.cpp
214

The idea with OF_TextWithCRLF is that there needs to be \r\n for Windows.

MaskRay requested changes to this revision.Apr 22 2021, 4:03 PM
This revision now requires changes to proceed.Apr 22 2021, 4:03 PM
ormris added a comment.EditedApr 22 2021, 4:41 PM

Sure, I can add one. What would OutputFilename mean in this context? A prefix for all output files?

llvm/tools/llvm-dis/llvm-dis.cpp
214

Will fix.

ormris updated this revision to Diff 340146.Apr 23 2021, 1:11 PM

Changelog:

  • Add test
  • Fix output mode
ormris marked an inline comment as done.Apr 23 2021, 1:12 PM
MaskRay added inline comments.May 3 2021, 1:53 PM
llvm/test/tools/llvm-dis/multiple-files.ll
2 ↗(On Diff #340146)

cp

4 ↗(On Diff #340146)

Delete llvm-dis < %t0. It's not this file's purpose to test single file.

9 ↗(On Diff #340146)

make sure the prefix error: and filename (if exists) is in the diagnostic.

llvm/tools/llvm-dis/llvm-dis.cpp
161

!OutputFilename.empty()

ormris updated this revision to Diff 343107.May 5 2021, 10:52 AM

Changelog:

  • Fix error condition
  • Remove unneeded test cases
  • Add error prefix
MaskRay accepted this revision.May 5 2021, 11:39 AM

LGTM.

This revision is now accepted and ready to land.May 5 2021, 11:39 AM
ormris added a comment.May 5 2021, 2:54 PM

Thanks for the review!

This revision was automatically updated to reflect the committed changes.