Subzero: Decorate the text asm output with register availability info.

Subzero: Decorate the text asm output with register availability info.

The -asm-verbose flag adds comments to the text asm output about register availability.  Specifically, it prints the registers in use at the beginning and end of each block, and it prints which registers' live ranges end at each instruction.

This is extremely helpful when studying the output to find opportunities to improve the code quality.

BUG= none
R=jvoung@chromium.org

Committed: https://gerrit.chromium.org/gerrit/gitweb?p=native_client/pnacl-subzero.git;a=commit;h=3d44fe8c06eb403ea499e563db886615f0fa2164

[Jim Stichnoth, 2014-10-30 23:34:14]

stichnot@chromium.org changed reviewers:
+ jvoung@chromium.org

[jvoung (off chromium), 2014-10-31 16:56:42]

https://codereview.chromium.org/682983004/diff/20001/src/IceCfgNode.cpp
File src/IceCfgNode.cpp (right):

https://codereview.chromium.org/682983004/diff/20001/src/IceCfgNode.cpp#newco...
src/IceCfgNode.cpp:759: if (I->isUnconditionalBranch())
This doesn't seem to check anymore if the UnconditionalBranch is not deleted. If
there are multiple UnconditionalBranches, all but one should be deleted, but
this depends on the last one being the non-deleted one?

https://codereview.chromium.org/682983004/diff/20001/src/IceCfgNode.cpp#newco...
src/IceCfgNode.cpp:805: void emitRegisterUsage(Ostream &Str, Cfg *Func, const
CfgNode *Node,
Func can be const?

https://codereview.chromium.org/682983004/diff/20001/src/IceCfgNode.cpp#newco...
src/IceCfgNode.cpp:877: Inst *Instr = I;
Can this be factored out into a function too?

At some point, I'm wondering if emitIAS should be factored out into a
Cfg::emitIAS() instead of being part of Cfg::emit(). At least there are some
things like this that wouldn't make sense for it, and I'm still figuring out how
to tie it into the object file container generation after filling the buffer --
something that wouldn't have to be done for emit().

https://codereview.chromium.org/682983004/diff/20001/src/llvm2ice.cpp
File src/llvm2ice.cpp (right):

https://codereview.chromium.org/682983004/diff/20001/src/llvm2ice.cpp#newcode124
src/llvm2ice.cpp:124: "decorate-asm",
fwiw, llc has "-asm-verbose" if you want the flags to be more similar.

[Jim Stichnoth, 2014-11-01 14:49:40]

https://codereview.chromium.org/682983004/diff/20001/src/IceCfgNode.cpp
File src/IceCfgNode.cpp (right):

https://codereview.chromium.org/682983004/diff/20001/src/IceCfgNode.cpp#newco...
src/IceCfgNode.cpp:759: if (I->isUnconditionalBranch())
On 2014/10/31 16:56:42, jvoung wrote:
> This doesn't seem to check anymore if the UnconditionalBranch is not deleted.
If
> there are multiple UnconditionalBranches, all but one should be deleted, but
> this depends on the last one being the non-deleted one?

Ouch, done.

I should try to make InstList into a container whose iterator by default
automatically skips over deleted instructions.

https://codereview.chromium.org/682983004/diff/20001/src/IceCfgNode.cpp#newco...
src/IceCfgNode.cpp:805: void emitRegisterUsage(Ostream &Str, Cfg *Func, const
CfgNode *Node,
On 2014/10/31 16:56:42, jvoung wrote:
> Func can be const?

Done.

https://codereview.chromium.org/682983004/diff/20001/src/IceCfgNode.cpp#newco...
src/IceCfgNode.cpp:877: Inst *Instr = I;
On 2014/10/31 16:56:42, jvoung wrote:
> Can this be factored out into a function too?

Done.

> 
> At some point, I'm wondering if emitIAS should be factored out into a
> Cfg::emitIAS() instead of being part of Cfg::emit(). At least there are some
> things like this that wouldn't make sense for it, and I'm still figuring out
how
> to tie it into the object file container generation after filling the buffer
--
> something that wouldn't have to be done for emit().

Yes, I think that's a good idea.  Another benefit is that it should make it
easier to #ifdef out all the textual emit() code in a minimal build.

https://codereview.chromium.org/682983004/diff/20001/src/llvm2ice.cpp
File src/llvm2ice.cpp (right):

https://codereview.chromium.org/682983004/diff/20001/src/llvm2ice.cpp#newcode124
src/llvm2ice.cpp:124: "decorate-asm",
On 2014/10/31 16:56:42, jvoung wrote:
> fwiw, llc has "-asm-verbose" if you want the flags to be more similar.

Ah, nice.  I changed the command flag string to that

[jvoung (off chromium), 2014-11-01 16:39:12]

lgtm

[Jim Stichnoth, 2014-11-01 17:10:24]

Committed patchset #3 (id:40001) manually as
3d44fe8c06eb403ea499e563db886615f0fa2164 (presubmit successful).