This is an archive of the discontinued LLVM Phabricator instance.

llc: Add -start-before/-stop-before options
ClosedPublic

Authored by MatzeB on Aug 2 2016, 3:27 PM.

Diff Detail

Repository
rL LLVM

Event Timeline

MatzeB updated this revision to Diff 66568.Aug 2 2016, 3:27 PM
MatzeB retitled this revision from to llc: Add -start-before/-stop-before options.
MatzeB updated this object.
MatzeB added reviewers: dexonsmith, qcolombet, arphaman.
MatzeB set the repository for this revision to rL LLVM.
MatzeB added a subscriber: llvm-commits.
dschuff added a subscriber: dschuff.Aug 8 2016, 1:56 PM

Hey, I just discovered this CL, and I'd love to have this for WebAssembly. LGTM, FWIW

dschuff added inline comments.Aug 8 2016, 1:59 PM
test/CodeGen/Generic/llc-start-stop.ll
5 ↗(On Diff #66568)

Could add another line like
; RUN: not llc < %s -debug-pass=Structure -start-before=nonexistent -o /dev/null 2>&1 | FileCheck %s -check-prefix=NONEXISTENT-PASS
...
; NONEXISTENT-PASS: start-before pass is not registered

MatzeB updated this revision to Diff 69273.Aug 25 2016, 11:27 AM

rebased, addressed comment. ping.

hfinkel added inline comments.
include/llvm/CodeGen/TargetPassConfig.h
151 ↗(On Diff #69273)

These asserts should be report_fatal_error; the user might trigger them with bad command-line options (and I assume we still want them in release builds).

qcolombet accepted this revision.Sep 21 2016, 3:45 PM
qcolombet edited edge metadata.

LGTM.

include/llvm/CodeGen/TargetPassConfig.h
151 ↗(On Diff #69273)

Agreed. A follow-up commit is fine given we use to assert on that already.

This revision is now accepted and ready to land.Sep 21 2016, 3:45 PM
MatzeB added inline comments.Sep 23 2016, 2:38 PM
include/llvm/CodeGen/TargetPassConfig.h
151 ↗(On Diff #69273)

I'll leave this as an assert, but will add some error handling/catching for this case to llc.

Reasoning: I'd like to keep the llvm free of (unnecessary) commandline handling logic, that's the frontends job IMO (llc in this case). Ideally we would have all of this start/stop/before/after handling purely in llc, however when I tried to do that it turned out to be quite tricky so I left the code as is and went for the easy enhancement.

This revision was automatically updated to reflect the committed changes.
rnk added a subscriber: rnk.Jan 25 2017, 10:43 AM

Sure, sounds reasonable. In this case we just happen to have fewer library consumers of the exported APIs. The only library consumers are the static thunk libraries.

In D23089#656538, @rnk wrote:

Sure, sounds reasonable. In this case we just happen to have fewer library consumers of the exported APIs. The only library consumers are the static thunk libraries.

Was this comment really meant for this review?

rnk added a comment.Jan 25 2017, 10:59 AM

Was this comment really meant for this review?

Hm, no, it was for https://reviews.llvm.org/D29052.

This patch did not add the options to the documentation.

Herald added projects: Restricted Project, Restricted Project. · View Herald TranscriptAug 16 2022, 6:09 AM
Herald added subscribers: modimo, wenlei. · View Herald Transcript
llvm/trunk/test/CodeGen/Generic/llc-start-stop.ll