<div dir="ltr"><br><br><div class="gmail_quote"><div dir="ltr">On Tue, Aug 1, 2017 at 1:09 PM Adrian Prantl <<a href="mailto:aprantl@apple.com">aprantl@apple.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word"><div><blockquote type="cite"><div>On Aug 1, 2017, at 10:53 AM, David Blaikie <<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>> wrote:</div><br class="m_-4714356479468025920Apple-interchange-newline"><div><div dir="ltr">DW_OP_bra looks like it'd be a bit of a pain to work with since it's number of bytes (so you'll have to figure out the actual length of the expression sequence - not just the number of operations, etc). The LEB encoding in particular worries me a bit as being a pain to have to do that first, then compute the encoded length to figure out how many bytes to skip, etc. But that's not impossible/that arcane or anything... If that works out OK, great. :)<br></div></div></blockquote><div><br></div></div></div><div style="word-wrap:break-word"><div><div>In DIExpression we could use the convention that the argument to DW_OP_bra encodes the number of operations (=DIExpression::ExprOperand iterators) to skip.</div></div></div></blockquote><div><br>Oh, right, the IR representation doesn't even known the number of bytes... blessing & a curse, I guess.<br> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word"><div><div> In DwarfExpression.cpp we could get AsmPrinter to report back the length of the everything emitted (including the LEBs) — what is tricky is how to encode a forward reference such as the number of bytes to skip until the end of the expression. </div></div></div></blockquote><div><br></div><div>Not quite sure I follow - I would picture it as: encode the OP_bra into the buffer but not its argument yet - encode aurgement-many operands into a temp buffer, measure its size, write that to the main buffer, then flush the temp buffer to the main buffer.<br><br>But I guess that's the two pass you mention below? (& this could be N pass if an OP_bra jumped over another OP_bra)</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word"><div><div>We would have to make DwarfExpression run two passes: first emit the expression into a buffer and then fix up the relative jump targets.</div></div></div><div style="word-wrap:break-word"><div><br><blockquote type="cite"><div><div dir="ltr">(alternatively we could do some arithmetic and take the 42/87 example: (87 - 42) * val + 42. Admittedly it works easily when the 1 boolean value is associated with the larger of the two numbers... - but may be easier to do the more verbose & fussy (in terms of the byte computatiotns) version that's a bit more explicit about the values being used, etc)<br></div></div></blockquote><div><br></div></div></div><div style="word-wrap:break-word"><div><div>Another reason to prefer purely arithmetic solutions is that they might be (though I don't know enough about that) easier to compile into CodeView that arbitrary programs with control flow in them.</div></div></div></blockquote><div><br>Maybe? I probably wouldn't speculate/bother about that without evidence, though. The conditional form might be fine to reformulate in limited ways (if we know we'll only see certain kindsn of conditionals, etc - or at least only support certain kinds and punt on the rest, etc)</div><div> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word"><div><div><br></div><div>Here's another attempt that doesn't assume anything about the two values:</div><div><br></div><div>// val * 42 | ((~val)&1) * 87</div><div>DW_OP_dup DW_OP_constu 42 DW_OP_mul DW_OP_swap</div><div>DW_OP_not DW_OP_lit1 DW_OP_and DW_OP_constu 87 DW_OP_mul</div><div>DW_OP_or DW_OP_stack_value</div></div></div></blockquote><div><br>Oh, not bad at all. (not sure which of these are longer and by how much - but I'm not sure minimizing complex locations is really a problem/priority - though I haven't done a complex breakdown of optimized code debug size, etc)<br><br>- Dave<br> </div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word"><div><div><br></div>-- adrian</div></div><div style="word-wrap:break-word"><div><br><blockquote type="cite"><div><div dir="ltr"><br>*shrug* :)</div><br><div class="gmail_quote"><div dir="ltr">On Tue, Aug 1, 2017 at 10:47 AM Adrian Prantl <<a href="mailto:aprantl@apple.com" target="_blank">aprantl@apple.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><div style="word-wrap:break-word"><div><blockquote type="cite"><div>On Aug 1, 2017, at 10:35 AM, Hal Finkel <<a href="mailto:hfinkel@anl.gov" target="_blank">hfinkel@anl.gov</a>> wrote:</div><br class="m_-4714356479468025920m_5082754805788286079Apple-interchange-newline"><div>
<div bgcolor="#FFFFFF" text="#000000"><p><br>
</p>
<div class="m_-4714356479468025920m_5082754805788286079moz-cite-prefix">On 08/01/2017 12:28 PM, Adrian Prantl
wrote:<br>
</div>
<blockquote type="cite">
<br>
<div>
<blockquote type="cite">
<div>On Aug 1, 2017, at 10:20 AM, Hal Finkel <<a href="mailto:hfinkel@anl.gov" target="_blank">hfinkel@anl.gov</a>> wrote:</div>
<br class="m_-4714356479468025920m_5082754805788286079Apple-interchange-newline">
<div>
<div bgcolor="#FFFFFF" text="#000000"><p><br>
</p>
<div class="m_-4714356479468025920m_5082754805788286079moz-cite-prefix">On 08/01/2017 11:54 AM, David
Blaikie wrote:<br>
</div>
<blockquote type="cite">
<div dir="ltr"><br>
<br>
<div class="gmail_quote">
<div dir="ltr">On Tue, Aug 1, 2017 at 9:43
AM Hal Finkel <<a href="mailto:hfinkel@anl.gov" target="_blank">hfinkel@anl.gov</a>>
wrote:<br>
</div>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div bgcolor="#FFFFFF" text="#000000"><p><br>
</p>
<div class="m_-4714356479468025920m_5082754805788286079m_838875341229329508moz-cite-prefix">On
08/01/2017 11:28 AM, Adrian Prantl via
llvm-commits wrote:<br>
</div>
<blockquote type="cite"> <br>
<div>
<blockquote type="cite">
<div>On Aug 1, 2017, at 9:24 AM,
David Blaikie <<a href="mailto:dblaikie@gmail.com" target="_blank">dblaikie@gmail.com</a>>
wrote:</div>
<br class="m_-4714356479468025920m_5082754805788286079m_838875341229329508Apple-interchange-newline">
<div>
<div dir="ltr" style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant-caps:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px"><br>
<br>
<div class="gmail_quote">
<div dir="ltr">On Tue, Aug
1, 2017 at 9:04 AM Adrian Prantl
via Phabricator <<a href="mailto:reviews@reviews.llvm.org" target="_blank">reviews@reviews.llvm.org</a>>
wrote:<br>
</div>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex">aprantl
added inline comments.<br>
<br>
<br>
================<br>
Comment at:
lib/Transforms/IPO/GlobalOpt.cpp:1577<br>
+ for(auto *GV : GVs)<br>
+ NewGV->addDebugInfo(GV);<br>
+<br>
----------------<br>
NikolaPrica wrote:<br>
> aprantl wrote:<br>
> > I'm not familiar with
this transformation: Do we need to
add a DIExpression to mask out all
but the first bit (i.e. can
multiple bools be packed into the
same uint32_t such that it could
confuse debuggers)?<br>
> The debug info which is
provided here with addDebugInfo
later generates address of the
variable (DW_OP_addr: xxxx) in
DW_AT_location. If we provide here
metadata which is for 1byte
variable the debugger would get
confused because the enum type is
written as 4-byte and he would try
to read 4 bytes. This is just
temporary fix until proper way to
handle this is found.<br>
If I understood you correctly then
the best way to represent this
would be a `DW_OP_LLVM_fragment
/*offset*/0 /*bitsize*/1 (or 8?)`<br>
expression. This will get lowered
into a DW_OP_bit_piece to tell the
debugger that this location is
describing of the first n bits of
the variable.<br>
</blockquote>
<div><br>
A slight problem with this is that
at least GDB won't print a single
value if it's partially
unavailable (eg: if it's a struct
and the fragment describes one
member but not the othe,r I think
that's OK - but if it's a single
int and only one out of 4 bytes
are described - well, the value is
known/unknowable).<br>
<br>
</div>
</div>
</div>
</div>
</blockquote>
<div><br>
</div>
<div>A debugger would have to print
the value as something like 0x??????01 to
indicate that pieces are missing. But no
debugger I'm aware of does that.</div>
<br>
<blockquote type="cite">
<div>
<div dir="ltr" style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant-caps:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px">
<div class="gmail_quote">
<div>If this optimization
is really based on the proof that
the other bytes are unused, never
written to or read, then a
fragment describing the other
bytes as constant zero would be
good to have as well.<br>
</div>
</div>
</div>
</div>
</blockquote>
</div>
</blockquote>
<br>
</div>
<div bgcolor="#FFFFFF" text="#000000">
No, it is more complicated than that. See
TryToShrinkGlobalToBoolean in
lib/Transforms/IPO/GlobalOpt.cpp. It is looking
for cases where the global is only set to one
initialized value and one other value. In that
case, it can make a Boolean global and replace
uses of that Global with selects over that
value.</div>
</blockquote>
<div><br>
Oh, wait - you mean a variable with value 42 and
87 gets collapsed to a boolean where true/1 is
used to represent 42 and false/0 is used to
represent 87?<br>
</div>
</div>
</div>
</blockquote>
<br>
Yes. The code does this:<br>
<br>
// Change the load into a load of bool then a
select.<br>
LoadInst *LI = cast<LoadInst>(UI);<br>
LoadInst *NLI = new LoadInst(NewGV,
LI->getName()+".b", false, 0,<br>
LI->getOrdering(),
LI->getSyncScopeID(), LI);<br>
Value *NSI;<br>
if (IsOneZero)<br>
NSI = new ZExtInst(NLI, LI->getType(), "", LI);<br>
else<br>
NSI = SelectInst::Create(NLI, OtherVal, InitVal,
"", LI);<br>
NSI->takeName(LI);<br>
LI->replaceAllUsesWith(NSI);<br>
</div>
</div>
</blockquote>
<div><br>
</div>
<div>Whoa — this is going to be fun! A DWARF expression for that
example might look like:</div>
<div><br>
</div>
<div>DW_OP_constu 42 DW_OP_mul DW_OP_dup DW_OP_bra +2 DW_OP_skip
+<end-1> DW_OP_constu 87 DW_OP_constu DW_OP_stack_value </div>
<div><br>
</div>
<div>roughly: if (Val * 42) return Val * 42; else return 87;</div>
</div>
</blockquote>
<br>
Why can't you just encode: 'if (Val) return 42; else return 87;'?<br></div></div></blockquote><div><br></div></div></div><div style="word-wrap:break-word"><div><div>Yes that would be even shorter.</div><div>DW_OP_bra +4? DW_OP_constu 42 DW_OP_skip +<end-1> DW_OP_constu 87 DW_OP_constu DW_OP_stack_value</div><div><div> I started out with trying to craft an expression that wouldn't need any control flow and then got carried away :-)</div></div></div></div><div style="word-wrap:break-word"><div><div><br></div><div>-- adrian</div></div></div><div style="word-wrap:break-word"><div><br><blockquote type="cite"><div><div bgcolor="#FFFFFF" text="#000000">
<br>
-Hal<br>
<br>
<blockquote type="cite">
<div>
<div><br>
</div>
<div>-- adrian</div>
<blockquote type="cite">
<div>
<div bgcolor="#FFFFFF" text="#000000"> <br>
-Hal<br>
<br>
<blockquote type="cite">
<div dir="ltr">
<div class="gmail_quote">
<div><br>
Oh. Yeah. That's a bit more involved. (but nice
that it means we can represent the values exactly
- hopefully)<br>
<br>
Thanks!<br>
</div>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">
<div bgcolor="#FFFFFF" text="#000000"> I
think you actually want to generate a DWARF
expression for these.<br>
<br>
-Hal<br>
<br>
</div>
<div bgcolor="#FFFFFF" text="#000000">
<blockquote type="cite">
<div>
<div><br>
</div>
Agreed, that would be the most practical
solution — if we can prove that it is
correct.</div>
<div><br>
</div>
<div>-- adrian<br>
<blockquote type="cite">
<div>
<div dir="ltr" style="font-family:Helvetica;font-size:12px;font-style:normal;font-variant-caps:normal;font-weight:normal;letter-spacing:normal;text-align:start;text-indent:0px;text-transform:none;white-space:normal;word-spacing:0px">
<div class="gmail_quote">
<div><br>
But I doubt that's the case - no
doubt if we can see all the reads
and writes, we optimize away the
writes if we know they won't be
read, so we may end up with only
the one byte. C'est la vie.<br>
<br>
Just mention it to
check/understand what the
optimization is/isn't doing, etc.<br>
</div>
<blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left-width:1px;border-left-style:solid;border-left-color:rgb(204,204,204);padding-left:1ex"><br>
<br>
<a href="https://reviews.llvm.org/D35994" rel="noreferrer" target="_blank">https://reviews.llvm.org/D35994</a></blockquote>
</div>
</div>
</div>
</blockquote>
</div>
<br>
<br>
<fieldset class="m_-4714356479468025920m_5082754805788286079m_838875341229329508mimeAttachmentHeader"></fieldset>
<br>
<pre>_______________________________________________
llvm-commits mailing list
<a class="m_-4714356479468025920m_5082754805788286079m_838875341229329508moz-txt-link-abbreviated" href="mailto:llvm-commits@lists.llvm.org" target="_blank">llvm-commits@lists.llvm.org</a>
<a class="m_-4714356479468025920m_5082754805788286079m_838875341229329508moz-txt-link-freetext" href="http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits</a>
</pre>
</blockquote>
</div>
<div bgcolor="#FFFFFF" text="#000000"> <br>
<pre class="m_-4714356479468025920m_5082754805788286079m_838875341229329508moz-signature" cols="72">--
Hal Finkel
Lead, Compiler Technology and Programming Languages
Leadership Computing Facility
Argonne National Laboratory</pre>
</div>
</blockquote>
</div>
</div>
</blockquote>
<br>
<pre class="m_-4714356479468025920m_5082754805788286079moz-signature" cols="72">--
Hal Finkel
Lead, Compiler Technology and Programming Languages
Leadership Computing Facility
Argonne National Laboratory</pre>
</div>
</div>
</blockquote>
</div>
<br>
</blockquote>
<br>
<pre class="m_-4714356479468025920m_5082754805788286079moz-signature" cols="72">--
Hal Finkel
Lead, Compiler Technology and Programming Languages
Leadership Computing Facility
Argonne National Laboratory</pre>
</div>
</div></blockquote></div></div></blockquote></div>
</div></blockquote></div></div></blockquote></div></div>