[PATCH] D35994: Debug info for variables whos type is shrinked to bool

[Hal Finkel via llvm-commits]

On 08/01/2017 11:54 AM, David Blaikie wrote:
>
>
> On Tue, Aug 1, 2017 at 9:43 AM Hal Finkel <hfinkel at anl.gov 
> <mailto:hfinkel at anl.gov>> wrote:
>
>
>     On 08/01/2017 11:28 AM, Adrian Prantl via llvm-commits wrote:
>>
>>>     On Aug 1, 2017, at 9:24 AM, David Blaikie <dblaikie at gmail.com
>>>     <mailto:dblaikie at gmail.com>> wrote:
>>>
>>>
>>>
>>>     On Tue, Aug 1, 2017 at 9:04 AM Adrian Prantl via Phabricator
>>>     <reviews at reviews.llvm.org <mailto:reviews at reviews.llvm.org>> wrote:
>>>
>>>         aprantl added inline comments.
>>>
>>>
>>>         ================
>>>         Comment at: lib/Transforms/IPO/GlobalOpt.cpp:1577
>>>         +  for(auto *GV : GVs)
>>>         +    NewGV->addDebugInfo(GV);
>>>         +
>>>         ----------------
>>>         NikolaPrica wrote:
>>>         > aprantl wrote:
>>>         > > 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)?
>>>         > 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.
>>>         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?)`
>>>         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.
>>>
>>>
>>>     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).
>>>
>>
>>     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.
>>
>>>     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.
>
>     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.
>
>
> 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?

Yes. The code does this:

       // Change the load into a load of bool then a select.
       LoadInst *LI = cast<LoadInst>(UI);
       LoadInst *NLI = new LoadInst(NewGV, LI->getName()+".b", false, 0,
                                    LI->getOrdering(), 
LI->getSyncScopeID(), LI);
       Value *NSI;
       if (IsOneZero)
         NSI = new ZExtInst(NLI, LI->getType(), "", LI);
       else
         NSI = SelectInst::Create(NLI, OtherVal, InitVal, "", LI);
       NSI->takeName(LI);
       LI->replaceAllUsesWith(NSI);

  -Hal

>
> Oh. Yeah. That's a bit more involved. (but nice that it means we can 
> represent the values exactly - hopefully)
>
> Thanks!
>
>     I think you actually want to generate a DWARF expression for these.
>
>      -Hal
>
>>
>>     Agreed, that would be the most practical solution — if we can
>>     prove that it is correct.
>>
>>     -- adrian
>>>
>>>     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.
>>>
>>>     Just mention it to check/understand what the optimization
>>>     is/isn't doing, etc.
>>>
>>>
>>>
>>>         https://reviews.llvm.org/D35994
>>>
>>
>>
>>
>>     _______________________________________________
>>     llvm-commits mailing list
>>     llvm-commits at lists.llvm.org <mailto:llvm-commits at lists.llvm.org>
>>     http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>
>     -- 
>     Hal Finkel
>     Lead, Compiler Technology and Programming Languages
>     Leadership Computing Facility
>     Argonne National Laboratory
>

-- 
Hal Finkel
Lead, Compiler Technology and Programming Languages
Leadership Computing Facility
Argonne National Laboratory

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170801/b2f2ce05/attachment.html>