[PATCH] D24426: DebugInfo: Pass non-zero alignment to DIBuilder only if aligment was forced

[Victor Leschuk via cfe-commits]

I think it's better to pass amount in bytes here, as specified by user 
(alignas takes bytes, not bits).
On 09/13/2016 09:00 PM, Robinson, Paul wrote:
>
> I hadn't thought Clang wanted to be *quite* so knowledgeable about 
> targets, and similarly not so tightly tied to byte-addressable 
> targets.  But if both of those things are actually okay, then it's 
> fine to set the alignment value here to what would be passed through 
> to DWARF.
>
> --paulr
>
> *From:*David Blaikie [mailto:dblaikie at gmail.com]
> *Sent:* Monday, September 12, 2016 6:11 PM
> *To:* Robinson, Paul; 
> reviews+D24426+public+6ee6274d38fdf0d6 at reviews.llvm.org; 
> vleschuk at accesssoftek.com; echristo at gmail.com; aprantl at apple.com; 
> mehdi.amini at apple.com
> *Cc:* cfe-commits (cfe-commits at lists.llvm.org)
> *Subject:* Re: [PATCH] D24426: DebugInfo: Pass non-zero alignment to 
> DIBuilder only if aligment was forced
>
> On Mon, Sep 12, 2016 at 6:01 PM Robinson, Paul <paul.robinson at sony.com 
> <mailto:paul.robinson at sony.com>> wrote:
>
>     The text in the committee draft is different (e.g., the
>     exhortation about non-default alignment is gone), with an example
>     to the effect that a value of 8 means the entity's address is a
>     multiple of 8 (not 2^8).  So, alignment is conceived in terms of
>     address bits, whatever those represent (not always bytes).
>
> Not sure I quite follow. OK, so in an octet addressable world (which 
> LLVM is - there have been some attempts to support non-octet 
> addressing, but I don't think any have been near to successful) then 
> DW_AT_alignment is byte alignment (1 means there are no zero bits in 
> the address, 2 means there's 1 trailing zero bit in the address, etc).
>
>     If Clang is being infested with more target knowledge, okay, but
>     that means tolerating the weirder targets in these cases. 
>     Dividing by CHAR_BITS makes an assumption that isn't necessarily
>     correct.
>
> Clang has the knowledge already - it knows the alignment of the types 
> its allocating, etc. So I'm not sure what infestation you're referring to.
>
> I've sort of lost track of what we're discussing here.
>
> Essentially what I'm suggesting is that Clang should put whatever 
> number is going to go in the DWARF, into the metadata. I don't believe 
> the LLVM backends have greater knowledge than the frontend does in 
> this domain - have I missed something there, are there examples where 
> that could/would be true?
>
> - David
>
>     --paulr
>
>     P.S. The committee is hoping to get a draft out for public comment
>     Real Soon Now.
>
> Looking forward to it :)
>
>     *From:*cfe-commits [mailto:cfe-commits-bounces at lists.llvm.org
>     <mailto:cfe-commits-bounces at lists.llvm.org>] *On Behalf Of *David
>     Blaikie via cfe-commits
>     *Sent:* Monday, September 12, 2016 5:12 PM
>     *To:* reviews+D24426+public+6ee6274d38fdf0d6 at reviews.llvm.org
>     <mailto:reviews%2BD24426%2Bpublic%2B6ee6274d38fdf0d6 at reviews.llvm.org>;
>     vleschuk at accesssoftek.com <mailto:vleschuk at accesssoftek.com>;
>     echristo at gmail.com <mailto:echristo at gmail.com>; aprantl at apple.com
>     <mailto:aprantl at apple.com>; mehdi.amini at apple.com
>     <mailto:mehdi.amini at apple.com>
>     *Cc:* cfe-commits at lists.llvm.org <mailto:cfe-commits at lists.llvm.org>
>     *Subject:* Re: [PATCH] D24426: DebugInfo: Pass non-zero alignment
>     to DIBuilder only if aligment was forced
>
>     On Mon, Sep 12, 2016 at 5:00 PM Paul Robinson
>     <paul.robinson at sony.com <mailto:paul.robinson at sony.com>> wrote:
>
>         probinson added a subscriber: probinson.
>
>         ================
>         Comment at: lib/CodeGen/CGDebugInfo.cpp:3691
>         @@ -3635,1 +3690,3 @@
>         +  if (D->hasAttr<AlignedAttr>())
>         +    AlignInBits = D->getMaxAlignment();
>            StringRef DeclName, LinkageName;
>         ----------------
>         dblaikie wrote:
>         > is max alignment the right thing here? Should it be min
>         alignment?
>         > (is alignment in bits the desired thing across all of this
>         too? It looked like in the backend patch there was some
>         division by CHAR_BITS, etc?)
>         I should think bits is the right choice here; seems more the
>         province of the backend to convert it into the appropriate
>         addressable units (commonly but not universally chars).
>
>
>     The alternative thinking is that we've a generally sense we want
>     to make more of this type information opaque to LLVM - so I'm
>     somewhat inclined to make the frontend do the work of choosing
>     what to emit and the backend just being as simple as possible.
>
>     Hmm, seems like the DWARF spec details I can find:
>     http://www.dwarfstd.org/ShowIssue.php?issue=140528.1 don't really
>     specify what the value of DW_AT_alignment is, it's sort of
>     assumed, by the looks of it? I'm assuming it's bytes, the same as
>     the byte_size attribute.
>
>
>
>
>         https://reviews.llvm.org/D24426
>

-- 
Best Regards,
Victor

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20160913/cc2132f4/attachment-0001.html>