Page MenuHomePhabricator

[Driver] Add output file to properties of Command
ClosedPublic

Authored by sepavloff on Apr 27 2020, 12:26 AM.

Details

Summary

Object of class Command contains various properties of a command to
execute, but output file was missed from them. This change adds this
property. It is required for reporting consumed time and memory implemented
in D78903 and may be used in other cases too.

Diff Detail

Event Timeline

sepavloff created this revision.Apr 27 2020, 12:26 AM
Herald added a project: Restricted Project. · View Herald Transcript
DavidTruby resigned from this revision.Apr 27 2020, 3:44 AM
lenary removed a subscriber: lenary.May 6 2020, 4:02 AM

Rebased patch

Clang-formatted the patch

sepavloff updated this revision to Diff 277069.Jul 10 2020, 9:07 AM

Rebased patch

Rebased patch

Adding output files property is a prerequisite for D78903 (Add option -fproc-stat-report). Despite large size of the patch, the change is very simple: it only adds new state variable OutputFilenames initialized by the new parameter to the constructor of Command.

This comment was removed by MaskRay.
sepavloff updated this revision to Diff 289693.Sep 3 2020, 5:35 AM

Rebased patch

sepavloff updated this revision to Diff 291899.Sep 15 2020, 6:50 AM

Rebase and ping

sepavloff edited the summary of this revision. (Show Details)Sep 15 2020, 6:55 AM

This change is similar to D82782.

yaxunl added inline comments.Mon, Oct 5, 5:18 AM
clang/lib/Driver/ToolChains/HIP.cpp
138

I would suggest to use Args.MakeArgString(OutputFileName) instead of Output.c_str() to make sure it persists through the whole compilation.

sepavloff updated this revision to Diff 296451.Tue, Oct 6, 7:01 AM

Use MakeArgString to get persistent string

sepavloff marked an inline comment as done.Tue, Oct 6, 7:08 AM
sepavloff added inline comments.
clang/lib/Driver/ToolChains/HIP.cpp
138

Thank you for advice!

yaxunl added inline comments.Tue, Oct 6, 8:47 AM
clang/include/clang/Driver/Job.h
165

Is this argument only used for reporting compile time and memory usage statistics?

Can it have a default empty argument?

There are lots of out-of-tree code creating 'Command'. Requiring this argument causes lots of hassles for them.

sepavloff updated this revision to Diff 296625.Wed, Oct 7, 2:33 AM
sepavloff marked an inline comment as done.

Add default argument for parameter Output in constructors of Command*

sepavloff added inline comments.Wed, Oct 7, 2:50 AM
clang/include/clang/Driver/Job.h
165

Added default empty argument for Output in Command, CC1Command and ForceSuccessCommand. In FallbackCommand this parameter is not the last, it would require putting None manually.

D78903 will be updated soon.

yaxunl accepted this revision.Wed, Oct 7, 4:46 AM

LGTM. Thanks!

This revision is now accepted and ready to land.Wed, Oct 7, 4:46 AM
This revision was landed with ongoing or failed builds.Thu, Oct 8, 4:26 AM
This revision was automatically updated to reflect the committed changes.