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

[David Blaikie via llvm-commits]

On Fri, Jul 28, 2017 at 8:36 AM David Blaikie <dblaikie at gmail.com> wrote:

> On Fri, Jul 28, 2017 at 8:31 AM Adrian Prantl via Phabricator via
> llvm-commits <llvm-commits at lists.llvm.org> wrote:
>
>> aprantl added inline comments.
>>
>>
>> ================
>> Comment at: lib/Transforms/IPO/GlobalOpt.cpp:1577
>> +  for(auto *GV : GVs)
>> +    NewGV->addDebugInfo(GV);
>> +
>> ----------------
>> 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)?
>>
>
Right - may be worth having two bools in the test to show how they get
packed, if necessary. (& even if they do get packed on byte boundaries -
GlobalOpt handles more than just bools, so in that case might be worth
checking what happens if two larger-than-byte size values are value
constrained to only need a byte get packed together, etc)


>
>>
>> ================
>> Comment at: test/Transforms/GlobalOpt/static-global-boolean-dwarf.c:2
>> +// RUN: clang -g -O2 -c %s -o %t
>> +// RUN: llvm-dwarfdump %t | FileCheck %s
>> +
>> ----------------
>> We shouldn't require a frontend to be available when running tests. Can
>> you use thew output of clang -S -emit-llvm instead an run it through opt?
>>
>
> Also I imagine the test could be further minimized? Is the enum necessary,
> for example (oh, perhaps since it's C it is? not sure - I'd probably write
> it in C++ using 'bool')? Could it be simplified by having the boolean value
> (but not its address) leak out of the translation unit - though I guess
> that's what 'get_foo' does. Does init_boolean need to be that complicated?
> Or could it be : "void set_foo(bool b) { foo = b; }" & still trigger the
> right amount of optimization? (so I'd expect the test case to come down to:
> "static bool foo; void set_foo(bool b) { foo = b; } bool get_foo() { return
> foo; }")
>
>
>>
>>
>> https://reviews.llvm.org/D35994
>>
>>
>>
>> _______________________________________________
>> llvm-commits mailing list
>> llvm-commits at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/llvm-commits/attachments/20170728/7888c050/attachment.html>