[llvm-commits] PATCH: BasicBlock metadata

[Richard Osborne]

On 08/05/12 09:22, Ralf Karrenberg wrote:
> Hi,
>
> please find attached a patch that adds metadata functionality to BasicBlocks.
> It should include all that is required for full support via BitcodeReader/Writer, AsmParser/Writer, etc.
>
> Essentially, the patch mimics behaviour of metadata that is attached to instructions.
>
> The chosen syntax for block metadata is as follows:
>
> entry:
>      %property1 !0, %property2 !1
>    %x = fadd float %a, %b
>    ret float %x
>
> Looking forward to getting feedback on the patch:).
Some general comments - why do you need special handling / caching of 
debug loc metadata? Unless the format of debug information is changed 
debug metadata will only be added to instructions and so this code is 
unnecessary. You should add some testcases to cover the new 
functionality and modify the language ref to include a description of 
basic block metadata. Finally the indentation is incorrect in a number 
of places. Please use 2 spaces for indentation and avoid tabs.
> Cheers,
> Ralf
>     /// hasAddressTaken - returns true if there are any uses of this basic block
>     /// other than direct branches, switches, etc. to it.
> -  bool hasAddressTaken() const { return getSubclassDataFromValue() != 0; }
> +  bool hasAddressTaken() const {
> +    return (getSubclassDataFromValue()&  ~HasMetadataBit) != 0;
> +  }
I think this will result in basic block metadata effecting 
optimizations. What would happen if you just ignored metadata here?

-- 
Richard Osborne | XMOS
http://www.xmos.com

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